r366449 - [CrossTU] Add a function to retrieve original source location.
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 18 08:38:36 PDT 2019
Hi Balazs,
Your commit introduces a layering violation: ASTImporter (which is inside
clangAST library) cannot depend on ASTUnit (which is inside clangFrontend
library).
I'm afraid I'll have to revert the commit to unbreak our integrate.
On Thu, Jul 18, 2019 at 5:22 PM Balazs Keri via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Author: balazske
> Date: Thu Jul 18 08:23:10 2019
> New Revision: 366449
>
> URL: http://llvm.org/viewvc/llvm-project?rev=366449&view=rev
> Log:
> [CrossTU] Add a function to retrieve original source location.
>
> Summary:
> A new function will be added to get the original SourceLocation
> for a SourceLocation that was imported as result of getCrossTUDefinition.
> The returned SourceLocation is in the context of the (original)
> SourceManager for the original source file. Additionally the
> ASTUnit object for that source file is returned. This is needed
> to get a SourceManager to operate on with the returned source location.
>
> The new function works if multiple different source files are loaded
> with the same CrossTU context.
>
> This patch can be treated as part of a bigger change that is needed to
> improve macro expansion handliong at plist generation.
>
> Reviewers: martong, shafik, a_sidorin, xazax.hun
>
> Reviewed By: martong
>
> Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D64554
>
> Modified:
> cfe/trunk/include/clang/AST/ASTImporter.h
> cfe/trunk/include/clang/AST/ASTImporterSharedState.h
> cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
> cfe/trunk/lib/AST/ASTImporter.cpp
> cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
> cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp
>
> Modified: cfe/trunk/include/clang/AST/ASTImporter.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTImporter.h?rev=366449&r1=366448&r2=366449&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/AST/ASTImporter.h (original)
> +++ cfe/trunk/include/clang/AST/ASTImporter.h Thu Jul 18 08:23:10 2019
> @@ -34,6 +34,7 @@ namespace clang {
>
> class ASTContext;
> class ASTImporterSharedState;
> +class ASTUnit;
> class Attr;
> class CXXBaseSpecifier;
> class CXXCtorInitializer;
> @@ -229,6 +230,11 @@ class TypeSourceInfo;
> /// The file managers we're importing to and from.
> FileManager &ToFileManager, &FromFileManager;
>
> + /// The ASTUnit for the From context, if any.
> + /// This is used to create a mapping of imported ('To') FileID's to
> the
> + /// original ('From') FileID and ASTUnit.
> + ASTUnit *FromUnit;
> +
> /// Whether to perform a minimal import.
> bool Minimal;
>
> @@ -277,6 +283,11 @@ class TypeSourceInfo;
>
> void AddToLookupTable(Decl *ToD);
>
> + ASTImporter(ASTContext &ToContext, FileManager &ToFileManager,
> + ASTContext &FromContext, FileManager &FromFileManager,
> + ASTUnit *FromUnit, bool MinimalImport,
> + std::shared_ptr<ASTImporterSharedState> SharedState);
> +
> protected:
> /// Can be overwritten by subclasses to implement their own import
> logic.
> /// The overwritten method should call this method if it didn't
> import the
> @@ -287,7 +298,6 @@ class TypeSourceInfo;
> virtual bool returnWithErrorInTest() { return false; };
>
> public:
> -
> /// \param ToContext The context we'll be importing into.
> ///
> /// \param ToFileManager The file manager we'll be importing into.
> @@ -307,6 +317,23 @@ class TypeSourceInfo;
> ASTContext &FromContext, FileManager &FromFileManager,
> bool MinimalImport,
> std::shared_ptr<ASTImporterSharedState> SharedState =
> nullptr);
> + /// \param ToContext The context we'll be importing into.
> + ///
> + /// \param ToFileManager The file manager we'll be importing into.
> + ///
> + /// \param FromUnit Pointer to an ASTUnit that contains the context
> and
> + /// file manager to import from.
> + ///
> + /// \param MinimalImport If true, the importer will attempt to import
> + /// as little as it can, e.g., by importing declarations as forward
> + /// declarations that can be completed at a later point.
> + ///
> + /// \param SharedState The importer specific lookup table which may be
> + /// shared amongst several ASTImporter objects.
> + /// If not set then the original C/C++ lookup is used.
> + ASTImporter(ASTContext &ToContext, FileManager &ToFileManager,
> + ASTUnit &FromUnit, bool MinimalImport,
> + std::shared_ptr<ASTImporterSharedState> SharedState =
> nullptr);
>
> virtual ~ASTImporter();
>
>
> Modified: cfe/trunk/include/clang/AST/ASTImporterSharedState.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTImporterSharedState.h?rev=366449&r1=366448&r2=366449&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/AST/ASTImporterSharedState.h (original)
> +++ cfe/trunk/include/clang/AST/ASTImporterSharedState.h Thu Jul 18
> 08:23:10 2019
> @@ -17,18 +17,24 @@
>
> #include "clang/AST/ASTImporterLookupTable.h"
> #include "clang/AST/Decl.h"
> +#include "clang/Basic/SourceLocation.h"
> #include "llvm/ADT/DenseMap.h"
> // FIXME We need this because of ImportError.
> #include "clang/AST/ASTImporter.h"
>
> namespace clang {
>
> +class ASTUnit;
> class TranslationUnitDecl;
>
> /// Importer specific state, which may be shared amongst several
> ASTImporter
> /// objects.
> class ASTImporterSharedState {
> +public:
> + using ImportedFileIDMap =
> + llvm::DenseMap<FileID, std::pair<FileID, ASTUnit *>>;
>
> +private:
> /// Pointer to the import specific lookup table.
> std::unique_ptr<ASTImporterLookupTable> LookupTable;
>
> @@ -43,6 +49,16 @@ class ASTImporterSharedState {
> // FIXME put ImportedFromDecls here!
> // And from that point we can better encapsulate the lookup table.
>
> + /// Map of imported FileID's (in "To" context) to FileID in "From"
> context
> + /// and the ASTUnit that contains the preprocessor and source manager
> for the
> + /// "From" FileID. This map is used to lookup a FileID and its
> SourceManager
> + /// when knowing only the FileID in the 'To' context. The FileID could
> be
> + /// imported by any of multiple ASTImporter objects. The map is used
> because
> + /// we do not want to loop over all ASTImporter's to find the one that
> + /// imported the FileID. (The ASTUnit is usable to get the
> SourceManager and
> + /// additional data.)
> + ImportedFileIDMap ImportedFileIDs;
> +
> public:
> ASTImporterSharedState() = default;
>
> @@ -75,6 +91,12 @@ public:
> void setImportDeclError(Decl *To, ImportError Error) {
> ImportErrors[To] = Error;
> }
> +
> + ImportedFileIDMap &getImportedFileIDs() { return ImportedFileIDs; }
> +
> + const ImportedFileIDMap &getImportedFileIDs() const {
> + return ImportedFileIDs;
> + }
> };
>
> } // namespace clang
>
> Modified: cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h?rev=366449&r1=366448&r2=366449&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h (original)
> +++ cfe/trunk/include/clang/CrossTU/CrossTranslationUnit.h Thu Jul 18
> 08:23:10 2019
> @@ -153,8 +153,10 @@ public:
> /// was passed to the constructor.
> ///
> /// \return Returns the resulting definition or an error.
> - llvm::Expected<const FunctionDecl *> importDefinition(const
> FunctionDecl *FD);
> - llvm::Expected<const VarDecl *> importDefinition(const VarDecl *VD);
> + llvm::Expected<const FunctionDecl *> importDefinition(const
> FunctionDecl *FD,
> + ASTUnit *Unit);
> + llvm::Expected<const VarDecl *> importDefinition(const VarDecl *VD,
> + ASTUnit *Unit);
>
> /// Get a name to identify a named decl.
> static std::string getLookupName(const NamedDecl *ND);
> @@ -162,9 +164,19 @@ 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.
> + /// \p ToLoc Source location in the imported-to AST.
> + /// \return Source location in the imported-from AST and the
> corresponding
> + /// ASTUnit.
> + /// 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;
> +
> private:
> void lazyInitImporterSharedSt(TranslationUnitDecl *ToTU);
> - ASTImporter &getOrCreateASTImporter(ASTContext &From);
> + ASTImporter &getOrCreateASTImporter(ASTUnit *Unit);
> template <typename T>
> llvm::Expected<const T *> getCrossTUDefinitionImpl(const T *D,
> StringRef CrossTUDir,
> @@ -174,7 +186,7 @@ private:
> const T *findDefInDeclContext(const DeclContext *DC,
> StringRef LookupName);
> template <typename T>
> - llvm::Expected<const T *> importDefinitionImpl(const T *D);
> + llvm::Expected<const T *> importDefinitionImpl(const T *D, ASTUnit
> *Unit);
>
> llvm::StringMap<std::unique_ptr<clang::ASTUnit>> FileASTUnitMap;
> llvm::StringMap<clang::ASTUnit *> NameASTUnitMap;
>
> Modified: cfe/trunk/lib/AST/ASTImporter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=366449&r1=366448&r2=366449&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/AST/ASTImporter.cpp (original)
> +++ cfe/trunk/lib/AST/ASTImporter.cpp Thu Jul 18 08:23:10 2019
> @@ -12,9 +12,9 @@
>
> //===----------------------------------------------------------------------===//
>
> #include "clang/AST/ASTImporter.h"
> -#include "clang/AST/ASTImporterSharedState.h"
> #include "clang/AST/ASTContext.h"
> #include "clang/AST/ASTDiagnostic.h"
> +#include "clang/AST/ASTImporterSharedState.h"
> #include "clang/AST/ASTStructuralEquivalence.h"
> #include "clang/AST/Attr.h"
> #include "clang/AST/Decl.h"
> @@ -52,13 +52,14 @@
> #include "clang/Basic/SourceLocation.h"
> #include "clang/Basic/SourceManager.h"
> #include "clang/Basic/Specifiers.h"
> +#include "clang/Frontend/ASTUnit.h"
> #include "llvm/ADT/APSInt.h"
> #include "llvm/ADT/ArrayRef.h"
> #include "llvm/ADT/DenseMap.h"
> #include "llvm/ADT/None.h"
> #include "llvm/ADT/Optional.h"
> -#include "llvm/ADT/ScopeExit.h"
> #include "llvm/ADT/STLExtras.h"
> +#include "llvm/ADT/ScopeExit.h"
> #include "llvm/ADT/SmallVector.h"
> #include "llvm/Support/Casting.h"
> #include "llvm/Support/ErrorHandling.h"
> @@ -7716,9 +7717,22 @@ ASTImporter::ASTImporter(ASTContext &ToC
> ASTContext &FromContext, FileManager
> &FromFileManager,
> bool MinimalImport,
> std::shared_ptr<ASTImporterSharedState>
> SharedState)
> + : ASTImporter(ToContext, ToFileManager, FromContext, FromFileManager,
> + nullptr, MinimalImport, SharedState){}
> +ASTImporter::ASTImporter(ASTContext &ToContext, FileManager
> &ToFileManager,
> + ASTUnit &FromUnit, bool MinimalImport,
> + std::shared_ptr<ASTImporterSharedState>
> SharedState)
> + : ASTImporter(ToContext, ToFileManager, FromUnit.getASTContext(),
> + FromUnit.getFileManager(), &FromUnit, MinimalImport,
> + SharedState){}
> +
> +ASTImporter::ASTImporter(ASTContext &ToContext, FileManager
> &ToFileManager,
> + ASTContext &FromContext, FileManager
> &FromFileManager,
> + ASTUnit *FromUnit, bool MinimalImport,
> + std::shared_ptr<ASTImporterSharedState>
> SharedState)
> : SharedState(SharedState), ToContext(ToContext),
> FromContext(FromContext),
> ToFileManager(ToFileManager), FromFileManager(FromFileManager),
> - Minimal(MinimalImport) {
> + FromUnit(FromUnit), Minimal(MinimalImport) {
>
> // Create a default state without the lookup table: LLDB case.
> if (!SharedState) {
> @@ -8421,6 +8435,13 @@ Expected<FileID> ASTImporter::Import(Fil
> assert(ToID.isValid() && "Unexpected invalid fileID was created.");
>
> ImportedFileIDs[FromID] = ToID;
> + if (FromUnit) {
> + assert(SharedState->getImportedFileIDs().find(ToID) ==
> + SharedState->getImportedFileIDs().end() &&
> + "FileID already imported!");
> + SharedState->getImportedFileIDs()[ToID] = std::make_pair(FromID,
> FromUnit);
> + }
> +
> return ToID;
> }
>
>
> Modified: cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp?rev=366449&r1=366448&r2=366449&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp (original)
> +++ cfe/trunk/lib/CrossTU/CrossTranslationUnit.cpp Thu Jul 18 08:23:10 2019
> @@ -295,7 +295,7 @@ llvm::Expected<const T *> CrossTranslati
>
> TranslationUnitDecl *TU =
> Unit->getASTContext().getTranslationUnitDecl();
> if (const T *ResultDecl = findDefInDeclContext<T>(TU, LookupName))
> - return importDefinition(ResultDecl);
> + return importDefinition(ResultDecl, Unit);
> return llvm::make_error<IndexError>(index_error_code::failed_import);
> }
>
> @@ -411,10 +411,13 @@ llvm::Expected<ASTUnit *> CrossTranslati
>
> template <typename T>
> llvm::Expected<const T *>
> -CrossTranslationUnitContext::importDefinitionImpl(const T *D) {
> +CrossTranslationUnitContext::importDefinitionImpl(const T *D, ASTUnit
> *Unit) {
> assert(hasBodyOrInit(D) && "Decls to be imported should have body or
> init.");
>
> - ASTImporter &Importer = getOrCreateASTImporter(D->getASTContext());
> + assert(&D->getASTContext() == &Unit->getASTContext() &&
> + "ASTContext of Decl and the unit should match.");
> + ASTImporter &Importer = getOrCreateASTImporter(Unit);
> +
> auto ToDeclOrError = Importer.Import(D);
> if (!ToDeclOrError) {
> handleAllErrors(ToDeclOrError.takeError(),
> @@ -441,13 +444,15 @@ CrossTranslationUnitContext::importDefin
> }
>
> llvm::Expected<const FunctionDecl *>
> -CrossTranslationUnitContext::importDefinition(const FunctionDecl *FD) {
> - return importDefinitionImpl(FD);
> +CrossTranslationUnitContext::importDefinition(const FunctionDecl *FD,
> + ASTUnit *Unit) {
> + return importDefinitionImpl(FD, Unit);
> }
>
> llvm::Expected<const VarDecl *>
> -CrossTranslationUnitContext::importDefinition(const VarDecl *VD) {
> - return importDefinitionImpl(VD);
> +CrossTranslationUnitContext::importDefinition(const VarDecl *VD,
> + ASTUnit *Unit) {
> + return importDefinitionImpl(VD, Unit);
> }
>
> void CrossTranslationUnitContext::lazyInitImporterSharedSt(
> @@ -457,17 +462,40 @@ void CrossTranslationUnitContext::lazyIn
> }
>
> ASTImporter &
> -CrossTranslationUnitContext::getOrCreateASTImporter(ASTContext &From) {
> +CrossTranslationUnitContext::getOrCreateASTImporter(ASTUnit *Unit) {
> + ASTContext &From = Unit->getASTContext();
> +
> auto I = ASTUnitImporterMap.find(From.getTranslationUnitDecl());
> if (I != ASTUnitImporterMap.end())
> return *I->second;
> lazyInitImporterSharedSt(Context.getTranslationUnitDecl());
> - ASTImporter *NewImporter = new ASTImporter(
> - Context, Context.getSourceManager().getFileManager(), From,
> - From.getSourceManager().getFileManager(), false, ImporterSharedSt);
> + ASTImporter *NewImporter =
> + new ASTImporter(Context,
> Context.getSourceManager().getFileManager(),
> + *Unit, false, ImporterSharedSt);
> ASTUnitImporterMap[From.getTranslationUnitDecl()].reset(NewImporter);
> return *NewImporter;
> }
>
> +llvm::Optional<std::pair<SourceLocation, ASTUnit *>>
> +CrossTranslationUnitContext::getImportedFromSourceLocation(
> + const clang::SourceLocation &ToLoc) const {
> + if (!ImporterSharedSt)
> + return {};
> +
> + const SourceManager &SM = Context.getSourceManager();
> + auto DecToLoc = SM.getDecomposedLoc(ToLoc);
> +
> + auto I = ImporterSharedSt->getImportedFileIDs().find(DecToLoc.first);
> + if (I == ImporterSharedSt->getImportedFileIDs().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);
> +}
> +
> } // namespace cross_tu
> } // namespace clang
>
> Modified: cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp?rev=366449&r1=366448&r2=366449&view=diff
>
> ==============================================================================
> --- cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp (original)
> +++ cfe/trunk/unittests/CrossTU/CrossTranslationUnitTest.cpp Thu Jul 18
> 08:23:10 2019
> @@ -28,13 +28,18 @@ public:
> : CTU(CI), Success(Success) {}
>
> void HandleTranslationUnit(ASTContext &Ctx) {
> + auto FindFInTU = [](const TranslationUnitDecl *TU) {
> + const FunctionDecl *FD = nullptr;
> + for (const Decl *D : TU->decls()) {
> + FD = dyn_cast<FunctionDecl>(D);
> + if (FD && FD->getName() == "f")
> + break;
> + }
> + return FD;
> + };
> +
> const TranslationUnitDecl *TU = Ctx.getTranslationUnitDecl();
> - const FunctionDecl *FD = nullptr;
> - for (const Decl *D : TU->decls()) {
> - FD = dyn_cast<FunctionDecl>(D);
> - if (FD && FD->getName() == "f")
> - break;
> - }
> + const FunctionDecl *FD = FindFInTU(TU);
> assert(FD && FD->getName() == "f");
> bool OrigFDHasBody = FD->hasBody();
>
> @@ -78,6 +83,28 @@ public:
> if (NewFDorError) {
> const FunctionDecl *NewFD = *NewFDorError;
> *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());
> + }
> + }
> + }
> }
> }
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
--
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190718/cfd0b2cd/attachment-0001.html>
More information about the cfe-commits
mailing list