[clang][ExtractAPI] Fix quirks in interaction with submodules (llvm#105868) (#9193)

Extension SGFs require the module system to be enabled in order to discover which module defines the extended external type.
This patch ensures the following:
- Associate symbols with their top level module name, and that only top level modules are considered as modules for emitting extension SGFs.
- Ensure we don't drop macro definitions that came from a submodule. To this end look at all defined macros in `PPCalbacks::EndOfMainFile` instead of relying on `PPCallbacks::MacroDefined` being called to detect a macro definition.

rdar://123020565

Author: daniel-grumberg

clang/include/clang/ExtractAPI/DeclarationFragments.h

@@ -411,9 +411,9 @@ class DeclarationFragmentsBuilder {
   /// Build DeclarationFragments for a macro.
   ///
   /// \param Name name of the macro.
-  /// \param MD the associated MacroDirective.
+  /// \param MI the associated MacroInfo.
   static DeclarationFragments getFragmentsForMacro(StringRef Name,
-                                                   const MacroDirective *MD);
+                                                   const MacroInfo *MI);
 
   /// Build DeclarationFragments for a typedef \p TypedefNameDecl.
   static DeclarationFragments

clang/include/clang/ExtractAPI/ExtractAPIVisitor.h

@@ -213,7 +213,7 @@ class ExtractAPIVisitorBase : public RecursiveASTVisitor<Derived> {
 
   StringRef getOwningModuleName(const Decl &D) {
     if (auto *OwningModule = D.getImportedOwningModule())
-      return OwningModule->Name;
+      return OwningModule->getTopLevelModule()->Name;
 
     return {};
   }

clang/lib/ExtractAPI/DeclarationFragments.cpp

@@ -1327,14 +1327,12 @@ DeclarationFragmentsBuilder::getFragmentsForFunctionTemplateSpecialization(
 
 DeclarationFragments
 DeclarationFragmentsBuilder::getFragmentsForMacro(StringRef Name,
-                                                  const MacroDirective *MD) {
+                                                  const MacroInfo *MI) {
   DeclarationFragments Fragments;
   Fragments.append("#define", DeclarationFragments::FragmentKind::Keyword)
       .appendSpace();
   Fragments.append(Name, DeclarationFragments::FragmentKind::Identifier);
 
-  auto *MI = MD->getMacroInfo();
-
   if (MI->isFunctionLike()) {
     Fragments.append("(", DeclarationFragments::FragmentKind::Text);
     unsigned numParameters = MI->getNumParams();

clang/lib/ExtractAPI/ExtractAPIConsumer.cpp

@@ -286,78 +286,59 @@ class MacroCallback : public PPCallbacks {
   MacroCallback(const SourceManager &SM, APISet &API, Preprocessor &PP)
       : SM(SM), API(API), PP(PP) {}
 
-  void MacroDefined(const Token &MacroNameToken,
-                    const MacroDirective *MD) override {
-    auto *MacroInfo = MD->getMacroInfo();
+  void EndOfMainFile() override {
+    for (const auto &M : PP.macros()) {
+      auto *II = M.getFirst();
+      auto MD = PP.getMacroDefinition(II);
+      auto *MI = MD.getMacroInfo();
 
-    if (MacroInfo->isBuiltinMacro())
-      return;
+      if (!MI)
+        continue;
 
-    auto SourceLoc = MacroNameToken.getLocation();
-    if (SM.isWrittenInBuiltinFile(SourceLoc) ||
-        SM.isWrittenInCommandLineFile(SourceLoc))
-      return;
+      // Ignore header guard macros
+      if (MI->isUsedForHeaderGuard())
+        continue;
 
-    PendingMacros.emplace_back(MacroNameToken, MD);
-  }
+      // Ignore builtin macros and ones defined via the command line.
+      if (MI->isBuiltinMacro())
+        continue;
 
-  // If a macro gets undefined at some point during preprocessing of the inputs
-  // it means that it isn't an exposed API and we should therefore not add a
-  // macro definition for it.
-  void MacroUndefined(const Token &MacroNameToken, const MacroDefinition &MD,
-                      const MacroDirective *Undef) override {
-    // If this macro wasn't previously defined we don't need to do anything
-    // here.
-    if (!Undef)
-      return;
-
-    llvm::erase_if(PendingMacros, [&MD, this](const PendingMacro &PM) {
-      return MD.getMacroInfo()->isIdenticalTo(*PM.MD->getMacroInfo(), PP,
-                                              /*Syntactically*/ false);
-    });
-  }
+      auto DefLoc = MI->getDefinitionLoc();
 
-  void EndOfMainFile() override {
-    for (auto &PM : PendingMacros) {
-      // `isUsedForHeaderGuard` is only set when the preprocessor leaves the
-      // file so check for it here.
-      if (PM.MD->getMacroInfo()->isUsedForHeaderGuard())
+      if (SM.isWrittenInBuiltinFile(DefLoc) ||
+          SM.isWrittenInCommandLineFile(DefLoc))
         continue;
 
-      if (!shouldMacroBeIncluded(PM))
+      auto AssociatedModuleMacros = MD.getModuleMacros();
+      StringRef OwningModuleName;
+      if (!AssociatedModuleMacros.empty())
+        OwningModuleName = AssociatedModuleMacros.back()
+                               ->getOwningModule()
+                               ->getTopLevelModuleName();
+
+      if (!shouldMacroBeIncluded(DefLoc, OwningModuleName))
         continue;
 
-      StringRef Name = PM.MacroNameToken.getIdentifierInfo()->getName();
-      PresumedLoc Loc = SM.getPresumedLoc(PM.MacroNameToken.getLocation());
+      StringRef Name = II->getName();
+      PresumedLoc Loc = SM.getPresumedLoc(DefLoc);
       SmallString<128> USR;
-      index::generateUSRForMacro(Name, PM.MacroNameToken.getLocation(), SM,
-                                 USR);
-
+      index::generateUSRForMacro(Name, DefLoc, SM, USR);
       API.createRecord<extractapi::MacroDefinitionRecord>(
           USR, Name, SymbolReference(), Loc,
-          DeclarationFragmentsBuilder::getFragmentsForMacro(Name, PM.MD),
+          DeclarationFragmentsBuilder::getFragmentsForMacro(Name, MI),
           DeclarationFragmentsBuilder::getSubHeadingForMacro(Name),
-          SM.isInSystemHeader(PM.MacroNameToken.getLocation()));
+          SM.isInSystemHeader(DefLoc));
     }
-
-    PendingMacros.clear();
   }
 
-protected:
-  struct PendingMacro {
-    Token MacroNameToken;
-    const MacroDirective *MD;
-
-    PendingMacro(const Token &MacroNameToken, const MacroDirective *MD)
-        : MacroNameToken(MacroNameToken), MD(MD) {}
-  };
-
-  virtual bool shouldMacroBeIncluded(const PendingMacro &PM) { return true; }
+  virtual bool shouldMacroBeIncluded(const SourceLocation &MacroLoc,
+                                     StringRef ModuleName) {
+    return true;
+  }
 
   const SourceManager &SM;
   APISet &API;
   Preprocessor &PP;
-  llvm::SmallVector<PendingMacro> PendingMacros;
 };
 
 class APIMacroCallback : public MacroCallback {
@@ -366,9 +347,10 @@ class APIMacroCallback : public MacroCallback {
                    LocationFileChecker &LCF)
       : MacroCallback(SM, API, PP), LCF(LCF) {}
 
-  bool shouldMacroBeIncluded(const PendingMacro &PM) override {
+  bool shouldMacroBeIncluded(const SourceLocation &MacroLoc,
+                             StringRef ModuleName) override {
     // Do not include macros from external files
-    return LCF(PM.MacroNameToken.getLocation());
+    return LCF(MacroLoc) || API.ProductName == ModuleName;
   }
 
 private:

clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp

@@ -928,8 +928,8 @@ bool SymbolGraphSerializer::traverseObjCCategoryRecord(
     return true;
 
   auto *CurrentModule = ModuleForCurrentSymbol;
-  if (Record->isExtendingExternalModule())
-    ModuleForCurrentSymbol = &ExtendedModules[Record->Interface.Source];
+  if (auto ModuleExtendedByRecord = Record->getExtendedExternalModule())
+    ModuleForCurrentSymbol = &ExtendedModules[*ModuleExtendedByRecord];
 
   if (!walkUpFromObjCCategoryRecord(Record))
     return false;

clang/test/ExtractAPI/bool.c

@@ -2,8 +2,8 @@
 // RUN: split-file %s %t
 // RUN: sed -e "s@INPUT_DIR@%{/t:regex_replacement}@g" \
 // RUN: %t/reference.output.json.in >> %t/reference.output.json
-// RUN: %clang -extract-api --pretty-sgf -target arm64-apple-macosx \
-// RUN: %t/input.h -o %t/output.json
+// RUN: %clang_cc1 -extract-api --product-name=BoolTest --pretty-sgf -triple arm64-apple-macosx \
+// RUN:   %t/input.h -o %t/output.json
 
 // Generator version is not consistent across test runs, normalize it.
 // RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
@@ -15,7 +15,7 @@
 bool Foo;
 
 bool IsFoo(bool Bar);
-/// expected-no-diagnostics
+// expected-no-diagnostics
 
 //--- reference.output.json.in
 {
@@ -28,7 +28,7 @@ bool IsFoo(bool Bar);
     "generator": "?"
   },
   "module": {
-    "name": "",
+    "name": "BoolTest",
     "platform": {
       "architecture": "arm64",
       "operatingSystem": {