[PATCH] D43197: [OpenMP] Add flag for linking runtime bitcode library
Hahnfeld added inline comments.
Comment at: lib/Driver/ToolChains/Cuda.cpp:536-542
+ StringRef CompilerPath = env;
+ while (!CompilerPath.empty()) {
+std::pair Split =
+CompilerPath.split(llvm::sys::EnvPathSeparator);
+LibraryPaths.push_back(Split.first);
+CompilerPath = Split.second;
+ }
gtbercea wrote:
> Hahnfeld wrote:
> > gtbercea wrote:
> > > Hahnfeld wrote:
> > > > `tools::addDirectoryList` uses `StringRef::find`, I'm not sure if
> > > > `StringRef::split` creates real copies of the string...
> > > What is your suggestion?
> > IMO you should use whatever existing code does, in that case
> > `StringRef::find`.
> Is this comment still relevant in the light of the most recent changes?
Probably not (although the code is now completely different from
`tools::addDirectoryList`)
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] D44293: [clangd] Fix irrelevant declaratations in goto definition (on macros).
hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: ioeric, jkorous-apple, klimek.
DeclrationAndMacrosFinder will find some declarations (not macro!) that are
referened inside the macro somehow, isSearchedLocation() is not sufficient, we
don't know whether the searched source location is macro or not.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44293
Files:
clangd/XRefs.cpp
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -213,6 +213,15 @@
#undef macro
)cpp",
+ R"cpp(// Macro
+ class TTT { public: int a; };
+ #define [[FF(S) if (int b = S.a) {}]]
+ void f() {
+ TTT t;
+ F^F(t);
+ }
+ )cpp",
+
R"cpp(// Forward class declaration
class Foo;
[[class Foo {}]];
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -197,19 +197,21 @@
indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
DeclMacrosFinder, IndexOpts);
- std::vector Decls = DeclMacrosFinder->takeDecls();
std::vector MacroInfos = DeclMacrosFinder->takeMacroInfos();
+ if (!MacroInfos.empty()) {
+for (auto Item : MacroInfos) {
+ SourceRange SR(Item.Info->getDefinitionLoc(),
+ Item.Info->getDefinitionEndLoc());
+ auto L = getDeclarationLocation(AST, SR);
+ if (L)
+Result.push_back(*L);
+}
- for (auto Item : Decls) {
-auto L = getDeclarationLocation(AST, Item->getSourceRange());
-if (L)
- Result.push_back(*L);
+return Result;
}
- for (auto Item : MacroInfos) {
-SourceRange SR(Item.Info->getDefinitionLoc(),
- Item.Info->getDefinitionEndLoc());
-auto L = getDeclarationLocation(AST, SR);
+ for (auto Item : DeclMacrosFinder->takeDecls()) {
+auto L = getDeclarationLocation(AST, Item->getSourceRange());
if (L)
Result.push_back(*L);
}
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -213,6 +213,15 @@
#undef macro
)cpp",
+ R"cpp(// Macro
+ class TTT { public: int a; };
+ #define [[FF(S) if (int b = S.a) {}]]
+ void f() {
+ TTT t;
+ F^F(t);
+ }
+ )cpp",
+
R"cpp(// Forward class declaration
class Foo;
[[class Foo {}]];
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -197,19 +197,21 @@
indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
DeclMacrosFinder, IndexOpts);
- std::vector Decls = DeclMacrosFinder->takeDecls();
std::vector MacroInfos = DeclMacrosFinder->takeMacroInfos();
+ if (!MacroInfos.empty()) {
+for (auto Item : MacroInfos) {
+ SourceRange SR(Item.Info->getDefinitionLoc(),
+ Item.Info->getDefinitionEndLoc());
+ auto L = getDeclarationLocation(AST, SR);
+ if (L)
+Result.push_back(*L);
+}
- for (auto Item : Decls) {
-auto L = getDeclarationLocation(AST, Item->getSourceRange());
-if (L)
- Result.push_back(*L);
+return Result;
}
- for (auto Item : MacroInfos) {
-SourceRange SR(Item.Info->getDefinitionLoc(),
- Item.Info->getDefinitionEndLoc());
-auto L = getDeclarationLocation(AST, SR);
+ for (auto Item : DeclMacrosFinder->takeDecls()) {
+auto L = getDeclarationLocation(AST, Item->getSourceRange());
if (L)
Result.push_back(*L);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44272: [clangd] Support incremental document syncing
ilya-biryukov requested changes to this revision.
ilya-biryukov added inline comments.
This revision now requires changes to proceed.
Comment at: clangd/ClangdLSPServer.cpp:101
json::obj{
-{"textDocumentSync", 1},
+{"textDocumentSync", 2},
{"documentFormattingProvider", true},
A comment mentioning that 2 means incremental document sync from LSP would be
useful here.
Comment at: clangd/ClangdLSPServer.cpp:149
void ClangdLSPServer::onDocumentDidChange(DidChangeTextDocumentParams &Params)
{
if (Params.contentChanges.size() != 1)
return replyError(ErrorCode::InvalidParams,
We should handle more than a single change here.
Comment at: clangd/ClangdServer.h:159
/// constructor will receive onDiagnosticsReady callback.
void addDocument(PathRef File, StringRef Contents,
+ WantDiagnostics WantDiags = WantDiagnostics::Auto,
I don't think we should do this on `ClangdServer` level. We will have to copy
the whole file anyway before passing it to clang, so there are no performance
wins here and it complicates the interface.
I suggest we update the document text from diffs on `ClangdLSPServer` level and
continue passing the whole document to `ClangdServer`.
It would mean `DraftMgr` will need to move to `ClangdLSPServer`, but it's fine.
Comment at: clangd/DraftStore.cpp:47
+DocVersion DraftStore::updateDraft(PathRef File, StringRef Contents,
+ llvm::Optional range,
+ std::string *NewContents) {
NIT: LLVM uses `UpperCamelCase` for parameters and local variables
Comment at: clangd/DraftStore.cpp:57
+if (!Entry.Draft.hasValue()) {
+ log(llvm::Twine(
+ "Trying to do incremental update on draft without content: ") +
We should return an error to the client in that case. Changing return type to
`llvm::Expected` and handling an error in the callers should do the
trick.
Comment at: clangd/DraftStore.cpp:66
+
+newContents = Entry.Draft->substr(0, startIndex);
+newContents += Contents;
We need to check that `startIndex` and `endIndex` are inside the bounds of the
string.
Comment at: clangd/DraftStore.h:60
/// \return The new version of the draft for \p File.
- DocVersion updateDraft(PathRef File, StringRef Contents);
+ DocVersion updateDraft(PathRef File, StringRef Contents,
+ llvm::Optional Range, std::string
*NewContents);
Let's model the LSP more closely here:
```
// Protocol.h
struct TextDocumentContentChangeEvent {
llvm::Optional range;
llvm::Optional rangeLength;
std::string text;
};
/// DraftManager
class DraftManager {
/// For initial didOpen.
DocVersion addDraft(PathRef File, std::string Contents);
/// For didChange, handles both incremental and full updates. \p Changes
cannot be empty.
DocVersion updateDraft(PathRef File, ArrayRef
Changes);
};
```
Keeping track of LSP versions here could also be useful to make sure we drop
any updates in between (which would definitely lead to surprising results), but
it's ok if we add a FIXME and do it later.
Comment at: clangd/Protocol.h:289
+ llvm::Optional range;
+
+ /// The new text of the range/document.
Please add `rangeLength` from lsp spec too. On one hand, it seems redundant,
but it can be used to add assertions that the text is the same.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44272
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44217: [clang-tidy] Enable Python 3 support for add_new_check.py
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. Thanks for the fix! https://reviews.llvm.org/D44217 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44294: [clangd] Fix diagnostic errors in the test code, NFC.
hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: ioeric, jkorous-apple, klimek.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44294
Files:
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -584,7 +584,11 @@
auto FooH = testPath("foo.h");
auto FooHUri = URIForFile{FooH};
- const char *HeaderContents = R"cpp([[]]int a;)cpp";
+ const char *HeaderContents = R"cpp([[]]#ifndef TEST_H_
+ #define TEST_H_
+ int a;
+ #endif
+ )cpp";
Annotations HeaderAnnotations(HeaderContents);
FS.Files[FooH] = HeaderAnnotations.code();
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -584,7 +584,11 @@
auto FooH = testPath("foo.h");
auto FooHUri = URIForFile{FooH};
- const char *HeaderContents = R"cpp([[]]int a;)cpp";
+ const char *HeaderContents = R"cpp([[]]#ifndef TEST_H_
+ #define TEST_H_
+ int a;
+ #endif
+ )cpp";
Annotations HeaderAnnotations(HeaderContents);
FS.Files[FooH] = HeaderAnnotations.code();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44272: [clangd] Support incremental document syncing
ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:557 // 64-bit unsigned integer. if (Version < LastReportedDiagsVersion) return; When you'll try remove the `DraftMgr`, this piece of code will be hard to refactor because it relies on `DocVersion`. This is a workaround for a possible race condition that should really be handled by TUScheduler rather than this code, but we haven't got around to fixing it yet. (It's on the list, though, would probably get in next week). A proper workaround for now would be to add `llvm::StringMap InternalVersions` field to `ClangdServer`, increment those versions on each call to `addDocument` and use them here in the same way we currently use DraftMgr's versions. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44272 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44295: Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods
zinovy.nis created this revision.
zinovy.nis added reviewers: klimek, alexfh.
zinovy.nis added a project: clang-tools-extra.
Herald added a subscriber: mgorny.
Warns if one calls grand-..parent's virtual method in child's virtual method
instead of parent's. Can automatically fix such cases by retargeting calls to
parent.
class A {
...
int virtual foo() {...}
}
class B: public A {
int foo() override {...}
}
class C: public B {
...
int foo() override {... A::foo()...} // will be replaced with "...B::foo()..."
}
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44295
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugprone/ParentVirtualCallCheck.cpp
clang-tidy/bugprone/ParentVirtualCallCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/bugprone-parent-virtual-call.cpp
Index: test/clang-tidy/bugprone-parent-virtual-call.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-parent-virtual-call.cpp
@@ -0,0 +1,103 @@
+// RUN: %check_clang_tidy %s bugprone-parent-virtual-call %t
+
+extern int f();
+
+class A {
+public:
+ A() = default;
+ virtual ~A() = default;
+
+ virtual int virt_1() { return f() + 1; }
+ virtual int virt_2() { return f() + 2; }
+
+ int non_virt() { return f() + 3; }
+ static int stat() { return f() + 4; }
+};
+
+class B : public A {
+public:
+ B() = default;
+
+ // Nothing to fix: calls to parent.
+ int virt_1() override { return A::virt_1() + 3; }
+ int virt_2() override { return A::virt_2() + 4; }
+};
+
+class C : public B {
+public:
+ int virt_1() override { return A::virt_1() + B::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'B'?
+ // CHECK-FIXES: int virt_1() override { return B::virt_1() + B::virt_1(); }
+ int virt_2() override { return A::virt_1() + B::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'B'?
+ // CHECK-FIXES: int virt_2() override { return B::virt_1() + B::virt_1(); }
+
+ // Test that non-virtual and static methods are not affected by this cherker.
+ int method_c() { return A::stat() + A::non_virt(); }
+};
+
+// Test that the check affects grand-grand..-parent calls too.
+class D : public C {
+public:
+ int virt_1() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: 'B::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-FIXES: int virt_1() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+ int virt_2() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: 'B::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-FIXES: int virt_2() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+};
+
+// Test classes in anonymous namespaces.
+namespace {
+class BN : public A {
+public:
+ int virt_1() override { return A::virt_1() + 3; }
+ int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace N
+
+class CN : public BN {
+public:
+ int virt_1() override { return A::virt_1() + BN::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean '(anonymous namespace)::BN'?
+ // CHECK-FIXES: int virt_1() override { return BN::virt_1() + BN::virt_1(); }
+ int virt_2() override { return A::virt_1() + BN::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean '(anonymous namespace)::BN'?
+ // CHECK-FIXES: int virt_2() override { return BN::virt_1() + BN::virt_1(); }
+};
+
+// Test multiple inheritance fixes
+class AA {
+public:
+ AA() = default;
+ virtual ~AA() = default;
+
+ virtual int virt_1() { return f() + 1; }
+ virtual int virt_2() { return f() + 2; }
+
+ int non_virt() { return f() + 3; }
+ static int stat() { return f() + 4; }
+};
+
+class BB_1 : virtual public AA {
+public:
+ BB_1() = default;
+
+ // Nothing to fix: calls to parent.
+ int virt_1() override { return AA::virt_1() + 3; }
+ int virt_2() override { return AA::virt_2() + 4; }
+};
+
+class BB_2 : virtual public AA {
+public:
+BB_2() = default;
+};
+
+class CC : public BB_1, public BB_2 {
+public:
+ int virt_1() override { return AA::virt_1() + 3; }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'AA::virt_1' is a grand-parent's method, not parent's. Did you mean 'BB_1' or 'BB_2'? [bugprone-parent-virtual-call]
+ // No fix av
[PATCH] D44294: [clangd] Fix diagnostic errors in the test code, NFC.
ilya-biryukov added inline comments. Comment at: unittests/clangd/XRefsTests.cpp:587 - const char *HeaderContents = R"cpp([[]]int a;)cpp"; + const char *HeaderContents = R"cpp([[]]#ifndef TEST_H_ + #define TEST_H_ I would go with `#pragma once` in the test code, it adds less noise. WDYT? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44294 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43162: [Parser] (C++) Make -Wextra-semi slightly more useful
lebedev.ri added a comment. Ping Repository: rC Clang https://reviews.llvm.org/D43162 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43015: clang-format: Introduce BreakInheritanceList option
Typz added a comment. ping? Repository: rC Clang https://reviews.llvm.org/D43015 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43290: clang-format: tweak formatting of variable initialization blocks
Typz added a comment. ping? Repository: rC Clang https://reviews.llvm.org/D43290 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D42684: clang-format: Allow optimizer to break template declaration.
Typz added a comment. ping? Repository: rC Clang https://reviews.llvm.org/D42684 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r327111 - [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith
Author: hokein
Date: Fri Mar 9 02:47:14 2018
New Revision: 327111
URL: http://llvm.org/viewvc/llvm-project?rev=327111&view=rev
Log:
[clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith
Patch by Niko Weh!
Reviewers: hokein
Subscribers: klimek, cfe-commits, ioeric, ilya-biryukov, ahedberg
Differential Revision: https://reviews.llvm.org/D43847
Added:
clang-tools-extra/trunk/clang-tidy/abseil/
clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.cpp
clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-string-find-startswith.rst
clang-tools-extra/trunk/test/clang-tidy/abseil-string-find-startswith.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/CMakeLists.txt?rev=327111&r1=327110&r2=327111&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt Fri Mar 9 02:47:14 2018
@@ -27,6 +27,7 @@ add_clang_library(clangTidy
)
add_subdirectory(android)
+add_subdirectory(abseil)
add_subdirectory(boost)
add_subdirectory(bugprone)
add_subdirectory(cert)
Added: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=327111&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Fri Mar 9
02:47:14 2018
@@ -0,0 +1,38 @@
+//===--- AbseilTidyModule.cpp - clang-tidy
===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+#include "StringFindStartswithCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+class AbseilModule : public ClangTidyModule {
+public:
+ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+CheckFactories.registerCheck(
+"abseil-string-find-startswith");
+ }
+};
+
+// Register the AbseilModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add X("abseil-module",
+"Add Abseil checks.");
+
+} // namespace abseil
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the AbseilModule.
+volatile int AbseilModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=327111&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Fri Mar 9
02:47:14 2018
@@ -0,0 +1,14 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyAbseilModule
+ AbseilTidyModule.cpp
+ StringFindStartswithCheck.cpp
+
+ LINK_LIBS
+ clangAST
+ clangASTMatchers
+ clangBasic
+ clangLex
+ clangTidy
+ clangTidyUtils
+ )
Added: clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.cpp?rev=327111&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.cpp
(added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.cpp Fri
Mar 9 02:47:14 2018
@@ -0,0 +1,133 @@
+//===--- StringFindStartswithCheck.cc - clang-tidy---*- C++
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--
[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE327111: [clang-tidy] Add check: replace string::find(...)
== 0 with absl::StartsWith (authored by hokein, committed by ).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43847
Files:
clang-tidy/CMakeLists.txt
clang-tidy/abseil/AbseilTidyModule.cpp
clang-tidy/abseil/CMakeLists.txt
clang-tidy/abseil/StringFindStartswithCheck.cpp
clang-tidy/abseil/StringFindStartswithCheck.h
clang-tidy/plugin/CMakeLists.txt
clang-tidy/tool/CMakeLists.txt
clang-tidy/tool/ClangTidyMain.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/abseil-string-find-startswith.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/abseil-string-find-startswith.cpp
Index: clang-tidy/CMakeLists.txt
===
--- clang-tidy/CMakeLists.txt
+++ clang-tidy/CMakeLists.txt
@@ -27,6 +27,7 @@
)
add_subdirectory(android)
+add_subdirectory(abseil)
add_subdirectory(boost)
add_subdirectory(bugprone)
add_subdirectory(cert)
Index: clang-tidy/tool/CMakeLists.txt
===
--- clang-tidy/tool/CMakeLists.txt
+++ clang-tidy/tool/CMakeLists.txt
@@ -18,6 +18,7 @@
clangBasic
clangTidy
clangTidyAndroidModule
+ clangTidyAbseilModule
clangTidyBoostModule
clangTidyBugproneModule
clangTidyCERTModule
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -499,6 +499,11 @@
static int LLVM_ATTRIBUTE_UNUSED CERTModuleAnchorDestination =
CERTModuleAnchorSource;
+// This anchor is used to force the linker to link the AbseilModule.
+extern volatile int AbseilModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED AbseilModuleAnchorDestination =
+AbseilModuleAnchorSource;
+
// This anchor is used to force the linker to link the BoostModule.
extern volatile int BoostModuleAnchorSource;
static int LLVM_ATTRIBUTE_UNUSED BoostModuleAnchorDestination =
Index: clang-tidy/abseil/CMakeLists.txt
===
--- clang-tidy/abseil/CMakeLists.txt
+++ clang-tidy/abseil/CMakeLists.txt
@@ -0,0 +1,14 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyAbseilModule
+ AbseilTidyModule.cpp
+ StringFindStartswithCheck.cpp
+
+ LINK_LIBS
+ clangAST
+ clangASTMatchers
+ clangBasic
+ clangLex
+ clangTidy
+ clangTidyUtils
+ )
Index: clang-tidy/abseil/StringFindStartswithCheck.h
===
--- clang-tidy/abseil/StringFindStartswithCheck.h
+++ clang-tidy/abseil/StringFindStartswithCheck.h
@@ -0,0 +1,48 @@
+//===--- StringFindStartswithCheck.h - clang-tidy*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRING_FIND_STARTSWITH_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRING_FIND_STARTSWITH_H_
+
+#include "../ClangTidy.h"
+#include "../utils/IncludeInserter.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+#include
+#include
+#include
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+// Find string.find(...) == 0 comparisons and suggest replacing with StartsWith.
+// FIXME(niko): Add similar check for EndsWith
+// FIXME(niko): Add equivalent modernize checks for C++20's std::starts_With
+class StringFindStartswithCheck : public ClangTidyCheck {
+public:
+ using ClangTidyCheck::ClangTidyCheck;
+ StringFindStartswithCheck(StringRef Name, ClangTidyContext *Context);
+ void registerPPCallbacks(CompilerInstance &Compiler) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+ std::unique_ptr IncludeInserter;
+ const std::vector StringLikeClasses;
+ const utils::IncludeSorter::IncludeStyle IncludeStyle;
+ const std::string AbseilStringsMatchHeader;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRING_FIND_STARTSWITH_H_
Index: clang-tidy/abseil/StringFindStartswithCheck.cpp
===
--- clang-tidy/abseil/StringFindStartswithCheck.cpp
+++ clang-tidy/abseil/StringFindStartswithCheck.cpp
@@ -0,0 +1,133 @@
+//===--- StringFindStartswithCheck.cc - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open
[PATCH] D44294: [clangd] Fix diagnostic errors in the test code, NFC.
hokein updated this revision to Diff 137712.
hokein marked an inline comment as done.
hokein added a comment.
Use #program once
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44294
Files:
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -584,7 +584,9 @@
auto FooH = testPath("foo.h");
auto FooHUri = URIForFile{FooH};
- const char *HeaderContents = R"cpp([[]]int a;)cpp";
+ const char *HeaderContents = R"cpp([[]]#pragma once
+ int a;
+ )cpp";
Annotations HeaderAnnotations(HeaderContents);
FS.Files[FooH] = HeaderAnnotations.code();
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -584,7 +584,9 @@
auto FooH = testPath("foo.h");
auto FooHUri = URIForFile{FooH};
- const char *HeaderContents = R"cpp([[]]int a;)cpp";
+ const char *HeaderContents = R"cpp([[]]#pragma once
+ int a;
+ )cpp";
Annotations HeaderAnnotations(HeaderContents);
FS.Files[FooH] = HeaderAnnotations.code();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44294: [clangd] Fix diagnostic errors in the test code, NFC.
hokein added inline comments. Comment at: unittests/clangd/XRefsTests.cpp:587 - const char *HeaderContents = R"cpp([[]]int a;)cpp"; + const char *HeaderContents = R"cpp([[]]#ifndef TEST_H_ + #define TEST_H_ ilya-biryukov wrote: > I would go with `#pragma once` in the test code, it adds less noise. WDYT? I thought `#pragma once` is non-standard, but the diagnostic message also suggest this fix. Changed to it. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44294 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41102: Setup clang-doc frontend framework
lebedev.ri added a comment.
Might have been better to not start landing until the all differentials are
understood/accepted, but i understand that it is not really up to me to decide.
Let's hope nothing in the next differentials will require changes to this
initial code :)
Comment at: clang-doc/BitcodeWriter.h:241
+/// \param I The info to emit to bitcode.
+template void ClangDocBitcodeWriter::emitBlock(const T &I) {
+ StreamSubBlockGuard Block(Stream, MapFromInfoToBlockId::ID);
sammccall wrote:
> OK, I don't get this at all.
>
> We have to declare emitBlockContent(NamespaceInfo) *and* the specialization
> of MapFromInfoToBlockId, and deal with the public interface
> emitBlock being a template function where you can't tell what's legal to
> pass, instead of writing:
>
> ```void emitBlock(const NamespaceInfo &I) {
> SubStreamBlockGuard Block(Stream, BI_NAMESPACE_BLOCK_ID); // <-- this one
> line
> ...
> }```
>
> This really seems like templates for the sake of templates :(
If you want to add a new block, in one case you just need to add one
```
template <> struct MapFromInfoToBlockId {
static const BlockId ID = BI_???_BLOCK_ID;
};
```
In the other case you need to add whole
```
void ClangDocBitcodeWriter::emitBlock(const ???Info &I) {
StreamSubBlockGuard Block(Stream, BI_???_BLOCK_ID);
emitBlockContent(I);
}
```
(and it was even longer initially)
It seems just templating one static variable is shorter than duplicating
`emitBlock()` each time, no?
Do compare the current diff with the original diff state.
I *think* these templates helped move much of the duplication to simplify the
code overall.
Repository:
rL LLVM
https://reviews.llvm.org/D41102
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44247: [clangd] Use identifier range as the definition range.
hokein updated this revision to Diff 137720.
hokein marked 7 inline comments as done.
hokein added a comment.
Herald added a subscriber: mgorny.
Address review comments.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44247
Files:
clangd/AST.cpp
clangd/AST.h
clangd/CMakeLists.txt
clangd/XRefs.cpp
clangd/index/SymbolCollector.cpp
unittests/clangd/ClangdTests.cpp
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -119,74 +119,74 @@
const char *Tests[] = {
R"cpp(// Local variable
int main() {
- [[int bonjour]];
+ int [[bonjour]];
^bonjour = 2;
int test1 = bonjour;
}
)cpp",
R"cpp(// Struct
namespace ns1 {
-[[struct MyClass {}]];
+struct [[MyClass]] {};
} // namespace ns1
int main() {
ns1::My^Class* Params;
}
)cpp",
R"cpp(// Function definition via pointer
-[[int foo(int) {}]]
+int [[foo]](int) {}
int main() {
auto *X = &^foo;
}
)cpp",
R"cpp(// Function declaration via call
-[[int foo(int)]];
+int [[foo]](int);
int main() {
return ^foo(42);
}
)cpp",
R"cpp(// Field
-struct Foo { [[int x]]; };
+struct Foo { int [[x]]; };
int main() {
Foo bar;
bar.^x;
}
)cpp",
R"cpp(// Field, member initializer
struct Foo {
- [[int x]];
+ int [[x]];
Foo() : ^x(0) {}
};
)cpp",
R"cpp(// Field, GNU old-style field designator
-struct Foo { [[int x]]; };
+struct Foo { int [[x]]; };
int main() {
Foo bar = { ^x : 1 };
}
)cpp",
R"cpp(// Field, field designator
-struct Foo { [[int x]]; };
+struct Foo { int [[x]]; };
int main() {
Foo bar = { .^x = 2 };
}
)cpp",
R"cpp(// Method call
-struct Foo { [[int x()]]; };
+struct Foo { int [[x]](); };
int main() {
Foo bar;
bar.^x();
}
)cpp",
R"cpp(// Typedef
-[[typedef int Foo]];
+typedef int [[Foo]];
int main() {
^Foo bar;
}
@@ -199,9 +199,9 @@
)cpp", */
R"cpp(// Namespace
-[[namespace ns {
+namespace [[ns]] {
struct Foo { static void bar(); }
-}]] // namespace ns
+} // namespace ns
int main() { ^ns::Foo::bar(); }
)cpp",
@@ -215,14 +215,26 @@
R"cpp(// Forward class declaration
class Foo;
-[[class Foo {}]];
+class [[Foo]] {};
F^oo* foo();
)cpp",
R"cpp(// Function declaration
void foo();
void g() { f^oo(); }
-[[void foo() {}]]
+void [[foo]]() {}
+ )cpp",
+
+ R"cpp(
+#define FF(name) class name##_Test {};
+[[FF]](my);
+void f() { my^_Test a; }
+ )cpp",
+
+ R"cpp(
+ #define FF() class [[Test]] {};
+ FF();
+ void f() { T^est a; }
)cpp",
};
for (const char *Test : Tests) {
@@ -236,7 +248,7 @@
TEST(GoToDefinition, RelPathsInCompileCommand) {
Annotations SourceAnnotations(R"cpp(
-[[int foo]];
+int [[foo]];
int baz = f^oo;
)cpp");
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -435,9 +435,9 @@
TEST_F(ClangdVFSTest, ReparseOpenedFiles) {
Annotations FooSource(R"cpp(
#ifdef MACRO
-$one[[static void bob() {}]]
+static void $one[[bob]]() {}
#else
-$two[[static void bob() {}]]
+static void $two[[bob]]() {}
#endif
int main () { bo^b (); return 0; }
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -8,6 +8,7 @@
//===--===//
#include "SymbolCollector.h"
+#include "../AST.h"
#include "../CodeCompletionStrings.h"
#include "../Logger.h"
#include "../URI.h"
@@ -178,30 +179,16 @@
llvm::Optional getSymbolLocation(
const NamedDecl &D, SourceManager &SM, const SymbolCollector::Options &Opts,
const clang::LangOptions &LangOpts, std::string &FileURIStorage) {
- 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
[PATCH] D44247: [clangd] Use identifier range as the definition range.
hokein added a comment.
Thanks for the comments!
Comment at: clangd/SourceCode.cpp:55
+ const auto& SM = D->getASTContext().getSourceManager();
+ SourceLocation SpellingLoc = SM.getSpellingLoc(D->getLocation());
+ if (D->getLocation().isMacroID()) {
sammccall wrote:
> As discussed offline, this heuristic is... limited.
>
> ```#define X Y
> class X{}```
>
> and now we say the definition of the class Y is on the first line.
>
> I think we should really be basing the heuristics on the whole definition
> range, even if we're then going to return just the position of the name.
>
> This patch just moves logic around so we don't need to do it now, but it
> could use a FIXME
Thanks for pointing it out! Added a FIXME.
Comment at: clangd/SourceCode.h:34
+/// Get the source location of the given D.
+///
sammccall wrote:
> This is a useful function, but it doesn't belong in SourceCode with its
> current scope. See the file header :)
>
> We could add an "AST.h" or similar file, or expand the scope of SourceCode,
> but we haven't needed to yet because clang/AST normally has what we need.
>
> Can this be a method on NamedDecl or a function alongside?
> But since (later comment) the logic seems wrong, I'm not sure it makes sense
> to move it at the moment. Maybe create a separate header?
Created a separate file.
Comment at: clangd/SourceCode.h:39
+/// * use spelling location, otherwise.
+SourceLocation getDeclSourceLoc(const clang::Decl* D);
+
sammccall wrote:
> sammccall wrote:
> > This name seems opaque to me.
> >
> > findName?
> would it be more useful to return the range, and just pick out the start if
> you're not interested in both?
Since this function only returns name location, a source location is enough IMO.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44247
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r327115 - [clang-tidy] fix header guard
Author: sammccall Date: Fri Mar 9 03:47:37 2018 New Revision: 327115 URL: http://llvm.org/viewvc/llvm-project?rev=327115&view=rev Log: [clang-tidy] fix header guard Modified: clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.h Modified: clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.h?rev=327115&r1=327114&r2=327115&view=diff == --- clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.h (original) +++ clang-tools-extra/trunk/clang-tidy/abseil/StringFindStartswithCheck.h Fri Mar 9 03:47:37 2018 @@ -7,8 +7,8 @@ // //===--===// -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRING_FIND_STARTSWITH_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRING_FIND_STARTSWITH_H_ +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRINGFINDSTARTSWITHCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRINGFINDSTARTSWITHCHECK_H #include "../ClangTidy.h" #include "../utils/IncludeInserter.h" @@ -45,4 +45,4 @@ private: } // namespace tidy } // namespace clang -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRING_FIND_STARTSWITH_H_ +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_STRINGFINDSTARTSWITHCHECK_H ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Expose clang_getOverriddenCursors to python clang module
This patch exposes an interface to libclang's clang_getOverriddenCursors
and clang_disposeOverriddenCursors functions to the python api.
The patch is adapted from this stack overflow response but cleaned up to
fit within the existing cindex.py codebase
https://stackoverflow.com/questions/35962473/libclang-python-binding-function-getoverridden
Let me know if there's anything I need to address. I can add unit tests if
desired but that would double the length of this patch.
Patch attached
diff --git a/bindings/python/clang/cindex.py b/bindings/python/clang/cindex.py
index b53661a..45d7708 100644
--- a/bindings/python/clang/cindex.py
+++ b/bindings/python/clang/cindex.py
@@ -1818,6 +1818,25 @@ class Cursor(Structure):
children)
return iter(children)
+def get_overridden_cursors(self):
+"""
+If this cursor is an override method, return an iterator for
+accessing the cursors that this overrides
+"""
+cursors = POINTER(Cursor)()
+num = c_uint()
+conf.lib.clang_getOverriddenCursors(self, byref(cursors), byref(num))
+
+updcursors = []
+for i in xrange(int(num.value)):
+c = cursors[i]
+updcursor = Cursor.from_location(self._tu, c.location)
+updcursors.append(updcursor)
+
+conf.lib.clang_disposeOverriddenCursors(cursors)
+
+return updcursors
+
def walk_preorder(self):
"""Depth-first preorder walk over the cursor and its descendants.
@@ -3926,6 +3945,14 @@ functionList = [
[Cursor, callbacks['cursor_visit'], py_object],
c_uint),
+ ("clang_getOverriddenCursors",
+ [Cursor, POINTER(POINTER(Cursor)), POINTER(c_uint)],
+ None),
+
+ ("clang_disposeOverriddenCursors",
+ [POINTER(Cursor)],
+ None),
+
("clang_Cursor_getNumArguments",
[Cursor],
c_int),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44298: [clangd] Don't index template specializations.
sammccall created this revision.
sammccall added a reviewer: ioeric.
Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek.
These have different USRs than the underlying entity, but are not typically
interesting in their own right and can be numerous (e.g. generated traits).
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44298
Files:
clangd/index/SymbolCollector.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -162,6 +162,10 @@
static const int KInt = 2;
const char* kStr = "123";
+// Template is indexed, specialization is not.
+template struct Tmpl;
+template <> struct Tmpl {};
+
namespace {
void ff() {} // ignore
}
@@ -190,12 +194,13 @@
} // namespace foo
)";
runSymbolCollector(Header, /*Main=*/"");
- EXPECT_THAT(Symbols,
- UnorderedElementsAreArray(
- {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
- QName("kStr"), QName("foo"), QName("foo::bar"),
- QName("foo::int32"), QName("foo::int32_t"),
QName("foo::v1"),
- QName("foo::bar::v2"), QName("foo::baz")}));
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAreArray(
+ {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
QName("Tmpl"),
+ QName("kStr"), QName("foo"), QName("foo::bar"), QName("foo::int32"),
+ QName("foo::int32_t"), QName("foo::v1"), QName("foo::bar::v2"),
+ QName("foo::baz")}));
}
TEST_F(SymbolCollectorTest, Locations) {
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -109,6 +109,14 @@
if (ND->isInAnonymousNamespace())
return true;
+ // Don't index template specializations.
+ if (!match(decl(anyOf(functionDecl(isExplicitTemplateSpecialization()),
+cxxRecordDecl(isExplicitTemplateSpecialization()),
+varDecl(isExplicitTemplateSpecialization(,
+ *ND, *ASTCtx)
+ .empty())
+return true;
+
// We only want:
// * symbols in namespaces or translation unit scopes (e.g. no class
// members)
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -162,6 +162,10 @@
static const int KInt = 2;
const char* kStr = "123";
+// Template is indexed, specialization is not.
+template struct Tmpl;
+template <> struct Tmpl {};
+
namespace {
void ff() {} // ignore
}
@@ -190,12 +194,13 @@
} // namespace foo
)";
runSymbolCollector(Header, /*Main=*/"");
- EXPECT_THAT(Symbols,
- UnorderedElementsAreArray(
- {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
- QName("kStr"), QName("foo"), QName("foo::bar"),
- QName("foo::int32"), QName("foo::int32_t"), QName("foo::v1"),
- QName("foo::bar::v2"), QName("foo::baz")}));
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAreArray(
+ {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"), QName("Tmpl"),
+ QName("kStr"), QName("foo"), QName("foo::bar"), QName("foo::int32"),
+ QName("foo::int32_t"), QName("foo::v1"), QName("foo::bar::v2"),
+ QName("foo::baz")}));
}
TEST_F(SymbolCollectorTest, Locations) {
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -109,6 +109,14 @@
if (ND->isInAnonymousNamespace())
return true;
+ // Don't index template specializations.
+ if (!match(decl(anyOf(functionDecl(isExplicitTemplateSpecialization()),
+cxxRecordDecl(isExplicitTemplateSpecialization()),
+varDecl(isExplicitTemplateSpecialization(,
+ *ND, *ASTCtx)
+ .empty())
+return true;
+
// We only want:
// * symbols in namespaces or translation unit scopes (e.g. no class
// members)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44298: [clangd] Don't index template specializations.
sammccall updated this revision to Diff 137729.
sammccall added a comment.
merge matchers
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44298
Files:
clangd/index/SymbolCollector.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -162,6 +162,10 @@
static const int KInt = 2;
const char* kStr = "123";
+// Template is indexed, specialization is not.
+template struct Tmpl;
+template <> struct Tmpl {};
+
namespace {
void ff() {} // ignore
}
@@ -190,12 +194,13 @@
} // namespace foo
)";
runSymbolCollector(Header, /*Main=*/"");
- EXPECT_THAT(Symbols,
- UnorderedElementsAreArray(
- {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
- QName("kStr"), QName("foo"), QName("foo::bar"),
- QName("foo::int32"), QName("foo::int32_t"),
QName("foo::v1"),
- QName("foo::bar::v2"), QName("foo::baz")}));
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAreArray(
+ {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
QName("Tmpl"),
+ QName("kStr"), QName("foo"), QName("foo::bar"), QName("foo::int32"),
+ QName("foo::int32_t"), QName("foo::v1"), QName("foo::bar::v2"),
+ QName("foo::baz")}));
}
TEST_F(SymbolCollectorTest, Locations) {
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -115,10 +115,16 @@
// * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")
auto InTopLevelScope = hasDeclContext(
anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl()));
+ // Don't index template specializations.
+ auto IsSpecialization =
+ anyOf(functionDecl(isExplicitTemplateSpecialization()),
+cxxRecordDecl(isExplicitTemplateSpecialization()),
+varDecl(isExplicitTemplateSpecialization()));
if (match(decl(allOf(unless(isExpansionInMainFile()),
anyOf(InTopLevelScope,
hasDeclContext(enumDecl(InTopLevelScope,
- unless(isScoped())),
+ unless(isScoped(),
+ unless(IsSpecialization))),
*ND, *ASTCtx)
.empty())
return true;
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -162,6 +162,10 @@
static const int KInt = 2;
const char* kStr = "123";
+// Template is indexed, specialization is not.
+template struct Tmpl;
+template <> struct Tmpl {};
+
namespace {
void ff() {} // ignore
}
@@ -190,12 +194,13 @@
} // namespace foo
)";
runSymbolCollector(Header, /*Main=*/"");
- EXPECT_THAT(Symbols,
- UnorderedElementsAreArray(
- {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
- QName("kStr"), QName("foo"), QName("foo::bar"),
- QName("foo::int32"), QName("foo::int32_t"), QName("foo::v1"),
- QName("foo::bar::v2"), QName("foo::baz")}));
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAreArray(
+ {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"), QName("Tmpl"),
+ QName("kStr"), QName("foo"), QName("foo::bar"), QName("foo::int32"),
+ QName("foo::int32_t"), QName("foo::v1"), QName("foo::bar::v2"),
+ QName("foo::baz")}));
}
TEST_F(SymbolCollectorTest, Locations) {
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -115,10 +115,16 @@
// * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")
auto InTopLevelScope = hasDeclContext(
anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl()));
+ // Don't index template specializations.
+ auto IsSpecialization =
+ anyOf(functionDecl(isExplicitTemplateSpecialization()),
+cxxRecordDecl(isExplicitTemplateSpecialization()),
+varDecl(isExplicitTemplateSpecialization()));
if (match(decl(allOf(unless(isExpansionInMainFile()),
anyOf(InTopLevelScope,
hasDeclContext(enumDecl(InTopLevelScope,
- unless(isScoped())),
+ unless(isScoped(),
+ unless(IsSpecialization))),
[PATCH] D44298: [clangd] Don't index template specializations.
sammccall updated this revision to Diff 137731.
sammccall added a comment.
verify that we don't emit extra symbols in other template-related cases
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44298
Files:
clangd/index/SymbolCollector.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -162,6 +162,12 @@
static const int KInt = 2;
const char* kStr = "123";
+// Template is indexed, specialization and redeclaration is not.
+template struct Tmpl {T x = 0};
+template <> struct Tmpl {};
+extern template struct Tmpl;
+template struct Tmpl;
+
namespace {
void ff() {} // ignore
}
@@ -190,12 +196,13 @@
} // namespace foo
)";
runSymbolCollector(Header, /*Main=*/"");
- EXPECT_THAT(Symbols,
- UnorderedElementsAreArray(
- {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
- QName("kStr"), QName("foo"), QName("foo::bar"),
- QName("foo::int32"), QName("foo::int32_t"),
QName("foo::v1"),
- QName("foo::bar::v2"), QName("foo::baz")}));
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAreArray(
+ {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
QName("Tmpl"),
+ QName("kStr"), QName("foo"), QName("foo::bar"), QName("foo::int32"),
+ QName("foo::int32_t"), QName("foo::v1"), QName("foo::bar::v2"),
+ QName("foo::baz")}));
}
TEST_F(SymbolCollectorTest, Locations) {
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -115,10 +115,16 @@
// * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")
auto InTopLevelScope = hasDeclContext(
anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl()));
+ // Don't index template specializations.
+ auto IsSpecialization =
+ anyOf(functionDecl(isExplicitTemplateSpecialization()),
+cxxRecordDecl(isExplicitTemplateSpecialization()),
+varDecl(isExplicitTemplateSpecialization()));
if (match(decl(allOf(unless(isExpansionInMainFile()),
anyOf(InTopLevelScope,
hasDeclContext(enumDecl(InTopLevelScope,
- unless(isScoped())),
+ unless(isScoped(),
+ unless(IsSpecialization))),
*ND, *ASTCtx)
.empty())
return true;
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -162,6 +162,12 @@
static const int KInt = 2;
const char* kStr = "123";
+// Template is indexed, specialization and redeclaration is not.
+template struct Tmpl {T x = 0};
+template <> struct Tmpl {};
+extern template struct Tmpl;
+template struct Tmpl;
+
namespace {
void ff() {} // ignore
}
@@ -190,12 +196,13 @@
} // namespace foo
)";
runSymbolCollector(Header, /*Main=*/"");
- EXPECT_THAT(Symbols,
- UnorderedElementsAreArray(
- {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
- QName("kStr"), QName("foo"), QName("foo::bar"),
- QName("foo::int32"), QName("foo::int32_t"), QName("foo::v1"),
- QName("foo::bar::v2"), QName("foo::baz")}));
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAreArray(
+ {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"), QName("Tmpl"),
+ QName("kStr"), QName("foo"), QName("foo::bar"), QName("foo::int32"),
+ QName("foo::int32_t"), QName("foo::v1"), QName("foo::bar::v2"),
+ QName("foo::baz")}));
}
TEST_F(SymbolCollectorTest, Locations) {
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -115,10 +115,16 @@
// * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")
auto InTopLevelScope = hasDeclContext(
anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl()));
+ // Don't index template specializations.
+ auto IsSpecialization =
+ anyOf(functionDecl(isExplicitTemplateSpecialization()),
+cxxRecordDecl(isExplicitTemplateSpecialization()),
+varDecl(isExplicitTemplateSpecialization()));
if (match(decl(allOf(unless(isExpansionInMainFile()),
anyOf(InTopLevelScope,
hasDeclContext(
[PATCH] D44298: [clangd] Don't index template specializations.
ioeric accepted this revision.
ioeric added a comment.
This revision is now accepted and ready to land.
lgtm
Comment at: unittests/clangd/SymbolCollectorTests.cpp:200
+ UnorderedElementsAreArray(
+ {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
QName("Tmpl"),
+ QName("kStr"), QName("foo"), QName("foo::bar"), QName("foo::int32"),
nit: I'd probably also check the range, just in case...
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r327127 - [clangd] Don't index template specializations.
Author: sammccall
Date: Fri Mar 9 05:25:29 2018
New Revision: 327127
URL: http://llvm.org/viewvc/llvm-project?rev=327127&view=rev
Log:
[clangd] Don't index template specializations.
Summary:
These have different USRs than the underlying entity, but are not typically
interesting in their own right and can be numerous (e.g. generated traits).
Reviewers: ioeric
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D44298
Modified:
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
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=327127&r1=327126&r2=327127&view=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Fri Mar 9
05:25:29 2018
@@ -115,10 +115,16 @@ bool shouldFilterDecl(const NamedDecl *N
// * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")
auto InTopLevelScope = hasDeclContext(
anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl()));
+ // Don't index template specializations.
+ auto IsSpecialization =
+ anyOf(functionDecl(isExplicitTemplateSpecialization()),
+cxxRecordDecl(isExplicitTemplateSpecialization()),
+varDecl(isExplicitTemplateSpecialization()));
if (match(decl(allOf(unless(isExpansionInMainFile()),
anyOf(InTopLevelScope,
hasDeclContext(enumDecl(InTopLevelScope,
- unless(isScoped())),
+ unless(isScoped(),
+ unless(IsSpecialization))),
*ND, *ASTCtx)
.empty())
return true;
Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=327127&r1=327126&r2=327127&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Fri Mar
9 05:25:29 2018
@@ -198,6 +198,19 @@ TEST_F(SymbolCollectorTest, CollectSymbo
QName("foo::bar::v2"), QName("foo::baz")}));
}
+TEST_F(SymbolCollectorTest, Template) {
+ Annotations Header(R"(
+// Template is indexed, specialization and instantiation is not.
+template struct [[Tmpl]] {T x = 0};
+template <> struct Tmpl {};
+extern template struct Tmpl;
+template struct Tmpl;
+ )");
+ runSymbolCollector(Header.code(), /*Main=*/"");
+ EXPECT_THAT(Symbols, UnorderedElementsAreArray({AllOf(
+ QName("Tmpl"), DeclRange(Header.offsetRange()))}));
+}
+
TEST_F(SymbolCollectorTest, Locations) {
Annotations Header(R"cpp(
// Declared in header, defined in main.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44298: [clangd] Don't index template specializations.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL327127: [clangd] Don't index template specializations.
(authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D44298?vs=137731&id=137733#toc
Repository:
rL LLVM
https://reviews.llvm.org/D44298
Files:
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
@@ -198,6 +198,19 @@
QName("foo::bar::v2"), QName("foo::baz")}));
}
+TEST_F(SymbolCollectorTest, Template) {
+ Annotations Header(R"(
+// Template is indexed, specialization and instantiation is not.
+template struct [[Tmpl]] {T x = 0};
+template <> struct Tmpl {};
+extern template struct Tmpl;
+template struct Tmpl;
+ )");
+ runSymbolCollector(Header.code(), /*Main=*/"");
+ EXPECT_THAT(Symbols, UnorderedElementsAreArray({AllOf(
+ QName("Tmpl"), DeclRange(Header.offsetRange()))}));
+}
+
TEST_F(SymbolCollectorTest, Locations) {
Annotations Header(R"cpp(
// Declared in header, defined in main.
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
@@ -115,10 +115,16 @@
// * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")
auto InTopLevelScope = hasDeclContext(
anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl()));
+ // Don't index template specializations.
+ auto IsSpecialization =
+ anyOf(functionDecl(isExplicitTemplateSpecialization()),
+cxxRecordDecl(isExplicitTemplateSpecialization()),
+varDecl(isExplicitTemplateSpecialization()));
if (match(decl(allOf(unless(isExpansionInMainFile()),
anyOf(InTopLevelScope,
hasDeclContext(enumDecl(InTopLevelScope,
- unless(isScoped())),
+ unless(isScoped(),
+ unless(IsSpecialization))),
*ND, *ASTCtx)
.empty())
return true;
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
@@ -198,6 +198,19 @@
QName("foo::bar::v2"), QName("foo::baz")}));
}
+TEST_F(SymbolCollectorTest, Template) {
+ Annotations Header(R"(
+// Template is indexed, specialization and instantiation is not.
+template struct [[Tmpl]] {T x = 0};
+template <> struct Tmpl {};
+extern template struct Tmpl;
+template struct Tmpl;
+ )");
+ runSymbolCollector(Header.code(), /*Main=*/"");
+ EXPECT_THAT(Symbols, UnorderedElementsAreArray({AllOf(
+ QName("Tmpl"), DeclRange(Header.offsetRange()))}));
+}
+
TEST_F(SymbolCollectorTest, Locations) {
Annotations Header(R"cpp(
// Declared in header, defined in main.
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
@@ -115,10 +115,16 @@
// * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")
auto InTopLevelScope = hasDeclContext(
anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl()));
+ // Don't index template specializations.
+ auto IsSpecialization =
+ anyOf(functionDecl(isExplicitTemplateSpecialization()),
+cxxRecordDecl(isExplicitTemplateSpecialization()),
+varDecl(isExplicitTemplateSpecialization()));
if (match(decl(allOf(unless(isExpansionInMainFile()),
anyOf(InTopLevelScope,
hasDeclContext(enumDecl(InTopLevelScope,
- unless(isScoped())),
+ unless(isScoped(),
+ unless(IsSpecialization))),
*ND, *ASTCtx)
.empty())
return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44231: [clang-tidy] Check for sizeof that call functions
aaron.ballman added a comment. In https://reviews.llvm.org/D44231#1032011, @pfultz2 wrote: > > If the return type of foo() is changed, then the use for s1 will be > > automatically updated > > But usually you write it as: > > using foo_return = uint16_t; > foo_return foo(); > ... > size_t s1 = sizeof(foo()); > size_t s2 = sizeof(foo_return); > > > So you just update the `foo_return` typedef, and it will update it everywhere > that type is used. Again, that only works for C++ and not C. >> I think we need some data measurements over several large (>500k LoC) C and >> C++ code bases to see how frequently this check yields false positives. > > So I ran it on llvm. It mainly complained about areas where `sizeof` is used > for building traits. I should add an extra check that the function is not > overloaded. > > It did complain once about `sizeof` in a detection idiom(in BitmaskEnum.h) > and once with `sizeof(getVersion())`, like you mentioned. Did it report any true positives that would need correcting? LLVM is a good start, but hardly representative. Can you check some other large repos (both C++ and C), such as: Qt, cocos2d, rethinkdb, redis, and php-src? Did they produce true positives? How was the false positive rate? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r327129 - [clangd] Use identifier range as the definition range.
Author: hokein
Date: Fri Mar 9 06:00:34 2018
New Revision: 327129
URL: http://llvm.org/viewvc/llvm-project?rev=327129&view=rev
Log:
[clangd] Use identifier range as the definition range.
Summary: This also matches the range in symbol index.
Reviewers: sammccall
Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, cfe-commits
Differential Revision: https://reviews.llvm.org/D44247
Added:
clang-tools-extra/trunk/clangd/AST.cpp
clang-tools-extra/trunk/clangd/AST.h
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
Added: clang-tools-extra/trunk/clangd/AST.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.cpp?rev=327129&view=auto
==
--- clang-tools-extra/trunk/clangd/AST.cpp (added)
+++ clang-tools-extra/trunk/clangd/AST.cpp Fri Mar 9 06:00:34 2018
@@ -0,0 +1,42 @@
+//===--- AST.cpp - Utility AST functions ---*- C++
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "AST.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/Basic/SourceManager.h"
+
+namespace clang {
+namespace clangd {
+using namespace llvm;
+
+SourceLocation findNameLoc(const clang::Decl* D) {
+ const auto& SM = D->getASTContext().getSourceManager();
+ // FIXME: Revisit the strategy, the heuristic is limitted when handling
+ // macros, we should use the location where the whole definition occurs.
+ 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 "".
+ SpellingLoc = SM.getExpansionRange(D->getLocation()).first;
+}
+ }
+ return SpellingLoc;
+}
+
+} // namespace clangd
+} // namespace clang
Added: clang-tools-extra/trunk/clangd/AST.h
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.h?rev=327129&view=auto
==
--- clang-tools-extra/trunk/clangd/AST.h (added)
+++ clang-tools-extra/trunk/clangd/AST.h Fri Mar 9 06:00:34 2018
@@ -0,0 +1,34 @@
+//===--- AST.h - Utility AST functions -*- C++
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Various code that examines C++ source code using AST.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
+
+#include "clang/Basic/SourceLocation.h"
+
+namespace clang {
+class SourceManager;
+class Decl;
+
+namespace clangd {
+
+/// Find the identifier source location of the given D.
+///
+/// The returned location is usually the spelling location where the name of
the
+/// decl occurs in the code.
+SourceLocation findNameLoc(const clang::Decl* D);
+
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_AST_H_
Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=327129&r1=327128&r2=327129&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Fri Mar 9 06:00:34 2018
@@ -3,6 +3,7 @@ set(LLVM_LINK_COMPONENTS
)
add_clang_library(clangDaemon
+ AST.cpp
ClangdLSPServer.cpp
ClangdServer.cpp
ClangdUnit.cpp
Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=327129&r1=327128&r2=327129&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
[PATCH] D44247: [clangd] Use identifier range as the definition range.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE327129: [clangd] Use identifier range as the definition
range. (authored by hokein, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D44247?vs=137720&id=137736#toc
Repository:
rL LLVM
https://reviews.llvm.org/D44247
Files:
clangd/AST.cpp
clangd/AST.h
clangd/CMakeLists.txt
clangd/XRefs.cpp
clangd/index/SymbolCollector.cpp
unittests/clangd/ClangdTests.cpp
unittests/clangd/XRefsTests.cpp
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -435,9 +435,9 @@
TEST_F(ClangdVFSTest, ReparseOpenedFiles) {
Annotations FooSource(R"cpp(
#ifdef MACRO
-$one[[static void bob() {}]]
+static void $one[[bob]]() {}
#else
-$two[[static void bob() {}]]
+static void $two[[bob]]() {}
#endif
int main () { bo^b (); return 0; }
Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -119,74 +119,74 @@
const char *Tests[] = {
R"cpp(// Local variable
int main() {
- [[int bonjour]];
+ int [[bonjour]];
^bonjour = 2;
int test1 = bonjour;
}
)cpp",
R"cpp(// Struct
namespace ns1 {
-[[struct MyClass {}]];
+struct [[MyClass]] {};
} // namespace ns1
int main() {
ns1::My^Class* Params;
}
)cpp",
R"cpp(// Function definition via pointer
-[[int foo(int) {}]]
+int [[foo]](int) {}
int main() {
auto *X = &^foo;
}
)cpp",
R"cpp(// Function declaration via call
-[[int foo(int)]];
+int [[foo]](int);
int main() {
return ^foo(42);
}
)cpp",
R"cpp(// Field
-struct Foo { [[int x]]; };
+struct Foo { int [[x]]; };
int main() {
Foo bar;
bar.^x;
}
)cpp",
R"cpp(// Field, member initializer
struct Foo {
- [[int x]];
+ int [[x]];
Foo() : ^x(0) {}
};
)cpp",
R"cpp(// Field, GNU old-style field designator
-struct Foo { [[int x]]; };
+struct Foo { int [[x]]; };
int main() {
Foo bar = { ^x : 1 };
}
)cpp",
R"cpp(// Field, field designator
-struct Foo { [[int x]]; };
+struct Foo { int [[x]]; };
int main() {
Foo bar = { .^x = 2 };
}
)cpp",
R"cpp(// Method call
-struct Foo { [[int x()]]; };
+struct Foo { int [[x]](); };
int main() {
Foo bar;
bar.^x();
}
)cpp",
R"cpp(// Typedef
-[[typedef int Foo]];
+typedef int [[Foo]];
int main() {
^Foo bar;
}
@@ -199,9 +199,9 @@
)cpp", */
R"cpp(// Namespace
-[[namespace ns {
+namespace [[ns]] {
struct Foo { static void bar(); }
-}]] // namespace ns
+} // namespace ns
int main() { ^ns::Foo::bar(); }
)cpp",
@@ -215,14 +215,26 @@
R"cpp(// Forward class declaration
class Foo;
-[[class Foo {}]];
+class [[Foo]] {};
F^oo* foo();
)cpp",
R"cpp(// Function declaration
void foo();
void g() { f^oo(); }
-[[void foo() {}]]
+void [[foo]]() {}
+ )cpp",
+
+ R"cpp(
+#define FF(name) class name##_Test {};
+[[FF]](my);
+void f() { my^_Test a; }
+ )cpp",
+
+ R"cpp(
+ #define FF() class [[Test]] {};
+ FF();
+ void f() { T^est a; }
)cpp",
};
for (const char *Test : Tests) {
@@ -236,7 +248,7 @@
TEST(GoToDefinition, RelPathsInCompileCommand) {
Annotations SourceAnnotations(R"cpp(
-[[int foo]];
+int [[foo]];
int baz = f^oo;
)cpp");
Index: clangd/AST.cpp
===
--- clangd/AST.cpp
+++ clangd/AST.cpp
@@ -0,0 +1,42 @@
+//===--- AST.cpp - Utility AST functions ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "AST.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/Basic/SourceManager.h"
+
+namespace clang {
+namespace clangd {
+using namespace llvm;
+
+SourceLocation findNameLoc(const clang::Decl* D) {
+ const auto& SM = D->getASTContext().getSourceManager();
+ // FIXME: Revisit the strategy, the heuristic is lim
[PATCH] D44247: [clangd] Use identifier range as the definition range.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL327129: [clangd] Use identifier range as the definition
range. (authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D44247?vs=137720&id=137737#toc
Repository:
rL LLVM
https://reviews.llvm.org/D44247
Files:
clang-tools-extra/trunk/clangd/AST.cpp
clang-tools-extra/trunk/clangd/AST.h
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
Index: clang-tools-extra/trunk/clangd/AST.cpp
===
--- clang-tools-extra/trunk/clangd/AST.cpp
+++ clang-tools-extra/trunk/clangd/AST.cpp
@@ -0,0 +1,42 @@
+//===--- AST.cpp - Utility AST functions ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "AST.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/Basic/SourceManager.h"
+
+namespace clang {
+namespace clangd {
+using namespace llvm;
+
+SourceLocation findNameLoc(const clang::Decl* D) {
+ const auto& SM = D->getASTContext().getSourceManager();
+ // FIXME: Revisit the strategy, the heuristic is limitted when handling
+ // macros, we should use the location where the whole definition occurs.
+ 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 "".
+ SpellingLoc = SM.getExpansionRange(D->getLocation()).first;
+}
+ }
+ return SpellingLoc;
+}
+
+} // namespace clangd
+} // namespace clang
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
@@ -8,6 +8,7 @@
//===--===//
#include "SymbolCollector.h"
+#include "../AST.h"
#include "../CodeCompletionStrings.h"
#include "../Logger.h"
#include "../URI.h"
@@ -184,30 +185,16 @@
llvm::Optional getSymbolLocation(
const NamedDecl &D, SourceManager &SM, const SymbolCollector::Options &Opts,
const clang::LangOptions &LangOpts, std::string &FileURIStorage) {
- 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 "".
- SpellingLoc = SM.getExpansionRange(D.getLocation()).first;
-}
- }
-
- auto U = toURI(SM, SM.getFilename(SpellingLoc), Opts);
+ SourceLocation NameLoc = findNameLoc(&D);
+ auto U = toURI(SM, SM.getFilename(NameLoc), Opts);
if (!U)
return llvm::None;
FileURIStorage = std::move(*U);
SymbolLocation Result;
Result.FileURI = FileURIStorage;
- Result.StartOffset = SM.getFileOffset(SpellingLoc);
+ Result.StartOffset = SM.getFileOffset(NameLoc);
Result.EndOffset = Result.StartOffset + clang::Lexer::MeasureTokenLength(
- SpellingLoc, SM, LangOpts);
+ NameLoc, SM, LangOpts);
return std::move(Result);
}
Index: clang-tools-extra/trunk/clangd/AST.h
===
--- clang-tools-extra/trunk/clangd/AST.h
+++ clang-tools-extra/trunk/clangd/AST.h
@@ -0,0 +1,34 @@
+//===--- AST.h - Utility AST functions -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.T
[clang-tools-extra] r327130 - [clangd-vscode] Add package-lock.json to .gitignore
Author: ioeric Date: Fri Mar 9 06:06:43 2018 New Revision: 327130 URL: http://llvm.org/viewvc/llvm-project?rev=327130&view=rev Log: [clangd-vscode] Add package-lock.json to .gitignore Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore?rev=327130&r1=327129&r2=327130&view=diff == --- clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore (original) +++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/.gitignore Fri Mar 9 06:06:43 2018 @@ -1,2 +1,3 @@ out -node_modules \ No newline at end of file +node_modules +package-lock.json ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r327131 - [clangd] Fix failing lit test.
Author: hokein
Date: Fri Mar 9 06:16:46 2018
New Revision: 327131
URL: http://llvm.org/viewvc/llvm-project?rev=327131&view=rev
Log:
[clangd] Fix failing lit test.
This test is missed in r327129.
Modified:
clang-tools-extra/trunk/test/clangd/xrefs.test
Modified: clang-tools-extra/trunk/test/clangd/xrefs.test
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/xrefs.test?rev=327131&r1=327130&r2=327131&view=diff
==
--- clang-tools-extra/trunk/test/clangd/xrefs.test (original)
+++ clang-tools-extra/trunk/test/clangd/xrefs.test Fri Mar 9 06:16:46 2018
@@ -10,11 +10,11 @@
# CHECK-NEXT:{
# CHECK-NEXT: "range": {
# CHECK-NEXT:"end": {
-# CHECK-NEXT: "character": 9,
+# CHECK-NEXT: "character": 5,
# CHECK-NEXT: "line": 0
# CHECK-NEXT:},
# CHECK-NEXT:"start": {
-# CHECK-NEXT: "character": 0,
+# CHECK-NEXT: "character": 4,
# CHECK-NEXT: "line": 0
# CHECK-NEXT:}
# CHECK-NEXT: },
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44300: [SemaOverload] Fixed crash on code completion
ilya-biryukov created this revision.
ilya-biryukov added reviewers: bkramer, sammccall, ioeric, hokein.
The relevant failing assertion message is:
../tools/clang/lib/Sema/SemaInit.cpp:8411: PerformCopyInitialization():
Assertion `InitE && "No initialization expression?"' failed.
See the added test case for a repro.
Repository:
rC Clang
https://reviews.llvm.org/D44300
Files:
lib/Sema/SemaOverload.cpp
test/CodeCompletion/enable-if-attr-crash.cpp
Index: test/CodeCompletion/enable-if-attr-crash.cpp
===
--- /dev/null
+++ test/CodeCompletion/enable-if-attr-crash.cpp
@@ -0,0 +1,8 @@
+int foo(bool x) __attribute__((enable_if(x, "")));
+
+int test() {
+ bool fff;
+ // RUN: %clang_cc1 -std=c++11 -code-completion-at=%s:7:8 %s | FileCheck %s
+ // CHECK: COMPLETION: fff : [#bool#]fff
+ foo(ff
+}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -6245,12 +6245,15 @@
if (!Function->isVariadic() && Args.size() < Function->getNumParams()) {
for (unsigned i = Args.size(), e = Function->getNumParams(); i != e; ++i) {
ParmVarDecl *P = Function->getParamDecl(i);
- ExprResult R = S.PerformCopyInitialization(
- InitializedEntity::InitializeParameter(S.Context,
- Function->getParamDecl(i)),
- SourceLocation(),
- P->hasUninstantiatedDefaultArg() ? P->getUninstantiatedDefaultArg()
- : P->getDefaultArg());
+ Expr *DefArg = P->hasUninstantiatedDefaultArg()
+ ? P->getUninstantiatedDefaultArg()
+ : P->getDefaultArg();
+ if (!DefArg)
+return false;
+ ExprResult R =
+ S.PerformCopyInitialization(InitializedEntity::InitializeParameter(
+ S.Context,
Function->getParamDecl(i)),
+ SourceLocation(), DefArg);
if (R.isInvalid())
return false;
ConvertedArgs.push_back(R.get());
Index: test/CodeCompletion/enable-if-attr-crash.cpp
===
--- /dev/null
+++ test/CodeCompletion/enable-if-attr-crash.cpp
@@ -0,0 +1,8 @@
+int foo(bool x) __attribute__((enable_if(x, "")));
+
+int test() {
+ bool fff;
+ // RUN: %clang_cc1 -std=c++11 -code-completion-at=%s:7:8 %s | FileCheck %s
+ // CHECK: COMPLETION: fff : [#bool#]fff
+ foo(ff
+}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -6245,12 +6245,15 @@
if (!Function->isVariadic() && Args.size() < Function->getNumParams()) {
for (unsigned i = Args.size(), e = Function->getNumParams(); i != e; ++i) {
ParmVarDecl *P = Function->getParamDecl(i);
- ExprResult R = S.PerformCopyInitialization(
- InitializedEntity::InitializeParameter(S.Context,
- Function->getParamDecl(i)),
- SourceLocation(),
- P->hasUninstantiatedDefaultArg() ? P->getUninstantiatedDefaultArg()
- : P->getDefaultArg());
+ Expr *DefArg = P->hasUninstantiatedDefaultArg()
+ ? P->getUninstantiatedDefaultArg()
+ : P->getDefaultArg();
+ if (!DefArg)
+return false;
+ ExprResult R =
+ S.PerformCopyInitialization(InitializedEntity::InitializeParameter(
+ S.Context, Function->getParamDecl(i)),
+ SourceLocation(), DefArg);
if (R.isInvalid())
return false;
ConvertedArgs.push_back(R.get());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44300: [SemaOverload] Fixed crash on code completion
sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: lib/Sema/SemaOverload.cpp:6251 + : P->getDefaultArg(); + if (!DefArg) +return false; comment this case can be hit in code completion Repository: rC Clang https://reviews.llvm.org/D44300 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44300: [SemaOverload] Fixed crash on code completion
ilya-biryukov updated this revision to Diff 137743.
ilya-biryukov added a comment.
- Added a comment
Repository:
rC Clang
https://reviews.llvm.org/D44300
Files:
lib/Sema/SemaOverload.cpp
test/CodeCompletion/enable-if-attr-crash.cpp
Index: test/CodeCompletion/enable-if-attr-crash.cpp
===
--- /dev/null
+++ test/CodeCompletion/enable-if-attr-crash.cpp
@@ -0,0 +1,8 @@
+int foo(bool x) __attribute__((enable_if(x, "")));
+
+int test() {
+ bool fff;
+ // RUN: %clang_cc1 -std=c++11 -code-completion-at=%s:7:8 %s | FileCheck %s
+ // CHECK: COMPLETION: fff : [#bool#]fff
+ foo(ff
+}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -6245,12 +6245,17 @@
if (!Function->isVariadic() && Args.size() < Function->getNumParams()) {
for (unsigned i = Args.size(), e = Function->getNumParams(); i != e; ++i) {
ParmVarDecl *P = Function->getParamDecl(i);
- ExprResult R = S.PerformCopyInitialization(
- InitializedEntity::InitializeParameter(S.Context,
- Function->getParamDecl(i)),
- SourceLocation(),
- P->hasUninstantiatedDefaultArg() ? P->getUninstantiatedDefaultArg()
- : P->getDefaultArg());
+ Expr *DefArg = P->hasUninstantiatedDefaultArg()
+ ? P->getUninstantiatedDefaultArg()
+ : P->getDefaultArg();
+ // This can only happen in code completion, i.e. when PartialOverloading
+ // is true.
+ if (!DefArg)
+return false;
+ ExprResult R =
+ S.PerformCopyInitialization(InitializedEntity::InitializeParameter(
+ S.Context,
Function->getParamDecl(i)),
+ SourceLocation(), DefArg);
if (R.isInvalid())
return false;
ConvertedArgs.push_back(R.get());
Index: test/CodeCompletion/enable-if-attr-crash.cpp
===
--- /dev/null
+++ test/CodeCompletion/enable-if-attr-crash.cpp
@@ -0,0 +1,8 @@
+int foo(bool x) __attribute__((enable_if(x, "")));
+
+int test() {
+ bool fff;
+ // RUN: %clang_cc1 -std=c++11 -code-completion-at=%s:7:8 %s | FileCheck %s
+ // CHECK: COMPLETION: fff : [#bool#]fff
+ foo(ff
+}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -6245,12 +6245,17 @@
if (!Function->isVariadic() && Args.size() < Function->getNumParams()) {
for (unsigned i = Args.size(), e = Function->getNumParams(); i != e; ++i) {
ParmVarDecl *P = Function->getParamDecl(i);
- ExprResult R = S.PerformCopyInitialization(
- InitializedEntity::InitializeParameter(S.Context,
- Function->getParamDecl(i)),
- SourceLocation(),
- P->hasUninstantiatedDefaultArg() ? P->getUninstantiatedDefaultArg()
- : P->getDefaultArg());
+ Expr *DefArg = P->hasUninstantiatedDefaultArg()
+ ? P->getUninstantiatedDefaultArg()
+ : P->getDefaultArg();
+ // This can only happen in code completion, i.e. when PartialOverloading
+ // is true.
+ if (!DefArg)
+return false;
+ ExprResult R =
+ S.PerformCopyInitialization(InitializedEntity::InitializeParameter(
+ S.Context, Function->getParamDecl(i)),
+ SourceLocation(), DefArg);
if (R.isInvalid())
return false;
ConvertedArgs.push_back(R.get());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r327134 - [SemaOverload] Fixed crash on code completion
Author: ibiryukov
Date: Fri Mar 9 06:43:29 2018
New Revision: 327134
URL: http://llvm.org/viewvc/llvm-project?rev=327134&view=rev
Log:
[SemaOverload] Fixed crash on code completion
Summary:
The relevant failing assertion message is:
../tools/clang/lib/Sema/SemaInit.cpp:8411: PerformCopyInitialization():
Assertion `InitE && "No initialization expression?"' failed.
See the added test case for a repro.
Reviewers: bkramer, sammccall, ioeric, hokein
Reviewed By: sammccall
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D44300
Added:
cfe/trunk/test/CodeCompletion/enable-if-attr-crash.cpp
Modified:
cfe/trunk/lib/Sema/SemaOverload.cpp
Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=327134&r1=327133&r2=327134&view=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Fri Mar 9 06:43:29 2018
@@ -6245,12 +6245,17 @@ convertArgsForAvailabilityChecks(Sema &S
if (!Function->isVariadic() && Args.size() < Function->getNumParams()) {
for (unsigned i = Args.size(), e = Function->getNumParams(); i != e; ++i) {
ParmVarDecl *P = Function->getParamDecl(i);
- ExprResult R = S.PerformCopyInitialization(
- InitializedEntity::InitializeParameter(S.Context,
- Function->getParamDecl(i)),
- SourceLocation(),
- P->hasUninstantiatedDefaultArg() ? P->getUninstantiatedDefaultArg()
- : P->getDefaultArg());
+ Expr *DefArg = P->hasUninstantiatedDefaultArg()
+ ? P->getUninstantiatedDefaultArg()
+ : P->getDefaultArg();
+ // This can only happen in code completion, i.e. when PartialOverloading
+ // is true.
+ if (!DefArg)
+return false;
+ ExprResult R =
+ S.PerformCopyInitialization(InitializedEntity::InitializeParameter(
+ S.Context,
Function->getParamDecl(i)),
+ SourceLocation(), DefArg);
if (R.isInvalid())
return false;
ConvertedArgs.push_back(R.get());
Added: cfe/trunk/test/CodeCompletion/enable-if-attr-crash.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/enable-if-attr-crash.cpp?rev=327134&view=auto
==
--- cfe/trunk/test/CodeCompletion/enable-if-attr-crash.cpp (added)
+++ cfe/trunk/test/CodeCompletion/enable-if-attr-crash.cpp Fri Mar 9 06:43:29
2018
@@ -0,0 +1,8 @@
+int foo(bool x) __attribute__((enable_if(x, "")));
+
+int test() {
+ bool fff;
+ // RUN: %clang_cc1 -std=c++11 -code-completion-at=%s:7:8 %s | FileCheck %s
+ // CHECK: COMPLETION: fff : [#bool#]fff
+ foo(ff
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44300: [SemaOverload] Fixed crash on code completion
This revision was automatically updated to reflect the committed changes.
Closed by commit rL327134: [SemaOverload] Fixed crash on code completion
(authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D44300
Files:
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/CodeCompletion/enable-if-attr-crash.cpp
Index: cfe/trunk/lib/Sema/SemaOverload.cpp
===
--- cfe/trunk/lib/Sema/SemaOverload.cpp
+++ cfe/trunk/lib/Sema/SemaOverload.cpp
@@ -6245,12 +6245,17 @@
if (!Function->isVariadic() && Args.size() < Function->getNumParams()) {
for (unsigned i = Args.size(), e = Function->getNumParams(); i != e; ++i) {
ParmVarDecl *P = Function->getParamDecl(i);
- ExprResult R = S.PerformCopyInitialization(
- InitializedEntity::InitializeParameter(S.Context,
- Function->getParamDecl(i)),
- SourceLocation(),
- P->hasUninstantiatedDefaultArg() ? P->getUninstantiatedDefaultArg()
- : P->getDefaultArg());
+ Expr *DefArg = P->hasUninstantiatedDefaultArg()
+ ? P->getUninstantiatedDefaultArg()
+ : P->getDefaultArg();
+ // This can only happen in code completion, i.e. when PartialOverloading
+ // is true.
+ if (!DefArg)
+return false;
+ ExprResult R =
+ S.PerformCopyInitialization(InitializedEntity::InitializeParameter(
+ S.Context,
Function->getParamDecl(i)),
+ SourceLocation(), DefArg);
if (R.isInvalid())
return false;
ConvertedArgs.push_back(R.get());
Index: cfe/trunk/test/CodeCompletion/enable-if-attr-crash.cpp
===
--- cfe/trunk/test/CodeCompletion/enable-if-attr-crash.cpp
+++ cfe/trunk/test/CodeCompletion/enable-if-attr-crash.cpp
@@ -0,0 +1,8 @@
+int foo(bool x) __attribute__((enable_if(x, "")));
+
+int test() {
+ bool fff;
+ // RUN: %clang_cc1 -std=c++11 -code-completion-at=%s:7:8 %s | FileCheck %s
+ // CHECK: COMPLETION: fff : [#bool#]fff
+ foo(ff
+}
Index: cfe/trunk/lib/Sema/SemaOverload.cpp
===
--- cfe/trunk/lib/Sema/SemaOverload.cpp
+++ cfe/trunk/lib/Sema/SemaOverload.cpp
@@ -6245,12 +6245,17 @@
if (!Function->isVariadic() && Args.size() < Function->getNumParams()) {
for (unsigned i = Args.size(), e = Function->getNumParams(); i != e; ++i) {
ParmVarDecl *P = Function->getParamDecl(i);
- ExprResult R = S.PerformCopyInitialization(
- InitializedEntity::InitializeParameter(S.Context,
- Function->getParamDecl(i)),
- SourceLocation(),
- P->hasUninstantiatedDefaultArg() ? P->getUninstantiatedDefaultArg()
- : P->getDefaultArg());
+ Expr *DefArg = P->hasUninstantiatedDefaultArg()
+ ? P->getUninstantiatedDefaultArg()
+ : P->getDefaultArg();
+ // This can only happen in code completion, i.e. when PartialOverloading
+ // is true.
+ if (!DefArg)
+return false;
+ ExprResult R =
+ S.PerformCopyInitialization(InitializedEntity::InitializeParameter(
+ S.Context, Function->getParamDecl(i)),
+ SourceLocation(), DefArg);
if (R.isInvalid())
return false;
ConvertedArgs.push_back(R.get());
Index: cfe/trunk/test/CodeCompletion/enable-if-attr-crash.cpp
===
--- cfe/trunk/test/CodeCompletion/enable-if-attr-crash.cpp
+++ cfe/trunk/test/CodeCompletion/enable-if-attr-crash.cpp
@@ -0,0 +1,8 @@
+int foo(bool x) __attribute__((enable_if(x, "")));
+
+int test() {
+ bool fff;
+ // RUN: %clang_cc1 -std=c++11 -code-completion-at=%s:7:8 %s | FileCheck %s
+ // CHECK: COMPLETION: fff : [#bool#]fff
+ foo(ff
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r327105 - CodeGen: simplify and validate exception personalities
Looks like this broke the Windows bot:
http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/9365 Can you
fix or revert, please?
On Fri, Mar 9, 2018 at 2:06 AM, Saleem Abdulrasool via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> Author: compnerd
> Date: Thu Mar 8 23:06:42 2018
> New Revision: 327105
>
> URL: http://llvm.org/viewvc/llvm-project?rev=327105&view=rev
> Log:
> CodeGen: simplify and validate exception personalities
>
> Simplify the dispatching for the personality routines. This really had
> no test coverage previously, so add test coverage for the various cases.
> This turns out to be pretty complicated as the various languages and
> models interact to change personalities around.
>
> You really should feel bad for the compiler if you are using exceptions.
> There is no reason for this type of cruelty.
>
> Added:
> cfe/trunk/test/CodeGen/personality.c
> cfe/trunk/test/CodeGenCXX/personality.cpp
> cfe/trunk/test/CodeGenObjC/personality.m
> cfe/trunk/test/CodeGenObjCXX/personality.mm
> Modified:
> cfe/trunk/lib/CodeGen/CGException.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGException.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/
> CGException.cpp?rev=327105&r1=327104&r2=327105&view=diff
>
> ==
> --- cfe/trunk/lib/CodeGen/CGException.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGException.cpp Thu Mar 8 23:06:42 2018
> @@ -116,6 +116,10 @@ static const EHPersonality &getCPersonal
> const LangOptions &L) {
>if (L.SjLjExceptions)
> return EHPersonality::GNU_C_SJLJ;
> + if (L.DWARFExceptions)
> +return EHPersonality::GNU_C;
> + if (T.isWindowsMSVCEnvironment())
> +return EHPersonality::MSVC_CxxFrameHandler3;
>if (L.SEHExceptions)
> return EHPersonality::GNU_C_SEH;
>return EHPersonality::GNU_C;
> @@ -129,6 +133,8 @@ static const EHPersonality &getObjCPerso
>case ObjCRuntime::MacOSX:
>case ObjCRuntime::iOS:
>case ObjCRuntime::WatchOS:
> +if (T.isWindowsMSVCEnvironment())
> + return EHPersonality::MSVC_CxxFrameHandler3;
> return EHPersonality::NeXT_ObjC;
>case ObjCRuntime::GNUstep:
> if (L.ObjCRuntime.getVersion() >= VersionTuple(1, 7))
> @@ -149,6 +155,10 @@ static const EHPersonality &getCXXPerson
>const LangOptions &L) {
>if (L.SjLjExceptions)
> return EHPersonality::GNU_CPlusPlus_SJLJ;
> + if (L.DWARFExceptions)
> +return EHPersonality::GNU_CPlusPlus;
> + if (T.isWindowsMSVCEnvironment())
> +return EHPersonality::MSVC_CxxFrameHandler3;
>if (L.SEHExceptions)
> return EHPersonality::GNU_CPlusPlus_SEH;
>return EHPersonality::GNU_CPlusPlus;
> @@ -199,25 +209,9 @@ const EHPersonality &EHPersonality::get(
>if (FD && FD->usesSEHTry())
> return getSEHPersonalityMSVC(T);
>
> - // Try to pick a personality function that is compatible with MSVC if
> we're
> - // not compiling Obj-C. Obj-C users better have an Obj-C runtime that
> supports
> - // the GCC-style personality function.
> - if (T.isWindowsMSVCEnvironment() && !L.ObjC1) {
> -if (L.SjLjExceptions)
> - return EHPersonality::GNU_CPlusPlus_SJLJ;
> -if (L.DWARFExceptions)
> - return EHPersonality::GNU_CPlusPlus;
> -return EHPersonality::MSVC_CxxFrameHandler3;
> - }
> -
> - if (L.CPlusPlus && L.ObjC1)
> -return getObjCXXPersonality(T, L);
> - else if (L.CPlusPlus)
> -return getCXXPersonality(T, L);
> - else if (L.ObjC1)
> -return getObjCPersonality(T, L);
> - else
> -return getCPersonality(T, L);
> + if (L.ObjC1)
> +return L.CPlusPlus ? getObjCXXPersonality(T, L) :
> getObjCPersonality(T, L);
> + return L.CPlusPlus ? getCXXPersonality(T, L) : getCPersonality(T, L);
> }
>
> const EHPersonality &EHPersonality::get(CodeGenFunction &CGF) {
>
> Added: cfe/trunk/test/CodeGen/personality.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> CodeGen/personality.c?rev=327105&view=auto
>
> ==
> --- cfe/trunk/test/CodeGen/personality.c (added)
> +++ cfe/trunk/test/CodeGen/personality.c Thu Mar 8 23:06:42 2018
> @@ -0,0 +1,46 @@
> +// RUN: %clang_cc1 -triple i686-unknown-linux-gnu -fexceptions -fblocks
> -S -emit-llvm %s -o - | FileCheck %s -check-prefix CHECK-GNU
> +// RUN: %clang_cc1 -triple i686-unknown-linux-gnu -fexceptions
> -fdwarf-exceptions -fblocks -S -emit-llvm %s -o - | FileCheck %s
> -check-prefix CHECK-DWARF
> +// RUN: %clang_cc1 -triple i686-unknown-linux-gnu -fexceptions
> -fseh-exceptions -fblocks -S -emit-llvm %s -o - | FileCheck %s
> -check-prefix CHECK-SEH
> +// RUN: %clang_cc1 -triple i686-unknown-linux-gnu -fexceptions
> -fsjlj-exceptions -fblocks -S -emit-llvm %s -o - | FileCheck %s
> -check-prefix CHECK-SJLJ
> +
> +// RUN: %clang_cc1 -triple i686-unknown-windows-msvc -fexcept
RE: [llvm] r325653 - [mips] Spectre variant two mitigation for MIPSR2
Hi,
Any downstream users should be able to apply this, the corresponding clang
patch (r325651) and the LLD patch (r325657, and r325713) cleanly to the
release sources of LLVM 6.0, should they require it.
Thanks,
Simon
Author: sdardis
Date: Tue Feb 20 16:06:53 2018
New Revision: 325653
URL: http://llvm.org/viewvc/llvm-project?rev=325653&view=rev
Log:
[mips] Spectre variant two mitigation for MIPSR2
This patch provides mitigation for CVE-2017-5715, Spectre variant two,
which affects the P5600 and P6600. It implements the LLVM part of
-mindirect-jump=hazard. It is _not_ enabled by default for the P5600.
The migitation strategy suggested by MIPS for these processors is to use
hazard barrier instructions. 'jalr.hb' and 'jr.hb' are hazard
barrier variants of the 'jalr' and 'jr' instructions respectively.
These instructions impede the execution of instruction stream until
architecturally defined hazards (changes to the instruction stream,
privileged registers which may affect execution) are cleared. These
instructions in MIPS' designs are not speculated past.
These instructions are used with the attribute +use-indirect-jump-hazard
when branching indirectly and for indirect function calls.
These instructions are defined by the MIPS32R2 ISA, so this mitigation
method is not compatible with processors which implement an earlier
revision of the MIPS ISA.
Performance benchmarking of this option with -fpic and lld using
-z hazardplt shows a difference of overall 10%~ time increase
for the LLVM testsuite. Certain benchmarks such as methcall show a
substantially larger increase in time due to their nature.
Reviewers: atanasyan, zoran.jovanovic
Differential Revision: https://reviews.llvm.org/D43486
Added:
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/calls.ll
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/guards-verify-call.mir
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/guards-verify-tailcall.mir
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/jumptables.ll
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/long-branch.ll
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/long-calls.ll
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/unsupported-micromips.ll
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/unsupported-mips32.ll
Modified:
llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
llvm/trunk/lib/Target/Mips/MicroMips32r6InstrInfo.td
llvm/trunk/lib/Target/Mips/MicroMipsInstrInfo.td
llvm/trunk/lib/Target/Mips/Mips.td
llvm/trunk/lib/Target/Mips/Mips32r6InstrInfo.td
llvm/trunk/lib/Target/Mips/Mips64InstrInfo.td
llvm/trunk/lib/Target/Mips/Mips64r6InstrInfo.td
llvm/trunk/lib/Target/Mips/MipsDSPInstrFormats.td
llvm/trunk/lib/Target/Mips/MipsInstrFormats.td
llvm/trunk/lib/Target/Mips/MipsInstrInfo.cpp
llvm/trunk/lib/Target/Mips/MipsInstrInfo.td
llvm/trunk/lib/Target/Mips/MipsLongBranch.cpp
llvm/trunk/lib/Target/Mips/MipsSubtarget.cpp
llvm/trunk/lib/Target/Mips/MipsSubtarget.h
Modified: llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
URL:
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp?rev=325653&r1=325652&r2=325653&view=diff
==
--- llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp (original)
+++ llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp Tue Feb 20 16:06:53
2018
@@ -5136,6 +5136,7 @@ unsigned MipsAsmParser::checkTargetMatch
// It also applies for registers Rt and Rs of microMIPSr6 jalrc.hb
instruction
// and registers Rd and Base for microMIPS lwp instruction
case Mips::JALR_HB:
+ case Mips::JALR_HB64:
case Mips::JALRC_HB_MMR6:
case Mips::JALRC_MMR6:
if (Inst.getOperand(0).getReg() == Inst.getOperand(1).getReg())
Modified: llvm/trunk/lib/Target/Mips/MicroMips32r6InstrInfo.td
URL:
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MicroMips32r6InstrInfo.td?rev=325653&r1=325652&r2=325653&view=diff
==
--- llvm/trunk/lib/Target/Mips/MicroMips32r6InstrInfo.td (original)
+++ llvm/trunk/lib/Target/Mips/MicroMips32r6InstrInfo.td Tue Feb 20 16:06:53
2018
@@ -1713,6 +1713,12 @@ let AddedComplexity = 41 in {
def TAILCALL_MMR6 : TailCall, ISA_MICROMIPS32R6;
+def TAILCALLREG_MMR6 : TailCallReg, ISA_MICROMIPS32R6;
+
+def PseudoIndirectBranch_MMR6 : PseudoIndirectBranchBase,
+ISA_MICROMIPS32R6;
+
def : MipsPat<(MipsTailCall (iPTR tglobaladdr:$dst)),
(TAILCALL_MMR6 tglobaladdr:$dst)>, ISA_MICROMIPS32R6;
Modified: llvm/trunk/lib/Target/Mips/MicroMipsInstrInfo.td
URL:
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MicroMipsInstrInfo.td?rev=325653&r1=325652&r2=325653&view=diff
=
[PATCH] D43197: [OpenMP] Add flag for linking runtime bitcode library
ABataev added inline comments.
Comment at: lib/Driver/ToolChains/Cuda.cpp:592
+Twine("lib") + CLANG_LIBDIR_SUFFIX);
+LibraryPaths.emplace_back(DefaultLibPath.c_str());
+
Do you still need `.c_str()` here?
Comment at: test/Driver/openmp-offload-gpu.c:150
+/// bitcode library and add it to the LIBRARY_PATH.
+// RUN: touch %T/libomptarget-nvptx-sm_60.bc
+// RUN: env LIBRARY_PATH=%T %clang -### -fopenmp=libomp
-fopenmp-targets=nvptx64-nvidia-cuda \
Create empty `libomptarget-nvptx-sm_60.bc` in `Driver/lib` directory and use it
in the test rather create|delete it dynamically.
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] D43197: [OpenMP] Add flag for linking runtime bitcode library
gtbercea marked an inline comment as done.
gtbercea added inline comments.
Comment at: lib/Driver/ToolChains/Cuda.cpp:536-542
+ StringRef CompilerPath = env;
+ while (!CompilerPath.empty()) {
+std::pair Split =
+CompilerPath.split(llvm::sys::EnvPathSeparator);
+LibraryPaths.push_back(Split.first);
+CompilerPath = Split.second;
+ }
Hahnfeld wrote:
> gtbercea wrote:
> > Hahnfeld wrote:
> > > gtbercea wrote:
> > > > Hahnfeld wrote:
> > > > > `tools::addDirectoryList` uses `StringRef::find`, I'm not sure if
> > > > > `StringRef::split` creates real copies of the string...
> > > > What is your suggestion?
> > > IMO you should use whatever existing code does, in that case
> > > `StringRef::find`.
> > Is this comment still relevant in the light of the most recent changes?
> Probably not (although the code is now completely different from
> `tools::addDirectoryList`)
Gotcha, do let me know if you see any other issue with this version of the
code. I will mark this one as done for now.
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
r327136 - [OPENMP] Fix the address of the original variable in task reductions.
Author: abataev
Date: Fri Mar 9 07:20:30 2018
New Revision: 327136
URL: http://llvm.org/viewvc/llvm-project?rev=327136&view=rev
Log:
[OPENMP] Fix the address of the original variable in task reductions.
If initialization of the task reductions requires pointer to original
variable, which is stored in the threadprivate storage, we used the
address of this pointer instead.
Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/taskloop_reduction_codegen.cpp
Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=327136&r1=327135&r2=327136&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Fri Mar 9 07:20:30 2018
@@ -5422,6 +5422,9 @@ static llvm::Value *emitReduceInitFuncti
CGM.getOpenMPRuntime().getAddrOfArtificialThreadPrivate(
CGF, CGM.getContext().VoidPtrTy,
generateUniqueName(CGM, "reduction", RCG.getRefExpr(N)));
+SharedAddr = CGF.EmitLoadOfPointer(
+SharedAddr,
+CGM.getContext().VoidPtrTy.castAs()->getTypePtr());
SharedLVal = CGF.MakeAddrLValue(SharedAddr, CGM.getContext().VoidPtrTy);
} else {
SharedLVal = CGF.MakeNaturalAlignAddrLValue(
Modified: cfe/trunk/test/OpenMP/taskloop_reduction_codegen.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/taskloop_reduction_codegen.cpp?rev=327136&r1=327135&r2=327136&view=diff
==
--- cfe/trunk/test/OpenMP/taskloop_reduction_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/taskloop_reduction_codegen.cpp Fri Mar 9 07:20:30
2018
@@ -164,7 +164,9 @@ sum = 0.0;
// CHECK: define internal void @[[RED_INIT2]](i8*)
// CHECK: call i8* @__kmpc_threadprivate_cached(
-// CHECK: call i8* @__kmpc_threadprivate_cached(
+// CHECK: [[ORIG_PTR_ADDR:%.+]] = call i8* @__kmpc_threadprivate_cached(
+// CHECK: [[ORIG_PTR_REF:%.+]] = bitcast i8* [[ORIG_PTR_ADDR]] to i8**
+// CHECK: load i8*, i8** [[ORIG_PTR_REF]],
// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(
// CHECK: ret void
___
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
grokos added inline comments. Comment at: test/Driver/openmp-offload-gpu.c:150 +/// bitcode library and add it to the LIBRARY_PATH. +// RUN: touch %T/libomptarget-nvptx-sm_60.bc +// RUN: env LIBRARY_PATH=%T %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \ ABataev wrote: > Create empty `libomptarget-nvptx-sm_60.bc` in `Driver/lib` directory and use > it in the test rather create|delete it dynamically. I'm also in favour of this approach. On some systems /tmp is not accessible and the regression test fails. 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] D44305: [clangd] Add an interface that finds symbol by SymbolID in SymbolIndex.
ioeric created this revision.
ioeric added a reviewer: sammccall.
Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek.
Potential use case: argument go-to-definition result with symbol
information (e.g. function definition in cc file) that might not be in the AST.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44305
Files:
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
clangd/index/Index.h
clangd/index/MemIndex.cpp
clangd/index/MemIndex.h
clangd/index/Merge.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/IndexTests.cpp
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -89,19 +89,30 @@
return generateSymbols(Names, WeakSymbols);
}
+std::string getQualifiedName(const Symbol &Sym) {
+ return (Sym.Scope + (Sym.Scope.empty() ? "" : "::") + Sym.Name).str();
+}
+
std::vector match(const SymbolIndex &I,
const FuzzyFindRequest &Req,
bool *Incomplete = nullptr) {
std::vector Matches;
bool IsIncomplete = I.fuzzyFind(Req, [&](const Symbol &Sym) {
-Matches.push_back(
-(Sym.Scope + (Sym.Scope.empty() ? "" : "::") + Sym.Name).str());
+Matches.push_back(getQualifiedName(Sym));
});
if (Incomplete)
*Incomplete = IsIncomplete;
return Matches;
}
+// Returns the qualified name of the symbol with ID in the index, or "" if there
+// is no such symbol.
+std::string getSymbol(const SymbolIndex &I, const SymbolID &ID) {
+ std::string Res = "";
+ I.getSymbol(ID, [&](const Symbol &Sym) { Res = getQualifiedName(Sym); });
+ return Res;
+}
+
TEST(MemIndexTest, MemIndexSymbolsRecycled) {
MemIndex I;
std::weak_ptr Symbols;
@@ -209,7 +220,14 @@
EXPECT_THAT(match(I, Req), UnorderedElementsAre("ns::ABC", "ns::abc"));
}
-TEST(MergeTest, MergeIndex) {
+TEST(MemIndexTest, GetSymbol) {
+ MemIndex I;
+ I.build(generateSymbols({"ns::abc", "ns::xyz"}));
+ EXPECT_EQ(getSymbol(I, SymbolID("ns::abc")), "ns::abc");
+ EXPECT_EQ(getSymbol(I, SymbolID("ns::nonono")), "");
+}
+
+TEST(MergeIndexTest, FuzzyFind) {
MemIndex I, J;
I.build(generateSymbols({"ns::A", "ns::B"}));
J.build(generateSymbols({"ns::B", "ns::C"}));
@@ -219,6 +237,16 @@
UnorderedElementsAre("ns::A", "ns::B", "ns::C"));
}
+TEST(MergeIndexTest, GetSymbol) {
+ MemIndex I, J;
+ I.build(generateSymbols({"ns::A", "ns::B"}));
+ J.build(generateSymbols({"ns::B", "ns::C"}));
+ EXPECT_EQ(getSymbol(*mergeIndex(&I, &J), SymbolID("ns::A")), "ns::A");
+ EXPECT_EQ(getSymbol(*mergeIndex(&I, &J), SymbolID("ns::B")), "ns::B");
+ EXPECT_EQ(getSymbol(*mergeIndex(&I, &J), SymbolID("ns::C")), "ns::C");
+ EXPECT_EQ(getSymbol(*mergeIndex(&I, &J), SymbolID("ns::D")), "");
+}
+
TEST(MergeTest, Merge) {
Symbol L, R;
L.ID = R.ID = SymbolID("hello");
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -690,6 +690,12 @@
return true;
}
+ bool getSymbol(
+ const SymbolID &ID,
+ llvm::function_ref /*Callback*/) const override {
+return false;
+ }
+
const std::vector allRequests() const { return Requests; }
private:
Index: clangd/index/Merge.cpp
===
--- clangd/index/Merge.cpp
+++ clangd/index/Merge.cpp
@@ -52,6 +52,23 @@
return More;
}
+ bool
+ getSymbol(const SymbolID &ID,
+llvm::function_ref Callback) const override {
+SymbolSlab::Builder B; // Used to store the symbol from dynamic index.
+if (!Dynamic->getSymbol(ID, [&](const Symbol &S) { B.insert(S); }))
+ return Static->getSymbol(ID, Callback);
+
+const auto *DynS = B.find(ID);
+assert(DynS != nullptr);
+if (!Static->getSymbol(ID, [&](const Symbol &S) {
+ Symbol::Details Scratch;
+ Callback(mergeSymbol(*DynS, S, &Scratch));
+}))
+ Callback(*DynS);
+return true;
+ }
+
private:
const SymbolIndex *Dynamic, *Static;
};
Index: clangd/index/MemIndex.h
===
--- clangd/index/MemIndex.h
+++ clangd/index/MemIndex.h
@@ -31,6 +31,10 @@
fuzzyFind(const FuzzyFindRequest &Req,
llvm::function_ref Callback) const override;
+ virtual bool
+ getSymbol(const SymbolID &ID,
+llvm::function_ref Callback) const override;
+
private:
std::shared_ptr> Symbols;
// Index is a set of symbols that are deduplicated by symbol IDs.
Index: clangd/index/MemIndex.cpp
===
--- clangd/index/MemIndex.cpp
+++ clangd/index/MemIndex.cpp
@@ -60,6 +60,16 @@
return More;
}
+bool MemIndex::getSymbol(
+const SymbolID &ID,
+llvm::fun
[PATCH] D43197: [OpenMP] Add flag for linking runtime bitcode library
gtbercea updated this revision to Diff 137754.
gtbercea added a comment.
Change test.
Repository:
rC Clang
https://reviews.llvm.org/D43197
Files:
include/clang/Basic/DiagnosticDriverKinds.td
lib/Driver/ToolChains/Cuda.cpp
test/Driver/Inputs/lib/libomptarget-nvptx-sm_60.bc
test/Driver/openmp-offload-gpu.c
Index: test/Driver/openmp-offload-gpu.c
===
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -142,3 +142,23 @@
// RUN: | FileCheck -check-prefix=CHK-NOLIBDEVICE %s
// CHK-NOLIBDEVICE-NOT: error:{{.*}}sm_60
+
+/// ###
+
+/// Check that the runtime bitcode library is part of the compile line. Create
a bogus
+/// bitcode library and add it to the LIBRARY_PATH.
+// RUN: env LIBRARY_PATH=%S/Inputs/lib %clang -### -fopenmp=libomp
-fopenmp-targets=nvptx64-nvidia-cuda \
+// RUN: -Xopenmp-target -march=sm_60 -fopenmp-relocatable-target -save-temps
\
+// RUN: -no-canonical-prefixes %s 2>&1 | FileCheck -check-prefix=CHK-BCLIB %s
+
+// CHK-BCLIB:
clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-cuda-bitcode{{.*}}libomptarget-nvptx-sm_60.bc
+
+/// ###
+
+/// Check that the warning is thrown when the libomptarget bitcode library is
not found.
+/// Libomptarget requires sm_35 or newer so an sm_20 bitcode library should
never exist.
+// RUN: %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
+// RUN: -Xopenmp-target -march=sm_20 -fopenmp-relocatable-target -save-temps
\
+// RUN: -no-canonical-prefixes %s 2>&1 | FileCheck
-check-prefix=CHK-BCLIB-WARN %s
+
+// CHK-BCLIB-WARN: No library 'libomptarget-nvptx-sm_20.bc' found in the
default clang lib directory or in LIBRARY_PATH. Expect degraded performance due
to no inlining of runtime functions on target devices.
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -21,6 +21,7 @@
#include "llvm/Option/ArgList.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
#include "llvm/Support/Program.h"
#include
@@ -580,6 +581,43 @@
CC1Args.push_back("-target-feature");
CC1Args.push_back("+ptx42");
}
+
+ if (DeviceOffloadingKind == Action::OFK_OpenMP) {
+SmallVector LibraryPaths;
+// Add path to lib and/or lib64 folders.
+SmallString<256> DefaultLibPath =
+ llvm::sys::path::parent_path(getDriver().Dir);
+llvm::sys::path::append(DefaultLibPath,
+Twine("lib") + CLANG_LIBDIR_SUFFIX);
+LibraryPaths.emplace_back(DriverArgs.MakeArgString(DefaultLibPath));
+
+// Add user defined library paths from LIBRARY_PATH.
+if (llvm::Optional LibPath =
+ llvm::sys::Process::GetEnv("LIBRARY_PATH")) {
+ SmallVector Frags;
+ const char EnvPathSeparatorStr[] = {llvm::sys::EnvPathSeparator, '\0'};
+ llvm::SplitString(*LibPath, Frags, EnvPathSeparatorStr);
+ for (auto Path : Frags)
+LibraryPaths.emplace_back(Path.trim());
+}
+
+std::string LibOmpTargetName =
+ "libomptarget-nvptx-" + GpuArch.str() + ".bc";
+bool FoundBCLibrary = false;
+for (const std::string &LibraryPath : LibraryPaths) {
+ SmallString<128> LibOmpTargetFile(LibraryPath);
+ llvm::sys::path::append(LibOmpTargetFile, LibOmpTargetName);
+ if (llvm::sys::fs::exists(LibOmpTargetFile)) {
+CC1Args.push_back("-mlink-cuda-bitcode");
+CC1Args.push_back(DriverArgs.MakeArgString(LibOmpTargetFile));
+FoundBCLibrary = true;
+break;
+ }
+}
+if (!FoundBCLibrary)
+ getDriver().Diag(diag::warn_drv_omp_offload_target_missingbcruntime)
+ << LibOmpTargetName;
+ }
}
void CudaToolChain::AddCudaIncludeArgs(const ArgList &DriverArgs,
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -203,6 +203,9 @@
def warn_drv_omp_offload_target_duplicate : Warning<
"The OpenMP offloading target '%0' is similar to target '%1' already
specified - will be ignored.">,
InGroup;
+def warn_drv_omp_offload_target_missingbcruntime : Warning<
+ "No library '%0' found in the default clang lib directory or in
LIBRARY_PATH. Expect degraded performance due to no inlining of runtime
functions on target devices.">,
+ InGroup;
def err_drv_bitcode_unsupported_on_toolchain : Error<
"-fembed-bitcode is not supported on versions of iOS prior to 6.0">;
Index: test/Driver/openmp-offload-gpu.c
===
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -142,3 +14
[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 and add it to the LIBRARY_PATH. +// RUN: touch %T/libomptarget-nvptx-sm_60.bc +// RUN: env LIBRARY_PATH=%T %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \ grokos wrote: > ABataev wrote: > > Create empty `libomptarget-nvptx-sm_60.bc` in `Driver/lib` directory and > > use it in the test rather create|delete it dynamically. > I'm also in favour of this approach. On some systems /tmp is not accessible > and the regression test fails. This test doesn't (and shouldn't!) use `/tmp`. The build directory and `%T` are always writable (if not, you have different issues on your system). Btw you need to pay attention that the driver now finds files next to the compiler directory. You may want to make sure that the test always passes / doesn't fail for wrong reasons. 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] D43197: [OpenMP] Add flag for linking runtime bitcode library
gtbercea added inline comments.
Comment at: lib/Driver/ToolChains/Cuda.cpp:592
+Twine("lib") + CLANG_LIBDIR_SUFFIX);
+LibraryPaths.emplace_back(DefaultLibPath.c_str());
+
ABataev wrote:
> Do you still need `.c_str()` here?
Doesn't compile without it but we can get there using Args.MakeArgString() also.
Comment at: test/Driver/openmp-offload-gpu.c:150
+/// bitcode library and add it to the LIBRARY_PATH.
+// RUN: touch %T/libomptarget-nvptx-sm_60.bc
+// RUN: env LIBRARY_PATH=%T %clang -### -fopenmp=libomp
-fopenmp-targets=nvptx64-nvidia-cuda \
Hahnfeld wrote:
> grokos wrote:
> > ABataev wrote:
> > > Create empty `libomptarget-nvptx-sm_60.bc` in `Driver/lib` directory and
> > > use it in the test rather create|delete it dynamically.
> > I'm also in favour of this approach. On some systems /tmp is not accessible
> > and the regression test fails.
> This test doesn't (and shouldn't!) use `/tmp`. The build directory and `%T`
> are always writable (if not, you have different issues on your system).
>
> Btw you need to pay attention that the driver now finds files next to the
> compiler directory. You may want to make sure that the test always passes /
> doesn't fail for wrong reasons.
Just added this.
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] D43197: [OpenMP] Add flag for linking runtime bitcode library
gtbercea updated this revision to Diff 137755.
gtbercea added a comment.
Revert to c_str().
Repository:
rC Clang
https://reviews.llvm.org/D43197
Files:
include/clang/Basic/DiagnosticDriverKinds.td
lib/Driver/ToolChains/Cuda.cpp
test/Driver/Inputs/lib/libomptarget-nvptx-sm_60.bc
test/Driver/openmp-offload-gpu.c
Index: test/Driver/openmp-offload-gpu.c
===
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -142,3 +142,23 @@
// RUN: | FileCheck -check-prefix=CHK-NOLIBDEVICE %s
// CHK-NOLIBDEVICE-NOT: error:{{.*}}sm_60
+
+/// ###
+
+/// Check that the runtime bitcode library is part of the compile line. Create
a bogus
+/// bitcode library and add it to the LIBRARY_PATH.
+// RUN: env LIBRARY_PATH=%S/Inputs/lib %clang -### -fopenmp=libomp
-fopenmp-targets=nvptx64-nvidia-cuda \
+// RUN: -Xopenmp-target -march=sm_60 -fopenmp-relocatable-target -save-temps
\
+// RUN: -no-canonical-prefixes %s 2>&1 | FileCheck -check-prefix=CHK-BCLIB %s
+
+// CHK-BCLIB:
clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-cuda-bitcode{{.*}}libomptarget-nvptx-sm_60.bc
+
+/// ###
+
+/// Check that the warning is thrown when the libomptarget bitcode library is
not found.
+/// Libomptarget requires sm_35 or newer so an sm_20 bitcode library should
never exist.
+// RUN: %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
+// RUN: -Xopenmp-target -march=sm_20 -fopenmp-relocatable-target -save-temps
\
+// RUN: -no-canonical-prefixes %s 2>&1 | FileCheck
-check-prefix=CHK-BCLIB-WARN %s
+
+// CHK-BCLIB-WARN: No library 'libomptarget-nvptx-sm_20.bc' found in the
default clang lib directory or in LIBRARY_PATH. Expect degraded performance due
to no inlining of runtime functions on target devices.
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -21,6 +21,7 @@
#include "llvm/Option/ArgList.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
#include "llvm/Support/Program.h"
#include
@@ -580,6 +581,43 @@
CC1Args.push_back("-target-feature");
CC1Args.push_back("+ptx42");
}
+
+ if (DeviceOffloadingKind == Action::OFK_OpenMP) {
+SmallVector LibraryPaths;
+// Add path to lib and/or lib64 folders.
+SmallString<256> DefaultLibPath =
+ llvm::sys::path::parent_path(getDriver().Dir);
+llvm::sys::path::append(DefaultLibPath,
+Twine("lib") + CLANG_LIBDIR_SUFFIX);
+LibraryPaths.emplace_back(DefaultLibPath.c_str());
+
+// Add user defined library paths from LIBRARY_PATH.
+if (llvm::Optional LibPath =
+ llvm::sys::Process::GetEnv("LIBRARY_PATH")) {
+ SmallVector Frags;
+ const char EnvPathSeparatorStr[] = {llvm::sys::EnvPathSeparator, '\0'};
+ llvm::SplitString(*LibPath, Frags, EnvPathSeparatorStr);
+ for (auto Path : Frags)
+LibraryPaths.emplace_back(Path.trim());
+}
+
+std::string LibOmpTargetName =
+ "libomptarget-nvptx-" + GpuArch.str() + ".bc";
+bool FoundBCLibrary = false;
+for (const std::string &LibraryPath : LibraryPaths) {
+ SmallString<128> LibOmpTargetFile(LibraryPath);
+ llvm::sys::path::append(LibOmpTargetFile, LibOmpTargetName);
+ if (llvm::sys::fs::exists(LibOmpTargetFile)) {
+CC1Args.push_back("-mlink-cuda-bitcode");
+CC1Args.push_back(DriverArgs.MakeArgString(LibOmpTargetFile));
+FoundBCLibrary = true;
+break;
+ }
+}
+if (!FoundBCLibrary)
+ getDriver().Diag(diag::warn_drv_omp_offload_target_missingbcruntime)
+ << LibOmpTargetName;
+ }
}
void CudaToolChain::AddCudaIncludeArgs(const ArgList &DriverArgs,
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -203,6 +203,9 @@
def warn_drv_omp_offload_target_duplicate : Warning<
"The OpenMP offloading target '%0' is similar to target '%1' already
specified - will be ignored.">,
InGroup;
+def warn_drv_omp_offload_target_missingbcruntime : Warning<
+ "No library '%0' found in the default clang lib directory or in
LIBRARY_PATH. Expect degraded performance due to no inlining of runtime
functions on target devices.">,
+ InGroup;
def err_drv_bitcode_unsupported_on_toolchain : Error<
"-fembed-bitcode is not supported on versions of iOS prior to 6.0">;
Index: test/Driver/openmp-offload-gpu.c
===
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -142,3 +142,23 @@
//
[PATCH] D43197: [OpenMP] Add flag for linking runtime bitcode library
gtbercea added inline comments. Comment at: test/Driver/openmp-offload-gpu.c:150 +/// bitcode library and add it to the LIBRARY_PATH. +// RUN: touch %T/libomptarget-nvptx-sm_60.bc +// RUN: env LIBRARY_PATH=%T %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \ gtbercea wrote: > Hahnfeld wrote: > > grokos wrote: > > > ABataev wrote: > > > > Create empty `libomptarget-nvptx-sm_60.bc` in `Driver/lib` directory > > > > and use it in the test rather create|delete it dynamically. > > > I'm also in favour of this approach. On some systems /tmp is not > > > accessible and the regression test fails. > > This test doesn't (and shouldn't!) use `/tmp`. The build directory and `%T` > > are always writable (if not, you have different issues on your system). > > > > Btw you need to pay attention that the driver now finds files next to the > > compiler directory. You may want to make sure that the test always passes / > > doesn't fail for wrong reasons. > Just added this. @Hahnfeld I've used %S instead. The only way in which the test can be a false positive is when the lib folder contains this .bc file. But there's no way to stop this from happening since we check DefaultLibPath first. 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] D44231: [clang-tidy] Check for sizeof that call functions
pfultz2 added a comment. > Again, that only works for C++ and not C. Typedef has always worked in C. > Did it report any true positives that would need correcting? Not for LLVM, but it has in other projects like I mentioned. > Can you check some other large repos (both C++ and C), such as: Qt, cocos2d, > rethinkdb, redis, and php-src? None of those projects use cmake, so it doesn't look easy to run clang-tidy on them. Do you have a script that will run clang-tidy on these repos? Actually, PVS-Studio has a more general check for `sizeof(expr)`: https://www.viva64.com/en/examples/v568/ It shows an example of FCEUX doing `sizeof(buf - 1)`, Asterisk doing `sizeof(data[0] * 2)` and ReactOS doing `sizeof(UnknownError [0] - 20)`. I think it might be a good idea to expand this from just a function call to any integer expression. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44272: [clangd] Support incremental document syncing
simark marked 2 inline comments as done.
simark added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:101
json::obj{
-{"textDocumentSync", 1},
+{"textDocumentSync", 2},
{"documentFormattingProvider", true},
ilya-biryukov wrote:
> A comment mentioning that 2 means incremental document sync from LSP would be
> useful here.
I opted to add a TextDocumentSyncKind enum to Protocol.h and use that. Though
instead of an enum, I did a class with static const fields:
```
struct TextDocumentSyncKind {
/// Documents should not be synced at all.
static const int None = 0;
/// Documents are synced by always sending the full content of the document.
static const int Full = 1;
/// Documents are synced by sending the full content on open. After that
/// only incremental updates to the document are send.
static const int Incremental = 2;
};
```
otherwise it would've required an (int) cast when using it to generate JSON:
{"textDocumentSync", TextDocumentSyncKind::Incremental},
Comment at: clangd/ClangdLSPServer.cpp:149
void ClangdLSPServer::onDocumentDidChange(DidChangeTextDocumentParams &Params)
{
if (Params.contentChanges.size() != 1)
return replyError(ErrorCode::InvalidParams,
ilya-biryukov wrote:
> We should handle more than a single change here.
Ok, I'll try that. I am not sure I interpret the spec correctly. If you have
two changes, the second change applies to the state of the document after
having applied the first change, is that right? If that's the case, I think we
only need to iterate on the changes and call addDocument on them sequentially
(which could be done in the DraftMgr, given the interface change to DraftStore
you suggest lower).
Comment at: clangd/ClangdServer.cpp:557
// 64-bit unsigned integer.
if (Version < LastReportedDiagsVersion)
return;
ilya-biryukov wrote:
> When you'll try remove the `DraftMgr`, this piece of code will be hard to
> refactor because it relies on `DocVersion`.
> This is a workaround for a possible race condition that should really be
> handled by TUScheduler rather than this code, but we haven't got around to
> fixing it yet. (It's on the list, though, would probably get in next week).
>
> A proper workaround for now would be to add `llvm::StringMap
> InternalVersions` field to `ClangdServer`, increment those versions on each
> call to `addDocument` and use them here in the same way we currently use
> DraftMgr's versions.
Hmm ok, it will probably make sense when I try to do it. The
`InternalVersions` map maps URIs to the latest version seen?
Is the race condition because we could get diagnostics for a stale version of
the document, so we don't want to emit them?
Comment at: clangd/ClangdServer.h:159
/// constructor will receive onDiagnosticsReady callback.
void addDocument(PathRef File, StringRef Contents,
+ WantDiagnostics WantDiags = WantDiagnostics::Auto,
ilya-biryukov wrote:
> I don't think we should do this on `ClangdServer` level. We will have to copy
> the whole file anyway before passing it to clang, so there are no performance
> wins here and it complicates the interface.
> I suggest we update the document text from diffs on `ClangdLSPServer` level
> and continue passing the whole document to `ClangdServer`.
> It would mean `DraftMgr` will need to move to `ClangdLSPServer`, but it's
> fine.
>
`ClangdServer` also needs `DraftMgr` for `forceReparse` and
`reparseOpenedFiles`. I guess that too would move to `ClangdLSPServer`? I'm
just not sure how to adapt the unit tests, since we don't have unittests using
`ClangdLSPServer` (yet).
Comment at: clangd/DraftStore.cpp:47
+DocVersion DraftStore::updateDraft(PathRef File, StringRef Contents,
+ llvm::Optional range,
+ std::string *NewContents) {
ilya-biryukov wrote:
> NIT: LLVM uses `UpperCamelCase` for parameters and local variables
Woops. I should learn to use clang-tidy. It found other instances (the local
variables) but it doesn't find the parameters not written in camel case. Do
you have an idea why? I dumped the config and see these:
```
- key: readability-identifier-naming.ClassCase
value: CamelCase
- key: readability-identifier-naming.EnumCase
value: CamelCase
- key: readability-identifier-naming.UnionCase
value: CamelCase
- key: readability-identifier-naming.VariableCase
value: CamelCase
```
I assume there must be a `ParamCase` or something like that, but I can't find
the exhaustive list of parameters for that check.
Comment at: clangd/DraftStore.cpp:66
+
+newContents = Entry.Draft-
RE: [llvm] r325653 - [mips] Spectre variant two mitigation for MIPSR2
Apoloiges all, I spoke too soon. Some tests fail due to mir changes, and one
which a slight different version of the memset intrinsic.
Attached is a patch which resolves the failures.
Thanks,
Simon
From: llvm-commits [llvm-commits-boun...@lists.llvm.org] on behalf of Simon
Dardis via llvm-commits [llvm-comm...@lists.llvm.org]
Sent: Friday, March 9, 2018 3:05 PM
To: llvm-comm...@lists.llvm.org; cfe-commits@lists.llvm.org
Subject: RE: [llvm] r325653 - [mips] Spectre variant two mitigation for MIPSR2
Hi,
Any downstream users should be able to apply this, the corresponding clang
patch (r325651) and the LLD patch (r325657, and r325713) cleanly to the
release sources of LLVM 6.0, should they require it.
Thanks,
Simon
Author: sdardis
Date: Tue Feb 20 16:06:53 2018
New Revision: 325653
URL: http://llvm.org/viewvc/llvm-project?rev=325653&view=rev
Log:
[mips] Spectre variant two mitigation for MIPSR2
This patch provides mitigation for CVE-2017-5715, Spectre variant two,
which affects the P5600 and P6600. It implements the LLVM part of
-mindirect-jump=hazard. It is _not_ enabled by default for the P5600.
The migitation strategy suggested by MIPS for these processors is to use
hazard barrier instructions. 'jalr.hb' and 'jr.hb' are hazard
barrier variants of the 'jalr' and 'jr' instructions respectively.
These instructions impede the execution of instruction stream until
architecturally defined hazards (changes to the instruction stream,
privileged registers which may affect execution) are cleared. These
instructions in MIPS' designs are not speculated past.
These instructions are used with the attribute +use-indirect-jump-hazard
when branching indirectly and for indirect function calls.
These instructions are defined by the MIPS32R2 ISA, so this mitigation
method is not compatible with processors which implement an earlier
revision of the MIPS ISA.
Performance benchmarking of this option with -fpic and lld using
-z hazardplt shows a difference of overall 10%~ time increase
for the LLVM testsuite. Certain benchmarks such as methcall show a
substantially larger increase in time due to their nature.
Reviewers: atanasyan, zoran.jovanovic
Differential Revision: https://reviews.llvm.org/D43486
Added:
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/calls.ll
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/guards-verify-call.mir
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/guards-verify-tailcall.mir
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/jumptables.ll
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/long-branch.ll
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/long-calls.ll
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/unsupported-micromips.ll
llvm/trunk/test/CodeGen/Mips/indirect-jump-hazard/unsupported-mips32.ll
Modified:
llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
llvm/trunk/lib/Target/Mips/MicroMips32r6InstrInfo.td
llvm/trunk/lib/Target/Mips/MicroMipsInstrInfo.td
llvm/trunk/lib/Target/Mips/Mips.td
llvm/trunk/lib/Target/Mips/Mips32r6InstrInfo.td
llvm/trunk/lib/Target/Mips/Mips64InstrInfo.td
llvm/trunk/lib/Target/Mips/Mips64r6InstrInfo.td
llvm/trunk/lib/Target/Mips/MipsDSPInstrFormats.td
llvm/trunk/lib/Target/Mips/MipsInstrFormats.td
llvm/trunk/lib/Target/Mips/MipsInstrInfo.cpp
llvm/trunk/lib/Target/Mips/MipsInstrInfo.td
llvm/trunk/lib/Target/Mips/MipsLongBranch.cpp
llvm/trunk/lib/Target/Mips/MipsSubtarget.cpp
llvm/trunk/lib/Target/Mips/MipsSubtarget.h
Modified: llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
URL:
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp?rev=325653&r1=325652&r2=325653&view=diff
==
--- llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp (original)
+++ llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp Tue Feb 20 16:06:53
2018
@@ -5136,6 +5136,7 @@ unsigned MipsAsmParser::checkTargetMatch
// It also applies for registers Rt and Rs of microMIPSr6 jalrc.hb
instruction
// and registers Rd and Base for microMIPS lwp instruction
case Mips::JALR_HB:
+ case Mips::JALR_HB64:
case Mips::JALRC_HB_MMR6:
case Mips::JALRC_MMR6:
if (Inst.getOperand(0).getReg() == Inst.getOperand(1).getReg())
Modified: llvm/trunk/lib/Target/Mips/MicroMips32r6InstrInfo.td
URL:
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MicroMips32r6InstrInfo.td?rev=325653&r1=325652&r2=325653&view=diff
==
--- llvm/trunk/lib/Target/Mips/MicroMips32r6InstrInfo.td (original)
+++ llvm/trunk/lib/Target/Mips/MicroMips32r6InstrInfo.td Tue Feb 20 16:06:53
2018
@@ -1713,6 +1713,12 @@ let AddedComplexity = 41 in {
def TAILCALL_MMR6 : TailCall, ISA_MICROMIPS32R6;
+def TAIL
[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 and add it to the LIBRARY_PATH. +// RUN: touch %T/libomptarget-nvptx-sm_60.bc +// RUN: env LIBRARY_PATH=%T %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \ gtbercea wrote: > gtbercea wrote: > > Hahnfeld wrote: > > > grokos wrote: > > > > ABataev wrote: > > > > > Create empty `libomptarget-nvptx-sm_60.bc` in `Driver/lib` directory > > > > > and use it in the test rather create|delete it dynamically. > > > > I'm also in favour of this approach. On some systems /tmp is not > > > > accessible and the regression test fails. > > > This test doesn't (and shouldn't!) use `/tmp`. The build directory and > > > `%T` are always writable (if not, you have different issues on your > > > system). > > > > > > Btw you need to pay attention that the driver now finds files next to the > > > compiler directory. You may want to make sure that the test always passes > > > / doesn't fail for wrong reasons. > > Just added this. > @Hahnfeld I've used %S instead. > > The only way in which the test can be a false positive is when the lib folder > contains this .bc file. But there's no way to stop this from happening since > we check DefaultLibPath first. (My comment was related to @grokos, the usage of `%T` and temporarily creating the bc lib. The current approach with `%S/Inputs` is much cleaner, but you need to create a subdirectory as everbody else did.) Then you need to find a way to stop this. There already are some flags to change the sysroot etc., but I don't know if the influence what you use in this patch. In the worst case, you need to add a new flag to disable `DefaultLibPath` and use it in the tests. You can't propose to commit a test that is known to break (although I acknowledge that `libomptarget-nvptx-sm_20.bc` will probably never exist). 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] D44272: [clangd] Support incremental document syncing
simark marked 2 inline comments as done. simark added inline comments. Comment at: clangd/ClangdServer.h:159 /// constructor will receive onDiagnosticsReady callback. void addDocument(PathRef File, StringRef Contents, + WantDiagnostics WantDiags = WantDiagnostics::Auto, simark wrote: > ilya-biryukov wrote: > > I don't think we should do this on `ClangdServer` level. We will have to > > copy the whole file anyway before passing it to clang, so there are no > > performance wins here and it complicates the interface. > > I suggest we update the document text from diffs on `ClangdLSPServer` level > > and continue passing the whole document to `ClangdServer`. > > It would mean `DraftMgr` will need to move to `ClangdLSPServer`, but it's > > fine. > > > `ClangdServer` also needs `DraftMgr` for `forceReparse` and > `reparseOpenedFiles`. I guess that too would move to `ClangdLSPServer`? I'm > just not sure how to adapt the unit tests, since we don't have unittests > using `ClangdLSPServer` (yet). Actually, `forceReparse` doesn't really need `DraftMgr` (it's only used for the assert), only `reparseOpenedFiles` needs it. So in the test, we can manually call `forceReparse` on all the files by hand... this just means that `reparseOpenedFiles` won't be tested anymore. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44272 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43764: [clang-apply-replacements] Convert tooling::Replacements to tooling::AtomicChange for conflict resolving of changes, code cleanup, and code formatting.
ioeric added inline comments.
Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:188
-bool mergeAndDeduplicate(const TUReplacements &TUs,
- FileToReplacementsMap &GroupedReplacements,
- clang::SourceManager &SM) {
-
- // Group all replacements by target file.
+static llvm::DenseMap>
+groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs,
Add a brief comment describe what this function does, specifically about `TUs`
and `TUDs`.
Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:190
+groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs,
+ FileToChangesMap &FileChanges, clang::SourceManager &SM) {
std::set Warned;
nit: `SM` can be `const`. And put `SM` (input arg) before `FileChanges` (output
arg).
Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:213-221
// Use the file manager to deduplicate paths. FileEntries are
// automatically canonicalized.
- if (const FileEntry *Entry =
SM.getFileManager().getFile(R.getFilePath())) {
+ if (const FileEntry *Entry =
+ SM.getFileManager().getFile(R.getFilePath())) {
GroupedReplacements[Entry].push_back(R);
} else if (Warned.insert(R.getFilePath()).second) {
+errs() << "Described file '" << R.getFilePath()
This code block is the same as the one in the above loop. Consider pulling it
into a lambda.
Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:276-279
+if (R.getLength() == 0)
+ addInsertion(R);
+else
+ addReplacement(R);
`AtomicChange::replace` also handles insertions, so I think there is no need
for the distinction here.
(See my comment in where `ReplacementsToAtomicChanges` is used; I think the
class might not be needed.)
Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:290
+std::vector Changes;
+for (const auto &R : AllChanges.getReplacements()) {
+ tooling::AtomicChange Change(Entry->getName(), Entry->getName());
I might be missing something, but why do we need to pull replacements from the
result change into a set of changes?
Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:317
+if (llvm::Error Error =
+AllChanges.insert(*SM, BeginLoc.getLocWithOffset(R.getOffset()),
+ R.getReplacementText())) {
By default, this inserts R after the existing insertion in the same location
(if there is any). But it's impossible for apply-all-replacements to know in
which order they should be applied, so I would suggest treating this as
conflict. Sorry that I might have confused you by asking to add a test case
where two identical insertions are both applied (because they are order
*independent*), but `AtomicChange::replace` also supports that and would report
error when two insertions are order *dependent*.
`AtomicChange::replace` and `AtomicChange::insert` have almost the same
semantic except that you could specify the expected order of insertions on the
same location with `insert`, but in our case, we wouldn't know which order is
desired.
Comment at: clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp:353
+const FileEntry *Entry = FileAndReplacements.first;
+ReplacementsToAtomicChanges DeduplicatedChanges(SM, Entry);
+for (const auto &R : FileAndReplacements.second)
Sorry that I didn't make myself clear... what you had in the previous revision
was correct (for the current use case of apply-all-replacements) i.e. store all
replacements in one `AtomicChange`, which allows you to detect conflicts on the
fly. So the code here can be simplified as:
```
...
Entry = ...;
AtomicChange FileChange;
for (const auto &R : FileAndReplacements.second) {
auto Err = FileChange.replace( );
if (Err)
reportConflict(Entry, std::move(Err)); // reportConflict as we go
}
FileChanges.emplace(Entry, {FileChange});
...
```
I think with this set up, you wouldn't need `ReplacementsToAtomicChanges`,
`ConflictError` and `reportConflicts`.
Comment at: test/clang-apply-replacements/Inputs/basic/file2.yaml:1
---
MainSourceFile: source2.cpp
Could you please add two test cases for insertions: 1) identical insertions
(e.g. "AB" and "AB") at the same location are applied sucessfully and 2)
order-dependent insertions (e.g. "AB" and "BA") are detected.
(It might make sense to do this in a different file)
Comment at: test/clang-apply-replacements/Inputs/conflict/expected.txt:8
+ Replace 9:5-9:11 with "elem"
The following changes conflict:
Remove 12:3-12:14
---
[PATCH] D43197: [OpenMP] Add flag for linking runtime bitcode library
gtbercea updated this revision to Diff 137769.
gtbercea added a comment.
Fix test.
Repository:
rC Clang
https://reviews.llvm.org/D43197
Files:
include/clang/Basic/DiagnosticDriverKinds.td
lib/Driver/ToolChains/Cuda.cpp
test/Driver/Inputs/lib/libomptarget-nvptx-sm_20.bc
test/Driver/openmp-offload-gpu.c
Index: test/Driver/openmp-offload-gpu.c
===
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -142,3 +142,23 @@
// RUN: | FileCheck -check-prefix=CHK-NOLIBDEVICE %s
// CHK-NOLIBDEVICE-NOT: error:{{.*}}sm_60
+
+/// ###
+
+/// Check that the runtime bitcode library is part of the compile line. Create
a bogus
+/// bitcode library and add it to the LIBRARY_PATH.
+// RUN: env LIBRARY_PATH=%S/Inputs/lib %clang -### -fopenmp=libomp
-fopenmp-targets=nvptx64-nvidia-cuda \
+// RUN: -Xopenmp-target -march=sm_20 -fopenmp-relocatable-target -save-temps
\
+// RUN: -no-canonical-prefixes %s 2>&1 | FileCheck -check-prefix=CHK-BCLIB %s
+
+// CHK-BCLIB:
clang{{.*}}-triple{{.*}}nvptx64-nvidia-cuda{{.*}}-mlink-cuda-bitcode{{.*}}libomptarget-nvptx-sm_20.bc
+
+/// ###
+
+/// Check that the warning is thrown when the libomptarget bitcode library is
not found.
+/// Libomptarget requires sm_35 or newer so an sm_20 bitcode library should
never exist.
+// RUN: %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
+// RUN: -Xopenmp-target -march=sm_20 -fopenmp-relocatable-target -save-temps
\
+// RUN: -no-canonical-prefixes %s 2>&1 | FileCheck
-check-prefix=CHK-BCLIB-WARN %s
+
+// CHK-BCLIB-WARN: No library 'libomptarget-nvptx-sm_20.bc' found in the
default clang lib directory or in LIBRARY_PATH. Expect degraded performance due
to no inlining of runtime functions on target devices.
Index: lib/Driver/ToolChains/Cuda.cpp
===
--- lib/Driver/ToolChains/Cuda.cpp
+++ lib/Driver/ToolChains/Cuda.cpp
@@ -21,6 +21,7 @@
#include "llvm/Option/ArgList.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/Process.h"
#include "llvm/Support/Program.h"
#include
@@ -580,6 +581,43 @@
CC1Args.push_back("-target-feature");
CC1Args.push_back("+ptx42");
}
+
+ if (DeviceOffloadingKind == Action::OFK_OpenMP) {
+SmallVector LibraryPaths;
+// Add path to lib and/or lib64 folders.
+SmallString<256> DefaultLibPath =
+ llvm::sys::path::parent_path(getDriver().Dir);
+llvm::sys::path::append(DefaultLibPath,
+Twine("lib") + CLANG_LIBDIR_SUFFIX);
+LibraryPaths.emplace_back(DefaultLibPath.c_str());
+
+// Add user defined library paths from LIBRARY_PATH.
+if (llvm::Optional LibPath =
+ llvm::sys::Process::GetEnv("LIBRARY_PATH")) {
+ SmallVector Frags;
+ const char EnvPathSeparatorStr[] = {llvm::sys::EnvPathSeparator, '\0'};
+ llvm::SplitString(*LibPath, Frags, EnvPathSeparatorStr);
+ for (auto Path : Frags)
+LibraryPaths.emplace_back(Path.trim());
+}
+
+std::string LibOmpTargetName =
+ "libomptarget-nvptx-" + GpuArch.str() + ".bc";
+bool FoundBCLibrary = false;
+for (const std::string &LibraryPath : LibraryPaths) {
+ SmallString<128> LibOmpTargetFile(LibraryPath);
+ llvm::sys::path::append(LibOmpTargetFile, LibOmpTargetName);
+ if (llvm::sys::fs::exists(LibOmpTargetFile)) {
+CC1Args.push_back("-mlink-cuda-bitcode");
+CC1Args.push_back(DriverArgs.MakeArgString(LibOmpTargetFile));
+FoundBCLibrary = true;
+break;
+ }
+}
+if (!FoundBCLibrary)
+ getDriver().Diag(diag::warn_drv_omp_offload_target_missingbcruntime)
+ << LibOmpTargetName;
+ }
}
void CudaToolChain::AddCudaIncludeArgs(const ArgList &DriverArgs,
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -203,6 +203,9 @@
def warn_drv_omp_offload_target_duplicate : Warning<
"The OpenMP offloading target '%0' is similar to target '%1' already
specified - will be ignored.">,
InGroup;
+def warn_drv_omp_offload_target_missingbcruntime : Warning<
+ "No library '%0' found in the default clang lib directory or in
LIBRARY_PATH. Expect degraded performance due to no inlining of runtime
functions on target devices.">,
+ InGroup;
def err_drv_bitcode_unsupported_on_toolchain : Error<
"-fembed-bitcode is not supported on versions of iOS prior to 6.0">;
Index: test/Driver/openmp-offload-gpu.c
===
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -142,3 +142,23 @@
// RUN: |
[PATCH] D43197: [OpenMP] Add flag for linking runtime bitcode library
gtbercea added inline comments. Comment at: test/Driver/openmp-offload-gpu.c:150 +/// bitcode library and add it to the LIBRARY_PATH. +// RUN: touch %T/libomptarget-nvptx-sm_60.bc +// RUN: env LIBRARY_PATH=%T %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \ Hahnfeld wrote: > gtbercea wrote: > > gtbercea wrote: > > > Hahnfeld wrote: > > > > grokos wrote: > > > > > ABataev wrote: > > > > > > Create empty `libomptarget-nvptx-sm_60.bc` in `Driver/lib` > > > > > > directory and use it in the test rather create|delete it > > > > > > dynamically. > > > > > I'm also in favour of this approach. On some systems /tmp is not > > > > > accessible and the regression test fails. > > > > This test doesn't (and shouldn't!) use `/tmp`. The build directory and > > > > `%T` are always writable (if not, you have different issues on your > > > > system). > > > > > > > > Btw you need to pay attention that the driver now finds files next to > > > > the compiler directory. You may want to make sure that the test always > > > > passes / doesn't fail for wrong reasons. > > > Just added this. > > @Hahnfeld I've used %S instead. > > > > The only way in which the test can be a false positive is when the lib > > folder contains this .bc file. But there's no way to stop this from > > happening since we check DefaultLibPath first. > (My comment was related to @grokos, the usage of `%T` and temporarily > creating the bc lib. The current approach with `%S/Inputs` is much cleaner, > but you need to create a subdirectory as everbody else did.) > > Then you need to find a way to stop this. There already are some flags to > change the sysroot etc., but I don't know if the influence what you use in > this patch. In the worst case, you need to add a new flag to disable > `DefaultLibPath` and use it in the tests. You can't propose to commit a test > that is known to break (although I acknowledge that > `libomptarget-nvptx-sm_20.bc` will probably never exist). I created a lib folder where the empty .bc is present: %S/Inputs/lib Good point. sm_20.bc cannot be created since libomptarget requires sm_30 at least which means that there can never be an sm_20 in the DefaultLibPath folder so the only way to find it is to follow LIBRARY_PATH. This resolves the issue. 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
Re: [PATCH] D44103: Adding additional UNSUPPORTED platform in libcxx/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp
Eric, Any chance you could have one more look at this and let me know if the patch is acceptable? Thanks, Mike On Mon, Mar 5, 2018 at 4:16 PM, Mike Edwards via Phabricator < revi...@reviews.llvm.org> wrote: > sqlbyme updated this revision to Diff 137104. > sqlbyme added a comment. > > I copied what Eric did here: https://github.com/llvm-mirror/libcxx/commit/ > 6878e852d1d26cca0abee3013822311cd894ca3e. I have tested this on our bots > and the fix works fine. > > > https://reviews.llvm.org/D44103 > > Files: > test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp > > > Index: test/std/strings/basic.string/string.cons/iter_alloc_ > deduction.fail.cpp > === > --- test/std/strings/basic.string/string.cons/iter_alloc_ > deduction.fail.cpp > +++ test/std/strings/basic.string/string.cons/iter_alloc_ > deduction.fail.cpp > @@ -9,8 +9,7 @@ > > // > // UNSUPPORTED: c++98, c++03, c++11, c++14 > -// UNSUPPORTED: clang-3.3, clang-3.4, clang-3.5, clang-3.6, clang-3.7, > clang-3.8, clang-3.9, clang-4.0 > -// UNSUPPORTED: apple-clang-6, apple-clang-7, apple-clang-8.0 > +// XFAIL: libcpp-no-deduction-guides > > // template // class Allocator = allocator iterator_traits::value_type>> > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44231: [clang-tidy] Check for sizeof that call functions
aaron.ballman added a comment. In https://reviews.llvm.org/D44231#1032782, @pfultz2 wrote: > > Again, that only works for C++ and not C. > > Typedef has always worked in C. This is true. I think what I'm struggling to say is: I don't think this is a common pattern in either C or C++. It's also unnecessary because you can avoid repeating the type by just using `sizeof` on the function call result. >> Did it report any true positives that would need correcting? > > Not for LLVM, but it has in other projects like I mentioned. > >> Can you check some other large repos (both C++ and C), such as: Qt, cocos2d, >> rethinkdb, redis, and php-src? > > None of those projects use cmake, so it doesn't look easy to run clang-tidy > on them. Do you have a script that will run clang-tidy on these repos? I don't have a script for it. I've used "bear" with at least some of those projects because they use makefiles rather than cmake (https://github.com/rizsotto/Bear). I'm not tied to those projects specifically, either, so if you have a different corpus you'd prefer to use, that's fine. The important bit is testing it across a considerable amount of C code and C++ code to see whether this diagnostic is too chatty or not. > Actually, PVS-Studio has a more general check for `sizeof(expr)`: > > https://www.viva64.com/en/examples/v568/ > > It shows an example of FCEUX doing `sizeof(buf - 1)`, Asterisk doing > `sizeof(data[0] * 2)` and ReactOS doing `sizeof(UnknownError [0] - 20)`. I > think it might be a good idea to expand this from just a function call to any > integer expression. That won't catch many (most?) of the issues demonstrated by PVS-Studio; the rule their check follows are to warn on side-effecting operations (which Clang already does with -Wunevaluated-expression) and arithmetic expressions in sizeof. The latter might be a reasonable extension to the check -- I have less concerns about the false positive rate with that than I do with the function call case. Another possible extension to consider is `sizeof(sizeof(foo))`, which it seems PVS-Studio will diagnose as well. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44295: Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods
malcolm.parsons added inline comments. Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:152 +Member->getQualifierLoc().getSourceRange(), +GetNameAsString(*(Parents.front())) + "::"); + } What does this do for templated parent classes? Comment at: docs/clang-tidy/checks/bugprone-parent-virtual-call.rst:6 + +FIXME: Describe what patterns does the check detect and why. Give examples. FIXME. Comment at: test/clang-tidy/bugprone-parent-virtual-call.cpp:102 + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'AA::virt_1' is a grand-parent's method, not parent's. Did you mean 'BB_1' or 'BB_2'? [bugprone-parent-virtual-call] + // No fix available due to multiple choise of parent class. +}; typo: choice Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44295 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44093: [BUILTINS] structure pretty printer
paulsemel added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:1252 + Types[getContext().getPointerType(getContext().CharTy)] = "%s"; + GENERATE_TYPE_QUALIFIERS_NO_RESTRICT(getContext().CharTy, "%s") +} aaron.ballman wrote: > paulsemel wrote: > > aaron.ballman wrote: > > > What about other types that have format specifiers (ptrdiff_t, size_t, > > > intptr_t, char16_t, etc)? > > So, for typedef, why not apply the `QualType::getCanonicalType` to our > > field type, so that we try to get rid of those intermediate typedefs ? > It should be possible to do for type aliases, because you know the canonical > type information. However, that won't work for builtin types that aren't a > typedef like `char16_t`. Sure, but in this case, the only soluntion is to determine how we want to print those builtins and add those and the static map Repository: rC Clang https://reviews.llvm.org/D44093 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44093: [BUILTINS] structure pretty printer
paulsemel updated this revision to Diff 137775.
paulsemel added a comment.
Added recursive type pretty-printing as suggested by Aaron.
Repository:
rC Clang
https://reviews.llvm.org/D44093
Files:
include/clang/Basic/Builtins.def
lib/CodeGen/CGBuiltin.cpp
lib/Sema/SemaChecking.cpp
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -1110,6 +1110,66 @@
// so ensure that they are declared.
DeclareGlobalNewDelete();
break;
+ case Builtin::BI__builtin_dump_struct: {
+// We first want to ensure we are called with 2 arguments
+if (checkArgCount(*this, TheCall, 2))
+ return ExprError();
+// Ensure that the first argument is of type 'struct XX *'
+const Expr *PtrArg = TheCall->getArg(0)->IgnoreParenImpCasts();
+const QualType PtrArgType = PtrArg->getType();
+if (!PtrArgType->isPointerType()) {
+ this->Diag(PtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< PtrArgType << "\'structure pointer type\'"
+<< 1 << 0 << 3 << 1
+<< PtrArgType << "\'structure pointer type\'";
+ return ExprError();
+}
+
+const RecordType *RT = PtrArgType->getPointeeType()->getAs();
+if (!RT) {
+ this->Diag(PtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< PtrArgType << "\'structure pointer type\'"
+<< 1 << 0 << 3 << 1
+<< PtrArgType << "\'structure pointer type\'";
+ return ExprError();
+}
+// Ensure that the second argument is of type 'FunctionType'
+const Expr *FnPtrArg = TheCall->getArg(1)->IgnoreImpCasts();
+const QualType FnPtrArgType = FnPtrArg->getType();
+if (!FnPtrArgType->isPointerType()) {
+ this->Diag(FnPtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'"
+<< 1 << 0 << 3 << 2
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'";
+ return ExprError();
+}
+
+const FunctionType *FuncType =
+ FnPtrArgType->getPointeeType()->getAs();
+
+if (!FuncType) {
+ this->Diag(FnPtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'"
+<< 1 << 0 << 3 << 2
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'";
+ return ExprError();
+}
+
+const FunctionProtoType *FT = dyn_cast(FuncType);
+if (FT) {
+ if (!FT->isVariadic() ||
+ FT->getReturnType() != Context.IntTy) {
+ this->Diag(FnPtrArg->getLocStart(), diag::err_typecheck_convert_incompatible)
+<< FnPtrArgType<< "\'int (*)(const char *, ...)\'"
+<< 1 << 0 << 3 << 2
+<< FnPtrArgType << "\'int (*)(const char *, ...)\'";
+return ExprError();
+ }
+}
+
+TheCall->setType(Context.IntTy);
+break;
+ }
// check secure string manipulation functions where overflows
// are detectable at compile time
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -14,6 +14,7 @@
#include "CGCXXABI.h"
#include "CGObjCRuntime.h"
#include "CGOpenCLRuntime.h"
+#include "CGRecordLayout.h"
#include "CodeGenFunction.h"
#include "CodeGenModule.h"
#include "ConstantEmitter.h"
@@ -930,6 +931,93 @@
return RValue::get(Overflow);
}
+static Value *dumpRecord(CodeGenFunction &CGF, QualType RType,
+ Value*& RecordPtr, CharUnits Align,
+ Value *Func, int Lvl)
+{
+ const RecordType *RT = RType->getAs();
+ ASTContext& Context = CGF.getContext();
+ RecordDecl *RD = RT->getDecl()->getDefinition();
+ ASTContext& Ctx = RD->getASTContext();
+ const ASTRecordLayout &RL = Ctx.getASTRecordLayout(RD);
+ std::string Pad = std::string(Lvl * 4, ' ');
+
+ Value *GString = CGF.Builder.CreateGlobalStringPtr(RType.getAsString()
+ + " {\n");
+ Value *Res = CGF.Builder.CreateCall(Func, {GString});
+
+ static llvm::DenseMap Types;
+ if (Types.empty()) {
+Types[Context.CharTy] = "%c";
+Types[Context.BoolTy] = "%d";
+Types[Context.IntTy] = "%d";
+Types[Context.UnsignedIntTy] = "%u";
+Types[Context.LongTy] = "%ld";
+Types[Context.UnsignedLongTy] = "%lu";
+Types[Context.LongLongTy] = "%lld";
+Types[Context.UnsignedLongLongTy] = "%llu";
+Types[Context.ShortTy] = "%hd";
+Types[Context.UnsignedShortTy] = "%hu";
+Types[Context.VoidPtrTy] = "%p";
+Types[Context.FloatTy] = "%f";
+Types[Context.DoubleTy] = "%f";
+Types[Context.LongDoubleTy] = "%Lf";
+Types[Context.getPointerType(Context.CharTy)] = "%s";
+}
+
+ for (const auto *FD : RD->fields()) {
+uint64_t Off = RL.getFieldOffset(FD->getFieldIndex());
+Off = Ctx.toCharUnitsFromBits(Off).getQuantity();
+
+Value *FieldPtr = Rec
[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods
zinovy.nis updated this revision to Diff 137783.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44295
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugprone/ParentVirtualCallCheck.cpp
clang-tidy/bugprone/ParentVirtualCallCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/bugprone-parent-virtual-call.cpp
Index: test/clang-tidy/bugprone-parent-virtual-call.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-parent-virtual-call.cpp
@@ -0,0 +1,116 @@
+// RUN: %check_clang_tidy %s bugprone-parent-virtual-call %t
+
+extern int f();
+
+class A {
+public:
+ A() = default;
+ virtual ~A() = default;
+
+ virtual int virt_1() { return f() + 1; }
+ virtual int virt_2() { return f() + 2; }
+
+ int non_virt() { return f() + 3; }
+ static int stat() { return f() + 4; }
+};
+
+class B : public A {
+public:
+ B() = default;
+
+ // Nothing to fix: calls to parent.
+ int virt_1() override { return A::virt_1() + 3; }
+ int virt_2() override { return A::virt_2() + 4; }
+};
+
+class C : public B {
+public:
+ int virt_1() override { return A::virt_1() + B::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'B'?
+ // CHECK-FIXES: int virt_1() override { return B::virt_1() + B::virt_1(); }
+ int virt_2() override { return A::virt_1() + B::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'B'?
+ // CHECK-FIXES: int virt_2() override { return B::virt_1() + B::virt_1(); }
+
+ // Test that non-virtual and static methods are not affected by this cherker.
+ int method_c() { return A::stat() + A::non_virt(); }
+};
+
+// Test that the check affects grand-grand..-parent calls too.
+class D : public C {
+public:
+ int virt_1() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: 'B::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-FIXES: int virt_1() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+ int virt_2() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: 'B::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-FIXES: int virt_2() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+};
+
+// Test classes in anonymous namespaces.
+namespace {
+class BN : public A {
+public:
+ int virt_1() override { return A::virt_1() + 3; }
+ int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace N
+
+class CN : public BN {
+public:
+ int virt_1() override { return A::virt_1() + BN::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean '(anonymous namespace)::BN'?
+ // CHECK-FIXES: int virt_1() override { return BN::virt_1() + BN::virt_1(); }
+ int virt_2() override { return A::virt_1() + BN::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean '(anonymous namespace)::BN'?
+ // CHECK-FIXES: int virt_2() override { return BN::virt_1() + BN::virt_1(); }
+};
+
+// Test multiple inheritance fixes
+class AA {
+public:
+ AA() = default;
+ virtual ~AA() = default;
+
+ virtual int virt_1() { return f() + 1; }
+ virtual int virt_2() { return f() + 2; }
+
+ int non_virt() { return f() + 3; }
+ static int stat() { return f() + 4; }
+};
+
+class BB_1 : virtual public AA {
+public:
+ BB_1() = default;
+
+ // Nothing to fix: calls to parent.
+ int virt_1() override { return AA::virt_1() + 3; }
+ int virt_2() override { return AA::virt_2() + 4; }
+};
+
+class BB_2 : virtual public AA {
+public:
+BB_2() = default;
+};
+
+class CC : public BB_1, public BB_2 {
+public:
+ int virt_1() override { return AA::virt_1() + 3; }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'AA::virt_1' is a grand-parent's method, not parent's. Did you mean 'BB_1' or 'BB_2'? [bugprone-parent-virtual-call]
+ // No fix available due to multiple choice of parent class.
+};
+
+// Test templated classes.
+template class BF : public A {
+public:
+ int virt_1() override { return A::virt_1() + 3; }
+};
+
+class CF : public BF {
+public:
+ int virt_1() override { return A::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'BF'?
+ // CHECK-FIXES: int virt_1() override { return BF::virt_1(); }
+};
Index: docs/clang
[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods
zinovy.nis updated this revision to Diff 137781.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44295
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugprone/ParentVirtualCallCheck.cpp
clang-tidy/bugprone/ParentVirtualCallCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/bugprone-parent-virtual-call.cpp
Index: test/clang-tidy/bugprone-parent-virtual-call.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-parent-virtual-call.cpp
@@ -0,0 +1,116 @@
+// RUN: %check_clang_tidy %s bugprone-parent-virtual-call %t
+
+extern int f();
+
+class A {
+public:
+ A() = default;
+ virtual ~A() = default;
+
+ virtual int virt_1() { return f() + 1; }
+ virtual int virt_2() { return f() + 2; }
+
+ int non_virt() { return f() + 3; }
+ static int stat() { return f() + 4; }
+};
+
+class B : public A {
+public:
+ B() = default;
+
+ // Nothing to fix: calls to parent.
+ int virt_1() override { return A::virt_1() + 3; }
+ int virt_2() override { return A::virt_2() + 4; }
+};
+
+class C : public B {
+public:
+ int virt_1() override { return A::virt_1() + B::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'B'?
+ // CHECK-FIXES: int virt_1() override { return B::virt_1() + B::virt_1(); }
+ int virt_2() override { return A::virt_1() + B::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'B'?
+ // CHECK-FIXES: int virt_2() override { return B::virt_1() + B::virt_1(); }
+
+ // Test that non-virtual and static methods are not affected by this cherker.
+ int method_c() { return A::stat() + A::non_virt(); }
+};
+
+// Test that the check affects grand-grand..-parent calls too.
+class D : public C {
+public:
+ int virt_1() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: 'B::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-FIXES: int virt_1() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+ int virt_2() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: 'B::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-FIXES: int virt_2() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+};
+
+// Test classes in anonymous namespaces.
+namespace {
+class BN : public A {
+public:
+ int virt_1() override { return A::virt_1() + 3; }
+ int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace N
+
+class CN : public BN {
+public:
+ int virt_1() override { return A::virt_1() + BN::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean '(anonymous namespace)::BN'?
+ // CHECK-FIXES: int virt_1() override { return BN::virt_1() + BN::virt_1(); }
+ int virt_2() override { return A::virt_1() + BN::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean '(anonymous namespace)::BN'?
+ // CHECK-FIXES: int virt_2() override { return BN::virt_1() + BN::virt_1(); }
+};
+
+// Test multiple inheritance fixes
+class AA {
+public:
+ AA() = default;
+ virtual ~AA() = default;
+
+ virtual int virt_1() { return f() + 1; }
+ virtual int virt_2() { return f() + 2; }
+
+ int non_virt() { return f() + 3; }
+ static int stat() { return f() + 4; }
+};
+
+class BB_1 : virtual public AA {
+public:
+ BB_1() = default;
+
+ // Nothing to fix: calls to parent.
+ int virt_1() override { return AA::virt_1() + 3; }
+ int virt_2() override { return AA::virt_2() + 4; }
+};
+
+class BB_2 : virtual public AA {
+public:
+BB_2() = default;
+};
+
+class CC : public BB_1, public BB_2 {
+public:
+ int virt_1() override { return AA::virt_1() + 3; }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'AA::virt_1' is a grand-parent's method, not parent's. Did you mean 'BB_1' or 'BB_2'? [bugprone-parent-virtual-call]
+ // No fix available due to multiple choise of parent class.
+};
+
+// Test templated classes.
+template class BF : public A {
+public:
+ int virt_1() override { return A::virt_1() + 3; }
+};
+
+class CF : public BF {
+public:
+ int virt_1() override { return A::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'BF'?
+ // CHECK-FIXES: int virt_1() override { return BF::virt_1(); }
+};
Index: docs/clang
[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods
zinovy.nis added inline comments. Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:152 +Member->getQualifierLoc().getSourceRange(), +GetNameAsString(*(Parents.front())) + "::"); + } malcolm.parsons wrote: > What does this do for templated parent classes? Please see my last test case in updated revision. Comment at: docs/clang-tidy/checks/bugprone-parent-virtual-call.rst:6 + +FIXME: Describe what patterns does the check detect and why. Give examples. malcolm.parsons wrote: > FIXME. Oops. Thanks! Comment at: test/clang-tidy/bugprone-parent-virtual-call.cpp:102 + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'AA::virt_1' is a grand-parent's method, not parent's. Did you mean 'BB_1' or 'BB_2'? [bugprone-parent-virtual-call] + // No fix available due to multiple choise of parent class. +}; malcolm.parsons wrote: > typo: choice Thanks! Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44295 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods
zinovy.nis updated this revision to Diff 137787.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44295
Files:
clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tidy/bugprone/CMakeLists.txt
clang-tidy/bugprone/ParentVirtualCallCheck.cpp
clang-tidy/bugprone/ParentVirtualCallCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/bugprone-parent-virtual-call.cpp
Index: test/clang-tidy/bugprone-parent-virtual-call.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-parent-virtual-call.cpp
@@ -0,0 +1,116 @@
+// RUN: %check_clang_tidy %s bugprone-parent-virtual-call %t
+
+extern int f();
+
+class A {
+public:
+ A() = default;
+ virtual ~A() = default;
+
+ virtual int virt_1() { return f() + 1; }
+ virtual int virt_2() { return f() + 2; }
+
+ int non_virt() { return f() + 3; }
+ static int stat() { return f() + 4; }
+};
+
+class B : public A {
+public:
+ B() = default;
+
+ // Nothing to fix: calls to parent.
+ int virt_1() override { return A::virt_1() + 3; }
+ int virt_2() override { return A::virt_2() + 4; }
+};
+
+class C : public B {
+public:
+ int virt_1() override { return A::virt_1() + B::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'B'?
+ // CHECK-FIXES: int virt_1() override { return B::virt_1() + B::virt_1(); }
+ int virt_2() override { return A::virt_1() + B::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'B'?
+ // CHECK-FIXES: int virt_2() override { return B::virt_1() + B::virt_1(); }
+
+ // Test that non-virtual and static methods are not affected by this cherker.
+ int method_c() { return A::stat() + A::non_virt(); }
+};
+
+// Test that the check affects grand-grand..-parent calls too.
+class D : public C {
+public:
+ int virt_1() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: 'B::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-FIXES: int virt_1() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+ int virt_2() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: 'B::virt_1' is a grand-parent's method, not parent's. Did you mean 'C'?
+ // CHECK-FIXES: int virt_2() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+};
+
+// Test classes in anonymous namespaces.
+namespace {
+class BN : public A {
+public:
+ int virt_1() override { return A::virt_1() + 3; }
+ int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace N
+
+class CN : public BN {
+public:
+ int virt_1() override { return A::virt_1() + BN::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean '(anonymous namespace)::BN'?
+ // CHECK-FIXES: int virt_1() override { return BN::virt_1() + BN::virt_1(); }
+ int virt_2() override { return A::virt_1() + BN::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean '(anonymous namespace)::BN'?
+ // CHECK-FIXES: int virt_2() override { return BN::virt_1() + BN::virt_1(); }
+};
+
+// Test multiple inheritance fixes
+class AA {
+public:
+ AA() = default;
+ virtual ~AA() = default;
+
+ virtual int virt_1() { return f() + 1; }
+ virtual int virt_2() { return f() + 2; }
+
+ int non_virt() { return f() + 3; }
+ static int stat() { return f() + 4; }
+};
+
+class BB_1 : virtual public AA {
+public:
+ BB_1() = default;
+
+ // Nothing to fix: calls to parent.
+ int virt_1() override { return AA::virt_1() + 3; }
+ int virt_2() override { return AA::virt_2() + 4; }
+};
+
+class BB_2 : virtual public AA {
+public:
+BB_2() = default;
+};
+
+class CC : public BB_1, public BB_2 {
+public:
+ int virt_1() override { return AA::virt_1() + 3; }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'AA::virt_1' is a grand-parent's method, not parent's. Did you mean 'BB_1' or 'BB_2'? [bugprone-parent-virtual-call]
+ // No fix available due to multiple choice of parent class.
+};
+
+// Test templated classes.
+template class BF : public A {
+public:
+ int virt_1() override { return A::virt_1() + 3; }
+};
+
+class CF : public BF {
+public:
+ int virt_1() override { return A::virt_1(); }
+ // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's method, not parent's. Did you mean 'BF'?
+ // CHECK-FIXES: int virt_1() override { return BF::virt_1(); }
+};
Index: docs/clang
[PATCH] D44314: [clangd] Collect the number of files referencing a symbol in the static index.
sammccall created this revision.
Herald added subscribers: cfe-commits, ioeric, jkorous-apple, ilya-biryukov,
klimek.
This is an important ranking signal.
It's off for the dynamic index for now.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44314
Files:
clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
clangd/index/Index.h
clangd/index/Merge.cpp
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/SymbolYAML.cpp
unittests/clangd/IndexTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -59,6 +59,7 @@
return arg.Definition.StartOffset == Offsets.first &&
arg.Definition.EndOffset == Offsets.second;
}
+MATCHER_P(Refs, R, "") { return int(arg.References) == R; }
namespace clang {
namespace clangd {
@@ -201,7 +202,7 @@
TEST_F(SymbolCollectorTest, Template) {
Annotations Header(R"(
// Template is indexed, specialization and instantiation is not.
-template struct [[Tmpl]] {T x = 0};
+template struct [[Tmpl]] {T x = 0;};
template <> struct Tmpl {};
extern template struct Tmpl;
template struct Tmpl;
@@ -242,6 +243,31 @@
AllOf(QName("Z"), DeclRange(Header.offsetRange("zdecl");
}
+TEST_F(SymbolCollectorTest, References) {
+ const std::string Header = R"(
+class W;
+class X {};
+class Y;
+class Z {}; // not used anywhere
+Y* y = nullptr; // used in header doesn't count
+ )";
+ const std::string Main = R"(
+W* w = nullptr;
+W* w2 = nullptr; // only one usage counts
+X x();
+class V;
+V* v = nullptr; // Used, but not eligible for indexing.
+class Y{}; // definition doesn't count as a reference
+ )";
+ CollectorOpts.CountReferences = true;
+ runSymbolCollector(Header, Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(AllOf(QName("W"), Refs(1)),
+ AllOf(QName("X"), Refs(1)),
+ AllOf(QName("Y"), Refs(0)),
+ AllOf(QName("Z"), Refs(0)), QName("y")));
+}
+
TEST_F(SymbolCollectorTest, SymbolRelativeNoFallback) {
runSymbolCollector("class Foo {};", /*Main=*/"");
EXPECT_THAT(Symbols, UnorderedElementsAre(
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -225,6 +225,8 @@
L.Name = R.Name = "Foo";// same in both
L.CanonicalDeclaration.FileURI = "file:///left.h"; // differs
R.CanonicalDeclaration.FileURI = "file:///right.h";
+ L.References = 1;
+ R.References = 2;
L.CompletionPlainInsertText = "f00";// present in left only
R.CompletionSnippetInsertText = "f0{$1:0}"; // present in right only
Symbol::Details DetL, DetR;
@@ -238,6 +240,7 @@
Symbol M = mergeSymbol(L, R, &Scratch);
EXPECT_EQ(M.Name, "Foo");
EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:///left.h");
+ EXPECT_EQ(M.References, 3u);
EXPECT_EQ(M.CompletionPlainInsertText, "f00");
EXPECT_EQ(M.CompletionSnippetInsertText, "f0{$1:0}");
ASSERT_TRUE(M.Detail);
Index: clangd/index/SymbolYAML.cpp
===
--- clangd/index/SymbolYAML.cpp
+++ clangd/index/SymbolYAML.cpp
@@ -100,6 +100,7 @@
IO.mapOptional("CanonicalDeclaration", Sym.CanonicalDeclaration,
SymbolLocation());
IO.mapOptional("Definition", Sym.Definition, SymbolLocation());
+IO.mapOptional("References", Sym.References, 0u);
IO.mapRequired("CompletionLabel", Sym.CompletionLabel);
IO.mapRequired("CompletionFilterText", Sym.CompletionFilterText);
IO.mapRequired("CompletionPlainInsertText", Sym.CompletionPlainInsertText);
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -45,6 +45,8 @@
/// If set, this is used to map symbol #include path to a potentially
/// different #include path.
const CanonicalIncludes *Includes = nullptr;
+// Populate the Symbol.References field.
+bool CountReferences = false;
};
SymbolCollector(Options Opts);
@@ -63,6 +65,8 @@
SymbolSlab takeSymbols() { return std::move(Symbols).build(); }
+ void finish() override;
+
private:
const Symbol *addDeclaration(const NamedDecl &, SymbolID);
void addDefinition(const NamedDecl &, const Symbol &DeclSymbol);
@@ -74,6 +78,8 @@
std::shared_ptr CompletionAllocator;
std::unique_ptr CompletionTUInfo;
Options Opts;
+ // Decls referenced from the current TU, flushed on finish().
+ llvm::DenseSet ReferencedDecls;
};
} // namespace clangd
Index: clangd/index/SymbolC
[PATCH] D44315: [clangd] Collect the number of files referencing a symbol in the static index.
sammccall created this revision.
sammccall added reviewers: ioeric, hokein.
Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek.
This is an important ranking signal.
It's off for the dynamic index for now. Correspondingly, tell the index
infrastructure only to report declarations for the dynamic index.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44315
Files:
clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.h
clangd/index/Merge.cpp
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/SymbolYAML.cpp
unittests/clangd/IndexTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -59,6 +59,7 @@
return arg.Definition.StartOffset == Offsets.first &&
arg.Definition.EndOffset == Offsets.second;
}
+MATCHER_P(Refs, R, "") { return int(arg.References) == R; }
namespace clang {
namespace clangd {
@@ -201,7 +202,7 @@
TEST_F(SymbolCollectorTest, Template) {
Annotations Header(R"(
// Template is indexed, specialization and instantiation is not.
-template struct [[Tmpl]] {T x = 0};
+template struct [[Tmpl]] {T x = 0;};
template <> struct Tmpl {};
extern template struct Tmpl;
template struct Tmpl;
@@ -242,6 +243,31 @@
AllOf(QName("Z"), DeclRange(Header.offsetRange("zdecl");
}
+TEST_F(SymbolCollectorTest, References) {
+ const std::string Header = R"(
+class W;
+class X {};
+class Y;
+class Z {}; // not used anywhere
+Y* y = nullptr; // used in header doesn't count
+ )";
+ const std::string Main = R"(
+W* w = nullptr;
+W* w2 = nullptr; // only one usage counts
+X x();
+class V;
+V* v = nullptr; // Used, but not eligible for indexing.
+class Y{}; // definition doesn't count as a reference
+ )";
+ CollectorOpts.CountReferences = true;
+ runSymbolCollector(Header, Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(AllOf(QName("W"), Refs(1)),
+ AllOf(QName("X"), Refs(1)),
+ AllOf(QName("Y"), Refs(0)),
+ AllOf(QName("Z"), Refs(0)), QName("y")));
+}
+
TEST_F(SymbolCollectorTest, SymbolRelativeNoFallback) {
runSymbolCollector("class Foo {};", /*Main=*/"");
EXPECT_THAT(Symbols, UnorderedElementsAre(
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -225,6 +225,8 @@
L.Name = R.Name = "Foo";// same in both
L.CanonicalDeclaration.FileURI = "file:///left.h"; // differs
R.CanonicalDeclaration.FileURI = "file:///right.h";
+ L.References = 1;
+ R.References = 2;
L.CompletionPlainInsertText = "f00";// present in left only
R.CompletionSnippetInsertText = "f0{$1:0}"; // present in right only
Symbol::Details DetL, DetR;
@@ -238,6 +240,7 @@
Symbol M = mergeSymbol(L, R, &Scratch);
EXPECT_EQ(M.Name, "Foo");
EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:///left.h");
+ EXPECT_EQ(M.References, 3u);
EXPECT_EQ(M.CompletionPlainInsertText, "f00");
EXPECT_EQ(M.CompletionSnippetInsertText, "f0{$1:0}");
ASSERT_TRUE(M.Detail);
Index: clangd/index/SymbolYAML.cpp
===
--- clangd/index/SymbolYAML.cpp
+++ clangd/index/SymbolYAML.cpp
@@ -100,6 +100,7 @@
IO.mapOptional("CanonicalDeclaration", Sym.CanonicalDeclaration,
SymbolLocation());
IO.mapOptional("Definition", Sym.Definition, SymbolLocation());
+IO.mapOptional("References", Sym.References, 0u);
IO.mapRequired("CompletionLabel", Sym.CompletionLabel);
IO.mapRequired("CompletionFilterText", Sym.CompletionFilterText);
IO.mapRequired("CompletionPlainInsertText", Sym.CompletionPlainInsertText);
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -45,6 +45,8 @@
/// If set, this is used to map symbol #include path to a potentially
/// different #include path.
const CanonicalIncludes *Includes = nullptr;
+// Populate the Symbol.References field.
+bool CountReferences = false;
};
SymbolCollector(Options Opts);
@@ -63,6 +65,8 @@
SymbolSlab takeSymbols() { return std::move(Symbols).build(); }
+ void finish() override;
+
private:
const Symbol *addDeclaration(const NamedDecl &, SymbolID);
void addDefinition(const NamedDecl &, const Symbol &DeclSymbol);
@@ -74,6 +78,8 @@
std::shared_ptr CompletionAllocator;
std::unique_ptr CompletionTUInfo;
Options O
[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods
Eugene.Zelenko added inline comments.
Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:26
+
+bool IsParentOf(const CXXRecordDecl *Parent, const CXXRecordDecl *ThisClass) {
+ assert(Parent);
Please make this function static and remove anonymous namespace.
Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:40
+
+bool IsRecursiveParentOf(const CXXRecordDecl *Parent,
+ const CXXRecordDecl *ThisClass) {
Please make this function static and remove anonymous namespace.
Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:48
+std::list
+GetParentsByGrandParent(const CXXRecordDecl *GrandParent,
+const CXXRecordDecl *ThisClass) {
Please make this function static and remove anonymous namespace.
Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:66
+// PrintingPolicy for anonymous namespaces.
+std::string GetNameAsString(const NamedDecl &Decl) {
+ PrintingPolicy PP(Decl.getASTContext().getPrintingPolicy());
Please make this function static and remove anonymous namespace.
Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:91
+void ParentVirtualCallCheck::check(const MatchFinder::MatchResult &Result) {
+
+ const auto *MatchedDecl = Result.Nodes.getNodeAs("call");
Please remove this line.
Comment at: docs/ReleaseNotes.rst:76
+
+ Warns if one calls grand-..parent's virtual method in child's virtual
+ method instead of parent's. Can automatically fix such cases by retargeting
Please make description here and first statement in documentation same.
Comment at: docs/clang-tidy/checks/bugprone-parent-virtual-call.rst:9
+
+class A {
+...
Please add .. code-block:: c++
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44059: [analyzer] AST-matching checker to detect global central dispatch performance anti-pattern
dcoughlin added a comment.
This looks good. Some minor post-commit review inline.
Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:615
+
+def GCDAsyncSemaphoreChecker : Checker<"GCDAsyncSemaphore">,
+ HelpText<"Checker for performance anti-pattern when using semaphors from
async API">,
This should have a more general name so that we can add related checks to it in
the future. I suggest "GCDAntipattern".
Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:616
+def GCDAsyncSemaphoreChecker : Checker<"GCDAsyncSemaphore">,
+ HelpText<"Checker for performance anti-pattern when using semaphors from
async API">,
+ DescFile<"GCDAsyncSemaphoreChecker.cpp">;
We don't use "checker" as a term in user-facing text. I suggest "Check for
performance anti-patterns when using Grand Central Dispatch".
Comment at: lib/StaticAnalyzer/Checkers/GCDAsyncSemaphoreChecker.cpp:12
+// antipattern when synchronous API is emulated from asynchronous callbacks
+// using a semaphor:
+//
Nit: missing "e" at end of "semaphor".
Comment at: lib/StaticAnalyzer/Checkers/GCDAsyncSemaphoreChecker.cpp:23
+// Such code is a common performance problem, due to inability of GCD to
+// properly handle QoS when a combination of queues and semaphors is used.
+// Good code would either use asynchronous API (when available), or perform
Nit: same here "semaphores"
Comment at: lib/StaticAnalyzer/Checkers/GCDAsyncSemaphoreChecker.cpp:92
+ if (const auto* ND = dyn_cast(D))
+if (ND->getName().startswith("test"))
+ return;
It would be good to look at both the method/function name and the class name
name for "test", "Test", "mock", and "Mock".
Comment at: lib/StaticAnalyzer/Checkers/GCDAsyncSemaphoreChecker.cpp:144
+ /*Category=*/"Performance",
+ "Possible semaphore performance anti-pattern: wait on a semaphore "
+ "signalled to in a callback",
We should tell the user more information about why they should believe this is
bad.
I suggest "Waiting on a semaphore with Grand Central Dispatch creates useless
threads and is subject to priority inversion"
Repository:
rC Clang
https://reviews.llvm.org/D44059
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods
malcolm.parsons added inline comments.
Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:152
+Member->getQualifierLoc().getSourceRange(),
+GetNameAsString(*(Parents.front())) + "::");
+ }
zinovy.nis wrote:
> malcolm.parsons wrote:
> > What does this do for templated parent classes?
> Please see my last test case in updated revision.
Also, a templated parent class of a templated class.
```
template
class DF : public BF {
public:
int virt_1() override { return A::virt_1(); }
// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: 'A::virt_1' is a grand-parent's
method, not parent's. Did you mean 'BF'?
// CHECK-FIXES: int virt_1() override { return BF::virt_1(); }
};
```
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43494: [Modules] Fix creating fake definition data for lambdas.
vsapsai updated this revision to Diff 137801.
vsapsai added a comment.
- Claim the definition data more eagerly.
Not sure that added "different" in the existing comment is actually useful.
It makes sense to me but don't know about others.
https://reviews.llvm.org/D43494
Files:
clang/lib/Serialization/ASTReaderDecl.cpp
clang/test/Modules/Inputs/self-referencing-lambda/a.h
clang/test/Modules/Inputs/self-referencing-lambda/module.modulemap
clang/test/Modules/self-referencing-lambda.cpp
Index: clang/test/Modules/self-referencing-lambda.cpp
===
--- /dev/null
+++ clang/test/Modules/self-referencing-lambda.cpp
@@ -0,0 +1,5 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I
%S/Inputs/self-referencing-lambda %s -verify -emit-obj -o %t2.o
+// expected-no-diagnostics
+
+#include "a.h"
Index: clang/test/Modules/Inputs/self-referencing-lambda/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/self-referencing-lambda/module.modulemap
@@ -0,0 +1,4 @@
+module "a.h" {
+ header "a.h"
+ export *
+}
Index: clang/test/Modules/Inputs/self-referencing-lambda/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/self-referencing-lambda/a.h
@@ -0,0 +1,4 @@
+void f() {
+ int x = 0;
+ auto q = [xm = x]{};
+}
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1782,21 +1782,27 @@
else
DD = new (C) struct CXXRecordDecl::DefinitionData(D);
+ CXXRecordDecl *Canon = D->getCanonicalDecl();
+ // Set decl definition data before reading it, so that during deserialization
+ // when we read CXXRecordDecl, it already has definition data and we don't
+ // set fake one.
+ if (!Canon->DefinitionData) {
+Canon->DefinitionData = DD;
+ }
+ D->DefinitionData = Canon->DefinitionData;
ReadCXXDefinitionData(*DD, D);
- // We might already have a definition for this record. This can happen either
- // because we're reading an update record, or because we've already done some
- // merging. Either way, just merge into it.
- CXXRecordDecl *Canon = D->getCanonicalDecl();
- if (Canon->DefinitionData) {
+ // We might already have a different definition for this record. This can
+ // happen either because we're reading an update record, or because we've
+ // already done some merging. Either way, just merge into it.
+ if (Canon->DefinitionData && Canon->DefinitionData != DD) {
MergeDefinitionData(Canon, std::move(*DD));
D->DefinitionData = Canon->DefinitionData;
return;
}
// Mark this declaration as being a definition.
D->IsCompleteDefinition = true;
- D->DefinitionData = DD;
// If this is not the first declaration or is an update record, we can have
// other redeclarations already. Make a note that we need to propagate the
Index: clang/test/Modules/self-referencing-lambda.cpp
===
--- /dev/null
+++ clang/test/Modules/self-referencing-lambda.cpp
@@ -0,0 +1,5 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/self-referencing-lambda %s -verify -emit-obj -o %t2.o
+// expected-no-diagnostics
+
+#include "a.h"
Index: clang/test/Modules/Inputs/self-referencing-lambda/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/self-referencing-lambda/module.modulemap
@@ -0,0 +1,4 @@
+module "a.h" {
+ header "a.h"
+ export *
+}
Index: clang/test/Modules/Inputs/self-referencing-lambda/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/self-referencing-lambda/a.h
@@ -0,0 +1,4 @@
+void f() {
+ int x = 0;
+ auto q = [xm = x]{};
+}
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1782,21 +1782,27 @@
else
DD = new (C) struct CXXRecordDecl::DefinitionData(D);
+ CXXRecordDecl *Canon = D->getCanonicalDecl();
+ // Set decl definition data before reading it, so that during deserialization
+ // when we read CXXRecordDecl, it already has definition data and we don't
+ // set fake one.
+ if (!Canon->DefinitionData) {
+Canon->DefinitionData = DD;
+ }
+ D->DefinitionData = Canon->DefinitionData;
ReadCXXDefinitionData(*DD, D);
- // We might already have a definition for this record. This can happen either
- // because we're reading an update record, or because we've already done some
- // merging. Either way, just merge into it.
- CXXRecordD
[PATCH] D16008: [clang-tidy] Add calling virtual functions in constructors/destructors check.
lebedev.ri added a comment. Herald added subscribers: llvm-commits, xazax.hun, mgorny, klimek. Any further thoughts here? I was slightly bitten by this recently, and i though that it already existed as a clang-tidy check (: Repository: rL LLVM https://reviews.llvm.org/D16008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44281: [analyzer] Suppress more MallocChecker positives in reference counting pointer destructors.
george.karpenkov added a comment.
> we often run out of inlining stack depth limit
Can we consider increasing that limit? I'd much rather have a limit on maximum
path *length* (which we currently don't have), as longer paths are more likely
to be false positives.
On the other hand, I don't see that many issues with paths which perform too
many inlinings.
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2835
+// FIXME: Use regular expressions when they get marked as acceptable
+// in the LLVM coding standard?
+if (N.contains_lower("ptr") || N.contains_lower("pointer")) {
There's `lib/Support/Regex.cpp`?
Comment at: lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2904
for (const LocationContext *LC = CurrentLC; LC; LC = LC->getParent()) {
-if (isa(LC->getDecl())) {
- assert(!ReleaseDestructorLC &&
- "There can be only one release point!");
- ReleaseDestructorLC = LC->getCurrentStackFrame();
- // It is unlikely that releasing memory is delegated to a destructor
- // inside a destructor of a shared pointer, because it's fairly hard
- // to pass the information that the pointer indeed needs to be
- // released into it. So we're only interested in the innermost
- // destructor.
- break;
+if (const CXXDestructorDecl *DD =
+dyn_cast(LC->getDecl())) {
auto
Repository:
rC Clang
https://reviews.llvm.org/D44281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43898: Preliminary refactoring in service of -Wreturn-std-move. NFC.
Quuxplusone added a comment. @rtrieu ping? Repository: rC Clang https://reviews.llvm.org/D43898 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r327165 - Don't use -pie in relocatable link.
Author: eugenis
Date: Fri Mar 9 11:35:16 2018
New Revision: 327165
URL: http://llvm.org/viewvc/llvm-project?rev=327165&view=rev
Log:
Don't use -pie in relocatable link.
Summary:
Android, in particular, got PIE enabled by default in r316606. It resulted in
relocatable links passing both -r and -pie to the linker, which is not allowed.
Reviewers: srhines
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D44229
Modified:
cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
cfe/trunk/test/Driver/android-pie.c
Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=327165&r1=327164&r2=327165&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Fri Mar 9 11:35:16 2018
@@ -307,7 +307,8 @@ static const char *getLDMOption(const ll
}
static bool getPIE(const ArgList &Args, const toolchains::Linux &ToolChain) {
- if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static))
+ if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
+ Args.hasArg(options::OPT_r))
return false;
Arg *A = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,
Modified: cfe/trunk/test/Driver/android-pie.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/android-pie.c?rev=327165&r1=327164&r2=327165&view=diff
==
--- cfe/trunk/test/Driver/android-pie.c (original)
+++ cfe/trunk/test/Driver/android-pie.c Fri Mar 9 11:35:16 2018
@@ -64,3 +64,20 @@
// RUN: | FileCheck --check-prefix=NO-PIE %s
// RUN: %clang %s -### -o %t.o 2>&1 -pie -no-pie
--target=arm-linux-androideabi24 \
// RUN: | FileCheck --check-prefix=NO-PIE %s
+
+// Static/shared/relocatable disable -pie
+
+// RUN: %clang %s -### --target=aarch64-linux-android -static 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-STATIC
+// CHECK-STATIC-NOT: "-pie"
+// CHECK-STATIC: -static
+
+// RUN: %clang %s -### --target=aarch64-linux-android -shared 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-SHARED
+// CHECK-SHARED-NOT: "-pie"
+// CHECK-SHARED: "-shared"
+
+// RUN: %clang %s -### --target=aarch64-linux-android -r 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-RELOCATABLE
+// CHECK-RELOCATABLE-NOT: "-pie"
+// CHECK-RELOCATABLE: "-r"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44229: Don't use -pie in relocatable link.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC327165: Don't use -pie in relocatable link. (authored
by eugenis, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D44229?vs=137493&id=137809#toc
Repository:
rC Clang
https://reviews.llvm.org/D44229
Files:
lib/Driver/ToolChains/Gnu.cpp
test/Driver/android-pie.c
Index: test/Driver/android-pie.c
===
--- test/Driver/android-pie.c
+++ test/Driver/android-pie.c
@@ -64,3 +64,20 @@
// RUN: | FileCheck --check-prefix=NO-PIE %s
// RUN: %clang %s -### -o %t.o 2>&1 -pie -no-pie
--target=arm-linux-androideabi24 \
// RUN: | FileCheck --check-prefix=NO-PIE %s
+
+// Static/shared/relocatable disable -pie
+
+// RUN: %clang %s -### --target=aarch64-linux-android -static 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-STATIC
+// CHECK-STATIC-NOT: "-pie"
+// CHECK-STATIC: -static
+
+// RUN: %clang %s -### --target=aarch64-linux-android -shared 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-SHARED
+// CHECK-SHARED-NOT: "-pie"
+// CHECK-SHARED: "-shared"
+
+// RUN: %clang %s -### --target=aarch64-linux-android -r 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-RELOCATABLE
+// CHECK-RELOCATABLE-NOT: "-pie"
+// CHECK-RELOCATABLE: "-r"
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -307,7 +307,8 @@
}
static bool getPIE(const ArgList &Args, const toolchains::Linux &ToolChain) {
- if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static))
+ if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
+ Args.hasArg(options::OPT_r))
return false;
Arg *A = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,
Index: test/Driver/android-pie.c
===
--- test/Driver/android-pie.c
+++ test/Driver/android-pie.c
@@ -64,3 +64,20 @@
// RUN: | FileCheck --check-prefix=NO-PIE %s
// RUN: %clang %s -### -o %t.o 2>&1 -pie -no-pie --target=arm-linux-androideabi24 \
// RUN: | FileCheck --check-prefix=NO-PIE %s
+
+// Static/shared/relocatable disable -pie
+
+// RUN: %clang %s -### --target=aarch64-linux-android -static 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-STATIC
+// CHECK-STATIC-NOT: "-pie"
+// CHECK-STATIC: -static
+
+// RUN: %clang %s -### --target=aarch64-linux-android -shared 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-SHARED
+// CHECK-SHARED-NOT: "-pie"
+// CHECK-SHARED: "-shared"
+
+// RUN: %clang %s -### --target=aarch64-linux-android -r 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-RELOCATABLE
+// CHECK-RELOCATABLE-NOT: "-pie"
+// CHECK-RELOCATABLE: "-r"
Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -307,7 +307,8 @@
}
static bool getPIE(const ArgList &Args, const toolchains::Linux &ToolChain) {
- if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static))
+ if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
+ Args.hasArg(options::OPT_r))
return false;
Arg *A = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r327166 - Fix Clang test case.
Author: pcc Date: Fri Mar 9 11:37:28 2018 New Revision: 327166 URL: http://llvm.org/viewvc/llvm-project?rev=327166&view=rev Log: Fix Clang test case. Modified: cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll Modified: cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll?rev=327166&r1=327165&r2=327166&view=diff == --- cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll (original) +++ cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll Fri Mar 9 11:37:28 2018 @@ -23,7 +23,7 @@ ; Ensure that typeids are in the index. ; RUN: llvm-bcanalyzer -dump %t.o.thinlto.bc | FileCheck %s ; CHECK-LABEL: +; CHECK: ; CHECK-LABEL: http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43494: [Modules] Fix creating fake definition data for lambdas.
rsmith added inline comments.
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:1789
+ // set fake one.
+ if (!Canon->DefinitionData) {
+Canon->DefinitionData = DD;
No braces around single-line `if` bodies, please.
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:1798
+ // already done some merging. Either way, just merge into it.
+ if (Canon->DefinitionData && Canon->DefinitionData != DD) {
MergeDefinitionData(Canon, std::move(*DD));
`Canon->DefinitionData` can't be null here, so the first half of this check is
redundant. If you're concerned about that, you can add an assert that it's
equal to `D->DefinitionData` (and that both are non-null).
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:1800
MergeDefinitionData(Canon, std::move(*DD));
D->DefinitionData = Canon->DefinitionData;
return;
This store is dead.
https://reviews.llvm.org/D43494
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44231: [clang-tidy] Check for sizeof that call functions
pfultz2 added a comment. > I don't have a script for it. I've used "bear" with at least some of those > projects because they use makefiles rather than cmake > (https://github.com/rizsotto/Bear). I'm not tied to those projects > specifically, either, so if you have a different corpus you'd prefer to use, > that's fine. The important bit is testing it across a considerable amount of > C code and C++ code to see whether this diagnostic is too chatty or not. So I did a grep over these codebases(with `grep -E '\bsizeof\([^()"*]+\([^()]*\)'`). Most of them are macros which access elements(ie no function call) or are used in type traits. The only false positive I saw was here: https://github.com/rethinkdb/rethinkdb/blob/v2.3.x/external/re2_20140111/re2/prog.cc#L317 So I dont think it will be too chatty. > That won't catch many (most?) of the issues demonstrated by PVS-Studio; the > rule their check follows are to warn on side-effecting operations (which > Clang already does with -Wunevaluated-expression) and arithmetic expressions > in sizeof. It finds function calls as well. I tried on MIOpen and it caught the errors like I mentioned earlier here: https://github.com/ROCmSoftwarePlatform/MIOpen/blob/master/src/convolution.cpp#L184 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44093: [BUILTINS] structure pretty printer
thegameg added a comment.
Thanks for working on this! Few remarks in the comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:934
+static Value *dumpRecord(CodeGenFunction &CGF, QualType RType,
+ Value*& RecordPtr, CharUnits Align,
`llvm::Value` for consistency?
Comment at: lib/CodeGen/CGBuiltin.cpp:966
+Types[Context.getPointerType(Context.CharTy)] = "%s";
+}
+
Indentation failed here.
Comment at: lib/CodeGen/CGBuiltin.cpp:976
+ FieldPtr = CGF.Builder.CreateAdd(FieldPtr,
ConstantInt::get(CGF.IntPtrTy, Off));
+ FieldPtr = CGF.Builder.CreateIntToPtr(FieldPtr, CGF.VoidPtrTy);
+}
I think you should use `getelementptr` instead of `ptrtoint` -> `inttoptr`
https://llvm.org/docs/GetElementPtr.html
Comment at: lib/CodeGen/CGBuiltin.cpp:997
+if (Types.find(CanonicalType) == Types.end())
+Format = Types[Context.VoidPtrTy];
+else
Indentation failed here too.
Comment at: lib/CodeGen/CGBuiltin.cpp:1003
+llvm::Type *ResType = CGF.ConvertType(ResPtrType);
+FieldPtr = CGF.Builder.CreatePointerCast(FieldPtr, ResType);
+Address FieldAddress = Address(FieldPtr, Align);
If you use GEP you should be able to get rid of this cast here.
Comment at: lib/CodeGen/CGBuiltin.cpp:1009
+
+GString = CGF.Builder.CreateGlobalStringPtr(Format + "\n");
+TmpRes = CGF.Builder.CreateCall(Func, {GString, FieldPtr});
You can probably use `llvm::Twine` for the concatenation of `Format`:
http://llvm.org/doxygen/classllvm_1_1Twine.html.
Comment at: lib/Sema/SemaChecking.cpp:1159
+const FunctionProtoType *FT = dyn_cast(FuncType);
+if (FT) {
+ if (!FT->isVariadic() ||
`if (const FunctionProtoType *FT = dyn_cast(FuncType))`
Repository:
rC Clang
https://reviews.llvm.org/D44093
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44231: [clang-tidy] Check for sizeof that call functions
pfultz2 updated this revision to Diff 137822.
https://reviews.llvm.org/D44231
Files:
clang-tidy/misc/SizeofExpressionCheck.cpp
clang-tidy/misc/SizeofExpressionCheck.h
docs/clang-tidy/checks/misc-sizeof-expression.rst
test/clang-tidy/misc-sizeof-expression.cpp
Index: test/clang-tidy/misc-sizeof-expression.cpp
===
--- test/clang-tidy/misc-sizeof-expression.cpp
+++ test/clang-tidy/misc-sizeof-expression.cpp
@@ -14,14 +14,75 @@
#pragma pack(1)
struct S { char a, b, c; };
+enum E { E_VALUE = 0 };
+
+int AsInt() { return 0; }
+E AsEnum() { return E_VALUE; }
+S AsStruct() { return {}; }
+
+struct M {
+ int AsInt() { return 0; }
+ E AsEnum() { return E_VALUE; }
+ S AsStruct() { return {}; }
+};
+
+int ReturnOverload(int) { return {}; }
+S ReturnOverload(S) { return {}; }
+
+template
+T ReturnTemplate(T) { return {}; }
+
+template
+bool TestTrait1() {
+ return sizeof(ReturnOverload(T{})) == sizeof(A);
+}
+
+template
+bool TestTrait2() {
+ return sizeof(ReturnTemplate(T{})) == sizeof(A);
+}
+
+template
+bool TestTrait3() {
+ return sizeof(ReturnOverload(0)) == sizeof(T{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof() on an expression that results in an integer
+}
+
+template
+bool TestTrait4() {
+ return sizeof(ReturnTemplate(0)) == sizeof(T{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof() on an expression that results in an integer
+}
+
+bool TestTemplates() {
+ bool b = true;
+ b &= TestTrait1();
+ b &= TestTrait1();
+ b &= TestTrait2();
+ b &= TestTrait2();
+ b &= TestTrait3();
+ b &= TestTrait3();
+ b &= TestTrait4();
+ b &= TestTrait4();
+ return b;
+}
+
int Test1(const char* ptr) {
int sum = 0;
sum += sizeof(LEN);
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
sum += sizeof(LEN + 1);
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
sum += sizeof(sum, LEN);
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+ sum += sizeof(AsInt());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof() on an expression that results in an integer
+ sum += sizeof(AsEnum());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof() on an expression that results in an integer
+ sum += sizeof(M{}.AsInt());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof() on an expression that results in an integer
+ sum += sizeof(M{}.AsEnum());
+ // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof() on an expression that results in an integer
sum += sizeof(sizeof(X));
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
sum += sizeof(LEN + sizeof(X));
@@ -171,6 +232,8 @@
if (sizeof(A) < 10)
sum += sizeof(A);
sum += sizeof(int);
+ sum += sizeof(AsStruct());
+ sum += sizeof(M{}.AsStruct());
sum += sizeof(A[sizeof(A) / sizeof(int)]);
sum += sizeof(&A[sizeof(A) / sizeof(int)]);
sum += sizeof(sizeof(0)); // Special case: sizeof size_t.
Index: docs/clang-tidy/checks/misc-sizeof-expression.rst
===
--- docs/clang-tidy/checks/misc-sizeof-expression.rst
+++ docs/clang-tidy/checks/misc-sizeof-expression.rst
@@ -22,6 +22,26 @@
char buf[BUFLEN];
memset(buf, 0, sizeof(BUFLEN)); // sizeof(42) ==> sizeof(int)
+Suspicious usage of 'sizeof(expr)'
+--
+
+A common mistake is to query the ``sizeof`` on an integer or enum that
+represents the type, which will give the size of the integer and not of the
+type the integer represents:
+
+.. code-block:: c++
+
+ enum data_type {
+FLOAT_TYPE,
+DOUBLE_TYPE
+ };
+
+ data_type get_type() {
+return FLOAT_TYPE;
+ }
+
+ int numBytes = numElements * sizeof(get_type()); // should be sizeof(float)
+
Suspicious usage of 'sizeof(this)'
--
@@ -142,6 +162,11 @@
When non-zero, the check will warn on an expression like
``sizeof(CONSTANT)``. Default is `1`.
+.. option:: WarnOnSizeOfIntegerExpression
+
+ When non-zero, the check will warn on an expression like ``sizeof(expr)``
+ where the expression results in an integer. Default is `1`.
+
.. option:: WarnOnSizeOfThis
When non-zero, the check will warn on an expression like ``sizeof(this)``.
Index: clang-tidy/misc/SizeofExpressionCheck.h
===
--- clang-tidy/misc/SizeofExpressionCheck.h
+++ clang-tidy/misc/SizeofExpressionCheck.h
@@ -29,6 +29,7 @@
private:
const bool WarnOnSizeOfConstant;
+ const bool WarnOnSizeOfIntegerExpression;
const bool WarnOnSizeOfThis;
const bool WarnOnSizeOfCompareToConstant;
};
Index: clang-tidy/misc/SizeofExpressionCheck.cpp
=
[PATCH] D43281: [AMDGPU] fixes for lds f32 builtins
dfukalov added a comment. ping... Repository: rC Clang https://reviews.llvm.org/D43281 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods
Eugene.Zelenko added inline comments. Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:12 + +#include +#include Please run Clang-format and remove empty line between headers. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44295 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43281: [AMDGPU] fixes for lds f32 builtins
arsenm added a comment.
In https://reviews.llvm.org/D43281#1023962, @dfukalov wrote:
> The problem is that if set addrspace "2" in description string,
> CanT.getAddressSpace() returns target addrspace value "11" (shifted in the
> enum) and compares it with input LangAS addrspace ("2", "opencl_local" in our
> case).
> So I cannot set a number a description string that will be equal to LangAS
> addrspace "opencl_local".
>
> Moreover, this change is preparation to move to custom processing of these
> builtins. Then I'm going to remove link (GCCBuiltin in IntrinsicsAMDGPU.td)
> from the llvm intrinsics definitions. Then I'll be able to switch on custom
> processing in cfe.
My real question was what happens if you put 11 in the description string?
Repository:
rC Clang
https://reviews.llvm.org/D43281
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44327: ObjCARC: teach the cloner about funclets
compnerd created this revision.
compnerd added a reviewer: gottesmm.
Herald added a subscriber: cfe-commits.
compnerd added a reviewer: ahatanak.
compnerd added subscribers: rnk, majnemer.
In the case that the CallInst that is being moved has an associated
operand bundle which is a funclet, the move will construct an invalid
instruction. The new site will have a different token and needs to be
reassociated with the new instruction.
Unfortunately, there is no way to alter the bundle after the
construction of the instruction. Replace the call instruction cloning
with a custom helper to clone the instruction and reassociate the
funclet token.
Repository:
rC Clang
https://reviews.llvm.org/D44327
Files:
lib/Transforms/ObjCARC/ObjCARCOpts.cpp
test/Transforms/ObjCARC/funclet.ll
Index: test/Transforms/ObjCARC/funclet.ll
===
--- /dev/null
+++ test/Transforms/ObjCARC/funclet.ll
@@ -0,0 +1,54 @@
+; RUN: %opt -mtriple x86_64-unknown-windows-msvc -objc-arc -S -o - %s | FileCheck %s
+
+declare zeroext i1 @"\01?g@@YA_NXZ"() local_unnamed_addr
+declare i8* @"\01?h@@YAPEAUobjc_object@@XZ"() local_unnamed_addr
+
+declare dllimport void @objc_release(i8*) local_unnamed_addr
+declare dllimport i8* @objc_retainAutoreleasedReturnValue(i8* returned) local_unnamed_addr
+
+declare i32 @__CxxFrameHandler3(...)
+
+define void @"\01?f@@YAXXZ"() local_unnamed_addr personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
+entry:
+ %call = invoke zeroext i1 @"\01?g@@YA_NXZ"()
+ to label %invoke.cont unwind label %ehcleanup6
+
+invoke.cont: ; preds = %entry
+ br i1 %call, label %if.then, label %if.end
+
+if.then: ; preds = %invoke.cont
+ %call2 = invoke i8* @"\01?h@@YAPEAUobjc_object@@XZ"()
+ to label %invoke.cont1 unwind label %ehcleanup6
+
+invoke.cont1: ; preds = %if.then
+ %0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %call2)
+ tail call void @objc_release(i8* null), !clang.imprecise_release !1
+ br label %if.end
+
+if.end: ; preds = %invoke.cont1, %invoke.cont
+ %a.0 = phi i8* [ %call2, %invoke.cont1 ], [ null, %invoke.cont ]
+ %call4 = invoke zeroext i1 @"\01?g@@YA_NXZ"()
+ to label %invoke.cont3 unwind label %ehcleanup
+
+invoke.cont3: ; preds = %if.end
+ tail call void @objc_release(i8* null), !clang.imprecise_release !1
+ tail call void @objc_release(i8* %a.0), !clang.imprecise_release !1
+ ret void
+
+ehcleanup:; preds = %if.end
+ %1 = cleanuppad within none []
+ call void @objc_release(i8* null) [ "funclet"(token %1) ], !clang.imprecise_release !1
+ cleanupret from %1 unwind label %ehcleanup6
+
+ehcleanup6: ; preds = %ehcleanup, %if.then, %entry
+ %a.1 = phi i8* [ %a.0, %ehcleanup ], [ null, %if.then ], [ null, %entry ]
+ %2 = cleanuppad within none []
+ call void @objc_release(i8* %a.1) [ "funclet"(token %2) ], !clang.imprecise_release !1
+ cleanupret from %2 unwind to caller
+}
+
+; CHECK: call void @objc_release(i8* {{.*}}) {{.*}}[ "funclet"(token %1) ]
+; CHECK-NOT: call void @objc_release(i8* {{.*}}) {{.*}}[ "funclet"(token %2) ]
+
+!1 = !{}
+
Index: lib/Transforms/ObjCARC/ObjCARCOpts.cpp
===
--- lib/Transforms/ObjCARC/ObjCARCOpts.cpp
+++ lib/Transforms/ObjCARC/ObjCARCOpts.cpp
@@ -684,6 +684,27 @@
DEBUG(dbgs() << "New: " << *AutoreleaseRV << "\n");
}
+namespace {
+Instruction *CloneCallInstForBB(Instruction &I, BasicBlock &BB) {
+ auto *CI = dyn_cast(&I);
+ assert(CI && "CloneCallInst must receive a CallInst");
+
+ SmallVector OpBundles;
+ for (unsigned I = 0, E = CI->getNumOperandBundles(); I != E; ++I) {
+auto Bundle = CI->getOperandBundleAt(I);
+// funclets will be reassociated in the future
+if (Bundle.getTagID() == LLVMContext::OB_funclet)
+ continue;
+OpBundles.emplace_back(Bundle);
+ }
+
+ if (auto *CleanupPad = dyn_cast(BB.getFirstNonPHI()))
+OpBundles.emplace_back("funclet", CleanupPad);
+
+ return CallInst::Create(CI, OpBundles);
+}
+}
+
/// Visit each call, one at a time, and make simplifications without doing any
/// additional analysis.
void ObjCARCOpt::OptimizeIndividualCalls(Function &F) {
@@ -927,9 +948,10 @@
Value *Incoming =
GetRCIdentityRoot(PN->getIncomingValue(i));
if (!IsNullOrUndef(Incoming)) {
- CallInst *Clone = cast(CInst->clone());
Value *Op = PN->getIncomingValue(i);
Instruction *InsertPos = &PN->getIncomingBlock(i)->back();
+ CallInst *Clone = cast(
+ CloneCallInstForBB(*CInst, *InsertPos->getParent()));
if (Op->getType() != ParamTy)
Op =
[PATCH] D44103: XFAIL: libcpp-no-deduction-guides in libcxx/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp
This revision was automatically updated to reflect the committed changes. Closed by commit rL327178: XFAIL: libcpp-no-deduction-guides in libcxx/test/std/strings/basic. (authored by sqlbyme, committed by ). Herald added subscribers: llvm-commits, christof. Changed prior to commit: https://reviews.llvm.org/D44103?vs=137104&id=137839#toc Repository: rL LLVM https://reviews.llvm.org/D44103 Files: libcxx/trunk/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp Index: libcxx/trunk/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp === --- libcxx/trunk/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp +++ libcxx/trunk/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp @@ -9,8 +9,7 @@ // // UNSUPPORTED: c++98, c++03, c++11, c++14 -// UNSUPPORTED: clang-3.3, clang-3.4, clang-3.5, clang-3.6, clang-3.7, clang-3.8, clang-3.9, clang-4.0 -// UNSUPPORTED: apple-clang-6, apple-clang-7, apple-clang-8.0 +// XFAIL: libcpp-no-deduction-guides // template::value_type>> Index: libcxx/trunk/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp === --- libcxx/trunk/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp +++ libcxx/trunk/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp @@ -9,8 +9,7 @@ // // UNSUPPORTED: c++98, c++03, c++11, c++14 -// UNSUPPORTED: clang-3.3, clang-3.4, clang-3.5, clang-3.6, clang-3.7, clang-3.8, clang-3.9, clang-4.0 -// UNSUPPORTED: apple-clang-6, apple-clang-7, apple-clang-8.0 +// XFAIL: libcpp-no-deduction-guides // template::value_type>> ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r327178 - XFAIL: libcpp-no-deduction-guides in libcxx/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp
Author: sqlbyme Date: Fri Mar 9 14:13:12 2018 New Revision: 327178 URL: http://llvm.org/viewvc/llvm-project?rev=327178&view=rev Log: XFAIL: libcpp-no-deduction-guides in libcxx/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp Summary: Refactor the previous version method of marking each apple-clang version as UNSUPPORTED and just XFAIL'ing the libcpp-no-deduction-guides instead. This brings this test inline with the same style as iter_alloc_deduction.pass.cpp Reviewers: EricWF, dexonsmith Reviewed By: EricWF Subscribers: EricWF, vsapsai, vsk, cfe-commits Differential Revision: https://reviews.llvm.org/D44103 Modified: libcxx/trunk/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp Modified: libcxx/trunk/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp?rev=327178&r1=327177&r2=327178&view=diff == --- libcxx/trunk/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp (original) +++ libcxx/trunk/test/std/strings/basic.string/string.cons/iter_alloc_deduction.fail.cpp Fri Mar 9 14:13:12 2018 @@ -9,8 +9,7 @@ // // UNSUPPORTED: c++98, c++03, c++11, c++14 -// UNSUPPORTED: clang-3.3, clang-3.4, clang-3.5, clang-3.6, clang-3.7, clang-3.8, clang-3.9, clang-4.0 -// UNSUPPORTED: apple-clang-6, apple-clang-7, apple-clang-8.0 +// XFAIL: libcpp-no-deduction-guides // template::value_type>> ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44327: ObjCARC: teach the cloner about funclets
majnemer added inline comments.
Comment at: lib/Transforms/ObjCARC/ObjCARCOpts.cpp:701
+
+ if (auto *CleanupPad = dyn_cast(BB.getFirstNonPHI()))
+OpBundles.emplace_back("funclet", CleanupPad);
What if the cleanuppad was introduced in a block which branched to this one?
Repository:
rC Clang
https://reviews.llvm.org/D44327
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43797: [CMake] Copy the generated __config header into build directory
compnerd accepted this revision.
compnerd added inline comments.
This revision is now accepted and ready to land.
Comment at: libcxx/include/CMakeLists.txt:19
+DEPENDS ${LIBCXX_BINARY_DIR}/__generated_config)
+ set(generated_config_deps generate_config_header)
+endif()
Where is `generated_config_deps` used?
Repository:
rCXX libc++
https://reviews.llvm.org/D43797
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44330: CMake option to allow enabling experimental new pass manager by default
phosek created this revision.
phosek added a reviewer: chandlerc.
Herald added subscribers: cfe-commits, mgorny.
This CMake flag allows setting the default value for the
-f[no]-experimental-new-pass-manager flag.
Repository:
rC Clang
https://reviews.llvm.org/D44330
Files:
clang/CMakeLists.txt
clang/include/clang/Config/config.h.cmake
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/lib/Frontend/CompilerInvocation.cpp
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -490,7 +490,7 @@
Opts.ExperimentalNewPassManager = Args.hasFlag(
OPT_fexperimental_new_pass_manager,
OPT_fno_experimental_new_pass_manager,
- /* Default */ false);
+ /* Default */ ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER);
Opts.DebugPassManager =
Args.hasFlag(OPT_fdebug_pass_manager, OPT_fno_debug_pass_manager,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -455,7 +455,7 @@
// Need this flag to turn on new pass manager via Gold plugin.
if (Args.hasFlag(options::OPT_fexperimental_new_pass_manager,
options::OPT_fno_experimental_new_pass_manager,
- /* Default */ false)) {
+ /* Default */ ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER)) {
CmdArgs.push_back("-plugin-opt=new-pass-manager");
}
Index: clang/include/clang/Config/config.h.cmake
===
--- clang/include/clang/Config/config.h.cmake
+++ clang/include/clang/Config/config.h.cmake
@@ -72,6 +72,9 @@
/* enable x86 relax relocations by default */
#cmakedefine01 ENABLE_X86_RELAX_RELOCATIONS
+/* Enable the experimental new pass manager by default */
+#cmakedefine01 ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER
+
/* Enable each functionality of modules */
#cmakedefine01 CLANG_ENABLE_ARCMT
#cmakedefine01 CLANG_ENABLE_OBJC_REWRITER
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -212,6 +212,9 @@
set(ENABLE_X86_RELAX_RELOCATIONS OFF CACHE BOOL
"enable x86 relax relocations by default")
+set(ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER FALSE CACHE BOOL
+ "Enable the experimental new pass manager by default.")
+
# TODO: verify the values against LangStandards.def?
set(CLANG_DEFAULT_STD_C "" CACHE STRING
"Default standard to use for C/ObjC code (IDENT from LangStandards.def,
empty for platform default)")
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -490,7 +490,7 @@
Opts.ExperimentalNewPassManager = Args.hasFlag(
OPT_fexperimental_new_pass_manager, OPT_fno_experimental_new_pass_manager,
- /* Default */ false);
+ /* Default */ ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER);
Opts.DebugPassManager =
Args.hasFlag(OPT_fdebug_pass_manager, OPT_fno_debug_pass_manager,
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -455,7 +455,7 @@
// Need this flag to turn on new pass manager via Gold plugin.
if (Args.hasFlag(options::OPT_fexperimental_new_pass_manager,
options::OPT_fno_experimental_new_pass_manager,
- /* Default */ false)) {
+ /* Default */ ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER)) {
CmdArgs.push_back("-plugin-opt=new-pass-manager");
}
Index: clang/include/clang/Config/config.h.cmake
===
--- clang/include/clang/Config/config.h.cmake
+++ clang/include/clang/Config/config.h.cmake
@@ -72,6 +72,9 @@
/* enable x86 relax relocations by default */
#cmakedefine01 ENABLE_X86_RELAX_RELOCATIONS
+/* Enable the experimental new pass manager by default */
+#cmakedefine01 ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER
+
/* Enable each functionality of modules */
#cmakedefine01 CLANG_ENABLE_ARCMT
#cmakedefine01 CLANG_ENABLE_OBJC_REWRITER
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -212,6 +212,9 @@
set(ENABLE_X86_RELAX_RELOCATIONS OFF CACHE BOOL
"enable x86 relax relocations by default")
+set(ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER FALSE CACHE BOOL
+ "Enable the experimental new pass manager by default.")
+
# TODO: verify the values against LangStandards.def?
set(CLANG_DEFAULT_STD_C "" CACHE STRING
"Default standard to use for C/ObjC co
[PATCH] D43797: [CMake] Copy the generated __config header into build directory
phosek added inline comments.
Comment at: libcxx/include/CMakeLists.txt:19
+DEPENDS ${LIBCXX_BINARY_DIR}/__generated_config)
+ set(generated_config_deps generate_config_header)
+endif()
compnerd wrote:
> Where is `generated_config_deps` used?
Line 69
Repository:
rCXX libc++
https://reviews.llvm.org/D43797
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r327183 - test: repair windows build after SVN r327105
Author: compnerd Date: Fri Mar 9 15:00:29 2018 New Revision: 327183 URL: http://llvm.org/viewvc/llvm-project?rev=327183&view=rev Log: test: repair windows build after SVN r327105 Thanks to Nico Weber for pointing out the failure. Add an explicit target for the test. Modified: cfe/trunk/test/CodeGenObjCXX/debug-info-line.mm Modified: cfe/trunk/test/CodeGenObjCXX/debug-info-line.mm URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/debug-info-line.mm?rev=327183&r1=327182&r2=327183&view=diff == --- cfe/trunk/test/CodeGenObjCXX/debug-info-line.mm (original) +++ cfe/trunk/test/CodeGenObjCXX/debug-info-line.mm Fri Mar 9 15:00:29 2018 @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -debug-info-kind=line-tables-only -fblocks -emit-llvm %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple x86_64-unknown-windows-gnu -fcxx-exceptions -fexceptions -debug-info-kind=line-tables-only -fblocks -emit-llvm %s -o - | FileCheck %s void fn(); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r327184 - [clangd-fuzzer] Update ClangdLSPServer constructor call.
Author: morehouse Date: Fri Mar 9 15:02:22 2018 New Revision: 327184 URL: http://llvm.org/viewvc/llvm-project?rev=327184&view=rev Log: [clangd-fuzzer] Update ClangdLSPServer constructor call. Build was broken by r326719. Modified: clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp Modified: clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp?rev=327184&r1=327183&r2=327184&view=diff == --- clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp (original) +++ clang-tools-extra/trunk/clangd/fuzzer/ClangdFuzzer.cpp Fri Mar 9 15:02:22 2018 @@ -14,6 +14,7 @@ //===--===// #include "ClangdLSPServer.h" +#include "ClangdServer.h" #include "CodeComplete.h" #include @@ -21,12 +22,10 @@ extern "C" int LLVMFuzzerTestOneInput(ui clang::clangd::JSONOutput Out(llvm::nulls(), llvm::nulls(), nullptr); clang::clangd::CodeCompleteOptions CCOpts; CCOpts.EnableSnippets = false; + clang::clangd::ClangdServer::Options Opts; // Initialize and run ClangdLSPServer. - clang::clangd::ClangdLSPServer LSPServer( - Out, clang::clangd::getDefaultAsyncThreadsCount(), - /*StorePreamblesInMemory=*/false, CCOpts, llvm::None, llvm::None, - /*BuildDynamicSymbolIndex=*/false); + clang::clangd::ClangdLSPServer LSPServer(Out, CCOpts, llvm::None, Opts); std::istringstream In(std::string(reinterpret_cast(data), size)); LSPServer.run(In); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r327186 - [clang-tidy] Update run-clang-tidy.py with config arg
Author: juliehockett
Date: Fri Mar 9 15:26:56 2018
New Revision: 327186
URL: http://llvm.org/viewvc/llvm-project?rev=327186&view=rev
Log:
[clang-tidy] Update run-clang-tidy.py with config arg
Updating the run-clang-tidy.py script to allow specification of the
config argument to the clang-tidy invocation.
Differential Revision: https://reviews.llvm.org/D43538
Modified:
clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
Modified: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py?rev=327186&r1=327185&r2=327186&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py Fri Mar 9
15:26:56 2018
@@ -75,7 +75,8 @@ def make_absolute(f, directory):
def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
-header_filter, extra_arg, extra_arg_before, quiet):
+header_filter, extra_arg, extra_arg_before, quiet,
+config):
"""Gets a command line for clang-tidy."""
start = [clang_tidy_binary]
if header_filter is not None:
@@ -99,6 +100,8 @@ def get_tidy_invocation(f, clang_tidy_bi
start.append('-p=' + build_path)
if quiet:
start.append('-quiet')
+ if config:
+ start.append('-config=' + config)
start.append(f)
return start
@@ -157,7 +160,7 @@ def run_tidy(args, tmpdir, build_path, q
invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks,
tmpdir, build_path, args.header_filter,
args.extra_arg, args.extra_arg_before,
- args.quiet)
+ args.quiet, args.config)
sys.stdout.write(' '.join(invocation) + '\n')
subprocess.call(invocation)
queue.task_done()
@@ -177,6 +180,14 @@ def main():
parser.add_argument('-checks', default=None,
help='checks filter, when not specified, use clang-tidy '
'default')
+ parser.add_argument('-config', default=None,
+ help='Specifies a configuration in YAML/JSON format: '
+ ' -config="{Checks: \'*\', '
+ ' CheckOptions: [{key: x, '
+ ' value: y}]}" '
+ 'When the value is empty, clang-tidy will '
+ 'attempt to find a file named .clang-tidy for '
+ 'each source file in its parent directories.')
parser.add_argument('-header-filter', default=None,
help='regular expression matching the names of the '
'headers to output diagnostics from. Diagnostics from '
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43538: [clang-tidy] Update run-clang-tidy.py with config arg
This revision was automatically updated to reflect the committed changes.
juliehockett marked an inline comment as done.
Closed by commit rCTE327186: [clang-tidy] Update run-clang-tidy.py with config
arg (authored by juliehockett, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D43538?vs=135174&id=137853#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D43538
Files:
clang-tidy/tool/run-clang-tidy.py
Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -75,7 +75,8 @@
def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
-header_filter, extra_arg, extra_arg_before, quiet):
+header_filter, extra_arg, extra_arg_before, quiet,
+config):
"""Gets a command line for clang-tidy."""
start = [clang_tidy_binary]
if header_filter is not None:
@@ -99,6 +100,8 @@
start.append('-p=' + build_path)
if quiet:
start.append('-quiet')
+ if config:
+ start.append('-config=' + config)
start.append(f)
return start
@@ -157,7 +160,7 @@
invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks,
tmpdir, build_path, args.header_filter,
args.extra_arg, args.extra_arg_before,
- args.quiet)
+ args.quiet, args.config)
sys.stdout.write(' '.join(invocation) + '\n')
subprocess.call(invocation)
queue.task_done()
@@ -177,6 +180,14 @@
parser.add_argument('-checks', default=None,
help='checks filter, when not specified, use clang-tidy '
'default')
+ parser.add_argument('-config', default=None,
+ help='Specifies a configuration in YAML/JSON format: '
+ ' -config="{Checks: \'*\', '
+ ' CheckOptions: [{key: x, '
+ ' value: y}]}" '
+ 'When the value is empty, clang-tidy will '
+ 'attempt to find a file named .clang-tidy for '
+ 'each source file in its parent directories.')
parser.add_argument('-header-filter', default=None,
help='regular expression matching the names of the '
'headers to output diagnostics from. Diagnostics from '
Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -75,7 +75,8 @@
def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
-header_filter, extra_arg, extra_arg_before, quiet):
+header_filter, extra_arg, extra_arg_before, quiet,
+config):
"""Gets a command line for clang-tidy."""
start = [clang_tidy_binary]
if header_filter is not None:
@@ -99,6 +100,8 @@
start.append('-p=' + build_path)
if quiet:
start.append('-quiet')
+ if config:
+ start.append('-config=' + config)
start.append(f)
return start
@@ -157,7 +160,7 @@
invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks,
tmpdir, build_path, args.header_filter,
args.extra_arg, args.extra_arg_before,
- args.quiet)
+ args.quiet, args.config)
sys.stdout.write(' '.join(invocation) + '\n')
subprocess.call(invocation)
queue.task_done()
@@ -177,6 +180,14 @@
parser.add_argument('-checks', default=None,
help='checks filter, when not specified, use clang-tidy '
'default')
+ parser.add_argument('-config', default=None,
+ help='Specifies a configuration in YAML/JSON format: '
+ ' -config="{Checks: \'*\', '
+ ' CheckOptions: [{key: x, '
+ ' value: y}]}" '
+ 'When the value is empty, clang-tidy will '
+ 'attempt to find a file named .clang-tidy for '
+ 'each source file in its parent directories.')
parser.add_argument('-header-filter', default=None,
help='regular expression matching the names of the '
'headers to output diagnostics from. Diagnostics from '
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r327189 - [ARM] Add ARMv8.2-A FP16 vector intrinsic
Author: az
Date: Fri Mar 9 15:39:34 2018
New Revision: 327189
URL: http://llvm.org/viewvc/llvm-project?rev=327189&view=rev
Log:
[ARM] Add ARMv8.2-A FP16 vector intrinsic
Add the fp16 neon vector intrinsic for ARM as described in the ARM ACLE
document.
Reviews in https://reviews.llvm.org/D43650
Added:
cfe/trunk/test/CodeGen/arm-v8.2a-neon-intrinsics.c
Modified:
cfe/trunk/include/clang/Basic/arm_neon.td
cfe/trunk/lib/Basic/Targets/ARM.cpp
cfe/trunk/lib/Basic/Targets/ARM.h
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/test/CodeGen/arm_neon_intrinsics.c
Modified: cfe/trunk/include/clang/Basic/arm_neon.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/arm_neon.td?rev=327189&r1=327188&r2=327189&view=diff
==
--- cfe/trunk/include/clang/Basic/arm_neon.td (original)
+++ cfe/trunk/include/clang/Basic/arm_neon.td Fri Mar 9 15:39:34 2018
@@ -1363,8 +1363,8 @@ def SCALAR_VDUP_LANE : IInst<"vdup_lane"
def SCALAR_VDUP_LANEQ : IInst<"vdup_laneq", "sji",
"ScSsSiSlSfSdSUcSUsSUiSUlSPcSPs">;
}
-// ARMv8.2-A FP16 intrinsics.
-let ArchGuard = "defined(__ARM_FEATURE_FP16_VECTOR_ARITHMETIC) &&
defined(__aarch64__)" in {
+// ARMv8.2-A FP16 vector intrinsics for A32/A64.
+let ArchGuard = "defined(__ARM_FEATURE_FP16_VECTOR_ARITHMETIC)" in {
// ARMv8.2-A FP16 one-operand vector intrinsics.
@@ -1395,14 +1395,12 @@ let ArchGuard = "defined(__ARM_FEATURE_F
def FRINTPH : SInst<"vrndp", "dd", "hQh">;
def FRINTMH : SInst<"vrndm", "dd", "hQh">;
def FRINTXH : SInst<"vrndx", "dd", "hQh">;
- def FRINTIH : SInst<"vrndi", "dd", "hQh">;
// Misc.
def VABSH: SInst<"vabs", "dd", "hQh">;
def VNEGH: SOpInst<"vneg", "dd", "hQh", OP_NEG>;
def VRECPEH : SInst<"vrecpe", "dd", "hQh">;
def FRSQRTEH : SInst<"vrsqrte", "dd", "hQh">;
- def FSQRTH : SInst<"vsqrt", "dd", "hQh">;
// ARMv8.2-A FP16 two-operands vector intrinsics.
@@ -1443,18 +1441,13 @@ let ArchGuard = "defined(__ARM_FEATURE_F
// Multiplication/Division
def VMULH : SOpInst<"vmul", "ddd", "hQh", OP_MUL>;
- def MULXH : SInst<"vmulx", "ddd", "hQh">;
- def FDIVH : IOpInst<"vdiv", "ddd", "hQh", OP_DIV>;
// Pairwise addition
- def VPADDH: SInst<"vpadd", "ddd", "hQh">;
+ def VPADDH: SInst<"vpadd", "ddd", "h">;
// Pairwise Max/Min
- def VPMAXH: SInst<"vpmax", "ddd", "hQh">;
- def VPMINH: SInst<"vpmin", "ddd", "hQh">;
- // Pairwise MaxNum/MinNum
- def FMAXNMPH : SInst<"vpmaxnm", "ddd", "hQh">;
- def FMINNMPH : SInst<"vpminnm", "ddd", "hQh">;
+ def VPMAXH: SInst<"vpmax", "ddd", "h">;
+ def VPMINH: SInst<"vpmin", "ddd", "h">;
// Reciprocal/Sqrt
def VRECPSH : SInst<"vrecps", "ddd", "hQh">;
@@ -1468,6 +1461,63 @@ let ArchGuard = "defined(__ARM_FEATURE_F
// ARMv8.2-A FP16 lane vector intrinsics.
+ // Mul lane
+ def VMUL_LANEH: IOpInst<"vmul_lane", "ddgi", "hQh", OP_MUL_LN>;
+ def VMUL_NH : IOpInst<"vmul_n", "dds", "hQh", OP_MUL_N>;
+
+ // Data processing intrinsics - section 5
+
+ // Logical operations
+ let isHiddenLInst = 1 in
+ def VBSLH: SInst<"vbsl", "dudd", "hQh">;
+
+ // Transposition operations
+ def VZIPH: WInst<"vzip", "2dd", "hQh">;
+ def VUZPH: WInst<"vuzp", "2dd", "hQh">;
+ def VTRNH: WInst<"vtrn", "2dd", "hQh">;
+
+
+ let ArchGuard = "!defined(__aarch64__)" in {
+// Set all lanes to same value.
+// Already implemented prior to ARMv8.2-A.
+def VMOV_NH : WOpInst<"vmov_n", "ds", "hQh", OP_DUP>;
+def VDUP_NH : WOpInst<"vdup_n", "ds", "hQh", OP_DUP>;
+def VDUP_LANE1H : WOpInst<"vdup_lane", "dgi", "hQh", OP_DUP_LN>;
+ }
+
+ // Vector Extract
+ def VEXTH : WInst<"vext", "dddi", "hQh">;
+
+ // Reverse vector elements
+ def VREV64H: WOpInst<"vrev64", "dd", "hQh", OP_REV64>;
+}
+
+// ARMv8.2-A FP16 vector intrinsics for A64 only.
+let ArchGuard = "defined(__ARM_FEATURE_FP16_VECTOR_ARITHMETIC) &&
defined(__aarch64__)" in {
+
+ // Vector rounding
+ def FRINTIH : SInst<"vrndi", "dd", "hQh">;
+
+ // Misc.
+ def FSQRTH : SInst<"vsqrt", "dd", "hQh">;
+
+ // Multiplication/Division
+ def MULXH : SInst<"vmulx", "ddd", "hQh">;
+ def FDIVH : IOpInst<"vdiv", "ddd", "hQh", OP_DIV>;
+
+ // Pairwise addition
+ def VPADDH1 : SInst<"vpadd", "ddd", "Qh">;
+
+ // Pairwise Max/Min
+ def VPMAXH1 : SInst<"vpmax", "ddd", "Qh">;
+ def VPMINH1 : SInst<"vpmin", "ddd", "Qh">;
+
+ // Pairwise MaxNum/MinNum
+ def FMAXNMPH : SInst<"vpmaxnm", "ddd", "hQh">;
+ def FMINNMPH : SInst<"vpminnm", "ddd", "hQh">;
+
+ // ARMv8.2-A FP16 lane vector intrinsics.
+
// FMA lane
def VFMA_LANEH : IInst<"vfma_lane", "dddgi", "hQh">;
def VFMA_LANEQH : IInst<"vfma_laneq", "dddji", "hQh">;
@@ -1488,
[PATCH] D43494: [Modules] Fix creating fake definition data for lambdas.
vsapsai added inline comments.
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:1811
if (Update || Canon != D) {
Canon->DefinitionData = D->DefinitionData;
Reader.PendingDefinitions.insert(D);
This store seems to be dead too. Need to spend more time in debugger to make
sure that the code works the way I think it works.
https://reviews.llvm.org/D43494
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44327: ObjCARC: teach the cloner about funclets
compnerd updated this revision to Diff 137867.
compnerd added a comment.
Use the BB colorizer to detect the token. Fortunately, there is no BB
removal/splitting happening here, so there is no state to maintain.
Repository:
rL LLVM
https://reviews.llvm.org/D44327
Files:
lib/Transforms/ObjCARC/ObjCARCOpts.cpp
test/Transforms/ObjCARC/funclet.ll
Index: test/Transforms/ObjCARC/funclet.ll
===
--- /dev/null
+++ test/Transforms/ObjCARC/funclet.ll
@@ -0,0 +1,112 @@
+; RUN: opt -mtriple x86_64-unknown-windows-msvc -objc-arc -S -o - %s | FileCheck %s
+
+; bool g();
+; id h();
+;
+; void f() {
+; id a = nullptr;
+; if (g())
+; a = h();
+; id b = nullptr;
+; g();
+; }
+
+declare zeroext i1 @"\01?g@@YA_NXZ"() local_unnamed_addr
+declare i8* @"\01?h@@YAPEAUobjc_object@@XZ"() local_unnamed_addr
+
+declare dllimport void @objc_release(i8*) local_unnamed_addr
+declare dllimport i8* @objc_retainAutoreleasedReturnValue(i8* returned) local_unnamed_addr
+
+declare i32 @__CxxFrameHandler3(...)
+
+define void @"\01?f@@YAXXZ"() local_unnamed_addr personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
+entry:
+ %call = invoke zeroext i1 @"\01?g@@YA_NXZ"()
+ to label %invoke.cont unwind label %ehcleanup6
+
+invoke.cont: ; preds = %entry
+ br i1 %call, label %if.then, label %if.end
+
+if.then: ; preds = %invoke.cont
+ %call2 = invoke i8* @"\01?h@@YAPEAUobjc_object@@XZ"()
+ to label %invoke.cont1 unwind label %ehcleanup6
+
+invoke.cont1: ; preds = %if.then
+ %0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %call2)
+ tail call void @objc_release(i8* null), !clang.imprecise_release !1
+ br label %if.end
+
+if.end: ; preds = %invoke.cont1, %invoke.cont
+ %a.0 = phi i8* [ %call2, %invoke.cont1 ], [ null, %invoke.cont ]
+ %call4 = invoke zeroext i1 @"\01?g@@YA_NXZ"()
+ to label %invoke.cont3 unwind label %ehcleanup
+
+invoke.cont3: ; preds = %if.end
+ tail call void @objc_release(i8* null), !clang.imprecise_release !1
+ tail call void @objc_release(i8* %a.0), !clang.imprecise_release !1
+ ret void
+
+ehcleanup:; preds = %if.end
+ %1 = cleanuppad within none []
+ call void @objc_release(i8* null) [ "funclet"(token %1) ], !clang.imprecise_release !1
+ cleanupret from %1 unwind label %ehcleanup6
+
+ehcleanup6: ; preds = %ehcleanup, %if.then, %entry
+ %a.1 = phi i8* [ %a.0, %ehcleanup ], [ null, %if.then ], [ null, %entry ]
+ %2 = cleanuppad within none []
+ call void @objc_release(i8* %a.1) [ "funclet"(token %2) ], !clang.imprecise_release !1
+ cleanupret from %2 unwind to caller
+}
+
+; CHECK-LABEL: ?f@@YAXXZ
+; CHECK: call void @objc_release(i8* {{.*}}) {{.*}}[ "funclet"(token %1) ]
+; CHECK-NOT: call void @objc_release(i8* {{.*}}) {{.*}}[ "funclet"(token %2) ]
+
+define void @"\01?i@@YAXXZ"() local_unnamed_addr personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
+entry:
+ %call = invoke zeroext i1 @"\01?g@@YA_NXZ"()
+ to label %invoke.cont unwind label %ehcleanup6
+
+invoke.cont: ; preds = %entry
+ br i1 %call, label %if.then, label %if.end
+
+if.then: ; preds = %invoke.cont
+ %call2 = invoke i8* @"\01?h@@YAPEAUobjc_object@@XZ"()
+ to label %invoke.cont1 unwind label %ehcleanup6
+
+invoke.cont1: ; preds = %if.then
+ %0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %call2)
+ tail call void @objc_release(i8* null), !clang.imprecise_release !1
+ br label %if.end
+
+if.end: ; preds = %invoke.cont1, %invoke.cont
+ %a.0 = phi i8* [ %call2, %invoke.cont1 ], [ null, %invoke.cont ]
+ %call4 = invoke zeroext i1 @"\01?g@@YA_NXZ"()
+ to label %invoke.cont3 unwind label %ehcleanup
+
+invoke.cont3: ; preds = %if.end
+ tail call void @objc_release(i8* null), !clang.imprecise_release !1
+ tail call void @objc_release(i8* %a.0), !clang.imprecise_release !1
+ ret void
+
+ehcleanup:; preds = %if.end
+ %1 = cleanuppad within none []
+ call void @objc_release(i8* null) [ "funclet"(token %1) ], !clang.imprecise_release !1
+ br label %ehcleanup.1
+
+ehcleanup.1:
+ cleanupret from %1 unwind label %ehcleanup6
+
+ehcleanup6: ; preds = %ehcleanup, %if.then, %entry
+ %a.1 = phi i8* [ %a.0, %ehcleanup.1 ], [ null, %if.then ], [ null, %entry ]
+ %2 = cleanuppad within none []
+ call void @objc_release(i8* %a.1) [ "funclet"(token %2) ], !clang.imprecise_release !1
+ cleanupret from %2 unwind to caller
+}
+
+; CHECK-LAB
