<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<br>
<br>
<div class="moz-cite-prefix">On 4/2/18 6:25 PM, Richard Smith wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAOfiQq=PPTFkdffbcEz_PqzJSzwZC1tzTvLR-Du7M0cWEwVsag@mail.gmail.com">
<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"
moz-do-not-send="true">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" moz-do-not-send="true">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"
moz-do-not-send="true">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?</div>
</div>
</div>
</div>
</blockquote>
<br>
We evaluate the default arguments in the caller stack frame before
jumping into the callee stack frame, and call destructors for them
after we're back to the callee stack frame. Or rather plan to
evaluate, because right now we aren't doing very sensible things
anyway when the argument is not an ICE. I mean, we're not yet
evaluating default arguments, but we're already having a very
conservative evaluation of their respective destructors, which
already suffers a bit from the expression agglutination problem
because it relies on the CXXBindTemporaryExpr in their initializer
expression.<br>
<br>
In the analyzer we not only have "stack frames", but we have a
generalized notion of "location context" which might represent a
stack frame (which is necessary to avoid collisions of expression
values during recursion) but it might as well represent an
if-statement branch or a single loop iteration (it still helps us
track local variable lifetime; this isn't in fact implemented yet,
but it was the original design). And we evaluate all expressions
keeping in mind that we're in a certain location that is represented
by the stack of "location contexts", which helps us de-duplicate
expressions. So the solution of having a special "location context"
for every CXXDefaultArgExpr, so that actual initialization
expressions were isolated in their own environment and didn't
collide, looks quite reliable / flexible / future-proof.<br>
<br>
<blockquote type="cite"
cite="mid:CAOfiQq=PPTFkdffbcEz_PqzJSzwZC1tzTvLR-Du7M0cWEwVsag@mail.gmail.com">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<div>(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"
moz-do-not-send="true">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>
</div>
</div>
</blockquote>
<br>
Hmm, right, this sounds really scary. We aren't defended from this
sort of things.<br>
<br>
<blockquote type="cite"
cite="mid:CAOfiQq=PPTFkdffbcEz_PqzJSzwZC1tzTvLR-Du7M0cWEwVsag@mail.gmail.com">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<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>
</div>
</blockquote>
<br>
CXXDefaultInitExprs look safe to me because every member is
initialized just once within a single constructor, and it also
indeed stays within the constructor's stack frame together with
their CXXCtorInitializers.<br>
<br>
<blockquote type="cite"
cite="mid:CAOfiQq=PPTFkdffbcEz_PqzJSzwZC1tzTvLR-Du7M0cWEwVsag@mail.gmail.com">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote">
<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>
</div>
</blockquote>
<br>
Yeah, that'd essentially involve deep-converting *every* statement
into a whole new format of CFG elements and it'd make the CFG much
more complex.<br>
<br>
If we want to avoid manipulating expressions directly, we need to
teach our CFG to answer questions like "here's the expression i'm
trying to evaluate, what CFG elements correspond to its respective
sub-expressions?", which constitutes a large chunk of the Stmt
hierarchy complexity. And poor-man's solutions like CFGStmtMap
wouldn't cut it as an answer because the whole point, to begin with,
is to de-duplicate potentially repeating expressions, so we can't
use those as keys anymore.<br>
<br>
<blockquote type="cite"
cite="mid:CAOfiQq=PPTFkdffbcEz_PqzJSzwZC1tzTvLR-Du7M0cWEwVsag@mail.gmail.com">
<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 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" moz-do-not-send="true">cfe-dev@lists.llvm.org</a><br>
<a
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev"
rel="noreferrer
 noreferrer" target="_blank" moz-do-not-send="true">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"
moz-do-not-send="true">cfe-dev@lists.llvm.org</a><br>
<a
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev"
rel="noreferrer
noreferrer"
target="_blank"
moz-do-not-send="true">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" moz-do-not-send="true">cfe-dev@lists.llvm.org</a><br>
<a
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev"
rel="noreferrer" target="_blank"
moz-do-not-send="true">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" moz-do-not-send="true">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" moz-do-not-send="true">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"
moz-do-not-send="true">cfe-dev@lists.llvm.org</a><br>
<a
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev"
rel="noreferrer" target="_blank" moz-do-not-send="true">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-dev</a><br>
<br>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
<br>
</body>
</html>