[clang] 38b1858 - [analyzer][CTU] API for CTU macro expansions
Balazs Benics via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 22 02:07:15 PST 2021
Author: Balazs Benics
Date: 2021-02-22T11:12:22+01:00
New Revision: 38b185832e04ed0588c43161f5c3a8c0ce267497
URL: https://github.com/llvm/llvm-project/commit/38b185832e04ed0588c43161f5c3a8c0ce267497
DIFF: https://github.com/llvm/llvm-project/commit/38b185832e04ed0588c43161f5c3a8c0ce267497.diff
LOG: [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
Added:
Modified:
clang/include/clang/AST/ASTImporter.h
clang/include/clang/CrossTU/CrossTranslationUnit.h
clang/lib/AST/ASTImporter.cpp
clang/lib/CrossTU/CrossTranslationUnit.cpp
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
clang/test/Analysis/plist-macros-with-expansion-ctu.c
clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/ASTImporter.h b/clang/include/clang/AST/ASTImporter.h
index a6d822ba2ea6..630d220deff7 100644
--- a/clang/include/clang/AST/ASTImporter.h
+++ b/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; }
diff --git a/clang/include/clang/CrossTU/CrossTranslationUnit.h b/clang/include/clang/CrossTU/CrossTranslationUnit.h
index 027c6f16430b..b419b9be67cb 100644
--- a/clang/include/clang/CrossTU/CrossTranslationUnit.h
+++ b/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 @@ class CrossTranslationUnitContext {
/// 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 @@ class CrossTranslationUnitContext {
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>>;
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 333a94d0cdfc..f4dfc54b36cb 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/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;
}
diff --git a/clang/lib/CrossTU/CrossTranslationUnit.cpp b/clang/lib/CrossTU/CrossTranslationUnit.cpp
index e27779f91abc..640538cae3bb 100644
--- a/clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ b/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
diff --git a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
index a90bed096486..9842f3ace484 100644
--- a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ b/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);
}
diff --git a/clang/test/Analysis/plist-macros-with-expansion-ctu.c b/clang/test/Analysis/plist-macros-with-expansion-ctu.c
index 0fef72801e03..82d59249ecdb 100644
--- a/clang/test/Analysis/plist-macros-with-expansion-ctu.c
+++ b/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(&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 (&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(&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 (&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>
diff --git a/clang/unittests/CrossTU/CrossTranslationUnitTest.cpp b/clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
index 4e6fbeee86a3..03401f933b87 100644
--- a/clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ b/clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -91,26 +91,6 @@ class CTUASTConsumer : public clang::ASTConsumer {
*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);
More information about the cfe-commits
mailing list