[analyzer][CTU] API for CTU macro expansions

Removes `CrossTranslationUnitContext::getImportedFromSourceLocation`
Removes the corresponding unit-test segment.

Introduces the `CrossTranslationUnitContext::getMacroExpansionContextForSourceLocation`
which will return the macro expansion context for an imported TU. Also adds a
few implementation FIXME notes where applicable, since this feature is
not implemented yet. This fact is also noted as Doxygen comments.

Uplifts a few CTU LIT test to match the current **incomplete** behavior.

It is a regression to some extent since now we don't expand any
macros in imported TUs. At least we don't crash anymore.

Note that the introduced function is already covered by LIT tests.
Eg.: Analysis/plist-macros-with-expansion-ctu.c

Reviewed By: balazske, Szelethus

Differential Revision: https://reviews.llvm.org/D94673

clang/include/clang/AST/ASTImporter.h

@@ -93,8 +93,6 @@ class TypeSourceInfo;
     using NonEquivalentDeclSet = llvm::DenseSet<std::pair<Decl *, Decl *>>;
     using ImportedCXXBaseSpecifierMap =
         llvm::DenseMap<const CXXBaseSpecifier *, CXXBaseSpecifier *>;
-    using FileIDImportHandlerType =
-        std::function<void(FileID /*ToID*/, FileID /*FromID*/)>;
 
     enum class ODRHandlingType { Conservative, Liberal };
 
@@ -220,8 +218,6 @@ class TypeSourceInfo;
     };
 
   private:
-    FileIDImportHandlerType FileIDImportHandler;
-
     std::shared_ptr<ASTImporterSharedState> SharedState = nullptr;
 
     /// The path which we go through during the import of a given AST node.
@@ -324,14 +320,6 @@ class TypeSourceInfo;
 
     virtual ~ASTImporter();
 
-    /// Set a callback function for FileID import handling.
-    /// The function is invoked when a FileID is imported from the From context.
-    /// The imported FileID in the To context and the original FileID in the
-    /// From context is passed to it.
-    void setFileIDImportHandler(FileIDImportHandlerType H) {
-      FileIDImportHandler = H;
-    }
-
     /// Whether the importer will perform a minimal import, creating
     /// to-be-completed forward declarations when possible.
     bool isMinimalImport() const { return Minimal; }

clang/include/clang/CrossTU/CrossTranslationUnit.h

@@ -15,6 +15,7 @@
 #define LLVM_CLANG_CROSSTU_CROSSTRANSLATIONUNIT_H
 
 #include "clang/AST/ASTImporterSharedState.h"
+#include "clang/Analysis/MacroExpansionContext.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
@@ -182,21 +183,18 @@ public:
   /// Emit diagnostics for the user for potential configuration errors.
   void emitCrossTUDiagnostics(const IndexError &IE);
 
-  /// Determine the original source location in the original TU for an
-  /// imported source location.
+  /// Returns the MacroExpansionContext for the imported TU to which the given
+  /// source-location corresponds.
   /// \p ToLoc Source location in the imported-to AST.
-  /// \return Source location in the imported-from AST and the corresponding
-  /// ASTUnit object (the AST was loaded from a file using an internal ASTUnit
-  /// object that is returned here).
-  /// If any error happens (ToLoc is a non-imported source location) empty is
-  /// returned.
-  llvm::Optional<std::pair<SourceLocation /*FromLoc*/, ASTUnit *>>
-  getImportedFromSourceLocation(const clang::SourceLocation &ToLoc) const;
+  /// \note If any error happens such as \p ToLoc is a non-imported
+  ///       source-location, empty is returned.
+  /// \note Macro expansion tracking for imported TUs is not implemented yet.
+  ///       It returns empty unconditionally.
+  llvm::Optional<clang::MacroExpansionContext>
+  getMacroExpansionContextForSourceLocation(
+      const clang::SourceLocation &ToLoc) const;
 
 private:
-  using ImportedFileIDMap =
-      llvm::DenseMap<FileID, std::pair<FileID, ASTUnit *>>;
-
   void lazyInitImporterSharedSt(TranslationUnitDecl *ToTU);
   ASTImporter &getOrCreateASTImporter(ASTUnit *Unit);
   template <typename T>
@@ -217,14 +215,6 @@ private:
 
   ASTContext &Context;
   std::shared_ptr<ASTImporterSharedState> ImporterSharedSt;
-  /// Map of imported FileID's (in "To" context) to FileID in "From" context
-  /// and the ASTUnit for the From context.
-  /// This map is used by getImportedFromSourceLocation to lookup a FileID and
-  /// its Preprocessor when knowing only the FileID in the 'To' context. The
-  /// FileID could be imported by any of multiple 'From' ASTImporter objects.
-  /// we do not want to loop over all ASTImporter's to find the one that
-  /// imported the FileID.
-  ImportedFileIDMap ImportedFileIDs;
 
   using LoadResultTy = llvm::Expected<std::unique_ptr<ASTUnit>>;
 

