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

Reply via email to