Skip to content

Commit 4f2e0eb

Browse files
AutomergerAutomerger
Automerger
authored and
Automerger
committed
Propagating prior merge from 'llvm.org/master'.
apple-llvm-split-commit: e3091e8d5f7d919686fcbc47342193123d27c505 apple-llvm-split-dir: clang-tools-extra/
2 parents 4fd8a18 + 6d53adf commit 4f2e0eb

14 files changed

+142
-244
lines changed

clang-tools-extra/clangd/AST.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,7 @@ std::string shortenNamespace(const llvm::StringRef OriginalName,
192192

193193
std::string printType(const QualType QT, const DeclContext & Context){
194194
PrintingPolicy PP(Context.getParentASTContext().getPrintingPolicy());
195+
PP.SuppressUnwrittenScope = 1;
195196
PP.SuppressTagKeyword = 1;
196197
return shortenNamespace(
197198
QT.getAsString(PP),

clang-tools-extra/clangd/AST.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,8 @@ llvm::Optional<SymbolID> getSymbolID(const IdentifierInfo &II,
6767
const MacroInfo *MI,
6868
const SourceManager &SM);
6969

70-
/// Returns a QualType as string.
70+
/// Returns a QualType as string. The result doesn't contain unwritten scopes
71+
/// like annoymous/inline namespace.
7172
std::string printType(const QualType QT, const DeclContext & Context);
7273

7374
/// Try to shorten the OriginalName by removing namespaces from the left of

clang-tools-extra/clangd/FS.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,5 +111,11 @@ PreambleFileStatusCache::getConsumingFS(
111111
return llvm::IntrusiveRefCntPtr<CacheVFS>(new CacheVFS(std::move(FS), *this));
112112
}
113113

114+
Path removeDots(PathRef File) {
115+
llvm::SmallString<128> CanonPath(File);
116+
llvm::sys::path::remove_dots(CanonPath, /*remove_dot_dot=*/true);
117+
return CanonPath.str().str();
118+
}
119+
114120
} // namespace clangd
115121
} // namespace clang

clang-tools-extra/clangd/FS.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FS_H
1010
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FS_H
1111

12+
#include "Path.h"
1213
#include "clang/Basic/LLVM.h"
1314
#include "llvm/ADT/Optional.h"
1415
#include "llvm/Support/VirtualFileSystem.h"
@@ -65,6 +66,13 @@ class PreambleFileStatusCache {
6566
llvm::StringMap<llvm::vfs::Status> StatCache;
6667
};
6768

69+
/// Returns a version of \p File that doesn't contain dots and dot dots.
70+
/// e.g /a/b/../c -> /a/c
71+
/// /a/b/./c -> /a/b/c
72+
/// FIXME: We should avoid encountering such paths in clangd internals by
73+
/// filtering everything we get over LSP, CDB, etc.
74+
Path removeDots(PathRef File);
75+
6876
} // namespace clangd
6977
} // namespace clang
7078

clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "GlobalCompilationDatabase.h"
10+
#include "FS.h"
1011
#include "Logger.h"
1112
#include "Path.h"
1213
#include "clang/Frontend/CompilerInvocation.h"
@@ -15,6 +16,7 @@
1516
#include "llvm/ADT/None.h"
1617
#include "llvm/ADT/Optional.h"
1718
#include "llvm/ADT/STLExtras.h"
19+
#include "llvm/ADT/SmallString.h"
1820
#include "llvm/Support/FileSystem.h"
1921
#include "llvm/Support/Path.h"
2022
#include <string>
@@ -147,12 +149,15 @@ DirectoryBasedGlobalCompilationDatabase::lookupCDB(
147149
getCDBInDirLocked(*CompileCommandsDir);
148150
Result.PI.SourceRoot = *CompileCommandsDir;
149151
} else {
150-
actOnAllParentDirectories(
151-
Request.FileName, [this, &SentBroadcast, &Result](PathRef Path) {
152-
std::tie(Result.CDB, SentBroadcast) = getCDBInDirLocked(Path);
153-
Result.PI.SourceRoot = Path;
154-
return Result.CDB != nullptr;
155-
});
152+
// Traverse the canonical version to prevent false positives. i.e.:
153+
// src/build/../a.cc can detect a CDB in /src/build if not canonicalized.
154+
actOnAllParentDirectories(removeDots(Request.FileName),
155+
[this, &SentBroadcast, &Result](PathRef Path) {
156+
std::tie(Result.CDB, SentBroadcast) =
157+
getCDBInDirLocked(Path);
158+
Result.PI.SourceRoot = Path;
159+
return Result.CDB != nullptr;
160+
});
156161
}
157162

