| Age | Commit message (Collapse) | Author |
|---|
|
When internal linkage is necessary, anonymous namespaces fulfil the same
purpose in a better way. See also:
https://stackoverflow.com/questions/4422507/superiority-of-unnamed-namespace-over-static
|
|
It's better than const char* because any interaction between const char*
and QString assumes that const char* contains UTF-8, which is
pessimistic and therefore inefficient; at the same time:
- construction of QString from QLatin1String is extremely fast
(boiling down to padding null bytes)
- "something"_ls is much shorter than QStringLiteral("something")
- "something"_ls produces a direct pointer to the literal at compile
time, using the benefits of raw string literals (deduplication, e.g.)
The library API will also transition to QLatin1String where applicable,
likely in 0.8.
|
|
|
|
|
|
Notably, Quotient::AccountRegistry::instance() is now deprecated
in favour of Quotient::Accounts inline variable.
|
|
In reality it's often not just the return type but the return type and
the preceding attribute, in particular [[deprecated("...")]] - and
unfortunately this attribute (and any other, actually) cannot be given
its own line using ClangFormat. So at least try to glue those two to
the function name as much as possible...
|
|
- Templates and constexpr imply inline
- A function called from a single site better be inlined.
|
|
Although parented to Connection, SsoSession was pretty leaky in that
unsuccessful login attempts didn't delete the object and in some errors
didn't even close the local HTTP socket (though new connections would no
more be accepted). Also, without the documentation it wasn't clear who
owns the object returned by Connection::prepareForSso(). Now it is.
Unfortunately, it's not easy to cover SsoSession with tests. Basically,
it takes a homeserver and a mock "SSO agent" that would check
the SSO URL for validity and then both send the login authorisation
to the homeserver as well as ping the callback given by SsoSession.
Maybe for another time.
|
|
Use 'auto'; range-for instead of an iterator loop.
|
|
|
|
|
|
It doesn't need all those things inside - order_type alias is no more
in use; operator<() is better outside; QLatin1String is better to
compare against than const char* (because const char* is assumed to be
UTF-8); and TagRecord is really small so it doesn't need const& for
parameters.
|
|
Those are already inherited with 'using'.
|
|
Older CMake versions fail if they don't find those (LGTM uses CMake 3.13
and that one does, at least).
|
|
ImplPtr
|
|
With the previous settings clang-format seemed too lenient to characters
beyond position 80.
[skip ci]
|
|
|
|
|
|
|
|
The original (more complex and comprehensive) solution belongs to
https://oliora.github.io/2015/12/29/pimpl-and-rule-of-zero.html - this
commit only provides a small wrapper for non-copyable Private class
implementations common throughout libQuotient. Unlike the original,
default initialisation is made explicit - you have to pass
ZeroImpl<Private>() instead (and I firmly believe it's a good thing:
normally pointers to Private should not remain nullptr). The reason
ZeroImpl<> is not a template variable is quite simple: unique_ptr is
non-copyable and so cannot be initialised from; while a template
function will initialise the value in-place thanks to copy elision.
|
|
|
|
|
|
On Windows and macOS, the assumed practice is to package things up in
a monolithic distribution package. On Linux, fine-grained library
packages are installed.
After adding CMAKE_INSTALL_RPATH_USE_LINK_PATH dynamic linkage is
testable on macOS but that's not the "proper" way to install things on
the platform, anyway. It also probably works on Windows but I couldn't
get a workable setup for quotest after a few attempts. Perhaps it's
a matter of running windeployqt instead of trying to figure out and
add to PATH all the locations needed; or maybe not; I don't have the
motivation to explore this further.
|
|
Those DESTINATION specifications match precisely what CMake does by
default (on Linux at least); what's worse is that they prevent CMake
to install the DLL file on Windows.
|
|
|
|
On Windows QUOTIENT_API expands to different things depending on whether
the library is built or used. This results in confusing statements (and
MSVC erroring out on them, in some cases - see below - quite
legitimately) not only when the application includes Quotient headers
but also when the application defines custom events and uses
REGISTER_EVENT_TYPE to make them creatable from /sync responses.
To avoid repeated registration when dynamic linking is involved,
EventFactory<>::addMethod() now bluntly looks up the method for this
type in the vector of already registered methods. It would surely be
quicker to use a static variable instead; but since the refreshed API
for addMethod returns a reference to the factory method it's necessary
to do this lookup anyway. Once the primary goal of this branch is
achieved across platforms I might experiment with lighter ways to
register factory methods; for now here's a minimal change to make
the code build on Windows.
|
|
Thanks to Sonar for reminding that constexpr implies inline.
|
|
Once visibility kicks in, MSVC changes its ways and tries to instantiate
Private classes wrapped in smart pointers upon their occurence in the
header file - which leads to build breakage because of a missing
destructor. Usually making the outer class destructor out-of-line helps
to fix this (see RoomEvent, for one example).
|
|
MSVC is quite picky to redeclaration with __declspec(dllexport), judging
it as changing the class of storage. This commit tries to reorder
declarations so that MSVC is made aware of dllexport attribute on the
first encounter rather than the second one.
|
|
|
|
|
|
|
|
Get LGTM working again
|
|
Unfortunately LGTM still sits on Ubuntu 19.10 (not even LTS) which
really limits the choice of compilers: the newest GCC is of version 9
and the newest Clang seems to be version 9 as well - both are quite old
and giving when it comes to modern features support. The latest error
is GCC 9 tripping over the assignment of the function specialisation to
std::function. Clang 9 doesn't and otherwise seems fine.
|
|
|
|
Pinned message support
|
|
`Room::Change` has been changed to be an enum class recently; and it's values are no more suffixed with `Change`.
|
|
|
|
Before all, this fixes the problem with double-initialising of type ids;
it could have been fixed with a smaller change but EventTypeRegistry
is fairly superfluous now when inline variables are a thing and it's
possible to have an extensible registry system using literally pointers
to the memory that are guaranteed to be unique. That being said,
event_type_t is still QLatin1String and not a bare const char* (or
void*), mostly to stay on the safe side when it comes to type
identities: unlike const char*, QLatin1String's are deep-compared,
meaning that matching for switchOnType (former visit) occurs a bit
slower now. This may change in the future; but this is the first step
in getting rid of EventTypeRegistry.
This change means that initializeTypeId is no more needed; also, two
static member functions, typeId() and matrixTypeId(), are being replaced
with a single inline static member variable, TypeId. This commit doesn't
apply that transition across the event types, meaning that you'll get
a pile of warnings when compiling the library. These warnings will be
tackled in further commits within this branch.
|
|
For now on Linux with GCC only, with a plan to add Windows eventually.
|
|
Also, -fvisibility-inlines-hidden is applied in a CMake-native way now.
As can be expected, BUILDING_SHARED_QUOTIENT is set when a dynamic
library is built while QUOTIENT_STATIC is set whenever static
libQuotient is around (both for building it and for building with it).
|
|
Strictly speaking, EventFactory can be further instantiated if any
client application figures they need a whole new base class for events
and respectively a separate EventFactory specialisation for it.
Where this whole commit started though was a linkage error because I
did not plan to expose Quotient-specific logging categories for linkage
(effectively, usage) from the client code - meanwhile the inline code
of EventFactory uses qDebug(EVENTS), meaning I had to either add
QUOTIENT_API to EVENTS or hide those invocations. This in turn led
to trimming the EventFactory constructor back to trivial implementation
and dropping the guard variable that was supposed to trace duplicate
EventFactory<BaseEventT> objects for the same BaseEventT - with the
reasoning that such situation is not really dangerous (unlike
EventTypeRegistry double-initialisation fiasco, see #413), and at the
same time it can be easily detected in the logs by duplicated factory
method registration messages. And while I was at it, I replaced the
meaningless bool in the return type of EventFactory<>::addMethod with
the slightly more (but still barely) useful reference to the inserted
factory method. One can (in theory) use it now if they need to turn
some event JSON into an object of some specific event type or nullptr
if the event type in the JSON payload doesn't match - but at the same
rate (for now at least) one can call makeIfMatches<EventT>() directly.
With this commit, both Quotest and Quaternion build and link using
either Clang or GCC even under -fvisibility=hidden. However, running
quotest now reproduces #413, which is a matter of event typeId
infrastructure refactoring, coming in further commits.
|
|
|
|
|
|
This include all (hopefully) classes/structures and functions that have
non-inline definitions, as well as namespaces with Q_NAMESPACE since
those have non-inline (as of Qt 5.15) QMetaObject - for that a new
macro, QUO_NAMESPACE, has been devised to accommodate the lack of
Q_NAMESPACE_EXPORT in Qt before 5.14.
|
|
This seems to be the crux of #413.
|
|
Instead of using CMake's generate_export_header macro, it's a bit easier
to maintain a static file (that is not supposed to ever change) with
necessary export/import/hidden visibility macros.
|
|
Fixes a clang-tidy warning.
|
|
At some point macros were prepended with QTNT (pronounced "cute-n't",
"Quotient" with vowels dropped) but that didn't go very far. Having
forgotten about this, I introduced QUO prefix in a few places. Being
initial letters of "Quotient", QUO feels more understandable (and
coincidentally is a well-known Latin word); so let's unify on this.
|
|
Because that's what it really is.
|
