GitLab

Fix test_codecompletion, test_duchain-clang, test_problems and test_files-clang failing on newer Ubuntu

* disable copying and moving of DynamicItem and rely on C++17's
  mandatory elision of copy/move operations;
* improve const correctness;
* make the constructor explicit (this should be the default);
* uint type alias => unsigned to match DUChainReferenceCounting's type;
* m_item: public => private.

The old DUChainReferenceCounting implementation is incorrect in many
places. But my assertions and tests in
kdevelop/kdevelop!190 indicate
that the wrong code was simply never executed even with global rather
than thread_local data. All overlapping ranges were always completely
equal (both the start and the size). The buggy range merging branch was
never taken.

I haven't encountered more than 2 different ranges at a time with
thread_local duchainReferenceCounting. So there is no need for a QMap or
a boost::icl::interval_map in DUChainReferenceCounting class. A simple
unsorted array of ranges should be perfect. The array size of 3 should
be safe enough.

Use a simple statically allocated array rather than QVector or
std::vector to safely eliminate a long-standing intentional memory leak
(new QMap without a matching delete).

Add a second parameter - unsigned size - to
disableDUChainReferenceCounting() in order to support nested reference
counting enabling/disabling with equal start but different size values.
While I haven't encountered any different overlapping requests,
supporting them doesn't significantly complicate the code. So why not?

Clean up DUChainReferenceCounting code:
* remove almost all reinterpret_cast uses;
* improve const correctness;
* char* => std::byte*;
* remove "DUChainReferenceCounting" suffixes from member function names.

Even with DUChainReferenceCounting::maxIntervalCount = 2, this change
does not break any kdevelop, kdev-python, kdev-php, kdev-ruby, kdev-css,
kdev-upload, kdev-verapp, kdev-krazy2, kdev-valgrind or kdev-xdebug
tests. No other maintained KDevelop plugin includes referencecounting.h
even indirectly, that is, none of the remaining maintained plugins
includes duchain, serialization or language headers.

This implementation simplification substantially speeds up
BenchItemRepository::shouldDoReferenceCounting() with both data rows.
But for some reason it slightly slows down
BenchIndexedString::bench_destroy(). The following table compares
performance of the previous, current and considered alternative
implementations in the affected benchmarks. The numbers denote
milliseconds per iteration.

version\benchmark       qhash   create  destroy shouldDo(-) shouldDo(+)
previous commit         1.2     149     103     212         318
inline f-l static       1.2     149     107     106         196
count == 0 -> false     1.2     148     106     106         212
Q_LIKELY(count == 0)    1.2     147     107     106         282
extern variable         1.4     153     128     636         671
inline variable         1.2     149     100     318         388
* return count != 0;    1.1     148     102     106         106
* return false;         0.87    141      67      70         <test fails>

        Legend
Benchmarks:
qhash       - BenchIndexedString::bench_qhashIndexedString()
create      - BenchIndexedString::bench_create()
destroy     - BenchIndexedString::bench_destroy()
shouldDo(-) - BenchItemRepository::shouldDoReferenceCounting(disabled)
shouldDo(+) - BenchItemRepository::shouldDoReferenceCounting(enabled)

Versions:
previous commit         - the version just before this commit
inline f-l static       - this commit
count == 0 -> false     - this commit with `if (count==0) return false;`
                          inserted at the beginning of
                          DUChainReferenceCounting::shouldDo()
Q_LIKELY(count == 0)    - same as "count == 0 -> false", but with
                          `Q_LIKELY` added in: `Q_LIKELY(count==0)`
extern variable         - this commit, but with extern thread_local
                          variable in the header instead of instance()
inline variable         - this commit, but with inline thread_local
                          variable in the header instead of instance()
* <code>                - this commit with <code> as a one-line wrong
                          implementation of
                          DUChainReferenceCounting::shouldDo()

The versions marked with '*' are incorrect. The table includes them to
show how much DUChainReferenceCounting affects each benchmark's speed.

Removing `Q_DISABLE_COPY_MOVE(DUChainReferenceCounting)` and
`DUChainReferenceCounting() = default;` has no effect on the benchmarks.

The 52216a13 commit message contains the
results of these same benchmarks at other versions of the code.

