[cfe-dev] Getting DecompositionDecl from BindingDecl
Artem Dergachev via cfe-dev
cfe-dev at lists.llvm.org
Mon Apr 2 18:04:08 PDT 2018
On 4/2/18 2:11 PM, Richard Smith via cfe-dev wrote:
> On 2 April 2018 at 13:52, George Karpenkov via cfe-dev
> <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
> Hi Richard,
> Thanks for your reply!
>> On Mar 30, 2018, at 8:49 PM, Richard Smith <richard at metafoo.co.uk
>> <mailto:richard at metafoo.co.uk>> wrote:
>> Right, sure, but I don’t have a convenient way to find that
>> DecompositionDecl from a given BindingDecl,
>> and sometimes I need to act based on the BindingDecl alone.
>> Can you give an example?
> Let’s say a structured binding is written into, and then some
> function is called, and then we read from that binding again.
> If it was global, we have to assume any function call can
> invalidate it, but if it’s local and it wasn’t passed as a parameter,
> chances are it will remain the same.
> I think that will just work if you expand references to BindingDecls
> into their binding expressions: both before and after, you'll end up
> evaluating an lvalue denoting the same subobject of the DecompositionDecl.
>> It was chosen because it matches the semantic model desired by
>> the designers of the feature. For example, you can't handle
>> bitfields if you model structured bindings as reference variables.
>> This was a somewhat controversial decision, but it doesn't look
>> like it's going to be reversed.
>>> That's what the CFG for the above function should represent.
>> Sorry, I’m not sure what do you mean here: from my
>> understanding, CFG does not transform AST inside statements
>> (apart from maybe tiny syntactic things).
>> IIRC, the CFG expands CXXDefaultArgExprs and CXXDefaultInitExprs;
>> this case is analogous to those.
Thank you for the explanation! I was also very confused.
We do expand CXXDefaultInitExprs for sema warnings CFG, but not for the
CFG analyzer. We don't expand CXXDefaultArgExprs yet.
I have a concern, which is probably invalid, about expanding expressions
that can appear more than once. So far in a lot of places (in the
analyzer in particular) we've been relying on every expression appearing
only once within a single CFG, which would no longer be the case if we
expand CXXDefaultArgExprs or DeclRefExprs-to-BindingDecls.
In particular, in the analyzer it's common to keep track of "the value
of the expression". We are unable to handle the situation when the same
expression, within the same call stack frame, in the same moment of
time, has two different values. For BindingDecl this shouldn't be a
problem because its expression will always have the same value. For
CXXDefaultArgExpr it seems to also not be a problem because it is always
surrounded by its respective function call, which is already the next
moment of time - and we won't expand the same CXXDefaultArgExpr twice in
the same call. Even if CXXDefaultArgExpr calls itself recursively, we're
not on the same stack frame anymore. Am i understanding correctly that
we're not ever going to be required to maintain two different values for
CXXDefaultArgExpr sub-expressions or for binding sub expressions? Is
there something that protects us from such situations, apart from
"that's where we are at the moment"?
The other problem i'm immediately seeing in the analyzer is to prevent
it from thinking that we've found an infinite loop when we visit the
same expression twice without any change in the state of the program
(which is a known but presumably relatively harmless bug in our current
CXXDefaultArgExpr handling). But that should be relatively easy to resolve.
There is also a certain amount of "unknown unknowns" here, because i've
no idea in how many other places we rely upon, or will need to rely upon
every expression appearing only once.
The alternative i've been considering for now was to provide "fake stack
frames" for evaluating such expandable expressions instead of expanding
them directly into the CFG. This would be an analyzer-side fix, but it
may be fine if other users of the CFG are fine with expanding.
It might be also possible to deep-copy the expressions when expanding.
But in case of CXXDefaultArgExpr it would assume deep-copying arbitrary
expressions, which sounds hard.
> Yes, you are right; To be honest, I wasn’t previously familiar
> with that part of the codebase.
> Yet still, those are very simple replacements, and IIRC those are
> the only two.
> Supporting structured bindings would be indeed easier if we could
> have a simpler AST,
> could you give an advice on what would be an equivalent AST we
> should rewrite to?
> You should rewrite a DeclRefExpr denoting a BindingDecl into the
> binding expression (BindingDecl::getBinding()) of that binding.
> Would producing a MemberExpr reading from the struct at *use* time
> be completely semantically equivalent?
> My previous impression was that for structured bindings load from
> the struct happens when the binding occurs,
> not when the actual read is performed.
> [though of course such rewrites would impede producing good
> diagnostic messages for the user]
> The member access happens each time the binding is named, not up-front.
> (However, for a tuple-like class type -- eg, std::tuple<...> or
> std::pair<...> -- the binding declarations have expressions that
> denote auxiliary variables (BindingDecl::getHoldingVar()) which are
> initialized up-front. In order to correctly handle those cases, when
> you build the CFG for a DecompositionDecl, you'll need to walk its
> bindings, check for holding variables, and also build CFG for those
> variables. You can search for "getHoldingVar()" through the clang
> codebase to see the other places this is already done -- in CodeGen
> and the constant expression evaluator.)
>>> The BindingDecls should not even show up, except as sugar so
>>> that clients who care can know that the 'e.x' expression was
>>> /written as/ 'v'.
>>>> I'd imagine this is best modeled by generating CFG
>>>> nodes for the subexpression on each occurrence of such
>>>> a DeclRefExpr, much like (IIRC) is done for
>>>> CXXDefaultArgExpr and CXXDefaultInitExpr. That seems
>>>> like it would not require too many special cases elsewhere.
>>>> (Perhaps the static analyzer needs more than that in
>>>> order to produce more user-friendly diagnostics?)
>>> We are not even there yet for structured bindings, just
>>> trying to make the analyzer understand them.
>>> cfe-dev mailing list
>>> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> cfe-dev mailing list
> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-dev