For /C++/ constructor initializers ExprEngine:computeUnderConstruction() asserts that they are all member initializers. This is not neccessarily true when this function is used to get the return value for the construction context thus attempts to fetch return values of base and delegating constructor initializers result in assertions. This small patch fixes this issue.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Shouldn't we create a new test care for this, instead of expanding an existing one? Btw, this looks great, but I lack the confidence to accept.
Why should we? This is just a fix for cases not covered, but it is the same functionality (retrieving the return value under construction). I added the missed cases to the test of this exact functionality.
I think its a bad experience if you break something while developing. Instead of getting a test failure for "delegating constructor initializers", you'll have to deal with a test that handles a variety of things at once, and are forced to tease it apart to find what just broke. When the introduced assert fires, this wouldn't be an issue, but in any non-crashing case it might be.
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | ||
---|---|---|
138–139 | For base initializer you probably want the base class region. It may have a non-trivial offset and it also has the correct type and extent. |
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp | ||
---|---|---|
138–139 | Thank you for noticing this! You are completely right! Now I extended the tests with comparing the type of the returned region with the type of the expression. It failed when I just returned ThisVal but it passes with the fixed code. |
For base initializer you probably want the base class region. It may have a non-trivial offset and it also has the correct type and extent.