martinboehme wrote:

@haoNoQ Thank you for going to the trouble to explain all of this in detail!

> I'm very open to changes in `ConstructionContext` to accommodate your needs, 
> like provide extra information that we didn't need but you did, or provide it 
> in a different way.

I haven't looked in detail yet, but I think all of the information we need is 
there.

The main thing I'm wondering is whether we can make it so there are fewer case 
distinctions that the code in `FlowSensitive` would need to perform -- i.e. so 
that we wouldn't need to handle all of the possible cases in 
`ConstructionContext::Kind` individually.

As far as I can tell, the various "intermediate" classes in the class hierarchy 
look as if they provide a mechanism for this. I.e. instead of distinguishing 
between all of the possible leaf classes, we would merely handle 
`VariableConstructionContext`, `ConstructorInitializerConstructionContext`, 
`TemporaryObjectConstructionContext`, and `ReturnedValueConstructionContext`. I 
_think_ that for our purposes (i.e. identifying the result object that a record 
prvalue initializes), these intermediate classes should provide all of the 
information we need. (I believe the finer-grained information is only necessary 
for the additional analysis that you do, e.g. for temporary destructors, as you 
explain.) Does this sound right?

If you agree that this works, then the main question to me is how we make sure 
that we make sure we update our code if more cases are added to 
`ConstructionContext`. This is easy to do with a switch-case over 
`ConstructionContext::Kind`, as the compiler will warn if any of the cases are 
not handled, but we don't get any such warning when doing a series of 
`dyn_cast`s over the various "intermediate" classes in the hierarchy (i.e. 
`VariableConstructionContext` and so on). We could, of course, still use a 
switch case but then cast to the appropriate intermediate class. I see that you 
also do in this in some places in the code, e.g. in 
[ExprEngineCXX.cpp](https://github.com/llvm/llvm-project/blob/86f7374078288e2b3d3d0fd66428f7752e2319e6/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp#L132).
 Is this the approach that you would recommend?


https://github.com/llvm/llvm-project/pull/99616
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to