[PATCH] D116751: [clang][lex] NFC: Extract module creation into function

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D116751#3227142 <https://reviews.llvm.org/D116751#3227142>, @ahoppen wrote:

> I suppose the idea is that all `Module` creations should go through `makeModule`, right? In that case I think we should either
>
> - make the `Module` constructor private and `ModuleMap` a friend of `Module`
> - or at least add a doc comment to the `Module` constructor that says `Module`s should only be created using `ModuleMap::makeModule`.
>
> Or are there other places that also create `Module`s but are not supposed to go through `ModuleMap::makeModule`?

The intent here is for all calls to the `Module` constructor in `ModuleMap` to go through `makeModule()`. This will make it possible to ensure a callback is always invoked when a `Module` is constructed in D113676 <https://reviews.llvm.org/D113676>.

Making the constructor private doesn't aid that goal, so I'd be inclined to do that in a separate patch. But would that be the right thing to do?
We don't instantiate `Module` outside of `ModuleMap` in the upstream repo, but I don't think there's anything fundamental that would prevent downstream projects doing that. Do we care about such use-cases though?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116751