rsmith added inline comments.
================
Comment at: clang/include/clang/Sema/Sema.h:2275-2276
+ llvm::SmallPtrSet<const Decl *, 32> SeenDecls;
+ llvm::SmallPtrSet<const clang::Type *, 32> SeenTypes;
+
----------------
These names seem too general to live directly in `Sema`.
================
Comment at: clang/include/clang/Sema/Sema.h:2278
+
+ /// Examine a type, unwrapping ay decls that might be in the GMF.
+ void findGMFReachableDeclsForType(const Type *T);
----------------
================
Comment at: clang/lib/Sema/SemaModule.cpp:916-920
+ markGMFDeclsReachableFrom(Target, /*MakeVisible*/ true);
+ }
+ } else {
+ markGMFDeclsReachableFrom(Child, /*MakeVisible*/ true);
+ }
----------------
Do we need the special `MakeVisible` handling here? I would have thought that
it would be sufficient for `Child` itself to be visible. Making the target of a
using declaration visible seems like it would do the wrong thing for a case
like:
```
module;
int f();
export module M;
namespace N { export using ::f; }
```
```
import M;
int a = f(); // should be an error, ::f should not be visible
int b = N::f(); // should be OK, UsingShadowDecl is visible
```
I wonder if we need any special handling here at all -- if the GMF decls are
referenced by something in an `export` block, then I'd have expected they'll
already be marked as used and that marking would have marked them as reachable.
================
Comment at: clang/lib/Sema/SemaModule.cpp:1024-1036
+ // Only visit each Decl once.
+ if (!SeenDecls.insert(Orig).second)
+ return;
+
+ // Do not alter the ownership unless it is ModuleDiscardable.
+ if (Orig->isModuleDiscardable()) {
+ assert(Orig->getOwningModule() &&
----------------
Instead of storing a `SeenDecls` set and checking it here, is it feasible to
check whether we're transitioning this declaration from discardable to
retained, and only doing the work below if so?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126694/new/
https://reviews.llvm.org/D126694
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
