ValueFlow: pass `ErrorLogger` by reference into `ValueFlow::setValues()` / removed need for `LifetimeStore::Context` by firewave · Pull Request #5299 · cppcheck-opensource/cppcheck
| ValuePtr<Analyzer> analyzer; | ||
| const TokenList& tokenList; | ||
| ErrorLogger* const errorLogger; | ||
| ErrorLogger& errorLogger; |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes the class not assignable. This should be a pointer.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That ship has already sailed - some one line above.
I will take a look at adding that clang-tidy check about this (there was a case of false positives which I am not sure is fixed yet) and there's also #4785 and some discussions which related to that (which apparently are not linked).
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI There also would be a compiler error if we would actually try to assign those types. I ran into this issue while working on it. So it is not a silent failure.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I encountered the first issue we have with this in some other changes. I will try to figure out how to properly handle it so it can be easily be applied in the future. Nothing to do here though since this doesn't change anything as we were using references before.
| mContext->errorLogger = errorLogger; | ||
| mContext->settings = &settings; | ||
| } | ||
| mutable Token* forwardTok{}; |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know about using the mutable keyword here.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me neither - but removing the const from the function had quite a ripple effect before - will check again. I think this might be a sensible use of mutable.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ripple effect wasn't as bad as I remember. The object was only const when passing it around so that was not fully enforced and is actually fine.