Skip to content

Commit 6ae86ea

Browse files
committed
[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 llvm-svn: 366541
1 parent f2eb403 commit 6ae86ea

12 files changed

+54
-21
lines changed

Diff for: clang-tools-extra/clangd/ClangdUnit.cpp

+2-3
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ class DeclTrackingASTConsumer : public ASTConsumer {
6868
bool HandleTopLevelDecl(DeclGroupRef DG) override {
6969
for (Decl *D : DG) {
7070
auto &SM = D->getASTContext().getSourceManager();
71-
if (!SM.isWrittenInMainFile(SM.getExpansionLoc(D->getLocation())))
71+
if (!isInsideMainFile(D->getLocation(), SM))
7272
continue;
7373

7474
// ObjCMethodDecl are not actually top-level decls.
@@ -355,8 +355,7 @@ ParsedAST::build(std::unique_ptr<CompilerInvocation> CI,
355355
// those might take us into a preamble file as well.
356356
bool IsInsideMainFile =
357357
Info.hasSourceManager() &&
358-
Info.getSourceManager().isWrittenInMainFile(
359-
Info.getSourceManager().getFileLoc(Info.getLocation()));
358+
isInsideMainFile(Info.getLocation(), Info.getSourceManager());
360359
if (IsInsideMainFile && tidy::ShouldSuppressDiagnostic(
361360
DiagLevel, Info, *CTContext,
362361
/* CheckMacroExpansion = */ false)) {

Diff for: clang-tools-extra/clangd/Diagnostics.cpp

+1-5
Original file line numberDiff line numberDiff line change
@@ -140,15 +140,11 @@ void adjustDiagFromHeader(Diag &D, const clang::Diagnostic &Info,
140140
D.Message = llvm::Twine("in included file: ", D.Message).str();
141141
}
142142

143-
bool isInsideMainFile(const SourceLocation Loc, const SourceManager &M) {
144-
return Loc.isValid() && M.isWrittenInMainFile(M.getFileLoc(Loc));
145-
}
146-
147143
bool isInsideMainFile(const clang::Diagnostic &D) {
148144
if (!D.hasSourceManager())
149145
return false;
150146

151-
return isInsideMainFile(D.getLocation(), D.getSourceManager());
147+
return clangd::isInsideMainFile(D.getLocation(), D.getSourceManager());
152148
}
153149

154150
bool isNote(DiagnosticsEngine::Level L) {

Diff for: clang-tools-extra/clangd/Headers.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class RecordHeaders : public PPCallbacks {
3535
llvm::StringRef /*RelativePath*/,
3636
const Module * /*Imported*/,
3737
SrcMgr::CharacteristicKind FileKind) override {
38-
if (SM.isWrittenInMainFile(HashLoc)) {
38+
if (isInsideMainFile(HashLoc, SM)) {
3939
Out->MainFileIncludes.emplace_back();
4040
auto &Inc = Out->MainFileIncludes.back();
4141
Inc.R = halfOpenToRange(SM, FilenameRange);

Diff for: clang-tools-extra/clangd/IncludeFixer.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ class IncludeFixer::UnresolvedNameRecorder : public ExternalSemaSource {
318318
assert(SemaPtr && "Sema must have been set.");
319319
if (SemaPtr->isSFINAEContext())
320320
return TypoCorrection();
321-
if (!SemaPtr->SourceMgr.isWrittenInMainFile(Typo.getLoc()))
321+
if (!isInsideMainFile(Typo.getLoc(), SemaPtr->SourceMgr))
322322
return clang::TypoCorrection();
323323

324324
// This is not done lazily because `SS` can get out of scope and it's

Diff for: clang-tools-extra/clangd/Quality.cpp

+3-4
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "Quality.h"
1010
#include "AST.h"
1111
#include "FileDistance.h"
12+
#include "SourceCode.h"
1213
#include "URI.h"
1314
#include "index/Symbol.h"
1415
#include "clang/AST/ASTContext.h"
@@ -42,8 +43,7 @@ static bool isReserved(llvm::StringRef Name) {
4243
static bool hasDeclInMainFile(const Decl &D) {
4344
auto &SourceMgr = D.getASTContext().getSourceManager();
4445
for (auto *Redecl : D.redecls()) {
45-
auto Loc = SourceMgr.getSpellingLoc(Redecl->getLocation());
46-
if (SourceMgr.isWrittenInMainFile(Loc))
46+
if (isInsideMainFile(Redecl->getLocation(), SourceMgr))
4747
return true;
4848
}
4949
return false;
@@ -53,8 +53,7 @@ static bool hasUsingDeclInMainFile(const CodeCompletionResult &R) {
5353
const auto &Context = R.Declaration->getASTContext();
5454
const auto &SourceMgr = Context.getSourceManager();
5555
if (R.ShadowDecl) {
56-
const auto Loc = SourceMgr.getExpansionLoc(R.ShadowDecl->getLocation());
57-
if (SourceMgr.isWrittenInMainFile(Loc))
56+
if (isInsideMainFile(R.ShadowDecl->getLocation(), SourceMgr))
5857
return true;
5958
}
6059
return false;

Diff for: clang-tools-extra/clangd/SourceCode.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,10 @@ static SourceRange getTokenFileRange(SourceLocation Loc,
328328
return FileRange;
329329
}
330330

331+
bool isInsideMainFile(SourceLocation Loc, const SourceManager &SM) {
332+
return Loc.isValid() && SM.isWrittenInMainFile(SM.getExpansionLoc(Loc));
333+
}
334+
331335
llvm::Optional<SourceRange> toHalfOpenFileRange(const SourceManager &SM,
332336
const LangOptions &LangOpts,
333337
SourceRange R) {

Diff for: clang-tools-extra/clangd/SourceCode.h

+8
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,14 @@ llvm::Optional<Range> getTokenRange(const SourceManager &SM,
7575
llvm::Expected<SourceLocation> sourceLocationInMainFile(const SourceManager &SM,
7676
Position P);
7777

78+
/// Returns true iff \p Loc is inside the main file. This function handles
79+
/// file & macro locations. For macro locations, returns iff the macro is being
80+
/// expanded inside the main file.
81+
///
82+
/// The function is usually used to check whether a declaration is inside the
83+
/// the main file.
84+
bool isInsideMainFile(SourceLocation Loc, const SourceManager &SM);
85+
7886
/// Turns a token range into a half-open range and checks its correctness.
7987
/// The resulting range will have only valid source location on both sides, both
8088
/// of which are file locations.

Diff for: clang-tools-extra/clangd/XRefs.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ class ReferenceFinder : public index::IndexDataConsumer {
406406
assert(D->isCanonicalDecl() && "expect D to be a canonical declaration");
407407
const SourceManager &SM = AST.getSourceManager();
408408
Loc = SM.getFileLoc(Loc);
409-
if (SM.isWrittenInMainFile(Loc) && CanonicalTargets.count(D))
409+
if (isInsideMainFile(Loc, SM) && CanonicalTargets.count(D))
410410
References.push_back({D, Loc, Roles});
411411
return true;
412412
}

Diff for: clang-tools-extra/clangd/index/SymbolCollector.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,7 @@ getTokenLocation(SourceLocation TokLoc, const SourceManager &SM,
185185
bool isPreferredDeclaration(const NamedDecl &ND, index::SymbolRoleSet Roles) {
186186
const auto &SM = ND.getASTContext().getSourceManager();
187187
return (Roles & static_cast<unsigned>(index::SymbolRole::Definition)) &&
188-
isa<TagDecl>(&ND) &&
189-
!SM.isWrittenInMainFile(SM.getExpansionLoc(ND.getLocation()));
188+
isa<TagDecl>(&ND) && !isInsideMainFile(ND.getLocation(), SM);
190189
}
191190

192191
RefKind toRefKind(index::SymbolRoleSet Roles) {

Diff for: clang-tools-extra/clangd/refactor/Rename.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,7 @@ llvm::Optional<ReasonToReject> renamableWithinFile(const Decl &RenameDecl,
104104
auto &ASTCtx = RenameDecl.getASTContext();
105105
const auto &SM = ASTCtx.getSourceManager();
106106
bool MainFileIsHeader = ASTCtx.getLangOpts().IsHeaderFile;
107-
bool DeclaredInMainFile =
108-
SM.isWrittenInMainFile(SM.getExpansionLoc(RenameDecl.getLocation()));
107+
bool DeclaredInMainFile = isInsideMainFile(RenameDecl.getBeginLoc(), SM);
109108

110109
// If the symbol is declared in the main file (which is not a header), we
111110
// rename it.

Diff for: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp

+30
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,36 @@ TEST(SourceCodeTests, GetMacros) {
422422
EXPECT_THAT(*Result, MacroName("MACRO"));
423423
}
424424

425+
TEST(SourceCodeTests, IsInsideMainFile){
426+
TestTU TU;
427+
TU.HeaderCode = R"cpp(
428+
#define DEFINE_CLASS(X) class X {};
429+
#define DEFINE_YY DEFINE_CLASS(YY)
430+
431+
class Header1 {};
432+
DEFINE_CLASS(Header2)
433+
class Header {};
434+
)cpp";
435+
TU.Code = R"cpp(
436+
class Main1 {};
437+
DEFINE_CLASS(Main2)
438+
DEFINE_YY
439+
class Main {};
440+
)cpp";
441+
TU.ExtraArgs.push_back("-DHeader=Header3");
442+
TU.ExtraArgs.push_back("-DMain=Main3");
443+
auto AST = TU.build();
444+
const auto& SM = AST.getSourceManager();
445+
auto DeclLoc = [&AST](llvm::StringRef Name) {
446+
return findDecl(AST, Name).getLocation();
447+
};
448+
for (const auto *HeaderDecl : {"Header1", "Header2", "Header3"})
449+
EXPECT_FALSE(isInsideMainFile(DeclLoc(HeaderDecl), SM));
450+
451+
for (const auto *MainDecl : {"Main1", "Main2", "Main3", "YY"})
452+
EXPECT_TRUE(isInsideMainFile(DeclLoc(MainDecl), SM));
453+
}
454+
425455
// Test for functions toHalfOpenFileRange and getHalfOpenFileRange
426456
TEST(SourceCodeTests, HalfOpenFileRange) {
427457
// Each marked range should be the file range of the decl with the same name

Diff for: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

+1-2
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,7 @@ class ShouldCollectSymbolTest : public ::testing::Test {
124124
const NamedDecl &ND =
125125
Qualified ? findDecl(*AST, Name) : findUnqualifiedDecl(*AST, Name);
126126
const SourceManager &SM = AST->getSourceManager();
127-
bool MainFile =
128-
SM.isWrittenInMainFile(SM.getExpansionLoc(ND.getBeginLoc()));
127+
bool MainFile = isInsideMainFile(ND.getBeginLoc(), SM);
129128
return SymbolCollector::shouldCollectSymbol(
130129
ND, AST->getASTContext(), SymbolCollector::Options(), MainFile);
131130
}

0 commit comments

Comments
 (0)