- improve assertion with side effect check

[theKBro]

• add more tests

[chrchr-github]

This still requires approval.

[theKBro]

Yes, do you know how to proceed here?

[chrchr-github]

Make noise until @danmar notices it? 😜

[theKBro]

Scream out loud This PR fixes some false positive assert with side effect detections! But it also comes with open points. If a qualified maintainer would have a look at this, would be asewome!

[chrchr-github]

Let's mention @danmar again...

[danmar]

Sorry my review took so long.

Please look at the CI failures. For instance:

lib/checkassert.h:69:20: error: 'vector' in namespace 'std' does not name a template type
         const std::vector<const Token *> *arguments;
                    ^~~~~~

[theKBro]

Sorry my review took so long.

Please look at the CI failures. For instance:

lib/checkassert.h:69:20: error: 'vector' in namespace 'std' does not name a template type
         const std::vector<const Token *> *arguments;
                    ^~~~~~

Also sry from my side, I was on vacation. Oddly enough, the compilation worked over here with visual studio toolchain but hopefully is now fixed for cygwin.

[theKBro]

@danmar could you please have a look at my questions?

[danmar]

could you please have a look at my questions?

sorry can you clarify which questions?

[theKBro]

I wrote some tests to show some defects that are not detected. How are you handling this kind of stuff? Should I just comment out the tests and hope that in the future somebody will improve the code even more or how is the correct procedure? These are the currently failing tests. The questions are part of the review.

[danmar]

Should I just comment out the tests and hope that in the future somebody will improve the code even more or how is the correct procedure?

If the check fails to detect some problems then please feel free to use the TODO_ASSERT_EQUALS macro.

[theKBro]

Thanks a lot! Thats what I was missing!

[theKBro]

Hey @danmar. I haven't changed a Script at all and don't really know what the Scriptcheck is doing. Could you please help fixing this (hopefully) last issue?

[chrchr-github]

Hey @danmar. I haven't changed a Script at all and don't really know what the Scriptcheck is doing. Could you please help fixing this (hopefully) last issue?

Have you maybe added an include or something? The output from the failing script shows you how to modify the Makefile.

[danmar]

It's almost good to merge this. do you think you might finish this?

[theKBro]

I think that the way that I used the Scope ( I don't know if it was already used this way before ) is completly wrong. And therefore this part is not good. Unfortunately I don't have the time right now to implement a better solution here. But as you stated this could maybe be replaced with any of these

bool isConstFunctionCall(const Token* ftok, const Library& library);
bool isConstExpression(const Token *tok, const Library& library, bool pure, bool cpp);
bool isWithoutSideEffects(bool cpp, const Token* tok, bool checkArrayAccess = false, bool checkReference = true);
bool isVariableChangedByFunctionCall(const Token *tok, int indirect, nonneg int varid, const Settings *settings, bool *inconclusive);
bool isVariableChanged(const Token *start, const Token *end, const nonneg int exprid, bool globalvar, const Settings *settings, bool cpp, int depth = 20);