[PATCH] D113545: [C++20] [Module] Support reachable definition initially/partially

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

I'm going to try rebasing my GMF elision code on top of this.

the test-suite changes could be made easier to read by using the file-split stuff we've been using for recent changes (I guess this is an older patch)?



================
Comment at: clang/include/clang/AST/DeclBase.h:234-236
     /// lookups that occur within that module.
+    /// The discarded declarations in global module fragment belongs
+    /// to this group too.
----------------
that is not going to be sufficient to exclude them - see D126694, we introduce a new category "ModuleUnreachable".


================
Comment at: clang/include/clang/AST/DeclBase.h:636-637
+
+  /// FIXME: Implement discarding declarations actually in global module
+  /// fragment. See [module.global.frag]p3,4 for details.
+  bool isDiscardedInGlobalModuleFragment() const { return false; }
----------------
see D126694.



================
Comment at: clang/lib/Sema/SemaLookup.cpp:1947
+  // DeclModule if it isn't (transitively) imported.
+  if (DeclModule->getTopLevelModule()->isModuleInterfaceUnit())
+    return true;
----------------
I think isModuleInterfaceUnit needs to include implementation partition units, since they also have a BMI  (is `isInterfaceOrPartition()` the right thing to use here?


================
Comment at: clang/lib/Sema/SemaLookup.cpp:1958
+  // additional translation unit is not considered as reachable for portability.
+  return SemaRef.getLangOpts().AllAdditionalTUReachable;
+}
----------------
could we separate this part of the change into a separate patch (along with the command line switch etc.) ?
it is not needed for standard compliance, right?



================
Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:1931
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2));  // AccessSpecifier
-  Abv->Add(BitCodeAbbrevOp(0));                       // ModulePrivate
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3));  // ModulePrivate
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // SubmoduleID
----------------
ModuleOwnershipKind? (and several cases below)



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

https://reviews.llvm.org/D113545