[modules][PATCH] Teach the ASTDeclReader to update the exception specs of the deserialized decl's redecl chain.

[Richard Smith]

+ } + else if (!isUnresolvedExceptionSpec(FPT->getExceptionSpecType()) &&

Per the LLVM coding style, put the close brace and the 'else' on the same
line.

+ isUnresolvedExceptionSpec(PrevFPT->getExceptionSpecType())) { + //
Happens in cases where module A contains only a fwd decl and module B + //
contains the definition.

There are other conditions here; I don't think this comment is particularly
helpful. Just remove it?

+ FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo(); + while
(PrevFD) { + Reader.Context.adjustExceptionSpec(PrevFD, EPI.ExceptionSpec);
+ PrevFD = PrevFD->getPreviousDecl();

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.

It seems like it should be possible to produce a testcase for this. You'd
need something like:

A.h:

struct A { A(); } b{}, c(b);  // computed exception spec for copy ctor and
move ctor

B.h:

struct A { A(); } a{};  // EST_Unevaluated for copy ctor and move ctor

... then import A and B, and do something that assumes that every
declaration has an exception specification if any declaration does.

On Mon, Nov 24, 2014 at 11:09 AM, Vassil Vassilev <vvasilev at cern.ch> wrote:

>  Sorry for the delay. Attaching the new version.
> Vassil
>
> On 14/11/14 02:47, Richard Smith wrote:
>
>  +    }
> +    else { // FPT->getExceptionSpecType() is resolved and the other is not
>
>  You're not checking for this condition; the code here is getting run
> even if both or neither are unresolved.
>
>  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.
>
> On Thu, Nov 6, 2014 at 4:23 AM, Vassil Vassilev <
> vasil.georgiev.vasilev at cern.ch> wrote:
>
>> Hi Richard,
>>   I am attaching the patch we discussed at the dev meeting. Still haven't
>> found small reproducer...
>>   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.
>>
>>   Could you review, please?
>> Many thanks,
>> Vassil
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141124/e8cf7bee/attachment.html>