[PATCH] D43224: Fix typos.
This revision was automatically updated to reflect the committed changes. Closed by commit rL324989: Fix typos. (authored by brucem, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D43224 Files: libcxx/trunk/docs/DesignDocs/AvailabilityMarkup.rst libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst Index: libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst === --- libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst +++ libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst @@ -46,7 +46,7 @@ Otherwise we create a custom installation rule that modifies the installed __config header. The rule first generates a dummy "__config_site" header containing the required -#defines. The contents of the dummy header are then prependend to the installed +#defines. The contents of the dummy header are then prepended to the installed __config header. By manually prepending the files we avoid the cost of an extra #include and we allow the __config header to be ignorant of the extra configuration all together. An example "__config" header generated when Index: libcxx/trunk/docs/DesignDocs/AvailabilityMarkup.rst === --- libcxx/trunk/docs/DesignDocs/AvailabilityMarkup.rst +++ libcxx/trunk/docs/DesignDocs/AvailabilityMarkup.rst @@ -58,7 +58,7 @@ Some parameters can be passed to lit to run the test-suite and exercising the availability. -* The `platform` parameter controls the deployement target. For example lit can +* The `platform` parameter controls the deployment target. For example lit can be invoked with `--param=platform=macosx10.8`. Default is the current host. * The `use_system_cxx_lib` parameter indicates to use another library than the just built one. Invoking lit with `--param=use_system_cxx_lib=true` will run Index: libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst === --- libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst +++ libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst @@ -46,7 +46,7 @@ Otherwise we create a custom installation rule that modifies the installed __config header. The rule first generates a dummy "__config_site" header containing the required -#defines. The contents of the dummy header are then prependend to the installed +#defines. The contents of the dummy header are then prepended to the installed __config header. By manually prepending the files we avoid the cost of an extra #include and we allow the __config header to be ignorant of the extra configuration all together. An example "__config" header generated when Index: libcxx/trunk/docs/DesignDocs/AvailabilityMarkup.rst === --- libcxx/trunk/docs/DesignDocs/AvailabilityMarkup.rst +++ libcxx/trunk/docs/DesignDocs/AvailabilityMarkup.rst @@ -58,7 +58,7 @@ Some parameters can be passed to lit to run the test-suite and exercising the availability. -* The `platform` parameter controls the deployement target. For example lit can +* The `platform` parameter controls the deployment target. For example lit can be invoked with `--param=platform=macosx10.8`. Default is the current host. * The `use_system_cxx_lib` parameter indicates to use another library than the just built one. Invoking lit with `--param=use_system_cxx_lib=true` will run ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43127: [clangd] Stop exposing Futures from ClangdServer operations.
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: unittests/clangd/ClangdTests.cpp:783
public:
-NoConcurrentAccessDiagConsumer(std::promise StartSecondReparse)
-: StartSecondReparse(std::move(StartSecondReparse)) {}
+std::atomic Count = {0};
sammccall wrote:
> ilya-biryukov wrote:
> > Maybe use paren initializers? Looks a bit less curly :-)
> > ```
> > std::atomic Count(0);
> > ```
> This is a member initializer, I think I have to use this syntax :-(
You're right, you have to do braced init here.
Sorry, my mistake.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43127
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43167: Fix incorrect indentation.
brucem updated this revision to Diff 134002. brucem added a comment. Rebased forward. Repository: rCXX libc++ https://reviews.llvm.org/D43167 Files: include/ios Index: include/ios === --- include/ios +++ include/ios @@ -670,7 +670,7 @@ void set_rdbuf(basic_streambuf* __sb); private: basic_ostream* __tie_; - mutable int_type __fill_; +mutable int_type __fill_; }; template Index: include/ios === --- include/ios +++ include/ios @@ -670,7 +670,7 @@ void set_rdbuf(basic_streambuf* __sb); private: basic_ostream* __tie_; - mutable int_type __fill_; +mutable int_type __fill_; }; template ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43226: __threading_support: Remove (void) in favor of ().
brucem created this revision.
brucem added reviewers: mclow.lists, EricWF.
This fixes a clang-tidy warning when building something that uses
this file.
Repository:
rCXX libc++
https://reviews.llvm.org/D43226
Files:
include/__threading_support
Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -160,7 +160,7 @@
// Execute once
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_execute_once(__libcpp_exec_once_flag *flag,
- void (*init_routine)(void));
+ void (*init_routine)());
// Thread id
_LIBCPP_THREAD_ABI_VISIBILITY
@@ -303,7 +303,7 @@
// Execute once
int __libcpp_execute_once(__libcpp_exec_once_flag *flag,
- void (*init_routine)(void)) {
+ void (*init_routine)()) {
return pthread_once(flag, init_routine);
}
Index: include/__threading_support
===
--- include/__threading_support
+++ include/__threading_support
@@ -160,7 +160,7 @@
// Execute once
_LIBCPP_THREAD_ABI_VISIBILITY
int __libcpp_execute_once(__libcpp_exec_once_flag *flag,
- void (*init_routine)(void));
+ void (*init_routine)());
// Thread id
_LIBCPP_THREAD_ABI_VISIBILITY
@@ -303,7 +303,7 @@
// Execute once
int __libcpp_execute_once(__libcpp_exec_once_flag *flag,
- void (*init_routine)(void)) {
+ void (*init_routine)()) {
return pthread_once(flag, init_routine);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r324988 - [DebugInfo] Avoid name conflict of generated VLA expression variable.
Author: s.desmalen
Date: Mon Feb 12 23:49:34 2018
New Revision: 324988
URL: http://llvm.org/viewvc/llvm-project?rev=324988&view=rev
Log:
[DebugInfo] Avoid name conflict of generated VLA expression variable.
Summary:
This patch also adds the 'DW_AT_artificial' flag to the generated variable.
Addresses the issues mentioned in http://llvm.org/PR30553.
Reviewers: CarlosAlbertoEnciso, probinson, aprantl
Reviewed By: aprantl
Subscribers: JDevlieghere, cfe-commits
Differential Revision: https://reviews.llvm.org/D43189
Modified:
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/test/CodeGen/debug-info-vla.c
cfe/trunk/test/CodeGenCXX/debug-info-vla.cpp
cfe/trunk/test/OpenMP/target_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_for_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_for_simd_codegen.cpp
cfe/trunk/test/OpenMP/target_simd_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_codegen.cpp
Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=324988&r1=324987&r2=324988&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Mon Feb 12 23:49:34 2018
@@ -969,8 +969,8 @@ void CodeGenFunction::EmitAndRegisterVar
if (auto *C = dyn_cast(VlaSize.NumElts))
Dimensions.emplace_back(C, Type1D.getUnqualifiedType());
else {
- auto SizeExprAddr =
- CreateDefaultAlignTempAlloca(VlaSize.NumElts->getType(), "vla_expr");
+ auto SizeExprAddr = CreateDefaultAlignTempAlloca(
+ VlaSize.NumElts->getType(), "__vla_expr");
Builder.CreateStore(VlaSize.NumElts, SizeExprAddr);
Dimensions.emplace_back(SizeExprAddr.getPointer(),
Type1D.getUnqualifiedType());
@@ -999,6 +999,7 @@ void CodeGenFunction::EmitAndRegisterVar
getContext(), const_cast(D.getDeclContext()),
D.getLocation(), D.getLocation(), &NameIdent, QT,
getContext().CreateTypeSourceInfo(QT), SC_Auto);
+ ArtificialDecl->setImplicit();
MD = DI->EmitDeclareOfAutoVariable(ArtificialDecl, VlaSize.NumElts,
Builder);
Modified: cfe/trunk/test/CodeGen/debug-info-vla.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-info-vla.c?rev=324988&r1=324987&r2=324988&view=diff
==
--- cfe/trunk/test/CodeGen/debug-info-vla.c (original)
+++ cfe/trunk/test/CodeGen/debug-info-vla.c Mon Feb 12 23:49:34 2018
@@ -2,9 +2,9 @@
void testVLAwithSize(int s)
{
-// CHECK-DAG: dbg.declare({{.*}} %vla_expr, metadata ![[VLAEXPR:[0-9]+]]
+// CHECK-DAG: dbg.declare({{.*}} %__vla_expr, metadata ![[VLAEXPR:[0-9]+]]
// CHECK-DAG: dbg.declare({{.*}} %vla, metadata ![[VAR:[0-9]+]]
-// CHECK-DAG: ![[VLAEXPR]] = !DILocalVariable(name: "vla_expr"
+// CHECK-DAG: ![[VLAEXPR]] = !DILocalVariable(name: "__vla_expr", {{.*}}
flags: DIFlagArtificial
// CHECK-DAG: ![[VAR]] = !DILocalVariable(name: "vla",{{.*}} line: [[@LINE+2]]
// CHECK-DAG: !DISubrange(count: ![[VLAEXPR]])
int vla[s];
Modified: cfe/trunk/test/CodeGenCXX/debug-info-vla.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-vla.cpp?rev=324988&r1=324987&r2=324988&view=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-vla.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-vla.cpp Mon Feb 12 23:49:34 2018
@@ -13,7 +13,7 @@ int (*fp)(int[][*]) = nullptr;
// CHECK: [[ELEM_TYPE]] = !{[[NOCOUNT:.*]]}
// CHECK: [[NOCOUNT]] = !DISubrange(count: -1)
//
-// CHECK: [[VAR:![0-9]+]] = !DILocalVariable(name: "vla_expr"
+// CHECK: [[VAR:![0-9]+]] = !DILocalVariable(name: "__vla_expr", {{.*}}flags:
DIFlagArtificial
// CHECK: !DICompositeType(tag: DW_TAG_array_type,
// CHECK-NOT: size:
// CHECK-SAME: elements: [[ELEM_TYPE:![0-9]+]]
Modified: cfe/trunk/test/OpenMP/target_codegen.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/target_codegen.cpp?rev=324988&r1=324987&r2=324988&view=diff
==
--- cfe/trunk/test/OpenMP/target_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/target_codegen.cpp Mon Feb 12 23:49:34 2018
@@ -511,7 +511,7 @@ int bar(int n){
// CHECK-64: store i32 %{{.+}}, i32* [[B_ADDR]],
// CHECK-64: [[B_CVAL:%.+]] = load i[[SZ]], i[[SZ]]* [[B_CADDR]],
-// CHECK-32: store i32 %{{.+}}, i32* %vla_expr
+// CHECK-32: store i32 %{{.+}}, i32* %__vla_expr
// CHECK-32: store i32 %{{.+}}, i32* [[B_ADDR:%.+]],
[libcxx] r324989 - Fix typos.
Author: brucem Date: Tue Feb 13 00:12:00 2018 New Revision: 324989 URL: http://llvm.org/viewvc/llvm-project?rev=324989&view=rev Log: Fix typos. Reviewers: mclow.lists, EricWF Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D43224 Modified: libcxx/trunk/docs/DesignDocs/AvailabilityMarkup.rst libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst Modified: libcxx/trunk/docs/DesignDocs/AvailabilityMarkup.rst URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/DesignDocs/AvailabilityMarkup.rst?rev=324989&r1=324988&r2=324989&view=diff == --- libcxx/trunk/docs/DesignDocs/AvailabilityMarkup.rst (original) +++ libcxx/trunk/docs/DesignDocs/AvailabilityMarkup.rst Tue Feb 13 00:12:00 2018 @@ -58,7 +58,7 @@ Testing Some parameters can be passed to lit to run the test-suite and exercising the availability. -* The `platform` parameter controls the deployement target. For example lit can +* The `platform` parameter controls the deployment target. For example lit can be invoked with `--param=platform=macosx10.8`. Default is the current host. * The `use_system_cxx_lib` parameter indicates to use another library than the just built one. Invoking lit with `--param=use_system_cxx_lib=true` will run Modified: libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst?rev=324989&r1=324988&r2=324989&view=diff == --- libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst (original) +++ libcxx/trunk/docs/DesignDocs/CapturingConfigInfo.rst Tue Feb 13 00:12:00 2018 @@ -46,7 +46,7 @@ we do NOTHING. Otherwise we create a custom installation rule that modifies the installed __config header. The rule first generates a dummy "__config_site" header containing the required -#defines. The contents of the dummy header are then prependend to the installed +#defines. The contents of the dummy header are then prepended to the installed __config header. By manually prepending the files we avoid the cost of an extra #include and we allow the __config header to be ignorant of the extra configuration all together. An example "__config" header generated when ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r324990 - [clangd] Stop exposing Futures from ClangdServer operations.
Author: sammccall
Date: Tue Feb 13 00:59:23 2018
New Revision: 324990
URL: http://llvm.org/viewvc/llvm-project?rev=324990&view=rev
Log:
[clangd] Stop exposing Futures from ClangdServer operations.
Summary:
LSP has asynchronous semantics, being able to block on an async operation
completing is unneccesary and leads to tighter coupling with the threading.
In practice only tests depend on this, so we add a general-purpose "block until
idle" function to the scheduler which will work for all operations.
To get this working, fix a latent condition-variable bug in ASTWorker, and make
AsyncTaskRunner const-correct.
Reviewers: ilya-biryukov
Subscribers: klimek, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D43127
Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/clangd/Threading.cpp
clang-tools-extra/trunk/clangd/Threading.h
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/ThreadingTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=324990&r1=324989&r2=324990&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Feb 13 00:59:23 2018
@@ -139,11 +139,11 @@ void ClangdServer::setRootPath(PathRef R
this->RootPath = NewRootPath;
}
-std::future ClangdServer::addDocument(PathRef File, StringRef Contents) {
+void ClangdServer::addDocument(PathRef File, StringRef Contents) {
DocVersion Version = DraftMgr.updateDraft(File, Contents);
auto TaggedFS = FSProvider.getTaggedFileSystem(File);
- return scheduleReparseAndDiags(File, VersionedDraft{Version, Contents.str()},
- std::move(TaggedFS));
+ scheduleReparseAndDiags(File, VersionedDraft{Version, Contents.str()},
+ std::move(TaggedFS));
}
void ClangdServer::removeDocument(PathRef File) {
@@ -152,7 +152,7 @@ void ClangdServer::removeDocument(PathRe
WorkScheduler.remove(File);
}
-std::future ClangdServer::forceReparse(PathRef File) {
+void ClangdServer::forceReparse(PathRef File) {
auto FileContents = DraftMgr.getDraft(File);
assert(FileContents.Draft &&
"forceReparse() was called for non-added document");
@@ -162,8 +162,7 @@ std::future ClangdServer::forceRep
CompileArgs.invalidate(File);
auto TaggedFS = FSProvider.getTaggedFileSystem(File);
- return scheduleReparseAndDiags(File, std::move(FileContents),
- std::move(TaggedFS));
+ scheduleReparseAndDiags(File, std::move(FileContents), std::move(TaggedFS));
}
void ClangdServer::codeComplete(
@@ -463,7 +462,7 @@ ClangdServer::findDocumentHighlights(Pat
return blockingRunWithAST(WorkScheduler, File, Action);
}
-std::future ClangdServer::scheduleReparseAndDiags(
+void ClangdServer::scheduleReparseAndDiags(
PathRef File, VersionedDraft Contents,
Tagged> TaggedFS) {
tooling::CompileCommand Command = CompileArgs.getCompileCommand(File);
@@ -474,12 +473,7 @@ std::future ClangdServer::schedule
Path FileStr = File.str();
VFSTag Tag = std::move(TaggedFS.Tag);
- std::promise DonePromise;
- std::future DoneFuture = DonePromise.get_future();
-
- auto Callback = [this, Version, FileStr, Tag](std::promise DonePromise,
-OptDiags Diags) {
-auto Guard = llvm::make_scope_exit([&]() { DonePromise.set_value(); });
+ auto Callback = [this, Version, FileStr, Tag](OptDiags Diags) {
if (!Diags)
return; // A new reparse was requested before this one completed.
@@ -503,8 +497,7 @@ std::future ClangdServer::schedule
ParseInputs{std::move(Command),
std::move(TaggedFS.Value),
std::move(*Contents.Draft)},
- BindWithForward(Callback, std::move(DonePromise)));
- return DoneFuture;
+ std::move(Callback));
}
void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams &Params) {
@@ -516,3 +509,8 @@ std::vector
ClangdServer::getUsedBytesPerFile() const {
return WorkScheduler.getUsedBytesPerFile();
}
+
+LLVM_NODISCARD bool
+ClangdServer::blockUntilIdleForTest(llvm::Optional TimeoutSeconds) {
+ return WorkScheduler.blockUntilIdle(timeoutSeconds(TimeoutSeconds));
+}
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=324990&r1=324989&r2=324990&view=diff
===
[PATCH] D43127: [clangd] Stop exposing Futures from ClangdServer operations.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL324990: [clangd] Stop exposing Futures from ClangdServer
operations. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D43127
Files:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/clangd/Threading.cpp
clang-tools-extra/trunk/clangd/Threading.h
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/trunk/unittests/clangd/ThreadingTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/ThreadingTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/ThreadingTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/ThreadingTests.cpp
@@ -45,7 +45,7 @@
scheduleIncrements();
}
-Tasks.waitForAll();
+Tasks.wait();
{
std::lock_guard Lock(Mutex);
ASSERT_EQ(Counter, TasksCnt * IncrementsPerTask);
Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -121,7 +121,8 @@
/*StorePreamblesInMemory=*/true);
auto File = getVirtualTestFilePath("foo.cpp");
Annotations Test(Text);
- Server.addDocument(File, Test.code()).wait();
+ Server.addDocument(File, Test.code());
+ EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
auto CompletionList = runCodeComplete(Server, File, Test.point(), Opts).Value;
// Sanity-check that filterText is valid.
EXPECT_THAT(CompletionList.items, Each(NameContainsFilter()));
@@ -551,21 +552,24 @@
void f() { ns::^; }
void f() { ns::preamble().$2^; }
)cpp");
- Server.addDocument(File, Test.code()).wait();
+ Server.addDocument(File, Test.code());
+ ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
clangd::CodeCompleteOptions Opts = {};
- auto WithoutIndex = runCodeComplete(Server, File, Test.point(), Opts).Value;
- EXPECT_THAT(WithoutIndex.items,
- UnorderedElementsAre(Named("local"), Named("preamble")));
-
auto I = memIndex({var("ns::index")});
Opts.Index = I.get();
auto WithIndex = runCodeComplete(Server, File, Test.point(), Opts).Value;
EXPECT_THAT(WithIndex.items,
UnorderedElementsAre(Named("local"), Named("index")));
auto ClassFromPreamble =
runCodeComplete(Server, File, Test.point("2"), Opts).Value;
EXPECT_THAT(ClassFromPreamble.items, Contains(Named("member")));
+
+ Opts.Index = nullptr;
+ auto WithoutIndex = runCodeComplete(Server, File, Test.point(), Opts).Value;
+ EXPECT_THAT(WithoutIndex.items,
+ UnorderedElementsAre(Named("local"), Named("preamble")));
+
}
TEST(CompletionTest, DynamicIndexMultiFile) {
@@ -576,11 +580,10 @@
/*StorePreamblesInMemory=*/true,
/*BuildDynamicSymbolIndex=*/true);
- Server
- .addDocument(getVirtualTestFilePath("foo.cpp"), R"cpp(
+ Server.addDocument(getVirtualTestFilePath("foo.cpp"), R"cpp(
namespace ns { class XYZ {}; void foo(int x) {} }
- )cpp")
- .wait();
+ )cpp");
+ ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
auto File = getVirtualTestFilePath("bar.cpp");
Annotations Test(R"cpp(
@@ -591,7 +594,8 @@
}
void f() { ns::^ }
)cpp");
- Server.addDocument(File, Test.code()).wait();
+ Server.addDocument(File, Test.code());
+ ASSERT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
auto Results = runCodeComplete(Server, File, Test.point(), {}).Value;
// "XYZ" and "foo" are not included in the file being completed but are still
@@ -621,6 +625,7 @@
auto File = getVirtualTestFilePath("foo.cpp");
Annotations Test(Text);
Server.addDocument(File, Test.code());
+ EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
auto R = Server.signatureHelp(File, Test.point());
assert(R);
return R.get().Value;
Index: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
@@ -40,11 +40,6 @@
namespace {
-// Don't wait for async ops in clangd test more than that to avoid blocking
-// indefinitely in case of bugs.
-static const std::chrono::seconds DefaultFutureTimeout =
-std::chrono::seconds(10);
-
static bool diagsContainErrors(ArrayRef Diagnostics) {
for (const auto &DiagAndFixIt
[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning
oren_ben_simhon added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D41880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r324991 - Fix for PR32992. Static const classes not exported.
Author: hans
Date: Tue Feb 13 01:19:43 2018
New Revision: 324991
URL: http://llvm.org/viewvc/llvm-project?rev=324991&view=rev
Log:
Fix for PR32992. Static const classes not exported.
Patch by zahiraam!
Differential Revision: https://reviews.llvm.org/D42968
Modified:
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CodeGenCXX/dllexport.cpp
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=324991&r1=324990&r2=324991&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Feb 13 01:19:43 2018
@@ -5476,7 +5476,7 @@ static void CheckAbstractClassUsage(Abst
}
}
-static void ReferenceDllExportedMethods(Sema &S, CXXRecordDecl *Class) {
+static void ReferenceDllExportedMembers(Sema &S, CXXRecordDecl *Class) {
Attr *ClassAttr = getDLLAttr(Class);
if (!ClassAttr)
return;
@@ -5491,6 +5491,16 @@ static void ReferenceDllExportedMethods(
return;
for (Decl *Member : Class->decls()) {
+// Defined static variables that are members of an exported base
+// class must be marked export too. Push them to implicit instantiation
+// queue.
+auto *VD = dyn_cast(Member);
+if (VD && Member->getAttr() &&
+VD->getStorageClass() == SC_Static &&
+TSK == TSK_ImplicitInstantiation)
+ S.PendingLocalImplicitInstantiations.push_back(
+ std::make_pair(VD, VD->getLocation()));
+
auto *MD = dyn_cast(Member);
if (!MD)
continue;
@@ -10902,12 +10912,12 @@ void Sema::ActOnFinishCXXNonNestedClass(
void Sema::referenceDLLExportedClassMethods() {
if (!DelayedDllExportClasses.empty()) {
-// Calling ReferenceDllExportedMethods might cause the current function to
+// Calling ReferenceDllExportedMembers might cause the current function to
// be called again, so use a local copy of DelayedDllExportClasses.
SmallVector WorkList;
std::swap(DelayedDllExportClasses, WorkList);
for (CXXRecordDecl *Class : WorkList)
- ReferenceDllExportedMethods(*this, Class);
+ ReferenceDllExportedMembers(*this, Class);
}
}
Modified: cfe/trunk/test/CodeGenCXX/dllexport.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllexport.cpp?rev=324991&r1=324990&r2=324991&view=diff
==
--- cfe/trunk/test/CodeGenCXX/dllexport.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/dllexport.cpp Tue Feb 13 01:19:43 2018
@@ -28,6 +28,7 @@ struct External { int v; };
// The vftable for struct W is comdat largest because we have RTTI.
// M32-DAG: $"\01??_7W@@6B@" = comdat largest
+// M32-DAG: $"\01?smember@?$Base@H@PR32992@@0HA" = comdat any
//===--===//
@@ -977,3 +978,21 @@ class __declspec(dllexport) ACE_Service_
// MSVC2015-DAG: define weak_odr dllexport
{{.+}}ACE_Service_Object@@Q{{.+}}@$$Q
// The declarations should not be exported.
// MSVC2013-NOT: define weak_odr dllexport
{{.+}}ACE_Service_Object@@Q{{.+}}@$$Q
+
+namespace PR32992 {
+// Static data members of a instantiated base class should be exported.
+template
+class Base {
+ virtual void myfunc() {}
+ static int smember;
+};
+// MS-DAG: @"\01?smember@?$Base@H@PR32992@@0HA" = weak_odr dllexport global
i32 77, comdat, align 4
+template int Base::smember = 77;
+template
+class __declspec(dllexport) Derived2 : Base {
+ void myfunc() {}
+};
+class Derived : public Derived2 {
+ void myfunc() {}
+};
+} // namespace PR32992
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43197: [OpenMP] Add flag for linking runtime bitcode library
Hahnfeld added inline comments. Comment at: test/Driver/openmp-offload-gpu.c:150 +/// bitcode library that will be found via the LIBRARY_PATH. +// RUN: touch /tmp/libomptarget-nvptx-sm_60.bc +// RUN: LIBRARY_PATH=/tmp %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \ This should not be in `/tmp` but probably `%T`. Comment at: test/Driver/openmp-offload-gpu.c:151 +// RUN: touch /tmp/libomptarget-nvptx-sm_60.bc +// RUN: LIBRARY_PATH=/tmp %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \ +// RUN: -Xopenmp-target -march=sm_60 -fopenmp-relocatable-target -save-temps \ You may want to add `env` which should make this check portable because `lit` on Windows does the right thing then (I don't know if this test is run on Windows, it probably is) Repository: rC Clang https://reviews.llvm.org/D43197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown
Szelethus updated this revision to Diff 134005.
https://reviews.llvm.org/D43120
Files:
clang-tidy/misc/CMakeLists.txt
clang-tidy/misc/MiscTidyModule.cpp
clang-tidy/misc/ThrowKeywordMissingCheck.cpp
clang-tidy/misc/ThrowKeywordMissingCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/misc-throw-keyword-missing.rst
test/clang-tidy/misc-throw-keyword-missing.cpp
Index: test/clang-tidy/misc-throw-keyword-missing.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-throw-keyword-missing.cpp
@@ -0,0 +1,167 @@
+// RUN: %check_clang_tidy %s misc-throw-keyword-missing %t
+
+namespace std {
+
+// std::string declaration (taken from test/clang-tidy/readability-redundant-string-cstr-msvc.cpp).
+template
+class allocator {};
+template
+class char_traits {};
+template , typename A = std::allocator>
+struct basic_string {
+ basic_string();
+ basic_string(const basic_string &);
+ // MSVC headers define two constructors instead of using optional arguments.
+ basic_string(const C *);
+ basic_string(const C *, const A &);
+ ~basic_string();
+};
+typedef basic_string string;
+typedef basic_string wstring;
+
+// std::exception and std::runtime_error declaration.
+struct exception {
+ exception();
+ exception(const exception &other);
+ virtual ~exception();
+};
+
+struct runtime_error : public exception {
+ explicit runtime_error(const std::string &what_arg);
+};
+
+} // namespace std
+
+// The usage of this class should never emit a warning.
+struct RegularClass {};
+
+// Classname contains the substring "exception", in certain cases using this class should emit a warning.
+struct RegularException {
+ RegularException() {}
+
+ // Constructors with a single argument are treated differently (cxxFunctionalCastExpr).
+ RegularException(int) {}
+};
+
+// --
+
+void stdExceptionNotTrownTest(int i) {
+ if (i < 0)
+// CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception object created but not thrown; did you mean 'throw {{.*}}'? [misc-throw-keyword-missing]
+std::exception();
+
+ if (i > 0)
+// CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception
+std::runtime_error("Unexpected argument");
+}
+
+void stdExceptionThrownTest(int i) {
+ if (i < 0)
+throw std::exception();
+
+ if (i > 0)
+throw std::runtime_error("Unexpected argument");
+}
+
+void regularClassNotThrownTest(int i) {
+ if (i < 0)
+RegularClass();
+}
+
+void regularClassThrownTest(int i) {
+ if (i < 0)
+throw RegularClass();
+}
+
+void nameContainsExceptionNotThrownTest(int i) {
+ if (i < 0)
+// CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception
+RegularException();
+
+ if (i > 0)
+// CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception
+RegularException(5);
+}
+
+void nameContainsExceptionThrownTest(int i) {
+ if (i < 0)
+throw RegularException();
+
+ if (i > 0)
+throw RegularException(5);
+}
+
+template
+void f(int i, Exception excToBeThrown) {}
+
+void funcCallWithTempExcTest() {
+ f(5, RegularException());
+}
+
+// Global variable initilization test.
+RegularException exc = RegularException();
+RegularException *excptr = new RegularException();
+
+void localVariableInitTest() {
+ RegularException exc = RegularException();
+ RegularException *excptr = new RegularException();
+}
+
+class CtorInitializerListTest {
+ RegularException exc;
+
+ CtorInitializerListTest() : exc(RegularException()) {}
+
+ CtorInitializerListTest(int) try : exc(RegularException()) {
+// Constructor body
+ } catch (...) {
+// CHECK-MESSAGES: :[[@LINE+1]]:5: warning: suspicious exception
+RegularException();
+ }
+
+ CtorInitializerListTest(float);
+};
+
+CtorInitializerListTest::CtorInitializerListTest(float) try : exc(RegularException()) {
+ // Constructor body
+} catch (...) {
+ // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: suspicious exception
+ RegularException();
+}
+
+RegularException funcReturningExceptionTest(int i) {
+ return RegularException();
+}
+
+void returnedValueTest() {
+ funcReturningExceptionTest(3);
+}
+
+struct ClassBracedInitListTest {
+ ClassBracedInitListTest(RegularException exc) {}
+};
+
+void foo(RegularException, ClassBracedInitListTest) {}
+
+void bracedInitListTest() {
+ RegularException exc{};
+ ClassBracedInitListTest test = {RegularException()};
+ foo({}, {RegularException()});
+}
+
+typedef std::exception ERROR_BASE;
+class RegularError : public ERROR_BASE {};
+
+void typedefTest() {
+ // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: suspicious exception
+ RegularError();
+}
+
+struct ExceptionRAII {
+ ExceptionRAII() {}
+ ~ExceptionRAII() {}
+};
+
+void exceptionRAIITest() {
+ ExceptionRAII E;
+}
Index: docs/clang-tidy/checks/misc-throw-keyword-missing.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-throw-keyword-missing.rst
@@ -0
[PATCH] D43223: [clang-tidy] Update fuchsia-multiple-inheritance to not fail
hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D43223 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch
djasper added a comment. Do you have a reference to style guides recommending any of this? Repository: rC Clang https://reviews.llvm.org/D43183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r324992 - [clangd] SymbolLocation only covers symbol name.
Author: hokein
Date: Tue Feb 13 01:53:50 2018
New Revision: 324992
URL: http://llvm.org/viewvc/llvm-project?rev=324992&view=rev
Log:
[clangd] SymbolLocation only covers symbol name.
Summary:
* Change the offset range to half-open, [start, end).
* Fix a few fixmes.
Reviewers: sammccall
Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D43182
Modified:
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=324992&r1=324991&r2=324992&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Tue Feb 13 01:53:50 2018
@@ -19,7 +19,7 @@ using namespace llvm;
raw_ostream &operator<<(raw_ostream &OS, const SymbolLocation &L) {
if (!L)
return OS << "(none)";
- return OS << L.FileURI << "[" << L.StartOffset << "-" << L.EndOffset << "]";
+ return OS << L.FileURI << "[" << L.StartOffset << "-" << L.EndOffset << ")";
}
SymbolID::SymbolID(StringRef USR)
Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=324992&r1=324991&r2=324992&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Tue Feb 13 01:53:50 2018
@@ -11,6 +11,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_INDEX_H
#include "clang/Index/IndexSymbol.h"
+#include "clang/Lex/Lexer.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/Hashing.h"
@@ -25,11 +26,9 @@ namespace clangd {
struct SymbolLocation {
// The URI of the source file where a symbol occurs.
llvm::StringRef FileURI;
- // The 0-based offset to the first character of the symbol from the beginning
- // of the source file.
+ // The 0-based offsets of the symbol from the beginning of the source file,
+ // using half-open range, [StartOffset, EndOffset).
unsigned StartOffset = 0;
- // The 0-based offset to the last character of the symbol from the beginning
- // of the source file.
unsigned EndOffset = 0;
operator bool() const { return !FileURI.empty(); }
@@ -121,9 +120,10 @@ struct Symbol {
// The containing namespace. e.g. "" (global), "ns::" (top-level namespace).
llvm::StringRef Scope;
// The location of the symbol's definition, if one was found.
- // This covers the whole definition (e.g. class body).
+ // This just covers the symbol name (e.g. without class/function body).
SymbolLocation Definition;
// The location of the preferred declaration of the symbol.
+ // This just covers the symbol name.
// This may be the same as Definition.
//
// A C++ symbol may have multiple declarations, and we pick one to prefer.
Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=324992&r1=324991&r2=324992&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Tue Feb 13
01:53:50 2018
@@ -132,21 +132,14 @@ bool shouldFilterDecl(const NamedDecl *N
// For symbols defined inside macros:
// * use expansion location, if the symbol is formed via macro concatenation.
// * use spelling location, otherwise.
-//
-// FIXME: EndOffset is inclusive (closed range), and should be exclusive.
-// FIXME: Because the underlying ranges are token ranges, this code chops the
-//last token in half if it contains multiple characters.
-// FIXME: We probably want to get just the location of the symbol name, not
-//the whole e.g. class.
llvm::Optional
getSymbolLocation(const NamedDecl &D, SourceManager &SM,
const SymbolCollector::Options &Opts,
+ const clang::LangOptions& LangOpts,
std::string &FileURIStorage) {
- SourceLocation Loc = D.getLocation();
- SourceLocation StartLoc = SM.getSpellingLoc(D.getLocStart());
- SourceLocation EndLoc = SM.getSpellingLoc(D.getLocEnd());
- if (Loc.isMacroID()) {
-std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM);
+ SourceLocation SpellingLoc = SM.getSpellingLoc(D.getLocation());
+ if (D.getLocation().isMacroID()) {
+std::string PrintLoc = SpellingLoc.printToString(SM);
if (llvm::StringRef(PrintLoc).startswith("")) {
// We use the expan
[PATCH] D42983: [clang-tidy] Add readability-simd-intrinsics check.
hokein added inline comments.
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:46
+
+ static const llvm::StringMap Mapping{
+// [simd.alg]
consider using `llvm::StringSwitch`?
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:102
+ } else {
+return;
+ }
nit: use early return for readability.
Comment at: docs/clang-tidy/checks/readability-simd-intrinsics.rst:38
+
+ If set to non-zero, the check will be enabled and it will suggest
``std::experimental`` alternatives. Default is ``0``.
+
We don't use check's option to enable the check in clang-tidy, if users want to
enable the check, they need to pass the check explicitly to clang-tidy (e.g.
-checks=""). I'd suggest to remove this option.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43182: [clangd] SymbolLocation only covers symbol name.
This revision was automatically updated to reflect the committed changes.
hokein marked 2 inline comments as done.
Closed by commit rL324992: [clangd] SymbolLocation only covers symbol name.
(authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D43182?vs=133839&id=134007#toc
Repository:
rL LLVM
https://reviews.llvm.org/D43182
Files:
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
@@ -48,15 +48,12 @@
MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; }
MATCHER_P(DeclRange, Offsets, "") {
- // Offset range in SymbolLocation is [start, end] while in Clangd is [start,
- // end).
- // FIXME: SymbolLocation should be [start, end).
return arg.CanonicalDeclaration.StartOffset == Offsets.first &&
- arg.CanonicalDeclaration.EndOffset == Offsets.second - 1;
+ arg.CanonicalDeclaration.EndOffset == Offsets.second;
}
MATCHER_P(DefRange, Offsets, "") {
return arg.Definition.StartOffset == Offsets.first &&
- arg.Definition.EndOffset == Offsets.second - 1;
+ arg.Definition.EndOffset == Offsets.second;
}
namespace clang {
@@ -177,25 +174,23 @@
}
TEST_F(SymbolCollectorTest, Locations) {
- // FIXME: the behavior here is bad: chopping tokens, including more than the
- // ident range, using half-open ranges. See fixmes in getSymbolLocation().
CollectorOpts.IndexMainFiles = true;
Annotations Header(R"cpp(
// Declared in header, defined in main.
-$xdecl[[extern int X]];
-$clsdecl[[class C]]ls;
-$printdecl[[void print()]];
+extern int $xdecl[[X]];
+class $clsdecl[[Cls]];
+void $printdecl[[print]]();
// Declared in header, defined nowhere.
-$zdecl[[extern int Z]];
+extern int $zdecl[[Z]];
)cpp");
Annotations Main(R"cpp(
-$xdef[[int X = 4]]2;
-$clsdef[[class Cls {}]];
-$printdef[[void print() {}]]
+int $xdef[[X]] = 42;
+class $clsdef[[Cls]] {};
+void $printdef[[print]]() {}
// Declared/defined in main only.
-$y[[int Y]];
+int $y[[Y]];
)cpp");
runSymbolCollector(Header.code(), Main.code());
EXPECT_THAT(
@@ -304,10 +299,10 @@
#define FF(name) \
class name##_Test {};
-$expansion[[FF(abc)]];
+$expansion[[FF]](abc);
#define FF2() \
- $spelling[[class Test {}]];
+ class $spelling[[Test]] {};
FF2();
)");
@@ -329,10 +324,10 @@
#define FF(name) \
class name##_Test {};
-$expansion[[FF(abc)]];
+$expansion[[FF]](abc);
#define FF2() \
- $spelling[[class Test {}]];
+ class $spelling[[Test]] {};
FF2();
)");
@@ -351,7 +346,7 @@
Annotations Header(R"(
#ifdef NAME
-$expansion[[class NAME {}]];
+class $expansion[[NAME]] {};
#endif
)");
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
@@ -132,42 +132,35 @@
// For symbols defined inside macros:
// * use expansion location, if the symbol is formed via macro concatenation.
// * use spelling location, otherwise.
-//
-// FIXME: EndOffset is inclusive (closed range), and should be exclusive.
-// FIXME: Because the underlying ranges are token ranges, this code chops the
-//last token in half if it contains multiple characters.
-// FIXME: We probably want to get just the location of the symbol name, not
-//the whole e.g. class.
llvm::Optional
getSymbolLocation(const NamedDecl &D, SourceManager &SM,
const SymbolCollector::Options &Opts,
+ const clang::LangOptions& LangOpts,
std::string &FileURIStorage) {
- SourceLocation Loc = D.getLocation();
- SourceLocation StartLoc = SM.getSpellingLoc(D.getLocStart());
- SourceLocation EndLoc = SM.getSpellingLoc(D.getLocEnd());
- if (Loc.isMacroID()) {
-std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM);
+ SourceLocation SpellingLoc = SM.getSpellingLoc(D.getLocation());
+ if (D.getLocation().isMacroID()) {
+std::string PrintLoc = SpellingLoc.printToString(SM);
if (llvm::StringRef(PrintLoc).startswith("")) {
// We use the expansion location for the following symbols, as spelling
// locations of these symbols are not interesting to us:
// *
[PATCH] D43182: [clangd] SymbolLocation only covers symbol name.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE324992: [clangd] SymbolLocation only covers symbol name.
(authored by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D43182?vs=133839&id=134008#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43182
Files:
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -48,15 +48,12 @@
MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; }
MATCHER_P(DeclRange, Offsets, "") {
- // Offset range in SymbolLocation is [start, end] while in Clangd is [start,
- // end).
- // FIXME: SymbolLocation should be [start, end).
return arg.CanonicalDeclaration.StartOffset == Offsets.first &&
- arg.CanonicalDeclaration.EndOffset == Offsets.second - 1;
+ arg.CanonicalDeclaration.EndOffset == Offsets.second;
}
MATCHER_P(DefRange, Offsets, "") {
return arg.Definition.StartOffset == Offsets.first &&
- arg.Definition.EndOffset == Offsets.second - 1;
+ arg.Definition.EndOffset == Offsets.second;
}
namespace clang {
@@ -177,25 +174,23 @@
}
TEST_F(SymbolCollectorTest, Locations) {
- // FIXME: the behavior here is bad: chopping tokens, including more than the
- // ident range, using half-open ranges. See fixmes in getSymbolLocation().
CollectorOpts.IndexMainFiles = true;
Annotations Header(R"cpp(
// Declared in header, defined in main.
-$xdecl[[extern int X]];
-$clsdecl[[class C]]ls;
-$printdecl[[void print()]];
+extern int $xdecl[[X]];
+class $clsdecl[[Cls]];
+void $printdecl[[print]]();
// Declared in header, defined nowhere.
-$zdecl[[extern int Z]];
+extern int $zdecl[[Z]];
)cpp");
Annotations Main(R"cpp(
-$xdef[[int X = 4]]2;
-$clsdef[[class Cls {}]];
-$printdef[[void print() {}]]
+int $xdef[[X]] = 42;
+class $clsdef[[Cls]] {};
+void $printdef[[print]]() {}
// Declared/defined in main only.
-$y[[int Y]];
+int $y[[Y]];
)cpp");
runSymbolCollector(Header.code(), Main.code());
EXPECT_THAT(
@@ -304,10 +299,10 @@
#define FF(name) \
class name##_Test {};
-$expansion[[FF(abc)]];
+$expansion[[FF]](abc);
#define FF2() \
- $spelling[[class Test {}]];
+ class $spelling[[Test]] {};
FF2();
)");
@@ -329,10 +324,10 @@
#define FF(name) \
class name##_Test {};
-$expansion[[FF(abc)]];
+$expansion[[FF]](abc);
#define FF2() \
- $spelling[[class Test {}]];
+ class $spelling[[Test]] {};
FF2();
)");
@@ -351,7 +346,7 @@
Annotations Header(R"(
#ifdef NAME
-$expansion[[class NAME {}]];
+class $expansion[[NAME]] {};
#endif
)");
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -132,42 +132,35 @@
// For symbols defined inside macros:
// * use expansion location, if the symbol is formed via macro concatenation.
// * use spelling location, otherwise.
-//
-// FIXME: EndOffset is inclusive (closed range), and should be exclusive.
-// FIXME: Because the underlying ranges are token ranges, this code chops the
-//last token in half if it contains multiple characters.
-// FIXME: We probably want to get just the location of the symbol name, not
-//the whole e.g. class.
llvm::Optional
getSymbolLocation(const NamedDecl &D, SourceManager &SM,
const SymbolCollector::Options &Opts,
+ const clang::LangOptions& LangOpts,
std::string &FileURIStorage) {
- SourceLocation Loc = D.getLocation();
- SourceLocation StartLoc = SM.getSpellingLoc(D.getLocStart());
- SourceLocation EndLoc = SM.getSpellingLoc(D.getLocEnd());
- if (Loc.isMacroID()) {
-std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM);
+ SourceLocation SpellingLoc = SM.getSpellingLoc(D.getLocation());
+ if (D.getLocation().isMacroID()) {
+std::string PrintLoc = SpellingLoc.printToString(SM);
if (llvm::StringRef(PrintLoc).startswith("")) {
// We use the expansion location for the following symbols, as spelling
// locations of these symbols are not interesting to us:
// * symbols formed via macro concatenation, the spelling location will
// be ""
// * symbols controlled and defined by a compile command-line option
// `-DName=foo`, the spelling location will be "".
- StartLoc = SM.getExpansionRange(D.getLocStart()).first;
- EndLoc =
[clang-tools-extra] r324993 - [clangd] Remove an already-done FIXME, NFC
Author: hokein
Date: Tue Feb 13 01:56:45 2018
New Revision: 324993
URL: http://llvm.org/viewvc/llvm-project?rev=324993&view=rev
Log:
[clangd] Remove an already-done FIXME, NFC
Modified:
clang-tools-extra/trunk/clangd/index/Index.h
Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=324993&r1=324992&r2=324993&view=diff
==
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Tue Feb 13 01:56:45 2018
@@ -162,7 +162,6 @@ struct Symbol {
// Optional details of the symbol.
const Details *Detail = nullptr;
- // FIXME: add definition location of the symbol.
// FIXME: add all occurrences support.
// FIXME: add extra fields for index scoring signals.
};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r324995 - [clang-format] Support text proto extensions
Author: krasimir
Date: Tue Feb 13 02:20:39 2018
New Revision: 324995
URL: http://llvm.org/viewvc/llvm-project?rev=324995&view=rev
Log:
[clang-format] Support text proto extensions
Summary:
This adds support for text proto extensions, like:
```
msg {
[type.type/ext] {
key: value
}
}
```
Reviewers: djasper
Reviewed By: djasper
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D43180
Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestProto.cpp
cfe/trunk/unittests/Format/FormatTestTextProto.cpp
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=324995&r1=324994&r2=324995&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Tue Feb 13 02:20:39 2018
@@ -943,6 +943,8 @@ unsigned ContinuationIndenter::getNewLin
if (Previous.is(tok::r_paren) && !Current.isBinaryOperator() &&
!Current.isOneOf(tok::colon, tok::comment))
return ContinuationIndent;
+ if (Current.is(TT_ProtoExtensionLSquare))
+return State.Stack.back().Indent;
if (State.Stack.back().Indent == State.FirstIndent && PreviousNonComment &&
PreviousNonComment->isNot(tok::r_brace))
// Ensure that we fall back to the continuation indent width instead of
Modified: cfe/trunk/lib/Format/FormatToken.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=324995&r1=324994&r2=324995&view=diff
==
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Tue Feb 13 02:20:39 2018
@@ -88,6 +88,7 @@ namespace format {
TYPE(TemplateCloser)
\
TYPE(TemplateOpener)
\
TYPE(TemplateString)
\
+ TYPE(ProtoExtensionLSquare)
\
TYPE(TrailingAnnotation)
\
TYPE(TrailingReturnArrow)
\
TYPE(TrailingUnaryOperator)
\
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=324995&r1=324994&r2=324995&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Feb 13 02:20:39 2018
@@ -368,12 +368,35 @@ private:
Parent->is(TT_TemplateCloser)) {
Left->Type = TT_ArraySubscriptLSquare;
} else if (Style.Language == FormatStyle::LK_Proto ||
- (!CppArrayTemplates && Parent &&
- Parent->isOneOf(TT_BinaryOperator, TT_TemplateCloser,
tok::at,
- tok::comma, tok::l_paren, tok::l_square,
- tok::question, tok::colon, tok::kw_return,
- // Should only be relevant to JavaScript:
- tok::kw_default))) {
+ Style.Language == FormatStyle::LK_TextProto) {
+// Square braces in LK_Proto can either be message field attributes:
+//
+// optional Aaa aaa = 1 [
+// (aaa) = aaa
+// ];
+//
+// or text proto extensions (in options):
+//
+// option (Aaa.options) = {
+// [type.type/type] {
+// key: value
+// }
+// }
+//
+// In the first case we want to spread the contents inside the square
+// braces; in the second we want to keep them inline.
+Left->Type = TT_ArrayInitializerLSquare;
+if (!Left->endsSequence(tok::l_square, tok::numeric_constant,
+tok::equal)) {
+ Left->Type = TT_ProtoExtensionLSquare;
+ BindingIncrease = 10;
+}
+ } else if (!CppArrayTemplates && Parent &&
+ Parent->isOneOf(TT_BinaryOperator, TT_TemplateCloser, tok::at,
+ tok::comma, tok::l_paren, tok::l_square,
+ tok::question, tok::colon, tok::kw_return,
+ // Should only be relevant to JavaScript:
+ tok::kw_default)) {
Left->Type = TT_ArrayInitializerLSquare;
} else {
BindingIncrease = 10;
@@ -2396,6 +2419,12 @@ bool TokenAnnotator::spaceRequiredBefore
return true;
if (Rig
[PATCH] D39074: [libunwind][MIPS]: Add support for unwinding in N32 processes.
sdardis added a comment. Yes. Something like LIBUNWIND_TEST_CFLAGS mapping to config.test_cflags, which libunwind/test/libunwind/test/config.py then appends when building the tests. Thanks. Repository: rUNW libunwind https://reviews.llvm.org/D39074 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43180: [clang-format] Support text proto extensions
This revision was automatically updated to reflect the committed changes.
Closed by commit rC324995: [clang-format] Support text proto extensions
(authored by krasimir, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D43180?vs=133872&id=134013#toc
Repository:
rC Clang
https://reviews.llvm.org/D43180
Files:
lib/Format/ContinuationIndenter.cpp
lib/Format/FormatToken.h
lib/Format/TokenAnnotator.cpp
unittests/Format/FormatTestProto.cpp
unittests/Format/FormatTestTextProto.cpp
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -368,12 +368,35 @@
Parent->is(TT_TemplateCloser)) {
Left->Type = TT_ArraySubscriptLSquare;
} else if (Style.Language == FormatStyle::LK_Proto ||
- (!CppArrayTemplates && Parent &&
- Parent->isOneOf(TT_BinaryOperator, TT_TemplateCloser, tok::at,
- tok::comma, tok::l_paren, tok::l_square,
- tok::question, tok::colon, tok::kw_return,
- // Should only be relevant to JavaScript:
- tok::kw_default))) {
+ Style.Language == FormatStyle::LK_TextProto) {
+// Square braces in LK_Proto can either be message field attributes:
+//
+// optional Aaa aaa = 1 [
+// (aaa) = aaa
+// ];
+//
+// or text proto extensions (in options):
+//
+// option (Aaa.options) = {
+// [type.type/type] {
+// key: value
+// }
+// }
+//
+// In the first case we want to spread the contents inside the square
+// braces; in the second we want to keep them inline.
+Left->Type = TT_ArrayInitializerLSquare;
+if (!Left->endsSequence(tok::l_square, tok::numeric_constant,
+tok::equal)) {
+ Left->Type = TT_ProtoExtensionLSquare;
+ BindingIncrease = 10;
+}
+ } else if (!CppArrayTemplates && Parent &&
+ Parent->isOneOf(TT_BinaryOperator, TT_TemplateCloser, tok::at,
+ tok::comma, tok::l_paren, tok::l_square,
+ tok::question, tok::colon, tok::kw_return,
+ // Should only be relevant to JavaScript:
+ tok::kw_default)) {
Left->Type = TT_ArrayInitializerLSquare;
} else {
BindingIncrease = 10;
@@ -2396,6 +2419,12 @@
return true;
if (Right.isOneOf(tok::l_brace, tok::less) && Left.is(TT_SelectorName))
return true;
+// Slashes occur in text protocol extension syntax: [type/type] { ... }.
+if (Left.is(tok::slash) || Right.is(tok::slash))
+ return false;
+if (Left.MatchingParen && Left.MatchingParen->is(TT_ProtoExtensionLSquare) &&
+Right.isOneOf(tok::l_brace, tok::less))
+ return !Style.Cpp11BracedListStyle;
} else if (Style.Language == FormatStyle::LK_JavaScript) {
if (Left.is(TT_JsFatArrow))
return true;
@@ -2732,6 +2761,9 @@
(Line.Last->is(tok::l_brace) || Style.BreakAfterJavaFieldAnnotations))
return true;
+ if (Right.is(TT_ProtoExtensionLSquare))
+return true;
+
return false;
}
Index: lib/Format/ContinuationIndenter.cpp
===
--- lib/Format/ContinuationIndenter.cpp
+++ lib/Format/ContinuationIndenter.cpp
@@ -943,6 +943,8 @@
if (Previous.is(tok::r_paren) && !Current.isBinaryOperator() &&
!Current.isOneOf(tok::colon, tok::comment))
return ContinuationIndent;
+ if (Current.is(TT_ProtoExtensionLSquare))
+return State.Stack.back().Indent;
if (State.Stack.back().Indent == State.FirstIndent && PreviousNonComment &&
PreviousNonComment->isNot(tok::r_brace))
// Ensure that we fall back to the continuation indent width instead of
Index: lib/Format/FormatToken.h
===
--- lib/Format/FormatToken.h
+++ lib/Format/FormatToken.h
@@ -88,6 +88,7 @@
TYPE(TemplateCloser) \
TYPE(TemplateOpener) \
TYPE(TemplateString) \
+ TYPE(ProtoExtensionLSquare) \
TYPE(TrailingAnnotation) \
TYPE(TrailingReturnArrow)\
TYPE(TrailingUnaryOperator) \
Index: unittests/Format/FormatTestTextProto.cpp
===
--- unittests/Format/FormatTestTextProto.cpp
+
[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based
ilya-biryukov created this revision.
ilya-biryukov added reviewers: sammccall, ioeric, hokein.
Herald added subscribers: jkorous-apple, klimek.
As a consequence, all LSP operations are now handled asynchronously,
i.e. they never block the main processing thread. However, if
-run-synchronously flag is specified, clangd still runs everything on
the main thread.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43227
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
unittests/clangd/ClangdTests.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SyncAPI.cpp
unittests/clangd/SyncAPI.h
Index: unittests/clangd/SyncAPI.h
===
--- unittests/clangd/SyncAPI.h
+++ unittests/clangd/SyncAPI.h
@@ -22,6 +22,22 @@
runCodeComplete(ClangdServer &Server, PathRef File, Position Pos,
clangd::CodeCompleteOptions Opts,
llvm::Optional OverridenContents = llvm::None);
+
+llvm::Expected>
+runSignatureHelp(ClangdServer &Server, PathRef File, Position Pos,
+ llvm::Optional OverridenContents = llvm::None);
+
+llvm::Expected>>
+runFindDefinitions(ClangdServer &Server, PathRef File, Position Pos);
+
+llvm::Expected>>
+runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos);
+
+llvm::Expected>
+runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName);
+
+std::string runDumpAST(ClangdServer &Server, PathRef File);
+
} // namespace clangd
} // namespace clang
Index: unittests/clangd/SyncAPI.cpp
===
--- unittests/clangd/SyncAPI.cpp
+++ unittests/clangd/SyncAPI.cpp
@@ -62,5 +62,47 @@
return Result;
}
+llvm::Expected>
+runSignatureHelp(ClangdServer &Server, PathRef File, Position Pos,
+ llvm::Optional OverridenContents) {
+ llvm::Expected> Result = Tagged();
+ (void)(bool)Result; // Expected has to be checked.
+ Server.signatureHelp(File, Pos, capture(Result), OverridenContents);
+ return Result;
+}
+
+llvm::Expected>>
+runFindDefinitions(ClangdServer &Server, PathRef File, Position Pos) {
+ llvm::Expected>> Result =
+ Tagged>();
+ (void)(bool)Result; // Expected has to be checked.
+ Server.findDefinitions(File, Pos, capture(Result));
+ return Result;
+}
+
+llvm::Expected>>
+runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos) {
+ llvm::Expected>> Result =
+ Tagged>();
+ (void)(bool)Result; // Expected has to be checked.
+ Server.findDocumentHighlights(File, Pos, capture(Result));
+ return Result;
+}
+
+llvm::Expected>
+runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName) {
+ llvm::Expected> Result =
+ std::vector();
+ (void)(bool)Result; // Expected has to be checked.
+ Server.rename(File, Pos, NewName, capture(Result));
+ return Result;
+}
+
+std::string runDumpAST(ClangdServer &Server, PathRef File) {
+ std::string Result;
+ Server.dumpAST(File, capture(Result));
+ return Result;
+}
+
} // namespace clangd
} // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -626,7 +626,7 @@
Annotations Test(Text);
Server.addDocument(File, Test.code());
EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
- auto R = Server.signatureHelp(File, Test.point());
+ auto R = runSignatureHelp(Server, File, Test.point());
assert(R);
return R.get().Value;
}
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -112,7 +112,7 @@
}
std::string dumpASTWithoutMemoryLocs(ClangdServer &Server, PathRef File) {
- auto DumpWithMemLocs = Server.dumpAST(File);
+ auto DumpWithMemLocs = runDumpAST(Server, File);
return replacePtrsInDump(DumpWithMemLocs);
}
@@ -450,17 +450,17 @@
// Clang can't parse command args in that case, but we shouldn't crash.
Server.addDocument(FooCpp, "int main() {}");
- EXPECT_EQ(Server.dumpAST(FooCpp), "");
- EXPECT_ERROR(Server.findDefinitions(FooCpp, Position{0, 0}));
- EXPECT_ERROR(Server.findDocumentHighlights(FooCpp, Position{0, 0}));
- EXPECT_ERROR(Server.rename(FooCpp, Position{0, 0}, "new_name"));
+ EXPECT_EQ(runDumpAST(Server, FooCpp), "");
+ EXPECT_ERROR(runFindDefinitions(Server, FooCpp, Position{0, 0}));
+ EXPECT_ERROR(runFindDocumentHighlights(Server, FooCpp, Position{0, 0}));
+ EXPECT_ERROR(runRename(Server, FooCpp, Position{0, 0}, "new_name"));
// FIXME: codeComplete and signatureHelp should also return errors when they
// can't parse the file.
EXPECT_THAT(runCodeComplete(Server, FooCpp, Position{0, 0},
clangd::CodeCompleteOptions())
[PATCH] D43229: [clangd] Enable snippet completion based on client capabilities.
ilya-biryukov created this revision.
ilya-biryukov added reviewers: hokein, ioeric, sammccall.
Herald added subscribers: jkorous-apple, klimek.
And remove -enable-snippets flag.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43229
Files:
clangd/ClangdLSPServer.cpp
clangd/Protocol.cpp
clangd/Protocol.h
clangd/tool/ClangdMain.cpp
test/clangd/completion-snippets-disabled.test
test/clangd/completion-snippets-enabled.test
test/clangd/completion-snippets-missing.test
Index: test/clangd/completion-snippets-missing.test
===
--- /dev/null
+++ test/clangd/completion-snippets-missing.test
@@ -0,0 +1,35 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+# RUN: clangd -lit-test -pch-storage=memory < %s | FileCheck -strict-whitespace %s
+{
+ "jsonrpc": "2.0",
+ "id": 0,
+ "method": "initialize",
+ "params": {
+"processId": 123,
+"rootPath": "clangd",
+"capabilities": {},
+"trace": "off"
+ }
+}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"int func_with_args(int a, int b);\nint main() {\nfunc_with\n}"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":7}}}
+# CHECK: "id": 1
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": {
+# CHECK-NEXT:"isIncomplete": {{.*}}
+# CHECK-NEXT:"items": [
+# CHECK-NEXT:{
+# CHECK-NEXT: "detail": "int",
+# CHECK-NEXT: "filterText": "func_with_args",
+# CHECK-NEXT: "insertText": "func_with_args",
+# CHECK-NEXT: "insertTextFormat": 1,
+# CHECK-NEXT: "kind": 3,
+# CHECK-NEXT: "label": "func_with_args(int a, int b)",
+# CHECK-NEXT: "sortText": "{{.*}}func_with_args"
+# CHECK-NEXT:}
+# CHECK-NEXT:]
+# CHECK-NEXT: }
+---
+{"jsonrpc":"2.0","id":4,"method":"shutdown"}
Index: test/clangd/completion-snippets-enabled.test
===
--- /dev/null
+++ test/clangd/completion-snippets-enabled.test
@@ -0,0 +1,43 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+# RUN: clangd -lit-test -pch-storage=memory < %s | FileCheck -strict-whitespace %s
+{
+ "jsonrpc": "2.0",
+ "id": 0,
+ "method": "initialize",
+ "params": {
+"processId": 123,
+"rootPath": "clangd",
+"capabilities": {
+ "textDocument": {
+"completion": {
+ "completionItem": {
+"snippetSupport": true
+ }
+}
+ }
+},
+"trace": "off"
+ }
+}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"int func_with_args(int a, int b);\nint main() {\nfunc_with\n}"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":7}}}
+# CHECK: "id": 1
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": {
+# CHECK-NEXT:"isIncomplete": {{.*}}
+# CHECK-NEXT:"items": [
+# CHECK-NEXT:{
+# CHECK-NEXT: "detail": "int",
+# CHECK-NEXT: "filterText": "func_with_args",
+# CHECK-NEXT: "insertText": "func_with_args(${1:int a}, ${2:int b})",
+# CHECK-NEXT: "insertTextFormat": 2,
+# CHECK-NEXT: "kind": 3,
+# CHECK-NEXT: "label": "func_with_args(int a, int b)",
+# CHECK-NEXT: "sortText": "{{.*}}func_with_args"
+# CHECK-NEXT:}
+# CHECK-NEXT:]
+# CHECK-NEXT: }
+---
+{"jsonrpc":"2.0","id":4,"method":"shutdown"}
Index: test/clangd/completion-snippets-disabled.test
===
--- /dev/null
+++ test/clangd/completion-snippets-disabled.test
@@ -0,0 +1,43 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+# RUN: clangd -lit-test -pch-storage=memory < %s | FileCheck -strict-whitespace %s
+{
+ "jsonrpc": "2.0",
+ "id": 0,
+ "method": "initialize",
+ "params": {
+"processId": 123,
+"rootPath": "clangd",
+"capabilities": {
+ "textDocument": {
+"completion": {
+ "completionItem": {
+"snippetSupport": false
+ }
+}
+ }
+},
+"trace": "off"
+ }
+}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"int func_with_args(int a, int b);\nint main() {\nfunc_with\n}"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/completion","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":2,"character":7}}}
+# CHECK: "id": 1
+# CHECK-NEXT: "jsonrpc": "2.0",
+# CHECK-NEXT: "result": {
+# CHECK-NEXT:"isIncomplete": {{.*}}
+# CHECK-NEXT:"items": [
+# CHECK-NEXT:{
+# CHECK-NEXT: "detail": "int",
+# CHECK-NEXT: "filterT
[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h
ilya-biryukov created this revision.
ilya-biryukov added reviewers: hokein, ioeric, sammccall.
Herald added subscribers: jkorous-apple, klimek.
Some of the existing structs had primimtive fields that were
not explicitly initialized on construction.
After this commit every struct consistently sets a defined value for
every field when default-initialized.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43230
Files:
clangd/Protocol.h
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -77,11 +77,14 @@
bool fromJSON(const json::Expr &, TextDocumentIdentifier &);
struct Position {
+ Position() = default;
+ Position(int line, int character) : line(line), character(character) {}
+
/// Line position in a document (zero-based).
- int line;
+ int line = 0;
/// Character offset on a line in a document (zero-based).
- int character;
+ int character = 0;
friend bool operator==(const Position &LHS, const Position &RHS) {
return std::tie(LHS.line, LHS.character) ==
@@ -160,7 +163,7 @@
std::string languageId;
/// The version number of this document (it will strictly increase after each
- int version;
+ int version = 0;
/// The content of the opened text document.
std::string text;
@@ -184,7 +187,7 @@
/// the server. Is null if the process has not been started by another
/// process. If the parent process is not alive then the server should exit
/// (see exit notification) its process.
- llvm::Optional processId;
+ llvm::Optional processId = 0;
/// The rootPath of the workspace. Is null
/// if no folder is open.
@@ -205,7 +208,7 @@
// ClientCapabilities capabilities;
/// The initial trace setting. If omitted trace is disabled ('off').
- llvm::Optional trace;
+ llvm::Optional trace = TraceLevel::Off;
};
bool fromJSON(const json::Expr &, InitializeParams &);
@@ -255,7 +258,7 @@
/// The file's URI.
URIForFile uri;
/// The change type.
- FileChangeType type;
+ FileChangeType type = FileChangeType::Created;
};
bool fromJSON(const json::Expr &, FileEvent &);
@@ -267,10 +270,10 @@
struct FormattingOptions {
/// Size of a tab in spaces.
- int tabSize;
+ int tabSize = 0;
/// Prefer spaces over tabs.
- bool insertSpaces;
+ bool insertSpaces = false;
};
bool fromJSON(const json::Expr &, FormattingOptions &);
json::Expr toJSON(const FormattingOptions &);
@@ -317,7 +320,7 @@
/// The diagnostic's severity. Can be omitted. If omitted it is up to the
/// client to interpret diagnostics as error, warning, info or hint.
- int severity;
+ int severity = 0;
/// The diagnostic's code. Can be omitted.
/// Note: Not currently used by clangd
@@ -388,7 +391,6 @@
std::string command;
// Arguments
-
llvm::Optional workspaceEdit;
};
bool fromJSON(const json::Expr &, ExecuteCommandParams &);
@@ -453,10 +455,13 @@
/// user keeps typing.
/// This is a clangd extension.
struct CompletionItemScores {
- float finalScore; /// The score that items are ranked by.
- /// This is filterScore * symbolScore.
- float filterScore; /// How the partial identifier matched filterText. [0-1]
- float symbolScore; /// How the symbol fits, ignoring the partial identifier.
+ /// The score that items are ranked by.
+ /// This is filterScore * symbolScore.
+ float finalScore = 0.f;
+ /// How the partial identifier matched filterText. [0-1]
+ float filterScore = 0.f;
+ /// How the symbol fits, ignoring the partial identifier.
+ float symbolScore = 0.f;
};
struct CompletionItem {
@@ -588,13 +593,10 @@
/// the background color of its range.
struct DocumentHighlight {
-
/// The range this highlight applies to.
-
Range range;
/// The highlight kind, default is DocumentHighlightKind.Text.
-
DocumentHighlightKind kind = DocumentHighlightKind::Text;
friend bool operator<(const DocumentHighlight &LHS,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h
ilya-biryukov updated this revision to Diff 134019.
ilya-biryukov added a comment.
- Removed initializers for Optional<> fields, they are not problematic
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43230
Files:
clangd/Protocol.h
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -77,11 +77,14 @@
bool fromJSON(const json::Expr &, TextDocumentIdentifier &);
struct Position {
+ Position() = default;
+ Position(int line, int character) : line(line), character(character) {}
+
/// Line position in a document (zero-based).
- int line;
+ int line = 0;
/// Character offset on a line in a document (zero-based).
- int character;
+ int character = 0;
friend bool operator==(const Position &LHS, const Position &RHS) {
return std::tie(LHS.line, LHS.character) ==
@@ -160,7 +163,7 @@
std::string languageId;
/// The version number of this document (it will strictly increase after each
- int version;
+ int version = 0;
/// The content of the opened text document.
std::string text;
@@ -255,7 +258,7 @@
/// The file's URI.
URIForFile uri;
/// The change type.
- FileChangeType type;
+ FileChangeType type = FileChangeType::Created;
};
bool fromJSON(const json::Expr &, FileEvent &);
@@ -267,10 +270,10 @@
struct FormattingOptions {
/// Size of a tab in spaces.
- int tabSize;
+ int tabSize = 0;
/// Prefer spaces over tabs.
- bool insertSpaces;
+ bool insertSpaces = false;
};
bool fromJSON(const json::Expr &, FormattingOptions &);
json::Expr toJSON(const FormattingOptions &);
@@ -317,7 +320,7 @@
/// The diagnostic's severity. Can be omitted. If omitted it is up to the
/// client to interpret diagnostics as error, warning, info or hint.
- int severity;
+ int severity = 0;
/// The diagnostic's code. Can be omitted.
/// Note: Not currently used by clangd
@@ -388,7 +391,6 @@
std::string command;
// Arguments
-
llvm::Optional workspaceEdit;
};
bool fromJSON(const json::Expr &, ExecuteCommandParams &);
@@ -453,10 +455,13 @@
/// user keeps typing.
/// This is a clangd extension.
struct CompletionItemScores {
- float finalScore; /// The score that items are ranked by.
- /// This is filterScore * symbolScore.
- float filterScore; /// How the partial identifier matched filterText. [0-1]
- float symbolScore; /// How the symbol fits, ignoring the partial identifier.
+ /// The score that items are ranked by.
+ /// This is filterScore * symbolScore.
+ float finalScore = 0.f;
+ /// How the partial identifier matched filterText. [0-1]
+ float filterScore = 0.f;
+ /// How the symbol fits, ignoring the partial identifier.
+ float symbolScore = 0.f;
};
struct CompletionItem {
@@ -588,13 +593,10 @@
/// the background color of its range.
struct DocumentHighlight {
-
/// The range this highlight applies to.
-
Range range;
/// The highlight kind, default is DocumentHighlightKind.Text.
-
DocumentHighlightKind kind = DocumentHighlightKind::Text;
friend bool operator<(const DocumentHighlight &LHS,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h
ilya-biryukov added a comment. This commit makes potential UB less likely. Logical errors (i.e. forgot to initialize some fields) will still be there. Sending this for review to make sure everyone agrees this is a good thing. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43230 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h
jkorous-apple added inline comments.
Comment at: clangd/Protocol.h:190
/// (see exit notification) its process.
- llvm::Optional processId;
+ llvm::Optional processId = 0;
Sorry if I am asking stupid question but since the type is Optional<> isn't
it's default-constructed value more appropriate here?
Comment at: clangd/Protocol.h:211
/// The initial trace setting. If omitted trace is disabled ('off').
- llvm::Optional trace;
+ llvm::Optional trace = TraceLevel::Off;
};
Does it still make more sense to use Optional than plain TraceLevel?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h
ioeric added a comment. I think it's useful to explicit initialize non-trivial types like `FileChangeType`. But I think it's safe and probably easier to rely on default values of primitive types like int, bool etc. Explicitly setting default values is probably fine (so this change LGTM), but do we really want to make this a requirement for future changes or even in our coding style? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43230 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43231: [clang-format] Refactor ObjC tests
jolesiak created this revision.
Herald added subscribers: cfe-commits, klimek.
Simplifies ObjC style tests.
Repository:
rC Clang
https://reviews.llvm.org/D43231
Files:
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -68,92 +68,84 @@
FormatStyle Style;
};
+// Checks whether language detected after running \p getStyle is expected.
+// \p Style cannot be a reference to const as casting to bool can change
+// internal state.
+void checkLanguage(llvm::Expected &Style,
+ FormatStyle::LanguageKind ExpectedLanguage) {
+ ASSERT_TRUE((bool)Style);
+ EXPECT_EQ(ExpectedLanguage, Style->Language);
+}
+
TEST(FormatTestObjCStyle, DetectsObjCInHeaders) {
auto Style = getStyle("LLVM", "a.h", "none", "@interface\n"
"- (id)init;");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("LLVM", "a.h", "none", "@interface\n"
"+ (id)init;");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("LLVM", "a.h", "none", "@interface\n"
"@end\n"
"//comment");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("LLVM", "a.h", "none", "@interface\n"
"@end //comment");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
// No recognizable ObjC.
Style = getStyle("LLVM", "a.h", "none", "void f() {}");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_Cpp);
Style = getStyle("{}", "a.h", "none", "@interface Foo\n@end\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("{}", "a.h", "none",
"const int interface = 1;\nconst int end = 2;\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_Cpp);
Style = getStyle("{}", "a.h", "none", "@protocol Foo\n@end\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("{}", "a.h", "none",
"const int protocol = 1;\nconst int end = 2;\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
-
- Style = getStyle("{}", "a.h", "none", "extern NSString *kFoo;\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_Cpp);
Style =
getStyle("{}", "a.h", "none", "typedef NS_ENUM(NSInteger, Foo) {};\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("{}", "a.h", "none", "enum Foo {};");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
-
- Style = getStyle("{}", "a.h", "none", "extern NSInteger Foo();\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_Cpp);
Style =
getStyle("{}", "a.h", "none", "inline void Foo() { Log(@\"Foo\"); }\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style =
getStyle("{}", "a.h", "none", "inline void Foo() { Log(\"Foo\"); }\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_Cpp);
Style =
getStyle("{}", "a.h", "none", "inline void Foo() { id = @[1, 2, 3]; }\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("{}", "a.h", "none",
"inline void Foo() { id foo = @{1: 2, 3: 4, 5: 6}; }\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_ObjC);
Style = getStyle("{}", "a.h", "none",
"inline void Foo() { int foo[] = {1, 2, 3}; }\n");
- ASSERT_TRUE((bool)Style);
- EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
+ checkLanguage(Style, FormatStyle::LK_Cpp);
+
+ // ObjC specific types.
+ Style = getStyle("{}", "a.h", "none", "extern NSString *kFoo;\n");
+ checkLangua
[PATCH] D43124: [clang-format] Improve ObjC headers detection
jolesiak planned changes to this revision. jolesiak added a comment. I'll add a few test cases after submitting https://reviews.llvm.org/D43231. Repository: rC Clang https://reviews.llvm.org/D43124 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43231: [clang-format] Refactor ObjC tests
krasimir added a comment. I don't believe this is needed: test fails before would fail at the line where test instance is checked, and after they will fail at the checkLanguage function. Repository: rC Clang https://reviews.llvm.org/D43231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35894: [clangd] Code hover for Clangd
simark added a comment. > I'm not aware of the code that pretty-prints macros. There's a code that > dumps debug output, but it's not gonna help in that case. > Alternatively, we could simply print the macro name and not print the > definition. That's super-simple and is in line with hovers for the AST decls. > Let's do that in the initial version. Ok, I'm considering printing "#define MACRO". Just printing the name would not be very useful, at least printing "#define" gives the information that the hovered entity is defined as a macro (and not a function or variable). Is there a way to get the macro name from the MacroInfo object? I couldn't find any, so the solution I'm considering is to make `DeclarationAndMacrosFinder::takeMacroInfos` return an `std::vector>`, where the first member of the pair is the macro name. It would come from `IdentifierInfo->getName()` in `DeclarationAndMacrosFinder::finish`. Does that make sense, or is there a simpler way? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D35894 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch
Typz added a comment. > Do you have a reference to style guides recommending any of this? Unfortunately not... I think this is a really advanced topic, and often not recommended way to format code at all (e.g. "if you need a block, you may as well use a function"). I can find the current implementation documented in google style guide, where there is an extra indent inside switch [e.g. `IndentCaseLabels`], which alleviates the ambiguity. But it is not documented (or shown) in the LLVM coding style, where it makes the code IMO really difficult to follow. I believe the 2 extra modes I introduce should provide for most cases, leaving the decision to user: - `None` mode matches google-style, and is perfectly fine when `IndentCaseLabels == true` - `ClosingBrace` mode is "logical" with what people may want (e.g. have a scope for the whole `case` block), but is not syntaxically correct and may thus mislead - `Block` mode is syntaxically correct, but somewhat breaks the switch layout by moving break to different indentation levels depending on the use of braces Which mode is use is then left to the user, depending on the "goals" of their coding style. Also, maybe this is a small-enough detail that it would be better moved to the `BraceWrapping` field, to allow finely tuning the expected behavior (e.g. in Custom brace mode) while keeping the 'main' options simple (even though it is not technically a way to wrap the braces). Repository: rC Clang https://reviews.llvm.org/D43183 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks
Typz created this revision.
Typz added reviewers: krasimir, djasper, klimek, benhamilton.
ObjC defines `@autoreleasepool` and `@synchronized` control blocks. These
used to be formatted according to the `AfterObjCDeclaration` brace-
wrapping flag, which is not very consistent.
This patch changes the behavior to use the `AfterControlStatement` flag
instead. This should not affect the behavior unless a custom brace
wrapping mode is used.
Repository:
rC Clang
https://reviews.llvm.org/D43232
Files:
include/clang/Format/Format.h
lib/Format/UnwrappedLineParser.cpp
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1115,7 +1115,7 @@
case tok::objc_autoreleasepool:
nextToken();
if (FormatTok->Tok.is(tok::l_brace)) {
- if (Style.BraceWrapping.AfterObjCDeclaration)
+ if (Style.BraceWrapping.AfterControlStatement)
addUnwrappedLine();
parseBlock(/*MustBeDeclaration=*/false);
}
@@ -1127,7 +1127,7 @@
// Skip synchronization object
parseParens();
if (FormatTok->Tok.is(tok::l_brace)) {
- if (Style.BraceWrapping.AfterObjCDeclaration)
+ if (Style.BraceWrapping.AfterControlStatement)
addUnwrappedLine();
parseBlock(/*MustBeDeclaration=*/false);
}
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -651,7 +651,9 @@
/// }
/// \endcode
bool AfterNamespace;
-/// \brief Wrap ObjC definitions (``@autoreleasepool``, interfaces, ..).
+/// \brief Wrap ObjC definitions (interfaces, implementations...).
+/// \note @autoreleasepool and @synchronized blocks are wrapped
+/// according to `AfterControlStatement` flag.
bool AfterObjCDeclaration;
/// \brief Wrap struct definitions.
/// \code
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1115,7 +1115,7 @@
case tok::objc_autoreleasepool:
nextToken();
if (FormatTok->Tok.is(tok::l_brace)) {
- if (Style.BraceWrapping.AfterObjCDeclaration)
+ if (Style.BraceWrapping.AfterControlStatement)
addUnwrappedLine();
parseBlock(/*MustBeDeclaration=*/false);
}
@@ -1127,7 +1127,7 @@
// Skip synchronization object
parseParens();
if (FormatTok->Tok.is(tok::l_brace)) {
- if (Style.BraceWrapping.AfterObjCDeclaration)
+ if (Style.BraceWrapping.AfterControlStatement)
addUnwrappedLine();
parseBlock(/*MustBeDeclaration=*/false);
}
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -651,7 +651,9 @@
/// }
/// \endcode
bool AfterNamespace;
-/// \brief Wrap ObjC definitions (``@autoreleasepool``, interfaces, ..).
+/// \brief Wrap ObjC definitions (interfaces, implementations...).
+/// \note @autoreleasepool and @synchronized blocks are wrapped
+/// according to `AfterControlStatement` flag.
bool AfterObjCDeclaration;
/// \brief Wrap struct definitions.
/// \code
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43232: clang-format: use AfterControlStatement to format ObjC control blocks
Typz added a comment. This is done as discussed in https://reviews.llvm.org/D43114. But I can instead add a new `AfterObjCSpecialBlock` brace wrapping flag. Repository: rC Clang https://reviews.llvm.org/D43232 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43114: clang-format: fix formatting of ObjC @synchronized blocks
Typz marked 4 inline comments as done.
Typz added inline comments.
Comment at: lib/Format/UnwrappedLineParser.cpp:1130
+if (FormatTok->Tok.is(tok::l_brace)) {
+ if (Style.BraceWrapping.AfterObjCDeclaration)
+addUnwrappedLine();
benhamilton wrote:
> Typz wrote:
> > benhamilton wrote:
> > > Typz wrote:
> > > > Wondering if formatting with this style is appropriate: the
> > > > clang-format doc points in that direction, but it seems to me both
> > > > `@synchronized` and `@autoreleasepool` are more akin to "control
> > > > statements".
> > > >
> > > > For consistency (esp. in ObjC++ code), as a user, I would tend to have
> > > > these blocks indented like control statements while
> > > > interfaces/implementations blocks would be indented like
> > > > classes/structs.
> > > >
> > > > So maybe it would be better to introduce a new BraceWrapping style
> > > > 'AfterObjCSpecialBlock` to control these cases, matching the
> > > > possibilities that are given for control statements vs classes. What do
> > > > you think?
> > > Hmm, I definitely agree with that logic. I don't see them acting as
> > > declarations in any way, they are definitely like control statements.
> > >
> > Ok, i can change this. Two questions though:
> > * Should I do this in this patch, or a separate patch? (won't be a big
> > change in any case, but it can still be seen as 2 separate issues/changes)
> > * Should I introduce a new BraceWrapping flag (`AfterObjCSpecialBlock`), or
> > simply apply `AfterControlStatement` to these blocks?
> Personally, I feel `AfterControlStatement` applies to these. I'm not an
> expert on this, though, so I will defer to others on the review.
I created another diff to change the field that is used to control brace
wrapping after these blocks: https://reviews.llvm.org/D43232
This patch here should now only relate to the parsing of `@synchronized` blocks.
Repository:
rC Clang
https://reviews.llvm.org/D43114
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch
djasper added a comment.
To me none of these options make sense. For the case you are describing, I
agree that the current behavior is not ideal, but neither are any of the
alternatives. However, I think that's fine. We'll just format those cases in a
somewhat weird way and users can either accept that or change their code to not
need it. I don't particularly care which of these options we go with, but I
would be really hesitant to introduce an style flag for it. This is so rare,
that almost no one needs this flag (or should need this flag). Thus, the cost
of the flag (discoverability of flags for users, increased maintenance cost,
etc.) strongly outweigh the benefit. IMO, for the same reason, this specific
issue will not become part of the style guide of projects.
If you want to change something, I'd vote for making clang fall back to this
behavior:
case A:
{
stuff();
}
moreStuff();
break;
}
i.e. not let it put the "{" on the same line as the "case ..." if there is a
trailing statement (other than "break;" maybe).
Repository:
rC Clang
https://reviews.llvm.org/D43183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38639: [clangd] #include statements support for Open definition
ilya-biryukov added inline comments.
Comment at: clangd/ClangdUnit.cpp:103
+
+if (SourceMgr.isInMainFile(FilenameRange.getAsRange().getBegin())) {
+ // Only inclusion directives in the main file make sense. The user cannot
NIT: replace `FilenameRange.getAsRange()` with `SR`
Comment at: clangd/ClangdUnit.cpp:126
+
+ CppFilePreambleCallbacks() : SourceMgr(nullptr) {}
+
NIT: remove ctor, use initializer on the member instead:
```
private:
SourceManager *SourceMgr = nullptr;
```
Comment at: clangd/ClangdUnit.cpp:160
+ SourceManager *SourceMgr;
+ InclusionLocations IncLocations;
};
Maybe swap `IncLocations` and `SourceMgr`? Grouping `TopLevelDecls`,
`TopLevelDeclIDs` and `IncLocations` together seems reasonable, as all of them
are essentially output parameters.
Comment at: clangd/ClangdUnit.cpp:296
StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ ASTDiags);
+ InclusionLocations IncLocations;
+ // Copy over the includes from the preamble, then combine with the
NIT: move `IncLocations` closer to their first use, i.e. create the variable
right before `addPPCallbacks()`
Comment at: clangd/ClangdUnit.h:51
+using IncludeReferenceMap = std::unordered_map;
+
malaperle wrote:
> ilya-biryukov wrote:
> > We use `unordered_map` as a `vector>` here. (i.e. we never look up
> > values by key, because we don't only the range, merely a point in the range)
> > Replace map with `vector>` and remove `RangeHash` that we don't need
> > anymore.
> Done. I also renamed, IncludeReferenceMap to InclusionLocations. I thought I
> was more suitable.
LG
Comment at: clangd/ClangdUnit.h:105
+ const InclusionLocations &getInclusionLocations() const {
+return IncLocations;
+ };
NIT: move to .cpp file to be consisten with other decls in this file.
Comment at: clangd/XRefs.cpp:171
+Range R = IncludeLoc.first;
+const SourceManager &SourceMgr = AST.getASTContext().getSourceManager();
+Position Pos = sourceLocToPosition(SourceMgr, SourceLocationBeg);
NIT: `SourceMgr` does not depend on the loop variable, declare it out of the
loop.
Comment at: clangd/XRefs.cpp:174
+
+if (R.start.line == Pos.line && R.start.character <= Pos.character &&
+Pos.character <= R.end.character) {
NIT: define `operator <=` for `Position` and do this instead: `R.start <= Pos
&& Pos < R.end` (note that LSP ranges are half-open, i.e. the end is excluded).
Even better: define a `bool contains(Position Pos) const` helper in the `Range`
class and use it here (will abstract away to half-open nature of the range)
Comment at: clangd/XRefs.cpp:178
+Range{Position{0, 0}, Position{0,
0}}});
+ return IncludeTargets;
+}
Let's follow the pattern of decls and macros here. I.e. not return from the
function, merely collect all the results.
Comment at: unittests/clangd/XRefsTests.cpp:53
+class IgnoreDiagnostics : public DiagnosticsConsumer {
+ void onDiagnosticsReady(
NIT: remove this class, use `IgnoreDiagnostics` from `Compiler.h` instead.
Comment at: unittests/clangd/XRefsTests.cpp:245
+ const char *SourceContents = R"cpp(
+ #include "$2^invalid.h"
+ #include "^foo.h"
Could we also add tests for corner cases: cursor before opening quote, cursor
after the closing quote, cursor in the middle of `#include` token? (we
shouldn't navigate anywhere in the middle of the #include token)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D38639
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch
Typz added a comment.
> We'll just format those cases in a somewhat weird way and users can either
> accept that or change their code to not need it.
I think we have a really diverging opinion on this. From my experience, people
will mostly accept the output of the formatter without question : therefore I
think we should strive to have the formatter format things as "correctly" as
possible. And looking at the existing options and various complex formatting
algorithms implemented I think this reasoning has been applied to some extent
:-)
So I personnally would be willing to add some code/options even for such kind
of corner cases; but then I am not the one maintaining the clang-format project.
> I don't particularly care which of these options we go with, but I would be
> really hesitant to introduce an style flag for it. This is so rare, that
> almost no one needs this flag (or should need this flag). Thus, the cost of
> the flag (discoverability of flags for users, increased maintenance cost,
> etc.) strongly outweigh the benefit.
Any change will still require a new flag somewhere, since the currently
implemented behavior is :
1- the documented way to format in Google style
2- the expected format in LLVM style, according to the clang-format unit test
It should thus probably not be changed.
> IMO, for the same reason, this specific issue will not become part of the
> style guide of projects.
I definitely agree on this, but it is actually part of some styles (e.g.
Google's)
> If you want to change something, I'd vote for making clang fall back to this
> behavior:
>
> case A:
> {
> stuff();
> }
> moreStuff();
> break;
> }
>
>
> i.e. not let it put the "{" on the same line as the "case ..." if there is a
> trailing statement (other than "break;" maybe).
I am fine with that formatting (though I did not implement it since it requires
tweaking the code in more places, instead of essentially modifying a single
function like I did).
Repository:
rC Clang
https://reviews.llvm.org/D43183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43167: Fix incorrect indentation.
mclow.lists accepted this revision. mclow.lists added a comment. This revision is now accepted and ready to land. This is fine. Repository: rCXX libc++ https://reviews.llvm.org/D43167 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39571: [clangd] DidChangeConfiguration Notification
ilya-biryukov added a comment.
In https://reviews.llvm.org/D39571#1005926, @malaperle wrote:
> I haven't looked at the newest patch yet but I gave it a quick try and saw
> something odd. If I change the configuration to something invalid (say I
> specify the path to a CMakeLists.txt), then I get many errors/diagnostics,
> which is normal. But then when I change the config to something valid, the
> same diagnostics re-emitted, as if the configuration failed to change. I'll
> check the code tomorrow a bit but I thought I'd share this with you early.
Maybe you were still seeing diagnostics from the older versions of the files?
clangd should **eventually** produce the right diagnostics, but if processing
of some of the requests is in-flight at the time there results may still be
reported.
BTW, I don't know if you've seen it before, but there's `-input-mirror-file`
option to clangd that records the lsp input which can be used to rerun clangd
later.
# This should be run by VSCode (or Theia)
clangd -input-mirror-file=/tmp/clangd.input
# Later we can rerun from command line
$ clangd < /tmp/clangd.input
# Pass the flag to execute everyhing on one thread
$ clangd -run-synchronously < /tmp/clangd.input
This is useful to debug/rerun and see the logs, etc.
This patch is good to go after remove `std::future<>` from
`reparseOpenedFiles`, unless @malaperle discovers there are other problems.
Ideally, it'd be also nice to have a test for it.
Comment at: clangd/ClangdServer.cpp:541
+std::vector>
+ClangdServer::reparseOpenedFiles() {
We're not returning futures from `forceReparse` anymore, this function has to
be updated accordingly.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D39571
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42624: [clang-tidy] Add a utility Matcher to match the next statement within a statement sequence
tbourvon added inline comments. Comment at: clang-tidy/utils/Matchers.h:16 +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Analysis/CFG.h" aaron.ballman wrote: > This will require linking in the clangAnalysis library as well; are we sure > we want to take on this dependency here for all matchers? Do you have a specific solution in mind? We could make the matcher local to the check it is being used in (see D37014), but I think it could prove useful for other checks... Comment at: clang-tidy/utils/Matchers.h:58-60 + // We get the first parent, making sure that we're not in a case statement + // not in a compound statement directly inside a switch, because this causes + // the buildCFG call to crash. aaron.ballman wrote: > Why does it cause the crash? Should that crash not be fixed instead of > applying this workaround? I'm not entirely sure if this is expected behavior or not. In terms of AST, `switch` statements are a bit special in terms of how they are represented (each case contains all the subsequent cases as its children IIRC). There probably is a way to make the CFG work in these cases, but I honestly don't have the time to look into that and attempt a fix. Couldn't this be good enough for now, maybe with a FIXME? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42624 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42623: [clang-tidy] Add a Lexer util to get the source text of a statement
tbourvon abandoned this revision. tbourvon added inline comments. Comment at: clang-tidy/utils/LexerUtils.h:26 +/// Get source code text for statement. +Optional getStmtText(const Stmt* Statement, const SourceManager& SM); + alexfh wrote: > aaron.ballman wrote: > > Formatting (elsewhere as well). > > > > You should run the patch under clang-format to handle this sort of thing. > Have you seen clang::tooling::fixit::getText? It should cover this use case. Hadn't seen that! Seems to be making this whole patch obsolete, I'm closing it Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42623 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43108: Support for the mno-stack-arg-probe flag
hans added a comment. In https://reviews.llvm.org/D43108#1005904, @nruslan wrote: > @hans: One real-world example is when it is used to compile UEFI code using > PE/COFF targets natively. Obviously, UEFI uses ABI which is basically almost > the same as MS ABI, except that chkstk is not used. It mostly works (I > actually was able to get it running) except the cases when the code contains > variable-sized arrays allocated on stacks. Unfortunately, stack-probe-size > will only help with fixed sized array but will not help to solve the problem > described in the bug description since stack usage is unknown at compile > time. MinGW does not have this problem because it provides this flag. I see, interesting. Might be worth mentioning in the commit message for others wondering what the flag is useful for. > @MatzeB : there is a test on LLVM side (related review). Do you think the > test is needed for clang side? If so, please let me know, what kind of test > it is supposed to be. Yes please, I think think there should be on in test/Driver/ to check that forwarding the flag to cc1 (and if we have a -mno-foo, there should maybe be a -mfoo variant too?), and a test in test/CodeGen/ to check that the attribute gets put on the functions correctly. Perhaps r321992 is a good example to look at. Repository: rC Clang https://reviews.llvm.org/D43108 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch
djasper added a comment.
In https://reviews.llvm.org/D43183#1006170, @Typz wrote:
> > We'll just format those cases in a somewhat weird way and users can either
> > accept that or change their code to not need it.
>
> I think we have a really diverging opinion on this. From my experience,
> people will mostly accept the output of the formatter without question :
> therefore I think we should strive to have the formatter format things as
> "correctly" as possible. And looking at the existing options and various
> complex formatting algorithms implemented I think this reasoning has been
> applied to some extent :-)
> So I personnally would be willing to add some code/options even for such
> kind of corner cases; but then I am not the one maintaining the clang-format
> project.
I have no problem with making clang-format format things "correctly" and I am
happy to add code. But I think we are doing the average clang-format user a
disservice if we provide options for every such corner case. Let's settle on
one good-enough behavior and go with that for everything/everyone.
>> I don't particularly care which of these options we go with, but I would be
>> really hesitant to introduce an style flag for it. This is so rare, that
>> almost no one needs this flag (or should need this flag). Thus, the cost of
>> the flag (discoverability of flags for users, increased maintenance cost,
>> etc.) strongly outweigh the benefit.
>
> Any change will still require a new flag somewhere, since the currently
> implemented behavior is :
> 1- the documented way to format in Google style
> 2- the expected format in LLVM style, according to the clang-format unit test
>
> It should thus probably not be changed.
I don't think that's true for the alternative I am suggesting.
>> IMO, for the same reason, this specific issue will not become part of the
>> style guide of projects.
>
> I definitely agree on this, but it is actually part of some styles (e.g.
> Google's)
>
>> If you want to change something, I'd vote for making clang fall back to this
>> behavior:
>>
>> case A:
>> {
>> stuff();
>> }
>> moreStuff();
>> break;
>> }
>>
>>
>> i.e. not let it put the "{" on the same line as the "case ..." if there is a
>> trailing statement (other than "break;" maybe).
>
> I am fine with that formatting (though I did not implement it since it
> requires tweaking the code in more places, instead of essentially modifying a
> single function like I did).
As we can implement this without additional flags (it doesn't contradict any
style guide I know of), I think this would be strictly preferable.
Repository:
rC Clang
https://reviews.llvm.org/D43183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43231: [clang-format] Refactor ObjC tests
jolesiak abandoned this revision. jolesiak added a comment. In https://reviews.llvm.org/D43231#1006123, @krasimir wrote: > I don't believe this is needed: test fails before would fail at the line > where test instance is checked, and after they will fail at the checkLanguage > function. Thank you for the comment! That's a valid argument. I'm reverting as a macro is a no go. Repository: rC Clang https://reviews.llvm.org/D43231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch
Typz added a comment.
It is explicitly documented in google style guide:
https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements :
> case blocks in switch statements can have curly braces or not, depending on
> your preference. If you do include curly braces they should be placed as
> shown below.
>
> If not conditional on an enumerated value, switch statements should always
> have a default case (in the case of an enumerated value, the compiler will
> warn you if any values are not handled). If the default case should never
> execute, simply assert:
>
> switch (var) {
> case 0: { // 2 space indent
> ... // 4 space indent
> break;
> }
> case 1: {
> ...
> break;
> }
> default: {
> assert(false);
> }
> }
So IMHO we cannot just change the current (or default) behaviour.
Repository:
rC Clang
https://reviews.llvm.org/D43183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r325011 - An updated test to show the current warnings produced for implicit conversions from 'double' to 'float'.
Author: avt77
Date: Tue Feb 13 07:20:29 2018
New Revision: 325011
URL: http://llvm.org/viewvc/llvm-project?rev=325011&view=rev
Log:
An updated test to show the current warnings produced for implicit conversions
from 'double' to 'float'.
Modified:
cfe/trunk/test/Sema/conversion.c
Modified: cfe/trunk/test/Sema/conversion.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/conversion.c?rev=325011&r1=325010&r2=325011&view=diff
==
--- cfe/trunk/test/Sema/conversion.c (original)
+++ cfe/trunk/test/Sema/conversion.c Tue Feb 13 07:20:29 2018
@@ -429,3 +429,17 @@ void test27(ushort16 constants) {
ushort16 crCbScale = pairedConstants.s4; // expected-warning {{implicit
conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to
'ushort16'}}
ushort16 brBias = pairedConstants.s6; // expected-warning {{implicit
conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to
'ushort16'}}
}
+
+
+float double2float_test1(double a) {
+return a; // expected-warning {{implicit conversion loses floating-point
precision: 'double' to 'float'}}
+}
+
+void double2float_test2(double a, float *b) {
+*b += a;
+}
+
+float sinf (float x);
+double double2float_test3(double a) {
+return sinf(a); // expected-warning {{implicit conversion loses
floating-point precision: 'double' to 'float'}}
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r325015 - [clang-tidy] Update fuchsia-multiple-inheritance to not fail
Author: juliehockett
Date: Tue Feb 13 07:40:40 2018
New Revision: 325015
URL: http://llvm.org/viewvc/llvm-project?rev=325015&view=rev
Log:
[clang-tidy] Update fuchsia-multiple-inheritance to not fail
Updating the fuchsia-multiple-inheritance to gracefully handle unknown
record types (e.g. templatized classes) by simply continuing, rather
than asserting and failing.
Fixes PR36052.
Differential Revision: https://reviews.llvm.org/D43223
Modified:
clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp?rev=325015&r1=325014&r2=325015&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
(original)
+++ clang-tools-extra/trunk/clang-tidy/fuchsia/MultipleInheritanceCheck.cpp Tue
Feb 13 07:40:40 2018
@@ -66,7 +66,7 @@ bool MultipleInheritanceCheck::isInterfa
for (const auto &I : Node->bases()) {
if (I.isVirtual()) continue;
const auto *Ty = I.getType()->getAs();
-assert(Ty && "RecordType of base class is unknown");
+if (!Ty) continue;
const RecordDecl *D = Ty->getDecl()->getDefinition();
if (!D) continue;
const auto *Base = cast(D);
@@ -96,9 +96,9 @@ void MultipleInheritanceCheck::check(con
// concrete classes
unsigned NumConcrete = 0;
for (const auto &I : D->bases()) {
- if (I.isVirtual() || I.getType()->getAs())
continue;
+ if (I.isVirtual()) continue;
const auto *Ty = I.getType()->getAs();
- assert(Ty && "RecordType of base class is unknown");
+ if (!Ty) continue;
const auto *Base = cast(Ty->getDecl()->getDefinition());
if (!isInterface(Base)) NumConcrete++;
}
@@ -107,7 +107,7 @@ void MultipleInheritanceCheck::check(con
// non-virtual base.
for (const auto &V : D->vbases()) {
const auto *Ty = V.getType()->getAs();
- assert(Ty && "RecordType of base class is unknown");
+ if (!Ty) continue;
const auto *Base = cast(Ty->getDecl()->getDefinition());
if (!isInterface(Base)) NumConcrete++;
}
Modified:
clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp?rev=325015&r1=325014&r2=325015&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/fuchsia-multiple-inheritance.cpp
Tue Feb 13 07:40:40 2018
@@ -131,3 +131,6 @@ struct D8 : V13, V14 {};
template struct A : T {};
template struct B : virtual T {};
+
+template struct C {};
+template struct D : C {};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43223: [clang-tidy] Update fuchsia-multiple-inheritance to not fail
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE325015: [clang-tidy] Update fuchsia-multiple-inheritance
to not fail (authored by juliehockett, committed by ).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43223
Files:
clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
test/clang-tidy/fuchsia-multiple-inheritance.cpp
Index: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
===
--- clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -66,7 +66,7 @@
for (const auto &I : Node->bases()) {
if (I.isVirtual()) continue;
const auto *Ty = I.getType()->getAs();
-assert(Ty && "RecordType of base class is unknown");
+if (!Ty) continue;
const RecordDecl *D = Ty->getDecl()->getDefinition();
if (!D) continue;
const auto *Base = cast(D);
@@ -96,18 +96,18 @@
// concrete classes
unsigned NumConcrete = 0;
for (const auto &I : D->bases()) {
- if (I.isVirtual() || I.getType()->getAs())
continue;
+ if (I.isVirtual()) continue;
const auto *Ty = I.getType()->getAs();
- assert(Ty && "RecordType of base class is unknown");
+ if (!Ty) continue;
const auto *Base = cast(Ty->getDecl()->getDefinition());
if (!isInterface(Base)) NumConcrete++;
}
// Check virtual bases to see if there is more than one concrete
// non-virtual base.
for (const auto &V : D->vbases()) {
const auto *Ty = V.getType()->getAs();
- assert(Ty && "RecordType of base class is unknown");
+ if (!Ty) continue;
const auto *Base = cast(Ty->getDecl()->getDefinition());
if (!isInterface(Base)) NumConcrete++;
}
Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- test/clang-tidy/fuchsia-multiple-inheritance.cpp
+++ test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -131,3 +131,6 @@
template struct A : T {};
template struct B : virtual T {};
+
+template struct C {};
+template struct D : C {};
Index: clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
===
--- clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
+++ clang-tidy/fuchsia/MultipleInheritanceCheck.cpp
@@ -66,7 +66,7 @@
for (const auto &I : Node->bases()) {
if (I.isVirtual()) continue;
const auto *Ty = I.getType()->getAs();
-assert(Ty && "RecordType of base class is unknown");
+if (!Ty) continue;
const RecordDecl *D = Ty->getDecl()->getDefinition();
if (!D) continue;
const auto *Base = cast(D);
@@ -96,18 +96,18 @@
// concrete classes
unsigned NumConcrete = 0;
for (const auto &I : D->bases()) {
- if (I.isVirtual() || I.getType()->getAs()) continue;
+ if (I.isVirtual()) continue;
const auto *Ty = I.getType()->getAs();
- assert(Ty && "RecordType of base class is unknown");
+ if (!Ty) continue;
const auto *Base = cast(Ty->getDecl()->getDefinition());
if (!isInterface(Base)) NumConcrete++;
}
// Check virtual bases to see if there is more than one concrete
// non-virtual base.
for (const auto &V : D->vbases()) {
const auto *Ty = V.getType()->getAs();
- assert(Ty && "RecordType of base class is unknown");
+ if (!Ty) continue;
const auto *Base = cast(Ty->getDecl()->getDefinition());
if (!isInterface(Base)) NumConcrete++;
}
Index: test/clang-tidy/fuchsia-multiple-inheritance.cpp
===
--- test/clang-tidy/fuchsia-multiple-inheritance.cpp
+++ test/clang-tidy/fuchsia-multiple-inheritance.cpp
@@ -131,3 +131,6 @@
template struct A : T {};
template struct B : virtual T {};
+
+template struct C {};
+template struct D : C {};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch
djasper added a comment.
In https://reviews.llvm.org/D43183#1006224, @Typz wrote:
> It is explicitly documented in google style guide:
> https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements
> :
>
> > case blocks in switch statements can have curly braces or not, depending on
> > your preference. If you do include curly braces they should be placed as
> > shown below.
> >
> > If not conditional on an enumerated value, switch statements should always
> > have a default case (in the case of an enumerated value, the compiler will
> > warn you if any values are not handled). If the default case should never
> > execute, simply assert:
> >
> > switch (var) {
> > case 0: { // 2 space indent
> > ... // 4 space indent
> > break;
> > }
> > case 1: {
> > ...
> > break;
> > }
> > default: {
> > assert(false);
> > }
> > }
>
> So IMHO we cannot just change the current (or default) behaviour.
My proposal does not contradict this style guide as the case in question is not
included in the example. It gives absolutely no guidance on how to format this
case.
If anything, you could argue that it tells the user never to have something
outside of the braces that make up a case statement (keep in mind that the
style guide does not only give formatting advise).
Repository:
rC Clang
https://reviews.llvm.org/D43183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43181: [CodeGen] Initialize large arrays by copying from a global
kosarev added inline comments.
Comment at: lib/CodeGen/CGExprAgg.cpp:421
+ // with explicit initializers should be large enough.
+ if (NumInitElements > 8 && elementType->isBuiltinType()) {
+CodeGen::CodeGenModule &CGM = CGF.CGM;
rjmccall wrote:
> Is there a good reason to use an element-count heuristic instead of a
> total-size heuristic here?
>
> Why only builtin types? That seems to pointlessly rule out nested arrays,
> complex types, vectors, C structs, and so on. I think the predicate you
> probably want here is isTriviallyCopyableType.
> Is there a good reason to use an element-count heuristic instead of a
> total-size heuristic here?
Yes, the code below generates per-element initialization only for explicitly
specified initializers. The rest, if any, is initialized with a filler, so it
doesn't affect the size of the resulting code much.
Repository:
rL LLVM
https://reviews.llvm.org/D43181
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43187: [AST] Refine the condition for element-dependent array fillers
kosarev added a comment. John, maybe you can review this or suggest some other reviewers? Thanks. Repository: rL LLVM https://reviews.llvm.org/D43187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43181: [CodeGen] Initialize large arrays by copying from a global
kosarev updated this revision to Diff 134042.
kosarev added a comment.
Improved as suggested to cover all trivially-copyable types.
https://reviews.llvm.org/D43181
Files:
lib/CodeGen/CGExprAgg.cpp
test/CodeGen/init.c
Index: test/CodeGen/init.c
===
--- test/CodeGen/init.c
+++ test/CodeGen/init.c
@@ -8,8 +8,9 @@
// CHECK-DAG: %struct.M = type { [2 x %struct.I] }
// CHECK-DAG: %struct.I = type { [3 x i32] }
-// CHECK: [1 x %struct.M] [%struct.M { [2 x %struct.I] [%struct.I { [3 x i32] [i32 4, i32 4, i32 0] }, %struct.I { [3 x i32] [i32 4, i32 4, i32 5] }] }],
-// CHECK: [2 x [3 x i32]] {{[[][[]}}3 x i32] [i32 , i32 , i32 0], [3 x i32] [i32 , i32 , i32 ]],
+// CHECK-DAG: [1 x %struct.M] [%struct.M { [2 x %struct.I] [%struct.I { [3 x i32] [i32 4, i32 4, i32 0] }, %struct.I { [3 x i32] [i32 4, i32 4, i32 5] }] }],
+// CHECK-DAG: [2 x [3 x i32]] {{[[][[]}}3 x i32] [i32 , i32 , i32 0], [3 x i32] [i32 , i32 , i32 ]],
+// CHECK-DAG: [[INIT14:.*]] = private global [16 x i32] [i32 0, i32 0, i32 0, i32 0, i32 0, i32 17, i32 17, i32 17, i32 17, i32 17, i32 17, i32 17, i32 0, i32 0, i32 0, i32 0], align 4
void f1() {
// Scalars in braces.
@@ -33,8 +34,8 @@
}
// Constants
-// CHECK: @g3 = constant i32 10
-// CHECK: @f4.g4 = internal constant i32 12
+// CHECK-DAG: @g3 = constant i32 10
+// CHECK-DAG: @f4.g4 = internal constant i32 12
const int g3 = 10;
int f4() {
static const int g4 = 12;
@@ -61,7 +62,7 @@
-// CHECK: @test7 = global{{.*}}{ i32 0, [4 x i8] c"bar\00" }
+// CHECK-DAG: @test7 = global{{.*}}{ i32 0, [4 x i8] c"bar\00" }
// PR8217
struct a7 {
int b;
@@ -151,3 +152,15 @@
// CHECK: memcpy{{.*}}getelementptr inbounds ([3 x i8], [3 x i8]* @
bar((char[3]) {""});
}
+
+// Test that we initialize large member arrays by copying from a global and not
+// with a series of stores.
+struct S14 { int a[16]; };
+
+void test14(struct S14 *s14) {
+// CHECK-LABEL: @test14
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 {{.*}}, i8* align 4 {{.*}} [[INIT14]] {{.*}}, i32 64, i1 false)
+// CHECK-NOT: store
+// CHECK: ret void
+ *s14 = (struct S14) { { [5 ... 11] = 17 } };
+}
Index: lib/CodeGen/CGExprAgg.cpp
===
--- lib/CodeGen/CGExprAgg.cpp
+++ lib/CodeGen/CGExprAgg.cpp
@@ -14,6 +14,7 @@
#include "CodeGenFunction.h"
#include "CGObjCRuntime.h"
#include "CodeGenModule.h"
+#include "ConstantEmitter.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclTemplate.h"
@@ -85,7 +86,7 @@
void EmitMoveFromReturnSlot(const Expr *E, RValue Src);
void EmitArrayInit(Address DestPtr, llvm::ArrayType *AType,
- QualType elementType, InitListExpr *E);
+ QualType ArrayQTy, InitListExpr *E);
AggValueSlot::NeedsGCBarriers_t needsGC(QualType T) {
if (CGF.getLangOpts().getGC() && TypeRequiresGCollection(T))
@@ -394,12 +395,15 @@
/// \brief Emit initialization of an array from an initializer list.
void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType,
- QualType elementType, InitListExpr *E) {
+ QualType ArrayQTy, InitListExpr *E) {
uint64_t NumInitElements = E->getNumInits();
uint64_t NumArrayElements = AType->getNumElements();
assert(NumInitElements <= NumArrayElements);
+ QualType elementType =
+ CGF.getContext().getAsArrayType(ArrayQTy)->getElementType();
+
// DestPtr is an array*. Construct an elementType* by drilling
// down a level.
llvm::Value *zero = llvm::ConstantInt::get(CGF.SizeTy, 0);
@@ -411,6 +415,29 @@
CharUnits elementAlign =
DestPtr.getAlignment().alignmentOfArrayElement(elementSize);
+ // Consider initializing the array by copying from a global. For this to be
+ // more efficient than per-element initialization, the number of the elements
+ // with explicit initializers should be large enough.
+ if (NumInitElements > 8 &&
+ elementType.isTriviallyCopyableType(CGF.getContext())) {
+CodeGen::CodeGenModule &CGM = CGF.CGM;
+ConstantEmitter Emitter(CGM);
+LangAS AS = ArrayQTy.getAddressSpace();
+if (llvm::Constant *C = Emitter.tryEmitForInitializer(E, AS, ArrayQTy)) {
+ auto GV = new llvm::GlobalVariable(
+ CGM.getModule(), C->getType(),
+ CGM.isTypeConstant(ArrayQTy, /* ExcludeCtorDtor= */ true),
+ llvm::GlobalValue::PrivateLinkage, C, "constinit",
+ /* InsertBefore= */ nullptr, llvm::GlobalVariable::NotThreadLocal,
+ CGM.getContext().getTargetAddressSpace(AS));
+ Emitter.finalize(GV);
+ CharUnits Align = CGM.getContext().getTypeAlignInChars(ArrayQTy);
+ GV->setAlignment(Align.getQuantity());
+ EmitFinalDestCopy(ArrayQTy, CGF.MakeAddrLValue(GV, ArrayQTy, Align));
+ return;
+}
+ }
+
//
[PATCH] D43240: [OpenCL] Fix __enqueue_block for block with captures
yaxunl created this revision.
yaxunl added reviewers: Anastasia, bader.
The following test case causes issue with codegen of __enqueue_block
void (^block)(void) = ^{ callee(id, out); };
enqueue_kernel(queue, 0, ndrange, block);
Clang first does codegen for block expression in the first line and deletes its
block info.
Clang then tries to do codegen for the same block expression again for the
second line,
and fails because the block info is gone.
The fix is to do normal codegen for both lines. Introduce an API to OpenCL
runtime to
record llvm block invoke function and llvm block literal emitted for each AST
block
expression, and use the recorded information for generating the wrapper kernel.
The EmitBlockLiteral APIs are cleaned up to minimize changes to the normal
codegen
of blocks.
Another minor issue is that some clean up AST expression is generated for block
with captures, which can be stripped by IgnoreImplicit.
https://reviews.llvm.org/D43240
Files:
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGOpenCLRuntime.cpp
lib/CodeGen/CGOpenCLRuntime.h
lib/CodeGen/CodeGenFunction.h
test/CodeGenOpenCL/enqueue-block-with-captures.cl
Index: test/CodeGenOpenCL/enqueue-block-with-captures.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/enqueue-block-with-captures.cl
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -emit-llvm -O0 -o - -triple spir-unknown-unknown | FileCheck %s
+
+typedef struct {int a;} ndrange_t;
+
+void callee(int id, __global int* out) {
+ out[id] = id;
+}
+
+kernel void test(int id, __global int* out) {
+
+ void (^block)(void) = ^{ callee(id, out); };
+
+ queue_t queue;
+ ndrange_t ndrange;
+ // CHECK: call i32 @__enqueue_kernel_basic
+ enqueue_kernel(queue, 0, ndrange, block);
+}
+
+// CHECK: define internal spir_kernel void @__test_block_invoke_kernel(i8 addrspace(4)*)
+// CHECK: call void @__test_block_invoke(i8 addrspace(4)* %0)
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -1583,10 +1583,7 @@
/// \return an LLVM value which is a pointer to a struct which contains
/// information about the block, including the block invoke function, the
/// captured variables, etc.
- /// \param InvokeF will contain the block invoke function if it is not
- /// nullptr.
- llvm::Value *EmitBlockLiteral(const BlockExpr *,
-llvm::Function **InvokeF = nullptr);
+ llvm::Value *EmitBlockLiteral(const BlockExpr *);
static void destroyBlockInfos(CGBlockInfo *info);
llvm::Function *GenerateBlockFunction(GlobalDecl GD,
@@ -3010,11 +3007,8 @@
LValue EmitOMPSharedLValue(const Expr *E);
private:
- /// Helpers for blocks. Returns invoke function by \p InvokeF if it is not
- /// nullptr. It should be called without \p InvokeF if the caller does not
- /// need invoke function to be returned.
- llvm::Value *EmitBlockLiteral(const CGBlockInfo &Info,
-llvm::Function **InvokeF = nullptr);
+ /// Helpers for blocks.
+ llvm::Value *EmitBlockLiteral(const CGBlockInfo &Info);
/// struct with the values to be passed to the OpenMP loop-related functions
struct OMPLoopArguments {
Index: lib/CodeGen/CGOpenCLRuntime.h
===
--- lib/CodeGen/CGOpenCLRuntime.h
+++ lib/CodeGen/CGOpenCLRuntime.h
@@ -23,6 +23,7 @@
namespace clang {
+class BlockExpr;
class Expr;
class VarDecl;
@@ -39,8 +40,9 @@
/// Structure for enqueued block information.
struct EnqueuedBlockInfo {
-llvm::Function *Kernel; /// Enqueued block kernel.
-llvm::Value *BlockArg; /// The first argument to enqueued block kernel.
+llvm::Function *InvokeFunc; /// Block invoke function.
+llvm::Function *Kernel; /// Enqueued block kernel.
+llvm::Value *BlockArg; /// The first argument to enqueued block kernel.
};
/// Maps block expression to block information.
llvm::DenseMap EnqueuedBlockMap;
@@ -76,6 +78,15 @@
/// \return enqueued block information for enqueued block.
EnqueuedBlockInfo emitOpenCLEnqueuedBlock(CodeGenFunction &CGF,
const Expr *E);
+
+ /// \brief Record invoke function and block literal emitted during normal
+ /// codegen for a block expression. The information is used by
+ /// emitOpenCLEnqueuedBlock to emit wrapper kernel.
+ ///
+ /// \param InvokeF invoke function emitted for the block expression.
+ /// \param Block block literal emitted for the block expression.
+ void recordBlockInfo(const BlockExpr *E, llvm::Function *InvokeF,
+ llvm::Value *Block);
};
}
Index: lib/CodeGen/CGOpenCLRuntime.cpp
===
--- lib/CodeGen/CGOpenCLRuntime.cpp
+++ lib/CodeGen/CGOpenCLRuntime.cpp
@@ -112,37 +112
[PATCH] D43171: [AMDGPU] Change constant addr space to 4 for clang
yaxunl added inline comments.
Comment at: test/CodeGenOpenCL/address-spaces.cl:37
+// SPIR: i32 addrspace(2)* %arg
+// GIZ: i32 addrspace(4)* %arg
void f__c(__constant int *arg) {}
t-tye wrote:
> Suggest using the same name across all the OpenCL tests as some are using
> GIZ, some AMD and some AMDGCN. AMDGCN seems the clearer now that only have a
> single address space mapping? Similar comment for other places GIZ/AMD is
> being mentioned.
>
>
Will have a separate patch to cleanup the tests.
Comment at: test/CodeGenOpenCL/amdgpu-env-amdgiz.cl:4
-// CHECK: target datalayout =
"e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:32:32-p5:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5"
+// CHECK: target datalayout =
"e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5"
void foo(void) {}
t-tye wrote:
> Should this test be renamed to amdgpu-env-amdgcn.cl now there is a single
> address space mapping (and delete any relating to the old mapping if they
> exist)?
will rename it when committing.
https://reviews.llvm.org/D43171
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43183: clang-format: introduce `CaseBlockIndent` to control indent in switch
Typz added a comment.
Hum, not sure I fully got your proposal. So you actually mean that clang-format
you format like this (with 4-space indent for clarity):
switch (x) {
case 0:
break;
case 1: {
foo();
break;
}
case 2: {
foo();
} break;
case 3:
{
foo();
}
bar();
break;
}
Is this right?
If so it does not really help me: I don't care so much how it is formatted, but
I think the current way is way too error prone (and I cannot change the style
to indent the case blocks themself). So i'll have to keep a patch in my fork :-(
Or maybe the behavior should be dependant on `IndentCaseLabels` (though this
would change LLVM style formatting) ?
Repository:
rC Clang
https://reviews.llvm.org/D43183
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r325021 - [clangd] Remove the RealFS layer from test VFS. NFC.
Author: ibiryukov
Date: Tue Feb 13 09:08:13 2018
New Revision: 325021
URL: http://llvm.org/viewvc/llvm-project?rev=325021&view=rev
Log:
[clangd] Remove the RealFS layer from test VFS. NFC.
It was required before because preambles could only be created on
disk. All tests use in-memory preambles now.
Modified:
clang-tools-extra/trunk/unittests/clangd/TestFS.cpp
Modified: clang-tools-extra/trunk/unittests/clangd/TestFS.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestFS.cpp?rev=325021&r1=325020&r2=325021&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/TestFS.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestFS.cpp Tue Feb 13 09:08:13 2018
@@ -12,112 +12,6 @@
namespace clang {
namespace clangd {
-namespace {
-
-/// An implementation of vfs::FileSystem that only allows access to
-/// files and folders inside a set of whitelisted directories.
-///
-/// FIXME(ibiryukov): should it also emulate access to parents of whitelisted
-/// directories with only whitelisted contents?
-class FilteredFileSystem : public vfs::FileSystem {
-public:
- /// The paths inside \p WhitelistedDirs should be absolute
- FilteredFileSystem(std::vector WhitelistedDirs,
- IntrusiveRefCntPtr InnerFS)
- : WhitelistedDirs(std::move(WhitelistedDirs)), InnerFS(InnerFS) {
-assert(std::all_of(WhitelistedDirs.begin(), WhitelistedDirs.end(),
- [](const std::string &Path) -> bool {
- return llvm::sys::path::is_absolute(Path);
- }) &&
- "Not all WhitelistedDirs are absolute");
- }
-
- virtual llvm::ErrorOr status(const Twine &Path) {
-if (!isInsideWhitelistedDir(Path))
- return llvm::errc::no_such_file_or_directory;
-return InnerFS->status(Path);
- }
-
- virtual llvm::ErrorOr>
- openFileForRead(const Twine &Path) {
-if (!isInsideWhitelistedDir(Path))
- return llvm::errc::no_such_file_or_directory;
-return InnerFS->openFileForRead(Path);
- }
-
- llvm::ErrorOr>
- getBufferForFile(const Twine &Name, int64_t FileSize = -1,
- bool RequiresNullTerminator = true,
- bool IsVolatile = false) {
-if (!isInsideWhitelistedDir(Name))
- return llvm::errc::no_such_file_or_directory;
-return InnerFS->getBufferForFile(Name, FileSize, RequiresNullTerminator,
- IsVolatile);
- }
-
- virtual vfs::directory_iterator dir_begin(const Twine &Dir,
-std::error_code &EC) {
-if (!isInsideWhitelistedDir(Dir)) {
- EC = llvm::errc::no_such_file_or_directory;
- return vfs::directory_iterator();
-}
-return InnerFS->dir_begin(Dir, EC);
- }
-
- virtual std::error_code setCurrentWorkingDirectory(const Twine &Path) {
-return InnerFS->setCurrentWorkingDirectory(Path);
- }
-
- virtual llvm::ErrorOr getCurrentWorkingDirectory() const {
-return InnerFS->getCurrentWorkingDirectory();
- }
-
- bool exists(const Twine &Path) {
-if (!isInsideWhitelistedDir(Path))
- return false;
-return InnerFS->exists(Path);
- }
-
- std::error_code makeAbsolute(SmallVectorImpl &Path) const {
-return InnerFS->makeAbsolute(Path);
- }
-
-private:
- bool isInsideWhitelistedDir(const Twine &InputPath) const {
-SmallString<128> Path;
-InputPath.toVector(Path);
-
-if (makeAbsolute(Path))
- return false;
-
-for (const auto &Dir : WhitelistedDirs) {
- if (Path.startswith(Dir))
-return true;
-}
-return false;
- }
-
- std::vector WhitelistedDirs;
- IntrusiveRefCntPtr InnerFS;
-};
-
-/// Create a vfs::FileSystem that has access only to temporary directories
-/// (obtained by calling system_temp_directory).
-IntrusiveRefCntPtr getTempOnlyFS() {
- llvm::SmallString<128> TmpDir1;
- llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, TmpDir1);
- llvm::SmallString<128> TmpDir2;
- llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true, TmpDir2);
-
- std::vector TmpDirs;
- TmpDirs.push_back(TmpDir1.str());
- if (TmpDir1 != TmpDir2)
-TmpDirs.push_back(TmpDir2.str());
- return new FilteredFileSystem(std::move(TmpDirs), vfs::getRealFileSystem());
-}
-
-} // namespace
-
IntrusiveRefCntPtr
buildTestFS(llvm::StringMap const &Files) {
IntrusiveRefCntPtr MemFS(
@@ -126,11 +20,7 @@ buildTestFS(llvm::StringMap
MemFS->addFile(FileAndContents.first(), time_t(),
llvm::MemoryBuffer::getMemBuffer(FileAndContents.second,
FileAndContents.first()));
-
- auto OverlayFS = IntrusiveRefCntPtr(
- new vfs::OverlayFileSystem(getTempOnlyFS()));
- OverlayFS->pushOverlay(std::move(MemFS));
- return OverlayFS;
+ return MemFS;
}
Tagged>
___
cfe-commits mailing list
cfe-
[libcxxabi] r325023 - [demangler] Support for inheriting constructors.
Author: epilk
Date: Tue Feb 13 09:09:07 2018
New Revision: 325023
URL: http://llvm.org/viewvc/llvm-project?rev=325023&view=rev
Log:
[demangler] Support for inheriting constructors.
Fixes PR33223.
Modified:
libcxxabi/trunk/src/cxa_demangle.cpp
libcxxabi/trunk/test/test_demangle.pass.cpp
Modified: libcxxabi/trunk/src/cxa_demangle.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_demangle.cpp?rev=325023&r1=325022&r2=325023&view=diff
==
--- libcxxabi/trunk/src/cxa_demangle.cpp (original)
+++ libcxxabi/trunk/src/cxa_demangle.cpp Tue Feb 13 09:09:07 2018
@@ -2200,10 +2200,15 @@ Node *Db::parseCtorDtorName(Node *&SoFar
}
if (consumeIf('C')) {
+bool IsInherited = consumeIf('I');
if (look() != '1' && look() != '2' && look() != '3' && look() != '5')
return nullptr;
++First;
ParsedCtorDtorCV = true;
+if (IsInherited) {
+ if (legacyParse() == nullptr)
+return nullptr;
+}
return make(SoFar, false);
}
Modified: libcxxabi/trunk/test/test_demangle.pass.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/test/test_demangle.pass.cpp?rev=325023&r1=325022&r2=325023&view=diff
==
--- libcxxabi/trunk/test/test_demangle.pass.cpp (original)
+++ libcxxabi/trunk/test/test_demangle.pass.cpp Tue Feb 13 09:09:07 2018
@@ -29690,6 +29690,10 @@ const char* cases[][2] =
// Designated init expressions
{"_ZN15designated_init1fINS_1AEEEvDTtlT_di1adi1bdxLi3EdXLi1ELi4ELi9EEE",
"void
designated_init::f(decltype(designated_init::A{.a.b[3][1
... 4] = 9}))"},
{"_Z1fIXtl1Xdi1adi1bdxLi3ELi1", "f"},
+
+// Inheriting constructors:
+{"_ZN1BCI21AEi", "B::B(int)"},
+{"_ZN1DCI21CIiEET_", "D::D(int)"},
};
const unsigned N = sizeof(cases) / sizeof(cases[0]);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxxabi] r325022 - [demangler] Rewrite parse_nested_name in the new style.
Author: epilk
Date: Tue Feb 13 09:09:03 2018
New Revision: 325022
URL: http://llvm.org/viewvc/llvm-project?rev=325022&view=rev
Log:
[demangler] Rewrite parse_nested_name in the new style.
Modified:
libcxxabi/trunk/src/cxa_demangle.cpp
Modified: libcxxabi/trunk/src/cxa_demangle.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxxabi/trunk/src/cxa_demangle.cpp?rev=325022&r1=325021&r2=325022&view=diff
==
--- libcxxabi/trunk/src/cxa_demangle.cpp (original)
+++ libcxxabi/trunk/src/cxa_demangle.cpp Tue Feb 13 09:09:03 2018
@@ -1983,12 +1983,28 @@ struct Db {
FunctionRefQual RefQuals = FrefQualNone;
unsigned EncodingDepth = 0;
bool ParsedCtorDtorCV = false;
+ bool EndsWithTemplateArgs = false;
bool TagTemplates = true;
bool FixForwardReferences = false;
bool TryToParseTemplateArgs = true;
BumpPointerAllocator ASTAllocator;
+ // A couple of members of Db are local to a specific name. When recursively
+ // parsing names we need to swap and restore them all.
+ struct SwapAndRestoreNameState {
+SwapAndRestore SaveQualifiers;
+SwapAndRestore SaveRefQualifiers;
+SwapAndRestore SaveEndsWithTemplateArgs;
+SwapAndRestore SaveParsedCtorDtorCV;
+
+SwapAndRestoreNameState(Db &Parser)
+: SaveQualifiers(Parser.CV, QualNone),
+ SaveRefQualifiers(Parser.RefQuals, FrefQualNone),
+ SaveEndsWithTemplateArgs(Parser.EndsWithTemplateArgs, false),
+ SaveParsedCtorDtorCV(Parser.ParsedCtorDtorCV, false) {}
+ };
+
template T *make(Args &&... args) {
return new (ASTAllocator.allocate(sizeof(T)))
T(std::forward(args)...);
@@ -2063,6 +2079,10 @@ struct Db {
Node *parseClassEnumType();
Node *parseQualifiedType(bool &AppliesToFunction);
+ Node *parseNestedName();
+ Node *parseCtorDtorName(Node *&SoFar);
+ Node *parseAbiTags(Node *N);
+
// FIXME: remove this when all the parse_* functions have been rewritten.
template
Node *legacyParse() {
@@ -2090,6 +2110,19 @@ struct Db {
}
};
+const char *parse_nested_name(const char *first, const char *last, Db &db,
+ bool *endsWithTemplateArgs) {
+ db.First = first;
+ db.Last = last;
+ Node *R = db.parseNestedName();
+ if (endsWithTemplateArgs)
+*endsWithTemplateArgs = db.EndsWithTemplateArgs;
+ if (R == nullptr)
+return first;
+ db.Names.push_back(R);
+ return db.First;
+}
+
const char *parse_expression(const char *first, const char *last, Db &db) {
db.First = first;
db.Last = last;
@@ -2143,6 +2176,176 @@ const char *parse_decltype(const char *f
const char *parse_unresolved_name(const char *, const char *, Db &);
const char *parse_substitution(const char *, const char *, Db &);
+
+// ::= C1 # complete object constructor
+// ::= C2 # base object constructor
+// ::= C3 # complete object allocating constructor
+// extension ::= C5# ?
+// ::= D0 # deleting destructor
+// ::= D1 # complete object destructor
+// ::= D2 # base object destructor
+// extension ::= D5# ?
+Node *Db::parseCtorDtorName(Node *&SoFar) {
+ if (SoFar->K == Node::KSpecialSubstitution) {
+auto SSK = static_cast(SoFar)->SSK;
+switch (SSK) {
+case SpecialSubKind::string:
+case SpecialSubKind::istream:
+case SpecialSubKind::ostream:
+case SpecialSubKind::iostream:
+ SoFar = make(SSK);
+default:
+ break;
+}
+ }
+
+ if (consumeIf('C')) {
+if (look() != '1' && look() != '2' && look() != '3' && look() != '5')
+ return nullptr;
+++First;
+ParsedCtorDtorCV = true;
+return make(SoFar, false);
+ }
+
+ if (look() == 'D' &&
+ (look(1) == '0' || look(1) == '1' || look(1) == '2' || look(1) == '5')) {
+First += 2;
+ParsedCtorDtorCV = true;
+return make(SoFar, true);
+ }
+
+ return nullptr;
+}
+
+// ::= N [] []
E
+// ::= N [] []
E
+//
+// ::=
+// ::=
+// ::=
+// ::=
+// ::= # empty
+// ::=
+// ::=
+// extension ::= L
+//
+// ::=
+// ::=
+// ::=
+Node *Db::parseNestedName() {
+ if (!consumeIf('N'))
+return nullptr;
+
+ CV = parseCVQualifiers();
+ if (consumeIf('O')) RefQuals = FrefQualRValue;
+ else if (consumeIf('R')) RefQuals = FrefQualLValue;
+ else RefQuals = FrefQualNone;
+
+ Node *SoFar = nullptr;
+ auto PushComponent = [&](Node *Comp) {
+if (SoFar) SoFar = make(SoFar, Comp);
+else SoFar = Comp;
+EndsWithTemplateArgs = false;
+ };
+
+ if (consumeIf("St"))
+SoFar = make("std");
+
+ while (!consumeIf('E')) {
+consumeIf('L'); // extension
+
+// ::=
+if (look() == 'T') {
+ Node *TP = legacyParse();
+ if (TP == nullptr)
+return nullptr;
+ PushComponent(TP);
+ Sub
[clang-tools-extra] r325024 - [clangd] Log if CWD could not be changed. NFC.
Author: ibiryukov
Date: Tue Feb 13 09:15:06 2018
New Revision: 325024
URL: http://llvm.org/viewvc/llvm-project?rev=325024&view=rev
Log:
[clangd] Log if CWD could not be changed. NFC.
Modified:
clang-tools-extra/trunk/clangd/ClangdUnit.cpp
clang-tools-extra/trunk/clangd/CodeComplete.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=325024&r1=325023&r2=325024&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Tue Feb 13 09:15:06 2018
@@ -379,7 +379,11 @@ CppFile::rebuild(ParseInputs &&Inputs) {
for (const auto &S : Inputs.CompileCommand.CommandLine)
ArgStrs.push_back(S.c_str());
- Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory);
+ if (Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) {
+log("Couldn't set working directory");
+// We run parsing anyway, our lit-tests rely on results for non-existing
+// working dirs.
+ }
// Prepare CompilerInvocation.
std::unique_ptr CI;
Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=325024&r1=325023&r2=325024&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Tue Feb 13 09:15:06 2018
@@ -642,7 +642,11 @@ bool semaCodeComplete(std::unique_ptrsetCurrentWorkingDirectory(Input.Command.Directory);
+ if (Input.VFS->setCurrentWorkingDirectory(Input.Command.Directory)) {
+log("Couldn't set working directory");
+// We run parsing anyway, our lit-tests rely on results for non-existing
+// working dirs.
+ }
IgnoreDiagnostics DummyDiagsConsumer;
auto CI = createInvocationFromCommandLine(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r325027 - Put type attributes after class keyword
Author: dim
Date: Tue Feb 13 09:40:59 2018
New Revision: 325027
URL: http://llvm.org/viewvc/llvm-project?rev=325027&view=rev
Log:
Put type attributes after class keyword
Summary:
Compiling `` in C++17 or higher mode results in:
```
functional:2500:1: warning: attribute '__visibility__' is ignored, place it
after "class" to apply attribute to type declaration [-Wignored-attributes]
_LIBCPP_TYPE_VIS
^
__config:701:46: note: expanded from macro '_LIBCPP_TYPE_VIS'
#define _LIBCPP_TYPE_VIS __attribute__ ((__visibility__("default")))
^
1 warning generated.
```
Fix it by putting the attribute after the `class` keyword.
Reviewers: EricWF, mclow.lists
Reviewed By: EricWF
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D43209
Modified:
libcxx/trunk/include/functional
Modified: libcxx/trunk/include/functional
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/functional?rev=325027&r1=325026&r2=325027&view=diff
==
--- libcxx/trunk/include/functional (original)
+++ libcxx/trunk/include/functional Tue Feb 13 09:40:59 2018
@@ -2497,8 +2497,7 @@ __search(_RandomAccessIterator1 __first1
// default searcher
template>
-_LIBCPP_TYPE_VIS
-class default_searcher {
+class _LIBCPP_TYPE_VIS default_searcher {
public:
_LIBCPP_INLINE_VISIBILITY
default_searcher(_ForwardIterator __f, _ForwardIterator __l,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43209: Put type attributes after class keyword
This revision was automatically updated to reflect the committed changes.
Closed by commit rCXX325027: Put type attributes after class keyword (authored
by dim, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D43209?vs=133932&id=134065#toc
Repository:
rCXX libc++
https://reviews.llvm.org/D43209
Files:
include/functional
Index: include/functional
===
--- include/functional
+++ include/functional
@@ -2497,8 +2497,7 @@
// default searcher
template>
-_LIBCPP_TYPE_VIS
-class default_searcher {
+class _LIBCPP_TYPE_VIS default_searcher {
public:
_LIBCPP_INLINE_VISIBILITY
default_searcher(_ForwardIterator __f, _ForwardIterator __l,
Index: include/functional
===
--- include/functional
+++ include/functional
@@ -2497,8 +2497,7 @@
// default searcher
template>
-_LIBCPP_TYPE_VIS
-class default_searcher {
+class _LIBCPP_TYPE_VIS default_searcher {
public:
_LIBCPP_INLINE_VISIBILITY
default_searcher(_ForwardIterator __f, _ForwardIterator __l,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r325028 - Make the ctype_byname::widen test cases pass on FreeBSD.
Author: dim
Date: Tue Feb 13 09:43:24 2018
New Revision: 325028
URL: http://llvm.org/viewvc/llvm-project?rev=325028&view=rev
Log:
Make the ctype_byname::widen test cases pass on FreeBSD.
Modified:
libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_1.pass.cpp
libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_many.pass.cpp
Modified:
libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_1.pass.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_1.pass.cpp?rev=325028&r1=325027&r2=325028&view=diff
==
---
libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_1.pass.cpp
(original)
+++
libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_1.pass.cpp
Tue Feb 13 09:43:24 2018
@@ -55,7 +55,7 @@ int main()
assert(f.widen('.') == L'.');
assert(f.widen('a') == L'a');
assert(f.widen('1') == L'1');
-#ifdef __APPLE__
+#if defined(__APPLE__) || defined(__FreeBSD__)
assert(f.widen(char(-5)) == L'\u00fb');
#else
assert(f.widen(char(-5)) == wchar_t(-1));
Modified:
libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_many.pass.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_many.pass.cpp?rev=325028&r1=325027&r2=325028&view=diff
==
---
libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_many.pass.cpp
(original)
+++
libcxx/trunk/test/std/localization/locale.categories/category.ctype/locale.ctype.byname/widen_many.pass.cpp
Tue Feb 13 09:43:24 2018
@@ -61,7 +61,7 @@ int main()
assert(v[3] == L'.');
assert(v[4] == L'a');
assert(v[5] == L'1');
-#ifdef __APPLE__
+#if defined(__APPLE__) || defined(__FreeBSD__)
assert(v[6] == L'\x85');
#else
assert(v[6] == wchar_t(-1));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r325029 - [clangd] Fixed findDefinitions to always return absolute paths.
Author: ibiryukov
Date: Tue Feb 13 09:47:16 2018
New Revision: 325029
URL: http://llvm.org/viewvc/llvm-project?rev=325029&view=rev
Log:
[clangd] Fixed findDefinitions to always return absolute paths.
Relative paths could be returned in some cases, e.g. when relative
path is used in compilation arguments. This led to crash when trying
to convert the path to URI.
Modified:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/TestFS.cpp
clang-tools-extra/trunk/unittests/clangd/TestFS.h
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=325029&r1=325028&r2=325029&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Tue Feb 13 09:47:16 2018
@@ -11,6 +11,7 @@
#include "URI.h"
#include "clang/Index/IndexDataConsumer.h"
#include "clang/Index/IndexingAction.h"
+#include "llvm/Support/Path.h"
namespace clang {
namespace clangd {
using namespace llvm;
@@ -138,10 +139,17 @@ getDeclarationLocation(ParsedAST &AST, c
Range R = {Begin, End};
Location L;
- StringRef FilePath = F->tryGetRealPathName();
+ SmallString<64> FilePath = F->tryGetRealPathName();
if (FilePath.empty())
FilePath = F->getName();
- L.uri.file = FilePath;
+ if (!llvm::sys::path::is_absolute(FilePath)) {
+if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath)) {
+ log("Could not turn relative path to absolute: " + FilePath);
+ return llvm::None;
+}
+ }
+
+ L.uri.file = FilePath.str();
L.range = R;
return L;
}
Modified: clang-tools-extra/trunk/unittests/clangd/TestFS.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestFS.cpp?rev=325029&r1=325028&r2=325029&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/TestFS.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestFS.cpp Tue Feb 13 09:47:16 2018
@@ -33,8 +33,10 @@ MockFSProvider::getTaggedFileSystem(Path
return make_tagged(FS, Tag);
}
-MockCompilationDatabase::MockCompilationDatabase()
-: ExtraClangFlags({"-ffreestanding"}) {} // Avoid implicit stdc-predef.h.
+MockCompilationDatabase::MockCompilationDatabase(bool UseRelPaths)
+: ExtraClangFlags({"-ffreestanding"}), UseRelPaths(UseRelPaths) {
+ // -ffreestanding avoids implicit stdc-predef.h.
+}
llvm::Optional
MockCompilationDatabase::getCompileCommand(PathRef File) const {
@@ -42,10 +44,10 @@ MockCompilationDatabase::getCompileComma
return llvm::None;
auto CommandLine = ExtraClangFlags;
+ auto FileName = llvm::sys::path::filename(File);
CommandLine.insert(CommandLine.begin(), "clang");
- CommandLine.insert(CommandLine.end(), File.str());
- return {tooling::CompileCommand(llvm::sys::path::parent_path(File),
- llvm::sys::path::filename(File),
+ CommandLine.insert(CommandLine.end(), UseRelPaths ? FileName : File);
+ return {tooling::CompileCommand(llvm::sys::path::parent_path(File), FileName,
std::move(CommandLine), "")};
}
Modified: clang-tools-extra/trunk/unittests/clangd/TestFS.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestFS.h?rev=325029&r1=325028&r2=325029&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/TestFS.h (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestFS.h Tue Feb 13 09:47:16 2018
@@ -37,12 +37,15 @@ public:
// A Compilation database that returns a fixed set of compile flags.
class MockCompilationDatabase : public GlobalCompilationDatabase {
public:
- MockCompilationDatabase();
+ /// When \p UseRelPaths is true, uses relative paths in compile commands.
+ /// When \p UseRelPaths is false, uses absoulte paths.
+ MockCompilationDatabase(bool UseRelPaths = false);
llvm::Optional
getCompileCommand(PathRef File) const override;
std::vector ExtraClangFlags;
+ const bool UseRelPaths;
};
// Returns an absolute (fake) test directory for this OS.
Modified: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp?rev=325029&r1=325028&r2=325029&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Tue Feb 13 09:47:16
2018
@@ -8,6 +8,7 @@
//===--===//
#include "Annotations.h"
#include "ClangdUnit.h"
+#include "Compiler.h"
#include "Matchers.h"
#in
[PATCH] D42606: [Coroutines] Use allocator overload when available
GorNishanov added inline comments.
Comment at: lib/Sema/SemaCoroutine.cpp:1062
+ // an argument list."
+ for (auto *PD : FD.parameters()) {
+if (PD->getType()->isDependentType())
This does not implement TS specified behavior for non static member functions:
[dcl.fct.def.coroutine]/7 states that an argument list is build up as follows:
> The first argument is the amount of space requested, and has type
> std::size_t. The lvalues p1 ... pn are the succeeding arguments.
Where p1 ... pn are defined earlier in
[dcl.fct.def.coroutine]/3 as:
> For a coroutine f that is a non-static member function, let P1 denote the
> type of the implicit object parameter (13.3.1) and P2 ... Pn be the types of
> the function parameters; otherwise let P1 ... Pn be the types of the function
> parameters.
Essentially for non-static member functions, we need to insert implicit object
parameter.
Note that lookupPromiseType in SemaCoroutine.cpp when building specialization
of `std::experimental::coroutine_traits<...>` includes implicit object
parameter:
```
// If the function is a non-static member function, add the type
// of the implicit object parameter before the formal parameters.
if (auto *MD = dyn_cast(FD)) {
if (MD->isInstance()) {
// [over.match.funcs]4
// For non-static member functions, the type of the implicit object
// parameter is
// -- "lvalue reference to cv X" for functions declared without a
// ref-qualifier or with the & ref-qualifier
// -- "rvalue reference to cv X" for functions declared with the &&
// ref-qualifier
QualType T =
MD->getThisType(S.Context)->getAs()->getPointeeType();
T = FnType->getRefQualifier() == RQ_RValue
? S.Context.getRValueReferenceType(T)
: S.Context.getLValueReferenceType(T, /*SpelledAsLValue*/ true);
AddArg(T);
}
}
```
Repository:
rC Clang
https://reviews.llvm.org/D42606
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42606: [Coroutines] Use allocator overload when available
GorNishanov added inline comments.
Comment at: lib/Sema/SemaCoroutine.cpp:1062
+ // an argument list."
+ for (auto *PD : FD.parameters()) {
+if (PD->getType()->isDependentType())
GorNishanov wrote:
> This does not implement TS specified behavior for non static member functions:
>
> [dcl.fct.def.coroutine]/7 states that an argument list is build up as
> follows:
>
> > The first argument is the amount of space requested, and has type
> > std::size_t. The lvalues p1 ... pn are the succeeding arguments.
>
> Where p1 ... pn are defined earlier in
>
> [dcl.fct.def.coroutine]/3 as:
>
> > For a coroutine f that is a non-static member function, let P1 denote the
> > type of the implicit object parameter (13.3.1) and P2 ... Pn be the types
> > of the function parameters; otherwise let P1 ... Pn be the types of the
> > function parameters.
>
> Essentially for non-static member functions, we need to insert implicit
> object parameter.
>
> Note that lookupPromiseType in SemaCoroutine.cpp when building specialization
> of `std::experimental::coroutine_traits<...>` includes implicit object
> parameter:
>
> ```
> // If the function is a non-static member function, add the type
> // of the implicit object parameter before the formal parameters.
> if (auto *MD = dyn_cast(FD)) {
> if (MD->isInstance()) {
> // [over.match.funcs]4
> // For non-static member functions, the type of the implicit object
> // parameter is
> // -- "lvalue reference to cv X" for functions declared without a
> // ref-qualifier or with the & ref-qualifier
> // -- "rvalue reference to cv X" for functions declared with the &&
> // ref-qualifier
> QualType T =
> MD->getThisType(S.Context)->getAs()->getPointeeType();
> T = FnType->getRefQualifier() == RQ_RValue
> ? S.Context.getRValueReferenceType(T)
> : S.Context.getLValueReferenceType(T, /*SpelledAsLValue*/ true);
> AddArg(T);
> }
> }
> ```
I think promise constructor argument preview is also missing the implicit
object parameter.
Repository:
rC Clang
https://reviews.llvm.org/D42606
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43246: [clangd] Assert path is absolute when assigning to URIForFile.
ilya-biryukov created this revision.
ilya-biryukov added reviewers: hokein, ioeric, sammccall.
Herald added subscribers: jkorous-apple, klimek.
The assertion will point directly to misbehaving code, so that
debugging related problems (like the one fixed by r325029) is easier.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43246
Files:
clangd/ClangdLSPServer.cpp
clangd/Protocol.cpp
clangd/Protocol.h
clangd/XRefs.cpp
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -149,7 +149,7 @@
}
}
- L.uri.file = FilePath.str();
+ L.uri.setFile(FilePath.str());
L.range = R;
return L;
}
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -25,6 +25,7 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PROTOCOL_H
#include "JSONExpr.h"
+#include "Path.h"
#include "URI.h"
#include "llvm/ADT/Optional.h"
#include
@@ -49,21 +50,28 @@
};
struct URIForFile {
- std::string file;
+ URIForFile() = default;
+ URIForFile(Path AbsPath) { setFile(AbsPath); }
- std::string uri() const { return URI::createFile(file).toString(); }
+ Path getFile() const { return File; }
+ void setFile(Path AbsPath);
+
+ std::string uri() const { return URI::createFile(File).toString(); }
friend bool operator==(const URIForFile &LHS, const URIForFile &RHS) {
-return LHS.file == RHS.file;
+return LHS.File == RHS.File;
}
friend bool operator!=(const URIForFile &LHS, const URIForFile &RHS) {
return !(LHS == RHS);
}
friend bool operator<(const URIForFile &LHS, const URIForFile &RHS) {
-return LHS.file < RHS.file;
+return LHS.File < RHS.File;
}
+
+private:
+ Path File;
};
/// Serialize/deserialize \p URIForFile to/from a string URI.
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -12,8 +12,8 @@
//===--===//
#include "Protocol.h"
-#include "URI.h"
#include "Logger.h"
+#include "URI.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/Support/Format.h"
@@ -24,6 +24,11 @@
namespace clang {
namespace clangd {
+void URIForFile::setFile(Path AbsPath) {
+ assert(llvm::sys::path::is_absolute(AbsPath) && "the path is relative");
+ File = std::move(AbsPath);
+}
+
bool fromJSON(const json::Expr &E, URIForFile &R) {
if (auto S = E.asString()) {
auto U = URI::parse(*S);
@@ -40,15 +45,13 @@
log(llvm::toString(Path.takeError()));
return false;
}
-R.file = *Path;
+R.setFile(*Path);
return true;
}
return false;
}
-json::Expr toJSON(const URIForFile &U) {
- return U.uri();
-}
+json::Expr toJSON(const URIForFile &U) { return U.uri(); }
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const URIForFile &U) {
return OS << U.uri();
Index: clangd/ClangdLSPServer.cpp
===
--- clangd/ClangdLSPServer.cpp
+++ clangd/ClangdLSPServer.cpp
@@ -116,8 +116,8 @@
{"commands", {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND}},
}},
);
- if (Params.rootUri && !Params.rootUri->file.empty())
-Server.setRootPath(Params.rootUri->file);
+ if (Params.rootUri && !Params.rootUri->getFile().empty())
+Server.setRootPath(Params.rootUri->getFile());
else if (Params.rootPath && !Params.rootPath->empty())
Server.setRootPath(*Params.rootPath);
}
@@ -132,17 +132,17 @@
void ClangdLSPServer::onDocumentDidOpen(DidOpenTextDocumentParams &Params) {
if (Params.metadata && !Params.metadata->extraFlags.empty())
-CDB.setExtraFlagsForFile(Params.textDocument.uri.file,
+CDB.setExtraFlagsForFile(Params.textDocument.uri.getFile(),
std::move(Params.metadata->extraFlags));
- Server.addDocument(Params.textDocument.uri.file, Params.textDocument.text);
+ Server.addDocument(Params.textDocument.uri.getFile(), Params.textDocument.text);
}
void ClangdLSPServer::onDocumentDidChange(DidChangeTextDocumentParams &Params) {
if (Params.contentChanges.size() != 1)
return replyError(ErrorCode::InvalidParams,
"can only apply one change at a time");
// We only support full syncing right now.
- Server.addDocument(Params.textDocument.uri.file,
+ Server.addDocument(Params.textDocument.uri.getFile(),
Params.contentChanges[0].text);
}
@@ -180,7 +180,7 @@
}
void ClangdLSPServer::onRename(RenameParams &Params) {
- auto File = Params.textDocument.uri.file;
+ auto File = Params.textDocument.uri.getFile();
auto Code = Server.getDocument(File);
if (!Code)
return replyError(ErrorCode::InvalidParams,
@@ -200,12 +200,12 @@
}
void ClangdLSPServer::onDocumentDidClos
[PATCH] D43120: [clang-tidy] New checker for exceptions that are created but not thrown
aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.
I apologize for not noticing this important detail earlier -- I think this
check should live under `bugprone` rather than `misc`. Other than where it
lives (and the related naming issues), I think this is looking good!
Comment at: test/clang-tidy/misc-throw-keyword-missing.cpp:38
+
+// Classname contains the substring "exception", in certain cases using this
class should emit a warning.
+struct RegularException {
Classname -> Class name
https://reviews.llvm.org/D43120
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based
ioeric added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:194
+if (!Replacements)
+ return replyError(ErrorCode::InternalError,
+llvm::toString(Replacements.takeError()));
nit: since we are not spelling out the return type here, it might be clearer if
we do:
```
replyError(...);
return;
```
`return replyError(...)` makes me wonder what the return type is.
Comment at: clangd/ClangdServer.cpp:209
+ auto Action = [Contents, Pos, TaggedFS,
+ PCHs](Path File, decltype(Callback) Callback,
+ llvm::Expected IP) {
nit: I'd probably use a different name than `Callback` for this parameter for
clarity.
Comment at: clangd/ClangdServer.cpp:441
- using RetType = llvm::Expected>>;
- auto Action = [=](llvm::Expected InpAST) -> RetType {
+ auto Action = [=](decltype(Callback) Callback,
+llvm::Expected InpAST) {
Consider spelling out the captured values, just in case new variables which are
not expected to be captured are added in the future.
Comment at: unittests/clangd/SyncAPI.cpp:69
+ llvm::Expected> Result = Tagged();
+ (void)(bool)Result; // Expected has to be checked.
+ Server.signatureHelp(File, Pos, capture(Result), OverridenContents);
I'd expect this to be checked by callers. Would `return std::move(Result);`
work?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43227
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h
ilya-biryukov added inline comments.
Comment at: clangd/Protocol.h:190
/// (see exit notification) its process.
- llvm::Optional processId;
+ llvm::Optional processId = 0;
jkorous-apple wrote:
> Sorry if I am asking stupid question but since the type is Optional<> isn't
> it's default-constructed value more appropriate here?
Totally, my mistake. Fixed now.
Comment at: clangd/Protocol.h:211
/// The initial trace setting. If omitted trace is disabled ('off').
- llvm::Optional trace;
+ llvm::Optional trace = TraceLevel::Off;
};
jkorous-apple wrote:
> Does it still make more sense to use Optional than plain
> TraceLevel?
Optional here makes sense (LSP defines the field as nullable), the intializer
didn't. Fixed now.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43171: [AMDGPU] Change constant addr space to 4 for clang
This revision was automatically updated to reflect the committed changes.
Closed by commit rL325031: [AMDGPU] Change constant addr space to 4 (authored
by yaxunl, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D43171?vs=133802&id=134070#toc
Repository:
rL LLVM
https://reviews.llvm.org/D43171
Files:
cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
cfe/trunk/test/CodeGen/target-data.c
cfe/trunk/test/CodeGenOpenCL/address-space-constant-initializers.cl
cfe/trunk/test/CodeGenOpenCL/address-spaces.cl
cfe/trunk/test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl
cfe/trunk/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
cfe/trunk/test/CodeGenOpenCL/amdgpu-env-amdgiz.cl
cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
cfe/trunk/test/CodeGenOpenCL/cast_image.cl
cfe/trunk/test/CodeGenOpenCL/opencl_types.cl
cfe/trunk/test/CodeGenOpenCL/private-array-initialization.cl
cfe/trunk/test/CodeGenOpenCL/size_t.cl
cfe/trunk/test/CodeGenOpenCL/vla.cl
Index: cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
===
--- cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
+++ cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def
@@ -21,9 +21,9 @@
// SI+ only builtins.
//===--===//
-BUILTIN(__builtin_amdgcn_dispatch_ptr, "Uc*2", "nc")
-BUILTIN(__builtin_amdgcn_kernarg_segment_ptr, "Uc*2", "nc")
-BUILTIN(__builtin_amdgcn_implicitarg_ptr, "Uc*2", "nc")
+BUILTIN(__builtin_amdgcn_dispatch_ptr, "Uc*4", "nc")
+BUILTIN(__builtin_amdgcn_kernarg_segment_ptr, "Uc*4", "nc")
+BUILTIN(__builtin_amdgcn_implicitarg_ptr, "Uc*4", "nc")
BUILTIN(__builtin_amdgcn_workgroup_id_x, "Ui", "nc")
BUILTIN(__builtin_amdgcn_workgroup_id_y, "Ui", "nc")
Index: cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
===
--- cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
+++ cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl
@@ -434,22 +434,22 @@
}
// CHECK-LABEL: @test_dispatch_ptr
-// CHECK: call i8 addrspace(2)* @llvm.amdgcn.dispatch.ptr()
-void test_dispatch_ptr(__attribute__((address_space(2))) unsigned char ** out)
+// CHECK: call i8 addrspace(4)* @llvm.amdgcn.dispatch.ptr()
+void test_dispatch_ptr(__attribute__((address_space(4))) unsigned char ** out)
{
*out = __builtin_amdgcn_dispatch_ptr();
}
// CHECK-LABEL: @test_kernarg_segment_ptr
-// CHECK: call i8 addrspace(2)* @llvm.amdgcn.kernarg.segment.ptr()
-void test_kernarg_segment_ptr(__attribute__((address_space(2))) unsigned char ** out)
+// CHECK: call i8 addrspace(4)* @llvm.amdgcn.kernarg.segment.ptr()
+void test_kernarg_segment_ptr(__attribute__((address_space(4))) unsigned char ** out)
{
*out = __builtin_amdgcn_kernarg_segment_ptr();
}
// CHECK-LABEL: @test_implicitarg_ptr
-// CHECK: call i8 addrspace(2)* @llvm.amdgcn.implicitarg.ptr()
-void test_implicitarg_ptr(__attribute__((address_space(2))) unsigned char ** out)
+// CHECK: call i8 addrspace(4)* @llvm.amdgcn.implicitarg.ptr()
+void test_implicitarg_ptr(__attribute__((address_space(4))) unsigned char ** out)
{
*out = __builtin_amdgcn_implicitarg_ptr();
}
Index: cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
===
--- cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
+++ cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -30,7 +30,7 @@
// CHECK: @global_p = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 8
global char *global_p = 0;
-// CHECK: @constant_p = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 8
+// CHECK: @constant_p = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 8
constant char *constant_p = 0;
// CHECK: @generic_p = local_unnamed_addr addrspace(1) global i8* null, align 8
@@ -47,7 +47,7 @@
// CHECK: @global_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(1)* null, align 8
global char *global_p_NULL = NULL;
-// CHECK: @constant_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(2)* null, align 8
+// CHECK: @constant_p_NULL = local_unnamed_addr addrspace(1) global i8 addrspace(4)* null, align 8
constant char *constant_p_NULL = NULL;
// CHECK: @generic_p_NULL = local_unnamed_addr addrspace(1) global i8* null, align 8
@@ -104,7 +104,7 @@
// NOOPT: @test_static_var_private.sp3 = internal addrspace(1) global i8 addrspace(5)* null, align 4
// NOOPT: @test_static_var_private.sp4 = internal addrspace(1) global i8 addrspace(5)* null, align 4
// NOOPT: @test_static_var_private.sp5 = internal addrspace(1) global i8 addrspace(5)* null, align 4
-// NOOPT: @test_static_var_private.SS1 = internal addrspace(1) global %struct.StructTy1 { i8 addrspace(5)* null, i8 addrspace(3)* addrspacecast (i8* null to i8 add
r325031 - [AMDGPU] Change constant addr space to 4
Author: yaxunl Date: Tue Feb 13 10:01:21 2018 New Revision: 325031 URL: http://llvm.org/viewvc/llvm-project?rev=325031&view=rev Log: [AMDGPU] Change constant addr space to 4 Differential Revision: https://reviews.llvm.org/D43171 Added: cfe/trunk/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl Removed: cfe/trunk/test/CodeGenOpenCL/amdgpu-env-amdgiz.cl Modified: cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def cfe/trunk/lib/Basic/Targets/AMDGPU.cpp cfe/trunk/test/CodeGen/target-data.c cfe/trunk/test/CodeGenOpenCL/address-space-constant-initializers.cl cfe/trunk/test/CodeGenOpenCL/address-spaces.cl cfe/trunk/test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl cfe/trunk/test/CodeGenOpenCL/amdgpu-nullptr.cl cfe/trunk/test/CodeGenOpenCL/builtins-amdgcn.cl cfe/trunk/test/CodeGenOpenCL/cast_image.cl cfe/trunk/test/CodeGenOpenCL/opencl_types.cl cfe/trunk/test/CodeGenOpenCL/private-array-initialization.cl cfe/trunk/test/CodeGenOpenCL/size_t.cl cfe/trunk/test/CodeGenOpenCL/vla.cl Modified: cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def?rev=325031&r1=325030&r2=325031&view=diff == --- cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def (original) +++ cfe/trunk/include/clang/Basic/BuiltinsAMDGPU.def Tue Feb 13 10:01:21 2018 @@ -21,9 +21,9 @@ // SI+ only builtins. //===--===// -BUILTIN(__builtin_amdgcn_dispatch_ptr, "Uc*2", "nc") -BUILTIN(__builtin_amdgcn_kernarg_segment_ptr, "Uc*2", "nc") -BUILTIN(__builtin_amdgcn_implicitarg_ptr, "Uc*2", "nc") +BUILTIN(__builtin_amdgcn_dispatch_ptr, "Uc*4", "nc") +BUILTIN(__builtin_amdgcn_kernarg_segment_ptr, "Uc*4", "nc") +BUILTIN(__builtin_amdgcn_implicitarg_ptr, "Uc*4", "nc") BUILTIN(__builtin_amdgcn_workgroup_id_x, "Ui", "nc") BUILTIN(__builtin_amdgcn_workgroup_id_y, "Ui", "nc") Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AMDGPU.cpp?rev=325031&r1=325030&r2=325031&view=diff == --- cfe/trunk/lib/Basic/Targets/AMDGPU.cpp (original) +++ cfe/trunk/lib/Basic/Targets/AMDGPU.cpp Tue Feb 13 10:01:21 2018 @@ -38,7 +38,7 @@ static const char *const DataLayoutStrin "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64"; static const char *const DataLayoutStringSIGenericIsZero = -"e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:32:32-p5:32:32-p6:32:32" +"e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32" "-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128" "-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5"; @@ -46,11 +46,11 @@ static const LangASMap AMDGPUPrivIsZeroD 4, // Default 1, // opencl_global 3, // opencl_local -2, // opencl_constant +4, // opencl_constant 0, // opencl_private 4, // opencl_generic 1, // cuda_device -2, // cuda_constant +4, // cuda_constant 3 // cuda_shared }; @@ -58,11 +58,11 @@ static const LangASMap AMDGPUGenIsZeroDe 0, // Default 1, // opencl_global 3, // opencl_local -2, // opencl_constant +4, // opencl_constant 5, // opencl_private 0, // opencl_generic 1, // cuda_device -2, // cuda_constant +4, // cuda_constant 3 // cuda_shared }; @@ -70,11 +70,11 @@ static const LangASMap AMDGPUPrivIsZeroD 0, // Default 1, // opencl_global 3, // opencl_local -2, // opencl_constant +4, // opencl_constant 0, // opencl_private 4, // opencl_generic 1, // cuda_device -2, // cuda_constant +4, // cuda_constant 3 // cuda_shared }; @@ -82,11 +82,11 @@ static const LangASMap AMDGPUGenIsZeroDe 5, // Default 1, // opencl_global 3, // opencl_local -2, // opencl_constant +4, // opencl_constant 5, // opencl_private 0, // opencl_generic 1, // cuda_device -2, // cuda_constant +4, // cuda_constant 3 // cuda_shared }; } // namespace targets Modified: cfe/trunk/test/CodeGen/target-data.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/target-data.c?rev=325031&r1=325030&r2=325031&view=diff == --- cfe/trunk/test/CodeGen/target-data.c (original) +++ cfe/trunk/test/CodeGen/target-data.c Tue Feb 13 10:01:21 2018 @@ -132,12 +132,12 @@ // RUN: %clang_cc1 -triple amdgcn-unknown -target-cpu hawaii -o - -emit-llvm %s \ // RUN: | FileCheck %s -check-prefix=R600SI -// R600SI: target datalayout = "e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:32:32-p5:32:32-p6:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5" +// R600SI: target datalayout = "e-p:64:64-p1:6
[PATCH] D43171: [AMDGPU] Change constant addr space to 4 for clang
This revision was automatically updated to reflect the committed changes.
Closed by commit rC325031: [AMDGPU] Change constant addr space to 4 (authored
by yaxunl, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D43171?vs=133802&id=134071#toc
Repository:
rC Clang
https://reviews.llvm.org/D43171
Files:
include/clang/Basic/BuiltinsAMDGPU.def
lib/Basic/Targets/AMDGPU.cpp
test/CodeGen/target-data.c
test/CodeGenOpenCL/address-space-constant-initializers.cl
test/CodeGenOpenCL/address-spaces.cl
test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl
test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
test/CodeGenOpenCL/amdgpu-env-amdgiz.cl
test/CodeGenOpenCL/amdgpu-nullptr.cl
test/CodeGenOpenCL/builtins-amdgcn.cl
test/CodeGenOpenCL/cast_image.cl
test/CodeGenOpenCL/opencl_types.cl
test/CodeGenOpenCL/private-array-initialization.cl
test/CodeGenOpenCL/size_t.cl
test/CodeGenOpenCL/vla.cl
Index: include/clang/Basic/BuiltinsAMDGPU.def
===
--- include/clang/Basic/BuiltinsAMDGPU.def
+++ include/clang/Basic/BuiltinsAMDGPU.def
@@ -21,9 +21,9 @@
// SI+ only builtins.
//===--===//
-BUILTIN(__builtin_amdgcn_dispatch_ptr, "Uc*2", "nc")
-BUILTIN(__builtin_amdgcn_kernarg_segment_ptr, "Uc*2", "nc")
-BUILTIN(__builtin_amdgcn_implicitarg_ptr, "Uc*2", "nc")
+BUILTIN(__builtin_amdgcn_dispatch_ptr, "Uc*4", "nc")
+BUILTIN(__builtin_amdgcn_kernarg_segment_ptr, "Uc*4", "nc")
+BUILTIN(__builtin_amdgcn_implicitarg_ptr, "Uc*4", "nc")
BUILTIN(__builtin_amdgcn_workgroup_id_x, "Ui", "nc")
BUILTIN(__builtin_amdgcn_workgroup_id_y, "Ui", "nc")
Index: test/CodeGenOpenCL/vla.cl
===
--- test/CodeGenOpenCL/vla.cl
+++ test/CodeGenOpenCL/vla.cl
@@ -1,13 +1,14 @@
// RUN: %clang_cc1 -emit-llvm -triple "spir-unknown-unknown" -O0 -cl-std=CL2.0 -o - %s | FileCheck -check-prefixes=CHECK,SPIR %s
-// RUN: %clang_cc1 -emit-llvm -triple amdgcn-amd-amdhsa-opencl -O0 -cl-std=CL2.0 -o - %s | FileCheck -check-prefixes=CHECK,SPIR %s
-// RUN: %clang_cc1 -emit-llvm -triple amdgcn-amd-amdhsa-amdgizcl -O0 -cl-std=CL2.0 -o - %s | FileCheck -check-prefixes=CHECK,GIZ %s
+// RUN: %clang_cc1 -emit-llvm -triple amdgcn-amd-amdhsa -O0 -cl-std=CL2.0 -o - %s | FileCheck -check-prefixes=CHECK,AMD %s
constant int sz0 = 5;
-// CHECK: @sz0 = addrspace(2) constant i32 5
+// SPIR: @sz0 = addrspace(2) constant i32 5
+// AMD: @sz0 = addrspace(4) constant i32 5
const global int sz1 = 16;
// CHECK: @sz1 = addrspace(1) constant i32 16
const constant int sz2 = 8;
-// CHECK: @sz2 = addrspace(2) constant i32 8
+// SPIR: @sz2 = addrspace(2) constant i32 8
+// AMD: @sz2 = addrspace(4) constant i32 8
// CHECK: @testvla.vla2 = internal addrspace(3) global [8 x i16] undef
kernel void testvla()
Index: test/CodeGenOpenCL/private-array-initialization.cl
===
--- test/CodeGenOpenCL/private-array-initialization.cl
+++ test/CodeGenOpenCL/private-array-initialization.cl
@@ -10,7 +10,7 @@
// PRIVATE5: %arr = alloca [3 x i32], align 4, addrspace(5)
// PRIVATE5: %0 = bitcast [3 x i32] addrspace(5)* %arr to i8 addrspace(5)*
-// PRIVATE5: call void @llvm.memcpy.p5i8.p2i8.i64(i8 addrspace(5)* align 4 %0, i8 addrspace(2)* align 4 bitcast ([3 x i32] addrspace(2)* @test.arr to i8 addrspace(2)*), i64 12, i1 false)
+// PRIVATE5: call void @llvm.memcpy.p5i8.p4i8.i64(i8 addrspace(5)* align 4 %0, i8 addrspace(4)* align 4 bitcast ([3 x i32] addrspace(4)* @test.arr to i8 addrspace(4)*), i64 12, i1 false)
}
__kernel void initializer_cast_is_valid_crash() {
Index: test/CodeGenOpenCL/builtins-amdgcn.cl
===
--- test/CodeGenOpenCL/builtins-amdgcn.cl
+++ test/CodeGenOpenCL/builtins-amdgcn.cl
@@ -434,22 +434,22 @@
}
// CHECK-LABEL: @test_dispatch_ptr
-// CHECK: call i8 addrspace(2)* @llvm.amdgcn.dispatch.ptr()
-void test_dispatch_ptr(__attribute__((address_space(2))) unsigned char ** out)
+// CHECK: call i8 addrspace(4)* @llvm.amdgcn.dispatch.ptr()
+void test_dispatch_ptr(__attribute__((address_space(4))) unsigned char ** out)
{
*out = __builtin_amdgcn_dispatch_ptr();
}
// CHECK-LABEL: @test_kernarg_segment_ptr
-// CHECK: call i8 addrspace(2)* @llvm.amdgcn.kernarg.segment.ptr()
-void test_kernarg_segment_ptr(__attribute__((address_space(2))) unsigned char ** out)
+// CHECK: call i8 addrspace(4)* @llvm.amdgcn.kernarg.segment.ptr()
+void test_kernarg_segment_ptr(__attribute__((address_space(4))) unsigned char ** out)
{
*out = __builtin_amdgcn_kernarg_segment_ptr();
}
// CHECK-LABEL: @test_implicitarg_ptr
-// CHECK: call i8 addrspace(2)* @llvm.amdgcn.implicitarg.ptr()
-void test_implicitarg_ptr(__attribute__((address_space(2))) unsigned char ** out)
+// CHECK: call i8 addrspace(4)* @llvm.amdgcn.implicitarg.ptr()
+void te
[PATCH] D38639: [clangd] #include statements support for Open definition
malaperle added inline comments.
Comment at: unittests/clangd/XRefsTests.cpp:53
+class IgnoreDiagnostics : public DiagnosticsConsumer {
+ void onDiagnosticsReady(
ilya-biryukov wrote:
> NIT: remove this class, use `IgnoreDiagnostics` from `Compiler.h` instead.
It looks like IgnoreDiagnostics from Compiler.h implements DiagnosticConsumer
from Compiler.h, and not DiagnosticsConsumer from ClangdServer.h so that
wouldn't work.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D38639
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h
ilya-biryukov added a comment. In https://reviews.llvm.org/D43230#1006104, @ioeric wrote: > But I think it's safe and probably easier to rely on default values of > primitive types like int, bool etc It's not always safe, as primitive types are sometimes left uninitialized (e.g. when constructed on the stack) and reading an uninitialized value is UB. > but do we really want to make this a requirement for future changes or even > in our coding style? I think we should, default values are less surprising than UB. Other people may disagree, though. @sammccall , @hokein , WDYT? Should we always initialize primitive types in our code? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43230 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35894: [clangd] Code hover for Clangd
ilya-biryukov added a comment.
In https://reviews.llvm.org/D35894#1006124, @simark wrote:
> Is there a way to get the macro name from the MacroInfo object? I couldn't
> find any, so the solution I'm considering is to make
> `DeclarationAndMacrosFinder::takeMacroInfos` return an
> `std::vector>`, where the first
> member of the pair is the macro name. It would come from
> `IdentifierInfo->getName()` in `DeclarationAndMacrosFinder::finish`. Does
> that make sense, or is there a simpler way?
I don't think there's a way to get macro name from `MacroInfo`. `pair` sounds good to me, I'd probably even use a named struct here:
`struct MacroDecl { StringRef Name; const MacroInfo &Info; }`
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D35894
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42983: [clang-tidy] Add readability-simd-intrinsics check.
MaskRay updated this revision to Diff 134076.
MaskRay marked an inline comment as done.
MaskRay added a comment.
Remove UseStdExperimental
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42983
Files:
clang-tidy/readability/CMakeLists.txt
clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tidy/readability/SIMDIntrinsicsCheck.cpp
clang-tidy/readability/SIMDIntrinsicsCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/list.rst
docs/clang-tidy/checks/readability-simd-intrinsics.rst
test/clang-tidy/readability-simd-intrinsics-ppc.cpp
test/clang-tidy/readability-simd-intrinsics-x86.cpp
Index: test/clang-tidy/readability-simd-intrinsics-x86.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-simd-intrinsics-x86.cpp
@@ -0,0 +1,26 @@
+// RUN: %check_clang_tidy %s readability-simd-intrinsics %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:{key: readability-simd-intrinsics.UseStdExperimental, value: 1} \
+// RUN: ]}' -- -target x86_64 -std=c++11
+
+typedef long long __m128i __attribute__((vector_size(16)));
+typedef double __m256 __attribute__((vector_size(32)));
+
+__m128i _mm_add_epi32(__m128i, __m128i);
+__m256 _mm256_load_pd(double const *);
+void _mm256_store_pd(double *, __m256);
+
+int _mm_add_fake(int, int);
+
+void X86() {
+ __m128i i0, i1;
+ __m256 d0;
+
+ _mm_add_epi32(i0, i1);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: '_mm_add_epi32' can be replaced by operator+ on std::experimental::simd objects [readability-simd-intrinsics]
+ d0 = _mm256_load_pd(0);
+ _mm256_store_pd(0, d0);
+
+ _mm_add_fake(0, 1);
+// CHECK-MESSAGES-NOT: :[[@LINE-1]]:
+}
Index: test/clang-tidy/readability-simd-intrinsics-ppc.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-simd-intrinsics-ppc.cpp
@@ -0,0 +1,13 @@
+// RUN: %check_clang_tidy %s readability-simd-intrinsics %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:{key: readability-simd-intrinsics.UseStdExperimental, value: 1} \
+// RUN: ]}' -- -target ppc64le -maltivec -std=c++11
+
+vector int vec_add(vector int, vector int);
+
+void PPC() {
+ vector int i0, i1;
+
+ vec_add(i0, i1);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'vec_add' can be replaced by operator+ on std::experimental::simd objects [readability-simd-intrinsics]
+}
Index: docs/clang-tidy/checks/readability-simd-intrinsics.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-simd-intrinsics.rst
@@ -0,0 +1,33 @@
+.. title:: clang-tidy - readability-simd-intrinsics
+
+readability-simd-intrinsics
+===
+
+Finds SIMD intrinsics calls and suggests ``std::experimental::simd`` (`P0214`_) alternatives.
+
+If the option ``UseStdExperimental`` is set to non-zero, for
+
+.. code-block:: c++
+
+ _mm_add_epi32(a, b); // x86
+ vec_add(a, b); // Power
+
+the check suggests an alternative:
+
+.. code-block::
+
+ operator+ on std::experimental::simd objects
+
+Many architectures provide SIMD operations (e.g. x86 SSE/AVX, Power AltiVec/VSX,
+ARM NEON). It is common that SIMD code implementing the same algorithm, is
+written in multiple target-dispatching pieces to optimize for different
+architectures or micro-architectures.
+
+The C++ standard proposal `P0214`_ and its extensions cover many common SIMD
+operations. By migrating from target-dependent intrinsics to `P0214` operations,
+the SIMD code can be simplified and pieces for different targets can be unified.
+
+Refer to `P0214`_ for introduction and motivation for the data-parallel standard
+library.
+
+.. _P0214: http://wg21.link/p0214
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -216,6 +216,7 @@
readability-redundant-smartptr-get
readability-redundant-string-cstr
readability-redundant-string-init
+ readability-simd-intrinsics
readability-simplify-boolean-expr
readability-static-accessed-through-instance
readability-static-definition-in-anonymous-namespace
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -88,6 +88,12 @@
Functions that have trailing returns are disallowed, except for those
using decltype specifiers and lambda with otherwise unutterable
return types.
+
+- New `readability-simd-intrinsics
+ `_ check
+
+ Warns if SIMD intrinsics are used which can be replaced by
+ ``std::experimental::simd`` operations.
- New alias `hicpp-avoid-goto
`_ to
Index: clang-tidy/readability/SIMDIntrinsicsCheck.h
=
[PATCH] D42624: [clang-tidy] Add a utility Matcher to match the next statement within a statement sequence
aaron.ballman added inline comments. Comment at: clang-tidy/utils/Matchers.h:16 +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Analysis/CFG.h" tbourvon wrote: > aaron.ballman wrote: > > This will require linking in the clangAnalysis library as well; are we sure > > we want to take on this dependency here for all matchers? > Do you have a specific solution in mind? We could make the matcher local to > the check it is being used in (see D37014), but I think it could prove useful > for other checks... No solution in mind -- I was mostly wondering if @alexfh was okay picking up this dependency or not, because it will impact all the clang-tidy modules. Comment at: clang-tidy/utils/Matchers.h:58-60 + // We get the first parent, making sure that we're not in a case statement + // not in a compound statement directly inside a switch, because this causes + // the buildCFG call to crash. tbourvon wrote: > aaron.ballman wrote: > > Why does it cause the crash? Should that crash not be fixed instead of > > applying this workaround? > I'm not entirely sure if this is expected behavior or not. In terms of AST, > `switch` statements are a bit special in terms of how they are represented > (each case contains all the subsequent cases as its children IIRC). > There probably is a way to make the CFG work in these cases, but I honestly > don't have the time to look into that and attempt a fix. Couldn't this be > good enough for now, maybe with a FIXME? I'm uncomfortable simply working around known crashes rather than fixing them, even with FIXME comments, because that just means we accrue more technical debt with no plan in place to pay it down in the future. However, I'll let @alexfh make the final call on it. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D42624 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line
russellmcc added a comment. Ping! I believe all feedback has been addressed - further consideration would be much appreciated. https://reviews.llvm.org/D40988 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h
ioeric added a comment. In https://reviews.llvm.org/D43230#1006469, @ilya-biryukov wrote: > In https://reviews.llvm.org/D43230#1006104, @ioeric wrote: > > > But I think it's safe and probably easier to rely on default values of > > primitive types like int, bool etc > > > It's not always safe, as primitive types are sometimes left uninitialized > (e.g. when constructed on the stack) and reading an uninitialized value is UB. Oh, you are absolutely right! I I think l had protos get into my mind... > > >> but do we really want to make this a requirement for future changes or even >> in our coding style? > > I think we should, default values are less surprising than UB. Other people > may disagree, though. > @sammccall , @hokein , WDYT? Should we always initialize primitive types in > our code? I think it would probably depend on whether we could find a sensible default value for a field (e.g. is 0 a better default value than UINT_MAX for line number?) and whether we expect users to always initialize a field (e.g. would it be better to pollute the field instead of providing a default value so that uninitialized values would be caught more easily). Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43230 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Yup, I got bitten recently from some of our plain-c-style structs with no default initializers (in Index). Definitely a fan of this change. Main downside is you can't use aggregate initialization, but the field-by-field initialization is often more readable anyway. And +1 to avoiding explicit `= None` for optionals, etc. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43230 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42983: [clang-tidy] Add readability-simd-intrinsics check.
MaskRay marked 8 inline comments as done.
MaskRay added inline comments.
Comment at: clang-tidy/readability/SIMDIntrinsicsCheck.cpp:46
+
+ static const llvm::StringMap Mapping{
+// [simd.alg]
hokein wrote:
> consider using `llvm::StringSwitch`?
The list is currently a scaffold and more operations will be added. It may be
more efficient to use a lookup table instead of cascading `.Case("add", ...)`?
Comment at: docs/clang-tidy/checks/readability-simd-intrinsics.rst:38
+
+ If set to non-zero, the check will be enabled and it will suggest
``std::experimental`` alternatives. Default is ``0``.
+
hokein wrote:
> We don't use check's option to enable the check in clang-tidy, if users want
> to enable the check, they need to pass the check explicitly to clang-tidy
> (e.g. -checks=""). I'd suggest to remove this option.
Removed.I don't think the option is necessary.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42983
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42995: [ThinLTO] Ignore object files with empty ThinLTO index
vitalybuka planned changes to this revision. vitalybuka added a comment. https://reviews.llvm.org/D42995 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43246: [clangd] Assert path is absolute when assigning to URIForFile.
ioeric added a comment. I think another option to prevent the bug in r325029 from happening would be providing a canonical approach for retrieving (absolute) paths from `FileManager`. We already have code in symbol collector that does this, and we have similar code in XRefs.cpp now. We should probably move them to `clangd/Path.h` and share the code? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43246 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43230: [clangd] Explicitly initialize all primitive fields in Protocol.h
sammccall added inline comments.
Comment at: clangd/Protocol.h:80
struct Position {
+ Position() = default;
+ Position(int line, int character) : line(line), character(character) {}
I'd lean to making callers initialize field-by-field here rather than provide
constructors.
It's not terrible here (you can get it wrong, but only one way), but it's a
pattern that doesn't scale so well I think.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43230
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based
ilya-biryukov added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:194
+if (!Replacements)
+ return replyError(ErrorCode::InternalError,
+llvm::toString(Replacements.takeError()));
ioeric wrote:
> nit: since we are not spelling out the return type here, it might be clearer
> if we do:
> ```
> replyError(...);
> return;
> ```
>
> `return replyError(...)` makes me wonder what the return type is.
This trick was used in the code before (there are usages below), so I figured
it's ok to do this.
If it's confusing I'll change it everywhere.
Comment at: clangd/ClangdServer.cpp:209
+ auto Action = [Contents, Pos, TaggedFS,
+ PCHs](Path File, decltype(Callback) Callback,
+ llvm::Expected IP) {
ioeric wrote:
> nit: I'd probably use a different name than `Callback` for this parameter for
> clarity.
I actually think we should keep it this way. It's a bit tricky to grasp, but it
has two important advantages:
- Makes referencing `Callback` from outer scope impossible
- saves us from coming up with names for that superficial variable, i.e.
should it be called `InnerCallback`, `Callback2`, `C` or something else?
Is it that too confusing or do you feel we can keep it?
Comment at: clangd/ClangdServer.cpp:441
- using RetType = llvm::Expected>>;
- auto Action = [=](llvm::Expected InpAST) -> RetType {
+ auto Action = [=](decltype(Callback) Callback,
+llvm::Expected InpAST) {
ioeric wrote:
> Consider spelling out the captured values, just in case new variables which
> are not expected to be captured are added in the future.
Thanks for spotting this!
Comment at: unittests/clangd/SyncAPI.cpp:69
+ llvm::Expected> Result = Tagged();
+ (void)(bool)Result; // Expected has to be checked.
+ Server.signatureHelp(File, Pos, capture(Result), OverridenContents);
ioeric wrote:
> I'd expect this to be checked by callers. Would `return std::move(Result);`
> work?
The reason why we need it is because `capture(Result)` writes return value of
the callback to the `Result` variable. But we have to first check the
default-constructed value that was there **before** calling
`Server.signatureHelp`
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43227
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based
ilya-biryukov updated this revision to Diff 134080.
ilya-biryukov added a comment.
- Capture only needed vars in lambda, not everything.
- Rebase onto head.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43227
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
unittests/clangd/ClangdTests.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SyncAPI.cpp
unittests/clangd/SyncAPI.h
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -10,6 +10,7 @@
#include "ClangdUnit.h"
#include "Compiler.h"
#include "Matchers.h"
+#include "SyncAPI.h"
#include "TestFS.h"
#include "XRefs.h"
#include "clang/Frontend/CompilerInvocation.h"
@@ -249,7 +250,8 @@
FS.Files[FooCpp] = "";
Server.addDocument(FooCpp, SourceAnnotations.code());
- auto Locations = Server.findDefinitions(FooCpp, SourceAnnotations.point());
+ auto Locations =
+ runFindDefinitions(Server, FooCpp, SourceAnnotations.point());
EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
EXPECT_THAT(Locations->Value,
Index: unittests/clangd/SyncAPI.h
===
--- unittests/clangd/SyncAPI.h
+++ unittests/clangd/SyncAPI.h
@@ -22,6 +22,22 @@
runCodeComplete(ClangdServer &Server, PathRef File, Position Pos,
clangd::CodeCompleteOptions Opts,
llvm::Optional OverridenContents = llvm::None);
+
+llvm::Expected>
+runSignatureHelp(ClangdServer &Server, PathRef File, Position Pos,
+ llvm::Optional OverridenContents = llvm::None);
+
+llvm::Expected>>
+runFindDefinitions(ClangdServer &Server, PathRef File, Position Pos);
+
+llvm::Expected>>
+runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos);
+
+llvm::Expected>
+runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName);
+
+std::string runDumpAST(ClangdServer &Server, PathRef File);
+
} // namespace clangd
} // namespace clang
Index: unittests/clangd/SyncAPI.cpp
===
--- unittests/clangd/SyncAPI.cpp
+++ unittests/clangd/SyncAPI.cpp
@@ -62,5 +62,47 @@
return Result;
}
+llvm::Expected>
+runSignatureHelp(ClangdServer &Server, PathRef File, Position Pos,
+ llvm::Optional OverridenContents) {
+ llvm::Expected> Result = Tagged();
+ (void)(bool)Result; // Expected has to be checked.
+ Server.signatureHelp(File, Pos, capture(Result), OverridenContents);
+ return Result;
+}
+
+llvm::Expected>>
+runFindDefinitions(ClangdServer &Server, PathRef File, Position Pos) {
+ llvm::Expected>> Result =
+ Tagged>();
+ (void)(bool)Result; // Expected has to be checked.
+ Server.findDefinitions(File, Pos, capture(Result));
+ return Result;
+}
+
+llvm::Expected>>
+runFindDocumentHighlights(ClangdServer &Server, PathRef File, Position Pos) {
+ llvm::Expected>> Result =
+ Tagged>();
+ (void)(bool)Result; // Expected has to be checked.
+ Server.findDocumentHighlights(File, Pos, capture(Result));
+ return Result;
+}
+
+llvm::Expected>
+runRename(ClangdServer &Server, PathRef File, Position Pos, StringRef NewName) {
+ llvm::Expected> Result =
+ std::vector();
+ (void)(bool)Result; // Expected has to be checked.
+ Server.rename(File, Pos, NewName, capture(Result));
+ return Result;
+}
+
+std::string runDumpAST(ClangdServer &Server, PathRef File) {
+ std::string Result;
+ Server.dumpAST(File, capture(Result));
+ return Result;
+}
+
} // namespace clangd
} // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -626,7 +626,7 @@
Annotations Test(Text);
Server.addDocument(File, Test.code());
EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for preamble";
- auto R = Server.signatureHelp(File, Test.point());
+ auto R = runSignatureHelp(Server, File, Test.point());
assert(R);
return R.get().Value;
}
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -112,7 +112,7 @@
}
std::string dumpASTWithoutMemoryLocs(ClangdServer &Server, PathRef File) {
- auto DumpWithMemLocs = Server.dumpAST(File);
+ auto DumpWithMemLocs = runDumpAST(Server, File);
return replacePtrsInDump(DumpWithMemLocs);
}
@@ -450,17 +450,17 @@
// Clang can't parse command args in that case, but we shouldn't crash.
Server.addDocument(FooCpp, "int main() {}");
- EXPECT_EQ(Server.dumpAST(FooCpp), "");
- EXPECT_ERROR(Server.findDefinitions(FooCpp, Position{0, 0}));
- EXPECT_ERROR(Server.findDocumentHighlights(FooCpp, Positi
[PATCH] D43181: [CodeGen] Initialize large arrays by copying from a global
rjmccall added inline comments.
Comment at: lib/CodeGen/CGExprAgg.cpp:421
+ // with explicit initializers should be large enough.
+ if (NumInitElements > 8 && elementType->isBuiltinType()) {
+CodeGen::CodeGenModule &CGM = CGF.CGM;
kosarev wrote:
> rjmccall wrote:
> > Is there a good reason to use an element-count heuristic instead of a
> > total-size heuristic here?
> >
> > Why only builtin types? That seems to pointlessly rule out nested arrays,
> > complex types, vectors, C structs, and so on. I think the predicate you
> > probably want here is isTriviallyCopyableType.
> > Is there a good reason to use an element-count heuristic instead of a
> > total-size heuristic here?
>
> Yes, the code below generates per-element initialization only for explicitly
> specified initializers. The rest, if any, is initialized with a filler, so it
> doesn't affect the size of the resulting code much.
That makes sense, but you could still base it on the total size being
initialized with explicit initializers. Such initializers, even when constant,
are likely to require code size basically proportionate to the number of bytes
initialized — sizes of immediate operands and all that.
https://reviews.llvm.org/D43181
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43187: [AST] Refine the condition for element-dependent array fillers
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. LGTM. Repository: rL LLVM https://reviews.llvm.org/D43187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43248: [Attr] Fix printing of parameter indices in attributes
jdenny created this revision.
jdenny added reviewers: aaron.ballman, hfinkel.
Parameter indices in some attributes (argument_with_type_tag,
pointer_with_type_tag, nonnull, ownership_takes, ownership_holds, and
ownership_returns) are specified in source as one-origin including any
this parameter, are stored as zero-origin excluding any this
parameter, and were erroneously printing (-ast-print) as the stored
values. This patch fixes those cases by printing re-incremented
values.
An alternative solution is to store only the original values, but that
requires modifying all users. I tried that for nonnull and found that
it required repeating index adjustment logic in many places while this
patch encapsulates that logic in relatively few places.
Another alternative is to store both sets of values, but that would be
less efficient.
This patch also fixes argument_with_type_tag and pointer_with_type_tag
not to print their fake IsPointer arguments.
https://reviews.llvm.org/D43248
Files:
include/clang/Basic/Attr.td
lib/Sema/SemaDeclAttr.cpp
test/Sema/attr-print.cpp
utils/TableGen/ClangAttrEmitter.cpp
Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -738,6 +738,42 @@
}
};
+ class VariadicParamIndexArgument : public VariadicArgument {
+std::string DisallowImplicitThisParamName;
+
+ protected:
+// Assumed to receive a parameter: raw_ostream OS.
+virtual void writeValueImpl(raw_ostream &OS) const {
+ OS << "OS << (Val + 1";
+ if (!DisallowImplicitThisParamName.empty())
+OS << " + get" << DisallowImplicitThisParamName << "()";
+ OS << ");\n";
+}
+ public:
+VariadicParamIndexArgument(const Record &Arg, StringRef Attr)
+: VariadicArgument(Arg, Attr, "unsigned"),
+ DisallowImplicitThisParamName(
+ Arg.getValueAsString("DisallowImplicitThisParamName")) {}
+ };
+
+ class ParamIndexArgument : public SimpleArgument {
+std::string DisallowImplicitThisParamName;
+
+ protected:
+void writeValue(raw_ostream &OS) const override {
+ OS << "\" << (get" << getUpperName() << "() + 1";
+ if (!DisallowImplicitThisParamName.empty())
+OS << " + get" << DisallowImplicitThisParamName << "()";
+ OS << ") << \"";
+}
+
+ public:
+ParamIndexArgument(const Record &Arg, StringRef Attr)
+: SimpleArgument(Arg, Attr, "unsigned"),
+ DisallowImplicitThisParamName(
+ Arg.getValueAsString("DisallowImplicitThisParamName")) {}
+ };
+
// Unique the enums, but maintain the original declaration ordering.
std::vector
uniqueEnumsInOrder(const std::vector &enums) {
@@ -1237,6 +1273,10 @@
Ptr = llvm::make_unique(Arg, Attr);
else if (ArgName == "VariadicExprArgument")
Ptr = llvm::make_unique(Arg, Attr);
+ else if (ArgName == "VariadicParamIndexArgument")
+Ptr = llvm::make_unique(Arg, Attr);
+ else if (ArgName == "ParamIndexArgument")
+Ptr = llvm::make_unique(Arg, Attr);
else if (ArgName == "VersionArgument")
Ptr = llvm::make_unique(Arg, Attr);
Index: test/Sema/attr-print.cpp
===
--- /dev/null
+++ test/Sema/attr-print.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 %s -ast-print | FileCheck %s
+
+void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 3, 2)));
+// CHECK: void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 3, 2)));
+void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 1, 2)));
+// CHECK: void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 1, 2)));
+
+void nn(int *, int *) __attribute__((nonnull(1, 2)));
+// CHECK: void nn(int *, int *) __attribute__((nonnull(1, 2)));
+
+void ownt(int *, int *) __attribute__((ownership_takes(foo, 1, 2)));
+// CHECK: void ownt(int *, int *) __attribute__((ownership_takes(foo, 1, 2)));
+void ownh(int *, int *) __attribute__((ownership_holds(foo, 1, 2)));
+// CHECK: void ownh(int *, int *) __attribute__((ownership_holds(foo, 1, 2)));
+void ownr(int) __attribute__((ownership_returns(foo, 1)));
+// CHECK: void ownr(int) __attribute__((ownership_returns(foo, 1)));
+
+class C {
+ void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 4, 3)));
+ // CHECK: void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 4, 3)));
+ void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 2, 3)));
+ // CHECK: void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 2, 3)));
+
+ void nn(int *, int *) __attribute__((nonnull(2, 3)));
+ // CHECK: void nn(int *, int *) __attribute__((nonnull(2, 3)));
+
+ void ownt(int *, int *) __attribute__((ownership_takes(foo, 2, 3)));
+ // CHECK: void ownt(int *, int *) __attribute__((ownership_takes(foo, 2, 3)));
+ void ownh(int *, int *) __attribute__((ownership_holds(foo, 2, 3)));
+
[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based
ioeric added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:194
+if (!Replacements)
+ return replyError(ErrorCode::InternalError,
+llvm::toString(Replacements.takeError()));
ilya-biryukov wrote:
> ioeric wrote:
> > nit: since we are not spelling out the return type here, it might be
> > clearer if we do:
> > ```
> > replyError(...);
> > return;
> > ```
> >
> > `return replyError(...)` makes me wonder what the return type is.
> This trick was used in the code before (there are usages below), so I figured
> it's ok to do this.
> If it's confusing I'll change it everywhere.
I guess it's not confusing after figuring out what `replyError` returns. There
is obviously tradeoff between LOC and readability. Either is not too bad. So up
to you :)
Comment at: clangd/ClangdServer.cpp:209
+ auto Action = [Contents, Pos, TaggedFS,
+ PCHs](Path File, decltype(Callback) Callback,
+ llvm::Expected IP) {
ilya-biryukov wrote:
> ioeric wrote:
> > nit: I'd probably use a different name than `Callback` for this parameter
> > for clarity.
> I actually think we should keep it this way. It's a bit tricky to grasp, but
> it has two important advantages:
> - Makes referencing `Callback` from outer scope impossible
> - saves us from coming up with names for that superficial variable, i.e.
> should it be called `InnerCallback`, `Callback2`, `C` or something else?
>
> Is it that too confusing or do you feel we can keep it?
> Makes referencing Callback from outer scope impossible.
For lambdas, I think we should rely on captures for this instead of the
parameter names.
>saves us from coming up with names for that superficial variable, i.e. should
>it be called InnerCallback, Callback2, C or something else?
I thought this is a common practice with llvm coding style :P I would vote for
`C` :)
> Is it that too confusing or do you feel we can keep it?
It is a bit confusing when I first looked at it, but nothing is *too* confusing
as long as the code is correct ;) I just think it would make the code easier to
read.
Comment at: unittests/clangd/SyncAPI.cpp:69
+ llvm::Expected> Result = Tagged();
+ (void)(bool)Result; // Expected has to be checked.
+ Server.signatureHelp(File, Pos, capture(Result), OverridenContents);
ilya-biryukov wrote:
> ioeric wrote:
> > I'd expect this to be checked by callers. Would `return std::move(Result);`
> > work?
> The reason why we need it is because `capture(Result)` writes return value of
> the callback to the `Result` variable. But we have to first check the
> default-constructed value that was there **before** calling
> `Server.signatureHelp`
I think we should probably fix the behavior of `capture` since this changes the
behavior of `llvm::Expected` in user code - the check is there to make sure
users always check the status. But here we always do this for users.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43227
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43227: [clangd] Make functions of ClangdServer callback-based
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
LG, suggest a tweak to capture() though.
I wonder whether we want to introduce `using Callback =
UniqueFunction` for readability at some point...
Comment at: clangd/ClangdServer.cpp:209
+ auto Action = [Contents, Pos, TaggedFS,
+ PCHs](Path File, decltype(Callback) Callback,
+ llvm::Expected IP) {
ioeric wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > nit: I'd probably use a different name than `Callback` for this parameter
> > > for clarity.
> > I actually think we should keep it this way. It's a bit tricky to grasp,
> > but it has two important advantages:
> > - Makes referencing `Callback` from outer scope impossible
> > - saves us from coming up with names for that superficial variable, i.e.
> > should it be called `InnerCallback`, `Callback2`, `C` or something else?
> >
> > Is it that too confusing or do you feel we can keep it?
> > Makes referencing Callback from outer scope impossible.
> For lambdas, I think we should rely on captures for this instead of the
> parameter names.
> >saves us from coming up with names for that superficial variable, i.e.
> >should it be called InnerCallback, Callback2, C or something else?
> I thought this is a common practice with llvm coding style :P I would vote
> for `C` :)
>
> > Is it that too confusing or do you feel we can keep it?
> It is a bit confusing when I first looked at it, but nothing is *too*
> confusing as long as the code is correct ;) I just think it would make the
> code easier to read.
I agree with you both :-)
Conceptually, this *is* a capture - BindWithForward simulates move-capture that
C++11 doesn't have native syntax for. So the same name seems appropriate to me
- you want shadowing for the same reasons you want captures to shadow.
Comment at: unittests/clangd/SyncAPI.cpp:69
+ llvm::Expected> Result = Tagged();
+ (void)(bool)Result; // Expected has to be checked.
+ Server.signatureHelp(File, Pos, capture(Result), OverridenContents);
ioeric wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > I'd expect this to be checked by callers. Would `return
> > > std::move(Result);` work?
> > The reason why we need it is because `capture(Result)` writes return value
> > of the callback to the `Result` variable. But we have to first check the
> > default-constructed value that was there **before** calling
> > `Server.signatureHelp`
> I think we should probably fix the behavior of `capture` since this changes
> the behavior of `llvm::Expected` in user code - the check is there to make
> sure users always check the status. But here we always do this for users.
+1
I suggested `capture(T&)` was the right API because I thought it'd be used
directly by test code (but we wrap it here) and because I thought T would be
default-constructible without problems (but it's not).
I'd suggest changing to `capture(Optional&)` instead, so the caller can just
create an empty optional. That makes the callsites here slightly less obscure.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43227
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43246: [clangd] Assert path is absolute when assigning to URIForFile.
sammccall added inline comments.
Comment at: clangd/Protocol.h:52
struct URIForFile {
+ URIForFile() = default;
I don't like how the API changes here take us further away from the other
structs in this file.
Why does this one enforce invariants, when the others don't?
If we do want to make this more "safe", I'd suggest making it look more like a
URI string (since that's what its protocol role is), like:
```
class LSPURI {
LSPURI(StringRef URI) { assert if bad }
static LSPURI ForFile(StringRef AbsPath) { assert if bad }
operator string() { return URI::createFile(File).toString(); }
StringRef file() { return AbsFile; }
operator bool() { return !File.empty(); }
// move/copy constructor/assignment defaulted
private:
string AbsFile;
}
```
But the approach Eric suggested does seem promising: more consistent with how
we handle invariants in protocol structs.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43246
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r325040 - Update StmtProfile.cpp to handle zero template arguments.
Author: rtrieu
Date: Tue Feb 13 11:53:40 2018
New Revision: 325040
URL: http://llvm.org/viewvc/llvm-project?rev=325040&view=rev
Log:
Update StmtProfile.cpp to handle zero template arguments.
Treat having no templates arguments differently than having zero template
arguments when profiling.
Modified:
cfe/trunk/lib/AST/StmtProfile.cpp
cfe/trunk/test/Modules/odr_hash.cpp
Modified: cfe/trunk/lib/AST/StmtProfile.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/StmtProfile.cpp?rev=325040&r1=325039&r2=325040&view=diff
==
--- cfe/trunk/lib/AST/StmtProfile.cpp (original)
+++ cfe/trunk/lib/AST/StmtProfile.cpp Tue Feb 13 11:53:40 2018
@@ -966,8 +966,11 @@ void StmtProfiler::VisitDeclRefExpr(cons
if (!Canonical)
VisitNestedNameSpecifier(S->getQualifier());
VisitDecl(S->getDecl());
- if (!Canonical)
-VisitTemplateArguments(S->getTemplateArgs(), S->getNumTemplateArgs());
+ if (!Canonical) {
+ID.AddBoolean(S->hasExplicitTemplateArgs());
+if (S->hasExplicitTemplateArgs())
+ VisitTemplateArguments(S->getTemplateArgs(), S->getNumTemplateArgs());
+ }
}
void StmtProfiler::VisitPredefinedExpr(const PredefinedExpr *S) {
Modified: cfe/trunk/test/Modules/odr_hash.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/odr_hash.cpp?rev=325040&r1=325039&r2=325040&view=diff
==
--- cfe/trunk/test/Modules/odr_hash.cpp (original)
+++ cfe/trunk/test/Modules/odr_hash.cpp Tue Feb 13 11:53:40 2018
@@ -2924,6 +2924,21 @@ int I10 = F10();
// expected-note@first.h:* {{but in 'FirstModule' found a different body}}
} // namespace FunctionDecl
+namespace DeclTemplateArguments {
+#if defined(FIRST)
+int foo() { return 1; }
+int bar() { return foo(); }
+#elif defined(SECOND)
+template
+int foo() { return 2; }
+int bar() { return foo<>(); }
+#else
+int num = bar();
+// expected-error@second.h:* {{'DeclTemplateArguments::bar' has different
definitions in different modules; definition in module 'SecondModule' first
difference is function body}}
+// expected-note@first.h:* {{but in 'FirstModule' found a different body}}
+#endif
+}
+
// Keep macros contained to one file.
#ifdef FIRST
#undef FIRST
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning
craig.topper added inline comments.
Comment at: lib/Sema/SemaDeclAttr.cpp:2007
+
+bool Sema::CheckAttrNoArgs(const AttributeList &Attr) {
+ if (!checkAttributeNumArgs(*this, Attr, 0)) {
Wy did this get renamed?
Comment at: lib/Sema/SemaDeclAttr.cpp:2016
-bool Sema::CheckNoCallerSavedRegsAttr(const AttributeList &Attr) {
+bool Sema::CheckAttrTarget(const AttributeList &Attr) {
// Check whether the attribute is valid on the current target.
Why did this get renamed?
Repository:
rL LLVM
https://reviews.llvm.org/D41880
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39571: [clangd] DidChangeConfiguration Notification
simark added a comment.
I think I managed to make some tests by using the `MockCompilationDatabase`.
Basically with some code like:
#ifndef MACRO
static void func () {} // 1
#else
static void func () {} // 2
#endif
and these steps:
1. Server.addDocument(...)
2. Server.findDefinitions (assert that it returns definition 1)
3. CDB.ExtraClangFlags.push_back("-DMACRO=1")
4. Server.reparseOpenedFiles()
5. Server.findDefinitions (assert that it returns definition 2)
Right now that test fails, but it's not clear to me whether it's because the
test is wrong or there's really a bug in there. I'll upload a work-in-progress
version.
Comment at: clangd/ClangdServer.cpp:541
+std::vector>
+ClangdServer::reparseOpenedFiles() {
ilya-biryukov wrote:
> We're not returning futures from `forceReparse` anymore, this function has to
> be updated accordingly.
Indeed, I found this by rebasing.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D39571
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39571: [clangd] DidChangeConfiguration Notification
simark updated this revision to Diff 134097.
simark added a comment.
Add tests, work in progress
Can you take a look at the "TEST(DidChangeConfiguration, DifferentDeclaration)"
test, and tell me if you see anything wrong with it? It seems to me like
changes in command line arguments (adding -DMACRO=1) are not taken into account
when changing configuration.
The other test (also very much work-in-progress) would be to verify that
changing configuration gets us the right diagnostics.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D39571
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/DraftStore.cpp
clangd/DraftStore.h
clangd/GlobalCompilationDatabase.cpp
clangd/GlobalCompilationDatabase.h
clangd/Protocol.cpp
clangd/Protocol.h
clangd/ProtocolHandlers.cpp
clangd/ProtocolHandlers.h
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -257,6 +257,86 @@
SourceAnnotations.range()}));
}
+TEST(DidChangeConfiguration, DifferentDeclaration) {
+ Annotations SourceAnnotations(R"cpp(
+#ifndef MACRO
+$before[[static void bob() {}]]
+#else
+$after[[static void bob() {}]]
+#endif
+
+int main() { b^ob(); }
+)cpp");
+
+ MockCompilationDatabase CDB(/*UseRelPaths=*/true);
+ MockFSProvider FS;
+
+ auto FooCpp = getVirtualTestFilePath("foo.cpp");
+ FS.Files[FooCpp] = "";
+
+ IgnoreDiagnostics DiagConsumer;
+ ClangdServer Server(CDB, DiagConsumer, FS, /*AsyncThreadsCount=*/0,
+ /*StorePreambleInMemory=*/true);
+
+ Server.addDocument(FooCpp, SourceAnnotations.code());
+
+ /* Without MACRO defined. */
+ auto Locations = Server.findDefinitions(FooCpp, SourceAnnotations.point());
+ EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+ EXPECT_THAT(Locations->Value,
+ ElementsAre(Location{URIForFile{FooCpp.str()},
+ SourceAnnotations.range("before")}));
+
+ CDB.ExtraClangFlags.push_back("-DMACRO=1");
+ Server.reparseOpenedFiles();
+
+ /* With MACRO defined. */
+ Locations = Server.findDefinitions(FooCpp, SourceAnnotations.point());
+ EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error";
+ EXPECT_THAT(Locations->Value,
+ ElementsAre(Location{URIForFile{FooCpp.str()},
+ SourceAnnotations.range("after")}));
+}
+
+class MyDiags : public DiagnosticsConsumer {
+ void onDiagnosticsReady(
+ PathRef File, Tagged> Diagnostics) override {
+std::cout << "There are " << Diagnostics.Value.size() << std::endl;
+for (DiagWithFixIts &Diag : Diagnostics.Value) {
+ std::cout << " " << Diag.Diag.message << std::endl;
+}
+ }
+};
+
+TEST(DidChangeConfiguration, Diagnostics) {
+ Annotations SourceAnnotations(R"cpp(
+#ifdef WITH_ERROR
+this is an error
+#endif
+
+int main() { return 0; }
+)cpp");
+
+ MockCompilationDatabase CDB(/*UseRelPaths=*/true);
+ CDB.ExtraClangFlags.push_back("-DWITH_ERROR=1");
+ MockFSProvider FS;
+
+ auto FooCpp = getVirtualTestFilePath("foo.cpp");
+ FS.Files[FooCpp] = "";
+
+ MyDiags DiagConsumer;
+ ClangdServer Server(CDB, DiagConsumer, FS, /*AsyncThreadsCount=*/0,
+ /*StorePreambleInMemory=*/true);
+
+ Server.addDocument(FooCpp, SourceAnnotations.code());
+
+
+ CDB.ExtraClangFlags.clear();
+
+ Server.reparseOpenedFiles();
+
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clangd/ProtocolHandlers.h
===
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -51,6 +51,7 @@
virtual void onCommand(ExecuteCommandParams &Params) = 0;
virtual void onRename(RenameParams &Parames) = 0;
virtual void onDocumentHighlight(TextDocumentPositionParams &Params) = 0;
+ virtual void onChangeConfiguration(DidChangeConfigurationParams &Params) = 0;
};
void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher, JSONOutput &Out,
Index: clangd/ProtocolHandlers.cpp
===
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -71,4 +71,6 @@
Register("workspace/executeCommand", &ProtocolCallbacks::onCommand);
Register("textDocument/documentHighlight",
&ProtocolCallbacks::onDocumentHighlight);
+ Register("workspace/didChangeConfiguration",
+ &ProtocolCallbacks::onChangeConfiguration);
}
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -265,6 +265,20 @@
};
bool fromJSON(const json::Expr &, DidChangeWatchedFilesParams &);
+/// Clangd extension to manage a workspace/didChangeConfiguration notification
+/// since the data received is
[PATCH] D41880: Adding nocf_check attribute for cf-protection fine tuning
aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added inline comments.
Comment at: include/clang/Basic/Attr.td:2089
+def AnyX86NoCfCheck : InheritableAttr, TargetSpecificAttr{
+ let Spellings = [GCC<"nocf_check">];
+ let Documentation = [AnyX86NoCfCheckDocs];
This attribute doesn't appear to be supported by GCC. Did you mean to use the
Clang spelling instead?
Comment at: include/clang/Basic/AttrDocs.td:2876
+ let Content = [{
+Jump Oriented Programming attacks rely on tampering addresses used by
+indirect call / jmp, e.g. redirect control-flow to non-programmer
tampering with addresses
Comment at: include/clang/Basic/AttrDocs.td:2878
+indirect call / jmp, e.g. redirect control-flow to non-programmer
+intended bytes in binary.
+X86 Supports Indirect Branch Tracking (IBT) as part of Control-Flow
bytes in the binary
Comment at: lib/Sema/SemaDeclAttr.cpp:1990
-bool Sema::CheckNoReturnAttr(const AttributeList &Attrs) {
- if (!checkAttributeNumArgs(*this, Attrs, 0)) {
-Attrs.setInvalid();
+static void handleNoCfCheckAttr(Sema &S, Decl *D, const AttributeList &attr) {
+ if (S.CheckAttrTarget(attr) || S.CheckAttrNoArgs(attr))
`attr` doesn't follow the proper naming conventions.
Comment at: lib/Sema/SemaDeclAttr.cpp:1991-1992
+static void handleNoCfCheckAttr(Sema &S, Decl *D, const AttributeList &attr) {
+ if (S.CheckAttrTarget(attr) || S.CheckAttrNoArgs(attr))
+return;
+
Why do you need to manually check the attribute has no args -- doesn't the
common handling in `handleCommonAttributeFeatures()` already cover that?
Comment at: lib/Sema/SemaDeclAttr.cpp:1994-2002
+ if (!isFunctionOrMethod(D)) {
+ValueDecl *VD = dyn_cast(D);
+if (!VD || (!VD->getType()->isFunctionPointerType())) {
+ S.Diag(attr.getLoc(), diag::warn_attribute_wrong_decl_type)
+ << attr.getName() << ExpectedFunctionOrMethod;
+ return;
+}
I think all of this can be replaced by setting the proper subject on the
attribute (`FunctionLike`) in Attr.td.
Comment at: test/Sema/attr-nocf_check.c:7-9
+// Allow function declaration and definition mismatch.
+void __attribute__((nocf_check)) testNoCfCheck(); // no-warning
+void __attribute__((nocf_check)) testNoCfCheck(){}; // no-warning
How is this mismatched?
Repository:
rL LLVM
https://reviews.llvm.org/D41880
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
