[PATCH] D75560: Make Decl:: setOwningModuleID() public. (NFC)

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

I'm surprised so few setters needed to be made public for this to work. If this required making lots more setters public, I'd be concerned, because we generally don't want large chunks of the AST to be mutable after creation. But this seems fine.



================
Comment at: clang/include/clang/AST/DeclBase.h:629-630
 
+public:
+  void setFromASTFile() { FromASTFile = true; }
+
----------------
aprantl wrote:
> 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.
The storage isn't necessarily allocated if the decl is created with `::Create` rather than `::CreateDeserialized`. Just changing the comment to say the declaration must have been created with `CreateDeserialized` would be fine.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75560/new/

https://reviews.llvm.org/D75560