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 emitted when
// we import the relevant module.
- if ((isa<ImportDecl>(D) || isa<VarDecl>(D)) && Ctx.DeclMustBeEmitted(D) &&
- D->getImportedOwningModule())
+ if ((isa<ImportDecl>(D) || isa<VarDecl>(D)) && D->getImportedOwningModule()
&&
+ Ctx.DeclMustBeEmitted(D))
return false;
----------------
It's not locally obvious why the order matters. Can you add a comment
explaining why you need to check getImportedOwningModule first? It might be
worth splitting Ctx.DeclMustBeEmitted into its own; e.g.,
```
// An ImportDecl or VarDecl imported from a module will get emitted when
// we import the relevant module.
if ((isa<ImportDecl>(D) || isa<VarDecl>(D)) && D->getImportedOwningModule())
// Only check DeclMustBeEmitted if D has been fully imported, since it may
// emit D as a side effect.
if (Ctx.DeclMustBeEmitted(D))
return false;
```
but anything that prevents someone from swapping the conditions when they
refactor this would be good enough I think.
https://reviews.llvm.org/D29753
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
