[clang-tools-extra] 6b2fed3 - [clangd] Upgrade vlog() to log() for preamble build stats
Author: Adam Czachorowski
Date: 2022-08-01T15:08:21+02:00
New Revision: 6b2fed3ab4193cfb50c4e60fb4cd19c7e6b3603f
URL:
https://github.com/llvm/llvm-project/commit/6b2fed3ab4193cfb50c4e60fb4cd19c7e6b3603f
DIFF:
https://github.com/llvm/llvm-project/commit/6b2fed3ab4193cfb50c4e60fb4cd19c7e6b3603f.diff
LOG: [clangd] Upgrade vlog() to log() for preamble build stats
This is very useful information for better understanding performance and
preamble builds don't happen that often, so it's not that spammy.
Differential Revision: https://reviews.llvm.org/D130636
Added:
Modified:
clang-tools-extra/clangd/Preamble.cpp
Removed:
diff --git a/clang-tools-extra/clangd/Preamble.cpp
b/clang-tools-extra/clangd/Preamble.cpp
index 9fc249afcc224..337f71bed9bae 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -552,7 +552,7 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
}
if (BuiltPreamble) {
-vlog("Built preamble of size {0} for file {1} version {2} in {3} seconds",
+log("Built preamble of size {0} for file {1} version {2} in {3} seconds",
BuiltPreamble->getSize(), FileName, Inputs.Version,
PreambleTimer.getTime());
std::vector Diags = PreambleDiagnostics.take();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] cab3cfd - [clang] Do not crash on "requires" after a fatal error occurred.
Author: Adam Czachorowski
Date: 2022-07-14T15:45:32+02:00
New Revision: cab3cfd013cf92c441c3acbfcc1844b267ab8659
URL:
https://github.com/llvm/llvm-project/commit/cab3cfd013cf92c441c3acbfcc1844b267ab8659
DIFF:
https://github.com/llvm/llvm-project/commit/cab3cfd013cf92c441c3acbfcc1844b267ab8659.diff
LOG: [clang] Do not crash on "requires" after a fatal error occurred.
The code would assume that SubstExpr() cannot fail on concept
specialization. This is incorret - we give up on some things after fatal
error occurred, since there's no value in doing futher work that the
user will not see anyway. In this case, this lead to crash.
The fatal error is simulated in tests with -ferror-limit=1, but this
could happen in other cases too.
Fixes https://github.com/llvm/llvm-project/issues/55401
Differential Revision: https://reviews.llvm.org/D129499
Added:
clang/test/SemaCXX/concept-fatal-error.cpp
Modified:
clang/lib/Sema/SemaExprCXX.cpp
Removed:
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 11f33c7c63633..5331193de863d 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -9006,14 +9006,14 @@ Sema::BuildExprRequirement(
cast(TPL->getParam(0))->getTypeConstraint()
->getImmediatelyDeclaredConstraint();
ExprResult Constraint = SubstExpr(IDC, MLTAL);
-assert(!Constraint.isInvalid() &&
- "Substitution cannot fail as it is simply putting a type template "
- "argument into a concept specialization expression's parameter.");
-
-SubstitutedConstraintExpr =
-cast(Constraint.get());
-if (!SubstitutedConstraintExpr->isSatisfied())
- Status = concepts::ExprRequirement::SS_ConstraintsNotSatisfied;
+if (Constraint.isInvalid()) {
+ Status = concepts::ExprRequirement::SS_ExprSubstitutionFailure;
+} else {
+ SubstitutedConstraintExpr =
+ cast(Constraint.get());
+ if (!SubstitutedConstraintExpr->isSatisfied())
+Status = concepts::ExprRequirement::SS_ConstraintsNotSatisfied;
+}
}
return new (Context) concepts::ExprRequirement(E, IsSimple, NoexceptLoc,
ReturnTypeRequirement, Status,
diff --git a/clang/test/SemaCXX/concept-fatal-error.cpp
b/clang/test/SemaCXX/concept-fatal-error.cpp
new file mode 100644
index 0..c299b39fdeb23
--- /dev/null
+++ b/clang/test/SemaCXX/concept-fatal-error.cpp
@@ -0,0 +1,10 @@
+// RUN: not %clang_cc1 -fsyntax-only -std=c++20 -ferror-limit 1 -verify %s
+
+template
+concept f = requires { 42; };
+struct h {
+ // The missing semicolon will trigger an error and -ferror-limit=1 will make
it fatal
+ // We test that we do not crash in such cases (#55401)
+ int i = requires { { i } f } // expected-error {{expected ';' at end of
declaration list}}
+ // expected-error@* {{too many errros emitted}}
+};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] ad46aae - [clangd] Add beforeExecute() callback to FeatureModules.
Author: Adam Czachorowski
Date: 2022-04-21T18:03:39+02:00
New Revision: ad46aaede6e4d5a6951fc9827da994d3fbe1af44
URL:
https://github.com/llvm/llvm-project/commit/ad46aaede6e4d5a6951fc9827da994d3fbe1af44
DIFF:
https://github.com/llvm/llvm-project/commit/ad46aaede6e4d5a6951fc9827da994d3fbe1af44.diff
LOG: [clangd] Add beforeExecute() callback to FeatureModules.
It runs immediatelly before FrontendAction::Execute() with a mutable
CompilerInstance, allowing FeatureModules to register callbacks, remap
files, etc.
Differential Revision: https://reviews.llvm.org/D124176
Added:
Modified:
clang-tools-extra/clangd/FeatureModule.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/unittests/FeatureModulesTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/FeatureModule.h
b/clang-tools-extra/clangd/FeatureModule.h
index 8f07b18412671..43007a297469b 100644
--- a/clang-tools-extra/clangd/FeatureModule.h
+++ b/clang-tools-extra/clangd/FeatureModule.h
@@ -20,6 +20,7 @@
#include
namespace clang {
+class CompilerInstance;
namespace clangd {
struct Diag;
class LSPBinder;
@@ -105,6 +106,11 @@ class FeatureModule {
/// Listeners are destroyed once the AST is built.
virtual ~ASTListener() = default;
+/// Called before every AST build, both for main file and preamble. The
call
+/// happens immediately before FrontendAction::Execute(), with Preprocessor
+/// set up already and after BeginSourceFile() on main file was called.
+virtual void beforeExecute(CompilerInstance &CI) {}
+
/// Called everytime a diagnostic is encountered. Modules can use this
/// modify the final diagnostic, or store some information to surface code
/// actions later on.
diff --git a/clang-tools-extra/clangd/ParsedAST.cpp
b/clang-tools-extra/clangd/ParsedAST.cpp
index ef744d1f893b0..235362b5d599c 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -550,6 +550,12 @@ ParsedAST::build(llvm::StringRef Filename, const
ParseInputs &Inputs,
// Collect tokens of the main file.
syntax::TokenCollector CollectTokens(Clang->getPreprocessor());
+ // To remain consistent with preamble builds, these callbacks must be called
+ // exactly here, after preprocessor is initialized and BeginSourceFile() was
+ // called already.
+ for (const auto &L : ASTListeners)
+L->beforeExecute(*Clang);
+
if (llvm::Error Err = Action->Execute())
log("Execute() failed when building AST for {0}: {1}", MainInput.getFile(),
toString(std::move(Err)));
diff --git a/clang-tools-extra/clangd/Preamble.cpp
b/clang-tools-extra/clangd/Preamble.cpp
index 0cc5b65a94d64..5ecd6f3a0bdf1 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -64,9 +64,12 @@ bool compileCommandsAreEqual(const tooling::CompileCommand
&LHS,
class CppFilePreambleCallbacks : public PreambleCallbacks {
public:
- CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback,
- PreambleBuildStats *Stats)
- : File(File), ParsedCallback(ParsedCallback), Stats(Stats) {}
+ CppFilePreambleCallbacks(
+ PathRef File, PreambleParsedCallback ParsedCallback,
+ PreambleBuildStats *Stats,
+ std::function BeforeExecuteCallback)
+ : File(File), ParsedCallback(ParsedCallback), Stats(Stats),
+BeforeExecuteCallback(std::move(BeforeExecuteCallback)) {}
IncludeStructure takeIncludes() { return std::move(Includes); }
@@ -115,6 +118,8 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {
LangOpts = &CI.getLangOpts();
SourceMgr = &CI.getSourceManager();
Includes.collect(CI);
+if (BeforeExecuteCallback)
+ BeforeExecuteCallback(CI);
}
std::unique_ptr createPPCallbacks() override {
@@ -156,6 +161,7 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {
const clang::LangOptions *LangOpts = nullptr;
const SourceManager *SourceMgr = nullptr;
PreambleBuildStats *Stats;
+ std::function BeforeExecuteCallback;
};
// Represents directives other than includes, where basic textual information
is
@@ -477,7 +483,11 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
// to read back. We rely on dynamic index for the comments instead.
CI.getPreprocessorOpts().WriteCommentListToPCH = false;
- CppFilePreambleCallbacks CapturedInfo(FileName, PreambleCallback, Stats);
+ CppFilePreambleCallbacks CapturedInfo(FileName, PreambleCallback, Stats,
+[&ASTListeners](CompilerInstance &CI) {
+ for (const auto &L : ASTListeners)
+L->beforeExecute(CI);
+});
auto VFS = Inputs.TFS->view(Inputs.CompileCom
[clang] 7e45912 - [clang] Do not crash on arrow operator on dependent type.
Author: Adam Czachorowski
Date: 2022-03-25T15:48:08+01:00
New Revision: 7e459126185f4d5115e6e2166a866aba1369d024
URL:
https://github.com/llvm/llvm-project/commit/7e459126185f4d5115e6e2166a866aba1369d024
DIFF:
https://github.com/llvm/llvm-project/commit/7e459126185f4d5115e6e2166a866aba1369d024.diff
LOG: [clang] Do not crash on arrow operator on dependent type.
There seems to be more than one way to get to that state. I included to
example cases in the test, both were noticed recently.
There is room for improvement, for example by creating RecoveryExpr in
place of the bad initializer, but for now let's stop the crashes.
Differential Revision: https://reviews.llvm.org/D121824
Added:
Modified:
clang/lib/Sema/TreeTransform.h
clang/test/SemaCXX/arrow-operator.cpp
Removed:
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index fb830ef2a118a..1ee457b0566ae 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -14757,6 +14757,10 @@
TreeTransform::RebuildCXXOperatorCallExpr(OverloadedOperatorKind Op,
return getSema().CreateBuiltinArraySubscriptExpr(
First, Callee->getBeginLoc(), Second, OpLoc);
} else if (Op == OO_Arrow) {
+// It is possible that the type refers to a RecoveryExpr created earlier
+// in the tree transformation.
+if (First->getType()->isDependentType())
+ return ExprError();
// -> is never a builtin operation.
return SemaRef.BuildOverloadedArrowExpr(nullptr, First, OpLoc);
} else if (Second == nullptr || isPostIncDec) {
diff --git a/clang/test/SemaCXX/arrow-operator.cpp
b/clang/test/SemaCXX/arrow-operator.cpp
index 3e32a6ba33eb2..c6d2a99251be4 100644
--- a/clang/test/SemaCXX/arrow-operator.cpp
+++ b/clang/test/SemaCXX/arrow-operator.cpp
@@ -65,3 +65,51 @@ void test() {
}
} // namespace arrow_suggest
+
+namespace no_crash_dependent_type {
+
+template
+struct A {
+ void call();
+ A *operator->();
+};
+
+template
+void foo() {
+ // The "requires an initializer" error seems unnecessary.
+ A &x = blah[7]; // expected-error {{use of undeclared identifier
'blah'}} \
+// expected-error {{requires an initializer}}
+ // x is dependent.
+ x->call();
+}
+
+void test() {
+ foo(); // expected-note {{requested here}}
+}
+
+} // namespace no_crash_dependent_type
+
+namespace clangd_issue_1073_no_crash_dependent_type {
+
+template struct Ptr {
+ T *operator->();
+};
+
+struct Struct {
+ int len;
+};
+
+template
+struct TemplateStruct {
+ Ptr val(); // expected-note {{declared here}}
+};
+
+template
+void templateFunc(const TemplateStruct &ts) {
+ Ptr ptr = ts.val(); // expected-error {{function is not marked
const}}
+ auto foo = ptr->len;
+}
+
+template void templateFunc<0>(const TemplateStruct<0> &); // expected-note
{{requested here}}
+
+} // namespace clangd_issue_1073_no_crash_dependent_type
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 6009d0d - [clangd] Track time spent in filesystem ops during preamble builds
Author: Adam Czachorowski
Date: 2022-03-21T18:33:01+01:00
New Revision: 6009d0d5801d8f4ff45b425f3fe3792e93aec553
URL:
https://github.com/llvm/llvm-project/commit/6009d0d5801d8f4ff45b425f3fe3792e93aec553
DIFF:
https://github.com/llvm/llvm-project/commit/6009d0d5801d8f4ff45b425f3fe3792e93aec553.diff
LOG: [clangd] Track time spent in filesystem ops during preamble builds
In some deployments, for example when running on FUSE or using some
network-based VFS implementation, the filesystem operations might add up
to a significant fraction of preamble build time. This change allows us
to track time spent in FS operations to better understand the problem.
Differential Revision: https://reviews.llvm.org/D121712
Added:
Modified:
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/Preamble.h
clang-tools-extra/clangd/TUScheduler.cpp
Removed:
diff --git a/clang-tools-extra/clangd/Preamble.cpp
b/clang-tools-extra/clangd/Preamble.cpp
index ea518f099689e..aab6dc78092f7 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -312,12 +312,98 @@ bool isMainFile(llvm::StringRef FileName, const
SourceManager &SM) {
return FE && *FE == SM.getFileEntryForID(SM.getMainFileID());
}
+// Accumulating wall time timer. Similar to llvm::Timer, but much cheaper,
+// it only tracks wall time.
+// Since this is a generic timer, We may want to move this to support/ if we
+// find a use case outside of FS time tracking.
+class WallTimer {
+public:
+ WallTimer() : TotalTime(std::chrono::steady_clock::duration::zero()) {}
+ // [Re-]Start the timer.
+ void startTimer() { StartTime = std::chrono::steady_clock::now(); }
+ // Stop the timer and update total time.
+ void stopTimer() {
+TotalTime += std::chrono::steady_clock::now() - StartTime;
+ }
+ // Return total time, in seconds.
+ double getTime() { return std::chrono::duration(TotalTime).count(); }
+
+private:
+ std::chrono::steady_clock::duration TotalTime;
+ std::chrono::steady_clock::time_point StartTime;
+};
+
+class WallTimerRegion {
+public:
+ WallTimerRegion(WallTimer &T) : T(T) { T.startTimer(); }
+ ~WallTimerRegion() { T.stopTimer(); }
+
+private:
+ WallTimer &T;
+};
+
+// Used by TimerFS, tracks time spent in status() and getBuffer() calls while
+// proxying to underlying File implementation.
+class TimerFile : public llvm::vfs::File {
+public:
+ TimerFile(WallTimer &Timer, std::unique_ptr InnerFile)
+ : Timer(Timer), InnerFile(std::move(InnerFile)) {}
+
+ llvm::ErrorOr status() override {
+WallTimerRegion T(Timer);
+return InnerFile->status();
+ }
+ llvm::ErrorOr>
+ getBuffer(const Twine &Name, int64_t FileSize, bool RequiresNullTerminator,
+bool IsVolatile) override {
+WallTimerRegion T(Timer);
+return InnerFile->getBuffer(Name, FileSize, RequiresNullTerminator,
+IsVolatile);
+ }
+ std::error_code close() override {
+WallTimerRegion T(Timer);
+return InnerFile->close();
+ }
+
+private:
+ WallTimer &Timer;
+ std::unique_ptr InnerFile;
+};
+
+// A wrapper for FileSystems that tracks the amount of time spent in status()
+// and openFileForRead() calls.
+class TimerFS : public llvm::vfs::ProxyFileSystem {
+public:
+ TimerFS(llvm::IntrusiveRefCntPtr FS)
+ : ProxyFileSystem(std::move(FS)) {}
+
+ llvm::ErrorOr>
+ openFileForRead(const llvm::Twine &Path) override {
+WallTimerRegion T(Timer);
+auto FileOr = getUnderlyingFS().openFileForRead(Path);
+if (!FileOr)
+ return FileOr;
+return std::make_unique(Timer, std::move(FileOr.get()));
+ }
+
+ llvm::ErrorOr status(const llvm::Twine &Path) override {
+WallTimerRegion T(Timer);
+return getUnderlyingFS().status(Path);
+ }
+
+ double getTime() { return Timer.getTime(); }
+
+private:
+ WallTimer Timer;
+};
+
} // namespace
std::shared_ptr
buildPreamble(PathRef FileName, CompilerInvocation CI,
const ParseInputs &Inputs, bool StoreInMemory,
- PreambleParsedCallback PreambleCallback) {
+ PreambleParsedCallback PreambleCallback,
+ PreambleBuildStats *Stats) {
// Note that we don't need to copy the input contents, preamble can live
// without those.
auto ContentsBuffer =
@@ -375,18 +461,25 @@ buildPreamble(PathRef FileName, CompilerInvocation CI,
llvm::SmallString<32> AbsFileName(FileName);
VFS->makeAbsolute(AbsFileName);
auto StatCache = std::make_unique(AbsFileName);
+ auto StatCacheFS = StatCache->getProducingFS(VFS);
+ llvm::IntrusiveRefCntPtr TimedFS(new TimerFS(StatCacheFS));
+
+ WallTimer PreambleTimer;
+ PreambleTimer.startTimer();
auto BuiltPreamble = PrecompiledPreamble::Build(
CI, ContentsBuffer.get(), Bounds, *PreambleDiagsEngine,
- StatCache->getProducingFS(VFS),
- std::make_shared(), StoreInMemory, CapturedInfo);
+ St
[clang] 8f4ea36 - [clang] Improve laziness of resolving module map headers.
Author: Adam Czachorowski
Date: 2022-03-01T15:56:23+01:00
New Revision: 8f4ea36bfe4caf7d08f9778ee2a347b78f02bc0f
URL:
https://github.com/llvm/llvm-project/commit/8f4ea36bfe4caf7d08f9778ee2a347b78f02bc0f
DIFF:
https://github.com/llvm/llvm-project/commit/8f4ea36bfe4caf7d08f9778ee2a347b78f02bc0f.diff
LOG: [clang] Improve laziness of resolving module map headers.
clang has support for lazy headers in module maps - if size and/or
modtime and provided in the cppmap file, headers are only resolved when
an include directive for a file with that size/modtime is encoutered.
Before this change, the lazy resolution was all-or-nothing per module.
That means as soon as even one file in that module potentially matched
an include, all lazy files in that module were resolved. With this
change, only files with matching size/modtime will be resolved.
The goal is to avoid unnecessary stat() calls on non-included files,
which is especially valuable on networked file systems, with higher
latency.
Differential Revision: https://reviews.llvm.org/D120569
Added:
Modified:
clang/include/clang/Lex/ModuleMap.h
clang/lib/Frontend/FrontendAction.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Serialization/ASTWriter.cpp
Removed:
diff --git a/clang/include/clang/Lex/ModuleMap.h
b/clang/include/clang/Lex/ModuleMap.h
index 26169ae9cee95..538dcc7c4bec1 100644
--- a/clang/include/clang/Lex/ModuleMap.h
+++ b/clang/include/clang/Lex/ModuleMap.h
@@ -456,8 +456,11 @@ class ModuleMap {
/// is effectively internal, but is exposed so HeaderSearch can call it.
void resolveHeaderDirectives(const FileEntry *File) const;
- /// Resolve all lazy header directives for the specified module.
- void resolveHeaderDirectives(Module *Mod) const;
+ /// Resolve lazy header directives for the specified module. If File is
+ /// provided, only headers with same size and modtime are resolved. If File
+ /// is not set, all headers are resolved.
+ void resolveHeaderDirectives(Module *Mod,
+ llvm::Optional File) const;
/// Reports errors if a module must not include a specific file.
///
diff --git a/clang/lib/Frontend/FrontendAction.cpp
b/clang/lib/Frontend/FrontendAction.cpp
index c5b9e80356db4..b8f92381613a3 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -331,7 +331,7 @@ static std::error_code collectModuleHeaderIncludes(
return std::error_code();
// Resolve all lazy header directives to header files.
- ModMap.resolveHeaderDirectives(Module);
+ ModMap.resolveHeaderDirectives(Module, /*File=*/llvm::None);
// If any headers are missing, we can't build this module. In most cases,
// diagnostics for this should have already been produced; we only get here
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 0b136aeb580fe..824b2bb192909 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -482,7 +482,7 @@ void ModuleMap::diagnoseHeaderInclusion(Module
*RequestingModule,
if (RequestingModule) {
resolveUses(RequestingModule, /*Complain=*/false);
-resolveHeaderDirectives(RequestingModule);
+resolveHeaderDirectives(RequestingModule, /*File=*/llvm::None);
}
bool Excluded = false;
@@ -1191,25 +1191,35 @@ void ModuleMap::resolveHeaderDirectives(const FileEntry
*File) const {
auto BySize = LazyHeadersBySize.find(File->getSize());
if (BySize != LazyHeadersBySize.end()) {
for (auto *M : BySize->second)
- resolveHeaderDirectives(M);
+ resolveHeaderDirectives(M, File);
LazyHeadersBySize.erase(BySize);
}
auto ByModTime = LazyHeadersByModTime.find(File->getModificationTime());
if (ByModTime != LazyHeadersByModTime.end()) {
for (auto *M : ByModTime->second)
- resolveHeaderDirectives(M);
+ resolveHeaderDirectives(M, File);
LazyHeadersByModTime.erase(ByModTime);
}
}
-void ModuleMap::resolveHeaderDirectives(Module *Mod) const {
+void ModuleMap::resolveHeaderDirectives(
+Module *Mod, llvm::Optional File) const {
bool NeedsFramework = false;
- for (auto &Header : Mod->UnresolvedHeaders)
-// This operation is logically const; we're just changing how we represent
-// the header information for this file.
-const_cast(this)->resolveHeader(Mod, Header, NeedsFramework);
- Mod->UnresolvedHeaders.clear();
+ SmallVector NewHeaders;
+ const auto Size = File ? File.getValue()->getSize() : 0;
+ const auto ModTime = File ? File.getValue()->getModificationTime() : 0;
+
+ for (auto &Header : Mod->UnresolvedHeaders) {
+if (File && ((Header.ModTime && Header.ModTime != ModTime) ||
+ (Header.Size && Header.Size != Size)))
+ NewHeaders.push_back(Header);
+else
+ // This operation is logically const; we're just changing how we
represent
+ // the header information for
[clang-tools-extra] 55a7931 - [clang][clangd] Improve signature help for variadic functions.
Author: Adam Czachorowski
Date: 2021-11-18T15:50:47+01:00
New Revision: 55a79318c60d8a39329195f43bf43b89da9a638e
URL:
https://github.com/llvm/llvm-project/commit/55a79318c60d8a39329195f43bf43b89da9a638e
DIFF:
https://github.com/llvm/llvm-project/commit/55a79318c60d8a39329195f43bf43b89da9a638e.diff
LOG: [clang][clangd] Improve signature help for variadic functions.
This covers both C-style variadic functions and template variadic w/
parameter packs.
Previously we would return no signatures when working with template
variadic functions once activeParameter reached the position of the
parameter pack (except when it was the only param, then we'd still
show it when no arguments were given). With this commit, we now show
signathure help correctly.
Additionally, this commit fixes the activeParameter value in LSP output
of clangd in the presence of variadic functions (both kinds). LSP does
not allow the activeParamter to be higher than the number of parameters
in the active signature. With "..." or parameter pack being just one
argument, for all but first argument passed to "..." we'd report
incorrect activeParameter value. Clients such as VSCode would then treat
it as 0, as suggested in the spec) and highlight the wrong parameter.
In the future, we should add support for per-signature activeParamter
value, which exists in LSP since 3.16.0. This is not part of this
commit.
Differential Revision: https://reviews.llvm.org/D111318
Added:
clang/test/CodeCompletion/variadic-template.cpp
Modified:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang/include/clang/Sema/Overload.h
clang/lib/Sema/SemaCodeComplete.cpp
clang/lib/Sema/SemaOverload.cpp
Removed:
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp
b/clang-tools-extra/clangd/CodeComplete.cpp
index f033b5ec001d8..ac6c3b077d472 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -872,6 +872,28 @@ struct ScoredSignature {
SignatureQualitySignals Quality;
};
+// Returns the index of the parameter matching argument number "Arg.
+// This is usually just "Arg", except for variadic functions/templates, where
+// "Arg" might be higher than the number of parameters. When that happens, we
+// assume the last parameter is variadic and assume all further args are
+// part of it.
+int paramIndexForArg(const CodeCompleteConsumer::OverloadCandidate &Candidate,
+ int Arg) {
+ int NumParams = 0;
+ if (const auto *F = Candidate.getFunction()) {
+NumParams = F->getNumParams();
+if (F->isVariadic())
+ ++NumParams;
+ } else if (auto *T = Candidate.getFunctionType()) {
+if (auto *Proto = T->getAs()) {
+ NumParams = Proto->getNumParams();
+ if (Proto->isVariadic())
+++NumParams;
+}
+ }
+ return std::min(Arg, std::max(NumParams - 1, 0));
+}
+
class SignatureHelpCollector final : public CodeCompleteConsumer {
public:
SignatureHelpCollector(const clang::CodeCompleteOptions &CodeCompleteOpts,
@@ -902,7 +924,9 @@ class SignatureHelpCollector final : public
CodeCompleteConsumer {
SigHelp.activeSignature = 0;
assert(CurrentArg <= (unsigned)std::numeric_limits::max() &&
"too many arguments");
+
SigHelp.activeParameter = static_cast(CurrentArg);
+
for (unsigned I = 0; I < NumCandidates; ++I) {
OverloadCandidate Candidate = Candidates[I];
// We want to avoid showing instantiated signatures, because they may be
@@ -912,6 +936,14 @@ class SignatureHelpCollector final : public
CodeCompleteConsumer {
if (auto *Pattern = Func->getTemplateInstantiationPattern())
Candidate = OverloadCandidate(Pattern);
}
+ if (static_cast(I) == SigHelp.activeSignature) {
+// The activeParameter in LSP relates to the activeSignature. There is
+// another, per-signature field, but we currently do not use it and not
+// all clients might support it.
+// FIXME: Add support for per-signature activeParameter field.
+SigHelp.activeParameter =
+paramIndexForArg(Candidate, SigHelp.activeParameter);
+ }
const auto *CCS = Candidate.CreateSignatureString(
CurrentArg, S, *Allocator, CCTUInfo, true);
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 81dfdcb371fb4..5612a0b17fbba 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2622,6 +2622,104 @@ TEST(SignatureHelpTest, ConstructorInitializeFields) {
}
}
+TEST(SignatureHelpTest, Variadic) {
+ const std::string Header = R"cpp(
+void fun(int x, ...) {}
+void test() {)cpp";
+ const std::string ExpectedSig = "fun([[int x]], [[...]]) -> vo
[clang] 6d09aae - Revert "[clang] Add early exit when checking for const init of arrays."
Author: Adam Czachorowski
Date: 2021-11-10T20:59:35+01:00
New Revision: 6d09aaecdfe51e13fc64d539aa7c9a790de341d7
URL:
https://github.com/llvm/llvm-project/commit/6d09aaecdfe51e13fc64d539aa7c9a790de341d7
DIFF:
https://github.com/llvm/llvm-project/commit/6d09aaecdfe51e13fc64d539aa7c9a790de341d7.diff
LOG: Revert "[clang] Add early exit when checking for const init of arrays."
This reverts commit 48bb5f4cbe8d5951c1153e469dc6713a122b7fa3.
Several breakages, including ARM (fixed later, but not sufficient) and
MSan (to be diagnosed later).
Differential Revision: https://reviews.llvm.org/D113599
Added:
Modified:
clang/lib/AST/ExprConstant.cpp
Removed:
clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index a6da3d3fbea2d..fe96db9ca918e 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -10596,55 +10596,28 @@ bool ArrayExprEvaluator::VisitCXXConstructExpr(const
CXXConstructExpr *E,
bool HadZeroInit = Value->hasValue();
if (const ConstantArrayType *CAT = Info.Ctx.getAsConstantArrayType(Type)) {
-unsigned FinalSize = CAT->getSize().getZExtValue();
+unsigned N = CAT->getSize().getZExtValue();
// Preserve the array filler if we had prior zero-initialization.
APValue Filler =
HadZeroInit && Value->hasArrayFiller() ? Value->getArrayFiller()
: APValue();
-*Value = APValue(APValue::UninitArray(), 0, FinalSize);
-if (FinalSize == 0)
- return true;
+*Value = APValue(APValue::UninitArray(), N, N);
+
+if (HadZeroInit)
+ for (unsigned I = 0; I != N; ++I)
+Value->getArrayInitializedElt(I) = Filler;
+// Initialize the elements.
LValue ArrayElt = Subobject;
ArrayElt.addArray(Info, E, CAT);
-// We do the whole initialization in two passes, first for just one
element,
-// then for the whole array. It's possible we may find out we can't do
const
-// init in the first pass, in which case we avoid allocating a potentially
-// large array. We don't do more passes because expanding array requires
-// copying the data, which is wasteful.
-for (const unsigned N : {1u, FinalSize}) {
- unsigned OldElts = Value->getArrayInitializedElts();
- if (OldElts == N)
-break;
-
- // Expand the array to appropriate size.
- APValue NewValue(APValue::UninitArray(), N, FinalSize);
- for (unsigned I = 0; I < OldElts; ++I)
-NewValue.getArrayInitializedElt(I).swap(
-Value->getArrayInitializedElt(I));
- Value->swap(NewValue);
-
- if (HadZeroInit)
-for (unsigned I = OldElts; I < N; ++I)
- Value->getArrayInitializedElt(I) = Filler;
-
- // Initialize the elements.
- for (unsigned I = OldElts; I < N; ++I) {
-if (!VisitCXXConstructExpr(E, ArrayElt,
- &Value->getArrayInitializedElt(I),
- CAT->getElementType()) ||
-!HandleLValueArrayAdjustment(Info, E, ArrayElt,
- CAT->getElementType(), 1))
- return false;
-// When checking for const initilization any diagnostic is considered
-// an error.
-if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
-!Info.keepEvaluatingAfterFailure())
- return false;
- }
-}
+for (unsigned I = 0; I != N; ++I)
+ if (!VisitCXXConstructExpr(E, ArrayElt,
&Value->getArrayInitializedElt(I),
+ CAT->getElementType()) ||
+ !HandleLValueArrayAdjustment(Info, E, ArrayElt,
CAT->getElementType(),
+ 1))
+return false;
return true;
}
diff --git a/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
b/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
deleted file mode 100644
index 1bbc1b5c863c0..0
--- a/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
+++ /dev/null
@@ -1,11 +0,0 @@
-// REQUIRES: shell
-// UNSUPPORTED: win32
-// RUN: ulimit -v 1048576
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -triple=x86_64 %s
-// expected-no-diagnostics
-
-// This used to require too much memory and crash with OOM.
-struct {
- int a, b, c, d;
-} arr[1<<30];
-
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 581a6a8 - [clang] Fix armv7-quick build by hardcoding -triple=x86_64 in OOM test.
Author: Adam Czachorowski Date: 2021-11-10T19:24:06+01:00 New Revision: 581a6a8118f55d2662e180880558db95f808b0dc URL: https://github.com/llvm/llvm-project/commit/581a6a8118f55d2662e180880558db95f808b0dc DIFF: https://github.com/llvm/llvm-project/commit/581a6a8118f55d2662e180880558db95f808b0dc.diff LOG: [clang] Fix armv7-quick build by hardcoding -triple=x86_64 in OOM test. The test was added in 48bb5f4cbe8d5951c1153e469dc6713a122b7fa3 and it creates a very large array, which is too large for ARM. Making the array smaller is not a good option, since we would need a very low ulimit and could then hit it for other reasons. Should fix the https://lab.llvm.org/buildbot/#/builders/171/builds/5851 failure. Differential Revision: https://reviews.llvm.org/D113583 Added: Modified: clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp Removed: diff --git a/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp b/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp index 4504c0caed55..1bbc1b5c863c 100644 --- a/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp +++ b/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp @@ -1,7 +1,7 @@ // REQUIRES: shell // UNSUPPORTED: win32 // RUN: ulimit -v 1048576 -// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -triple=x86_64 %s // expected-no-diagnostics // This used to require too much memory and crash with OOM. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 48bb5f4 - [clang] Add early exit when checking for const init of arrays.
Author: Adam Czachorowski
Date: 2021-11-10T18:11:21+01:00
New Revision: 48bb5f4cbe8d5951c1153e469dc6713a122b7fa3
URL:
https://github.com/llvm/llvm-project/commit/48bb5f4cbe8d5951c1153e469dc6713a122b7fa3
DIFF:
https://github.com/llvm/llvm-project/commit/48bb5f4cbe8d5951c1153e469dc6713a122b7fa3.diff
LOG: [clang] Add early exit when checking for const init of arrays.
Before this commit, on code like:
struct S { ... };
S arr[1000];
while checking if arr is constexpr, clang would reserve memory for
arr before running constructor for S. If S turned out to not have a
valid constexpr c-tor, clang would still try to initialize each element
(and, in case the c-tor was trivial, even skipping the constexpr step
limit), only to discard that whole APValue later, since the first
element generated a diagnostic.
With this change, we start by allocating just 1 element in the array to
try out the c-tor and take an early exit if any diagnostics are
generated, avoiding possibly large memory allocation and a lot of work
initializing to-be-discarded APValues.
Fixes 51712 and 51843.
In the future we may want to be smarter about large possibly-constexrp
arrays and maybe make the allocation lazy.
Differential Revision: https://reviews.llvm.org/D113120
Added:
clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
Modified:
clang/lib/AST/ExprConstant.cpp
Removed:
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index fa0d22064f0eb..a6da3d3fbea2d 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -10596,28 +10596,55 @@ bool ArrayExprEvaluator::VisitCXXConstructExpr(const
CXXConstructExpr *E,
bool HadZeroInit = Value->hasValue();
if (const ConstantArrayType *CAT = Info.Ctx.getAsConstantArrayType(Type)) {
-unsigned N = CAT->getSize().getZExtValue();
+unsigned FinalSize = CAT->getSize().getZExtValue();
// Preserve the array filler if we had prior zero-initialization.
APValue Filler =
HadZeroInit && Value->hasArrayFiller() ? Value->getArrayFiller()
: APValue();
-*Value = APValue(APValue::UninitArray(), N, N);
-
-if (HadZeroInit)
- for (unsigned I = 0; I != N; ++I)
-Value->getArrayInitializedElt(I) = Filler;
+*Value = APValue(APValue::UninitArray(), 0, FinalSize);
+if (FinalSize == 0)
+ return true;
-// Initialize the elements.
LValue ArrayElt = Subobject;
ArrayElt.addArray(Info, E, CAT);
-for (unsigned I = 0; I != N; ++I)
- if (!VisitCXXConstructExpr(E, ArrayElt,
&Value->getArrayInitializedElt(I),
- CAT->getElementType()) ||
- !HandleLValueArrayAdjustment(Info, E, ArrayElt,
- CAT->getElementType(), 1))
-return false;
+// We do the whole initialization in two passes, first for just one
element,
+// then for the whole array. It's possible we may find out we can't do
const
+// init in the first pass, in which case we avoid allocating a potentially
+// large array. We don't do more passes because expanding array requires
+// copying the data, which is wasteful.
+for (const unsigned N : {1u, FinalSize}) {
+ unsigned OldElts = Value->getArrayInitializedElts();
+ if (OldElts == N)
+break;
+
+ // Expand the array to appropriate size.
+ APValue NewValue(APValue::UninitArray(), N, FinalSize);
+ for (unsigned I = 0; I < OldElts; ++I)
+NewValue.getArrayInitializedElt(I).swap(
+Value->getArrayInitializedElt(I));
+ Value->swap(NewValue);
+
+ if (HadZeroInit)
+for (unsigned I = OldElts; I < N; ++I)
+ Value->getArrayInitializedElt(I) = Filler;
+
+ // Initialize the elements.
+ for (unsigned I = OldElts; I < N; ++I) {
+if (!VisitCXXConstructExpr(E, ArrayElt,
+ &Value->getArrayInitializedElt(I),
+ CAT->getElementType()) ||
+!HandleLValueArrayAdjustment(Info, E, ArrayElt,
+ CAT->getElementType(), 1))
+ return false;
+// When checking for const initilization any diagnostic is considered
+// an error.
+if (Info.EvalStatus.Diag && !Info.EvalStatus.Diag->empty() &&
+!Info.keepEvaluatingAfterFailure())
+ return false;
+ }
+}
return true;
}
diff --git a/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
b/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
new file mode 100644
index 0..4504c0caed55f
--- /dev/null
+++ b/clang/test/SemaCXX/PR51712-large-array-constexpr-check-oom.cpp
@@ -0,0 +1,11 @@
+// REQUIRES: shell
+// UNSUPPORTED: win32
+// RUN: ulimit -v 1048576
+// RUN: %clang_cc1 -std=c++11 -fs
[clang] 7a2b1bd - [clang] Do not crash in APValue::prettyPrint() on forward-decl structs.
Author: Adam Czachorowski
Date: 2021-11-10T17:17:00+01:00
New Revision: 7a2b1bdb4c8a099ebc38b7f802988244ad21fcc0
URL:
https://github.com/llvm/llvm-project/commit/7a2b1bdb4c8a099ebc38b7f802988244ad21fcc0
DIFF:
https://github.com/llvm/llvm-project/commit/7a2b1bdb4c8a099ebc38b7f802988244ad21fcc0.diff
LOG: [clang] Do not crash in APValue::prettyPrint() on forward-decl structs.
The call to getTypeSizeInChars() is replaced with
getTypeSizeInCharsIfKnown(), which does not crash on forward declared
structs. This only affects printing.
Differential Revision: https://reviews.llvm.org/D113570
Added:
Modified:
clang-tools-extra/clangd/unittests/HoverTests.cpp
clang/lib/AST/APValue.cpp
Removed:
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 2e33ce4c5d101..53df965fef2fa 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2963,6 +2963,20 @@ TEST(Hover, SpaceshipTemplateNoCrash) {
EXPECT_EQ(HI->Documentation, "Foo bar baz");
}
+TEST(Hover, ForwardStructNoCrash) {
+ Annotations T(R"cpp(
+ struct Foo;
+ int bar;
+ auto baz = (Fo^o*)&bar;
+)cpp");
+
+ TestTU TU = TestTU::withCode(T.code());
+ auto AST = TU.build();
+ auto HI = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+ ASSERT_TRUE(HI);
+ EXPECT_EQ(*HI->Value, "&bar");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
diff --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
index 9a9233bc1ea74..ef333c7711663 100644
--- a/clang/lib/AST/APValue.cpp
+++ b/clang/lib/AST/APValue.cpp
@@ -700,7 +700,9 @@ void APValue::printPretty(raw_ostream &Out, const
PrintingPolicy &Policy,
if (!hasLValuePath()) {
// No lvalue path: just print the offset.
CharUnits O = getLValueOffset();
- CharUnits S = Ctx ? Ctx->getTypeSizeInChars(InnerTy) : CharUnits::Zero();
+ CharUnits S = Ctx ? Ctx->getTypeSizeInCharsIfKnown(InnerTy).getValueOr(
+ CharUnits::Zero())
+: CharUnits::Zero();
if (!O.isZero()) {
if (IsReference)
Out << "*(";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 97fbc97 - [clangd] Find definition of ClassTemplate without going through index.
Author: Adam Czachorowski
Date: 2021-11-04T15:25:21+01:00
New Revision: 97fbc975fab19be68eb6a643ddac850ef71c2ecd
URL:
https://github.com/llvm/llvm-project/commit/97fbc975fab19be68eb6a643ddac850ef71c2ecd
DIFF:
https://github.com/llvm/llvm-project/commit/97fbc975fab19be68eb6a643ddac850ef71c2ecd.diff
LOG: [clangd] Find definition of ClassTemplate without going through index.
I noticed that, while go-to-def works on cases like:
namespace ns {
template struct Foo {};
}
using ::ns::Fo^o;
it only works because of the FileIndex. We can get definition location
directly from AST too.
Differential Revision: https://reviews.llvm.org/D113029
Added:
Modified:
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/XRefs.cpp
b/clang-tools-extra/clangd/XRefs.cpp
index 85c6b7b771fce..b29d29e5104ee 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -80,6 +80,9 @@ const NamedDecl *getDefinition(const NamedDecl *D) {
return VD->getDefinition();
if (const auto *FD = dyn_cast(D))
return FD->getDefinition();
+ if (const auto *CTD = dyn_cast(D))
+if (const auto *RD = CTD->getTemplatedDecl())
+ return RD->getDefinition();
// Objective-C classes can have three types of declarations:
//
// - forward declaration: @class MyClass;
diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 802367645c859..d567e0d77b39c 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -675,7 +675,7 @@ TEST(LocateSymbol, All) {
R"cpp(// Declaration of explicit template specialization
template
-struct $decl[[Foo]] {};
+struct $decl[[$def[[Foo {};
template <>
struct Fo^o {};
@@ -683,12 +683,25 @@ TEST(LocateSymbol, All) {
R"cpp(// Declaration of partial template specialization
template
-struct $decl[[Foo]] {};
+struct $decl[[$def[[Foo {};
template
struct Fo^o {};
)cpp",
+ R"cpp(// Definition on ClassTemplateDecl
+namespace ns {
+ // Forward declaration.
+ template
+ struct $decl[[Foo]];
+
+ template
+ struct $def[[Foo]] {};
+}
+
+using ::ns::Fo^o;
+ )cpp",
+
R"cpp(// auto builtin type (not supported)
^auto x = 42;
)cpp",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 2174524 - [clangd] AddUsing: Fix support for template specializations.
Author: Adam Czachorowski
Date: 2021-10-26T17:32:33+02:00
New Revision: 2174524116a8379fb7a6453253524ec972b158df
URL:
https://github.com/llvm/llvm-project/commit/2174524116a8379fb7a6453253524ec972b158df
DIFF:
https://github.com/llvm/llvm-project/commit/2174524116a8379fb7a6453253524ec972b158df.diff
LOG: [clangd] AddUsing: Fix support for template specializations.
Before this change, we would add "using std::vector" instead of
just "using std::vector;", which would not even compile.
Fixes https://github.com/clangd/clangd/issues/904
Differential Revision: https://reviews.llvm.org/D112530
Added:
Modified:
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
index a8c937a951df..c1adbdde6199 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -278,8 +278,13 @@ bool AddUsing::prepare(const Selection &Inputs) {
if (!QualifierToRemove)
return false;
- auto SpelledTokens =
- TB.spelledForExpanded(TB.expandedTokens(E.getSourceRange()));
+ auto NameRange = E.getSourceRange();
+ if (auto T = E.getNamedTypeLoc().getAs())
{
+// Remove the template arguments from the name.
+NameRange.setEnd(T.getLAngleLoc().getLocWithOffset(-1));
+ }
+
+ auto SpelledTokens = TB.spelledForExpanded(TB.expandedTokens(NameRange));
if (!SpelledTokens)
return false;
auto SpelledRange = syntax::Token::range(SM, SpelledTokens->front(),
diff --git a/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp
b/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp
index c0c66dbc14d7..b0c2d71328bd 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp
@@ -444,6 +444,17 @@ one::t^wo::cc c;
#include "test.hpp"
using one::two::cc;using one::two::ee::ee_one;
cc c;
+)cpp"},
+// Template (like std::vector).
+{R"cpp(
+#include "test.hpp"
+one::v^ec foo;
+)cpp",
+ R"cpp(
+#include "test.hpp"
+using one::vec;
+
+vec foo;
)cpp"}};
llvm::StringMap EditedFiles;
for (const auto &Case : Cases) {
@@ -461,6 +472,7 @@ class cc {
};
}
using uu = two::cc;
+template struct vec {};
})cpp";
EXPECT_EQ(apply(SubCase, &EditedFiles), Case.ExpectedSource);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] e8f4a01 - [clangd] Fix a hover crash on templated spaceship operator.
Author: Adam Czachorowski
Date: 2021-10-26T17:28:40+02:00
New Revision: e8f4a01189143854f30e2bb622baa729a42f152d
URL:
https://github.com/llvm/llvm-project/commit/e8f4a01189143854f30e2bb622baa729a42f152d
DIFF:
https://github.com/llvm/llvm-project/commit/e8f4a01189143854f30e2bb622baa729a42f152d.diff
LOG: [clangd] Fix a hover crash on templated spaceship operator.
We make assumption that:
getDeclForComment(getDeclForComment(X)) == getDeclForComment(X)
but this is not true if you have a template
instantionation of a template instantiation, which is the case when, for
example, you have a <=> operator in a templated class.
This fix makes getDeclForComment() call itself recursively to ensure
this property is always true.
Fixes https://github.com/clangd/clangd/issues/901
Differential Revision: https://reviews.llvm.org/D112527
Added:
Modified:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/Hover.cpp
b/clang-tools-extra/clangd/Hover.cpp
index 8ef7d20c1d53..4285b3595059 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -262,24 +262,30 @@ const FunctionDecl *getUnderlyingFunction(const Decl *D) {
// Returns the decl that should be used for querying comments, either from
index
// or AST.
const NamedDecl *getDeclForComment(const NamedDecl *D) {
+ const NamedDecl *DeclForComment = D;
if (const auto *TSD = llvm::dyn_cast(D)) {
// Template may not be instantiated e.g. if the type didn't need to be
// complete; fallback to primary template.
if (TSD->getTemplateSpecializationKind() == TSK_Undeclared)
- return TSD->getSpecializedTemplate();
-if (const auto *TIP = TSD->getTemplateInstantiationPattern())
- return TIP;
- }
- if (const auto *TSD = llvm::dyn_cast(D)) {
+ DeclForComment = TSD->getSpecializedTemplate();
+else if (const auto *TIP = TSD->getTemplateInstantiationPattern())
+ DeclForComment = TIP;
+ } else if (const auto *TSD =
+ llvm::dyn_cast(D)) {
if (TSD->getTemplateSpecializationKind() == TSK_Undeclared)
- return TSD->getSpecializedTemplate();
-if (const auto *TIP = TSD->getTemplateInstantiationPattern())
- return TIP;
- }
- if (const auto *FD = D->getAsFunction())
+ DeclForComment = TSD->getSpecializedTemplate();
+else if (const auto *TIP = TSD->getTemplateInstantiationPattern())
+ DeclForComment = TIP;
+ } else if (const auto *FD = D->getAsFunction())
if (const auto *TIP = FD->getTemplateInstantiationPattern())
- return TIP;
- return D;
+ DeclForComment = TIP;
+ // Ensure that getDeclForComment(getDeclForComment(X)) =
getDeclForComment(X).
+ // This is usually not needed, but in strange cases of comparision operators
+ // being instantiated from spasceship operater, which itself is a template
+ // instantiation the recursrive call is necessary.
+ if (D != DeclForComment)
+DeclForComment = getDeclForComment(DeclForComment);
+ return DeclForComment;
}
// Look up information about D from the index, and add it to Hover.
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 6395da91463a..2e33ce4c5d10 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2934,6 +2934,35 @@ Value = val
def)pt";
EXPECT_EQ(HI.present().asPlainText(), ExpectedPlaintext);
}
+
+TEST(Hover, SpaceshipTemplateNoCrash) {
+ Annotations T(R"cpp(
+ namespace std {
+ struct strong_ordering {
+int n;
+constexpr operator int() const { return n; }
+static const strong_ordering equal, greater, less;
+ };
+ constexpr strong_ordering strong_ordering::equal = {0};
+ constexpr strong_ordering strong_ordering::greater = {1};
+ constexpr strong_ordering strong_ordering::less = {-1};
+ }
+
+ template
+ struct S {
+// Foo bar baz
+friend auto operator<=>(S, S) = default;
+ };
+ static_assert(S() =^= S());
+)cpp");
+
+ TestTU TU = TestTU::withCode(T.code());
+ TU.ExtraArgs.push_back("-std=c++20");
+ auto AST = TU.build();
+ auto HI = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+ EXPECT_EQ(HI->Documentation, "Foo bar baz");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 8fbac4e - [clangd] Add code completion of param name on /* inside function calls.
Author: Adam Czachorowski
Date: 2021-10-19T12:49:46+02:00
New Revision: 8fbac4e88ac3dde30310bb63b234045075cd338b
URL:
https://github.com/llvm/llvm-project/commit/8fbac4e88ac3dde30310bb63b234045075cd338b
DIFF:
https://github.com/llvm/llvm-project/commit/8fbac4e88ac3dde30310bb63b234045075cd338b.diff
LOG: [clangd] Add code completion of param name on /* inside function calls.
For example, if you have:
void foo(int bar);
foo(/*^
it should auto-complete to "bar=".
Because Sema callbacks for code completion in comments happen before we
have an AST we need to cheat in clangd by detecting completion on /*
before, moving cursor back by two characters, then running a simplified
verion of SignatureHelp to extract argument name(s) from possible
overloads.
Differential Revision: https://reviews.llvm.org/D110823
Added:
Modified:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/test/initialize-params.test
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index a9debfd2a6ed5..c01a4286884b8 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -542,7 +542,7 @@ void ClangdLSPServer::onInitialize(const InitializeParams
&Params,
"^", "&", "#", "?", ".", "=", "\"", "'", "|"}},
{"resolveProvider", false},
// We do extra checks, e.g. that > is part of ->.
- {"triggerCharacters", {".", "<", ">", ":", "\"", "/"}},
+ {"triggerCharacters", {".", "<", ">", ":", "\"", "/", "*"}},
}},
{"semanticTokensProvider",
llvm::json::Object{
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp
b/clang-tools-extra/clangd/CodeComplete.cpp
index 2d01a163431e2..8c4b292d4 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -1098,6 +1098,50 @@ class SignatureHelpCollector final : public
CodeCompleteConsumer {
const SymbolIndex *Index;
}; // SignatureHelpCollector
+// Used only for completion of C-style comments in function call (i.e.
+// /*foo=*/7). Similar to SignatureHelpCollector, but needs to do less work.
+class ParamNameCollector final : public CodeCompleteConsumer {
+public:
+ ParamNameCollector(const clang::CodeCompleteOptions &CodeCompleteOpts,
+ std::set &ParamNames)
+ : CodeCompleteConsumer(CodeCompleteOpts),
+Allocator(std::make_shared()),
+CCTUInfo(Allocator), ParamNames(ParamNames) {}
+
+ void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
+ OverloadCandidate *Candidates,
+ unsigned NumCandidates,
+ SourceLocation OpenParLoc) override {
+assert(CurrentArg <= (unsigned)std::numeric_limits::max() &&
+ "too many arguments");
+
+for (unsigned I = 0; I < NumCandidates; ++I) {
+ OverloadCandidate Candidate = Candidates[I];
+ auto *Func = Candidate.getFunction();
+ if (!Func || Func->getNumParams() <= CurrentArg)
+continue;
+ auto *PVD = Func->getParamDecl(CurrentArg);
+ if (!PVD)
+continue;
+ auto *Ident = PVD->getIdentifier();
+ if (!Ident)
+continue;
+ auto Name = Ident->getName();
+ if (!Name.empty())
+ParamNames.insert(Name.str());
+}
+ }
+
+private:
+ GlobalCodeCompletionAllocator &getAllocator() override { return *Allocator; }
+
+ CodeCompletionTUInfo &getCodeCompletionTUInfo() override { return CCTUInfo; }
+
+ std::shared_ptr Allocator;
+ CodeCompletionTUInfo CCTUInfo;
+ std::set &ParamNames;
+};
+
struct SemaCompleteInput {
PathRef FileName;
size_t Offset;
@@ -1860,6 +1904,59 @@ CompletionPrefix guessCompletionPrefix(llvm::StringRef
Content,
return Result;
}
+// Code complete the argument name on "/*" inside function call.
+// Offset should be pointing to the start of the comment, i.e.:
+// foo(^/*, rather than foo(/*^) where the cursor probably is.
+CodeCompleteResult codeCompleteComment(PathRef FileName, unsigned Offset,
+ llvm::StringRef Prefix,
+ const PreambleData *Preamble,
+ const ParseInputs &ParseInput) {
+ if (Preamble == nullptr) // Can't run without Sema.
+return CodeCompleteResult();
+
+ clang::CodeCompleteOptions Options;
+ Options.IncludeGlobals = false;
+ Options.IncludeMacros = false;
+ Options.IncludeCodePatterns = false;
+ Options.IncludeBriefComments = false;
+ std::set ParamNames;
+ // We want to see signatures coming from newly introduced includes, hence a
+ // full patch.
+ semaCodeComplete(
+ std::make_uniq
[clang-tools-extra] fba563e - [clangd] TargetFinder: Fix assert-crash on TemplateExpansion args.
Author: Adam Czachorowski
Date: 2021-10-13T13:15:36+02:00
New Revision: fba563e92b6412f49e7e49299d3d27f04f2e1400
URL:
https://github.com/llvm/llvm-project/commit/fba563e92b6412f49e7e49299d3d27f04f2e1400
DIFF:
https://github.com/llvm/llvm-project/commit/fba563e92b6412f49e7e49299d3d27f04f2e1400.diff
LOG: [clangd] TargetFinder: Fix assert-crash on TemplateExpansion args.
Previously we would call getAsTemplate() when kind == TemplateExpansion,
which triggers an assertion. The call is now replaced with
getAsTemplateOrTemplatePattern(), which is exactly the same as
getAsTemplate(), except it allows calls when kind == TemplateExpansion.
No change in behavior for no-assert builds.
Differential Revision: https://reviews.llvm.org/D111648
Added:
Modified:
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/FindTarget.cpp
b/clang-tools-extra/clangd/FindTarget.cpp
index 8419043f4462a..82e4dfeccf43d 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -507,7 +507,8 @@ struct TargetFinder {
// DeclRefExpr).
if (Arg.getKind() == TemplateArgument::Template ||
Arg.getKind() == TemplateArgument::TemplateExpansion) {
- if (TemplateDecl *TD = Arg.getAsTemplate().getAsTemplateDecl()) {
+ if (TemplateDecl *TD =
+ Arg.getAsTemplateOrTemplatePattern().getAsTemplateDecl()) {
report(TD, Flags);
}
}
diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index 14cb15a7644b0..9620db9838ae2 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -341,6 +341,15 @@ TEST_F(TargetDeclTest, Types) {
EXPECT_DECLS("TemplateSpecializationTypeLoc", "template class T");
Flags.clear();
+ Code = R"cpp(
+ template class ...T>
+ class C {
+C<[[T...]]> foo;
+};
+ )cpp";
+ EXPECT_DECLS("TemplateArgumentLoc", {"template class ...T"});
+ Flags.clear();
+
Code = R"cpp(
struct S{};
S X;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 08128fe - [clang] Make member var invalid when static initializer is invalid.
Author: Adam Czachorowski
Date: 2021-08-03T11:52:52+02:00
New Revision: 08128fe7059e20b3f97ae5abbdeff2e6f6c711ed
URL:
https://github.com/llvm/llvm-project/commit/08128fe7059e20b3f97ae5abbdeff2e6f6c711ed
DIFF:
https://github.com/llvm/llvm-project/commit/08128fe7059e20b3f97ae5abbdeff2e6f6c711ed.diff
LOG: [clang] Make member var invalid when static initializer is invalid.
Previously we would show an error, but keep the member, and also the
CXXRrecordDecl, valid. This could lead to crashes when attempting to
access the record layout or size.
Differential Revision: https://reviews.llvm.org/D105478
Added:
clang/test/AST/ast-dump-undeduced-expr.cpp
Modified:
clang/lib/Parse/ParseDeclCXX.cpp
clang/test/SemaCXX/crash-auto-36064.cpp
clang/test/SemaCXX/cxx11-crashes.cpp
Removed:
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp
b/clang/lib/Parse/ParseDeclCXX.cpp
index ca5c013a51fe0..760600a3ea3ca 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -2989,9 +2989,11 @@ Parser::ParseCXXClassMemberDeclaration(AccessSpecifier
AS,
ExprResult Init = ParseCXXMemberInitializer(
ThisDecl, DeclaratorInfo.isDeclarationOfFunction(), EqualLoc);
- if (Init.isInvalid())
+ if (Init.isInvalid()) {
+if (ThisDecl)
+ Actions.ActOnUninitializedDecl(ThisDecl);
SkipUntil(tok::comma, StopAtSemi | StopBeforeMatch);
- else if (ThisDecl)
+ } else if (ThisDecl)
Actions.AddInitializerToDecl(ThisDecl, Init.get(),
EqualLoc.isInvalid());
} else if (ThisDecl && DS.getStorageClassSpec() == DeclSpec::SCS_static)
// No initializer.
diff --git a/clang/test/AST/ast-dump-undeduced-expr.cpp
b/clang/test/AST/ast-dump-undeduced-expr.cpp
new file mode 100644
index 0..d404db9285974
--- /dev/null
+++ b/clang/test/AST/ast-dump-undeduced-expr.cpp
@@ -0,0 +1,7 @@
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -ast-dump %s | FileCheck
%s
+
+struct Foo {
+ static constexpr auto Bar = ;
+};
+
+// CHECK: -VarDecl {{.*}} invalid Bar 'const auto' static constexpr
diff --git a/clang/test/SemaCXX/crash-auto-36064.cpp
b/clang/test/SemaCXX/crash-auto-36064.cpp
index 5678cd8b730b8..a34e5eea51cb0 100644
--- a/clang/test/SemaCXX/crash-auto-36064.cpp
+++ b/clang/test/SemaCXX/crash-auto-36064.cpp
@@ -1,8 +1,9 @@
// RUN: %clang_cc1 -fsyntax-only -std=c++11 %s -verify
-template // expected-error{{new expression for
type 'auto' requires a constructor argument}}
+template
struct b;
struct d {
static auto c = ; // expected-error{{expected expression}}
+ // expected-error@-1 {{declaration of
variable 'c' with deduced type 'auto' requires an initializer}}
+
decltype(b); // expected-error{{expected '(' for
function-style cast or type construction}}
- // expected-note@-1{{while substituting prior
template arguments into non-type template parameter [with A = auto]}}
};
diff --git a/clang/test/SemaCXX/cxx11-crashes.cpp
b/clang/test/SemaCXX/cxx11-crashes.cpp
index d60782df35f45..e63a9be956942 100644
--- a/clang/test/SemaCXX/cxx11-crashes.cpp
+++ b/clang/test/SemaCXX/cxx11-crashes.cpp
@@ -104,3 +104,22 @@ namespace pr29091 {
bool baz() { return __has_nothrow_constructor(B); }
bool qux() { return __has_nothrow_copy(B); }
}
+
+namespace undeduced_field {
+template
+struct Foo {
+ typedef T type;
+};
+
+struct Bar {
+ Bar();
+ // The missing expression makes A undeduced.
+ static constexpr auto A = ; // expected-error {{expected expression}}
+ // expected-error@-1 {{declaration of variable
'A' with deduced type 'const auto' requires an initializer}}
+
+ Foo::type B; // The type of B is also undeduced (wrapped in
Elaborated).
+};
+
+// This used to crash when trying to get the layout of B.
+Bar x;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 49eba8b - [clang] Do not crash when ArgTy is null in CheckArgAlignment
Author: Adam Czachorowski
Date: 2021-06-10T16:54:15+02:00
New Revision: 49eba8bf1780684f1173a455b909ce37008eaa09
URL:
https://github.com/llvm/llvm-project/commit/49eba8bf1780684f1173a455b909ce37008eaa09
DIFF:
https://github.com/llvm/llvm-project/commit/49eba8bf1780684f1173a455b909ce37008eaa09.diff
LOG: [clang] Do not crash when ArgTy is null in CheckArgAlignment
This can happen around RecoveryExpr.
Differential Revision: https://reviews.llvm.org/D103825
Added:
Modified:
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaCXX/recovery-expr-type.cpp
Removed:
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 99110c6b5d8dd..86c888548077e 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -4571,8 +4571,9 @@ void Sema::CheckArgAlignment(SourceLocation Loc,
NamedDecl *FDecl,
// Find expected alignment, and the actual alignment of the passed object.
// getTypeAlignInChars requires complete types
- if (ParamTy->isIncompleteType() || ArgTy->isIncompleteType() ||
- ParamTy->isUndeducedType() || ArgTy->isUndeducedType())
+ if (ArgTy.isNull() || ParamTy->isIncompleteType() ||
+ ArgTy->isIncompleteType() || ParamTy->isUndeducedType() ||
+ ArgTy->isUndeducedType())
return;
CharUnits ParamAlign = Context.getTypeAlignInChars(ParamTy);
@@ -4654,6 +4655,9 @@ void Sema::checkCall(NamedDecl *FDecl, const
FunctionProtoType *Proto,
for (unsigned ArgIdx = 0; ArgIdx < N; ++ArgIdx) {
// Args[ArgIdx] can be null in malformed code.
if (const Expr *Arg = Args[ArgIdx]) {
+if (Arg->containsErrors())
+ continue;
+
QualType ParamTy = Proto->getParamType(ArgIdx);
QualType ArgTy = Arg->getType();
CheckArgAlignment(Arg->getExprLoc(), FDecl, std::to_string(ArgIdx + 1),
diff --git a/clang/test/SemaCXX/recovery-expr-type.cpp
b/clang/test/SemaCXX/recovery-expr-type.cpp
index 8bdf83e9512fd..d3ac772db0089 100644
--- a/clang/test/SemaCXX/recovery-expr-type.cpp
+++ b/clang/test/SemaCXX/recovery-expr-type.cpp
@@ -136,3 +136,9 @@ void baz() {
bar(S(123)); // expected-error {{no matching conversion}}
}
} // namespace test11
+
+namespace test12 {
+// Verify we do not crash.
+void fun(int *foo = no_such_function()); // expected-error {{undeclared
identifier}}
+void baz() { fun(); }
+} // namespace test12
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] a959374 - [clang] Make CXXDefaultArgExpr inherit dependence from the inner Expr
Author: Adam Czachorowski
Date: 2021-06-10T14:51:08+02:00
New Revision: a95937452f237fad10e6b7e43154c17c6b8476c4
URL:
https://github.com/llvm/llvm-project/commit/a95937452f237fad10e6b7e43154c17c6b8476c4
DIFF:
https://github.com/llvm/llvm-project/commit/a95937452f237fad10e6b7e43154c17c6b8476c4.diff
LOG: [clang] Make CXXDefaultArgExpr inherit dependence from the inner Expr
Before this change, CXXDefaultArgExpr would always have
ExprDependence::None. This can lead to issues when, for example, the
inner expression is RecoveryExpr and yet containsErrors() on the default
expression is false.
Differential Revision: https://reviews.llvm.org/D103982
Added:
clang/test/AST/ast-dump-default-arg-dep.cpp
Modified:
clang/include/clang/AST/ComputeDependence.h
clang/include/clang/AST/ExprCXX.h
clang/lib/AST/ComputeDependence.cpp
Removed:
diff --git a/clang/include/clang/AST/ComputeDependence.h
b/clang/include/clang/AST/ComputeDependence.h
index 7dde42ee71ba0..8db09e6b57d0f 100644
--- a/clang/include/clang/AST/ComputeDependence.h
+++ b/clang/include/clang/AST/ComputeDependence.h
@@ -71,6 +71,7 @@ class OverloadExpr;
class DependentScopeDeclRefExpr;
class CXXConstructExpr;
class CXXDefaultInitExpr;
+class CXXDefaultArgExpr;
class LambdaExpr;
class CXXUnresolvedConstructExpr;
class CXXDependentScopeMemberExpr;
@@ -156,6 +157,7 @@ ExprDependence computeDependence(OverloadExpr *E, bool
KnownDependent,
ExprDependence computeDependence(DependentScopeDeclRefExpr *E);
ExprDependence computeDependence(CXXConstructExpr *E);
ExprDependence computeDependence(CXXDefaultInitExpr *E);
+ExprDependence computeDependence(CXXDefaultArgExpr *E);
ExprDependence computeDependence(LambdaExpr *E,
bool ContainsUnexpandedParameterPack);
ExprDependence computeDependence(CXXUnresolvedConstructExpr *E);
diff --git a/clang/include/clang/AST/ExprCXX.h
b/clang/include/clang/AST/ExprCXX.h
index c46f9bd7f4db0..2626f9b925962 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -1257,7 +1257,7 @@ class CXXDefaultArgExpr final : public Expr {
Param->getDefaultArg()->getObjectKind()),
Param(Param), UsedContext(UsedContext) {
CXXDefaultArgExprBits.Loc = Loc;
-setDependence(ExprDependence::None);
+setDependence(computeDependence(this));
}
public:
diff --git a/clang/lib/AST/ComputeDependence.cpp
b/clang/lib/AST/ComputeDependence.cpp
index 1a5d2f7075fb5..5648cf2103d64 100644
--- a/clang/lib/AST/ComputeDependence.cpp
+++ b/clang/lib/AST/ComputeDependence.cpp
@@ -748,6 +748,10 @@ ExprDependence clang::computeDependence(CXXDefaultInitExpr
*E) {
return E->getExpr()->getDependence();
}
+ExprDependence clang::computeDependence(CXXDefaultArgExpr *E) {
+ return E->getExpr()->getDependence();
+}
+
ExprDependence clang::computeDependence(LambdaExpr *E,
bool ContainsUnexpandedParameterPack) {
auto D = toExprDependence(E->getType()->getDependence());
diff --git a/clang/test/AST/ast-dump-default-arg-dep.cpp
b/clang/test/AST/ast-dump-default-arg-dep.cpp
new file mode 100644
index 0..a804ac120fca4
--- /dev/null
+++ b/clang/test/AST/ast-dump-default-arg-dep.cpp
@@ -0,0 +1,10 @@
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump
-frecovery-ast %s | FileCheck %s
+
+// CXXDefaultArgExpr should inherit dependence from the inner Expr, in this
case
+// RecoveryExpr.
+void fun(int arg = foo());
+
+void test() {
+ fun();
+}
+// CHECK: -CXXDefaultArgExpr 0x{{[^ ]*}} <> ''
contains-errors lvalue
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 721476e - [clang] Fix a crash during code completion
Author: Adam Czachorowski
Date: 2021-06-07T13:29:58+02:00
New Revision: 721476e6b2119a93033903109b54f429b6e8c91b
URL:
https://github.com/llvm/llvm-project/commit/721476e6b2119a93033903109b54f429b6e8c91b
DIFF:
https://github.com/llvm/llvm-project/commit/721476e6b2119a93033903109b54f429b6e8c91b.diff
LOG: [clang] Fix a crash during code completion
During code completion, lookupInDeclContext() calls
CodeCompletionDeclConsumer::FoundDecl(),which can mutate StoredDeclsMap,
over which lookupInDeclContext() iterates. This can lead to invalidation
of iterators and an assert()-crash.
Example code where this happens:
#include
int main() {
std::list;
std::^
}
with code completion on ^ with -std=c++20.
I do not have a repro case that does not need standard library.
This fix stores pointers to NamedDecls in a temporary vector, then
visits them outside of the main loop, when StoredDeclsMap iterators are
gone.
Differential Revision: https://reviews.llvm.org/D103472
Added:
Modified:
clang/lib/Sema/SemaLookup.cpp
Removed:
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index db6a01543d76..4b3d7de04bf7 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -3835,6 +3835,7 @@ class LookupVisibleHelper {
if (CXXRecordDecl *Class = dyn_cast(Ctx))
Result.getSema().ForceDeclarationOfImplicitMembers(Class);
+llvm::SmallVector DeclsToVisit;
// We sometimes skip loading namespace-level results (they tend to be
huge).
bool Load = LoadExternal ||
!(isa(Ctx) || isa(Ctx));
@@ -3844,12 +3845,21 @@ class LookupVisibleHelper {
: Ctx->noload_lookups(/*PreserveInternalState=*/false)) {
for (auto *D : R) {
if (auto *ND = Result.getAcceptableDecl(D)) {
- Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
- Visited.add(ND);
+ // Rather than visit immediatelly, we put ND into a vector and visit
+ // all decls, in order, outside of this loop. The reason is that
+ // Consumer.FoundDecl() may invalidate the iterators used in the two
+ // loops above.
+ DeclsToVisit.push_back(ND);
}
}
}
+for (auto *ND : DeclsToVisit) {
+ Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
+ Visited.add(ND);
+}
+DeclsToVisit.clear();
+
// Traverse using directives for qualified name lookup.
if (QualifiedNameLookup) {
ShadowContextRAII Shadow(Visited);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] eba3ee0 - [clangd] Run code completion on each token coverd by --check-lines
Author: Adam Czachorowski
Date: 2021-06-04T17:51:42+02:00
New Revision: eba3ee04d450230f7ac1f88b1abd7b09c600c82d
URL:
https://github.com/llvm/llvm-project/commit/eba3ee04d450230f7ac1f88b1abd7b09c600c82d
DIFF:
https://github.com/llvm/llvm-project/commit/eba3ee04d450230f7ac1f88b1abd7b09c600c82d.diff
LOG: [clangd] Run code completion on each token coverd by --check-lines
In --check mode we do not run code completion because it is too slow,
especially on larger files. With the introducation of --check-lines we
can narrow down the scope and thus we can afford to do code completion.
We vlog() the top completion result, but that's not really the point.
The most value will come from being able to reproduce crashes that occur
during code completion and require preamble build or index (and thus are
more difficult to reproduce with -code-complete-at).
Differential Revision: https://reviews.llvm.org/D103538
Added:
Modified:
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp
Removed:
diff --git a/clang-tools-extra/clangd/tool/Check.cpp
b/clang-tools-extra/clangd/tool/Check.cpp
index 89487bd8607f1..88eb945d20d43 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -193,10 +193,15 @@ class Checker {
// Run AST-based features at each token in the file.
void testLocationFeatures(
- llvm::function_ref ShouldCheckLine) {
+ llvm::function_ref ShouldCheckLine,
+ const bool EnableCodeCompletion) {
log("Testing features at each token (may be slow in large files)");
auto &SM = AST->getSourceManager();
auto SpelledTokens = AST->getTokens().spelledTokens(SM.getMainFileID());
+
+CodeCompleteOptions CCOpts = Opts.CodeComplete;
+CCOpts.Index = &Index;
+
for (const auto &Tok : SpelledTokens) {
unsigned Start = AST->getSourceManager().getFileOffset(Tok.location());
unsigned End = Start + Tok.length();
@@ -233,8 +238,12 @@ class Checker {
auto Hover = getHover(*AST, Pos, Style, &Index);
vlog("hover: {0}", Hover.hasValue());
- // FIXME: it'd be nice to include code completion, but it's too slow.
- // Maybe in combination with a line restriction?
+ if (EnableCodeCompletion) {
+Position EndPos = offsetToPosition(Inputs.Contents, End);
+auto CC = codeComplete(File, EndPos, Preamble.get(), Inputs, CCOpts);
+vlog("code completion: {0}",
+ CC.Completions.empty() ? "" : CC.Completions[0].Name);
+ }
}
}
};
@@ -243,7 +252,8 @@ class Checker {
bool check(llvm::StringRef File,
llvm::function_ref ShouldCheckLine,
- const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts) {
+ const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
+ bool EnableCodeCompletion) {
llvm::SmallString<0> FakeFile;
llvm::Optional Contents;
if (File.empty()) {
@@ -267,7 +277,7 @@ bool check(llvm::StringRef File,
if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
!C.buildAST())
return false;
- C.testLocationFeatures(ShouldCheckLine);
+ C.testLocationFeatures(ShouldCheckLine, EnableCodeCompletion);
log("All checks completed, {0} errors", C.ErrCount);
return C.ErrCount == 0;
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index f0aa89b8091fb..0f9e5c89e42c8 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -62,7 +62,8 @@ namespace clangd {
// Implemented in Check.cpp.
bool check(const llvm::StringRef File,
llvm::function_ref ShouldCheckLine,
- const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts);
+ const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
+ bool EnableCodeCompletion);
namespace {
@@ -929,7 +930,11 @@ clangd accepts flags on the commandline, and in the
CLANGD_FLAGS environment var
uint32_t Line = Pos.line + 1; // Position::line is 0-based.
return Line >= Begin && Line <= End;
};
-return check(Path, ShouldCheckLine, TFS, Opts)
+// For now code completion is enabled any time the range is limited via
+// --check-lines. If it turns out to be to slow, we can introduce a
+// dedicated flag for that instead.
+return check(Path, ShouldCheckLine, TFS, Opts,
+ /*EnableCodeCompletion=*/!CheckFileLines.empty())
? 0
: static_cast(ErrorResultCode::CheckFailed);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] fbfcfdb - [clang] Fix assert() crash when checking undeduced arg alignment
Author: Adam Czachorowski
Date: 2021-04-30T16:24:33+02:00
New Revision: fbfcfdbf6828b8d36f4ec0ff5f4eac11fb1411a5
URL:
https://github.com/llvm/llvm-project/commit/fbfcfdbf6828b8d36f4ec0ff5f4eac11fb1411a5
DIFF:
https://github.com/llvm/llvm-project/commit/fbfcfdbf6828b8d36f4ec0ff5f4eac11fb1411a5.diff
LOG: [clang] Fix assert() crash when checking undeduced arg alignment
There already was a check for undeduced and incomplete types, but it
failed to trigger when outer type (SubstTemplateTypeParm in test) looked
fine, but inner type was not.
Differential Revision: https://reviews.llvm.org/D100667
Added:
Modified:
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/recovery-expr-type.cpp
Removed:
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 7b6e0541aa4d7..38cda657bac8b 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -4540,8 +4540,7 @@ void Sema::CheckArgAlignment(SourceLocation Loc,
NamedDecl *FDecl,
// Find expected alignment, and the actual alignment of the passed object.
// getTypeAlignInChars requires complete types
- if (ParamTy->isIncompleteType() || ArgTy->isIncompleteType() ||
- ParamTy->isUndeducedType() || ArgTy->isUndeducedType())
+ if (ParamTy->isIncompleteType() || ArgTy->isIncompleteType())
return;
CharUnits ParamAlign = Context.getTypeAlignInChars(ParamTy);
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index e0f46a0ead40a..73d7675eb189f 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -19662,8 +19662,10 @@ ExprResult Sema::CreateRecoveryExpr(SourceLocation
Begin, SourceLocation End,
if (isSFINAEContext())
return ExprError();
- if (T.isNull() || !Context.getLangOpts().RecoveryASTType)
+ if (T.isNull() || T->isUndeducedType() ||
+ !Context.getLangOpts().RecoveryASTType)
// We don't know the concrete type, fallback to dependent type.
T = Context.DependentTy;
+
return RecoveryExpr::Create(Context, T, Begin, End, SubExprs);
}
diff --git a/clang/test/SemaCXX/recovery-expr-type.cpp
b/clang/test/SemaCXX/recovery-expr-type.cpp
index ed18b7f262cdc..8bdf83e9512fd 100644
--- a/clang/test/SemaCXX/recovery-expr-type.cpp
+++ b/clang/test/SemaCXX/recovery-expr-type.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -frecovery-ast
-frecovery-ast-type -o - %s -fsyntax-only -verify
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -frecovery-ast
-frecovery-ast-type -o - %s -std=gnu++17 -fsyntax-only -verify
namespace test0 {
struct Indestructible {
@@ -26,7 +26,7 @@ namespace test2 {
void foo(); // expected-note 3{{requires 0 arguments}}
void func() {
// verify that "field has incomplete type" diagnostic is suppressed.
- typeof(foo(42)) var; // expected-error {{no matching function}}
+ typeof(foo(42)) var; // expected-error {{no matching function}} \
// FIXME: suppress the "cannot initialize a variable" diagnostic.
int a = foo(1); // expected-error {{no matching function}} \
@@ -116,3 +116,23 @@ int f(); // expected-note {{candidate}}
template const int k = f(T()); // expected-error {{no matching
function}}
static_assert(k == 1, ""); // expected-note {{instantiation of}}
}
+
+namespace test11 {
+// Verify we do not assert()-fail here.
+template void foo(T &t);
+template
+void bar(T t) {
+ foo(t);
+}
+
+template
+struct S { // expected-note {{candidate}}
+ S(T t); // expected-note {{candidate}}
+ ~S();
+};
+template S(T t) -> S;
+
+void baz() {
+ bar(S(123)); // expected-error {{no matching conversion}}
+}
+} // namespace test11
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] ddfbdbf - [clang] Do not crash on template specialization following a fatal error
Author: Adam Czachorowski
Date: 2021-04-23T13:34:05+02:00
New Revision: ddfbdbfefae04ea71391a38ed5e9cb6975f6630b
URL:
https://github.com/llvm/llvm-project/commit/ddfbdbfefae04ea71391a38ed5e9cb6975f6630b
DIFF:
https://github.com/llvm/llvm-project/commit/ddfbdbfefae04ea71391a38ed5e9cb6975f6630b.diff
LOG: [clang] Do not crash on template specialization following a fatal error
There was a missing isInvalid() check leading to an attempt to
instantiate template with an empty instantiation stack.
Differential Revision: https://reviews.llvm.org/D100675
Added:
clang/test/SemaCXX/template-specialization-fatal.cpp
Modified:
clang/lib/Sema/SemaTemplateDeduction.cpp
Removed:
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index d755696c698c6..7d548e4f8dee1 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -5466,6 +5466,9 @@ static bool isAtLeastAsSpecializedAs(Sema &S, QualType
T1, QualType T2,
Deduced.end());
Sema::InstantiatingTemplate Inst(S, Info.getLocation(), P2, DeducedArgs,
Info);
+ if (Inst.isInvalid())
+return false;
+
auto *TST1 = T1->castAs();
bool AtLeastAsSpecialized;
S.runWithSufficientStackSpace(Info.getLocation(), [&] {
diff --git a/clang/test/SemaCXX/template-specialization-fatal.cpp
b/clang/test/SemaCXX/template-specialization-fatal.cpp
new file mode 100644
index 0..2191e54c8f162
--- /dev/null
+++ b/clang/test/SemaCXX/template-specialization-fatal.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+// Verify clang doesn't assert()-fail on template specialization happening
after
+// fatal error.
+
+#include "not_found.h" // expected-error {{'not_found.h' file not found}}
+
+template
+struct foo {};
+
+template
+struct foo(0)(static_cast(0)()))> {};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 3b4936b - [clangd] Add --check-lines to restrict --check to specific lines
Author: Adam Czachorowski
Date: 2021-04-09T13:47:20+02:00
New Revision: 3b4936ba290594cda4e53169958fe11c83119657
URL:
https://github.com/llvm/llvm-project/commit/3b4936ba290594cda4e53169958fe11c83119657
DIFF:
https://github.com/llvm/llvm-project/commit/3b4936ba290594cda4e53169958fe11c83119657.diff
LOG: [clangd] Add --check-lines to restrict --check to specific lines
This will allow us to add code completion, which is too expensive at
every token, to --check too.
Differential Revision: https://reviews.llvm.org/D98970
Added:
clang-tools-extra/clangd/test/check-lines.test
Modified:
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp
Removed:
diff --git a/clang-tools-extra/clangd/test/check-lines.test
b/clang-tools-extra/clangd/test/check-lines.test
new file mode 100644
index ..bfd30dd6104f
--- /dev/null
+++ b/clang-tools-extra/clangd/test/check-lines.test
@@ -0,0 +1,15 @@
+// RUN: cp %s %t.cpp
+// RUN: not clangd -check=%t.cpp -check-lines=6-14 2>&1 | FileCheck
-strict-whitespace %s
+// RUN: not clangd -check=%t.cpp -check-lines=14 2>&1 | FileCheck
-strict-whitespace %s
+
+// CHECK: Testing on source file {{.*}}check-lines.test
+// CHECK: internal (cc1) args are: -cc1
+// CHECK: Building preamble...
+// CHECK: Building AST...
+// CHECK: Testing features at each token
+// CHECK: tweak: ExpandAutoType ==> FAIL
+// CHECK: All checks completed, 1 errors
+
+void fun();
+auto x = fun; // This line is tested
+auto y = fun; // This line is not tested
diff --git a/clang-tools-extra/clangd/tool/Check.cpp
b/clang-tools-extra/clangd/tool/Check.cpp
index 20b86daff8af..0138ae3eb13b 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -192,14 +192,19 @@ class Checker {
}
// Run AST-based features at each token in the file.
- void testLocationFeatures() {
+ void testLocationFeatures(
+ llvm::function_ref ShouldCheckLine) {
log("Testing features at each token (may be slow in large files)");
-auto SpelledTokens =
-
AST->getTokens().spelledTokens(AST->getSourceManager().getMainFileID());
+auto &SM = AST->getSourceManager();
+auto SpelledTokens = AST->getTokens().spelledTokens(SM.getMainFileID());
for (const auto &Tok : SpelledTokens) {
unsigned Start = AST->getSourceManager().getFileOffset(Tok.location());
unsigned End = Start + Tok.length();
Position Pos = offsetToPosition(Inputs.Contents, Start);
+
+ if (!ShouldCheckLine(Pos))
+continue;
+
// FIXME: dumping the tokens may leak sensitive code into bug reports.
// Add an option to turn this off, once we decide how options work.
vlog(" {0} {1}", Pos, Tok.text(AST->getSourceManager()));
@@ -229,8 +234,9 @@ class Checker {
} // namespace
-bool check(llvm::StringRef File, const ThreadsafeFS &TFS,
- const ClangdLSPServer::Options &Opts) {
+bool check(llvm::StringRef File,
+ llvm::function_ref ShouldCheckLine,
+ const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts) {
llvm::SmallString<0> FakeFile;
llvm::Optional Contents;
if (File.empty()) {
@@ -254,7 +260,7 @@ bool check(llvm::StringRef File, const ThreadsafeFS &TFS,
if (!C.buildCommand(TFS) || !C.buildInvocation(TFS, Contents) ||
!C.buildAST())
return false;
- C.testLocationFeatures();
+ C.testLocationFeatures(ShouldCheckLine);
log("All checks completed, {0} errors", C.ErrCount);
return C.ErrCount == 0;
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index c48821cba2c7..6dd13a503887 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -60,8 +60,9 @@ namespace clang {
namespace clangd {
// Implemented in Check.cpp.
-bool check(const llvm::StringRef File, const ThreadsafeFS &TFS,
- const ClangdLSPServer::Options &Opts);
+bool check(const llvm::StringRef File,
+ llvm::function_ref ShouldCheckLine,
+ const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts);
namespace {
@@ -347,6 +348,17 @@ opt CheckFile{
ValueOptional,
};
+opt CheckFileLines{
+"check-lines",
+cat(Misc),
+desc("If specified, limits the range of tokens in -check file on which "
+ "various features are tested. Example --check-lines=3-7 restricts "
+ "testing to lines 3 to 7 (inclusive) or --check-lines=5 to restrict "
+ "to one line. Default is testing entire file."),
+init(""),
+ValueOptional,
+};
+
enum PCHStorageFlag { Disk, Memory };
opt PCHStorage{
"pch-storage",
@@ -883,10 +895,33 @@ clangd accepts flags on the commandline, and in the
CLANGD_FLAGS environment var
llvm::SmallString<256> Path;
llvm::sys::fs::real_path(CheckFile, Path, /*expand_tilde=*/t
[clang] 4e1c487 - [clang] Fix crash when creating deduction guide.
Author: Adam Czachorowski
Date: 2021-03-09T16:57:56+01:00
New Revision: 4e1c487004a29ec9bc56fd47fc30336d033c57dd
URL:
https://github.com/llvm/llvm-project/commit/4e1c487004a29ec9bc56fd47fc30336d033c57dd
DIFF:
https://github.com/llvm/llvm-project/commit/4e1c487004a29ec9bc56fd47fc30336d033c57dd.diff
LOG: [clang] Fix crash when creating deduction guide.
We used to trigger assertion when transforming c-tor with unparsed
default argument. Now we ignore such constructors for this purpose.
Differential Revision: https://reviews.llvm.org/D97965
Added:
Modified:
clang/lib/Sema/SemaTemplate.cpp
clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
Removed:
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 12880b95b9c6..7a65c2a6fe2c 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -2494,6 +2494,12 @@ void Sema::DeclareImplicitDeductionGuides(TemplateDecl
*Template,
if (!CD || (!FTD && CD->isFunctionTemplateSpecialization()))
continue;
+// Cannot make a deduction guide when unparsed arguments are present.
+if (std::any_of(CD->param_begin(), CD->param_end(), [](ParmVarDecl *P) {
+ return !P || P->hasUnparsedDefaultArg();
+}))
+ continue;
+
Transform.transformConstructor(FTD, CD);
AddedAny = true;
}
diff --git a/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
b/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
index 305cc3e976b0..161944f9e64f 100644
--- a/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ b/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -416,6 +416,17 @@ B b(0, {});
}
+namespace no_crash_on_default_arg {
+class A {
+ template class B {
+B(int c = 1);
+ };
+ // This used to crash due to unparsed default arg above. The diagnostic could
+ // be improved, but the point of this test is to simply check we do not
crash.
+ B(); // expected-error {{deduction guide declaration without trailing return
type}}
+};
+} // namespace no_crash_on_default_arg
+
#pragma clang diagnostic push
#pragma clang diagnostic warning "-Wctad-maybe-unsupported"
namespace test_implicit_ctad_warning {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 0005438 - [clangd] Fix a crash when indexing invalid ObjC method declaration
Author: Adam Czachorowski
Date: 2021-01-25T15:43:11+01:00
New Revision: 00054382b95a9d95e2df6457e7fe1fca2323d287
URL:
https://github.com/llvm/llvm-project/commit/00054382b95a9d95e2df6457e7fe1fca2323d287
DIFF:
https://github.com/llvm/llvm-project/commit/00054382b95a9d95e2df6457e7fe1fca2323d287.diff
LOG: [clangd] Fix a crash when indexing invalid ObjC method declaration
This fix will make us not crash, but ideally we would handle this case
better.
Differential Revision: https://reviews.llvm.org/D94919
Added:
Modified:
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
clang/lib/Sema/SemaCodeComplete.cpp
Removed:
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 7e36cff7afa6..924cfd03cba7 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1838,6 +1838,20 @@ TEST_F(SymbolCollectorTest, UndefOfModuleMacro) {
EXPECT_THAT(TU.headerSymbols(), Not(Contains(QName("X";
}
+TEST_F(SymbolCollectorTest, NoCrashOnObjCMethodCStyleParam) {
+ auto TU = TestTU::withCode(R"objc(
+@interface Foo
+- (void)fun:(bool)foo, bool bar;
+@end
+ )objc");
+ TU.ExtraArgs.push_back("-xobjective-c++");
+
+ TU.build();
+ // We mostly care about not crashing.
+ EXPECT_THAT(TU.headerSymbols(),
+ UnorderedElementsAre(QName("Foo"), QName("Foo::fun:")));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp
b/clang/lib/Sema/SemaCodeComplete.cpp
index d77c9e43a9bd..c2785fd60fc2 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -3529,9 +3529,11 @@ CodeCompletionString
*CodeCompletionResult::createCodeCompletionStringForDecl(
Result.AddTypedTextChunk("");
}
unsigned Idx = 0;
+// The extra Idx < Sel.getNumArgs() check is needed due to legacy C-style
+// method parameters.
for (ObjCMethodDecl::param_const_iterator P = Method->param_begin(),
PEnd = Method->param_end();
- P != PEnd; (void)++P, ++Idx) {
+ P != PEnd && Idx < Sel.getNumArgs(); (void)++P, ++Idx) {
if (Idx > 0) {
std::string Keyword;
if (Idx > StartParameter)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] d462aa5 - [clang] Fix a nullptr dereference bug on invalid code
Author: Adam Czachorowski
Date: 2021-01-25T15:02:25+01:00
New Revision: d462aa5a619ab9fdf8b024e48c19bc8820fe8781
URL:
https://github.com/llvm/llvm-project/commit/d462aa5a619ab9fdf8b024e48c19bc8820fe8781
DIFF:
https://github.com/llvm/llvm-project/commit/d462aa5a619ab9fdf8b024e48c19bc8820fe8781.diff
LOG: [clang] Fix a nullptr dereference bug on invalid code
When working with invalid code, we would try to dereference a nullptr
while deducing template arguments in some dependend code operating on a
lambda with invalid return type.
Differential Revision: https://reviews.llvm.org/D95145
Added:
clang/test/SemaCXX/subst-func-type-invalid-ret-type.cpp
Modified:
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
Removed:
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index d3d6df5e0064..dc1e0ef60cac 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -4189,6 +4189,9 @@ TemplateDeclInstantiator::SubstFunctionType(FunctionDecl
*D,
for (unsigned OldIdx = 0, NumOldParams = OldProtoLoc.getNumParams();
OldIdx != NumOldParams; ++OldIdx) {
ParmVarDecl *OldParam = OldProtoLoc.getParam(OldIdx);
+if (!OldParam)
+ return nullptr;
+
LocalInstantiationScope *Scope = SemaRef.CurrentInstantiationScope;
Optional NumArgumentsInExpansion;
diff --git a/clang/test/SemaCXX/subst-func-type-invalid-ret-type.cpp
b/clang/test/SemaCXX/subst-func-type-invalid-ret-type.cpp
new file mode 100644
index ..c783ff50
--- /dev/null
+++ b/clang/test/SemaCXX/subst-func-type-invalid-ret-type.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang -fsyntax-only -std=c++17 %s -Xclang -verify
+
+// The important part is that we do not crash.
+
+template T declval();
+
+template
+auto Call(T x) -> decltype(declval()(0)) {} // expected-note{{candidate
template ignored}}
+
+class Status {};
+
+void fun() {
+ // The Status() (instead of Status) here used to cause a crash.
+ Call([](auto x) -> Status() {}); // expected-error{{function cannot return
function type 'Status ()}}
+ // expected-error@-1{{no matching function for call to 'Call'}}
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] a6f9077 - [clang] Check for nullptr when instantiating late attrs
Author: Adam Czachorowski Date: 2021-01-19T13:43:15+01:00 New Revision: a6f9077b16da90204b296acd4f840769e83460ac URL: https://github.com/llvm/llvm-project/commit/a6f9077b16da90204b296acd4f840769e83460ac DIFF: https://github.com/llvm/llvm-project/commit/a6f9077b16da90204b296acd4f840769e83460ac.diff LOG: [clang] Check for nullptr when instantiating late attrs This was already done in SemaTemplateInstantiateDecl.cpp, but not in SemaTemplateInstantiate.cpp. Anecdotally I've seen some clangd crashes where coredumps point to this being a problem, but I cannot reproduce this so far. Differential Revision: https://reviews.llvm.org/D94933 Added: Modified: clang/lib/Sema/SemaTemplateInstantiate.cpp Removed: diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index cb74f08830c8..7679063ead71 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -2796,7 +2796,8 @@ Sema::InstantiateClass(SourceLocation PointOfInstantiation, Attr *NewAttr = instantiateTemplateAttribute(I->TmplAttr, Context, *this, TemplateArgs); -I->NewDecl->addAttr(NewAttr); +if (NewAttr) + I->NewDecl->addAttr(NewAttr); LocalInstantiationScope::deleteScopes(I->Scope, Instantiator.getStartingScope()); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 196cc96 - [clang] Allow LifetimeExtendedTemporary to have no access specifier
Author: Adam Czachorowski
Date: 2021-01-18T19:19:57+01:00
New Revision: 196cc96f9a643d1cb828f48ef15ec30d0de24df7
URL:
https://github.com/llvm/llvm-project/commit/196cc96f9a643d1cb828f48ef15ec30d0de24df7
DIFF:
https://github.com/llvm/llvm-project/commit/196cc96f9a643d1cb828f48ef15ec30d0de24df7.diff
LOG: [clang] Allow LifetimeExtendedTemporary to have no access specifier
The check only runs in debug mode during serialization, but
assert()-fail on:
struct S { const int& x = 7; };
in C++ mode.
Differential Revision: https://reviews.llvm.org/D94804
Added:
Modified:
clang/lib/AST/DeclBase.cpp
clang/test/PCH/cxx-reference.h
Removed:
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index 0656efae5489..dc59f3dd1f15 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -971,21 +971,19 @@ bool Decl::AccessDeclContextSanity() const {
// 5. it's invalid
// 6. it's a C++0x static_assert.
// 7. it's a block literal declaration
- if (isa(this) ||
- isa(this) ||
- isa(this) ||
- !getDeclContext() ||
- !isa(getDeclContext()) ||
- isInvalidDecl() ||
- isa(this) ||
- isa(this) ||
+ // 8. it's a temporary with lifetime extended due to being default value.
+ if (isa(this) || isa(this) ||
+ isa(this) || !getDeclContext() ||
+ !isa(getDeclContext()) || isInvalidDecl() ||
+ isa(this) || isa(this) ||
// FIXME: a ParmVarDecl can have ClassTemplateSpecialization
// as DeclContext (?).
isa(this) ||
// FIXME: a ClassTemplateSpecialization or CXXRecordDecl can have
// AS_none as access specifier.
isa(this) ||
- isa(this))
+ isa(this) ||
+ isa(this))
return true;
assert(Access != AS_none &&
diff --git a/clang/test/PCH/cxx-reference.h b/clang/test/PCH/cxx-reference.h
index b46a3671a325..a65d3feee072 100644
--- a/clang/test/PCH/cxx-reference.h
+++ b/clang/test/PCH/cxx-reference.h
@@ -11,3 +11,7 @@ LR &lrlr = c;
LR &&rrlr = c;
RR &lrrr = c;
RR && = 'c';
+
+struct S {
+ const int &x = 1; // LifetimeExtendedTemporary inside struct
+};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] c77c3d1 - [clangd] Set correct CWD when using compile_flags.txt
Author: Adam Czachorowski
Date: 2021-01-15T14:26:24+01:00
New Revision: c77c3d1d18cdd58989f9d35bbf6c31f5fda0a125
URL:
https://github.com/llvm/llvm-project/commit/c77c3d1d18cdd58989f9d35bbf6c31f5fda0a125
DIFF:
https://github.com/llvm/llvm-project/commit/c77c3d1d18cdd58989f9d35bbf6c31f5fda0a125.diff
LOG: [clangd] Set correct CWD when using compile_flags.txt
This fixes a bug where clangd would attempt to set CWD to the
compile_flags.txt file itself.
Differential Revision: https://reviews.llvm.org/D94699
Added:
Modified:
clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
index d983f76e227f..fde4e56ac72d 100644
--- a/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
+++ b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
@@ -271,7 +271,8 @@ parseJSON(PathRef Path, llvm::StringRef Data, std::string
&Error) {
}
static std::unique_ptr
parseFixed(PathRef Path, llvm::StringRef Data, std::string &Error) {
- return tooling::FixedCompilationDatabase::loadFromBuffer(Path, Data, Error);
+ return tooling::FixedCompilationDatabase::loadFromBuffer(
+ llvm::sys::path::parent_path(Path), Data, Error);
}
bool DirectoryBasedGlobalCompilationDatabase::DirectoryCache::load(
diff --git
a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
index 63b1b731656a..fbb85684af5f 100644
--- a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -279,6 +279,17 @@ TEST(GlobalCompilationDatabaseTest, BuildDir) {
<< "x/build/compile_flags.json only applicable to x/";
}
+TEST(GlobalCompilationDatabaseTest, CompileFlagsDirectory) {
+ MockFS FS;
+ FS.Files[testPath("x/compile_flags.txt")] = "-DFOO";
+ DirectoryBasedGlobalCompilationDatabase CDB(FS);
+ auto Commands = CDB.getCompileCommand(testPath("x/y.cpp"));
+ ASSERT_TRUE(Commands.hasValue());
+ EXPECT_THAT(Commands.getValue().CommandLine, Contains("-DFOO"));
+ // Make sure we pick the right working directory.
+ EXPECT_EQ(testPath("x"), Commands.getValue().Directory);
+}
+
TEST(GlobalCompilationDatabaseTest, NonCanonicalFilenames) {
OverlayCDB DB(nullptr);
std::vector DiscoveredFiles;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] aeaeb9e - [clangd] Make ExpandAutoType not available on template params.
Author: Adam Czachorowski
Date: 2021-01-15T14:19:05+01:00
New Revision: aeaeb9e6bdc90d9c4b839ac0e4edc6255021cced
URL:
https://github.com/llvm/llvm-project/commit/aeaeb9e6bdc90d9c4b839ac0e4edc6255021cced
DIFF:
https://github.com/llvm/llvm-project/commit/aeaeb9e6bdc90d9c4b839ac0e4edc6255021cced.diff
LOG: [clangd] Make ExpandAutoType not available on template params.
We cannot expand auto when used inside a template param (C++17 feature),
so do not offer it there.
Differential Revision: https://reviews.llvm.org/D94719
Added:
Modified:
clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
b/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
index cb87cc764bc3..3776e1c3505d 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
@@ -82,6 +82,15 @@ bool isDeducedAsLambda(const SelectionTree::Node *Node,
SourceLocation Loc) {
return false;
}
+// Returns true iff "auto" in Node is really part of the template parameter,
+// which we cannot expand.
+bool isTemplateParam(const SelectionTree::Node *Node) {
+ if (Node->Parent)
+if (Node->Parent->ASTNode.get())
+ return true;
+ return false;
+}
+
bool ExpandAutoType::prepare(const Selection& Inputs) {
CachedLocation = llvm::None;
if (auto *Node = Inputs.ASTSelection.commonAncestor()) {
@@ -90,7 +99,8 @@ bool ExpandAutoType::prepare(const Selection& Inputs) {
// Code in apply() does handle 'decltype(auto)' yet.
if (!Result.getTypePtr()->isDecltypeAuto() &&
!isStructuredBindingType(Node) &&
-!isDeducedAsLambda(Node, Result.getBeginLoc()))
+!isDeducedAsLambda(Node, Result.getBeginLoc()) &&
+!isTemplateParam(Node))
CachedLocation = Result;
}
}
diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
b/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
index 6c96ecc2332f..5554b68b31c7 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
@@ -80,6 +80,9 @@ TEST_F(ExpandAutoTypeTest, Test) {
// unknown types in a template should not be replaced
EXPECT_THAT(apply("template void x() { ^auto y = T::z(); }"),
StartsWith("fail: Could not deduce type for 'auto' type"));
+
+ ExtraArgs.push_back("-std=c++17");
+ EXPECT_UNAVAILABLE("template class Y;");
}
} // namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] a71877e - [clang] Do not crash when CXXRecordDecl has a non-CXXRecordDecl base.
Author: Adam Czachorowski
Date: 2021-01-14T21:20:06+01:00
New Revision: a71877edfbb7094584f6d20d93f6091e7d374024
URL:
https://github.com/llvm/llvm-project/commit/a71877edfbb7094584f6d20d93f6091e7d374024
DIFF:
https://github.com/llvm/llvm-project/commit/a71877edfbb7094584f6d20d93f6091e7d374024.diff
LOG: [clang] Do not crash when CXXRecordDecl has a non-CXXRecordDecl base.
This can happen on some invalid code, like the included test case.
Differential Revision: https://reviews.llvm.org/D94704
Added:
Modified:
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/SemaTemplate/temp_class_spec.cpp
Removed:
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 27679ac6f8d3..8bfaa46162bc 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -5520,8 +5520,9 @@
Sema::MarkBaseAndMemberDestructorsReferenced(SourceLocation Location,
// Bases.
for (const auto &Base : ClassDecl->bases()) {
-// Bases are always records in a well-formed non-dependent class.
const RecordType *RT = Base.getType()->getAs();
+if (!RT)
+ continue;
// Remember direct virtual bases.
if (Base.isVirtual()) {
diff --git a/clang/test/SemaTemplate/temp_class_spec.cpp
b/clang/test/SemaTemplate/temp_class_spec.cpp
index 8a07fd7292c2..f92c52e9624e 100644
--- a/clang/test/SemaTemplate/temp_class_spec.cpp
+++ b/clang/test/SemaTemplate/temp_class_spec.cpp
@@ -361,3 +361,17 @@ namespace PR6181 {
};
}
+
+// Check that we do not crash on invalid code that leads to invalid base.
+namespace {
+template
+class Foo {};
+
+template
+class Bar;
+
+template
+class Bar<0> : public Foo { // expected-error{{partial specialization of
'Bar' does not use any of its template parameters}}
+ Bar() : Foo() {}
+};
+} // namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 2e1bb79 - [clangd] Add missing "override" to fix the build.
Author: Adam Czachorowski
Date: 2021-01-08T17:24:47+01:00
New Revision: 2e1bb7940a4ddc847cebd25092d10f40866a7fad
URL:
https://github.com/llvm/llvm-project/commit/2e1bb7940a4ddc847cebd25092d10f40866a7fad
DIFF:
https://github.com/llvm/llvm-project/commit/2e1bb7940a4ddc847cebd25092d10f40866a7fad.diff
LOG: [clangd] Add missing "override" to fix the build.
Follow-up to d4af86581e80ef0f7a6f4a4fff1c97260a726e71
Differential Revision: https://reviews.llvm.org/D94314
Added:
Modified:
clang-tools-extra/clangd/AST.cpp
Removed:
diff --git a/clang-tools-extra/clangd/AST.cpp
b/clang-tools-extra/clangd/AST.cpp
index 16298f3e0326..8af4cbb19a3d 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -310,7 +310,7 @@ std::string printType(const QualType QT, const DeclContext
&CurContext) {
public:
PrintCB(const DeclContext *CurContext) : CurContext(CurContext) {}
virtual ~PrintCB() {}
-virtual bool isScopeVisible(const DeclContext *DC) const {
+virtual bool isScopeVisible(const DeclContext *DC) const override {
return DC->Encloses(CurContext);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] d4af865 - [clangd] Fix type printing in the presence of qualifiers
Author: Adam Czachorowski
Date: 2021-01-08T17:00:39+01:00
New Revision: d4af86581e80ef0f7a6f4a4fff1c97260a726e71
URL:
https://github.com/llvm/llvm-project/commit/d4af86581e80ef0f7a6f4a4fff1c97260a726e71
DIFF:
https://github.com/llvm/llvm-project/commit/d4af86581e80ef0f7a6f4a4fff1c97260a726e71.diff
LOG: [clangd] Fix type printing in the presence of qualifiers
When printing QualType with qualifiers like "const", or pointing to an
elaborated type, we would print garbage like:
std::const std::vector&
with the initial std:: being calculated correctly, but inserted in the
wrong place and the second std:: not removed (due to elaborated type).
This affected, among others, ExtractFunction and ExpandAuto tweaks.
This change introduces a new callback to PrintingPolicy, which allows us
to influence the printing of namespace qualifiers. In the future, the
same callback can be used to improve handling of "using namespace"
directives as well.
Fixes:
https://github.com/clangd/clangd/issues/640 (ExtractFunction)
https://github.com/clangd/clangd/issues/264 (ExpandAuto)
First point of https://github.com/clangd/clangd/issues/524
Differential Revision: https://reviews.llvm.org/D94259
Added:
Modified:
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
clang/include/clang/AST/PrettyPrinter.h
clang/lib/AST/TypePrinter.cpp
Removed:
diff --git a/clang-tools-extra/clangd/AST.cpp
b/clang-tools-extra/clangd/AST.cpp
index 39ab48843b28..16298f3e0326 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -299,19 +299,27 @@ SymbolID getSymbolID(const llvm::StringRef MacroName,
const MacroInfo *MI,
return SymbolID(USR);
}
-// FIXME: This should be handled while printing underlying decls instead.
std::string printType(const QualType QT, const DeclContext &CurContext) {
std::string Result;
llvm::raw_string_ostream OS(Result);
- auto Decls =
- explicitReferenceTargets(DynTypedNode::create(QT), DeclRelation::Alias);
- if (!Decls.empty())
-OS << getQualification(CurContext.getParentASTContext(), &CurContext,
- Decls.front(),
-
/*VisibleNamespaces=*/llvm::ArrayRef{});
PrintingPolicy PP(CurContext.getParentASTContext().getPrintingPolicy());
- PP.SuppressScope = true;
PP.SuppressTagKeyword = true;
+ PP.SuppressUnwrittenScope = true;
+
+ class PrintCB : public PrintingCallbacks {
+ public:
+PrintCB(const DeclContext *CurContext) : CurContext(CurContext) {}
+virtual ~PrintCB() {}
+virtual bool isScopeVisible(const DeclContext *DC) const {
+ return DC->Encloses(CurContext);
+}
+
+ private:
+const DeclContext *CurContext;
+ };
+ PrintCB PCB(&CurContext);
+ PP.Callbacks = &PCB;
+
QT.print(OS, PP);
return OS.str();
}
diff --git a/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
b/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
index ba783e551e84..6c96ecc2332f 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
@@ -65,6 +65,11 @@ TEST_F(ExpandAutoTypeTest, Test) {
EXPECT_EQ(apply(R"cpp(au^to x = "test";)cpp"),
R"cpp(const char * x = "test";)cpp");
+ EXPECT_EQ(apply("ns::Class * foo() { au^to c = foo(); }"),
+"ns::Class * foo() { ns::Class * c = foo(); }");
+ EXPECT_EQ(apply("void ns::Func() { au^to x = new ns::Class::Nested{}; }"),
+"void ns::Func() { Class::Nested * x = new ns::Class::Nested{};
}");
+
EXPECT_UNAVAILABLE("dec^ltype(au^to) x = 10;");
// expanding types in structured bindings is syntactically invalid.
EXPECT_UNAVAILABLE("const ^auto &[x,y] = (int[]){1,2};");
diff --git
a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
index 2033b479896b..94cc4b0a0d84 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/ExtractFunctionTests.cpp
@@ -97,6 +97,22 @@ void f(const int c) {
})cpp";
EXPECT_EQ(apply(ConstCheckInput), ConstCheckOutput);
+ // Check const qualifier with namespace
+ std::string ConstNamespaceCheckInput = R"cpp(
+namespace X { struct Y { int z; }; }
+int f(const X::Y &y) {
+ [[return y.z + y.z;]]
+})cpp";
+ std::string ConstNamespaceCheckOutput = R"cpp(
+namespace X { struct Y { int z; }; }
+int extracted(const X::Y &y) {
+return y.z + y.z;
+}
+int f(const X::Y &y) {
+ return extracted(y);
+})cpp";
+ EXPECT_EQ(apply(ConstNamespaceCheckInput), ConstNamespaceCheckOutput);
+
// Don't extract when we need to make a function as a parameter.
EXPECT_THAT(apply("void f() {
[clang-tools-extra] 0999408 - [clangd] Add error handling (elog) in code completion.
Author: Adam Czachorowski
Date: 2020-12-28T15:22:54+01:00
New Revision: 0999408aea79dd69f182cfcb618006f6cf2b6d4e
URL:
https://github.com/llvm/llvm-project/commit/0999408aea79dd69f182cfcb618006f6cf2b6d4e
DIFF:
https://github.com/llvm/llvm-project/commit/0999408aea79dd69f182cfcb618006f6cf2b6d4e.diff
LOG: [clangd] Add error handling (elog) in code completion.
Differential Revision: https://reviews.llvm.org/D93220
Added:
Modified:
clang-tools-extra/clangd/CodeComplete.cpp
Removed:
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp
b/clang-tools-extra/clangd/CodeComplete.cpp
index cc6b5dbc9904..ebdbd56c286a 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -182,12 +182,18 @@ struct CompletionCandidate {
// strings (literal or URI) mapping to the same file. We still want to
// bundle those, so we must resolve the header to be included here.
std::string HeaderForHash;
-if (Inserter)
- if (auto Header = headerToInsertIfAllowed(Opts))
-if (auto HeaderFile = toHeaderFile(*Header, FileName))
+if (Inserter) {
+ if (auto Header = headerToInsertIfAllowed(Opts)) {
+if (auto HeaderFile = toHeaderFile(*Header, FileName)) {
if (auto Spelled =
Inserter->calculateIncludePath(*HeaderFile, FileName))
HeaderForHash = *Spelled;
+} else {
+ vlog("Code completion header path manipulation failed {0}",
+ HeaderFile.takeError());
+}
+ }
+}
llvm::SmallString<256> Scratch;
if (IndexResult) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 3c5bed7 - [clangd] ExpandAutoType: Do not offer code action on lambdas.
Author: Adam Czachorowski
Date: 2020-12-08T20:03:16+01:00
New Revision: 3c5bed734f9e02bd3bc4fbd1e0acc53506823ebf
URL:
https://github.com/llvm/llvm-project/commit/3c5bed734f9e02bd3bc4fbd1e0acc53506823ebf
DIFF:
https://github.com/llvm/llvm-project/commit/3c5bed734f9e02bd3bc4fbd1e0acc53506823ebf.diff
LOG: [clangd] ExpandAutoType: Do not offer code action on lambdas.
We can't expand lambda types anyway. Now we simply not offer the code
action instead of showing it and then returning an error in apply().
Differential Revision: https://reviews.llvm.org/D92847
Added:
Modified:
clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
clang-tools-extra/clangd/test/check-fail.test
clang-tools-extra/clangd/unittests/TweakTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
b/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
index 61f68a688252..6d38eb1de82a 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
@@ -63,6 +63,25 @@ bool isStructuredBindingType(const SelectionTree::Node *N) {
return N && N->ASTNode.get();
}
+// Returns true iff Node is a lambda, and thus should not be expanded. Loc is
+// the location of the auto type.
+bool isDeducedAsLambda(const SelectionTree::Node *Node, SourceLocation Loc) {
+ // getDeducedType() does a traversal, which we want to avoid in prepare().
+ // But at least check this isn't auto x = []{...};, which can't ever be
+ // expanded.
+ // (It would be nice if we had an efficient getDeducedType(), instead).
+ for (const auto *It = Node; It; It = It->Parent) {
+if (const auto *DD = It->ASTNode.get()) {
+ if (DD->getTypeSourceInfo() &&
+ DD->getTypeSourceInfo()->getTypeLoc().getBeginLoc() == Loc) {
+if (auto *RD = DD->getType()->getAsRecordDecl())
+ return RD->isLambda();
+ }
+}
+ }
+ return false;
+}
+
bool ExpandAutoType::prepare(const Selection& Inputs) {
CachedLocation = llvm::None;
if (auto *Node = Inputs.ASTSelection.commonAncestor()) {
@@ -70,11 +89,13 @@ bool ExpandAutoType::prepare(const Selection& Inputs) {
if (const AutoTypeLoc Result = TypeNode->getAs()) {
// Code in apply() does handle 'decltype(auto)' yet.
if (!Result.getTypePtr()->isDecltypeAuto() &&
-!isStructuredBindingType(Node))
+!isStructuredBindingType(Node) &&
+!isDeducedAsLambda(Node, Result.getBeginLoc()))
CachedLocation = Result;
}
}
}
+
return (bool) CachedLocation;
}
diff --git a/clang-tools-extra/clangd/test/check-fail.test
b/clang-tools-extra/clangd/test/check-fail.test
index 0ee777f02cc5..dd50b59b2c79 100644
--- a/clang-tools-extra/clangd/test/check-fail.test
+++ b/clang-tools-extra/clangd/test/check-fail.test
@@ -11,4 +11,5 @@
// CHECK: All checks completed, 2 errors
#include "missing.h"
-auto x = []{};
+void fun();
+auto x = fun;
diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index 85edd92d27da..0dcf8feb786f 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -559,8 +559,7 @@ TEST_F(ExpandAutoTypeTest, Test) {
EXPECT_THAT(apply("au^to x = &ns::Func;"),
StartsWith("fail: Could not expand type of function pointer"));
// lambda types are not replaced
- EXPECT_THAT(apply("au^to x = []{};"),
- StartsWith("fail: Could not expand type of lambda expression"));
+ EXPECT_UNAVAILABLE("au^to x = []{};");
// inline namespaces
EXPECT_EQ(apply("au^to x = inl_ns::Visible();"),
"Visible x = inl_ns::Visible();");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] f6b205d - [clangd] ExtractFunction: disable on regions that sometimes, but not always return.
Author: Adam Czachorowski
Date: 2020-12-08T15:55:32+01:00
New Revision: f6b205dae16392382324fbca676ef6afe3920642
URL:
https://github.com/llvm/llvm-project/commit/f6b205dae16392382324fbca676ef6afe3920642
DIFF:
https://github.com/llvm/llvm-project/commit/f6b205dae16392382324fbca676ef6afe3920642.diff
LOG: [clangd] ExtractFunction: disable on regions that sometimes, but not
always return.
apply() will fail in those cases, so it's better to detect it in
prepare() already and hide code action from the user.
This was especially annoying on code bases that use a lot of
RETURN_IF_ERROR-like macros.
Differential Revision: https://reviews.llvm.org/D92408
Added:
Modified:
clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
index 18fe7bf39160..8eec42876d6b 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
@@ -708,6 +708,27 @@ tooling::Replacement createFunctionDefinition(const
NewFunction &ExtractedFunc,
return tooling::Replacement(SM, ExtractedFunc.InsertionPoint, 0,
FunctionDef);
}
+// Returns true if ExtZone contains any ReturnStmts.
+bool hasReturnStmt(const ExtractionZone &ExtZone) {
+ class ReturnStmtVisitor
+ : public clang::RecursiveASTVisitor {
+ public:
+bool VisitReturnStmt(ReturnStmt *Return) {
+ Found = true;
+ return false; // We found the answer, abort the scan.
+}
+bool Found = false;
+ };
+
+ ReturnStmtVisitor V;
+ for (const Stmt *RootStmt : ExtZone.RootStmts) {
+V.TraverseStmt(const_cast(RootStmt));
+if (V.Found)
+ break;
+ }
+ return V.Found;
+}
+
bool ExtractFunction::prepare(const Selection &Inputs) {
const LangOptions &LangOpts = Inputs.AST->getLangOpts();
if (!LangOpts.CPlusPlus)
@@ -715,8 +736,12 @@ bool ExtractFunction::prepare(const Selection &Inputs) {
const Node *CommonAnc = Inputs.ASTSelection.commonAncestor();
const SourceManager &SM = Inputs.AST->getSourceManager();
auto MaybeExtZone = findExtractionZone(CommonAnc, SM, LangOpts);
+ if (!MaybeExtZone ||
+ (hasReturnStmt(*MaybeExtZone) && !alwaysReturns(*MaybeExtZone)))
+return false;
+
// FIXME: Get rid of this check once we support hoisting.
- if (!MaybeExtZone || MaybeExtZone->requiresHoisting(SM))
+ if (MaybeExtZone->requiresHoisting(SM))
return false;
ExtZone = std::move(*MaybeExtZone);
diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index 07f061b3f2e3..85edd92d27da 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -607,7 +607,11 @@ TEST_F(ExtractFunctionTest, FunctionTest) {
// Extract certain return
EXPECT_THAT(apply(" if(true) [[{ return; }]] "), HasSubstr("extracted"));
// Don't extract uncertain return
- EXPECT_THAT(apply(" if(true) [[if (false) return;]] "), StartsWith("fail"));
+ EXPECT_THAT(apply(" if(true) [[if (false) return;]] "),
+ StartsWith("unavailable"));
+ EXPECT_THAT(
+ apply("#define RETURN_IF_ERROR(x) if (x) return\nRETU^RN_IF_ERROR(4);"),
+ StartsWith("unavailable"));
FileName = "a.c";
EXPECT_THAT(apply(" for([[int i = 0;]];);"), HasSubstr("unavailable"));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] c282b7d - [clangd] AddUsing: Fix a crash on ElaboratedTypes without NestedNameSpecfiiers.
Author: Adam Czachorowski
Date: 2020-12-03T20:25:38+01:00
New Revision: c282b7de5a5de8151a19228702867e2299f1d3fe
URL:
https://github.com/llvm/llvm-project/commit/c282b7de5a5de8151a19228702867e2299f1d3fe
DIFF:
https://github.com/llvm/llvm-project/commit/c282b7de5a5de8151a19228702867e2299f1d3fe.diff
LOG: [clangd] AddUsing: Fix a crash on ElaboratedTypes without
NestedNameSpecfiiers.
Differential Revision: https://reviews.llvm.org/D92579
Added:
Modified:
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
index b00c2716005c7..d6a57efeeef13 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -274,6 +274,8 @@ bool AddUsing::prepare(const Selection &Inputs) {
} else if (auto *T = Node->ASTNode.get()) {
if (auto E = T->getAs()) {
QualifierToRemove = E.getQualifierLoc();
+ if (!QualifierToRemove)
+return false;
auto SpelledTokens =
TB.spelledForExpanded(TB.expandedTokens(E.getSourceRange()));
diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index eefc50d754e2a..07f061b3f2e39 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2523,6 +2523,9 @@ class cc {
// Do not offer code action on typo-corrections.
EXPECT_UNAVAILABLE(Header + "/*error-ok*/c^c C;");
+ // NestedNameSpecifier, but no namespace.
+ EXPECT_UNAVAILABLE(Header + "class Foo {}; class F^oo foo;");
+
// Check that we do not trigger in header files.
FileName = "test.h";
ExtraArgs.push_back("-xc++-header"); // .h file is treated a C by default.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 517828a - [clangd] Bundle code completion items when the include paths differ, but resolve to the same file.
Author: Adam Czachorowski
Date: 2020-12-03T16:33:15+01:00
New Revision: 517828a31b0d1b7cfd1fd261046746bd8778420a
URL:
https://github.com/llvm/llvm-project/commit/517828a31b0d1b7cfd1fd261046746bd8778420a
DIFF:
https://github.com/llvm/llvm-project/commit/517828a31b0d1b7cfd1fd261046746bd8778420a.diff
LOG: [clangd] Bundle code completion items when the include paths differ, but
resolve to the same file.
This can happen when, for example, merging results from an external
index that generates IncludeHeaders with full URI rather than just
literal include.
Differential Revision: https://reviews.llvm.org/D92494
Added:
Modified:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp
b/clang-tools-extra/clangd/CodeComplete.cpp
index 1d85439f53af..cc6b5dbc9904 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -173,9 +173,22 @@ struct CompletionCandidate {
// Returns a token identifying the overload set this is part of.
// 0 indicates it's not part of any overload set.
- size_t overloadSet(const CodeCompleteOptions &Opts) const {
+ size_t overloadSet(const CodeCompleteOptions &Opts, llvm::StringRef FileName,
+ IncludeInserter *Inserter) const {
if (!Opts.BundleOverloads.getValueOr(false))
return 0;
+
+// Depending on the index implementation, we can see
diff erent header
+// strings (literal or URI) mapping to the same file. We still want to
+// bundle those, so we must resolve the header to be included here.
+std::string HeaderForHash;
+if (Inserter)
+ if (auto Header = headerToInsertIfAllowed(Opts))
+if (auto HeaderFile = toHeaderFile(*Header, FileName))
+ if (auto Spelled =
+ Inserter->calculateIncludePath(*HeaderFile, FileName))
+HeaderForHash = *Spelled;
+
llvm::SmallString<256> Scratch;
if (IndexResult) {
switch (IndexResult->SymInfo.Kind) {
@@ -192,7 +205,7 @@ struct CompletionCandidate {
// This could break #include insertion.
return llvm::hash_combine(
(IndexResult->Scope + IndexResult->Name).toStringRef(Scratch),
-headerToInsertIfAllowed(Opts).getValueOr(""));
+HeaderForHash);
default:
return 0;
}
@@ -206,8 +219,7 @@ struct CompletionCandidate {
llvm::raw_svector_ostream OS(Scratch);
D->printQualifiedName(OS);
}
- return llvm::hash_combine(Scratch,
-headerToInsertIfAllowed(Opts).getValueOr(""));
+ return llvm::hash_combine(Scratch, HeaderForHash);
}
assert(IdentifierResult);
return 0;
@@ -1570,7 +1582,8 @@ class CodeCompleteFlow {
assert(IdentifierResult);
C.Name = IdentifierResult->Name;
}
- if (auto OverloadSet = C.overloadSet(Opts)) {
+ if (auto OverloadSet = C.overloadSet(
+ Opts, FileName, Inserter ? Inserter.getPointer() : nullptr)) {
auto Ret = BundleLookup.try_emplace(OverloadSet, Bundles.size());
if (Ret.second)
Bundles.emplace_back();
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index a7e1c6c48143..a19c6a83e954 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1628,6 +1628,29 @@ TEST(CompletionTest, OverloadBundling) {
EXPECT_EQ(A.SnippetSuffix, "($0)");
}
+TEST(CompletionTest, OverloadBundlingSameFileDifferentURI) {
+ clangd::CodeCompleteOptions Opts;
+ Opts.BundleOverloads = true;
+
+ Symbol SymX = sym("ns::X", index::SymbolKind::Function, "@F@\\0#");
+ Symbol SymY = sym("ns::X", index::SymbolKind::Function, "@F@\\0#I#");
+ std::string BarHeader = testPath("bar.h");
+ auto BarURI = URI::create(BarHeader).toString();
+ SymX.CanonicalDeclaration.FileURI = BarURI.c_str();
+ SymY.CanonicalDeclaration.FileURI = BarURI.c_str();
+ // The include header is
diff erent, but really it's the same file.
+ SymX.IncludeHeaders.emplace_back("\"bar.h\"", 1);
+ SymY.IncludeHeaders.emplace_back(BarURI.c_str(), 1);
+
+ auto Results = completions("void f() { ::ns::^ }", {SymX, SymY}, Opts);
+ // Expect both results are bundled, despite the
diff erent-but-same
+ // IncludeHeader.
+ ASSERT_EQ(1u, Results.Completions.size());
+ const auto &R = Results.Completions.front();
+ EXPECT_EQ("X", R.Name);
+ EXPECT_EQ(2u, R.BundleSize);
+}
+
TEST(CompletionTest, DocumentationFromChangedFileCrash) {
MockFS FS;
auto FooH = testPath("foo.h");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/lis
[clang-tools-extra] 9d87739 - [clangd] AddUsing: do not crash on non-namespace using decls.
Author: Adam Czachorowski
Date: 2020-11-26T20:07:56+01:00
New Revision: 9d87739f664b5b454ff78a3016ab05a1987f0d7c
URL:
https://github.com/llvm/llvm-project/commit/9d87739f664b5b454ff78a3016ab05a1987f0d7c
DIFF:
https://github.com/llvm/llvm-project/commit/9d87739f664b5b454ff78a3016ab05a1987f0d7c.diff
LOG: [clangd] AddUsing: do not crash on non-namespace using decls.
Differential Revision: https://reviews.llvm.org/D92186
Added:
Modified:
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
index 4b3fbc02c411..b00c2716005c 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -152,12 +152,14 @@ findInsertionPoint(const Tweak::Selection &Inputs,
if (SM.isBeforeInTranslationUnit(Inputs.Cursor, U->getUsingLoc()))
// "Usings" is sorted, so we're done.
break;
-if (U->getQualifier()->getAsNamespace()->getCanonicalDecl() ==
-QualifierToRemove.getNestedNameSpecifier()
-->getAsNamespace()
-->getCanonicalDecl() &&
-U->getName() == Name) {
- return InsertionPointData();
+if (const auto *Namespace = U->getQualifier()->getAsNamespace()) {
+ if (Namespace->getCanonicalDecl() ==
+ QualifierToRemove.getNestedNameSpecifier()
+ ->getAsNamespace()
+ ->getCanonicalDecl() &&
+ U->getName() == Name) {
+return InsertionPointData();
+ }
}
// Insertion point will be before last UsingDecl that affects cursor
diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index 4a2360dda739..eefc50d754e2 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2884,6 +2884,17 @@ using xx::yy;
void fun() {
yy();
}
+)cpp"},
+// Existing using with non-namespace part.
+{R"cpp(
+#include "test.hpp"
+using one::two::ee::ee_one;
+one::t^wo::cc c;
+)cpp",
+ R"cpp(
+#include "test.hpp"
+using one::two::cc;using one::two::ee::ee_one;
+cc c;
)cpp"}};
llvm::StringMap EditedFiles;
for (const auto &Case : Cases) {
@@ -2892,7 +2903,7 @@ void fun() {
namespace one {
void oo() {}
namespace two {
-enum ee {};
+enum ee {ee_one};
void ff() {}
class cc {
public:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] f697050 - [clangd] PopulateSwitch: disable on dependent enums.
Author: Adam Czachorowski
Date: 2020-11-25T14:12:29+01:00
New Revision: f6970503d291b7cae70fe583bed392387f93f9e4
URL:
https://github.com/llvm/llvm-project/commit/f6970503d291b7cae70fe583bed392387f93f9e4
DIFF:
https://github.com/llvm/llvm-project/commit/f6970503d291b7cae70fe583bed392387f93f9e4.diff
LOG: [clangd] PopulateSwitch: disable on dependent enums.
If the enum is a dependent type, we would crash somewhere in
getIntWidth(). -Wswitch diagnostic doesn't work on dependent enums
either.
Differential Revision: https://reviews.llvm.org/D92051
Added:
Modified:
clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
b/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
index 852888f6a043..bae80cdecf59 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp
@@ -126,7 +126,7 @@ bool PopulateSwitch::prepare(const Selection &Sel) {
return false;
EnumD = EnumT->getDecl();
- if (!EnumD)
+ if (!EnumD || EnumD->isDependentType())
return false;
// We trigger if there are any values in the enum that aren't covered by the
diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index fd815d2c4c27..4a2360dda739 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -3084,6 +3084,12 @@ TEST_F(PopulateSwitchTest, Test) {
R""(enum Enum {A,B,b=B}; ^switch (A) {case A:case B:break;})"",
"unavailable",
},
+ {
+ // Enum is dependent type
+ File,
+ R""(template void f() {enum Enum {A}; ^switch (A) {}})"",
+ "unavailable",
+ },
};
for (const auto &Case : Cases) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] a200501 - [clangd] Addusing tweak: find insertion point after definition
Author: Adam Czachorowski
Date: 2020-11-24T22:57:02+01:00
New Revision: a200501bca4dc7f3292d824f249fa21a479e9873
URL:
https://github.com/llvm/llvm-project/commit/a200501bca4dc7f3292d824f249fa21a479e9873
DIFF:
https://github.com/llvm/llvm-project/commit/a200501bca4dc7f3292d824f249fa21a479e9873.diff
LOG: [clangd] Addusing tweak: find insertion point after definition
When type/function is defined in the middle of the file, previuosly we
would sometimes insert a "using" line before that definition, leading to
a compilation error. With this fix, we pick a point after such
definition in translation unit.
This is not a perfect solution. For example, it still doesn't handle
"using namespace" directives. It is, however, a significant improvement.
Differential Revision: https://reviews.llvm.org/D92053
Added:
Modified:
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
index b53df446c732..4b3fbc02c411 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -48,6 +48,10 @@ class AddUsing : public Tweak {
NestedNameSpecifierLoc QualifierToRemove;
// The name following QualifierToRemove.
llvm::StringRef Name;
+ // If valid, the insertion point for "using" statement must come after this.
+ // This is relevant when the type is defined in the main file, to make sure
+ // the type/function is already defined at the point where "using" is added.
+ SourceLocation MustInsertAfterLoc;
};
REGISTER_TWEAK(AddUsing)
@@ -120,7 +124,8 @@ struct InsertionPointData {
llvm::Expected
findInsertionPoint(const Tweak::Selection &Inputs,
const NestedNameSpecifierLoc &QualifierToRemove,
- const llvm::StringRef Name) {
+ const llvm::StringRef Name,
+ const SourceLocation MustInsertAfterLoc) {
auto &SM = Inputs.AST->getSourceManager();
// Search for all using decls that affect this point in file. We need this
for
@@ -132,6 +137,11 @@ findInsertionPoint(const Tweak::Selection &Inputs,
SM)
.TraverseAST(Inputs.AST->getASTContext());
+ auto IsValidPoint = [&](const SourceLocation Loc) {
+return MustInsertAfterLoc.isInvalid() ||
+ SM.isBeforeInTranslationUnit(MustInsertAfterLoc, Loc);
+ };
+
bool AlwaysFullyQualify = true;
for (auto &U : Usings) {
// Only "upgrade" to fully qualified is all relevant using decls are fully
@@ -149,12 +159,13 @@ findInsertionPoint(const Tweak::Selection &Inputs,
U->getName() == Name) {
return InsertionPointData();
}
+
// Insertion point will be before last UsingDecl that affects cursor
// position. For most cases this should stick with the local convention of
// add using inside or outside namespace.
LastUsingLoc = U->getUsingLoc();
}
- if (LastUsingLoc.isValid()) {
+ if (LastUsingLoc.isValid() && IsValidPoint(LastUsingLoc)) {
InsertionPointData Out;
Out.Loc = LastUsingLoc;
Out.AlwaysFullyQualify = AlwaysFullyQualify;
@@ -175,7 +186,7 @@ findInsertionPoint(const Tweak::Selection &Inputs,
if (Tok == Toks.end() || Tok->endLocation().isInvalid()) {
return error("Namespace with no {");
}
-if (!Tok->endLocation().isMacroID()) {
+if (!Tok->endLocation().isMacroID() && IsValidPoint(Tok->endLocation())) {
InsertionPointData Out;
Out.Loc = Tok->endLocation();
Out.Suffix = "\n";
@@ -183,15 +194,17 @@ findInsertionPoint(const Tweak::Selection &Inputs,
}
}
// No using, no namespace, no idea where to insert. Try above the first
- // top level decl.
+ // top level decl after MustInsertAfterLoc.
auto TLDs = Inputs.AST->getLocalTopLevelDecls();
- if (TLDs.empty()) {
-return error("Cannot find place to insert \"using\"");
+ for (const auto &TLD : TLDs) {
+if (!IsValidPoint(TLD->getBeginLoc()))
+ continue;
+InsertionPointData Out;
+Out.Loc = SM.getExpansionLoc(TLD->getBeginLoc());
+Out.Suffix = "\n\n";
+return Out;
}
- InsertionPointData Out;
- Out.Loc = SM.getExpansionLoc(TLDs[0]->getBeginLoc());
- Out.Suffix = "\n\n";
- return Out;
+ return error("Cannot find place to insert \"using\"");
}
bool isNamespaceForbidden(const Tweak::Selection &Inputs,
@@ -254,6 +267,7 @@ bool AddUsing::prepare(const Selection &Inputs) {
if (auto *II = D->getDecl()->getIdentifier()) {
QualifierToRemove = D->getQualifierLoc();
Name = II->getName();
+ MustInsertAfterLoc = D->getDecl()->getBeginLoc();
}
} else if (auto *T = Node->ASTNode.get()) {
if (auto E = T->getAs()) {
@@ -271,6 +285,15 @@ bool AddUsing::prepare(const Selection &Inputs) {
[clang-tools-extra] f6e5929 - [clangd] AddUsing: Used spelled text instead of type name.
Author: Adam Czachorowski
Date: 2020-11-24T18:59:09+01:00
New Revision: f6e59294b63e1fd0b25720f24111cd17865004be
URL:
https://github.com/llvm/llvm-project/commit/f6e59294b63e1fd0b25720f24111cd17865004be
DIFF:
https://github.com/llvm/llvm-project/commit/f6e59294b63e1fd0b25720f24111cd17865004be.diff
LOG: [clangd] AddUsing: Used spelled text instead of type name.
This improves the behavior related to type aliases, as well as cases of
typo correction.
Differential Revision: https://reviews.llvm.org/D91966
Added:
Modified:
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
index cf8347f312a3..b53df446c732 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -43,9 +43,10 @@ class AddUsing : public Tweak {
}
private:
- // The qualifier to remove. Set by prepare().
+ // All of the following are set by prepare().
+ // The qualifier to remove.
NestedNameSpecifierLoc QualifierToRemove;
- // The name following QualifierToRemove. Set by prepare().
+ // The name following QualifierToRemove.
llvm::StringRef Name;
};
REGISTER_TWEAK(AddUsing)
@@ -206,8 +207,17 @@ bool isNamespaceForbidden(const Tweak::Selection &Inputs,
return false;
}
+std::string getNNSLAsString(NestedNameSpecifierLoc &NNSL,
+const PrintingPolicy &Policy) {
+ std::string Out;
+ llvm::raw_string_ostream OutStream(Out);
+ NNSL.getNestedNameSpecifier()->print(OutStream, Policy);
+ return OutStream.str();
+}
+
bool AddUsing::prepare(const Selection &Inputs) {
auto &SM = Inputs.AST->getSourceManager();
+ const auto &TB = Inputs.AST->getTokens();
// Do not suggest "using" in header files. That way madness lies.
if (isHeaderFile(SM.getFileEntryForID(SM.getMainFileID())->getName(),
@@ -247,11 +257,20 @@ bool AddUsing::prepare(const Selection &Inputs) {
}
} else if (auto *T = Node->ASTNode.get()) {
if (auto E = T->getAs()) {
- if (auto *BaseTypeIdentifier =
- E.getType().getUnqualifiedType().getBaseTypeIdentifier()) {
-Name = BaseTypeIdentifier->getName();
-QualifierToRemove = E.getQualifierLoc();
- }
+ QualifierToRemove = E.getQualifierLoc();
+
+ auto SpelledTokens =
+ TB.spelledForExpanded(TB.expandedTokens(E.getSourceRange()));
+ if (!SpelledTokens)
+return false;
+ auto SpelledRange = syntax::Token::range(SM, SpelledTokens->front(),
+ SpelledTokens->back());
+ Name = SpelledRange.text(SM);
+
+ std::string QualifierToRemoveStr = getNNSLAsString(
+ QualifierToRemove, Inputs.AST->getASTContext().getPrintingPolicy());
+ if (!Name.consume_front(QualifierToRemoveStr))
+return false; // What's spelled doesn't match the qualifier.
}
}
@@ -283,20 +302,13 @@ bool AddUsing::prepare(const Selection &Inputs) {
Expected AddUsing::apply(const Selection &Inputs) {
auto &SM = Inputs.AST->getSourceManager();
- auto &TB = Inputs.AST->getTokens();
- // Determine the length of the qualifier under the cursor, then remove it.
- auto SpelledTokens = TB.spelledForExpanded(
- TB.expandedTokens(QualifierToRemove.getSourceRange()));
- if (!SpelledTokens) {
-return error("Could not determine length of the qualifier");
- }
- unsigned Length =
- syntax::Token::range(SM, SpelledTokens->front(), SpelledTokens->back())
- .length();
+ std::string QualifierToRemoveStr = getNNSLAsString(
+ QualifierToRemove, Inputs.AST->getASTContext().getPrintingPolicy());
tooling::Replacements R;
if (auto Err = R.add(tooling::Replacement(
- SM, SpelledTokens->front().location(), Length, ""))) {
+ SM, SM.getSpellingLoc(QualifierToRemove.getBeginLoc()),
+ QualifierToRemoveStr.length(), ""))) {
return std::move(Err);
}
@@ -313,9 +325,8 @@ Expected AddUsing::apply(const Selection
&Inputs) {
if (InsertionPoint->AlwaysFullyQualify &&
!isFullyQualified(QualifierToRemove.getNestedNameSpecifier()))
UsingTextStream << "::";
-QualifierToRemove.getNestedNameSpecifier()->print(
-UsingTextStream, Inputs.AST->getASTContext().getPrintingPolicy());
-UsingTextStream << Name << ";" << InsertionPoint->Suffix;
+UsingTextStream << QualifierToRemoveStr << Name << ";"
+<< InsertionPoint->Suffix;
assert(SM.getFileID(InsertionPoint->Loc) == SM.getMainFileID());
if (auto Err = R.add(tooling::Replacement(SM, InsertionPoint->Loc, 0,
diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index 98d797aacd8f..
[clang] 95ce9fb - [clang] Do not crash on pointer wchar_t pointer assignment.
Author: Adam Czachorowski
Date: 2020-11-20T15:27:15+01:00
New Revision: 95ce9fbc235a467b84b2ffa2571f1d1a45af9427
URL:
https://github.com/llvm/llvm-project/commit/95ce9fbc235a467b84b2ffa2571f1d1a45af9427
DIFF:
https://github.com/llvm/llvm-project/commit/95ce9fbc235a467b84b2ffa2571f1d1a45af9427.diff
LOG: [clang] Do not crash on pointer wchar_t pointer assignment.
wchar_t can be signed (thus hasSignedIntegerRepresentation() returns
true), but it doesn't have an unsigned type, which would lead to a crash
when trying to get it.
With this fix, we special-case WideChar types in the pointer assignment
code.
Differential Revision: https://reviews.llvm.org/D91625
Added:
Modified:
clang/lib/AST/ASTContext.cpp
clang/test/SemaCXX/wchar_t.cpp
Removed:
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 836065291fea..f54916babed7 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -10007,6 +10007,11 @@ QualType
ASTContext::getCorrespondingUnsignedType(QualType T) const {
return UnsignedLongLongTy;
case BuiltinType::Int128:
return UnsignedInt128Ty;
+ // wchar_t is special. It is either signed or not, but when it's signed,
+ // there's no matching "unsigned wchar_t". Therefore we return the unsigned
+ // version of it's underlying type instead.
+ case BuiltinType::WChar_S:
+return getUnsignedWCharType();
case BuiltinType::ShortAccum:
return UnsignedShortAccumTy;
diff --git a/clang/test/SemaCXX/wchar_t.cpp b/clang/test/SemaCXX/wchar_t.cpp
index f8e9addf27b7..cc7c6de7b37f 100644
--- a/clang/test/SemaCXX/wchar_t.cpp
+++ b/clang/test/SemaCXX/wchar_t.cpp
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wno-signed-unsigned-wchar
-verify=allow-signed %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-signed-unsigned-wchar
-verify=allow-signed -DSKIP_ERROR_TESTS %s
// allow-signed-no-diagnostics
wchar_t x;
@@ -32,3 +32,10 @@ int t(void) {
// rdar://8040728
wchar_t in[] = L"\x434" "\x434"; // No warning
+#ifndef SKIP_ERROR_TESTS
+// Verify that we do not crash when assigning wchar_t* to another pointer type.
+void assignment(wchar_t *x) {
+ char *y;
+ y = x; // expected-error {{incompatible pointer types assigning to 'char *'
from 'wchar_t *'}}
+}
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] bcd8422 - [clangd] Fix argument type (bool->float).
Author: Adam Czachorowski
Date: 2020-10-07T17:22:00+02:00
New Revision: bcd8422d75069624dc2daf7e5ff4b4f6cbcd6b71
URL:
https://github.com/llvm/llvm-project/commit/bcd8422d75069624dc2daf7e5ff4b4f6cbcd6b71
DIFF:
https://github.com/llvm/llvm-project/commit/bcd8422d75069624dc2daf7e5ff4b4f6cbcd6b71.diff
LOG: [clangd] Fix argument type (bool->float).
The default value is 1.3f, but it was cast to true, which is not a good
base for code completion score.
Differential Revision: https://reviews.llvm.org/D88970
Added:
Modified:
clang-tools-extra/clangd/tool/ClangdMain.cpp
Removed:
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 98daaf957359..78d8355a2c5d 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -185,7 +185,7 @@ opt
RankingModel{
Hidden,
};
-opt DecisionForestBase{
+opt DecisionForestBase{
"decision-forest-base",
cat(Features),
desc("Base for exponentiating the prediction from DecisionForest."),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] c894bfd - [clangd] Add option for disabling AddUsing tweak on some namespaces.
Author: Adam Czachorowski
Date: 2020-09-18T16:46:09+02:00
New Revision: c894bfd1f580e5807fc98cc353b0834e0c5ddc21
URL:
https://github.com/llvm/llvm-project/commit/c894bfd1f580e5807fc98cc353b0834e0c5ddc21
DIFF:
https://github.com/llvm/llvm-project/commit/c894bfd1f580e5807fc98cc353b0834e0c5ddc21.diff
LOG: [clangd] Add option for disabling AddUsing tweak on some namespaces.
For style guides forbid "using" declarations for namespaces like "std".
With this new config option, AddUsing can be selectively disabled on
those.
Differential Revision: https://reviews.llvm.org/D87775
Added:
Modified:
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/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/Config.h
b/clang-tools-extra/clangd/Config.h
index f8dc2df51814..087dc4fb805d 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -62,6 +62,14 @@ struct Config {
/// Whether this TU should be indexed.
BackgroundPolicy Background = BackgroundPolicy::Build;
} Index;
+
+ /// 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 FullyQualifiedNamespaces;
+ } Style;
};
} // namespace clangd
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 9b8a48fdaf7b..62fda5c9eacc 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -208,6 +208,25 @@ struct FragmentCompiler {
}
}
+ void compile(Fragment::StyleBlock &&F) {
+if (!F.FullyQualifiedNamespaces.empty()) {
+ std::vector FullyQualifiedNamespaces;
+ for (auto &N : F.FullyQualifiedNamespaces) {
+// Normalize the data by dropping both leading and trailing ::
+StringRef Namespace(*N);
+Namespace.consume_front("::");
+Namespace.consume_back("::");
+FullyQualifiedNamespaces.push_back(Namespace.str());
+ }
+ Out.Apply.push_back([FullyQualifiedNamespaces(
+ std::move(FullyQualifiedNamespaces))](Config &C)
{
+C.Style.FullyQualifiedNamespaces.insert(
+C.Style.FullyQualifiedNamespaces.begin(),
+FullyQualifiedNamespaces.begin(), FullyQualifiedNamespaces.end());
+ });
+}
+ }
+
constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
constexpr static llvm::SourceMgr::DiagKind Warning =
llvm::SourceMgr::DK_Warning;
diff --git a/clang-tools-extra/clangd/ConfigFragment.h
b/clang-tools-extra/clangd/ConfigFragment.h
index 330f157326f2..5b4865e48f3a 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -161,6 +161,16 @@ struct Fragment {
llvm::Optional> Background;
};
IndexBlock Index;
+
+ // Describes the style of the codebase, beyond formatting.
+ struct StyleBlock {
+// 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.
+// Affects availability of the AddUsing tweak.
+std::vector> FullyQualifiedNamespaces;
+ };
+ StyleBlock Style;
};
} // namespace config
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp
b/clang-tools-extra/clangd/ConfigYAML.cpp
index 9988fe376648..bc7d4fdfe85b 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -39,6 +39,7 @@ class Parser {
Dict.handle("If", [&](Node &N) { parse(F.If, N); });
Dict.handle("CompileFlags", [&](Node &N) { parse(F.CompileFlags, N); });
Dict.handle("Index", [&](Node &N) { parse(F.Index, N); });
+Dict.handle("Style", [&](Node &N) { parse(F.Style, N); });
Dict.parse(N);
return !(N.failed() || HadError);
}
@@ -72,6 +73,15 @@ class Parser {
Dict.parse(N);
}
+ void parse(Fragment::StyleBlock &F, Node &N) {
+DictParser Dict("Style", this);
+Dict.handle("FullyQualifiedNamespaces", [&](Node &N) {
+ if (auto Values = scalarValues(N))
+F.FullyQualifiedNamespaces = std::move(*Values);
+});
+Dict.parse(N);
+ }
+
void parse(Fragment::IndexBlock &F, Node &N) {
DictParser Dict("Index", this);
Dict.handle("Background",
diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
index d5e6e12b31aa..8c69b64c5aff 100644
--- a/clang-tools-extra/clangd/
[clang-tools-extra] 7029e5d - [clangd] Actually parse Index section of the YAML file.
Author: Adam Czachorowski
Date: 2020-09-16T13:11:02+02:00
New Revision: 7029e5d4ca20d20982da8efe89de27acd8d7d75b
URL:
https://github.com/llvm/llvm-project/commit/7029e5d4ca20d20982da8efe89de27acd8d7d75b
DIFF:
https://github.com/llvm/llvm-project/commit/7029e5d4ca20d20982da8efe89de27acd8d7d75b.diff
LOG: [clangd] Actually parse Index section of the YAML file.
This fixes a bug in dbf486c0de92c76df77c1a1f815cf16533ecbb3a, which
introduced the Index section of the config, but did not register the
parse method, so it didn't work in a YAML file (but did in a test).
Differential Revision: https://reviews.llvm.org/D87710
Added:
Modified:
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp
b/clang-tools-extra/clangd/ConfigYAML.cpp
index 16639f6649c2..9988fe376648 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -38,6 +38,7 @@ class Parser {
DictParser Dict("Config", this);
Dict.handle("If", [&](Node &N) { parse(F.If, N); });
Dict.handle("CompileFlags", [&](Node &N) { parse(F.CompileFlags, N); });
+Dict.handle("Index", [&](Node &N) { parse(F.Index, N); });
Dict.parse(N);
return !(N.failed() || HadError);
}
diff --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
index a9526ce2367c..27b1c0cfc56d 100644
--- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -47,16 +47,21 @@ CompileFlags: { Add: [foo, bar] }
Add: |
b
az
+---
+Index:
+ Background: Skip
)yaml";
auto Results = Fragment::parseYAML(YAML, "config.yaml", Diags.callback());
EXPECT_THAT(Diags.Diagnostics, IsEmpty());
- ASSERT_EQ(Results.size(), 2u);
- EXPECT_FALSE(Results.front().If.HasUnrecognizedCondition);
- EXPECT_THAT(Results.front().If.PathMatch, ElementsAre(Val("abc")));
- EXPECT_THAT(Results.front().CompileFlags.Add,
- ElementsAre(Val("foo"), Val("bar")));
+ ASSERT_EQ(Results.size(), 3u);
+ EXPECT_FALSE(Results[0].If.HasUnrecognizedCondition);
+ EXPECT_THAT(Results[0].If.PathMatch, ElementsAre(Val("abc")));
+ EXPECT_THAT(Results[0].CompileFlags.Add, ElementsAre(Val("foo"),
Val("bar")));
+
+ EXPECT_THAT(Results[1].CompileFlags.Add, ElementsAre(Val("b\naz\n")));
- EXPECT_THAT(Results.back().CompileFlags.Add, ElementsAre(Val("b\naz\n")));
+ ASSERT_TRUE(Results[2].Index.Background);
+ EXPECT_EQ("Skip", *Results[2].Index.Background.getValue());
}
TEST(ParseYAML, Locations) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] eed0af6 - [clang] Exclude invalid destructors from lookups.
Author: Adam Czachorowski
Date: 2020-08-26T19:29:30+02:00
New Revision: eed0af6179ca4fe9e60121e0829ed8d3849b1ce5
URL:
https://github.com/llvm/llvm-project/commit/eed0af6179ca4fe9e60121e0829ed8d3849b1ce5
DIFF:
https://github.com/llvm/llvm-project/commit/eed0af6179ca4fe9e60121e0829ed8d3849b1ce5.diff
LOG: [clang] Exclude invalid destructors from lookups.
This fixes a crash when declaring a destructor with a wrong name, then
writing result to pch file and loading it again. The PCH storage uses
DeclarationNameKey as key and it is the same key for both the invalid
destructor and the implicit one that was created because the other one
was invalid. When querying for the Foo::~Foo we end up getting
Foo::~Bar, which is then rejected and we end up with nullptr in
CXXRecordDecl::GetDestructor().
Fixes https://bugs.llvm.org/show_bug.cgi?id=47270
Differential Revision: https://reviews.llvm.org/D86624
Added:
clang/test/PCH/cxx-invalid-destructor.cpp
clang/test/PCH/cxx-invalid-destructor.h
Modified:
clang/lib/AST/DeclBase.cpp
Removed:
diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index da1eadd9d931d..f4314d0bd9614 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1487,6 +1487,13 @@ static bool shouldBeHidden(NamedDecl *D) {
if (FD->isFunctionTemplateSpecialization())
return true;
+ // Hide destructors that are invalid. There should always be one destructor,
+ // but if it is an invalid decl, another one is created. We need to hide the
+ // invalid one from places that expect exactly one destructor, like the
+ // serialization code.
+ if (isa(D) && D->isInvalidDecl())
+return true;
+
return false;
}
diff --git a/clang/test/PCH/cxx-invalid-destructor.cpp
b/clang/test/PCH/cxx-invalid-destructor.cpp
new file mode 100644
index 0..fc89cf1f3dfc1
--- /dev/null
+++ b/clang/test/PCH/cxx-invalid-destructor.cpp
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -x c++ -std=c++11 -emit-pch -o %t
%S/cxx-invalid-destructor.h -fallow-pch-with-compiler-errors
+// RUN: %clang_cc1 -x c++ -std=c++11 -include-pch %t
%S/cxx-invalid-destructor.cpp -fsyntax-only -fno-validate-pch
+
+Foo f;
diff --git a/clang/test/PCH/cxx-invalid-destructor.h
b/clang/test/PCH/cxx-invalid-destructor.h
new file mode 100644
index 0..59095a37c203e
--- /dev/null
+++ b/clang/test/PCH/cxx-invalid-destructor.h
@@ -0,0 +1,7 @@
+struct Base {
+ ~Base();
+};
+
+struct Foo : public Base {
+ ~Base();
+};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 4d90ff5 - [clangd] When inserting "using", add "::" in front if that's the style.
Author: Adam Czachorowski
Date: 2020-08-25T14:07:49+02:00
New Revision: 4d90ff59ac453a67ac692ffdf8242e4cfbd2b34f
URL:
https://github.com/llvm/llvm-project/commit/4d90ff59ac453a67ac692ffdf8242e4cfbd2b34f
DIFF:
https://github.com/llvm/llvm-project/commit/4d90ff59ac453a67ac692ffdf8242e4cfbd2b34f.diff
LOG: [clangd] When inserting "using", add "::" in front if that's the style.
We guess the style based on the existing using declarations. If there
are any and they all start with ::, we add it to the newly added one
too.
Differential Revision: https://reviews.llvm.org/D86473
Added:
Modified:
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
index 9e64ceeeaead..e4900041671a 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -86,6 +86,13 @@ class UsingFinder : public RecursiveASTVisitor {
const SourceManager &SM;
};
+bool isFullyQualified(const NestedNameSpecifier *NNS) {
+ if (!NNS)
+return false;
+ return NNS->getKind() == NestedNameSpecifier::Global ||
+ isFullyQualified(NNS->getPrefix());
+}
+
struct InsertionPointData {
// Location to insert the "using" statement. If invalid then the statement
// should not be inserted at all (it already exists).
@@ -94,6 +101,9 @@ struct InsertionPointData {
// insertion point is anchored to, we may need one or more \n to ensure
// proper formatting.
std::string Suffix;
+ // Whether using should be fully qualified, even if what the user typed was
+ // not. This is based on our detection of the local style.
+ bool AlwaysFullyQualify = false;
};
// Finds the best place to insert the "using" statement. Returns invalid
@@ -118,7 +128,13 @@ findInsertionPoint(const Tweak::Selection &Inputs,
SM)
.TraverseAST(Inputs.AST->getASTContext());
+ bool AlwaysFullyQualify = true;
for (auto &U : Usings) {
+// Only "upgrade" to fully qualified is all relevant using decls are fully
+// qualified. Otherwise trust what the user typed.
+if (!isFullyQualified(U->getQualifier()))
+ AlwaysFullyQualify = false;
+
if (SM.isBeforeInTranslationUnit(Inputs.Cursor, U->getUsingLoc()))
// "Usings" is sorted, so we're done.
break;
@@ -137,6 +153,7 @@ findInsertionPoint(const Tweak::Selection &Inputs,
if (LastUsingLoc.isValid()) {
InsertionPointData Out;
Out.Loc = LastUsingLoc;
+Out.AlwaysFullyQualify = AlwaysFullyQualify;
return Out;
}
@@ -278,6 +295,9 @@ Expected AddUsing::apply(const Selection
&Inputs) {
std::string UsingText;
llvm::raw_string_ostream UsingTextStream(UsingText);
UsingTextStream << "using ";
+if (InsertionPoint->AlwaysFullyQualify &&
+!isFullyQualified(QualifierToRemove.getNestedNameSpecifier()))
+ UsingTextStream << "::";
QualifierToRemove.getNestedNameSpecifier()->print(
UsingTextStream, Inputs.AST->getASTContext().getPrintingPolicy());
UsingTextStream << Name << ";" << InsertionPoint->Suffix;
diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index b4f135b3efe2..5626b7530599 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2723,6 +2723,63 @@ namespace foo { void fun(); }
void foo::fun() {
ff();
+})cpp"},
+// If all other using are fully qualified, add ::
+{R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ee;
+
+void fun() {
+ one::two::f^f();
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ff;using ::one::two::ee;
+
+void fun() {
+ ff();
+})cpp"},
+// Make sure we don't add :: if it's already there
+{R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ee;
+
+void fun() {
+ ::one::two::f^f();
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ff;using ::one::two::ee;
+
+void fun() {
+ ff();
+})cpp"},
+// If even one using doesn't start with ::, do not add it
+{R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using one::two::ee;
+
+void fun() {
+ one::two::f^f();
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using one::two::ff;using one::two::ee;
+
+void fun() {
+ ff();
})cpp"}};
llvm::StringMap EditedFiles;
for (const auto &Case : Cases) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] b488935 - [clangd] Discard diagnostics from another SourceManager.
Author: Adam Czachorowski
Date: 2020-08-21T13:11:21+02:00
New Revision: b4889353207aefd6f2641cef0301f78838c5b52e
URL:
https://github.com/llvm/llvm-project/commit/b4889353207aefd6f2641cef0301f78838c5b52e
DIFF:
https://github.com/llvm/llvm-project/commit/b4889353207aefd6f2641cef0301f78838c5b52e.diff
LOG: [clangd] Discard diagnostics from another SourceManager.
This can happen when building implicit modules, as demonstrated in test.
The CompilerInstance uses the same StoredDiags, but different
SourceManager. This used to crash clangd when it tried to relocate the
diagnostic to the main file, which, according to SourceManager from the
diagnostic, is a fake file.
Differential Revision: https://reviews.llvm.org/D85753
Added:
Modified:
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/Diagnostics.h
clang-tools-extra/clangd/unittests/ModulesTests.cpp
Removed:
diff --git a/clang-tools-extra/clangd/Diagnostics.cpp
b/clang-tools-extra/clangd/Diagnostics.cpp
index 674fa0dbf9ec..afa72f9d4051 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -518,13 +518,17 @@ std::vector StoreDiags::take(const
clang::tidy::ClangTidyContext *Tidy) {
}
void StoreDiags::BeginSourceFile(const LangOptions &Opts,
- const Preprocessor *) {
+ const Preprocessor *PP) {
LangOpts = Opts;
+ if (PP) {
+OrigSrcMgr = &PP->getSourceManager();
+ }
}
void StoreDiags::EndSourceFile() {
flushLastDiag();
LangOpts = None;
+ OrigSrcMgr = nullptr;
}
/// Sanitizes a piece for presenting it in a synthesized fix message. Ensures
@@ -560,6 +564,16 @@ static void fillNonLocationData(DiagnosticsEngine::Level
DiagLevel,
void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) {
+ // If the diagnostic was generated for a
diff erent SourceManager, skip it.
+ // This happens when a module is imported and needs to be implicitly built.
+ // The compilation of that module will use the same StoreDiags, but
diff erent
+ // SourceManager.
+ if (OrigSrcMgr && Info.hasSourceManager() &&
+ OrigSrcMgr != &Info.getSourceManager()) {
+IgnoreDiagnostics::log(DiagLevel, Info);
+return;
+ }
+
DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
bool OriginallyError =
Info.getDiags()->getDiagnosticIDs()->isDefaultMappingAsError(
diff --git a/clang-tools-extra/clangd/Diagnostics.h
b/clang-tools-extra/clangd/Diagnostics.h
index 58f69287f256..4a00abfc450f 100644
--- a/clang-tools-extra/clangd/Diagnostics.h
+++ b/clang-tools-extra/clangd/Diagnostics.h
@@ -122,7 +122,8 @@ class StoreDiags : public DiagnosticConsumer {
// The ClangTidyContext populates Source and Name for clang-tidy diagnostics.
std::vector take(const clang::tidy::ClangTidyContext *Tidy = nullptr);
- void BeginSourceFile(const LangOptions &Opts, const Preprocessor *) override;
+ void BeginSourceFile(const LangOptions &Opts,
+ const Preprocessor *PP) override;
void EndSourceFile() override;
void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) override;
@@ -148,6 +149,7 @@ class StoreDiags : public DiagnosticConsumer {
llvm::Optional LastDiag;
llvm::Optional LastDiagLoc; // Valid only when LastDiag is
set.
bool LastDiagOriginallyError = false; // Valid only when LastDiag is
set.
+ SourceManager *OrigSrcMgr = nullptr;
llvm::DenseSet> IncludedErrorLocations;
bool LastPrimaryDiagnosticWasSuppressed = false;
diff --git a/clang-tools-extra/clangd/unittests/ModulesTests.cpp
b/clang-tools-extra/clangd/unittests/ModulesTests.cpp
index a10b9e897a48..83d6b28d6dfc 100644
--- a/clang-tools-extra/clangd/unittests/ModulesTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -64,6 +64,34 @@ TEST(Modules, PreambleBuildVisibility) {
EXPECT_TRUE(TU.build().getDiagnostics().empty());
}
+TEST(Modules, Diagnostic) {
+ // Produce a diagnostic while building an implicit module. Use
+ // -fmodules-strict-decluse, but any non-silenced diagnostic will do.
+ TestTU TU = TestTU::withCode(R"cpp(
+/*error-ok*/
+#include "modular.h"
+
+void bar() {}
+)cpp");
+ TU.OverlayRealFileSystemForModules = true;
+ TU.ExtraArgs.push_back("-fmodule-map-file=" + testPath("m.modulemap"));
+ TU.ExtraArgs.push_back("-fmodules");
+ TU.ExtraArgs.push_back("-fimplicit-modules");
+ TU.ExtraArgs.push_back("-fmodules-strict-decluse");
+ TU.AdditionalFiles["modular.h"] = R"cpp(
+#include "non-modular.h"
+ )cpp";
+ TU.AdditionalFiles["non-modular.h"] = "";
+ TU.AdditionalFiles["m.modulemap"] = R"modulemap(
+module M {
+ header "modular.h"
+}
+)modulemap";
+
+ // Test that we do not crash.
+ T
[clang-tools-extra] 707138d - [clangd] Remove useless stderr logging.
Author: Adam Czachorowski
Date: 2020-08-20T14:52:04+02:00
New Revision: 707138d677861182083b3c6c3b44b76951fd36ef
URL:
https://github.com/llvm/llvm-project/commit/707138d677861182083b3c6c3b44b76951fd36ef
DIFF:
https://github.com/llvm/llvm-project/commit/707138d677861182083b3c6c3b44b76951fd36ef.diff
LOG: [clangd] Remove useless stderr logging.
This was accidentally added in 53b9199a5cdba8a6e294e1fb183f308ec558db22
Differential Revision: https://reviews.llvm.org/D86284
Added:
Modified:
clang-tools-extra/clangd/unittests/TestTU.cpp
Removed:
diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp
b/clang-tools-extra/clangd/unittests/TestTU.cpp
index c468642e6338..03254f876721 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.cpp
+++ b/clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -74,8 +74,6 @@ void initializeModuleCache(CompilerInvocation &CI) {
ASSERT_FALSE(
llvm::sys::fs::createUniqueDirectory("module-cache", ModuleCachePath));
CI.getHeaderSearchOpts().ModuleCachePath = ModuleCachePath.c_str();
- llvm::errs() << "MC: " << ModuleCachePath << "\n";
- llvm::errs().flush();
}
void deleteModuleCache(const std::string ModuleCachePath) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] 53b9199 - [clangd] Fix crash-bug in preamble indexing when using modules.
Author: Adam Czachorowski
Date: 2020-08-20T14:19:52+02:00
New Revision: 53b9199a5cdba8a6e294e1fb183f308ec558db22
URL:
https://github.com/llvm/llvm-project/commit/53b9199a5cdba8a6e294e1fb183f308ec558db22
DIFF:
https://github.com/llvm/llvm-project/commit/53b9199a5cdba8a6e294e1fb183f308ec558db22.diff
LOG: [clangd] Fix crash-bug in preamble indexing when using modules.
When preamble contains #undef, indexing code finds the matching #define
and uses that during indexing. However, it would only look for local
definitions. If the macro was defined in a module, MacroInfo
would be nullptr and clangd would crash.
This change makes clangd ignore any #undef without a matching #define
inside the same TU.
The indexing of macros happens for preamble only, so then #undef must be
in the preamble, which is why we need two .h files in a test.
Note that clangd is currently not ready for module support, but this
brings us one step closer.
This was previously attempted in
4061d9e42cff621462931ac7df9666806c77a237, but had to be reverted due to
broken test. This version fixes that test-only bug by setting a custom module
cache path to avoid re-use of modules across test invocations.
Differential Revision: https://reviews.llvm.org/D85923
Added:
Modified:
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
clang-tools-extra/clangd/unittests/TestFS.h
clang-tools-extra/clangd/unittests/TestTU.cpp
clang-tools-extra/clangd/unittests/TestTU.h
clang/lib/Index/IndexingAction.cpp
Removed:
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index d89db8f015ce..3940946d8016 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1623,6 +1623,31 @@ TEST_F(SymbolCollectorTest, MacrosInHeaders) {
EXPECT_THAT(Symbols,
UnorderedElementsAre(AllOf(QName("X"),
ForCodeCompletion(true;
}
+
+// Regression test for a crash-bug we used to have.
+TEST_F(SymbolCollectorTest, UndefOfModuleMacro) {
+ auto TU = TestTU::withCode(R"cpp(#include "bar.h")cpp");
+ TU.AdditionalFiles["bar.h"] = R"cpp(
+#include "foo.h"
+#undef X
+)cpp";
+ TU.AdditionalFiles["foo.h"] = "#define X 1";
+ TU.AdditionalFiles["module.map"] = R"cpp(
+module foo {
+ header "foo.h"
+ export *
+ }
+ )cpp";
+ TU.ExtraArgs.push_back("-fmodules");
+ TU.ExtraArgs.push_back("-fmodule-map-file=" + testPath("module.map"));
+ TU.OverlayRealFileSystemForModules = true;
+
+ TU.build();
+ // We mostly care about not crashing, but verify that we didn't insert
garbage
+ // about X too.
+ EXPECT_THAT(TU.headerSymbols(), Not(Contains(QName("X";
+}
+
} // namespace
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/unittests/TestFS.h
b/clang-tools-extra/clangd/unittests/TestFS.h
index 7972fe37c7fe..82de319d96cf 100644
--- a/clang-tools-extra/clangd/unittests/TestFS.h
+++ b/clang-tools-extra/clangd/unittests/TestFS.h
@@ -34,12 +34,21 @@ buildTestFS(llvm::StringMap const &Files,
class MockFS : public ThreadsafeFS {
public:
IntrusiveRefCntPtr viewImpl() const override {
-return buildTestFS(Files, Timestamps);
+auto MemFS = buildTestFS(Files, Timestamps);
+if (!OverlayRealFileSystemForModules)
+ return MemFS;
+llvm::IntrusiveRefCntPtr OverlayFileSystem =
+new llvm::vfs::OverlayFileSystem(llvm::vfs::getRealFileSystem());
+OverlayFileSystem->pushOverlay(MemFS);
+return OverlayFileSystem;
}
// If relative paths are used, they are resolved with testPath().
llvm::StringMap Files;
llvm::StringMap Timestamps;
+ // If true, real file system will be used as fallback for the in-memory one.
+ // This is useful for testing module support.
+ bool OverlayRealFileSystemForModules = false;
};
// A Compilation database that returns a fixed set of compile flags.
diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp
b/clang-tools-extra/clangd/unittests/TestTU.cpp
index 7d5c29f23393..c468642e6338 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.cpp
+++ b/clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -16,6 +16,7 @@
#include "clang/Basic/Diagnostic.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/Utils.h"
+#include "llvm/ADT/ScopeExit.h"
namespace clang {
namespace clangd {
@@ -54,6 +55,8 @@ ParseInputs TestTU::inputs(MockFS &FS) const {
Inputs.CompileCommand.Filename = FullFilename;
Inputs.CompileCommand.Directory = testRoot();
Inputs.Contents = Code;
+ if (OverlayRealFileSystemForModules)
+FS.OverlayRealFileSystemForModules = true;
Inputs.TFS = &FS;
Inputs.Opts = ParseOptions();
Inputs.Opts.BuildRecoveryAST = true;
@@ -66,12 +69,31 @@ ParseInputs TestTU::inputs(MockFS &FS) const {
return
[clang-tools-extra] baeff98 - [clang] When loading preamble from AST file, re-export modules in Sema.
Author: Adam Czachorowski
Date: 2020-08-20T14:19:52+02:00
New Revision: baeff989b050e0f63412c52c1b8f9d8f3e91f671
URL:
https://github.com/llvm/llvm-project/commit/baeff989b050e0f63412c52c1b8f9d8f3e91f671
DIFF:
https://github.com/llvm/llvm-project/commit/baeff989b050e0f63412c52c1b8f9d8f3e91f671.diff
LOG: [clang] When loading preamble from AST file, re-export modules in Sema.
This addresses a FIXME in ASTReader.
Modules were already re-exported for Preprocessor, but not for Sema.
The result was that, with -fmodules-local-submodule-visibility, all AST
nodes belonging to a module that was loaded in a premable where not
accesible from the main part of the file and a diagnostic recommending
importing those modules would be generated.
Differential Revision: https://reviews.llvm.org/D86069
Added:
clang/test/PCH/preamble-modules.cpp
Modified:
clang-tools-extra/clangd/unittests/ModulesTests.cpp
clang/include/clang/Sema/Sema.h
clang/lib/Serialization/ASTReader.cpp
clang/test/PCH/Inputs/modules/Foo.h
Removed:
diff --git a/clang-tools-extra/clangd/unittests/ModulesTests.cpp
b/clang-tools-extra/clangd/unittests/ModulesTests.cpp
index 0098a15f64bb..a10b9e897a48 100644
--- a/clang-tools-extra/clangd/unittests/ModulesTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -26,7 +26,7 @@ TEST(Modules, TextualIncludeInPreamble) {
void foo() {}
)cpp");
TU.ExtraArgs.push_back("-fmodule-name=M");
- TU.ExtraArgs.push_back("-fmodule-map-file=m.modulemap");
+ TU.ExtraArgs.push_back("-fmodule-map-file=" + testPath("m.modulemap"));
TU.AdditionalFiles["Textual.h"] = "void foo();";
TU.AdditionalFiles["m.modulemap"] = R"modulemap(
module M {
@@ -39,6 +39,31 @@ TEST(Modules, TextualIncludeInPreamble) {
TU.index();
}
+// Verify that visibility of AST nodes belonging to modules, but loaded from
+// preamble PCH, is restored.
+TEST(Modules, PreambleBuildVisibility) {
+ TestTU TU = TestTU::withCode(R"cpp(
+#include "module.h"
+
+foo x;
+)cpp");
+ TU.OverlayRealFileSystemForModules = true;
+ TU.ExtraArgs.push_back("-fmodules");
+ TU.ExtraArgs.push_back("-fmodules-strict-decluse");
+ TU.ExtraArgs.push_back("-Xclang");
+ TU.ExtraArgs.push_back("-fmodules-local-submodule-visibility");
+ TU.ExtraArgs.push_back("-fmodule-map-file=" + testPath("m.modulemap"));
+ TU.AdditionalFiles["module.h"] = R"cpp(
+typedef int foo;
+)cpp";
+ TU.AdditionalFiles["m.modulemap"] = R"modulemap(
+module M {
+ header "module.h"
+}
+)modulemap";
+ EXPECT_TRUE(TU.build().getDiagnostics().empty());
+}
+
} // namespace
} // namespace clangd
} // namespace clang
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 49ab94f9df14..84e66b85208e 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1912,6 +1912,12 @@ class Sema final {
bool isModuleVisible(const Module *M, bool ModulePrivate = false);
+ // When loading a non-modular PCH files, this is used to restore module
+ // visibility.
+ void makeModuleVisible(Module *Mod, SourceLocation ImportLoc) {
+VisibleModules.setVisible(Mod, ImportLoc);
+ }
+
/// Determine whether a declaration is visible to name lookup.
bool isVisible(const NamedDecl *D) {
return D->isUnconditionallyVisible() || isVisibleSlow(D);
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 21cdde9e8455..464bbc662230 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -4987,10 +4987,10 @@ void ASTReader::InitializeContext() {
/*ImportLoc=*/Import.ImportLoc);
if (Import.ImportLoc.isValid())
PP.makeModuleVisible(Imported, Import.ImportLoc);
- // FIXME: should we tell Sema to make the module visible too?
+ // This updates visibility for Preprocessor only. For Sema, which can be
+ // nullptr here, we do the same later, in UpdateSema().
}
}
- ImportedModules.clear();
}
void ASTReader::finalizeForWriting() {
@@ -7943,6 +7943,15 @@ void ASTReader::UpdateSema() {
SemaObj->FpPragmaStack.CurrentPragmaLocation = FpPragmaCurrentLocation;
}
}
+
+ // For non-modular AST files, restore visiblity of modules.
+ for (auto &Import : ImportedModules) {
+if (Import.ImportLoc.isInvalid())
+ continue;
+if (Module *Imported = getSubmodule(Import.ID)) {
+ SemaObj->makeModuleVisible(Imported, Import.ImportLoc);
+}
+ }
}
IdentifierInfo *ASTReader::get(StringRef Name) {
diff --git a/clang/test/PCH/Inputs/modules/Foo.h
b/clang/test/PCH/Inputs/modules/Foo.h
index d661dbccbc33..c93614e0683f 100644
--- a/clang/test/PCH/Inputs/modules/Foo.h
+++ b/clang/test/PCH/Inputs/modules/Foo.h
@@ -1 +1,3 @@
void make_foo(void);
+
+typedef int MyType;
diff --git a/clang/test/PCH/preamble-modules.cpp
b
[clang] 73f0772 - [clangd] Revert "[clangd] Fix crash-bug in preamble indexing when using modules."
Author: Adam Czachorowski
Date: 2020-08-13T17:09:54+02:00
New Revision: 73f0772c0baf1c7cac2995341c11d83c4d7a37f4
URL:
https://github.com/llvm/llvm-project/commit/73f0772c0baf1c7cac2995341c11d83c4d7a37f4
DIFF:
https://github.com/llvm/llvm-project/commit/73f0772c0baf1c7cac2995341c11d83c4d7a37f4.diff
LOG: [clangd] Revert "[clangd] Fix crash-bug in preamble indexing when using
modules."
This reverts commit 4061d9e42cff621462931ac7df9666806c77a237.
Tests are failing in some configuration, likely due to not cleaning up
module cache path before running the test.
Differential Revision: https://reviews.llvm.org/D85907
Added:
Modified:
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
clang-tools-extra/clangd/unittests/TestFS.h
clang-tools-extra/clangd/unittests/TestTU.cpp
clang-tools-extra/clangd/unittests/TestTU.h
clang/lib/Index/IndexingAction.cpp
Removed:
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 48a6952b2610..70a8e6832d02 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1610,31 +1610,6 @@ TEST_F(SymbolCollectorTest, MacrosInHeaders) {
EXPECT_THAT(Symbols,
UnorderedElementsAre(AllOf(QName("X"),
ForCodeCompletion(true;
}
-
-// Regression test for a crash-bug we used to have.
-TEST_F(SymbolCollectorTest, UndefOfModuleMacro) {
- auto TU = TestTU::withCode(R"cpp(#include "bar.h")cpp");
- TU.AdditionalFiles["bar.h"] = R"cpp(
-#include "foo.h"
-#undef X
-)cpp";
- TU.AdditionalFiles["foo.h"] = "#define X 1";
- TU.AdditionalFiles["module.map"] = R"cpp(
-module foo {
- header "foo.h"
- export *
- }
- )cpp";
- TU.ExtraArgs.push_back("-fmodules");
- TU.ExtraArgs.push_back("-fmodule-map-file=" + testPath("module.map"));
- TU.OverlayRealFileSystemForModules = true;
-
- TU.build();
- // We mostly care about not crashing, but verify that we didn't insert
garbage
- // about X too.
- EXPECT_THAT(TU.headerSymbols(), Not(Contains(QName("X";
-}
-
} // namespace
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/unittests/TestFS.h
b/clang-tools-extra/clangd/unittests/TestFS.h
index 82de319d96cf..7972fe37c7fe 100644
--- a/clang-tools-extra/clangd/unittests/TestFS.h
+++ b/clang-tools-extra/clangd/unittests/TestFS.h
@@ -34,21 +34,12 @@ buildTestFS(llvm::StringMap const &Files,
class MockFS : public ThreadsafeFS {
public:
IntrusiveRefCntPtr viewImpl() const override {
-auto MemFS = buildTestFS(Files, Timestamps);
-if (!OverlayRealFileSystemForModules)
- return MemFS;
-llvm::IntrusiveRefCntPtr OverlayFileSystem =
-new llvm::vfs::OverlayFileSystem(llvm::vfs::getRealFileSystem());
-OverlayFileSystem->pushOverlay(MemFS);
-return OverlayFileSystem;
+return buildTestFS(Files, Timestamps);
}
// If relative paths are used, they are resolved with testPath().
llvm::StringMap Files;
llvm::StringMap Timestamps;
- // If true, real file system will be used as fallback for the in-memory one.
- // This is useful for testing module support.
- bool OverlayRealFileSystemForModules = false;
};
// A Compilation database that returns a fixed set of compile flags.
diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp
b/clang-tools-extra/clangd/unittests/TestTU.cpp
index 16305400307a..7d5c29f23393 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.cpp
+++ b/clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -54,8 +54,6 @@ ParseInputs TestTU::inputs(MockFS &FS) const {
Inputs.CompileCommand.Filename = FullFilename;
Inputs.CompileCommand.Directory = testRoot();
Inputs.Contents = Code;
- if (OverlayRealFileSystemForModules)
-FS.OverlayRealFileSystemForModules = true;
Inputs.TFS = &FS;
Inputs.Opts = ParseOptions();
Inputs.Opts.BuildRecoveryAST = true;
diff --git a/clang-tools-extra/clangd/unittests/TestTU.h
b/clang-tools-extra/clangd/unittests/TestTU.h
index dd0cecc406ac..06abd64dc623 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.h
+++ b/clang-tools-extra/clangd/unittests/TestTU.h
@@ -66,16 +66,6 @@ struct TestTU {
// Simulate a header guard of the header (using an #import directive).
bool ImplicitHeaderGuard = true;
- // Whether to use overlay the TestFS over the real filesystem. This is
- // required for use of implicit modules.where the module file is written to
- // disk and later read back.
- // FIXME: Change the way reading/writing modules work to allow us to keep
them
- // in memory across multiple clang invocations, at least in tests, to
- // eliminate the need for real file system here.
- // Please avoid using this for things other than implicit modules. The plan
is
- // to eliminate this option some day
[clang] 4061d9e - [clangd] Fix crash-bug in preamble indexing when using modules.
Author: Adam Czachorowski
Date: 2020-08-10T18:42:57+02:00
New Revision: 4061d9e42cff621462931ac7df9666806c77a237
URL:
https://github.com/llvm/llvm-project/commit/4061d9e42cff621462931ac7df9666806c77a237
DIFF:
https://github.com/llvm/llvm-project/commit/4061d9e42cff621462931ac7df9666806c77a237.diff
LOG: [clangd] Fix crash-bug in preamble indexing when using modules.
Summary:
When preamble contains #undef, indexing code finds the matching #define
and uses that during indexing. However, it would only look for local
definitions. If the macro was defined in a module, MacroInfo
would be nullptr and clangd would crash.
This change makes clangd ignore any #undef without a matching #define
inside the same TU.
The indexing of macros happens for preamble only, so then #undef must be
in the preamble, which is why we need two .h files in a test.
Note that clangd is currently not ready for module support, but this
brings us one step closer.
Reviewers: sammccall
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95,
cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D80525
Added:
Modified:
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
clang-tools-extra/clangd/unittests/TestFS.h
clang-tools-extra/clangd/unittests/TestTU.cpp
clang-tools-extra/clangd/unittests/TestTU.h
clang/lib/Index/IndexingAction.cpp
Removed:
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 3614ab2c5cb9..8e8f1a4f9af5 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1520,6 +1520,31 @@ TEST_F(SymbolCollectorTest, MacrosInHeaders) {
EXPECT_THAT(Symbols,
UnorderedElementsAre(AllOf(QName("X"),
ForCodeCompletion(true;
}
+
+// Regression test for a crash-bug we used to have.
+TEST_F(SymbolCollectorTest, UndefOfModuleMacro) {
+ auto TU = TestTU::withCode(R"cpp(#include "bar.h")cpp");
+ TU.AdditionalFiles["bar.h"] = R"cpp(
+#include "foo.h"
+#undef X
+)cpp";
+ TU.AdditionalFiles["foo.h"] = "#define X 1";
+ TU.AdditionalFiles["module.map"] = R"cpp(
+module foo {
+ header "foo.h"
+ export *
+ }
+ )cpp";
+ TU.ExtraArgs.push_back("-fmodules");
+ TU.ExtraArgs.push_back("-fmodule-map-file=" + testPath("module.map"));
+ TU.OverlayRealFileSystemForModules = true;
+
+ TU.build();
+ // We mostly care about not crashing, but verify that we didn't insert
garbage
+ // about X too.
+ EXPECT_THAT(TU.headerSymbols(), Not(Contains(QName("X";
+}
+
} // namespace
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/unittests/TestFS.h
b/clang-tools-extra/clangd/unittests/TestFS.h
index 7972fe37c7fe..82de319d96cf 100644
--- a/clang-tools-extra/clangd/unittests/TestFS.h
+++ b/clang-tools-extra/clangd/unittests/TestFS.h
@@ -34,12 +34,21 @@ buildTestFS(llvm::StringMap const &Files,
class MockFS : public ThreadsafeFS {
public:
IntrusiveRefCntPtr viewImpl() const override {
-return buildTestFS(Files, Timestamps);
+auto MemFS = buildTestFS(Files, Timestamps);
+if (!OverlayRealFileSystemForModules)
+ return MemFS;
+llvm::IntrusiveRefCntPtr OverlayFileSystem =
+new llvm::vfs::OverlayFileSystem(llvm::vfs::getRealFileSystem());
+OverlayFileSystem->pushOverlay(MemFS);
+return OverlayFileSystem;
}
// If relative paths are used, they are resolved with testPath().
llvm::StringMap Files;
llvm::StringMap Timestamps;
+ // If true, real file system will be used as fallback for the in-memory one.
+ // This is useful for testing module support.
+ bool OverlayRealFileSystemForModules = false;
};
// A Compilation database that returns a fixed set of compile flags.
diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp
b/clang-tools-extra/clangd/unittests/TestTU.cpp
index 7d5c29f23393..16305400307a 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.cpp
+++ b/clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -54,6 +54,8 @@ ParseInputs TestTU::inputs(MockFS &FS) const {
Inputs.CompileCommand.Filename = FullFilename;
Inputs.CompileCommand.Directory = testRoot();
Inputs.Contents = Code;
+ if (OverlayRealFileSystemForModules)
+FS.OverlayRealFileSystemForModules = true;
Inputs.TFS = &FS;
Inputs.Opts = ParseOptions();
Inputs.Opts.BuildRecoveryAST = true;
diff --git a/clang-tools-extra/clangd/unittests/TestTU.h
b/clang-tools-extra/clangd/unittests/TestTU.h
index 06abd64dc623..dd0cecc406ac 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.h
+++ b/clang-tools-extra/clangd/unittests/TestTU.h
@@ -66,6 +66,16 @@ struct TestTU {
// Simulate a header guard of the header (using an #import directive).
bool ImplicitHeaderGuard = true;
+ // W
[clang-tools-extra] e2d61ae - Correctly set CompilingPCH in PrecompilePreambleAction.
Author: Adam Czachorowski
Date: 2020-08-10T17:49:23+02:00
New Revision: e2d61ae5733316a14783b36c84b8e7681b0e3d59
URL:
https://github.com/llvm/llvm-project/commit/e2d61ae5733316a14783b36c84b8e7681b0e3d59
DIFF:
https://github.com/llvm/llvm-project/commit/e2d61ae5733316a14783b36c84b8e7681b0e3d59.diff
LOG: Correctly set CompilingPCH in PrecompilePreambleAction.
This fixes a crash bug in clangd when used with modules. ASTWriter would
end up writing references to submodules into the PCH file, but upon
reading the submodules would not exists and
HeaderFileInfoTrait::ReadData would crash.
Differential Revision: https://reviews.llvm.org/D85532
Added:
clang-tools-extra/clangd/unittests/ModulesTests.cpp
Modified:
clang-tools-extra/clangd/unittests/CMakeLists.txt
clang/lib/Frontend/PrecompiledPreamble.cpp
clang/unittests/Frontend/ASTUnitTest.cpp
Removed:
diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt
b/clang-tools-extra/clangd/unittests/CMakeLists.txt
index c25e2b7f8103..966fa9630852 100644
--- a/clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -63,6 +63,7 @@ add_unittest(ClangdUnitTests ClangdTests
IndexTests.cpp
JSONTransportTests.cpp
LSPClient.cpp
+ ModulesTests.cpp
ParsedASTTests.cpp
PathMappingTests.cpp
PreambleTests.cpp
diff --git a/clang-tools-extra/clangd/unittests/ModulesTests.cpp
b/clang-tools-extra/clangd/unittests/ModulesTests.cpp
new file mode 100644
index ..0098a15f64bb
--- /dev/null
+++ b/clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -0,0 +1,44 @@
+//===-- ModulesTests.cpp ---*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Annotations.h"
+#include "TestFS.h"
+#include "TestTU.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include
+#include
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(Modules, TextualIncludeInPreamble) {
+ TestTU TU = TestTU::withCode(R"cpp(
+#include "Textual.h"
+
+void foo() {}
+)cpp");
+ TU.ExtraArgs.push_back("-fmodule-name=M");
+ TU.ExtraArgs.push_back("-fmodule-map-file=m.modulemap");
+ TU.AdditionalFiles["Textual.h"] = "void foo();";
+ TU.AdditionalFiles["m.modulemap"] = R"modulemap(
+module M {
+ module Textual {
+textual header "Textual.h"
+ }
+}
+)modulemap";
+ // Test that we do not crash.
+ TU.index();
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
diff --git a/clang/lib/Frontend/PrecompiledPreamble.cpp
b/clang/lib/Frontend/PrecompiledPreamble.cpp
index 6cdfc595dcae..3fb61f37ae2b 100644
--- a/clang/lib/Frontend/PrecompiledPreamble.cpp
+++ b/clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -208,6 +208,11 @@ class PrecompilePreambleAction : public ASTFrontendAction {
Callbacks.AfterPCHEmitted(Writer);
}
+ bool BeginSourceFileAction(CompilerInstance &CI) override {
+assert(CI.getLangOpts().CompilingPCH);
+return ASTFrontendAction::BeginSourceFileAction(CI);
+ }
+
bool shouldEraseOutputFiles() override { return !hasEmittedPreamblePCH(); }
bool hasCodeCompletionSupport() const override { return false; }
bool hasASTFileSupport() const override { return false; }
@@ -396,6 +401,8 @@ llvm::ErrorOr
PrecompiledPreamble::Build(
auto PreambleDepCollector = std::make_shared();
Clang->addDependencyCollector(PreambleDepCollector);
+ Clang->getLangOpts().CompilingPCH = true;
+
// Remap the main source file to the preamble buffer.
StringRef MainFilePath = FrontendOpts.Inputs[0].getFile();
auto PreambleInputBuffer = llvm::MemoryBuffer::getMemBufferCopy(
diff --git a/clang/unittests/Frontend/ASTUnitTest.cpp
b/clang/unittests/Frontend/ASTUnitTest.cpp
index bdbc462e2bcb..c5b84e74cee9 100644
--- a/clang/unittests/Frontend/ASTUnitTest.cpp
+++ b/clang/unittests/Frontend/ASTUnitTest.cpp
@@ -13,6 +13,7 @@
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Lex/HeaderSearch.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/ToolOutputFile.h"
@@ -111,4 +112,42 @@ TEST_F(ASTUnitTest, GetBufferForFileMemoryMapping) {
llvm::MemoryBuffer::MemoryBuffer_MMap);
}
+TEST_F(ASTUnitTest, ModuleTextualHeader) {
+ llvm::IntrusiveRefCntPtr InMemoryFs =
+ new llvm::vfs::InMemoryFileSystem();
+ InMemoryFs->addFile("test.cpp", 0, llvm::MemoryBuffer::getMemBuffer(R"cpp(
+ #include "Textual.h"
+ void foo() {}
+)cpp"));
+ InMemoryFs->addFile("m.modulem
