r355266 - [clang-format] clang-format off/on not respected when using C Style comments
Author: paulhoad
Date: Sat Mar 2 01:08:51 2019
New Revision: 355266
URL: http://llvm.org/viewvc/llvm-project?rev=355266&view=rev
Log:
[clang-format] clang-format off/on not respected when using C Style comments
Summary:
If the clang-format on/off is in a /* comment */ then the sorting of headers is
not ignored
PR40901 - https://bugs.llvm.org/show_bug.cgi?id=40901
Reviewers: djasper, klimek, JonasToth, krasimir, alexfh
Reviewed By: alexfh
Subscribers: alexfh, cfe-commits, llvm-commits
Tags: #clang, #clang-tools-extra
Differential Revision: https://reviews.llvm.org/D58819
Modified:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/SortIncludesTest.cpp
Modified: cfe/trunk/lib/Format/Format.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=355266&r1=355265&r2=355266&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Sat Mar 2 01:08:51 2019
@@ -1786,9 +1786,10 @@ tooling::Replacements sortCppIncludes(co
Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
StringRef Trimmed = Line.trim();
-if (Trimmed == "// clang-format off")
+if (Trimmed == "// clang-format off" || Trimmed == "/* clang-format off
*/")
FormattingOff = true;
-else if (Trimmed == "// clang-format on")
+else if (Trimmed == "// clang-format on" ||
+ Trimmed == "/* clang-format on */")
FormattingOff = false;
const bool EmptyLineSkipped =
Modified: cfe/trunk/unittests/Format/SortIncludesTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/SortIncludesTest.cpp?rev=355266&r1=355265&r2=355266&view=diff
==
--- cfe/trunk/unittests/Format/SortIncludesTest.cpp (original)
+++ cfe/trunk/unittests/Format/SortIncludesTest.cpp Sat Mar 2 01:08:51 2019
@@ -117,6 +117,43 @@ TEST_F(SortIncludesTest, SupportClangFor
"// clang-format on\n"));
}
+TEST_F(SortIncludesTest, SupportClangFormatOffCStyle) {
+ EXPECT_EQ("#include \n"
+"#include \n"
+"#include \n"
+"/* clang-format off */\n"
+"#include \n"
+"#include \n"
+"#include \n"
+"/* clang-format on */\n",
+sort("#include \n"
+ "#include \n"
+ "#include \n"
+ "/* clang-format off */\n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "/* clang-format on */\n"));
+
+ // Not really turning it off
+ EXPECT_EQ("#include \n"
+"#include \n"
+"#include \n"
+"/* clang-format offically */\n"
+"#include \n"
+"#include \n"
+"#include \n"
+"/* clang-format onwards */\n",
+sort("#include \n"
+ "#include \n"
+ "#include \n"
+ "/* clang-format offically */\n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "/* clang-format onwards */\n"));
+}
+
TEST_F(SortIncludesTest, IncludeSortingCanBeDisabled) {
FmtStyle.SortIncludes = false;
EXPECT_EQ("#include \"a.h\"\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58819: [clang-format] clang-format off/on not respected when using C Style comments
This revision was automatically updated to reflect the committed changes.
Closed by commit rL355266: [clang-format] clang-format off/on not respected
when using C Style comments (authored by paulhoad, committed by ).
Herald added a project: LLVM.
Changed prior to commit:
https://reviews.llvm.org/D58819?vs=188931&id=189039#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58819/new/
https://reviews.llvm.org/D58819
Files:
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/unittests/Format/SortIncludesTest.cpp
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -1786,9 +1786,10 @@
Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
StringRef Trimmed = Line.trim();
-if (Trimmed == "// clang-format off")
+if (Trimmed == "// clang-format off" || Trimmed == "/* clang-format off
*/")
FormattingOff = true;
-else if (Trimmed == "// clang-format on")
+else if (Trimmed == "// clang-format on" ||
+ Trimmed == "/* clang-format on */")
FormattingOff = false;
const bool EmptyLineSkipped =
Index: cfe/trunk/unittests/Format/SortIncludesTest.cpp
===
--- cfe/trunk/unittests/Format/SortIncludesTest.cpp
+++ cfe/trunk/unittests/Format/SortIncludesTest.cpp
@@ -117,6 +117,43 @@
"// clang-format on\n"));
}
+TEST_F(SortIncludesTest, SupportClangFormatOffCStyle) {
+ EXPECT_EQ("#include \n"
+"#include \n"
+"#include \n"
+"/* clang-format off */\n"
+"#include \n"
+"#include \n"
+"#include \n"
+"/* clang-format on */\n",
+sort("#include \n"
+ "#include \n"
+ "#include \n"
+ "/* clang-format off */\n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "/* clang-format on */\n"));
+
+ // Not really turning it off
+ EXPECT_EQ("#include \n"
+"#include \n"
+"#include \n"
+"/* clang-format offically */\n"
+"#include \n"
+"#include \n"
+"#include \n"
+"/* clang-format onwards */\n",
+sort("#include \n"
+ "#include \n"
+ "#include \n"
+ "/* clang-format offically */\n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "/* clang-format onwards */\n"));
+}
+
TEST_F(SortIncludesTest, IncludeSortingCanBeDisabled) {
FmtStyle.SortIncludes = false;
EXPECT_EQ("#include \"a.h\"\n"
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -1786,9 +1786,10 @@
Code.substr(Prev, (Pos != StringRef::npos ? Pos : Code.size()) - Prev);
StringRef Trimmed = Line.trim();
-if (Trimmed == "// clang-format off")
+if (Trimmed == "// clang-format off" || Trimmed == "/* clang-format off */")
FormattingOff = true;
-else if (Trimmed == "// clang-format on")
+else if (Trimmed == "// clang-format on" ||
+ Trimmed == "/* clang-format on */")
FormattingOff = false;
const bool EmptyLineSkipped =
Index: cfe/trunk/unittests/Format/SortIncludesTest.cpp
===
--- cfe/trunk/unittests/Format/SortIncludesTest.cpp
+++ cfe/trunk/unittests/Format/SortIncludesTest.cpp
@@ -117,6 +117,43 @@
"// clang-format on\n"));
}
+TEST_F(SortIncludesTest, SupportClangFormatOffCStyle) {
+ EXPECT_EQ("#include \n"
+"#include \n"
+"#include \n"
+"/* clang-format off */\n"
+"#include \n"
+"#include \n"
+"#include \n"
+"/* clang-format on */\n",
+sort("#include \n"
+ "#include \n"
+ "#include \n"
+ "/* clang-format off */\n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "/* clang-format on */\n"));
+
+ // Not really turning it off
+ EXPECT_EQ("#include \n"
+"#include \n"
+"#include \n"
+"/* clang-format offically */\n"
+"#include \n"
+"#include \n"
+"#include \n"
+"/* clang-format onwards */\n",
+sort("#include \n"
+ "#include \n"
+ "#include \n"
+ "/* clang-format offically */\n"
+ "#include \n"
+ "#include \n"
+ "#include \n"
+ "/* clang-format onwards */\n"));
+}
+
TEST_F(S
[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives
to-miz updated this revision to Diff 189048.
to-miz added a comment.
Added an entry to ReleaseNotes.rst about the new option.
This diff doesn't contain unrelated formatting changes due to running
clang-format on some source files that apparently weren't formatted before.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D52150/new/
https://reviews.llvm.org/D52150
Files:
docs/ClangFormatStyleOptions.rst
docs/ReleaseNotes.rst
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/TokenAnnotator.cpp
lib/Format/UnwrappedLineFormatter.cpp
lib/Format/UnwrappedLineParser.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1812,8 +1812,8 @@
Style.CompactNamespaces = true;
verifyFormat("namespace A { namespace B {\n"
- "}} // namespace A::B",
- Style);
+ "}} // namespace A::B",
+ Style);
EXPECT_EQ("namespace out { namespace in {\n"
"}} // namespace out::in",
@@ -2953,22 +2953,25 @@
EXPECT_EQ(Expected, format(ToFormat, Style));
EXPECT_EQ(Expected, format(Expected, Style));
}
- // Test with tabs.
- Style.UseTab = FormatStyle::UT_Always;
- Style.IndentWidth = 8;
- Style.TabWidth = 8;
- verifyFormat("#ifdef _WIN32\n"
- "#\tdefine A 0\n"
- "#\tifdef VAR2\n"
- "#\t\tdefine B 1\n"
- "#\t\tinclude \n"
- "#\t\tdefine MACRO \\\n"
- "\t\t\tsome_very_long_func_aa();\n"
- "#\tendif\n"
- "#else\n"
- "#\tdefine A 1\n"
- "#endif",
- Style);
+ // Test AfterHash with tabs.
+ {
+FormatStyle Tabbed = Style;
+Tabbed.UseTab = FormatStyle::UT_Always;
+Tabbed.IndentWidth = 8;
+Tabbed.TabWidth = 8;
+verifyFormat("#ifdef _WIN32\n"
+ "#\tdefine A 0\n"
+ "#\tifdef VAR2\n"
+ "#\t\tdefine B 1\n"
+ "#\t\tinclude \n"
+ "#\t\tdefine MACRO \\\n"
+ "\t\t\tsome_very_long_func_aa();\n"
+ "#\tendif\n"
+ "#else\n"
+ "#\tdefine A 1\n"
+ "#endif",
+ Tabbed);
+ }
// Regression test: Multiline-macro inside include guards.
verifyFormat("#ifndef HEADER_H\n"
@@ -2978,6 +2981,102 @@
" int j;\n"
"#endif // HEADER_H",
getLLVMStyleWithColumns(20));
+
+ Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash;
+ // Basic before hash indent tests
+ verifyFormat("#ifdef _WIN32\n"
+ " #define A 0\n"
+ " #ifdef VAR2\n"
+ "#define B 1\n"
+ "#include \n"
+ "#define MACRO \\\n"
+ " some_very_long_func_aa();\n"
+ " #endif\n"
+ "#else\n"
+ " #define A 1\n"
+ "#endif",
+ Style);
+ verifyFormat("#if A\n"
+ " #define MACRO\\\n"
+ "void a(int x) {\\\n"
+ " b(); \\\n"
+ " c(); \\\n"
+ " d(); \\\n"
+ " e(); \\\n"
+ " f(); \\\n"
+ "}\n"
+ "#endif",
+ Style);
+ // Keep comments aligned with indented directives. These
+ // tests cannot use verifyFormat because messUp manipulates leading
+ // whitespace.
+ {
+const char *Expected = "void f() {\n"
+ "// Aligned to preprocessor.\n"
+ "#if 1\n"
+ " // Aligned to code.\n"
+ " int a;\n"
+ " #if 1\n"
+ "// Aligned to preprocessor.\n"
+ "#define A 0\n"
+ " // Aligned to code.\n"
+ " int b;\n"
+ " #endif\n"
+ "#endif\n"
+ "}";
+const char *ToFormat = "void f() {\n"
+ "// Aligned to preprocessor.\n"
+ "#if 1\n"
+ "// Aligned to code.\n"
+ "int a;\n"
+ "#if 1\n"
+ "// Aligned to preprocessor.\n"
+ "#define A 0\n"
+ "// Aligned to code.\n"
+ "int b;\n"
+ "#endif\n"
+ "#endif\n
[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. Thank you for helping to address one of the many clang-format issues from bugzilla, I'm not the code owner here but this looks good to me. If we want to address the many issues we need to be able to move forwards. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 189054.
lewmpk added a comment.
refactor and improved tests
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58818/new/
https://reviews.llvm.org/D58818
Files:
clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp
clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-use-raii-locks.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp
Index: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp
@@ -0,0 +1,194 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-use-raii-locks %t
+
+// Mock implementation of std::mutex
+namespace std {
+struct mutex {
+ void lock();
+ void try_lock();
+ void unlock();
+};
+typedef mutex recursive_mutex;
+} // namespace std
+
+void warn_me1_simple() {
+ std::mutex m;
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+}
+
+void warn_me2_nested() {
+ std::mutex m;
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ {
+std::mutex m;
+m.lock();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII
+m.unlock();
+ }
+ m.unlock();
+}
+
+void warn_me3_nested_extra() {
+ std::mutex m1;
+ std::mutex m2;
+ {
+m1.lock();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII
+{
+ m2.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use RAII
+ m2.unlock();
+}
+m1.unlock();
+ }
+}
+
+void warn_me4_multi_mutex() {
+ std::mutex m1;
+ std::mutex m2;
+
+ m1.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m1.unlock();
+ m2.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m2.unlock();
+}
+
+void warn_me5_multi_mutex_extra() {
+ std::mutex m1;
+ std::mutex m2;
+
+ m1.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m2.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m1.unlock();
+ m2.unlock();
+}
+
+void warn_me6() {
+ std::mutex m;
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+ m.try_lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+}
+
+void warn_me7_loop() {
+ std::mutex m;
+ for (auto i = 0; i < 3; i++) {
+m.lock();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII
+m.unlock();
+ }
+}
+
+template
+void attempt(M m) {
+ m.lock();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+}
+
+void warn_me8_template_func() {
+ std::mutex m;
+ attempt(m);
+}
+
+// clang-format off
+#define ATTEMPT(m) {\
+m.lock();\
+m.unlock();\
+ }
+// clang-format on
+
+void warn_me9_macro() {
+ std::mutex m;
+ ATTEMPT(m);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use RAII
+}
+
+void warn_me10_inline() {
+ std::mutex m;
+ // clang-format off
+ m.lock(); m.unlock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ // clang-format on
+}
+
+void warn_me11_recursive_mutex() {
+ std::recursive_mutex m;
+ m.lock();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+}
+
+void ignore_me1_rev_order() {
+ std::mutex m;
+ m.unlock();
+ m.lock();
+ // CHECK-MESSAGES-NOT: warning:
+}
+
+void ignore_me2_diff_mtx() {
+ std::mutex m1;
+ std::mutex m2;
+ m1.lock();
+ // CHECK-MESSAGES-NOT: warning:
+ m2.unlock();
+}
+
+void ignore_me3() {
+ std::recursive_mutex m1;
+ std::recursive_mutex m2;
+ m1.lock();
+ // CHECK-MESSAGES-NOT: warning:
+ m2.unlock();
+}
+
+void ignore_me4() {
+ std::mutex m;
+ m.unlock();
+ m.lock();
+ m.lock();
+ // CHECK-MESSAGES-NOT: warning:
+}
+
+void ignore_me5_inline() {
+ std::mutex m;
+ m.unlock();
+ m.lock();
+ // CHECK-MESSAGES-NOT: warning:
+}
+
+void ignore_me6_seperate_scopes() {
+ std::mutex m;
+ {
+m.lock();
+// CHECK-MESSAGES-NOT: warning:
+ }
+ {
+m.unlock();
+ }
+}
+
+void ignore_me7_seperate_scopes_nested() {
+ std::mutex m;
+ {
+m.lock();
+// CHECK-MESSAGES-NOT: warning:
+{
+ m.unlock();
+}
+ }
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
cppcoreguidelines-pro-type-vararg
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
+ cppcoreguidelines-use-raii-locks
fuchsia-default-arguments
fuchsia-header-anon-namespaces (redirects to google-build-namespaces)
fuchsia-multiple-inherit
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk updated this revision to Diff 189055.
lewmpk added a comment.
added support for boost lockable types
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58818/new/
https://reviews.llvm.org/D58818
Files:
clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp
clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.h
docs/ReleaseNotes.rst
docs/clang-tidy/checks/cppcoreguidelines-use-raii-locks.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp
Index: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp
@@ -0,0 +1,194 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-use-raii-locks %t
+
+// Mock implementation of std::mutex
+namespace std {
+struct mutex {
+ void lock();
+ void try_lock();
+ void unlock();
+};
+typedef mutex recursive_mutex;
+} // namespace std
+
+void warn_me1_simple() {
+ std::mutex m;
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+}
+
+void warn_me2_nested() {
+ std::mutex m;
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ {
+std::mutex m;
+m.lock();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII
+m.unlock();
+ }
+ m.unlock();
+}
+
+void warn_me3_nested_extra() {
+ std::mutex m1;
+ std::mutex m2;
+ {
+m1.lock();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII
+{
+ m2.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use RAII
+ m2.unlock();
+}
+m1.unlock();
+ }
+}
+
+void warn_me4_multi_mutex() {
+ std::mutex m1;
+ std::mutex m2;
+
+ m1.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m1.unlock();
+ m2.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m2.unlock();
+}
+
+void warn_me5_multi_mutex_extra() {
+ std::mutex m1;
+ std::mutex m2;
+
+ m1.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m2.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m1.unlock();
+ m2.unlock();
+}
+
+void warn_me6() {
+ std::mutex m;
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+ m.try_lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.lock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+}
+
+void warn_me7_loop() {
+ std::mutex m;
+ for (auto i = 0; i < 3; i++) {
+m.lock();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII
+m.unlock();
+ }
+}
+
+template
+void attempt(M m) {
+ m.lock();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+}
+
+void warn_me8_template_func() {
+ std::mutex m;
+ attempt(m);
+}
+
+// clang-format off
+#define ATTEMPT(m) {\
+m.lock();\
+m.unlock();\
+ }
+// clang-format on
+
+void warn_me9_macro() {
+ std::mutex m;
+ ATTEMPT(m);
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use RAII
+}
+
+void warn_me10_inline() {
+ std::mutex m;
+ // clang-format off
+ m.lock(); m.unlock();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use RAII
+ // clang-format on
+}
+
+void warn_me11_recursive_mutex() {
+ std::recursive_mutex m;
+ m.lock();
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: use RAII
+ m.unlock();
+}
+
+void ignore_me1_rev_order() {
+ std::mutex m;
+ m.unlock();
+ m.lock();
+ // CHECK-MESSAGES-NOT: warning:
+}
+
+void ignore_me2_diff_mtx() {
+ std::mutex m1;
+ std::mutex m2;
+ m1.lock();
+ // CHECK-MESSAGES-NOT: warning:
+ m2.unlock();
+}
+
+void ignore_me3() {
+ std::recursive_mutex m1;
+ std::recursive_mutex m2;
+ m1.lock();
+ // CHECK-MESSAGES-NOT: warning:
+ m2.unlock();
+}
+
+void ignore_me4() {
+ std::mutex m;
+ m.unlock();
+ m.lock();
+ m.lock();
+ // CHECK-MESSAGES-NOT: warning:
+}
+
+void ignore_me5_inline() {
+ std::mutex m;
+ m.unlock();
+ m.lock();
+ // CHECK-MESSAGES-NOT: warning:
+}
+
+void ignore_me6_seperate_scopes() {
+ std::mutex m;
+ {
+m.lock();
+// CHECK-MESSAGES-NOT: warning:
+ }
+ {
+m.unlock();
+ }
+}
+
+void ignore_me7_seperate_scopes_nested() {
+ std::mutex m;
+ {
+m.lock();
+// CHECK-MESSAGES-NOT: warning:
+{
+ m.unlock();
+}
+ }
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
cppcoreguidelines-pro-type-vararg
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
+ cppcoreguidelines-use-raii-locks
fuchsia-default-arguments
fuchsia-header-anon-namespaces (redirects to google-build-namespaces)
fuchsia-multi
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
lewmpk marked 7 inline comments as done.
lewmpk added inline comments.
Comment at: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp:4
+// Mock implementation of std::mutex
+namespace std {
+struct mutex {
JonasToth wrote:
> Please add more tests
>
> What happens for this?
> ```
> void foo() {
> std::mutex m;
> m.lock();
> m.unlock();
> m.lock();
> m.unlock();
> m.try_lock();
> m.lock();
> m.unlock();
> }
> ```
>
> - Please add tests for templates, where the lock-type is a template parameter
> - please add tests where the locking happens within macros
> - please add tests for usage within loops
> - where cases like `std::mutex m1; std::mutex &m2 = m1; // usage`. This
> should not be diagnosed, right?
I've added a test case for your example, templates, macros and loops.
I can't catch the case `std::mutex m1; std::mutex &m2 = m1; // usage`, but i
can catch trivial cases.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58818/new/
https://reviews.llvm.org/D58818
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r355278 - Make the new SanitizerMask code added in r355190 constexpr.
Author: jyknight
Date: Sat Mar 2 12:22:48 2019
New Revision: 355278
URL: http://llvm.org/viewvc/llvm-project?rev=355278&view=rev
Log:
Make the new SanitizerMask code added in r355190 constexpr.
Then, as a consequence, remove the complex set of workarounds for
initialization order -- which are apparently not 100% reliable.
The only downside is that some of the member functions are now
specific to kNumElem == 2, and will need to be updated if that
constant is increased in the future.
Unfortunately, the current code caused an initialization-order runtime
failure for me in some compilation modes. It appears that in a
toolchain without init-array enabled, the order of initialization of
static data members of a template can be reversed w.r.t. the order
within a file.
This caused e.g. SanitizerKind::CFI to be initialized to 0.
I'm not quite sure if that is an allowable ordering variation, or
nonconforming behavior, but in any case, making everything constexpr
eliminates the possibility of such an issue.
Modified:
cfe/trunk/include/clang/Basic/Sanitizers.h
cfe/trunk/lib/Basic/Sanitizers.cpp
Modified: cfe/trunk/include/clang/Basic/Sanitizers.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Sanitizers.h?rev=355278&r1=355277&r2=355278&view=diff
==
--- cfe/trunk/include/clang/Basic/Sanitizers.h (original)
+++ cfe/trunk/include/clang/Basic/Sanitizers.h Sat Mar 2 12:22:48 2019
@@ -27,6 +27,10 @@ class hash_code;
namespace clang {
class SanitizerMask {
+ // NOTE: this class assumes kNumElem == 2 in most of the constexpr functions,
+ // in order to work within the C++11 constexpr function constraints. If you
+ // change kNumElem, you'll need to update those member functions as well.
+
/// Number of array elements.
static constexpr unsigned kNumElem = 2;
/// Mask value initialized to 0.
@@ -36,17 +40,22 @@ class SanitizerMask {
/// Number of bits in a mask element.
static constexpr unsigned kNumBitElem = sizeof(decltype(maskLoToHigh[0])) *
8;
+ constexpr SanitizerMask(uint64_t mask1, uint64_t mask2)
+ : maskLoToHigh{mask1, mask2} {}
+
public:
+ SanitizerMask() = default;
+
static constexpr bool checkBitPos(const unsigned Pos) {
return Pos < kNumBits;
}
/// Create a mask with a bit enabled at position Pos.
- static SanitizerMask bitPosToMask(const unsigned Pos) {
-assert(Pos < kNumBits && "Bit position too big.");
-SanitizerMask mask;
-mask.maskLoToHigh[Pos / kNumBitElem] = 1ULL << Pos % kNumBitElem;
-return mask;
+ static constexpr SanitizerMask bitPosToMask(const unsigned Pos) {
+return SanitizerMask((Pos < kNumBitElem) ? 1ULL << Pos % kNumBitElem : 0,
+ (Pos >= kNumBitElem && Pos < kNumBitElem * 2)
+ ? 1ULL << Pos % kNumBitElem
+ : 0);
}
unsigned countPopulation() const {
@@ -67,19 +76,13 @@ public:
llvm::hash_code hash_value() const;
- explicit operator bool() const {
-for (const auto &Val : maskLoToHigh)
- if (Val)
-return true;
-return false;
- };
+ constexpr explicit operator bool() const {
+return maskLoToHigh[0] || maskLoToHigh[1];
+ }
- bool operator==(const SanitizerMask &V) const {
-for (unsigned k = 0; k < kNumElem; k++) {
- if (maskLoToHigh[k] != V.maskLoToHigh[k])
-return false;
-}
-return true;
+ constexpr bool operator==(const SanitizerMask &V) const {
+return maskLoToHigh[0] == V.maskLoToHigh[0] &&
+ maskLoToHigh[1] == V.maskLoToHigh[1];
}
SanitizerMask &operator&=(const SanitizerMask &RHS) {
@@ -94,42 +97,35 @@ public:
return *this;
}
- bool operator!() const {
-for (const auto &Val : maskLoToHigh)
- if (Val)
-return false;
-return true;
+ constexpr bool operator!() const { return !bool(*this); }
+
+ constexpr bool operator!=(const SanitizerMask &RHS) const {
+return !((*this) == RHS);
}
- bool operator!=(const SanitizerMask &RHS) const { return !((*this) == RHS); }
+ friend constexpr inline SanitizerMask operator~(SanitizerMask v) {
+return SanitizerMask(~v.maskLoToHigh[0], ~v.maskLoToHigh[1]);
+ }
+
+ friend constexpr inline SanitizerMask operator&(SanitizerMask a,
+ const SanitizerMask &b) {
+return SanitizerMask(a.maskLoToHigh[0] & b.maskLoToHigh[0],
+ a.maskLoToHigh[1] & b.maskLoToHigh[1]);
+ }
+
+ friend constexpr inline SanitizerMask operator|(SanitizerMask a,
+ const SanitizerMask &b) {
+return SanitizerMask(a.maskLoToHigh[0] | b.maskLoToHigh[0],
+ a.maskLoToHigh[1] | b.maskLoToHigh[1]);
+ }
};
// Declaring in clang namespace so that it can be found by ADL.
llvm::hash_code hash_value(const clang::SanitizerMask &Arg);
-inline San
[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)
joerg added a comment. Can you include a patch for something like (int *)0xdeadbeef on amd64? That's a valid value for "n", but clearly too large for int. Thanks for looking at this, it is one of the two large remaining show stoppers for the asm constraint check. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58821/new/ https://reviews.llvm.org/D58821 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.
jyknight added a comment. The intricate initialization-order workarounds apparently don't work in all build modes, so I've updated this code to have constexpr functions and initializations in r355278. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57914/new/ https://reviews.llvm.org/D57914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
JonasToth added inline comments.
Comment at: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp:4
+// Mock implementation of std::mutex
+namespace std {
+struct mutex {
lewmpk wrote:
> JonasToth wrote:
> > Please add more tests
> >
> > What happens for this?
> > ```
> > void foo() {
> > std::mutex m;
> > m.lock();
> > m.unlock();
> > m.lock();
> > m.unlock();
> > m.try_lock();
> > m.lock();
> > m.unlock();
> > }
> > ```
> >
> > - Please add tests for templates, where the lock-type is a template
> > parameter
> > - please add tests where the locking happens within macros
> > - please add tests for usage within loops
> > - where cases like `std::mutex m1; std::mutex &m2 = m1; // usage`. This
> > should not be diagnosed, right?
> I've added a test case for your example, templates, macros and loops.
> I can't catch the case `std::mutex m1; std::mutex &m2 = m1; // usage`, but i
> can catch trivial cases.
Yes, your not supposed to catch those. But i feel things like this should be
documented. In theory catching this particular case is possible (we do similar
analysis for `const`.
But it is totally acceptable to leave as is!
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58818/new/
https://reviews.llvm.org/D58818
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
JonasToth added inline comments.
Comment at: test/clang-tidy/cppcoreguidelines-use-raii-locks.cpp:90
+ for (auto i = 0; i < 3; i++) {
+m.lock();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use RAII
I got another one that I think defeats the analysis:
```
while (true)
{
my_label:
m.lock();
if (some_condition)
goto my_label;
m.unlock();
}
```
Usage of `goto` can interfer and make the situation more complicated. I am not
asking you to implement that correctly (not sure if even possible with pure
clang-tidy) but I think a pathological example should be documented for the
user.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58818/new/
https://reviews.llvm.org/D58818
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r355279 - Tweak r355278 for compatibility with gcc 6 and earlier.
Author: jyknight
Date: Sat Mar 2 13:20:30 2019
New Revision: 355279
URL: http://llvm.org/viewvc/llvm-project?rev=355279&view=rev
Log:
Tweak r355278 for compatibility with gcc 6 and earlier.
Modified:
cfe/trunk/lib/Basic/Sanitizers.cpp
Modified: cfe/trunk/lib/Basic/Sanitizers.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Sanitizers.cpp?rev=355279&r1=355278&r2=355279&view=diff
==
--- cfe/trunk/lib/Basic/Sanitizers.cpp (original)
+++ cfe/trunk/lib/Basic/Sanitizers.cpp Sat Mar 2 13:20:30 2019
@@ -20,8 +20,8 @@ using namespace clang;
// won't need this.
#define SANITIZER(NAME, ID) const SanitizerMask SanitizerKind::ID;
#define SANITIZER_GROUP(NAME, ID, ALIAS)
\
- const SanitizerMask SanitizerKind::ID;
\
- const SanitizerMask SanitizerKind::ID##Group;
+ constexpr SanitizerMask SanitizerKind::ID;
\
+ constexpr SanitizerMask SanitizerKind::ID##Group;
#include "clang/Basic/Sanitizers.def"
SanitizerMask clang::parseSanitizerValue(StringRef Value, bool AllowGroups) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58878: [Diagnostics] Warn for assignments in bool contexts
xbolva00 created this revision.
xbolva00 added reviewers: RKSimon, aaron.ballman.
Herald added subscribers: cfe-commits, jdoerfert.
Herald added a project: clang.
Follow up for (abandoned) patch https://reviews.llvm.org/D45401
Fixes https://bugs.llvm.org/show_bug.cgi?id=34180
Repository:
rC Clang
https://reviews.llvm.org/D58878
Files:
clang/lib/Sema/SemaExprCXX.cpp
clang/test/Sema/assigment_in_bool_context.cpp
Index: clang/test/Sema/assigment_in_bool_context.cpp
===
--- /dev/null
+++ clang/test/Sema/assigment_in_bool_context.cpp
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -Wparentheses -fsyntax-only -verify %s
+
+bool warn1(int x) {
+ return x = 0; // expected-warning {{using the result of an assignment as a
condition without parentheses}}
+// expected-note@-1 {{place parentheses around the assignment
to silence this warning}}
+// expected-note@-2 {{use '==' to turn this assignment into an
equality comparison}}
+}
+
+bool warn2(int x, bool a, bool b) {
+ return x = 0 || (a && b); // expected-warning {{using the result of an
assignment as a condition without parentheses}}
+// expected-note@-1 {{place parentheses around the
assignment to silence this warning}}
+// expected-note@-2 {{use '==' to turn this
assignment into an equality comparison}}
+}
+
+bool warn3(int x, bool a) {
+ return x = 0 || a; // expected-warning {{using the result of an assignment
as a condition without parentheses}}
+ // expected-note@-1 {{place parentheses around the
assignment to silence this warning}}
+ // expected-note@-2 {{use '==' to turn this assignment
into an equality comparison}}
+}
+
+bool warn4(int x) {
+ return bool(x = 0); // expected-warning {{using the result of an assignment
as a condition without parentheses}}
+ // expected-note@-1 {{place parentheses around the
assignment to silence this warning}}
+ // expected-note@-2 {{use '==' to turn this assignment
into an equality comparison}}
+}
+
+int warn5(int x) {
+ return bool(x = 0); // expected-warning {{using the result of an assignment
as a condition without parentheses}}
+ // expected-note@-1 {{place parentheses around the
assignment to silence this warning}}
+ // expected-note@-2 {{use '==' to turn this assignment
into an equality comparison}}
+}
+
+bool warn6(int x, int a) {
+ return x = a; // expected-warning {{using the result of an assignment as a
condition without parentheses}}
+// expected-note@-1 {{place parentheses around the assignment
to silence this warning}}
+// expected-note@-2 {{use '==' to turn this assignment into an
equality comparison}}
+}
+
+int nowarn1(int x) {
+ return (x = 0);
+}
+
+int nowarn2(int x) {
+ return x = 0;
+}
+
+int nowarn3(int x) {
+ return x == 0;
+}
+
+bool nowarn4(int x) {
+ return x == 0;
+}
+
+void nowarn5(int x) {
+ return void(x = 0);
+}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -4136,6 +4136,7 @@
}
case ICK_Boolean_Conversion:
+DiagnoseAssignmentAsCondition(From->IgnoreImpCasts());
// Perform half-to-boolean conversion via float.
if (From->getType()->isHalfType()) {
From = ImpCastExprToType(From, Context.FloatTy, CK_FloatingCast).get();
Index: clang/test/Sema/assigment_in_bool_context.cpp
===
--- /dev/null
+++ clang/test/Sema/assigment_in_bool_context.cpp
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -Wparentheses -fsyntax-only -verify %s
+
+bool warn1(int x) {
+ return x = 0; // expected-warning {{using the result of an assignment as a condition without parentheses}}
+// expected-note@-1 {{place parentheses around the assignment to silence this warning}}
+// expected-note@-2 {{use '==' to turn this assignment into an equality comparison}}
+}
+
+bool warn2(int x, bool a, bool b) {
+ return x = 0 || (a && b); // expected-warning {{using the result of an assignment as a condition without parentheses}}
+// expected-note@-1 {{place parentheses around the assignment to silence this warning}}
+// expected-note@-2 {{use '==' to turn this assignment into an equality comparison}}
+}
+
+bool warn3(int x, bool a) {
+ return x = 0 || a; // expected-warning {{using the result of an assignment as a condition without parentheses}}
+ // expected-note@-1 {{place parentheses around the assignment to silence this warning}}
+ // expected-note@-2 {{use '==' to turn this assignment into an equality comparison}}
+}
+
+bool warn4(int x) {
+ return bool(x = 0); /
[PATCH] D58818: [clang-tidy] added cppcoreguidelines-use-raii-locks check
Eugene.Zelenko added inline comments.
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:29
+
+DeclRefExpr *findDeclRefExpr(const CXXMemberCallExpr *MemberCallExpr) {
+ auto *ObjectExpr = MemberCallExpr->getImplicitObjectArgument();
Please use static for functions instead of anonymous namespaces. See LLVM
coding guidelines.
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:30
+DeclRefExpr *findDeclRefExpr(const CXXMemberCallExpr *MemberCallExpr) {
+ auto *ObjectExpr = MemberCallExpr->getImplicitObjectArgument();
+ if (auto *ObjectCast = dyn_cast(ObjectExpr)) {
Please don't use auto where return type is not obvious.
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:44
+void UseRaiiLocksCheck::registerMatchers(MatchFinder *Finder) {
+
+ if (!getLangOpts().CPlusPlus)
Unnecessary empty line.
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:72
+void UseRaiiLocksCheck::check(const MatchFinder::MatchResult &Result) {
+
+ const auto *LockCallExpr = Result.Nodes.getNodeAs("lock");
Unnecessary empty line.
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:77
+
+ const auto *LockDeclRef = findDeclRefExpr(LockCallExpr);
+ const auto *UnlockDeclRef = findDeclRefExpr(UnlockCallExpr);
Please don't use auto where return type is not obvious.
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:78
+ const auto *LockDeclRef = findDeclRefExpr(LockCallExpr);
+ const auto *UnlockDeclRef = findDeclRefExpr(UnlockCallExpr);
+
Please don't use auto where return type is not obvious.
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:82
+
+ auto LockObjectName = LockDeclRef->getFoundDecl()->getName();
+ auto UnlockObjectName = UnlockDeclRef->getFoundDecl()->getName();
Please don't use auto where return type is not obvious.
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:83
+ auto LockObjectName = LockDeclRef->getFoundDecl()->getName();
+ auto UnlockObjectName = UnlockDeclRef->getFoundDecl()->getName();
+
Please don't use auto where return type is not obvious.
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:89
+
+ auto LockLocation =
+ Result.SourceManager->getPresumedLoc(LockCallExpr->getBeginLoc());
Please don't use auto where return type is not obvious.
Comment at: clang-tidy/cppcoreguidelines/UseRaiiLocksCheck.cpp:91
+ Result.SourceManager->getPresumedLoc(LockCallExpr->getBeginLoc());
+ auto UnlockLocation =
+ Result.SourceManager->getPresumedLoc(UnlockCallExpr->getBeginLoc());
Please don't use auto where return type is not obvious.
Comment at: docs/ReleaseNotes.rst:97
+
+ Checks for explicit usage of``std::mutex`` functions ``lock()``,
+ ``try_lock()`` and ``unlock()``.
Missing space before ``std::mutex
Comment at: docs/clang-tidy/checks/cppcoreguidelines-use-raii-locks.rst:6
+
+The explicit use of ``std::mutex`` functions ``lock()``, ``try_lock()`` and
+``unlock()`` is error prone and should be avoided.
Please synchronize first statement with Release Notes.
Comment at: docs/clang-tidy/checks/cppcoreguidelines-use-raii-locks.rst:18
+ locking and unlocking a mutex.
+ Defaults to: ``::std::mutex;::std::recursive_mutex;::std::timed_mutex;
+ ::std::recursive_timed_mutex;::std::unique_lock``.
Please use single ` for options values.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58818/new/
https://reviews.llvm.org/D58818
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r355280 - Tweak r355278 for compatibility with gcc 6 and earlier.
Author: jyknight Date: Sat Mar 2 13:55:36 2019 New Revision: 355280 URL: http://llvm.org/viewvc/llvm-project?rev=355280&view=rev Log: Tweak r355278 for compatibility with gcc 6 and earlier. Modified: cfe/trunk/lib/Basic/Sanitizers.cpp Modified: cfe/trunk/lib/Basic/Sanitizers.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Sanitizers.cpp?rev=355280&r1=355279&r2=355280&view=diff == --- cfe/trunk/lib/Basic/Sanitizers.cpp (original) +++ cfe/trunk/lib/Basic/Sanitizers.cpp Sat Mar 2 13:55:36 2019 @@ -18,7 +18,7 @@ using namespace clang; // Once LLVM switches to C++17, the constexpr variables can be inline and we // won't need this. -#define SANITIZER(NAME, ID) const SanitizerMask SanitizerKind::ID; +#define SANITIZER(NAME, ID) constexpr SanitizerMask SanitizerKind::ID; #define SANITIZER_GROUP(NAME, ID, ALIAS) \ constexpr SanitizerMask SanitizerKind::ID; \ constexpr SanitizerMask SanitizerKind::ID##Group; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)
nridge updated this revision to Diff 189065.
nridge added a comment.
Rebased.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56370/new/
https://reviews.llvm.org/D56370
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/FindSymbols.cpp
clang-tools-extra/clangd/FindSymbols.h
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/XRefs.h
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/index/SymbolCollector.h
clang-tools-extra/test/clangd/initialize-params.test
clang-tools-extra/unittests/clangd/Matchers.h
clang-tools-extra/unittests/clangd/XRefsTests.cpp
Index: clang-tools-extra/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/unittests/clangd/XRefsTests.cpp
@@ -15,6 +15,8 @@
#include "XRefs.h"
#include "index/FileIndex.h"
#include "index/SymbolCollector.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
#include "clang/Index/IndexingAction.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/ScopedPrinter.h"
@@ -25,9 +27,13 @@
namespace clangd {
namespace {
+using testing::AllOf;
using testing::ElementsAre;
+using testing::Eq;
+using testing::Field;
using testing::IsEmpty;
using testing::Matcher;
+using testing::Pointee;
using testing::UnorderedElementsAreArray;
class IgnoreDiagnostics : public DiagnosticsConsumer {
@@ -39,6 +45,15 @@
return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
}
+// GMock helpers for matching TypeHierarchyItem.
+MATCHER_P(WithName, N, "") { return arg.name == N; }
+MATCHER_P(WithKind, Kind, "") { return arg.kind == Kind; }
+MATCHER_P(SelectionRangeIs, R, "") { return arg.selectionRange == R; }
+template
+testing::Matcher Parents(ParentMatchers... ParentsM) {
+ return Field(&TypeHierarchyItem::parents, HasValue(ElementsAre(ParentsM...)));
+}
+
// Extracts ranges from an annotated example, and constructs a matcher for a
// highlight set. Ranges should be named $read/$write as appropriate.
Matcher &>
@@ -1478,6 +1493,325 @@
}
}
+TEST(FindRecordTypeAt, TypeOrVariable) {
+ Annotations Source(R"cpp(
+struct Ch^ild2 {
+ int c;
+};
+
+int main() {
+ Ch^ild2 ch^ild2;
+ ch^ild2.c = 1;
+}
+)cpp");
+
+ TestTU TU = TestTU::withCode(Source.code());
+ auto AST = TU.build();
+
+ ASSERT_TRUE(AST.getDiagnostics().empty());
+
+ for (Position Pt : Source.points()) {
+const CXXRecordDecl *RD = findRecordTypeAt(AST, Pt);
+EXPECT_EQ(&findDecl(AST, "Child2"), static_cast(RD));
+ }
+}
+
+TEST(FindRecordTypeAt, Method) {
+ Annotations Source(R"cpp(
+struct Child2 {
+ void met^hod ();
+ void met^hod (int x);
+};
+
+int main() {
+ Child2 child2;
+ child2.met^hod(5);
+}
+)cpp");
+
+ TestTU TU = TestTU::withCode(Source.code());
+ auto AST = TU.build();
+
+ ASSERT_TRUE(AST.getDiagnostics().empty());
+
+ for (Position Pt : Source.points()) {
+const CXXRecordDecl *RD = findRecordTypeAt(AST, Pt);
+EXPECT_EQ(&findDecl(AST, "Child2"), static_cast(RD));
+ }
+}
+
+TEST(FindRecordTypeAt, Field) {
+ Annotations Source(R"cpp(
+struct Child2 {
+ int fi^eld;
+};
+
+int main() {
+ Child2 child2;
+ child2.fi^eld = 5;
+}
+)cpp");
+
+ TestTU TU = TestTU::withCode(Source.code());
+ auto AST = TU.build();
+
+ ASSERT_TRUE(AST.getDiagnostics().empty());
+
+ for (Position Pt : Source.points()) {
+const CXXRecordDecl *RD = findRecordTypeAt(AST, Pt);
+EXPECT_EQ(nullptr, RD);
+ }
+}
+
+// TODO: FindRecordTypeAt, TemplateSpec
+
+TEST(TypeAncestors, SimpleInheritance) {
+ Annotations Source(R"cpp(
+struct Parent {
+ int a;
+};
+
+struct Child1 : Parent {
+ int b;
+};
+
+struct Child2 : Child1 {
+ int c;
+};
+)cpp");
+
+ TestTU TU = TestTU::withCode(Source.code());
+ auto AST = TU.build();
+
+ ASSERT_TRUE(AST.getDiagnostics().empty());
+
+ const CXXRecordDecl *Parent =
+ dyn_cast(&findDecl(AST, "Parent"));
+ const CXXRecordDecl *Child1 =
+ dyn_cast(&findDecl(AST, "Child1"));
+ const CXXRecordDecl *Child2 =
+ dyn_cast(&findDecl(AST, "Child2"));
+
+ EXPECT_THAT(typeAncestors(Parent), ElementsAre());
+ EXPECT_THAT(typeAncestors(Child1), ElementsAre(Parent));
+ EXPECT_THAT(typeAncestors(Child2), ElementsAre(Child1));
+}
+
+TEST(TypeAncestors, MultipleInheritance) {
+ Annotations Source(R"cpp(
+struct Parent1 {
+ int a;
+};
+
+struct Parent2 {
+ int b;
+};
+
+struct Parent3 : Parent2 {
+ int c;
+};
+
+struct Child : Parent1, Parent3 {
+ int d;
+};
+)cpp");
+
+ TestTU TU = TestTU::withCode(Source.code());
+ auto AST = TU.build();
+
+ ASSERT_TRUE(AST.getDiagnostics().empty());
+
+ const CXXRecordDecl *Parent1 =
+ dy
[PATCH] D58880: [WIP] [Looking for API feedback] [clangd] Type hierarchy subtypes
nridge created this revision.
nridge added reviewers: sammccall, kadircet.
Herald added subscribers: cfe-commits, jdoerfert, arphaman, mgrang, jkorous,
MaskRay, ioeric, ilya-biryukov.
Herald added a project: clang.
This is an early draft of an implementation of type hierarchy subtypes.
There is enough here to pass some basic tests, but the implementation is
incomplete (for example, the index support is in place for MemIndex but not
Dex, and the serialization support for YAML but not RIFF).
My objective at this point is to get some high-level feedback on the addition
of "relations" to the index model and API. If this direction seems reasonable,
I will proceed and fill in the missing pieces of the implementation.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D58880
Files:
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/XRefs.h
clang-tools-extra/clangd/index/Background.cpp
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/FileIndex.h
clang-tools-extra/clangd/index/Index.cpp
clang-tools-extra/clangd/index/Index.h
clang-tools-extra/clangd/index/MemIndex.cpp
clang-tools-extra/clangd/index/MemIndex.h
clang-tools-extra/clangd/index/Merge.cpp
clang-tools-extra/clangd/index/Merge.h
clang-tools-extra/clangd/index/Serialization.cpp
clang-tools-extra/clangd/index/Serialization.h
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/index/SymbolCollector.h
clang-tools-extra/clangd/index/YAMLSerialization.cpp
clang-tools-extra/clangd/index/dex/Dex.cpp
clang-tools-extra/clangd/index/dex/Dex.h
clang-tools-extra/unittests/clangd/CodeCompleteTests.cpp
clang-tools-extra/unittests/clangd/DiagnosticsTests.cpp
clang-tools-extra/unittests/clangd/FileIndexTests.cpp
clang-tools-extra/unittests/clangd/IndexTests.cpp
clang-tools-extra/unittests/clangd/TestTU.cpp
clang-tools-extra/unittests/clangd/XRefsTests.cpp
Index: clang-tools-extra/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/unittests/clangd/XRefsTests.cpp
@@ -1761,6 +1761,87 @@
EXPECT_THAT(typeAncestors(Child3), ElementsAre());
}
+SymbolID findSymbolIDByName(llvm::StringRef Name, SymbolIndex *Index) {
+ SymbolID Result;
+ FuzzyFindRequest Request;
+ Request.Query = Name;
+ Request.AnyScope = true;
+ Request.Limit = 1;
+ int ResultCount = 0;
+ Index->fuzzyFind(Request, [&](const Symbol &S) {
+Result = S.ID;
+++ResultCount;
+ });
+ EXPECT_EQ(1, ResultCount);
+ return Result;
+}
+
+std::vector collectSubtypes(SymbolID Type, SymbolIndex *Index) {
+ std::vector Result;
+ llvm::errs() << "Querying subtypes of " << Type << "\n";
+ Index->relations(Type, RelationKind::Subtype,
+ [&Result](const SymbolID &ID) { Result.push_back(ID); });
+ return Result;
+}
+
+TEST(Subtypes, SimpleInheritance) {
+ Annotations Source(R"cpp(
+struct Parent {
+ int a;
+};
+
+struct Child1 : Parent {
+ int b;
+};
+
+struct Child2 : Child1 {
+ int c;
+};
+)cpp");
+
+ TestTU TU = TestTU::withCode(Source.code());
+ auto Index = TU.index();
+
+ SymbolID Parent = findSymbolIDByName("Parent", Index.get());
+ SymbolID Child1 = findSymbolIDByName("Child1", Index.get());
+ SymbolID Child2 = findSymbolIDByName("Child2", Index.get());
+
+ EXPECT_THAT(collectSubtypes(Parent, Index.get()), ElementsAre(Child1));
+ EXPECT_THAT(collectSubtypes(Child1, Index.get()), ElementsAre(Child2));
+}
+
+TEST(Subtypes, MultipleInheritance) {
+ Annotations Source(R"cpp(
+struct Parent1 {
+ int a;
+};
+
+struct Parent2 {
+ int b;
+};
+
+struct Parent3 : Parent2 {
+ int c;
+};
+
+struct Child : Parent1, Parent3 {
+ int d;
+};
+)cpp");
+
+ TestTU TU = TestTU::withCode(Source.code());
+ auto Index = TU.index();
+
+ SymbolID Parent1 = findSymbolIDByName("Parent1", Index.get());
+ SymbolID Parent2 = findSymbolIDByName("Parent2", Index.get());
+ SymbolID Parent3 = findSymbolIDByName("Parent3", Index.get());
+ SymbolID Child = findSymbolIDByName("Child", Index.get());
+
+ EXPECT_THAT(collectSubtypes(Parent1, Index.get()), ElementsAre(Child));
+ EXPECT_THAT(collectSubtypes(Parent2, Index.get()), ElementsAre(Parent3));
+ EXPECT_THAT(collectSubtypes(Parent3, Index.get()), ElementsAre(Child));
+}
+
// Parts of getTypeHierarchy() are tested in more detail by the
// FindRecordTypeAt.* and TypeAncestor.* tests above. This test exercises the
// entire operation.
Index: clang-tools-extra/unittests/clangd/TestTU.cpp
===
--- clang-tools-extra/unittests/clangd/TestTU.cpp
+++ clang-tools-extra/unittests/clangd/TestTU.cpp
@@ -65,7 +65,9 @@
std::unique_ptr TestTU::index() const {
auto AST = build();
- auto Idx = llvm::make_unique(/*UseDex=*/true);
+ // UseDex is temporarily set to false so we can test subtypes s
[PATCH] D58882: [Diagnostics] Address of a standard library function
xbolva00 created this revision.
xbolva00 added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
"Taking the address of a standard library function is not allowed" -
https://usingstdcpp.org/2018/03/18/jacksonville18-iso-cpp-report/
Repository:
rC Clang
https://reviews.llvm.org/D58882
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/SemaCXX/warn_address_standard_library_function.cpp
Index: test/SemaCXX/warn_address_standard_library_function.cpp
===
--- test/SemaCXX/warn_address_standard_library_function.cpp
+++ test/SemaCXX/warn_address_standard_library_function.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Waddress -std=c++14 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Waddress -std=c++17 %s
+
+namespace std {
+void foo (void) { }
+}
+
+namespace mystd {
+void foo (void) { }
+}
+
+auto warn1(void) {
+return &std::foo; // expected-warning{{taking the address of the standard
library function 'std::foo' is not allowed}}
+}
+
+auto warn2(void) {
+return &(std::foo); // expected-warning{{taking the address of the
standard library function 'std::foo' is not allowed}}
+}
+
+auto nowarn1(void) {
+return &mystd::foo;
+}
+
+auto nowarn2(void) {
+return &(mystd::foo);
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -11934,7 +11934,6 @@
Expr::LValueClassification lval = op->ClassifyLValue(Context);
unsigned AddressOfError = AO_No_Error;
-
if (lval == Expr::LV_ClassTemporary || lval == Expr::LV_ArrayTemporary) {
bool sfinae = (bool)isSFINAEContext();
Diag(OpLoc, isSFINAEContext() ? diag::err_typecheck_addrof_temporary
@@ -12048,8 +12047,11 @@
return MPTy;
}
}
-} else if (!isa(dcl) && !isa(dcl) &&
- !isa(dcl))
+} else if (isa(dcl)) {
+ StringRef QualName =
cast(dcl)->getQualifiedNameAsString();
+ if (QualName.startswith("std::"))
+Diag(OpLoc, diag::warn_taking_address_standard_library_function) <<
QualName;
+} else if (!isa(dcl) && !isa(dcl))
llvm_unreachable("Unknown/unexpected decl type");
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6730,6 +6730,9 @@
def err_cannot_form_pointer_to_member_of_reference_type : Error<
"cannot form a pointer-to-member to member %0 of reference type %1">;
+def warn_taking_address_standard_library_function : Warning<
+ "taking the address of the standard library function '%0' is not allowed">
+ , InGroup;
def err_incomplete_object_call : Error<
"incomplete type in call to object of type %0">;
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -687,7 +687,7 @@
// Aggregation warning settings.
// Populate -Waddress with warnings from other groups.
-def : DiagGroup<"address", [PointerBoolConversion,
+def Address : DiagGroup<"address", [PointerBoolConversion,
StringCompare,
TautologicalPointerCompare]>;
Index: test/SemaCXX/warn_address_standard_library_function.cpp
===
--- test/SemaCXX/warn_address_standard_library_function.cpp
+++ test/SemaCXX/warn_address_standard_library_function.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Waddress -std=c++14 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Waddress -std=c++17 %s
+
+namespace std {
+void foo (void) { }
+}
+
+namespace mystd {
+void foo (void) { }
+}
+
+auto warn1(void) {
+return &std::foo; // expected-warning{{taking the address of the standard library function 'std::foo' is not allowed}}
+}
+
+auto warn2(void) {
+return &(std::foo); // expected-warning{{taking the address of the standard library function 'std::foo' is not allowed}}
+}
+
+auto nowarn1(void) {
+return &mystd::foo;
+}
+
+auto nowarn2(void) {
+return &(mystd::foo);
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -11934,7 +11934,6 @@
Expr::LValueClassification lval = op->ClassifyLValue(Context);
unsigned AddressOfError = AO_No_Error;
-
if (lval == Expr::LV_ClassTemporary || lval == Expr::LV_ArrayTemporary) {
bool sfinae = (bool)isSFINAEContext();
Diag(OpLoc, isSFINAEContext() ? diag::err_typecheck_addrof_temporary
@@ -12048,8 +12047,11 @@
return MPTy;
}
}
-} else if (!isa(dcl)
[PATCH] D58882: [Diagnostics] Address of a standard library function
xbolva00 updated this revision to Diff 189072.
xbolva00 added a comment.
Removed extra new line, changed test
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58882/new/
https://reviews.llvm.org/D58882
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/SemaCXX/warn_address_standard_library_function.cpp
Index: test/SemaCXX/warn_address_standard_library_function.cpp
===
--- test/SemaCXX/warn_address_standard_library_function.cpp
+++ test/SemaCXX/warn_address_standard_library_function.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Waddress -std=c++14 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Waddress -std=c++17 %s
+
+namespace std {
+void terminate (void) { }
+}
+
+namespace mystd {
+void terminate (void) { }
+}
+
+auto warn1(void) {
+return &std::terminate; // expected-warning{{taking the address of the
standard library function 'std::foo' is not allowed}}
+}
+
+auto warn2(void) {
+return &(std::terminate); // expected-warning{{taking the address of the
standard library function 'std::foo' is not allowed}}
+}
+
+auto nowarn1(void) {
+return &mystd::terminate;
+}
+
+auto nowarn2(void) {
+return &(mystd::terminate);
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -12048,8 +12048,11 @@
return MPTy;
}
}
-} else if (!isa(dcl) && !isa(dcl) &&
- !isa(dcl))
+} else if (isa(dcl)) {
+ StringRef QualName =
cast(dcl)->getQualifiedNameAsString();
+ if (QualName.startswith("std::"))
+Diag(OpLoc, diag::warn_taking_address_standard_library_function) <<
QualName;
+} else if (!isa(dcl) && !isa(dcl))
llvm_unreachable("Unknown/unexpected decl type");
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6730,6 +6730,9 @@
def err_cannot_form_pointer_to_member_of_reference_type : Error<
"cannot form a pointer-to-member to member %0 of reference type %1">;
+def warn_taking_address_standard_library_function : Warning<
+ "taking the address of the standard library function '%0' is not allowed">
+ , InGroup;
def err_incomplete_object_call : Error<
"incomplete type in call to object of type %0">;
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -687,7 +687,7 @@
// Aggregation warning settings.
// Populate -Waddress with warnings from other groups.
-def : DiagGroup<"address", [PointerBoolConversion,
+def Address : DiagGroup<"address", [PointerBoolConversion,
StringCompare,
TautologicalPointerCompare]>;
Index: test/SemaCXX/warn_address_standard_library_function.cpp
===
--- test/SemaCXX/warn_address_standard_library_function.cpp
+++ test/SemaCXX/warn_address_standard_library_function.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Waddress -std=c++14 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Waddress -std=c++17 %s
+
+namespace std {
+void terminate (void) { }
+}
+
+namespace mystd {
+void terminate (void) { }
+}
+
+auto warn1(void) {
+return &std::terminate; // expected-warning{{taking the address of the standard library function 'std::foo' is not allowed}}
+}
+
+auto warn2(void) {
+return &(std::terminate); // expected-warning{{taking the address of the standard library function 'std::foo' is not allowed}}
+}
+
+auto nowarn1(void) {
+return &mystd::terminate;
+}
+
+auto nowarn2(void) {
+return &(mystd::terminate);
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -12048,8 +12048,11 @@
return MPTy;
}
}
-} else if (!isa(dcl) && !isa(dcl) &&
- !isa(dcl))
+} else if (isa(dcl)) {
+ StringRef QualName = cast(dcl)->getQualifiedNameAsString();
+ if (QualName.startswith("std::"))
+Diag(OpLoc, diag::warn_taking_address_standard_library_function) << QualName;
+} else if (!isa(dcl) && !isa(dcl))
llvm_unreachable("Unknown/unexpected decl type");
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6730,6 +6730,9 @@
def err_cannot_form_pointer_to_member_of_reference_type : Error<
"cannot form a pointer-to-member to
[PATCH] D58882: [Diagnostics] Address of a standard library function
xbolva00 updated this revision to Diff 189073.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58882/new/
https://reviews.llvm.org/D58882
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/SemaCXX/warn_address_standard_library_function.cpp
Index: test/SemaCXX/warn_address_standard_library_function.cpp
===
--- test/SemaCXX/warn_address_standard_library_function.cpp
+++ test/SemaCXX/warn_address_standard_library_function.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Waddress -std=c++14 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Waddress -std=c++17 %s
+
+namespace std {
+void terminate (void) { }
+}
+
+namespace mystd {
+void terminate (void) { }
+}
+
+auto warn1(void) {
+return &std::terminate; // expected-warning{{taking the address of the
standard library function 'std::terminate' is not allowed}}
+}
+
+auto warn2(void) {
+return &(std::terminate); // expected-warning{{taking the address of the
standard library function 'std::terminate' is not allowed}}
+}
+
+auto nowarn1(void) {
+return &mystd::terminate;
+}
+
+auto nowarn2(void) {
+return &(mystd::terminate);
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -12048,8 +12048,11 @@
return MPTy;
}
}
-} else if (!isa(dcl) && !isa(dcl) &&
- !isa(dcl))
+} else if (isa(dcl)) {
+ StringRef QualName = cast(dcl)->getQualifiedNameAsString();
+ if (QualName.startswith("std::"))
+Diag(OpLoc, diag::warn_taking_address_standard_library_function) <<
QualName;
+} else if (!isa(dcl) && !isa(dcl))
llvm_unreachable("Unknown/unexpected decl type");
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6730,6 +6730,9 @@
def err_cannot_form_pointer_to_member_of_reference_type : Error<
"cannot form a pointer-to-member to member %0 of reference type %1">;
+def warn_taking_address_standard_library_function : Warning<
+ "taking the address of the standard library function '%0' is not allowed">
+ , InGroup;
def err_incomplete_object_call : Error<
"incomplete type in call to object of type %0">;
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -687,7 +687,7 @@
// Aggregation warning settings.
// Populate -Waddress with warnings from other groups.
-def : DiagGroup<"address", [PointerBoolConversion,
+def Address : DiagGroup<"address", [PointerBoolConversion,
StringCompare,
TautologicalPointerCompare]>;
Index: test/SemaCXX/warn_address_standard_library_function.cpp
===
--- test/SemaCXX/warn_address_standard_library_function.cpp
+++ test/SemaCXX/warn_address_standard_library_function.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Waddress -std=c++14 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Waddress -std=c++17 %s
+
+namespace std {
+void terminate (void) { }
+}
+
+namespace mystd {
+void terminate (void) { }
+}
+
+auto warn1(void) {
+return &std::terminate; // expected-warning{{taking the address of the standard library function 'std::terminate' is not allowed}}
+}
+
+auto warn2(void) {
+return &(std::terminate); // expected-warning{{taking the address of the standard library function 'std::terminate' is not allowed}}
+}
+
+auto nowarn1(void) {
+return &mystd::terminate;
+}
+
+auto nowarn2(void) {
+return &(mystd::terminate);
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -12048,8 +12048,11 @@
return MPTy;
}
}
-} else if (!isa(dcl) && !isa(dcl) &&
- !isa(dcl))
+} else if (isa(dcl)) {
+ StringRef QualName = cast(dcl)->getQualifiedNameAsString();
+ if (QualName.startswith("std::"))
+Diag(OpLoc, diag::warn_taking_address_standard_library_function) << QualName;
+} else if (!isa(dcl) && !isa(dcl))
llvm_unreachable("Unknown/unexpected decl type");
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6730,6 +6730,9 @@
def err_cannot_form_pointer_to_member_of_reference_type : Error<
"cannot form a pointer-to-member to member %0 of reference type %1">;
+def warn_