Each number in this and the older commit message's table is the minimum,
not the average, value of milliseconds per iteration after at least 10
runs of each benchmark. These minimum values were very consistent,
repeated many times. Usually the minimum value was also the most
frequent value. So even the slightest differences in performance are
unlikely to be caused by random fluctuations. Though a few tiny
variations are so strange that they most likely *are* spurious.

The two "count == 0" optimization attempts slightly speed up creating
and destroying IndexedString but significantly slow down the synthetic
shouldDoReferenceCounting(enabled) benchmark.

The "inline variable" version somewhat speeds up destroying
IndexedString but dramatically slows down both data rows of the
synthetic shouldDoReferenceCounting() benchmark. This inconsistent
effect on performance is surprising. As is the fact that the
"extern variable" version is by far the slowest across the board...

In case of conflict in i18n, keep the version of the branch "ours"
To resolve a particular conflict, "git checkout --ours path/to/file.desktop"

In case of conflict in i18n, keep the version of the branch "ours"
To resolve a particular conflict, "git checkout --ours path/to/file.desktop"

Sadly, the order in which system and project includes are handled
is not as flexible as I hoped - system includes will always be handled
after the non-system includes.

Then, we try to auto-deduce system includes instead of relying on the
project manager for that distinction, which may lead to changed order
of include paths. In the kdevelop code base e.g. we often will show
parse errors for the debug categories, because the wrong `debug.h`
file is seen. Now this is fixed as the build dir includes are not
converted into system includes anymore and thus the order is correct.

Use a timer with a lambda instead of trying (and failing) to call
a non-slot via QMetaObject::invoke. While at it, cleanup the code
to not be so horrible.

BUG: 429962

When we use the cmake file API we have to run configure to reload the
model. In such a case, when we run configure to apply the new cmake
preferences, we can just run reloadModel directly. Otherwise, we'd
end up running configure twice.

BUG: 429605

We store the cmake build type also in the kdevelo project settings
in order to recreate the build dir on demand if it gets wiped.
But when the user now manually changes it through the cmake cache
value for an existing build dir, the values could get out of sync.
Prevent that by keeping the two values in sync, i.e. once in the
list of cache values and once in the project setting drop down.

BUG: 429605

MSVC intentionally does not support DLL import/export of thread_local
variables. The result is the following compilation error:
C:\CI\Job Build\kdevplatform\serialization\referencecounting.h(57): error C2492:
'duchainReferenceCounting': data with thread storage duration may not have dll interface

In addition, this inline function-local static version unexpectedly
turned out to be much faster than the previous exported extern variable
version. The following table compares performance of the past, current
and considered alternative implementations in the affected benchmarks.
The numbers denote milliseconds per iteration.

version\benchmark       qhash   create  destroy shouldDo(-) shouldDo(+)
no thread_local         0.67    136     50      70          3146
bool thread_local       1.0     144     85      247         3294
exported thread_local   1.4     151     121     636         707
internal linkage        1.5     151     131     883         1025
non-inline f-l static   1.5     152     138     954         1060
inline variable         1.2     147     107     388         459
inline f-l static       1.2     149     103     212         318

        Legend
Benchmarks:
qhash       - BenchIndexedString::bench_qhashIndexedString()
create      - BenchIndexedString::bench_create()
destroy     - BenchIndexedString::bench_destroy()
shouldDo(-) - BenchItemRepository::shouldDoReferenceCounting(disabled)
shouldDo(+) - BenchItemRepository::shouldDoReferenceCounting(enabled)

Versions:
no thread_local         - 8fca91a0
bool thread_local       - db67d592
exported thread_local   - the version just before this commit
internal linkage        - static thread_local variable in the cpp file
non-inline f-l static   - instance() defined in the cpp file
inline variable         - inline thread_local variable in the header
inline f-l static       - this commit

In case of conflict in i18n, keep the version of the branch "ours"
To resolve a particular conflict, "git checkout --ours path/to/file.desktop"

In case of conflict in i18n, keep the version of the branch "ours"
To resolve a particular conflict, "git checkout --ours path/to/file.desktop"

Among other things this ensures that we don't load plugins that
we don't need in some of our tests, as we no longer check the
user's configuration for which plugin is enabled.

