aprantl marked an inline comment as done. aprantl added inline comments.
================ Comment at: clang/include/clang/AST/DeclBase.h:629-630 +public: + void setFromASTFile() { FromASTFile = true; } + ---------------- rsmith wrote: > Setting this after creating a `Decl` is not safe in general; declarations > with this flag set have 8 bytes of additional storage allocated before their > start. > > An `assert(FromASTFile || hasLocalOwningModuleStorage());` would at least > catch the unsafe cases. This also needs a comment saying that it's not safe > to use in general, and perhaps a scarier name to discourage people from > calling it. (Unless you're creating declarations with `::CreateDeserialized` > -- in which case you'd need all the `Decl` subclasses to befriend the > creator, making this function unnecessary -- this is currently only going to > be safe if `ModulesLocalVisibility` is enabled.) I think the warning is unnecessary, since the storage is always allocated. Please let me know if I misunderstood something. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75560/new/ https://reviews.llvm.org/D75560 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits