[C++20] [Modules] Implementing Eliding Unreachable Decls of GMF in ASTWriter

clang/include/clang/AST/DeclBase.h

@@ -233,9 +233,12 @@ class alignas(8) Decl {
 
     /// This declaration has an owning module, but is only visible to
     /// lookups that occur within that module.
-    /// The discarded declarations in global module fragment belongs
-    /// to this group too.
-    ModulePrivate
+    ModulePrivate,
+
+    /// This declaration is part of a Global Module Fragment, it is permitted
+    /// to discard it and therefore it is not reachable or visible to importers
+    /// of the named module of which the GMF is part.
+    ModuleDiscardable
   };
 
 protected:
@@ -658,9 +661,10 @@ class alignas(8) Decl {
   /// Whether this declaration comes from another module unit.
   bool isInAnotherModuleUnit() const;
 
-  /// FIXME: Implement discarding declarations actually in global module
-  /// fragment. See [module.global.frag]p3,4 for details.
-  bool isDiscardedInGlobalModuleFragment() const { return false; }
+  /// See [module.global.frag]p3,4 for details.
+  bool isDiscardedInGlobalModuleFragment() const {
+    return getModuleOwnershipKind() == ModuleOwnershipKind::ModuleDiscardable;
+  }
 
   /// Return true if this declaration has an attribute which acts as
   /// definition of the entity, such as 'alias' or 'ifunc'.

clang/include/clang/Basic/LangOptions.def

@@ -178,6 +178,7 @@ LANGOPT(BuiltinHeadersInSystemModules, 1, 0, "builtin headers belong to system m
 BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 3, CMK_None,
                     "compiling a module interface")
 BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
+LANGOPT(DiscardGMFDecls   , 1, 1, "Discard unreachable decls in GMF")
 BENIGN_LANGOPT(BuildingPCHWithObjectFile, 1, 0, "building a pch which has a corresponding object file")
 BENIGN_LANGOPT(CacheGeneratedPCH, 1, 0, "cache generated PCH files in memory")
 BENIGN_LANGOPT(PCHInstantiateTemplates, 1, 0, "instantiate templates while building a PCH")

clang/include/clang/Driver/Options.td

@@ -2970,6 +2970,13 @@ defm prebuilt_implicit_modules : BoolFOption<"prebuilt-implicit-modules",
   PosFlag<SetTrue, [], [ClangOption], "Look up implicit modules in the prebuilt module path">,
   NegFlag<SetFalse>, BothFlags<[], [ClangOption, CC1Option]>>;
 
+defm discarding_gmf_decls : BoolFOption<"discarding-gmf-decls",
+  LangOpts<"DiscardGMFDecls">, DefaultTrue,
+  PosFlag<SetTrue>,
+  NegFlag<SetFalse, [], [ClangOption],
+          "Disable to discard unreachable decls in global module fragment">,
+  BothFlags<[], [ClangOption, CC1Option]>>;
+
 def fmodule_output_EQ : Joined<["-"], "fmodule-output=">,
   Flags<[NoXarchOption]>, Visibility<[ClangOption, CC1Option]>,
   HelpText<"Save intermediate module file results when compiling a standard C++ module unit.">;

clang/include/clang/Serialization/ASTWriter.h

@@ -467,6 +467,22 @@ class ASTWriter : public ASTDeserializationListener,
   std::vector<SourceRange> NonAffectingRanges;
   std::vector<SourceLocation::UIntTy> NonAffectingOffsetAdjustments;
 
+  /// Mark ModuleDiscardable Decl D and its file scope top level declaration D
+  /// as reachable. This is a no-op if D is not ModuleDiscardable. We'll mark
+  /// the file scope top level declaration D as reachable too. Otherwise, it is
+  /// problematic if some parts of a decl is discarded in some TU and these
+  /// parts are not discarded in other TUs. This is an ODR violation. So if a
+  /// sub-decl is reachable, the top level decl and all of its children should
+  /// be reachable too.
+  void MarkDeclReachable(const Decl *D);
+  /// A helper to IsDeclModuleDiscardable. There are special declarations which
+  /// may not be referenced directly. But they can't be discarded if their
+  /// correspond decls are reachable. e.g., the deduction guides decls.
+  bool IsSpecialDeclNotDiscardable(Decl *D);
+  /// Callbacks to mark special decls as reachable once their corresponding
+  /// decls become reachable.
+  llvm::DenseMap<Decl *, llvm::SmallVector<Decl *, 8>> ReachableMarkerCallbacks;
+
   /// Collects input files that didn't affect compilation of the current module,
   /// and initializes data structures necessary for leaving those files out
   /// during \c SourceManager serialization.
@@ -792,6 +808,14 @@ class ASTWriter : public ASTDeserializationListener,
     return WritingModule && WritingModule->isNamedModule();
   }
 
+  /// Whether or not D is module discardable. Besides the case that D is marked
+  /// not module discardable explicitly, `IsDeclModuleDiscardable` will return
+  /// false if:
+  /// - The file scope top level declaration of D is not module discardable.
+  /// - D is a deduction guide for another template declaration TD and TD is not
+  /// module discardable.
+  bool IsDeclModuleDiscardable(const Decl *D);
+
 private:
   // ASTDeserializationListener implementation
   void ReaderInitialized(ASTReader *Reader) override;

clang/lib/Sema/SemaModule.cpp

@@ -88,7 +88,11 @@ Sema::ActOnGlobalModuleFragmentDecl(SourceLocation ModuleLoc) {
   // within the module unit.
   //
   // So the declations in the global module shouldn't be visible by default.
-  TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ReachableWhenImported);
+  if (getLangOpts().DiscardGMFDecls)
+    TU->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ModuleDiscardable);
+  else
+    TU->setModuleOwnershipKind(
+        Decl::ModuleOwnershipKind::ReachableWhenImported);
   TU->setLocalOwningModule(GlobalModule);
 
   // FIXME: Consider creating an explicit representation of this declaration.

clang/lib/Serialization/ASTReaderDecl.cpp

@@ -656,6 +656,9 @@ void ASTDeclReader::VisitDecl(Decl *D) {
     case Decl::ModuleOwnershipKind::ReachableWhenImported:
     case Decl::ModuleOwnershipKind::ModulePrivate:
       break;
+
+    case Decl::ModuleOwnershipKind::ModuleDiscardable:
+      llvm_unreachable("We should never read module discardable decls");
     }
 
     D->setModuleOwnershipKind(ModuleOwnership);

clang/lib/Serialization/ASTWriter.cpp

@@ -2994,7 +2994,8 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
     // Emit the initializers, if any.
     RecordData Inits;
     for (Decl *D : Context->getModuleInitializers(Mod))
-      Inits.push_back(GetDeclRef(D));
+      if (!IsDeclModuleDiscardable(D))
+        Inits.push_back(GetDeclRef(D));
     if (!Inits.empty())
       Stream.EmitRecord(SUBMODULE_INITIALIZERS, Inits);
 
@@ -3171,6 +3172,13 @@ uint64_t ASTWriter::WriteDeclContextLexicalBlock(ASTContext &Context,
   uint64_t Offset = Stream.GetCurrentBitNo();
   SmallVector<uint32_t, 128> KindDeclPairs;
   for (const auto *D : DC->decls()) {
+    if (IsDeclModuleDiscardable(D)) {
+      if (DC->isFileContext())
+        continue;
+      else
+        MarkDeclReachable(D);
+    }
+
     KindDeclPairs.push_back(D->getKind());
     KindDeclPairs.push_back(GetDeclRef(D));
   }
@@ -3819,9 +3827,12 @@ class ASTDeclContextNameLookupTrait {
   template<typename Coll>
   data_type getData(const Coll &Decls) {
     unsigned Start = DeclIDs.size();
-    for (NamedDecl *D : Decls) {
-      DeclIDs.push_back(
-          Writer.GetDeclRef(getDeclForLocalLookup(Writer.getLangOpts(), D)));
+    for (NamedDecl *ND : Decls) {
+      auto *D = getDeclForLocalLookup(Writer.getLangOpts(), ND);
+      if (Writer.IsDeclModuleDiscardable(D))
+        continue;
+
+      DeclIDs.push_back(Writer.GetDeclRef(D));
     }
     return std::make_pair(Start, DeclIDs.size());
   }
@@ -3976,6 +3987,15 @@ ASTWriter::GenerateNameLookupTable(const DeclContext *ConstDC,
         isLookupResultEntirelyExternal(Result, DC))
       continue;
 
+    if (!DC->isFileContext() && !DoneWritingDeclsAndTypes) {
+      for (auto *D : Result.getLookupResult())
+        if (IsDeclModuleDiscardable(D))
+          MarkDeclReachable(D);
+    } else if (llvm::all_of(Result.getLookupResult(), [this](NamedDecl *D) {
+            return IsDeclModuleDiscardable(D);
+          }))
+        continue;
+
     // We also skip empty results. If any of the results could be external and
     // the currently available results are empty, then all of the results are
     // external and we skip it above. So the only way we get here with an empty
@@ -4165,7 +4185,7 @@ uint64_t ASTWriter::WriteDeclContextVisibleBlock(ASTContext &Context,
       }
 
       for (NamedDecl *ND : Result)
-        if (!ND->isFromASTFile())
+        if (!ND->isFromASTFile() && !IsDeclModuleDiscardable(ND))
           GetDeclRef(ND);
     }
 
@@ -4903,7 +4923,7 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot,
   const TranslationUnitDecl *TU = Context.getTranslationUnitDecl();
   SmallVector<uint32_t, 128> NewGlobalKindDeclPairs;
   for (const auto *D : TU->noload_decls()) {
-    if (!D->isFromASTFile()) {
+    if (!D->isFromASTFile() && !IsDeclModuleDiscardable(D)) {
       NewGlobalKindDeclPairs.push_back(D->getKind());
       NewGlobalKindDeclPairs.push_back(GetDeclRef(D));
     }
@@ -4955,9 +4975,9 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot,
   // Make sure visible decls, added to DeclContexts previously loaded from
   // an AST file, are registered for serialization. Likewise for template
   // specializations added to imported templates.
-  for (const auto *I : DeclsToEmitEvenIfUnreferenced) {
-    GetDeclRef(I);
-  }
+  for (const auto *I : DeclsToEmitEvenIfUnreferenced)
+    if (!IsDeclModuleDiscardable(I))
+      GetDeclRef(I);
 
   // Make sure all decls associated with an identifier are registered for
   // serialization, if we're storing decls with identifiers.
@@ -5280,6 +5300,7 @@ void ASTWriter::WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord) {
       case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION:
       case UPD_CXX_ADDED_ANONYMOUS_NAMESPACE:
         assert(Update.getDecl() && "no decl to add?");
+        MarkDeclReachable(Update.getDecl());
         Record.push_back(GetDeclRef(Update.getDecl()));
         break;
 
@@ -5419,6 +5440,7 @@ void ASTWriter::WriteDeclUpdatesBlocks(RecordDataImpl &OffsetsRecord) {
       Record.AddVarDeclInit(VD);
     }
 
+    MarkDeclReachable(D);
     OffsetsRecord.push_back(GetDeclRef(D));
     OffsetsRecord.push_back(Record.Emit(DECL_UPDATES));
   }
@@ -5679,7 +5701,125 @@ TypeID ASTWriter::getTypeID(QualType T) const {
   });
 }
 
+bool ASTWriter::IsSpecialDeclNotDiscardable(Decl *D) {
+  assert(D->isDiscardedInGlobalModuleFragment());
+
+  /// Currently, the only special decl is the deduction guide.
+  if (auto *ND = dyn_cast<NamedDecl>(D)) {
+    DeclarationName Name = ND->getDeclName();
+    if (TemplateDecl *TD = Name.getCXXDeductionGuideTemplate()) {
+      if (!IsDeclModuleDiscardable(TD)) {
+        MarkDeclReachable(D);
+        return true;
+      }
+
+      ReachableMarkerCallbacks[TD].push_back(D);
+    }
+  }
+
+  // FIXME:
+  //
+  // There is a wide used pattern in libstdc++:
+  //
+  // namespace std
+  // {
+  //   inline namespace __cxx11 __attribute__((__abi_tag__ ("cxx11"))) { }
+  // }
+  // namespace __gnu_cxx
+  // {
+  //   inline namespace __cxx11 __attribute__((__abi_tag__ ("cxx11"))) { }
+  // }
+  // ...
+  // namespace std {
+  //   some declarations for STL.
+  //   ...
+  //   namespace __cxx11 {
+  //     some declarations for STL.
+  //   }
+  // }
+  //
+  // Then in a real project, we observed false-positive ODR violations
+  // since some module units discard `__gnu_cxx::__cxx11` namespace while
+  // other module units don't discard `__gnu_cxx::__cxx11` namespace.
+  //
+  // This may imply that the ODR checking process is context sensitive.
+  // That said, the same type in different module units can be considered to be
+  // different if some module units discard the unused `__gnu_cxx::__cxx11` namespace
+  // while other module units don't. This is incorrect.
+  //
+  // This is a workaround to make things happen but we indeed to fix the ODR checking
+  // process indeed.
+  if (auto *ND = dyn_cast<NamedDecl>(D);
+      ND && ND->getAttr<AbiTagAttr>()) {
+    MarkDeclReachable(D);
+    return true;
+  }
+
+  return false;
+}
+
+bool ASTWriter::IsDeclModuleDiscardable(const Decl *ConstD) {
+  Decl *D = const_cast<Decl *>(ConstD);
+
+  if (!D->isDiscardedInGlobalModuleFragment())
+    return false;
+
+  // The Translation Unit should never be module discardable.
+  if (!D->getDeclContext()) {
+    assert(isa<TranslationUnitDecl>(D));
+    return false;
+  }
+
+  if (IsSpecialDeclNotDiscardable(D))
+    return false;
+
+  const DeclContext *DC = D->getNonTransparentDeclContext();
+  while (DC && DC->getParent() &&
+         !DC->getParent()->getNonTransparentContext()->isFileContext())
+    DC = DC->getParent()->getNonTransparentContext();
+
+  assert(DC && "Why is the decl not covered by file context?");
+  if (!DC->isFileContext() && !cast<Decl>(DC)->isDiscardedInGlobalModuleFragment()) {
+    MarkDeclReachable(D);
+    return false;
+  }
+
+  return true;
+}
+
+void ASTWriter::MarkDeclReachable(const Decl *ConstD) {
+  Decl *D = const_cast<Decl *>(ConstD);
+
+  if (!D || !D->isDiscardedInGlobalModuleFragment())
+    return;
+
+  D->setModuleOwnershipKind(Decl::ModuleOwnershipKind::ReachableWhenImported);
+  if (D->getNonTransparentDeclContext()->isFileContext()) {
+    // Update the decl contexts so that we can still find the decl with name
+    // lookup.
+    UpdatedDeclContexts.insert(D->getNonTransparentDeclContext());
+  }
+
+  auto Iter = ReachableMarkerCallbacks.find(D);
+  if (Iter != ReachableMarkerCallbacks.end()) {
+    for (Decl *ToBeMarked : Iter->second) {
+      MarkDeclReachable(ToBeMarked);
+      GetDeclRef(ToBeMarked);
+    }
+    ReachableMarkerCallbacks.erase(D);
+  }
+
+  DeclContext *DC = D->getNonTransparentDeclContext();
+  while (DC && DC->getParent() &&
+         !DC->getParent()->getNonTransparentContext()->isFileContext())
+    DC = DC->getParent()->getNonTransparentContext();
+
+  if (DC && !DC->isFileContext())
+    MarkDeclReachable(cast<Decl>(DC));
+}
+
 void ASTWriter::AddDeclRef(const Decl *D, RecordDataImpl &Record) {
+  MarkDeclReachable(D);
   Record.push_back(GetDeclRef(D));
 }
 
@@ -5695,6 +5835,8 @@ DeclID ASTWriter::GetDeclRef(const Decl *D) {
   if (D->isFromASTFile())
     return D->getGlobalID();
 
+  assert(!D->isDiscardedInGlobalModuleFragment() && "We shouldn't write discarded decl.\n");
+
   assert(!(reinterpret_cast<uintptr_t>(D) & 0x01) && "Invalid decl pointer");
   DeclID &ID = DeclIDs[D];
   if (ID == 0) {

clang/lib/Serialization/ASTWriterDecl.cpp

@@ -1987,6 +1987,8 @@ void ASTDeclWriter::VisitRedeclarable(Redeclarable<T> *D) {
     //
     // FIXME: This is not correct; when we reach an imported declaration we
     // won't emit its previous declaration.
+    Writer.MarkDeclReachable(D->getPreviousDecl());
+    Writer.MarkDeclReachable(MostRecent);
     (void)Writer.GetDeclRef(D->getPreviousDecl());
     (void)Writer.GetDeclRef(MostRecent);
   } else {

clang/test/CXX/basic/basic.scope/basic.scope.namespace/p2.cpp

@@ -30,7 +30,6 @@ module;
 
 void test_early() {
   in_header = 1; // expected-error {{use of undeclared identifier 'in_header'}}
-  // expected-note@* {{not visible}}
 
   global_module_fragment = 1; // expected-error {{use of undeclared identifier 'global_module_fragment'}}
 
@@ -53,10 +52,9 @@ import A;
 #endif
 
 void test_late() {
-  in_header = 1; // expected-error {{missing '#include "foo.h"'; 'in_header' must be declared before it is used}}
-  // expected-note@* {{not visible}}
+  in_header = 1; // expected-error {{use of undeclared identifier 'in_header'}}
 
-  global_module_fragment = 1; // expected-error {{missing '#include'; 'global_module_fragment' must be declared before it is used}}
+  global_module_fragment = 1; // expected-error {{use of undeclared identifier 'global_module_fragment'}}
 
   exported = 1;