<div dir="ltr"><span style="color:rgb(0,0,0);font-family:'Courier New',Courier,monospace;font-size:14px;white-space:pre-wrap">+ }
+ else if (!isUnresolvedExceptionSpec(FPT->getExceptionSpecType()) &&</span><div><span style="color:rgb(0,0,0);font-family:'Courier New',Courier,monospace;font-size:14px;white-space:pre-wrap"><br></span></div>Per the LLVM coding style, put the close brace and the 'else' on the same line.<div><br><div><span style="color:rgb(0,0,0);font-family:'Courier New',Courier,monospace;font-size:14px;white-space:pre-wrap">+ isUnresolvedExceptionSpec(PrevFPT->getExceptionSpecType())) {
+ // Happens in cases where module A contains only a fwd decl and module B
+ // contains the definition.</span></div><div><span style="color:rgb(0,0,0);font-family:'Courier New',Courier,monospace;font-size:14px;white-space:pre-wrap"><br></span></div>There are other conditions here; I don't think this comment is particularly helpful. Just remove it?</div><div><br><div><span style="color:rgb(0,0,0);font-family:'Courier New',Courier,monospace;font-size:14px;white-space:pre-wrap">+ FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+ while (PrevFD) {
+ Reader.Context.adjustExceptionSpec(PrevFD, EPI.ExceptionSpec);
+ PrevFD = PrevFD->getPreviousDecl();</span></div><div><span style="color:rgb(0,0,0);font-family:'Courier New',Courier,monospace;font-size:14px;white-space:pre-wrap"><br></span></div>Please use a separate variable for this loop rather than reusing PrevFD. It's only by coincidence that this code is at the end of the function; it shouldn't be changing function-scope state like this.</div><div><br></div><div>It seems like it should be possible to produce a testcase for this. You'd need something like:</div><div><br></div><div>A.h:</div><div><br></div><div>struct A { A(); } b{}, c(b); // computed exception spec for copy ctor and move ctor<br></div><div><br></div><div>B.h:</div><div><br></div><div><div>struct A { A(); } a{}; // EST_Unevaluated for copy ctor and move ctor</div><div><br></div><div></div></div><div>... then import A and B, and do something that assumes that every declaration has an exception specification if any declaration does.</div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 24, 2014 at 11:09 AM, Vassil Vassilev <span dir="ltr"><<a href="mailto:vvasilev@cern.ch" target="_blank">vvasilev@cern.ch</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<div>Sorry for the delay. Attaching the new
version.<span class=""><font color="#888888"><br>
Vassil</font></span><div><div class="h5"><br>
On 14/11/14 02:47, Richard Smith wrote:<br>
</div></div></div><div><div class="h5">
<blockquote type="cite">
<div dir="ltr">
<div>+ }</div>
<div>+ else { // FPT->getExceptionSpecType() is resolved
and the other is not</div>
<div><br>
</div>
<div>You're not checking for this condition; the code here is
getting run even if both or neither are unresolved.</div>
<div><br>
</div>
<div>The patch needs rebasing (we have a new helper function in
ASTContext to update the exception specification of a
declaration), but looks like the right direction.</div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Thu, Nov 6, 2014 at 4:23 AM, Vassil
Vassilev <span dir="ltr"><<a href="mailto:vasil.georgiev.vasilev@cern.ch" target="_blank">vasil.georgiev.vasilev@cern.ch</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi
Richard,<br>
I am attaching the patch we discussed at the dev meeting.
Still haven't found small reproducer...<br>
The issue appears to stem from the fact that module A
contains only a forward declaration of a function and it
exception spec cannot be computed. In module B is the
definition with computed exception spec, which gets
deserialized after the one in module A. This patch teaches
the ASTDeclReader to update all the exception specs of the
previous decls to the current one.<br>
<br>
Could you review, please?<br>
Many thanks,<br>
Vassil</blockquote>
</div>
</div>
</blockquote>
<br>
</div></div></div>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div></div></div></div>