<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 2 April 2018 at 18:04, Artem Dergachev via cfe-dev <span dir="ltr"><<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF"><span class="gmail-"><div class="gmail-m_-7055416578314946084moz-cite-prefix">On 4/2/18 2:11 PM, Richard Smith via
cfe-dev wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">On 2 April 2018 at 13:52, George
Karpenkov via cfe-dev <span dir="ltr"><<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="word-wrap:break-word">Hi
Richard,
<div><br>
</div>
<div>Thanks for your reply!<br>
<div><span><br>
<blockquote type="cite">
<div>On Mar 30, 2018, at 8:49 PM, Richard Smith
<<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>>
wrote:</div>
<br class="gmail-m_-7055416578314946084m_4536569605347199083Apple-interchange-newline">
<div>
<div dir="auto" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<div>
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="word-wrap:break-word">
<div>
<div>
<div><br>
<br>
</div>
Right, sure, but I don’t have a
convenient way to find that
DecompositionDecl from a given
BindingDecl,</div>
<div>and sometimes I need to act
based on the BindingDecl alone.</div>
</div>
</div>
</blockquote>
</div>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">Can you give an example?</div>
</div>
</div>
</blockquote>
<div><br>
</div>
</span>
<div>Let’s say a structured binding is written into,
and then some function is called, and then we read
from that binding again.</div>
<div>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,</div>
<div>chances are it will remain the same.</div>
</div>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>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.</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="word-wrap:break-word">
<div>
<div><span>
<blockquote type="cite">
<div dir="auto" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<div dir="auto">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.</div>
<div dir="auto"><br>
</div>
<div dir="auto">This was a somewhat
controversial decision, but it doesn't look
like it's going to be reversed.</div>
<div dir="auto">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="word-wrap:break-word">
<div>
<div>
<blockquote type="cite">
<div>
<div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<div class="gmail_extra">
<div class="gmail_quote">
<div>That's what the CFG
for the above function
should represent.</div>
</div>
</div>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>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).</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">IIRC, the CFG expands
CXXDefaultArgExprs and CXXDefaultInitExprs;
this case is analogous to those.</div>
</div>
</blockquote>
</span></div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
<br></span>
Thank you for the explanation! I was also very confused.<br>
<br>
We do expand CXXDefaultInitExprs for sema warnings CFG, but not for
the CFG analyzer. We don't expand CXXDefaultArgExprs yet.<br>
<br>
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.<br>
<br>
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.</div></blockquote><div><br></div><div>Yes, a BindingDecl should always evaluate to the same lvalue throughout its lifetime (at least, per the current language rules; there's been some discussion of changing the semantics so that get<N>(e) is evaluated each time the binding is named, but so far that looks unlikely to happen).</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">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.</div></blockquote><div><br></div><div>Interesting, you evaluate default arguments in the callee's stack frame? (We've historically had problems with this in the constant expressoin evaluator, where we're now numbering "versions" of variables and temporaries to distinguish them in situations like this -- see <a href="https://reviews.llvm.org/D42776">https://reviews.llvm.org/D42776</a>). It's not completely clear to me that you can't need to have the same CXXDefaultArgExpr live twice at the same time; a construct like:</div><div><br></div><div>int f(int a = f(0), int b = g());</div><div>f();</div><div><br></div><div>... would trigger that if it were valid, and it's conceivable that there are variants of that which are valid.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">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"?<br></div></blockquote><div><br></div><div>Good question. I don't think so. And we already can't rely on that for CXXDefaultInitExprs.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
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.<br>
<br>
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.<br>
<br>
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.<br>
<br>
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.</div></blockquote><div><br></div><div>Yes. Another option might be to stop using the Expr* to identify the CFG element / value, but I suspect that would be a very substantial undertaking?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF"><div><div class="gmail-h5"><blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="word-wrap:break-word">
<div>
<div><span>
</span>
<div>Yes, you are right; To be honest, I wasn’t
previously familiar with that part of the
codebase.</div>
<div>Yet still, those are very simple replacements,
and IIRC those are the only two.</div>
<div><br>
</div>
<div>Supporting structured bindings would be indeed
easier if we could have a simpler AST,</div>
<div>could you give an advice on what would be an
equivalent AST we should rewrite to?</div>
</div>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>You should rewrite a DeclRefExpr denoting a BindingDecl
into the binding expression (BindingDecl::getBinding()) of
that binding.</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="word-wrap:break-word">
<div>
<div>
<div>Would producing a MemberExpr reading from the
struct at *use* time be completely semantically
equivalent?</div>
</div>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>Yes.</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="word-wrap:break-word">
<div>
<div>
<div>My previous impression was that for structured
bindings load from the struct happens when the
binding occurs,</div>
<div>not when the actual read is performed.</div>
<div>[though of course such rewrites would impede
producing good diagnostic messages for the user]</div>
</div>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>The member access happens each time the binding is
named, not up-front.</div>
<div><br>
</div>
<div>(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.)</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="word-wrap:break-word">
<div>
<div><span>
<blockquote type="cite">
<div dir="auto" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<div dir="auto">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="word-wrap:break-word">
<div>
<div>
<blockquote type="cite">
<div>
<div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<div class="gmail_extra">
<div class="gmail_quote">
<div>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'.</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="word-wrap:break-word">
<div><span>
<blockquote type="cite">
<div>
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div>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.</div>
<div><br>
</div>
<div>(Perhaps
the static
analyzer needs
more than that
in order to
produce more
user-friendly
diagnostics?)</div>
</div>
</div>
</div>
</div>
</blockquote>
<div><br>
</div>
</span>We are not
even there yet for
structured
bindings, just
trying to make the
analyzer
understand them.<br>
<br>
</div>
<div><br>
</div>
<br>
</div>
<br>
______________________________<wbr>_________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" rel="noreferrer" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer
noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-dev</a></blockquote>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
______________________________<wbr>_________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" rel="noreferrer" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-dev</a></blockquote>
</div>
</div>
</div>
</blockquote>
</span></div>
<br>
</div>
</div>
<br>
______________________________<wbr>_________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-dev</a><br>
<br>
</blockquote>
</div>
<br>
</div>
</div>
<br>
<fieldset class="gmail-m_-7055416578314946084mimeAttachmentHeader"></fieldset>
<pre class="gmail-m_-7055416578314946084moz-quote-pre">______________________________<wbr>_________________
cfe-dev mailing list
<a class="gmail-m_-7055416578314946084moz-txt-link-abbreviated" href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>
<a class="gmail-m_-7055416578314946084moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-dev</a>
</pre>
</blockquote>
<br>
</div></div></div>
<br>______________________________<wbr>_________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div><br></div></div>