ADKaster updated this revision to Diff 506409.
ADKaster added a comment.
Change from a boolean parameter to an enum.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145843/new/
https://reviews.llvm.org/D145843
Files:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/Headers.cpp
clang-tools-extra/clangd/Headers.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
clang-tools-extra/clangd/unittests/HeadersTests.cpp
Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -108,7 +108,8 @@
IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
CDB.getCompileCommand(MainFile)->Directory,
- &Clang->getPreprocessor().getHeaderSearchInfo());
+ &Clang->getPreprocessor().getHeaderSearchInfo(),
+ IncludeDelimiter);
for (const auto &Inc : Inclusions)
Inserter.addExisting(Inc);
auto Inserted = ToHeaderFile(Preferred);
@@ -128,7 +129,8 @@
IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
CDB.getCompileCommand(MainFile)->Directory,
- &Clang->getPreprocessor().getHeaderSearchInfo());
+ &Clang->getPreprocessor().getHeaderSearchInfo(),
+ IncludeDelimiter);
auto Edit = Inserter.insert(VerbatimHeader, Directive);
Action.EndSourceFile();
return Edit;
@@ -139,6 +141,8 @@
std::string MainFile = testPath("main.cpp");
std::string Subdir = testPath("sub");
std::string SearchDirArg = (llvm::Twine("-I") + Subdir).str();
+ Config::IncludeDelimiterPolicy IncludeDelimiter =
+ Config::IncludeDelimiterPolicy::Auto;
IgnoringDiagConsumer IgnoreDiags;
std::unique_ptr<CompilerInstance> Clang;
};
@@ -309,6 +313,9 @@
std::string Path = testPath("sub/bar.h");
FS.Files[Path] = "";
EXPECT_EQ(calculate(Path), "\"bar.h\"");
+
+ IncludeDelimiter = Config::IncludeDelimiterPolicy::AlwaysBrackets;
+ EXPECT_EQ(calculate(Path), "<bar.h>");
}
TEST_F(HeadersTest, DoNotInsertIfInSameFile) {
@@ -331,6 +338,17 @@
EXPECT_EQ(calculate(BarHeader), "\"sub/bar.h\"");
}
+TEST_F(HeadersTest, ShortenIncludesInSearchPathBracketed) {
+ IncludeDelimiter = Config::IncludeDelimiterPolicy::AlwaysBrackets;
+ std::string BarHeader = testPath("sub/bar.h");
+ EXPECT_EQ(calculate(BarHeader), "<bar.h>");
+
+ SearchDirArg = (llvm::Twine("-I") + Subdir + "/..").str();
+ CDB.ExtraClangFlags = {SearchDirArg.c_str()};
+ BarHeader = testPath("sub/bar.h");
+ EXPECT_EQ(calculate(BarHeader), "<sub/bar.h>");
+}
+
TEST_F(HeadersTest, ShortenedIncludeNotInSearchPath) {
std::string BarHeader =
llvm::sys::path::convert_to_slash(testPath("sub-2/bar.h"));
@@ -343,6 +361,10 @@
std::string BazHeader = testPath("sub/baz.h");
EXPECT_EQ(calculate(BarHeader, BazHeader), "\"baz.h\"");
+
+ IncludeDelimiter = Config::IncludeDelimiterPolicy::AlwaysBrackets;
+ std::string BiffHeader = testPath("sub/biff.h");
+ EXPECT_EQ(calculate(BarHeader, BiffHeader), "<biff.h>");
}
TEST_F(HeadersTest, DontInsertDuplicatePreferred) {
@@ -374,8 +396,10 @@
TEST(Headers, NoHeaderSearchInfo) {
std::string MainFile = testPath("main.cpp");
- IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
- /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
+ IncludeInserter Inserter(
+ MainFile, /*Code=*/"", format::getLLVMStyle(),
+ /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr,
+ /*IncludeDelimiter*/ Config::IncludeDelimiterPolicy::Auto);
auto HeaderPath = testPath("sub/bar.h");
auto Inserting = HeaderFile{HeaderPath, /*Verbatim=*/false};
Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -265,13 +265,15 @@
CapturedDiags Diags;
Annotations YAML(R"yaml(
Style:
- FullyQualifiedNamespaces: [foo, bar])yaml");
+ FullyQualifiedNamespaces: [foo, bar]
+ IncludeDelimiter: AlwaysBrackets)yaml");
auto Results =
Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
ASSERT_THAT(Diags.Diagnostics, IsEmpty());
ASSERT_EQ(Results.size(), 1u);
EXPECT_THAT(Results[0].Style.FullyQualifiedNamespaces,
ElementsAre(val("foo"), val("bar")));
+ EXPECT_EQ("AlwaysBrackets", **Results[0].Style.IncludeDelimiter);
}
TEST(ParseYAML, DiagnosticsMode) {
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -248,20 +248,17 @@
TEST_F(ConfigCompileTests, DiagnosticsIncludeCleaner) {
// Defaults to None.
EXPECT_TRUE(compileAndApply());
- EXPECT_EQ(Conf.Diagnostics.UnusedIncludes,
- Config::IncludesPolicy::None);
+ EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, Config::IncludesPolicy::None);
Frag = {};
Frag.Diagnostics.UnusedIncludes.emplace("None");
EXPECT_TRUE(compileAndApply());
- EXPECT_EQ(Conf.Diagnostics.UnusedIncludes,
- Config::IncludesPolicy::None);
+ EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, Config::IncludesPolicy::None);
Frag = {};
Frag.Diagnostics.UnusedIncludes.emplace("Strict");
EXPECT_TRUE(compileAndApply());
- EXPECT_EQ(Conf.Diagnostics.UnusedIncludes,
- Config::IncludesPolicy::Strict);
+ EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, Config::IncludesPolicy::Strict);
Frag = {};
EXPECT_TRUE(Conf.Diagnostics.Includes.IgnoreHeader.empty())
@@ -542,6 +539,21 @@
Frag.Style.FullyQualifiedNamespaces.push_back(std::string("bar"));
EXPECT_TRUE(compileAndApply());
EXPECT_THAT(Conf.Style.FullyQualifiedNamespaces, ElementsAre("foo", "bar"));
+
+ Frag.Style.IncludeDelimiter.emplace("AlwaysBrackets");
+ EXPECT_TRUE(compileAndApply());
+ EXPECT_EQ(Conf.Style.IncludeDelimiter,
+ Config::IncludeDelimiterPolicy::AlwaysBrackets);
+
+ Frag = {};
+ Frag.Style.IncludeDelimiter.emplace("Foo");
+ EXPECT_TRUE(compileAndApply());
+ EXPECT_EQ(Conf.Style.IncludeDelimiter, Config::IncludeDelimiterPolicy::Auto)
+ << "by default";
+ EXPECT_THAT(
+ Diags.Diagnostics,
+ ElementsAre(diagMessage("Invalid IncludeDelimiter value 'Foo'. Valid "
+ "values are Auto, AlwaysBrackets.")));
}
TEST_F(ConfigCompileTests, AllowDiagsFromStalePreamble) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -565,7 +565,8 @@
getFormatStyleForFile(Filename, Inputs.Contents, *Inputs.TFS);
auto Inserter = std::make_shared<IncludeInserter>(
Filename, Inputs.Contents, Style, BuildDir.get(),
- &Clang->getPreprocessor().getHeaderSearchInfo());
+ &Clang->getPreprocessor().getHeaderSearchInfo(),
+ Cfg.Style.IncludeDelimiter);
ArrayRef<Inclusion> MainFileIncludes;
if (Preamble) {
MainFileIncludes = Preamble->Includes.MainFileIncludes;
Index: clang-tools-extra/clangd/Headers.h
===================================================================
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -9,6 +9,7 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_HEADERS_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_HEADERS_H
+#include "Config.h"
#include "Protocol.h"
#include "SourceCode.h"
#include "index/Symbol.h"
@@ -219,10 +220,12 @@
// include path of non-verbatim header will not be shortened.
IncludeInserter(StringRef FileName, StringRef Code,
const format::FormatStyle &Style, StringRef BuildDir,
- HeaderSearch *HeaderSearchInfo)
+ HeaderSearch *HeaderSearchInfo,
+ Config::IncludeDelimiterPolicy IncludeDelimiter)
: FileName(FileName), Code(Code), BuildDir(BuildDir),
HeaderSearchInfo(HeaderSearchInfo),
- Inserter(FileName, Code, Style.IncludeStyle) {}
+ Inserter(FileName, Code, Style.IncludeStyle),
+ IncludeDelimiter(IncludeDelimiter) {}
void addExisting(const Inclusion &Inc);
@@ -266,6 +269,7 @@
HeaderSearch *HeaderSearchInfo = nullptr;
llvm::StringSet<> IncludedHeaders; // Both written and resolved.
tooling::HeaderIncludes Inserter; // Computers insertion replacement.
+ Config::IncludeDelimiterPolicy IncludeDelimiter;
};
} // namespace clangd
Index: clang-tools-extra/clangd/Headers.cpp
===================================================================
--- clang-tools-extra/clangd/Headers.cpp
+++ clang-tools-extra/clangd/Headers.cpp
@@ -349,7 +349,8 @@
// FIXME: should we allow (some limited number of) "../header.h"?
if (llvm::sys::path::is_absolute(Suggested))
return std::nullopt;
- if (IsSystem)
+ if (IsSystem ||
+ IncludeDelimiter == Config::IncludeDelimiterPolicy::AlwaysBrackets)
Suggested = "<" + Suggested + ">";
else
Suggested = "\"" + Suggested + "\"";
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -116,6 +116,9 @@
if (auto Values = scalarValues(N))
F.FullyQualifiedNamespaces = std::move(*Values);
});
+ Dict.handle("IncludeDelimiter", [&](Node &N) {
+ F.IncludeDelimiter = scalarValue(N, "IncludeDelimiter");
+ });
Dict.parse(N);
}
Index: clang-tools-extra/clangd/ConfigFragment.h
===================================================================
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -293,6 +293,12 @@
// ::). All nested namespaces are affected as well.
// Affects availability of the AddUsing tweak.
std::vector<Located<std::string>> FullyQualifiedNamespaces;
+
+ // Whether inserted includes should use <> or "". By default, system
+ // headers use <> and non-system headers use "". This only affects headers
+ // inserted by IncludeInserter, and will not reformat existing includes.
+ // Legal values are "Auto" and "AlwaysBracket".
+ std::optional<Located<std::string>> IncludeDelimiter;
};
StyleBlock Style;
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -488,6 +488,17 @@
FullyQualifiedNamespaces.begin(), FullyQualifiedNamespaces.end());
});
}
+ if (F.IncludeDelimiter) {
+ if (auto Val = compileEnum<Config::IncludeDelimiterPolicy>(
+ "IncludeDelimiter", **F.IncludeDelimiter)
+ .map("Auto", Config::IncludeDelimiterPolicy::Auto)
+ .map("AlwaysBrackets",
+ Config::IncludeDelimiterPolicy::AlwaysBrackets)
+ .value())
+ Out.Apply.push_back([Val](const Params &, Config &C) {
+ C.Style.IncludeDelimiter = *Val;
+ });
+ }
}
void appendTidyCheckSpec(std::string &CurSpec,
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -118,12 +118,16 @@
} Includes;
} Diagnostics;
+ enum class IncludeDelimiterPolicy { Auto, AlwaysBrackets };
/// Style of the codebase.
struct {
// Namespaces that should always be fully qualified, meaning no "using"
// declarations, always spell out the whole name (with or without leading
// ::). All nested namespaces are affected as well.
std::vector<std::string> FullyQualifiedNamespaces;
+
+ // Whether inserted includes should use <> or "".
+ IncludeDelimiterPolicy IncludeDelimiter = IncludeDelimiterPolicy::Auto;
} Style;
/// Configures code completion feature.
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -21,6 +21,7 @@
#include "AST.h"
#include "CodeCompletionStrings.h"
#include "Compiler.h"
+#include "Config.h"
#include "ExpectedTypes.h"
#include "Feature.h"
#include "FileDistance.h"
@@ -1554,7 +1555,8 @@
Inserter.emplace(
SemaCCInput.FileName, SemaCCInput.ParseInput.Contents, Style,
SemaCCInput.ParseInput.CompileCommand.Directory,
- &Recorder->CCSema->getPreprocessor().getHeaderSearchInfo());
+ &Recorder->CCSema->getPreprocessor().getHeaderSearchInfo(),
+ Config::current().Style.IncludeDelimiter);
for (const auto &Inc : Includes.MainFileIncludes)
Inserter->addExisting(Inc);
@@ -1637,7 +1639,8 @@
auto Style = getFormatStyleForFile(FileName, Content, TFS);
// This will only insert verbatim headers.
Inserter.emplace(FileName, Content, Style,
- /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
+ /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr,
+ Config::current().Style.IncludeDelimiter);
auto Identifiers = collectIdentifiers(Content, Style);
std::vector<RawIdentifier> IdentifierResults;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