clang/lib/AST/ASTImporter.cpp

@@ -8843,10 +8843,6 @@ Expected<FileID> ASTImporter::Import(FileID FromID, bool IsBuiltin) {
   assert(ToID.isValid() && "Unexpected invalid fileID was created.");
 
   ImportedFileIDs[FromID] = ToID;
-
-  if (FileIDImportHandler)
-    FileIDImportHandler(ToID, FromID);
-
   return ToID;
 }
 

clang/lib/CrossTU/CrossTranslationUnit.cpp

@@ -754,31 +754,15 @@ CrossTranslationUnitContext::getOrCreateASTImporter(ASTUnit *Unit) {
   ASTImporter *NewImporter = new ASTImporter(
       Context, Context.getSourceManager().getFileManager(), From,
       From.getSourceManager().getFileManager(), false, ImporterSharedSt);
-  NewImporter->setFileIDImportHandler([this, Unit](FileID ToID, FileID FromID) {
-    assert(ImportedFileIDs.find(ToID) == ImportedFileIDs.end() &&
-           "FileID already imported, should not happen.");
-    ImportedFileIDs[ToID] = std::make_pair(FromID, Unit);
-  });
   ASTUnitImporterMap[From.getTranslationUnitDecl()].reset(NewImporter);
   return *NewImporter;
 }
 
-llvm::Optional<std::pair<SourceLocation, ASTUnit *>>
-CrossTranslationUnitContext::getImportedFromSourceLocation(
+llvm::Optional<clang::MacroExpansionContext>
+CrossTranslationUnitContext::getMacroExpansionContextForSourceLocation(
     const clang::SourceLocation &ToLoc) const {
-  const SourceManager &SM = Context.getSourceManager();
-  auto DecToLoc = SM.getDecomposedLoc(ToLoc);
-
-  auto I = ImportedFileIDs.find(DecToLoc.first);
-  if (I == ImportedFileIDs.end())
-    return {};
-
-  FileID FromID = I->second.first;
-  clang::ASTUnit *Unit = I->second.second;
-  SourceLocation FromLoc =
-      Unit->getSourceManager().getComposedLoc(FromID, DecToLoc.second);
-
-  return std::make_pair(FromLoc, Unit);
+  // FIXME: Implement: Record such a context for every imported ASTUnit; lookup.
+  return llvm::None;
 }
 
 } // namespace cross_tu

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

