<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div class="gmail_quote"><div>Artem, please find my comments bellow.</div><div dir="ltr"><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">
Hmm, do we have a backtrace from a debug+assertions build? Like,
what are we specifically crashing at? It kinda makes sense that we
should have the variable in the vals list, because we went through
its respective DeclStmt, right? So i dunno where we are crashing.<br></div></blockquote><div><br></div><div>Its breaking at "return scratch[idx.getValue()];" (lib/Analysis/UninitializedValues.cpp at line 213). After some "errs() <<" debugging I noticed that my VarDecls are note being mapped by DeclToIndex::computeMap (lib/Analysis/UninitializedValues.cpp at line 78).<br><br></div><div>Probably I am using the wrong DeclContext then? (I am retrieving it from SemaRef.CurContext)</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">I also dumped the AST for the pseudo-code that you provided and i
noticed more inconsistencies with your AST:<br>
- Integral cast from int to unsigned is missing within operator ==.<br>
- The lvalue-to-rvalue around operator & in the __spec_begin
call is unnecessary because operator & already returns an
rvalue.<br>
- On the other hand, DeclRefExprs for both __setjmp_buf and
__setjmp_ret must be lvalues.</div></blockquote><div><br></div><div>Yes, I had noticed this as well. Its fixed now! I am not doing the Integral cast, as of now, just to see if it triggers a warning.</div><div><br></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">
It's also a bit weird that there's no control flow in your CFG. I
would have expected that a new block is going to start after
operator ==, but i guess if the user code is unmodified anyway,
there probably isn't much control flow anyway. But then what's the
point of comparing? I'm confused.<br></div></blockquote><div><br></div><div>You are right. However, I am emitting at CGStmt a BranchOnBoolExpr and both blocks as intended. In the future a intend to reflect the control change in the AST as well (enabling thus static analysis on my construct).</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">
> One thing I have noticed from the CFG is that all my calls were
tagged "template".<br>
CFG just calls Stmt::printPretty() in this case. A quick look at
Stmt pretty-printing suggests that you have created the DeclRefExpr
for the function with a non-empty template keyword source location,
i.e. this AST pretends to be constructed from code<br></div></blockquote><div><br></div><div>Fixed this as well.</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">
unsigned int __spec_mode = template __spec_begin(...);<br>
Also your variables have type 'auto'. And, as far as i understand,
we're dealing with C code.<br></div></blockquote><div><br></div><div>Also fixed.</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">
So just to be sure - are you sure you want to auto-generate AST? Did
you consider implementing your feature as a macro instead, probably
with the help of a custom compiler builtin for __spec_begin() if it
does something that a normal function cannot do? I.e., implement all
special features in the builtin, and instead of auto-generating the
surrounding AST, just add a macro into the header that expands to
whatever you want? It's not as omnipotent, but it's much easier.
Like, i mean, this AST compiles, but all clang tools are screwed
unless the AST makes perfect sense; this compiler warning is just
the first slightly-advanced tool you encountered.<br></div></blockquote><div><br></div><div>I think it makes sense to implement this at the AST level because, in the future, I intend to add more functionality. For instance, allow the user to inform which variables do not need to be speculated. Also, once in the AST I will be able to perform static analysis. But I might be wrong and appreciate your take on this.<br><br></div><div><br></div></div><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div>