[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

2017-03-01 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL296656: [PCH] Avoid VarDecl emission attempt if no owning module avaiable (authored by bruno). Changed prior to commit: https://reviews.llvm.org/D29753?vs=87774=90212#toc Repository: rL LLVM

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

2017-02-28 Thread Richard Smith via Phabricator via cfe-commits
rsmith added a comment. In https://reviews.llvm.org/D29753#688834, @bruno wrote: > > It seems to me that the problem here is that `DeclMustBeEmitted` is not > > safe to call in the middle of deserialization if anything > > partially-deserialized might be reachable from the declaration we're >

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

2017-02-28 Thread Richard Smith via Phabricator via cfe-commits
rsmith accepted this revision. rsmith added a comment. Yes, I'm OK with this as a stopgap fix for 4.0. I would have preferred a more full fix for 4.0 but we've run out of time for that, and the PCH case does seem more pressing. https://reviews.llvm.org/D29753

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

2017-02-28 Thread Hans Wennborg via Phabricator via cfe-commits
hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D29753#688845, @bruno wrote: > > What do folks think? > > IMO we should do it. Go ahead and commit this, but keep the bug open so we can work on fixing it properly

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

2017-02-28 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added a comment. > What do folks think? IMO we should do it. https://reviews.llvm.org/D29753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

2017-02-28 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added a comment. > It seems to me that the problem here is that `DeclMustBeEmitted` is not safe > to call in the middle of deserialization if anything partially-deserialized > might be reachable from the declaration we're querying, and yet we're > currently calling it in that case.

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

2017-02-28 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a subscriber: mehdi_amini. hans added a comment. To unblock 4.0, I'm leaning towards merging Bruno's patch as a stop-gap fix. I realize it probably only fixes the problem for PCH, and not modules. But PCH is used more widely than modules, so maybe it's good enough for now? We've run

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

2017-02-21 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment. This seems to be stuck. Bruno, Richard, do you think there's a chance this can be fixed for 4.0? https://reviews.llvm.org/D29753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

2017-02-16 Thread Richard Smith via Phabricator via cfe-commits
rsmith added a comment. It seems to me that the problem here is that `DeclMustBeEmitted` is not safe to call in the middle of deserialization if anything partially-deserialized might be reachable from the declaration we're querying, and yet we're currently calling it in that case. I don't see

Re: [PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

2017-02-16 Thread Hans Wennborg via cfe-commits
Richard, can you take a look when you have a moment? The PR is marked as a release blocker. On Thu, Feb 9, 2017 at 1:54 PM, Duncan P. N. Exon Smith via Phabricator via cfe-commits wrote: > dexonsmith added a comment. > > I'm not comfortable signing off on this, but it

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

2017-02-09 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. I'm not comfortable signing off on this, but it seems like this should be set up as a blocker for LLVM 4.0 if it isn't already. Comment at: lib/Serialization/ASTReaderDecl.cpp:2518-2523 // An ImportDecl or VarDecl imported from a module will get

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

2017-02-09 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno created this revision. This fixes PR31863, a regression introduced in r276159. Consider this snippet: struct FVector; struct FVector {}; struct FBox { FVector Min; FBox(int); }; namespace { FBox InvalidBoundingBox(0); } While parsing the DECL_VAR for 'struct FBox', clang