Merged to release_90 in r368669 to help subsequent merges.
On Fri, Jul 19, 2019 at 10:33 AM Haojian Wu via cfe-commits <cfe-commits@lists.llvm.org> wrote: > > 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