@@ -830,9 +830,9 @@ getExpandedMacro(SourceLocation MacroExpansionLoc,
                  const cross_tu::CrossTranslationUnitContext &CTU,
                  const MacroExpansionContext &MacroExpansions,
                  const SourceManager &SM) {
-  if (auto LocAndUnit = CTU.getImportedFromSourceLocation(MacroExpansionLoc)) {
-    // TODO: Implement macro expansions for CTU.
-    return llvm::None;
+  if (auto CTUMacroExpCtx =
+          CTU.getMacroExpansionContextForSourceLocation(MacroExpansionLoc)) {
+    return CTUMacroExpCtx->getExpandedText(MacroExpansionLoc);
   }
   return MacroExpansions.getExpandedText(MacroExpansionLoc);
 }

clang/test/Analysis/plist-macros-with-expansion-ctu.c

@@ -2,13 +2,13 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -emit-pch -o %t/ctudir/plist-macros-ctu.c.ast %S/Inputs/plist-macros-ctu.c
 // RUN: cp %S/Inputs/plist-macros-with-expansion-ctu.c.externalDefMap.txt %t/ctudir/externalDefMap.txt
-
+//
 // RUN: %clang_analyze_cc1 -analyzer-checker=core \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
 // RUN:   -analyzer-config ctu-dir=%t/ctudir \
 // RUN:   -analyzer-config expand-macros=true \
 // RUN:   -analyzer-output=plist-multi-file -o %t.plist -verify %s
-// XFAIL: *
+//
 // Check the macro expansions from the plist output here, to make the test more
 // understandable.
 //   RUN: FileCheck --input-file=%t.plist %s
@@ -23,25 +23,30 @@ void test0() {
   F3(&X);
   *X = 1; // expected-warning{{Dereference of null pointer}}
 }
-// CHECK: <key>name</key><string>M1</string>
-// CHECK-NEXT: <key>expansion</key><string>*Z = (int *)0</string>
-
+// FIXME: Macro expansion for other TUs should also work.
+// CHECK:      <key>macro_expansions</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: </array>
 
 void test1() {
   int *X;
   F1(&X);
   *X = 1; // expected-warning{{Dereference of null pointer}}
 }
-// CHECK: <key>name</key><string>M</string>
-// CHECK-NEXT: <key>expansion</key><string>*X = (int *)0</string>
+
+// CHECK:      <key>macro_expansions</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: </array>
 
 void test2() {
   int *X;
   F2(&X);
   *X = 1; // expected-warning{{Dereference of null pointer}}
 }
-// CHECK: <key>name</key><string>M</string>
-// CHECK-NEXT: <key>expansion</key><string>*Y = (int *)0</string>
+
+// CHECK:      <key>macro_expansions</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: </array>
 
 #define M F1(&X)
 
@@ -50,10 +55,20 @@ void test3() {
   M;
   *X = 1; // expected-warning{{Dereference of null pointer}}
 }
-// CHECK: <key>name</key><string>M</string>
-// CHECK-NEXT: <key>expansion</key><string>F1(&amp;X)</string>
-// CHECK: <key>name</key><string>M</string>
-// CHECK-NEXT: <key>expansion</key><string>*X = (int *)0</string>
+// Macro expansions for the main TU still works, even in CTU mode.
+// CHECK:      <key>macro_expansions</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT:  <dict>
+// CHECK-NEXT:   <key>location</key>
+// CHECK-NEXT:   <dict>
+// CHECK-NEXT:    <key>line</key><integer>55</integer>
+// CHECK-NEXT:    <key>col</key><integer>3</integer>
+// CHECK-NEXT:    <key>file</key><integer>0</integer>
+// CHECK-NEXT:   </dict>
+// CHECK-NEXT:   <key>name</key><string>M</string>
+// CHECK-NEXT:   <key>expansion</key><string>F1 (&amp;X )</string>
+// CHECK-NEXT:  </dict>
+// CHECK-NEXT: </array>
 
 #undef M
 #define M F2(&X)
@@ -64,10 +79,19 @@ void test4() {
   *X = 1; // expected-warning{{Dereference of null pointer}}
 }
 
-// CHECK: <key>name</key><string>M</string>
-// CHECK-NEXT: <key>expansion</key><string>F2(&amp;X)</string>
-// CHECK: <key>name</key><string>M</string>
-// CHECK-NEXT: <key>expansion</key><string>*Y = (int *)0</string>
+// CHECK:      <key>macro_expansions</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT:  <dict>
+// CHECK-NEXT:   <key>location</key>
+// CHECK-NEXT:   <dict>
+// CHECK-NEXT:    <key>line</key><integer>78</integer>
+// CHECK-NEXT:    <key>col</key><integer>3</integer>
+// CHECK-NEXT:    <key>file</key><integer>0</integer>
+// CHECK-NEXT:   </dict>
+// CHECK-NEXT:   <key>name</key><string>M</string>
+// CHECK-NEXT:   <key>expansion</key><string>F2 (&amp;X )</string>
+// CHECK-NEXT:  </dict>
+// CHECK-NEXT: </array>
 
 void test_h() {
   int *X;
@@ -75,5 +99,6 @@ void test_h() {
   *X = 1; // expected-warning{{Dereference of null pointer}}
 }
 
-// CHECK: <key>name</key><string>M_H</string>
-// CHECK-NEXT: <key>expansion</key><string>*A = (int *)0</string>
+// CHECK:      <key>macro_expansions</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: </array>

clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

@@ -91,26 +91,6 @@ public:
       *Success = NewFD && NewFD->hasBody() && !OrigFDHasBody;
 
       if (NewFD) {
-        // Check GetImportedFromSourceLocation.
-        llvm::Optional<std::pair<SourceLocation, ASTUnit *>> SLocResult =
-            CTU.getImportedFromSourceLocation(NewFD->getLocation());
-        EXPECT_TRUE(SLocResult);
-        if (SLocResult) {
-          SourceLocation OrigSLoc = (*SLocResult).first;
-          ASTUnit *OrigUnit = (*SLocResult).second;
-          // OrigUnit is created internally by CTU (is not the
-          // ASTWithDefinition).
-          TranslationUnitDecl *OrigTU =
-              OrigUnit->getASTContext().getTranslationUnitDecl();
-          const FunctionDecl *FDWithDefinition = FindFInTU(OrigTU);
-          EXPECT_TRUE(FDWithDefinition);
-          if (FDWithDefinition) {
-            EXPECT_EQ(FDWithDefinition->getName(), "f");
-            EXPECT_TRUE(FDWithDefinition->isThisDeclarationADefinition());
-            EXPECT_EQ(OrigSLoc, FDWithDefinition->getLocation());
-          }
-        }
-
         // Check parent map.
         const DynTypedNodeList ParentsAfterImport =
             Ctx.getParentMapContext().getParents<Decl>(*FD);