158163
if (!Result.CDB)
@@ -209,7 +214,8 @@ void DirectoryBasedGlobalCompilationDatabase::broadcastCDB(
209214
actOnAllParentDirectories(File, [&](PathRef Path) {
210215
if (DirectoryHasCDB.lookup(Path)) {
211216
if (Path == Result.PI.SourceRoot)
212-
GovernedFiles.push_back(File);
217+
// Make sure listeners always get a canonical path for the file.
218+
GovernedFiles.push_back(removeDots(File));
213219
// Stop as soon as we hit a CDB.
214220
return true;
215221
}
@@ -248,7 +254,7 @@ OverlayCDB::getCompileCommand(PathRef File) const {
248254
llvm::Optional<tooling::CompileCommand> Cmd;
249255
{
250256
std::lock_guard<std::mutex> Lock(Mutex);
251-
auto It = Commands.find(File);
257+
auto It = Commands.find(removeDots(File));
252258
if (It != Commands.end())
253259
Cmd = It->second;
254260
}
@@ -272,20 +278,24 @@ tooling::CompileCommand OverlayCDB::getFallbackCommand(PathRef File) const {
272278

273279
void OverlayCDB::setCompileCommand(
274280
PathRef File, llvm::Optional<tooling::CompileCommand> Cmd) {
281+
// We store a canonical version internally to prevent mismatches between set
282+
// and get compile commands. Also it assures clients listening to broadcasts
283+
// doesn't receive different names for the same file.
284+
std::string CanonPath = removeDots(File);
275285
{
276286
std::unique_lock<std::mutex> Lock(Mutex);
277287
if (Cmd)
278-
Commands[File] = std::move(*Cmd);
288+
Commands[CanonPath] = std::move(*Cmd);
279289
else
280-
Commands.erase(File);
290+
Commands.erase(CanonPath);
281291
}
282-
OnCommandChanged.broadcast({File});
292+
OnCommandChanged.broadcast({CanonPath});
283293
}
284294

285295
llvm::Optional<ProjectInfo> OverlayCDB::getProjectInfo(PathRef File) const {
286296
{
287297
std::lock_guard<std::mutex> Lock(Mutex);
288-
auto It = Commands.find(File);
298+
auto It = Commands.find(removeDots(File));
289299
if (It != Commands.end())
290300
return ProjectInfo{};
291301
}

clang-tools-extra/clangd/refactor/Tweak.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ void validateRegistry() {
4040

4141
Tweak::Selection::Selection(ParsedAST &AST, unsigned RangeBegin,
4242
unsigned RangeEnd)
43-
: AST(AST), ASTSelection(AST.getASTContext(), RangeBegin, RangeEnd) {
43+
: AST(AST), SelectionBegin(RangeBegin), SelectionEnd(RangeEnd),
44+
ASTSelection(AST.getASTContext(), RangeBegin, RangeEnd) {
4445
auto &SM = AST.getSourceManager();
4546
Code = SM.getBufferData(SM.getMainFileID());
4647
Cursor = SM.getComposedLoc(SM.getMainFileID(), RangeBegin);

clang-tools-extra/clangd/refactor/Tweak.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,12 @@ class Tweak {
4646
/// Parsed AST of the active file.
4747
ParsedAST &AST;
4848
/// A location of the cursor in the editor.
49+
// FIXME: Cursor is redundant and should be removed
4950
SourceLocation Cursor;
51+
/// The begin offset of the selection
52+
unsigned SelectionBegin;
53+
/// The end offset of the selection
54+
unsigned SelectionEnd;
5055
/// The AST nodes that were selected.
5156
SelectionTree ASTSelection;
5257
// FIXME: provide a way to get sources and ASTs for other files.

clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ set(LLVM_LINK_COMPONENTS
1414
add_clang_library(clangDaemonTweaks OBJECT
1515
AnnotateHighlightings.cpp
1616
DumpAST.cpp
17+
ExpandAutoType.cpp
1718
ExpandMacro.cpp
19+
ExtractVariable.cpp
1820
RawStringLiteral.cpp
1921
SwapIfBranches.cpp
20-
ExtractVariable.cpp
21-
ExpandAutoType.cpp
2222

2323
LINK_LIBS
2424
clangAST

clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,11 @@ class DumpRecordLayout : public Tweak {
128128
TypeWithKeyword::getTagTypeKindName(Record->getTagKind()));
129129
}
130130
Intent intent() const override { return Info; }
131+
// FIXME: this is interesting to most users. However:
132+
// - triggering is too broad (e.g. triggers on comments within a class)
133+
// - showMessage has inconsistent UX (e.g. newlines are stripped in VSCode)
134+
// - the output itself is a bit hard to decipher.
135+
bool hidden() const override { return true; }
131136

132137
private:
133138
const RecordDecl *Record = nullptr;

clang-tools-extra/clangd/refactor/tweaks/ExtractVariable.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,8 @@ bool ExtractVariable::prepare(const Selection &Inputs) {
219219
const ASTContext &Ctx = Inputs.AST.getASTContext();
220220
const SourceManager &SM = Inputs.AST.getSourceManager();
221221
const SelectionTree::Node *N = Inputs.ASTSelection.commonAncestor();
222-
if (!N)
222+
// we don't trigger on empty selections for now
223+
if (!N || Inputs.SelectionBegin == Inputs.SelectionEnd)
223224
return false;
224225
Target = llvm::make_unique<ExtractionContext>(N, SM, Ctx);
225226
return Target->isExtractable();

clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
#include "Path.h"
1212
#include "TestFS.h"
13+
#include "clang/Tooling/CompilationDatabase.h"
1314
#include "llvm/ADT/Optional.h"
1415
#include "llvm/ADT/SmallString.h"
1516
#include "llvm/ADT/StringExtras.h"
@@ -31,6 +32,7 @@ using ::testing::AllOf;
3132
using ::testing::Contains;
3233
using ::testing::ElementsAre;
3334
using ::testing::EndsWith;
35+
using ::testing::HasSubstr;
3436
using ::testing::IsEmpty;
3537
using ::testing::Not;
3638
using ::testing::StartsWith;
@@ -247,9 +249,10 @@ TEST(GlobalCompilationDatabaseTest, DiscoveryWithNestedCDBs) {
247249
});
248250

249251
File = FS.Root;
250-
llvm::sys::path::append(File, "a.cc");
252+
llvm::sys::path::append(File, "build", "..", "a.cc");
251253
DB.getCompileCommand(File.str());
252-
EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(EndsWith("a.cc")));
254+
EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(AllOf(
255+
EndsWith("a.cc"), Not(HasSubstr("..")))));
253256
DiscoveredFiles.clear();
254257

255258
File = FS.Root;
@@ -282,6 +285,27 @@ TEST(GlobalCompilationDatabaseTest, DiscoveryWithNestedCDBs) {
282285
}
283286
}
284287

288+
TEST(GlobalCompilationDatabaseTest, NonCanonicalFilenames) {
289+
OverlayCDB DB(nullptr);
290+
std::vector<std::string> DiscoveredFiles;
291+
auto Sub =
292+
DB.watch([&DiscoveredFiles](const std::vector<std::string> Changes) {
293+
DiscoveredFiles = Changes;
294+
});
295+
296+
llvm::SmallString<128> Root(testRoot());
297+
llvm::sys::path::append(Root, "build", "..", "a.cc");
298+
DB.setCompileCommand(Root.str(), tooling::CompileCommand());
299+
EXPECT_THAT(DiscoveredFiles, UnorderedElementsAre(testPath("a.cc")));
300+
DiscoveredFiles.clear();
301+
302+
llvm::SmallString<128> File(testRoot());
303+
llvm::sys::path::append(File, "blabla", "..", "a.cc");
304+
305+
EXPECT_TRUE(DB.getCompileCommand(File));
306+
EXPECT_TRUE(DB.getProjectInfo(File));
307+
}
308+
285309
} // namespace
286310
} // namespace clangd
287311
} // namespace clang

0 commit comments

Comments
 (0)