[09:01] <lubot_> [telegram] <RikMills> As a note, rebuilding clazy in a PPA the build time tests fail in the same fashion
[09:09] <lubot_> [telegram] <mitya57> No ideas about clazy, but I will upload fixed veusz today.
[10:05] <lubot_> [telegram] <RikMills> @tsimonq2 @mitya57 also same fail seen if I build current debian/ubuntu clazy on KDE Neon with their Qt 5.15.8
[10:06] <lubot_> [telegram] <RikMills> i.e. jammy base
[10:08] <lubot_> [telegram] <mitya57> According to test history, it _is_ related to Qt 5.15.8 somehow.
[10:08] <lubot_> [telegram] <mitya57> Maybe https://code.qt.io/cgit/qt/qtbase.git/commit/?id=747800675044bc1782b9bc9b97cea2cc2ef2c06f is somehow related?
[10:10] <lubot_> [telegram] <RikMills> Reported: https://bugs.kde.org/show_bug.cgi?id=464441
[10:12] <lubot_> [telegram] <mitya57> Thanks!
[10:41] <lubot_> [telegram] <RikMills> Would that imply that the test is defunct, or the test case used is at least? (re @mitya57: According to test history, it _is_ related to Qt 5.15.8 somehow.
[10:41] <lubot_> [telegram] <RikMills> Maybe https://code.qt.io/cgit/qt/qtbase.git/commit/?id=747800675044bc1782b9bc9b97cea2cc2ef2c06f is somehow related?)
[10:41] <lubot_> [telegram] <RikMills> c++ foo is failing me ATM
[10:42] <lubot_> [telegram] <mitya57> I think clazy no longer generates the warning it needs to generate, maybe because it was relying on some QVector implementation detail that changed now. But that's just a hypothesis.
[10:44] <lubot_> [telegram] <RikMills> ```
[10:44] <lubot_> [telegram] <RikMills> # reserve-candidates
[10:44] <lubot_> [telegram] <RikMills> 
[10:44] <lubot_> [telegram] <RikMills> 
[10:44] <lubot_> [telegram] <RikMills> Finds places that could use a `reserve()` call.
[10:44] <lubot_> [telegram] <RikMills> Whenever you know how many elements a container will hold you should reserve
[10:44] <lubot_> [telegram] <RikMills> space in order to avoid repeated memory allocations.
[10:44] <lubot_> [telegram] <RikMills> 
[10:44] <lubot_> [telegram] <RikMills> #### Trivial example missing reserve()
[10:44] <lubot_> [telegram] <RikMills> 
[10:44] <lubot_> [telegram] <RikMills>     QList<int> ages;
[10:44] <lubot_> [telegram] <RikMills>     // list.reserve(people.size());
[10:44] <lubot_> [telegram] <RikMills>     for (auto person : people)
[10:44] <lubot_> [telegram] <RikMills>         list << person.age();
[10:44] <lubot_> [telegram] <RikMills> 
[10:44] <lubot_> [telegram] <RikMills> Example where reserve shouldn't be used:
[10:44] <lubot_> [telegram] <RikMills> 
[10:44] <lubot_> [telegram] <RikMills>     QList<int> list;
[10:44] <lubot_> [telegram] <RikMills>     for (int i = 0; i < 1000; ++i) {
[10:44] <lubot_> [telegram] <RikMills>         // reserve() will be called 1000 times, meaning 1000 allocations
[10:44] <lubot_> [telegram] <RikMills>         // whilst without a reserve the internal exponential growth algorithm would do a better job
[10:44] <lubot_> [telegram] <RikMills>         list.reserve(list.size() + 2);
[10:44] <lubot_> [telegram] <RikMills>         for (int j = 0; j < 2; ++j) {
[10:44] <lubot_> [telegram] <RikMills>             list << m;
[10:44] <lubot_> [telegram] <RikMills>         }
[10:44] <lubot_> [telegram] <RikMills>     }
[10:44] <lubot_> [telegram] <RikMills> 
[10:44] <lubot_> [telegram] <RikMills> #### Supported containers
[10:44] <lubot_> [telegram] <RikMills> `QVector`, `std::vector`, `QList`, `QSet` and `QVarLengthArray`
[10:44] <lubot_> [telegram] <RikMills> 
[10:44] <lubot_> [telegram] <RikMills> #### Pitfalls
[10:44] <lubot_> [telegram] <RikMills> clazy/docs/checks/README-reserve-candidates.md
[11:03] <lubot_> [telegram] <RikMills> building qtbase with that commit reverted to test
[14:20] <lubot_> [telegram] <tsimonq2> I can concur, that's about as far as I got (re @mitya57: I think clazy no longer generates the warning it needs to generate, maybe because it was relying on some QVector implementation detail that changed now. But that's just a hypothesis.)
[14:26] <lubot_> [telegram] <RikMills> build time test still fails with that reverted in qtbase (re @RikMills: building qtbase with that commit reverted to test)
[14:33] <lubot_> [telegram] <tsimonq2> FWIW swapping that test to a QList didn't work either
[15:14] <lubot_> [telegram] <RikMills> as usual, not sure any other distros bother to run the tests 🙄
[19:43] <lubot_> [telegram] <tsimonq2> I figured it out
[19:43] <lubot_> [telegram] <tsimonq2> https://code.qt.io/cgit/qt/qtbase.git/commit/src/corelib?h=5.15&id=826e076ef53a5809dee7d887b16ed961895049ea
[19:43] <lubot_> [telegram] <tsimonq2> /src/checks/manuallevel/reserve-candidates.cpp:313
[19:58] <lubot_> [telegram] <RikMills> fix?
[20:17] <lubot_> [telegram] <tsimonq2> confirming that the fix is to remove the test
[20:39] <lubot_> [telegram] <mitya57> Removing the test doesn't sound right. Can you try replacing `foreach (int i2, d)` with `for (int i2 : d)`?
[20:42] <lubot_> [telegram] <mitya57> Or maybe even better idea. Try this please:
[20:42] <lubot_> [telegram] <mitya57> ```
[20:42] <lubot_> [telegram] <mitya57> --- a/src/checks/manuallevel/reserve-candidates.cpp
[20:42] <lubot_> [telegram] <mitya57> +++ b/src/checks/manuallevel/reserve-candidates.cpp
[20:42] <lubot_> [telegram] <mitya57> @@ -164,7 +164,7 @@ void ReserveCandidates::VisitStmt(clang::Stmt *stm)
[20:42] <lubot_> [telegram] <mitya57>      if (!body)
[20:42] <lubot_> [telegram] <mitya57>          return;
[20:42] <lubot_> [telegram] <mitya57>  
[20:42] <lubot_> [telegram] <mitya57> -    const bool isForeach = clazy::isInMacro(&m_astContext, clazy::getLocStart(stm), "Q_FOREACH");
[20:42] <lubot_> [telegram] <mitya57> +    const bool isForeach = clazy::isInMacro(&m_astContext, clazy::getLocStart(stm), "Q_FOREACH_IMPL");
[20:42] <lubot_> [telegram] <mitya57>  
[20:42] <lubot_> [telegram] <mitya57>      // If the body is another loop, we have nesting, ignore it now since the inner loops will be visited soon.
[20:42] <lubot_> [telegram] <mitya57>      if (isa<DoStmt>(body) || isa<WhileStmt>(body) || (!isForeach && isa<ForStmt>(body)))```
[20:43] <lubot_> [telegram] <tsimonq2> ack :)
[20:54] <lubot_> [telegram] <tsimonq2> This works, but for loops are already tested for (re @mitya57: Removing the test doesn't sound right. Can you try replacing foreach (int i2, d) with for (int i2 : d)?)
[20:54] <lubot_> [telegram] <tsimonq2> This does not work, unfortunately (re @mitya57: Or maybe even better idea. Try this please:
[20:54] <lubot_> [telegram] <tsimonq2> --- a/src/checks/manuallevel/reserve-candidates.cpp
[20:54] <lubot_> [telegram] <tsimonq2> +++ b/src/checks/manuallevel/reserve-candidates.cpp
[20:55] <lubot_> [telegram] <tsimonq2> @@ -164,7 +164,7 @@ void ReserveCandidates::VisitStmt(clang::Stmt *stm)
[20:55] <lubot_> [telegram] <tsimonq2>      if (!body)
[20:55] <lubot_> [telegram] <tsimonq2>          return;
[20:55] <lubot_> [telegram] <tsimonq2>  
[20:55] <lubot_> [telegram] <tsimonq2> -    const bool isForeach = clazy::isInMacro(&m_astContext, clazy::getLocStart(stm), "Q_FOREACH");
[20:55] <lubot_> [telegram] <tsimonq2> +    const bool isForeach = clazy::isInMacro(&m_astContext, clazy::getLocStart(stm), "Q_FOREACH_IMPL");
[20:55] <lubot_> [telegram] <tsimonq2>  
[20:55] <lubot_> [telegram] <tsimonq2>      // If the body is another loop, we have nesting, ignore it now since the inner loops will be visited soon.
[20:55] <lubot_> [telegram] <tsimonq2>      if (isa<DoStmt>(body) || isa<WhileStmt>(body) || (!isForeach && isa<ForStmt>(body))))
[20:57] <lubot_> [telegram] <mitya57> Ah, I see
[20:57] <lubot_> [telegram] <mitya57> ```
[20:57] <lubot_> [telegram] <mitya57> void rangeLoop()
[20:57] <lubot_> [telegram] <mitya57> {
[20:57] <lubot_> [telegram] <mitya57>     QVector<int> v1, v2;
[20:57] <lubot_> [telegram] <mitya57>     for (auto i : v1)
[20:57] <lubot_> [telegram] <mitya57>         v2.push_back(i);
[20:57] <lubot_> [telegram] <mitya57> }```
[20:57] <lubot_> [telegram] <mitya57>  (re @tsimonq2: This works, but for loops are already tested for)
[20:58] <lubot_> [telegram] <mitya57> OK, then I don't have better ideas, so maybe disabling is the only option. (re @tsimonq2: This does not work, unfortunately)
[20:59] <lubot_> [telegram] <mitya57> tests/reserve-candidates/config.json has `"minimum_qt_version" : 50300` for this test, maybe you can add `"maximum_qt_version" : 51507`.