Author: hokein Date: Fri Jul 19 01:33:39 2019 New Revision: 366541 URL: http://llvm.org/viewvc/llvm-project?rev=366541&view=rev Log: [clangd] cleanup: unify the implemenation of checking a location is inside main file.
Summary: We have variant implementations in the codebase, this patch unifies them. Reviewers: ilya-biryukov, kadircet Subscribers: MaskRay, jkorous, arphaman, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D64915 Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp clang-tools-extra/trunk/clangd/Diagnostics.cpp clang-tools-extra/trunk/clangd/Headers.cpp clang-tools-extra/trunk/clangd/IncludeFixer.cpp clang-tools-extra/trunk/clangd/Quality.cpp clang-tools-extra/trunk/clangd/SourceCode.cpp clang-tools-extra/trunk/clangd/SourceCode.h clang-tools-extra/trunk/clangd/XRefs.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp clang-tools-extra/trunk/clangd/refactor/Rename.cpp clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=366541&r1=366540&r2=366541&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Fri Jul 19 01:33:39 2019 @@ -68,7 +68,7 @@ public: bool HandleTopLevelDecl(DeclGroupRef DG) override { for (Decl *D : DG) { auto &SM = D->getASTContext().getSourceManager(); - if (!SM.isWrittenInMainFile(SM.getExpansionLoc(D->getLocation()))) + if (!isInsideMainFile(D->getLocation(), SM)) continue; // ObjCMethodDecl are not actually top-level decls. @@ -355,8 +355,7 @@ ParsedAST::build(std::unique_ptr<Compile // those might take us into a preamble file as well. bool IsInsideMainFile = Info.hasSourceManager() && - Info.getSourceManager().isWrittenInMainFile( - Info.getSourceManager().getFileLoc(Info.getLocation())); + isInsideMainFile(Info.getLocation(), Info.getSourceManager()); if (IsInsideMainFile && tidy::ShouldSuppressDiagnostic( DiagLevel, Info, *CTContext, /* CheckMacroExpansion = */ false)) { Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=366541&r1=366540&r2=366541&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original) +++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Fri Jul 19 01:33:39 2019 @@ -140,15 +140,11 @@ void adjustDiagFromHeader(Diag &D, const D.Message = llvm::Twine("in included file: ", D.Message).str(); } -bool isInsideMainFile(const SourceLocation Loc, const SourceManager &M) { - return Loc.isValid() && M.isWrittenInMainFile(M.getFileLoc(Loc)); -} - bool isInsideMainFile(const clang::Diagnostic &D) { if (!D.hasSourceManager()) return false; - return isInsideMainFile(D.getLocation(), D.getSourceManager()); + return clangd::isInsideMainFile(D.getLocation(), D.getSourceManager()); } bool isNote(DiagnosticsEngine::Level L) { Modified: clang-tools-extra/trunk/clangd/Headers.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Headers.cpp?rev=366541&r1=366540&r2=366541&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Headers.cpp (original) +++ clang-tools-extra/trunk/clangd/Headers.cpp Fri Jul 19 01:33:39 2019 @@ -35,7 +35,7 @@ public: llvm::StringRef /*RelativePath*/, const Module * /*Imported*/, SrcMgr::CharacteristicKind FileKind) override { - if (SM.isWrittenInMainFile(HashLoc)) { + if (isInsideMainFile(HashLoc, SM)) { Out->MainFileIncludes.emplace_back(); auto &Inc = Out->MainFileIncludes.back(); Inc.R = halfOpenToRange(SM, FilenameRange); Modified: clang-tools-extra/trunk/clangd/IncludeFixer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/IncludeFixer.cpp?rev=366541&r1=366540&r2=366541&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/IncludeFixer.cpp (original) +++ clang-tools-extra/trunk/clangd/IncludeFixer.cpp Fri Jul 19 01:33:39 2019 @@ -318,7 +318,7 @@ public: assert(SemaPtr && "Sema must have been set."); if (SemaPtr->isSFINAEContext()) return TypoCorrection(); - if (!SemaPtr->SourceMgr.isWrittenInMainFile(Typo.getLoc())) + if (!isInsideMainFile(Typo.getLoc(), SemaPtr->SourceMgr)) return clang::TypoCorrection(); // This is not done lazily because `SS` can get out of scope and it's Modified: clang-tools-extra/trunk/clangd/Quality.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Quality.cpp?rev=366541&r1=366540&r2=366541&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/Quality.cpp (original) +++ clang-tools-extra/trunk/clangd/Quality.cpp Fri Jul 19 01:33:39 2019 @@ -9,6 +9,7 @@ #include "Quality.h" #include "AST.h" #include "FileDistance.h" +#include "SourceCode.h" #include "URI.h" #include "index/Symbol.h" #include "clang/AST/ASTContext.h" @@ -42,8 +43,7 @@ static bool isReserved(llvm::StringRef N static bool hasDeclInMainFile(const Decl &D) { auto &SourceMgr = D.getASTContext().getSourceManager(); for (auto *Redecl : D.redecls()) { - auto Loc = SourceMgr.getSpellingLoc(Redecl->getLocation()); - if (SourceMgr.isWrittenInMainFile(Loc)) + if (isInsideMainFile(Redecl->getLocation(), SourceMgr)) return true; } return false; @@ -53,8 +53,7 @@ static bool hasUsingDeclInMainFile(const const auto &Context = R.Declaration->getASTContext(); const auto &SourceMgr = Context.getSourceManager(); if (R.ShadowDecl) { - const auto Loc = SourceMgr.getExpansionLoc(R.ShadowDecl->getLocation()); - if (SourceMgr.isWrittenInMainFile(Loc)) + if (isInsideMainFile(R.ShadowDecl->getLocation(), SourceMgr)) return true; } return false; Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=366541&r1=366540&r2=366541&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/SourceCode.cpp (original) +++ clang-tools-extra/trunk/clangd/SourceCode.cpp Fri Jul 19 01:33:39 2019 @@ -328,6 +328,10 @@ static SourceRange getTokenFileRange(Sou return FileRange; } +bool isInsideMainFile(SourceLocation Loc, const SourceManager &SM) { + return Loc.isValid() && SM.isWrittenInMainFile(SM.getExpansionLoc(Loc)); +} + llvm::Optional<SourceRange> toHalfOpenFileRange(const SourceManager &SM, const LangOptions &LangOpts, SourceRange R) { Modified: clang-tools-extra/trunk/clangd/SourceCode.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.h?rev=366541&r1=366540&r2=366541&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/SourceCode.h (original) +++ clang-tools-extra/trunk/clangd/SourceCode.h Fri Jul 19 01:33:39 2019 @@ -75,6 +75,14 @@ llvm::Optional<Range> getTokenRange(cons llvm::Expected<SourceLocation> sourceLocationInMainFile(const SourceManager &SM, Position P); +/// Returns true iff \p Loc is inside the main file. This function handles +/// file & macro locations. For macro locations, returns iff the macro is being +/// expanded inside the main file. +/// +/// The function is usually used to check whether a declaration is inside the +/// the main file. +bool isInsideMainFile(SourceLocation Loc, const SourceManager &SM); + /// Turns a token range into a half-open range and checks its correctness. /// The resulting range will have only valid source location on both sides, both /// of which are file locations. Modified: clang-tools-extra/trunk/clangd/XRefs.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=366541&r1=366540&r2=366541&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/XRefs.cpp (original) +++ clang-tools-extra/trunk/clangd/XRefs.cpp Fri Jul 19 01:33:39 2019 @@ -406,7 +406,7 @@ public: assert(D->isCanonicalDecl() && "expect D to be a canonical declaration"); const SourceManager &SM = AST.getSourceManager(); Loc = SM.getFileLoc(Loc); - if (SM.isWrittenInMainFile(Loc) && CanonicalTargets.count(D)) + if (isInsideMainFile(Loc, SM) && CanonicalTargets.count(D)) References.push_back({D, Loc, Roles}); return true; } Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=366541&r1=366540&r2=366541&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original) +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Fri Jul 19 01:33:39 2019 @@ -185,8 +185,7 @@ getTokenLocation(SourceLocation TokLoc, bool isPreferredDeclaration(const NamedDecl &ND, index::SymbolRoleSet Roles) { const auto &SM = ND.getASTContext().getSourceManager(); return (Roles & static_cast<unsigned>(index::SymbolRole::Definition)) && - isa<TagDecl>(&ND) && - !SM.isWrittenInMainFile(SM.getExpansionLoc(ND.getLocation())); + isa<TagDecl>(&ND) && !isInsideMainFile(ND.getLocation(), SM); } RefKind toRefKind(index::SymbolRoleSet Roles) { Modified: clang-tools-extra/trunk/clangd/refactor/Rename.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/refactor/Rename.cpp?rev=366541&r1=366540&r2=366541&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/refactor/Rename.cpp (original) +++ clang-tools-extra/trunk/clangd/refactor/Rename.cpp Fri Jul 19 01:33:39 2019 @@ -104,8 +104,7 @@ llvm::Optional<ReasonToReject> renamable auto &ASTCtx = RenameDecl.getASTContext(); const auto &SM = ASTCtx.getSourceManager(); bool MainFileIsHeader = ASTCtx.getLangOpts().IsHeaderFile; - bool DeclaredInMainFile = - SM.isWrittenInMainFile(SM.getExpansionLoc(RenameDecl.getLocation())); + bool DeclaredInMainFile = isInsideMainFile(RenameDecl.getBeginLoc(), SM); // If the symbol is declared in the main file (which is not a header), we // rename it. Modified: clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp?rev=366541&r1=366540&r2=366541&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp (original) +++ clang-tools-extra/trunk/clangd/unittests/SourceCodeTests.cpp Fri Jul 19 01:33:39 2019 @@ -422,6 +422,36 @@ TEST(SourceCodeTests, GetMacros) { EXPECT_THAT(*Result, MacroName("MACRO")); } +TEST(SourceCodeTests, IsInsideMainFile){ + TestTU TU; + TU.HeaderCode = R"cpp( + #define DEFINE_CLASS(X) class X {}; + #define DEFINE_YY DEFINE_CLASS(YY) + + class Header1 {}; + DEFINE_CLASS(Header2) + class Header {}; + )cpp"; + TU.Code = R"cpp( + class Main1 {}; + DEFINE_CLASS(Main2) + DEFINE_YY + class Main {}; + )cpp"; + TU.ExtraArgs.push_back("-DHeader=Header3"); + TU.ExtraArgs.push_back("-DMain=Main3"); + auto AST = TU.build(); + const auto& SM = AST.getSourceManager(); + auto DeclLoc = [&AST](llvm::StringRef Name) { + return findDecl(AST, Name).getLocation(); + }; + for (const auto *HeaderDecl : {"Header1", "Header2", "Header3"}) + EXPECT_FALSE(isInsideMainFile(DeclLoc(HeaderDecl), SM)); + + for (const auto *MainDecl : {"Main1", "Main2", "Main3", "YY"}) + EXPECT_TRUE(isInsideMainFile(DeclLoc(MainDecl), SM)); +} + // Test for functions toHalfOpenFileRange and getHalfOpenFileRange TEST(SourceCodeTests, HalfOpenFileRange) { // Each marked range should be the file range of the decl with the same name Modified: clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp?rev=366541&r1=366540&r2=366541&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp (original) +++ clang-tools-extra/trunk/clangd/unittests/SymbolCollectorTests.cpp Fri Jul 19 01:33:39 2019 @@ -124,8 +124,7 @@ public: const NamedDecl &ND = Qualified ? findDecl(*AST, Name) : findUnqualifiedDecl(*AST, Name); const SourceManager &SM = AST->getSourceManager(); - bool MainFile = - SM.isWrittenInMainFile(SM.getExpansionLoc(ND.getBeginLoc())); + bool MainFile = isInsideMainFile(ND.getBeginLoc(), SM); return SymbolCollector::shouldCollectSymbol( ND, AST->getASTContext(), SymbolCollector::Options(), MainFile); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
