[clang-tools-extra] r366541 - [clangd] cleanup: unify the implemenation of checking a location is inside main file.
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 13 04:34:17 PDT 2019
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 at 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 at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list