This fixes an UB cast when we call this on a ClassFunctionDeclaration
with setDeclarationIsDefinition(true) as shown in this UBSAN report:

```
/home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/functiondefinition.cpp:93:49: runtime error: downcast of address 0x6060005c72c0 which does not point to an object of type 'FunctionDefinition'
0x6060005c72c0: note: object is of type 'KDevelop::ClassFunctionDeclaration'
 26 02 00 76  f0 06 b1 d3 05 7f 00 00  a0 64 21 00 b0 60 00 00  50 6e 22 00 20 60 00 00  10 2b 01 01
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'KDevelop::ClassFunctionDeclaration'
    #0 0x7f05d2c5377f in KDevelop::FunctionDefinition::definition(KDevelop::Declaration const*) /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/functiondefinition.cpp:93
    #1 0x7f05dddae858 in accept /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/plugins/clang/duchain/documentfinderhelpers.cpp:63
    #2 0x7f05d2f7e0c2 in KDevelop::DUChainUtils::collectItems(KDevelop::DUContext*, KDevelop::DUChainUtils::DUChainItemFilter&) /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/duchainutils.cpp:422
    #3 0x7f05d2f7e4ed in KDevelop::DUChainUtils::collectItems(KDevelop::DUContext*, KDevelop::DUChainUtils::DUChainItemFilter&) /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/duchainutils.cpp:432
    #4 0x7f05dddacb36 in duchainBuddyFile /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/plugins/clang/duchain/documentfinderhelpers.cpp:136
    #5 0x7f05dddb2fcc in DocumentFinderHelpers::potentialBuddies(QUrl const&, bool) /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/plugins/clang/duchain/documentfinderhelpers.cpp:256
    #6 0x7f05dddb3a5d in DocumentFinderHelpers::sourceForHeader(QString const&) /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/plugins/clang/duchain/documentfinderhelpers.cpp:272
    #7 0x7f05dda0a7d5 in ClangRefactoring::moveIntoSource(KDevelop::IndexedDeclaration const&) /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/plugins/clang/codegen/clangrefactoring.cpp:158
    #8 0x563aae0570b3 in TestAssistants::testMoveIntoSource() /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/plugins/clang/tests/test_assistants.cpp:686
    #9 0x563aae01da1f in TestAssistants::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) plugins/clang/tests/test_assistants_autogen/EWIEGA46WW/moc_test_assistants.cpp:127
    #10 0x7f05c7f8458a in QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const (/usr/lib/libQt5Core.so.5+0x2c458a)
    #11 0x7f05df029e96  (/usr/lib/libQt5Test.so.5+0x18e96)
    #12 0x7f05df02a750  (/usr/lib/libQt5Test.so.5+0x19750)
    #13 0x7f05df02ad03  (/usr/lib/libQt5Test.so.5+0x19d03)
    #14 0x7f05df02b1cd in QTest::qRun() (/usr/lib/libQt5Test.so.5+0x1a1cd)
    #15 0x7f05df02b57d in QTest::qExec(QObject*, int, char**) (/usr/lib/libQt5Test.so.5+0x1a57d)
    #16 0x563aae03e824 in main /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/plugins/clang/tests/test_assistants.cpp:61
    #17 0x7f05c6e72151 in __libc_start_main (/usr/lib/libc.so.6+0x28151)
    #18 0x563aae01d1bd in _start (/home/milian/projects/kf5/build-dbg/extragear/kdevelop/kdevelop-sanitized/bin/test_assistants+0xe41bd)
```

Fixes memory leak reported by LSAN:

```
Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x7fc27ed0df41 in operator new(unsigned long) /build/gcc/src/gcc/libsanitizer/asan/asan_new_delete.cpp:99
    #1 0x555bcde300f3 in void KDevelop::TypeSystem::registerTypeClass<QmlJS::FunctionType, KDevelop::MergeIdentifiedType<KDevelop::FunctionType>::Data>() /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/kdevplatform/language/duchain/types/typeregister.h:144
    #2 0x555bcde300f3 in QmlJS::registerDUChainItems() /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/plugins/qmljs/duchain/helper.cpp:307
```

Don't cast partially destroyed object, fixes UBSAN report:

```
/home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/kdevplatform/language/highlighting/codehighlighting.cpp:612:21: runtime error: downcast of address 0x608000240320 which does not point to an object of type 'DocumentChangeTracker'
0x608000240320: note: object is of type 'QObject'
 14 01 80 2f  e0 84 05 4b 5d 7f 00 00  a0 03 24 00 80 60 00 00  10 ee 11 00 20 60 00 00  01 be be be
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'QObject'
    #0 0x7f5d566f5920 in KDevelop::CodeHighlighting::trackerDestroyed(QObject*) /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/kdevplatform/language/highlighting/codehighlighting.cpp:612
```

Order isn't important for these maps, use the faster hash instead.

Fixes various LSAN reports

Only connect to qApp in init and disconnect in cleanup.
Then don't call cleanup in NoUi tests.

Fixes UBSAN report:
```
/home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/kdevplatform/shell/uicontroller.cpp:241:16: runtime error: downcast of address 0x7f30c868a1c0 which does not point to an object of type 'QApplication'
0x7f30c868a1c0: note: object is of type 'QCoreApplication'
 00 00 00 00  f0 4f 59 de 30 7f 00 00  e0 55 02 00 e0 60 00 00  00 00 00 00 00 00 00 00  00 00 00 00
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'QCoreApplication'
    #0 0x7f30eeacada3 in KDevelop::UiController::~UiController() /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/kdevplatform/shell/uicontroller.cpp:241
    #1 0x7f30eeacb874 in KDevelop::UiController::~UiController() /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/kdevplatform/shell/uicontroller.cpp:242
    #2 0x7f30eea745d3 in KDevelop::CorePrivate::~CorePrivate() /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/kdevplatform/shell/core.cpp:279
    #3 0x7f30eea7f4be in KDevelop::Core::~Core() /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/kdevplatform/shell/core.cpp:330
    #4 0x7f30f1250435 in KDevelop::TestCore::~TestCore() kdevplatform/tests/KDevPlatformTests_autogen/EWIEGA46WW/../../../../../../../../src/extragear/kdevelop/kdevelop/kdevplatform/tests/testcore.h:75
    #5 0x7f30f1250435 in KDevelop::TestCore::~TestCore() kdevplatform/tests/KDevPlatformTests_autogen/EWIEGA46WW/../../../../../../../../src/extragear/kdevelop/kdevelop/kdevplatform/tests/testcore.h:75
    #6 0x7f30de32a33f in QObject::event(QEvent*) (/usr/lib/libQt5Core.so.5+0x2e233f)
    #7 0x7f30de2fda4f in QCoreApplication::notifyInternal2(QObject*, QEvent*) (/usr/lib/libQt5Core.so.5+0x2b5a4f)
    #8 0x7f30de300572 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (/usr/lib/libQt5Core.so.5+0x2b8572)
    #9 0x7f30de3522dd in QTest::qWait(int) (/usr/lib/libQt5Core.so.5+0x30a2dd)
    #10 0x7f30f1279cef in KDevelop::TestCore::shutdown() /home/milian/projects/kf5/src/extragear/kdevelop/kdevelop/kdevplatform/tests/testcore.cpp:99
```

It's used in debug builds

When we get a parse job with the ForceUpdate flag set, we used to
apply this flag to all imports. So basically ForceUpdateRecursive
and ForceUpdate where handled in the same way, which is obviously
not a good idea.

Instead, track the document for which the parse job got created and
only apply ForceUpdate to that one.

This greatly improves the performance of reparsing after switching
git branches or using git stash: These actions would always trigger
a document reload and a parse job is created with ForceUpdate set.
Previously, this then meant that we reparsed all imports too. For
files with many imports, this was extremely slow and totally unneeded.

As an example: Editing heaptrack's accumulatedtracedata.cpp and then
applying `git stash` tackes ~34s on my machine before this patch. With
this patch, it only takes ~200ms instead.

The previous code only ever tracked the the modification revision
of a file itself, but not of the files it imported. Now, we actually
do that, which ensures we automatically reparse a file when one of
its imports has changed.

The path is to a file, not to a dir, so it's pretty odd that we
used QDir here.

Missing QLatin1String(...) wrappers

BUG: 429815

GIT_SILENT