Page MenuHomePhabricator

[analyzer] Unbreak body farms in presence of multiple declarations.
ClosedPublic

Authored by NoQ on Apr 18 2019, 6:07 PM.

Details

Summary

Split out of D60808.

When growing a body on a body farm, it's essential to use the same redeclaration of the function that's going to be used during analysis. Otherwise our ParmVarDecls won't match the ones that are used to identify argument regions. This boils down to trusting the reasoning in AnalysisDeclContext. We shouldn't canonicalize the declaration before farming the body because it makes us not obey the sophisticated decision-making process of AnalysisDeclContext.

Diff Detail

Repository
rL LLVM

Event Timeline

NoQ created this revision.Apr 18 2019, 6:07 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 18 2019, 6:07 PM
a_sidorin accepted this revision.Apr 20 2019, 3:03 AM

Hi Artem,
This looks good to me. However, it seems to me that the correct solution is to synthesize a shining new function and include it into the redeclaration chain. This requires much more work, however.

This revision is now accepted and ready to land.Apr 20 2019, 3:03 AM
NoQ added a comment.Apr 22 2019, 4:07 PM

the correct solution is to synthesize a shining new function and include it into the redeclaration chain.

Mmm, what are the pros and cons?

Mmm, what are the pros and cons?

This is how we do it in the ASTImporter. It allows us to correctly handle redeclarations with and without bodies - the declarations in the current AST remain unchanged but a new declaration appears. But it can be a kind of a professional deformation :) BodyFarm is a much lighter facility and I won't argue that my approach is better - just an idea.

NoQ added a comment.Apr 22 2019, 7:53 PM

With body farms, i think, the current declaration also remains unchanged, we're just getting a shining new CompoundStmt as a body in the middle of nowhere, i.e., ADC->getDecl()->getBody() isn't the same as ADC->getBody(). Which is, yeah, still weird :)

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 22 2019, 7:53 PM