date:20180916

[PATCH] D52136: [clang-tidy] Add modernize-concat-nested-namespaces check

2018-09-16 Thread Wojtek Gumuła via Phabricator via cfe-commits

wgml updated this revision to Diff 165677.
wgml added a comment.

Fixed typo in documentation.
Refactored a bit of check code to make it more readable.


https://reviews.llvm.org/D52136

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
  clang-tidy/modernize/ConcatNestedNamespacesCheck.h
  clang-tidy/modernize/ModernizeTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
  test/clang-tidy/modernize-concat-nested-namespaces.cpp

Index: test/clang-tidy/modernize-concat-nested-namespaces.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-concat-nested-namespaces.cpp
@@ -0,0 +1,161 @@
+// RUN: %check_clang_tidy %s modernize-concat-nested-namespaces %t -- -- -std=c++17
+
+namespace n1 {}
+
+namespace n2 {
+namespace n3 {
+void t();
+}
+namespace n4 {
+void t();
+}
+} // namespace n2
+
+namespace n5 {
+inline namespace n6 {
+void t();
+}
+} // namespace n5
+
+namespace n7 {
+void t();
+
+namespace n8 {
+void t();
+}
+} // namespace n7
+
+namespace n9 {
+namespace n10 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n9::n10
+void t();
+} // namespace n10
+} // namespace n9
+// CHECK-FIXES: }
+
+namespace n11 {
+namespace n12 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n11::n12
+namespace n13 {
+void t();
+}
+namespace n14 {
+void t();
+}
+} // namespace n12
+} // namespace n11
+// CHECK-FIXES: }
+
+namespace n15 {
+namespace n16 {
+void t();
+}
+
+inline namespace n17 {
+void t();
+}
+
+namespace n18 {
+namespace n19 {
+namespace n20 {
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n18::n19::n20
+void t();
+} // namespace n20
+} // namespace n19
+} // namespace n18
+// CHECK-FIXES: }
+
+namespace n21 {
+void t();
+}
+} // namespace n15
+
+namespace n22 {
+namespace {
+void t();
+}
+} // namespace n22
+
+namespace n23 {
+namespace {
+namespace n24 {
+namespace n25 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n24::n25
+void t();
+} // namespace n25
+} // namespace n24
+// CHECK-FIXES: }
+} // namespace
+} // namespace n23
+
+namespace n26::n27 {
+namespace n28 {
+namespace n29::n30 {
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n26::n27::n28::n29::n30
+void t() {}
+} // namespace n29::n30
+} // namespace n28
+} // namespace n26::n27
+// CHECK-FIXES: }
+
+namespace n31 {
+namespace n32 {}
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+} // namespace n31
+// CHECK-FIXES-EMPTY
+
+namespace n33 {
+namespace n34 {
+namespace n35 {}
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+} // namespace n34
+// CHECK-FIXES-EMPTY
+namespace n36 {
+void t();
+}
+} // namespace n33
+
+namespace n37::n38 {
+void t();
+}
+
+#define IEXIST
+namespace n39 {
+namespace n40 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n39::n40
+#ifdef IEXIST
+void t() {}
+#endif
+} // namespace n40
+} // namespace n39
+// CHECK-FIXES: }
+
+namespace n41 {
+namespace n42 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n41::n42
+#ifdef IDONTEXIST
+void t() {}
+#endif
+} // namespace n42
+} // namespace n41
+// CHECK-FIXES: }
+
+int main() {
+  n26::n27::n28::n29::n30::t();
+#ifdef IEXIST
+  n39::n40::t();
+#endif
+
+#ifdef IDONTEXIST
+  n41::n42::t();
+#endif
+
+  return 0;
+}
Index: docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
@@ -0,0 +1,49 @@
+.. title:: clang-tidy - modernize-concat-nested-namespaces
+
+modernize-concat-nested-namespaces
+==
+
+Checks for use of nested namespaces in a form of ``namespace a { namespace b { ... } }``
+and offers change to syntax introduced in C++17: ``namespace a::b { ... }``.
+Inlined namespaces are not modified.
+
+For example:
+
+.. code-block:: c++
+
+  namespace n1 {
+  namespace n2 {
+  void t();
+  }
+  }
+
+  namespace n3 {
+  namespace n4 {
+  namespace n5 {
+  void t();
+  }
+  }
+  namespace n6 {
+  namespace n7 {
+  void t();
+  }
+  }
+  }
+
+Will be modified to:
+
+.. code-

[PATCH] D52135: [Clang-Tidy: modernize] Fix for modernize-redundant-void-arg: complains about variable cast to void

2018-09-16 Thread Jonas Toth via Phabricator via cfe-commits

JonasToth added a comment.

Could you please add a test, to show that non-templated function are still 
diagnosed correctly?
The `isInstantiated` would only apply to templated functions.

Why is the cast to void diagnosed anyway? It is not an argument nor is it used 
in a function context. Do you have an explaination for that?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52135



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52135: [Clang-Tidy: modernize] Fix for modernize-redundant-void-arg: complains about variable cast to void

2018-09-16 Thread Idriss via Phabricator via cfe-commits

IdrissRio updated this revision to Diff 165683.
IdrissRio edited the summary of this revision.
IdrissRio added a comment.

Thank you @JonasToth.
I have update the patch and i have added two cases. The first an
instantiation of a non-templated function. The second is the instantiation
of a templatic funciton.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52135

Files:
  clang-tidy/modernize/RedundantVoidArgCheck.cpp
  test/clang-tidy/modernize-redundant-void-arg.cpp


Index: test/clang-tidy/modernize-redundant-void-arg.cpp
===
--- test/clang-tidy/modernize-redundant-void-arg.cpp
+++ test/clang-tidy/modernize-redundant-void-arg.cpp
@@ -488,3 +488,42 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant void argument list in 
lambda expression [modernize-redundant-void-arg]
   // CHECK-FIXES: []() BODY;
 }
+
+
+struct S_1{
+ void g_1(void) const{
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant void argument list in 
function definition [modernize-redundant-void-arg]
+  // CHECK-FIXES: void g_1() const{
+   int a;
+   (void)a;
+ }
+   
+ void g_2() const {
+   int a;
+   (void)a;
+ }
+   
+};
+
+template 
+struct S_2{
+ void g_1(void) const{
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant void argument list in 
function definition [modernize-redundant-void-arg]
+  // CHECK-FIXES: void g_1() const{
+   int a;
+   (void)a;
+ }
+   
+ void g_2() const {
+   int a;
+   (void)a;
+ }
+};
+
+//Instantiation of the two structs.
+void f_testTemplate(){
+  S_1();
+  S_2();
+}
+
+
Index: clang-tidy/modernize/RedundantVoidArgCheck.cpp
===
--- clang-tidy/modernize/RedundantVoidArgCheck.cpp
+++ clang-tidy/modernize/RedundantVoidArgCheck.cpp
@@ -49,7 +49,7 @@
 return;
 
   Finder->addMatcher(functionDecl(parameterCountIs(0), unless(isImplicit()),
-  unless(isExternC()))
+  unless(isInstantiated()), 
unless(isExternC()))
  .bind(FunctionId),
  this);
   Finder->addMatcher(typedefNameDecl().bind(TypedefId), this);


Index: test/clang-tidy/modernize-redundant-void-arg.cpp
===
--- test/clang-tidy/modernize-redundant-void-arg.cpp
+++ test/clang-tidy/modernize-redundant-void-arg.cpp
@@ -488,3 +488,42 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: redundant void argument list in lambda expression [modernize-redundant-void-arg]
   // CHECK-FIXES: []() BODY;
 }
+
+
+struct S_1{
+ void g_1(void) const{
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant void argument list in function definition [modernize-redundant-void-arg]
+  // CHECK-FIXES: void g_1() const{
+   int a;
+   (void)a;
+ }
+	
+ void g_2() const {
+   int a;
+   (void)a;
+ }
+	
+};
+
+template 
+struct S_2{
+ void g_1(void) const{
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: redundant void argument list in function definition [modernize-redundant-void-arg]
+  // CHECK-FIXES: void g_1() const{
+   int a;
+   (void)a;
+ }
+	
+ void g_2() const {
+   int a;
+   (void)a;
+ }
+};
+
+//Instantiation of the two structs.
+void f_testTemplate(){
+  S_1();
+  S_2();
+}
+
+
Index: clang-tidy/modernize/RedundantVoidArgCheck.cpp
===
--- clang-tidy/modernize/RedundantVoidArgCheck.cpp
+++ clang-tidy/modernize/RedundantVoidArgCheck.cpp
@@ -49,7 +49,7 @@
 return;
 
   Finder->addMatcher(functionDecl(parameterCountIs(0), unless(isImplicit()),
-  unless(isExternC()))
+  unless(isInstantiated()), unless(isExternC()))
  .bind(FunctionId),
  this);
   Finder->addMatcher(typedefNameDecl().bind(TypedefId), this);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52120: [analyzer] Treat std::{move, forward} as casts in ExprMutationAnalyzer.

2018-09-16 Thread Shuai Wang via Phabricator via cfe-commits

shuaiwang marked 2 inline comments as done.
shuaiwang added inline comments.



Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:387
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("std::move(x)"));
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+

JonasToth wrote:
> JonasToth wrote:
> > I am thinking about the correctness of the change.
> > 
> > If we have this case:
> > ```
> > std::string s1 = "Hello World!";
> > std::string s2 = std::move(s1);
> > ```
> > `s1` gets mutated, but it looks like it would not be considered as mutation?
> > 
> > On the other hand
> > ```
> > int i1 = 42;
> > int i2 = std::move(i1);
> > ```
> > should resolve in a copy `i1` and therefor not be a mutation.
> > 
> > Could you please add tests demonstrating this difference and the correct 
> > diagnostic detection for that.
> > 
> > Potentially interesting:
> > - Builtin types should be Copy-Only?, builtin arrays as expensive to move 
> > types for the completeness please as well
> > - Types with deleted move operations, should resolve in copy too
> > - Move-Only types like unique_ptr
> > - Types with both move and copy constructor (vector-like)
> > - and something with everything defaulted
> > 
> > These thoughts are just thinking and writing, I just wondered that moving a 
> > variable with std::move is not considered as mutation here, even though 
> > there are cases were it is actually a mutation.
> Related: 
> https://clang.llvm.org/extra/clang-tidy/checks/performance-move-const-arg.html
Added these tests:
* std::move copy only type doesn't mutate
* std::move move only type does mutate
* std::move copiable & movable type does mutate
* std::move const copiable & movable type doesn't mutate
Didn't test array because array can't be assigned (so copy/move isn't relevant) 
and passing array to function can only be done via reference (passing by value 
results in array to pointer decay) so no copy/move there as well.


Repository:
  rC Clang

https://reviews.llvm.org/D52120



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52120: [analyzer] Treat std::{move, forward} as casts in ExprMutationAnalyzer.

2018-09-16 Thread Shuai Wang via Phabricator via cfe-commits

shuaiwang updated this revision to Diff 165684.
shuaiwang marked an inline comment as done.
shuaiwang added a comment.

Added more test cases around std::move


Repository:
  rC Clang

https://reviews.llvm.org/D52120

Files:
  lib/Analysis/ExprMutationAnalyzer.cpp
  unittests/Analysis/ExprMutationAnalyzerTest.cpp

Index: unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -66,6 +66,25 @@
   return s;
 }
 
+const std::string StdRemoveReference =
+"namespace std {"
+"template struct remove_reference { typedef T type; };"
+"template struct remove_reference { typedef T type; };"
+"template struct remove_reference { typedef T type; }; }";
+
+const std::string StdMove =
+"namespace std {"
+"template typename remove_reference::type&& "
+"move(T&& t) noexcept {"
+"return static_cast::type&&>(t); } }";
+
+const std::string StdForward =
+"namespace std {"
+"template T&& "
+"forward(typename remove_reference::type& t) noexcept { return t; }"
+"template T&& "
+"forward(typename remove_reference::type&&) noexcept { return t; } }";
+
 } // namespace
 
 TEST(ExprMutationAnalyzerTest, Trivial) {
@@ -373,36 +392,81 @@
 }
 
 TEST(ExprMutationAnalyzerTest, Move) {
-  // Technically almost the same as ByNonConstRRefArgument, just double checking
-  const auto AST = tooling::buildASTFromCode(
-  "namespace std {"
-  "template struct remove_reference { typedef T type; };"
-  "template struct remove_reference { typedef T type; };"
-  "template struct remove_reference { typedef T type; };"
-  "template typename std::remove_reference::type&& "
-  "move(T&& t) noexcept; }"
-  "void f() { struct A {}; A x; std::move(x); }");
-  const auto Results =
+  auto AST =
+  tooling::buildASTFromCode(StdRemoveReference + StdMove +
+"void f() { struct A {}; A x; std::move(x); }");
+  auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("std::move(x)"));
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+  StdRemoveReference + StdMove +
+  "void f() { struct A {}; A x, y; std::move(x) = y; }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("std::move(x) = y"));
+
+  AST = tooling::buildASTFromCode(StdRemoveReference + StdMove +
+  "void f() { int x, y; y = std::move(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+  StdRemoveReference + StdMove +
+  "struct S { S(); S(const S&); S& operator=(const S&); };"
+  "void f() { S x, y; y = std::move(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST =
+  tooling::buildASTFromCode(StdRemoveReference + StdMove +
+"struct S { S(); S(S&&); S& operator=(S&&); };"
+"void f() { S x, y; y = std::move(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("y = std::move(x)"));
+
+  AST =
+  tooling::buildASTFromCode(StdRemoveReference + StdMove +
+"struct S { S(); S(const S&); S(S&&);"
+"S& operator=(const S&); S& operator=(S&&); };"
+"void f() { S x, y; y = std::move(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("y = std::move(x)"));
+
+  AST = tooling::buildASTFromCode(
+  StdRemoveReference + StdMove +
+  "struct S { S(); S(const S&); S(S&&);"
+  "S& operator=(const S&); S& operator=(S&&); };"
+  "void f() { const S x; S y; y = std::move(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
+  AST = tooling::buildASTFromCode(
+  StdRemoveReference + StdMove +
+  "struct S{}; void f() { S x, y; y = std::move(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("y = std::move(x)"));
+
+  AST = tooling::buildASTFromCode(
+  StdRemoveReference + StdMove +
+  "struct S{}; void f() { const S x; S y; y = std::move(x); }");
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_FALSE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnaly

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

2018-09-16 Thread Tolga Mizrak via Phabricator via cfe-commits

to-miz created this revision.
to-miz added reviewers: djasper, klimek, krasimir, sammccall, mprobst, Nicola.
Herald added a subscriber: cfe-commits.

The option BeforeHash added to IndentPPDirectives.
Fixes Bug 36019.


Repository:
  rC Clang

https://reviews.llvm.org/D52150

Files:
  docs/ClangFormatStyleOptions.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
@@ -1754,8 +1754,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",
@@ -2846,22 +2846,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"
@@ -2871,6 +2874,95 @@
"  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"
+   "}";
+EXPECT_EQ(Expected, format(ToFormat, Style));
+EXPECT_EQ(Expected, format(Expec

[PATCH] D52137: Added warning for unary minus used with unsigned type

2018-09-16 Thread Joerg Sonnenberger via Phabricator via cfe-commits

joerg added a comment.

I find this warning confusing. I find a4 to be perfectly expected. IMO this 
warning should be applied only, if the effective value of the expression is not 
the same as in the modulo-n arithmetic. This means that if (-x) is explicitly 
or implicitly cast to a less wide unsigned type, it should not warn. It would 
consider a warning for the case of using (-x) if integer promotion rules makes 
it negative though. The question is, how to best patch around the warning 
though. What options does MSVC have for that? I.e. what equivalent expressions 
do not trigger this warning?


https://reviews.llvm.org/D52137



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52137: Added warning for unary minus used with unsigned type

2018-09-16 Thread Dávid Bolvanský via Phabricator via cfe-commits

xbolva00 added a comment.

MSVC warns for every case in the test file I uploaded here. 
https://godbolt.org/z/jiMFp6


https://reviews.llvm.org/D52137



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52151: Also manages clang-X as tool for scan-build

2018-09-16 Thread Sylvestre Ledru via Phabricator via cfe-commits

sylvestre.ledru created this revision.
sylvestre.ledru added a reviewer: jroelofs.

This will make
scan-build-7 clang-7 -c foo.c &> /dev/null


Repository:
  rC Clang

https://reviews.llvm.org/D52151

Files:
  tools/scan-build/bin/scan-build


Index: tools/scan-build/bin/scan-build
===
--- tools/scan-build/bin/scan-build
+++ tools/scan-build/bin/scan-build
@@ -1079,7 +1079,7 @@
   if ($Cmd =~ /(.*\/?gcc[^\/]*$)/ or
   $Cmd =~ /(.*\/?cc[^\/]*$)/ or
   $Cmd =~ /(.*\/?llvm-gcc[^\/]*$)/ or
-  $Cmd =~ /(.*\/?clang$)/ or
+  $Cmd =~ /(.*\/?clang[^\/]*$)/ or
   $Cmd =~ /(.*\/?ccc-analyzer[^\/]*$)/) {
 
 if (!($Cmd =~ /ccc-analyzer/) and !defined $ENV{"CCC_CC"}) {


Index: tools/scan-build/bin/scan-build
===
--- tools/scan-build/bin/scan-build
+++ tools/scan-build/bin/scan-build
@@ -1079,7 +1079,7 @@
   if ($Cmd =~ /(.*\/?gcc[^\/]*$)/ or
   $Cmd =~ /(.*\/?cc[^\/]*$)/ or
   $Cmd =~ /(.*\/?llvm-gcc[^\/]*$)/ or
-  $Cmd =~ /(.*\/?clang$)/ or
+  $Cmd =~ /(.*\/?clang[^\/]*$)/ or
   $Cmd =~ /(.*\/?ccc-analyzer[^\/]*$)/) {
 
 if (!($Cmd =~ /ccc-analyzer/) and !defined $ENV{"CCC_CC"}) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52151: Also manages clang-X as tool for scan-build

2018-09-16 Thread Kristina Brooks via Phabricator via cfe-commits

kristina added a comment.

Nitpick: This regex is far too broad to cover the rare use case where you'd be 
invoking clang as `clang-N`. I think something like `clang(?:\-[789])?` would 
be more suitable?


Repository:
  rC Clang

https://reviews.llvm.org/D52151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45741: Python bindings: Fix handling of file bodies with multi-byte characters

2018-09-16 Thread Kristina Brooks via Phabricator via cfe-commits

kristina added a comment.

Would you mind re-uploading these patches with full context (with `diff 
-U9`). 3 lines of context around changes makes this very difficult to 
review. Also I would suggest testing for Python version and using appropriate 
semantics (using `encode` in case of Python3). Not entirely sure if the 
bindings are supposed to be compatible with Python3, but I don't see any harm. 
However explicit tests that avoid changing functionality of existing Python2.7 
code would be better in my opinion.


Repository:
  rC Clang

https://reviews.llvm.org/D45741



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r342350 - Also manages clang-X as tool for scan-build

2018-09-16 Thread Sylvestre Ledru via cfe-commits

Author: sylvestre
Date: Sun Sep 16 12:36:59 2018
New Revision: 342350

URL: http://llvm.org/viewvc/llvm-project?rev=342350&view=rev
Log:
Also manages clang-X as tool for scan-build

Summary:
This will make
scan-build-7 clang-7 -c foo.c &> /dev/null

Reviewers: jroelofs

Subscribers: kristina, cfe-commits

Differential Revision: https://reviews.llvm.org/D52151

Modified:
cfe/trunk/tools/scan-build/bin/scan-build

Modified: cfe/trunk/tools/scan-build/bin/scan-build
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build/bin/scan-build?rev=342350&r1=342349&r2=342350&view=diff
==
--- cfe/trunk/tools/scan-build/bin/scan-build (original)
+++ cfe/trunk/tools/scan-build/bin/scan-build Sun Sep 16 12:36:59 2018
@@ -1079,7 +1079,7 @@ sub RunBuildCommand {
   if ($Cmd =~ /(.*\/?gcc[^\/]*$)/ or
   $Cmd =~ /(.*\/?cc[^\/]*$)/ or
   $Cmd =~ /(.*\/?llvm-gcc[^\/]*$)/ or
-  $Cmd =~ /(.*\/?clang$)/ or
+  $Cmd =~ /(.*\/?clang[^\/]*$)/ or
   $Cmd =~ /(.*\/?ccc-analyzer[^\/]*$)/) {
 
 if (!($Cmd =~ /ccc-analyzer/) and !defined $ENV{"CCC_CC"}) {


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52151: Also manages clang-X as tool for scan-build

2018-09-16 Thread Phabricator via Phabricator via cfe-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC342350: Also manages clang-X as tool for scan-build 
(authored by sylvestre, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52151?vs=165687&id=165689#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52151

Files:
  tools/scan-build/bin/scan-build


Index: tools/scan-build/bin/scan-build
===
--- tools/scan-build/bin/scan-build
+++ tools/scan-build/bin/scan-build
@@ -1079,7 +1079,7 @@
   if ($Cmd =~ /(.*\/?gcc[^\/]*$)/ or
   $Cmd =~ /(.*\/?cc[^\/]*$)/ or
   $Cmd =~ /(.*\/?llvm-gcc[^\/]*$)/ or
-  $Cmd =~ /(.*\/?clang$)/ or
+  $Cmd =~ /(.*\/?clang[^\/]*$)/ or
   $Cmd =~ /(.*\/?ccc-analyzer[^\/]*$)/) {
 
 if (!($Cmd =~ /ccc-analyzer/) and !defined $ENV{"CCC_CC"}) {


Index: tools/scan-build/bin/scan-build
===
--- tools/scan-build/bin/scan-build
+++ tools/scan-build/bin/scan-build
@@ -1079,7 +1079,7 @@
   if ($Cmd =~ /(.*\/?gcc[^\/]*$)/ or
   $Cmd =~ /(.*\/?cc[^\/]*$)/ or
   $Cmd =~ /(.*\/?llvm-gcc[^\/]*$)/ or
-  $Cmd =~ /(.*\/?clang$)/ or
+  $Cmd =~ /(.*\/?clang[^\/]*$)/ or
   $Cmd =~ /(.*\/?ccc-analyzer[^\/]*$)/) {
 
 if (!($Cmd =~ /ccc-analyzer/) and !defined $ENV{"CCC_CC"}) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52153: scan-build: Add support of the option --exclude like in scan-build-py

2018-09-16 Thread Sylvestre Ledru via Phabricator via cfe-commits

sylvestre.ledru created this revision.
sylvestre.ledru added a reviewer: jroelofs.
Herald added a subscriber: whisperity.
sylvestre.ledru edited the summary of this revision.

To exclude thirdparty code.

To test:
With /tmp/foo.c

  void test() {
  int x;
  x = 1; // warn
  }  



  $ scan-build --exclude non-existing/  --exclude /tmp/ -v gcc -c foo.c 


  
  
  scan-build: Using '/usr/lib/llvm-7/bin/clang' for static analysis
  scan-build: Emitting reports for this run to 
'/tmp/scan-build-2018-09-16-214531-8410-1'.
  foo.c:3:3: warning: Value stored to 'x' is never read
x = 1; // warn
^   ~
  1 warning generated.
  scan-build: File '/tmp/foo.c' deleted: part of an ignored directory.
  scan-build: 0 bugs found.


Repository:
  rC Clang

https://reviews.llvm.org/D52153

Files:
  tools/scan-build/bin/scan-build


Index: tools/scan-build/bin/scan-build
===
--- tools/scan-build/bin/scan-build
+++ tools/scan-build/bin/scan-build
@@ -58,6 +58,7 @@
   KeepEmpty => 0,# Don't remove output directory even with 0 
results.
   EnableCheckers => {},
   DisableCheckers => {},
+  Excludes => [],
   UseCC => undef,# C compiler to use for compilation.
   UseCXX => undef,   # C++ compiler to use for compilation.
   AnalyzerTarget => undef,
@@ -420,6 +421,20 @@
  # The file no longer exists: use the original path.
  $BugFile = $1;
   }
+
+  # Get just the path
+  my $p = dirname($BugFile);
+  # Check if the path is found in the list of exclude
+  if (grep { $p =~ m/$_/ } @{$Options{Excludes}}) {
+ if ($Options{Verbose}) {
+ Diag("File '$BugFile' deleted: part of an ignored directory.\n");
+ }
+
+ # File in an ignored directory. Remove it
+ unlink("$Dir/$FName");
+ return;
+  }
+
   UpdatePrefix($BugFile);
 }
 elsif (/$/) {
@@ -1194,6 +1209,12 @@
command. Specifying this option causes the exit status of scan-build to be 1
if it found potential bugs and 0 otherwise.
 
+ --exclude 
+
+   Do not run static analyzer against files found in this
+   directory (You can specify this option multiple times).
+   Could be useful when project contains 3rd party libraries.
+
  --use-cc [compiler path]
  --use-cc=[compiler path]
 
@@ -1698,6 +1719,15 @@
   next;
 }
 
+if ($arg eq "--exclude") {
+  shift @$Args;
+  my $arg = shift @$Args;
+  # Remove the trailing slash if any
+  $arg =~ s|/$||;
+  push @{$Options{Excludes}}, $arg;
+  next;
+}
+
 if ($arg eq "-load-plugin") {
   shift @$Args;
   push @{$Options{PluginsToLoad}}, shift @$Args;


Index: tools/scan-build/bin/scan-build
===
--- tools/scan-build/bin/scan-build
+++ tools/scan-build/bin/scan-build
@@ -58,6 +58,7 @@
   KeepEmpty => 0,# Don't remove output directory even with 0 results.
   EnableCheckers => {},
   DisableCheckers => {},
+  Excludes => [],
   UseCC => undef,# C compiler to use for compilation.
   UseCXX => undef,   # C++ compiler to use for compilation.
   AnalyzerTarget => undef,
@@ -420,6 +421,20 @@
  # The file no longer exists: use the original path.
  $BugFile = $1;
   }
+
+  # Get just the path
+  my $p = dirname($BugFile);
+  # Check if the path is found in the list of exclude
+  if (grep { $p =~ m/$_/ } @{$Options{Excludes}}) {
+	  if ($Options{Verbose}) {
+	  Diag("File '$BugFile' deleted: part of an ignored directory.\n");
+	  }
+
+	  # File in an ignored directory. Remove it
+	  unlink("$Dir/$FName");
+	  return;
+  }
+
   UpdatePrefix($BugFile);
 }
 elsif (/$/) {
@@ -1194,6 +1209,12 @@
command. Specifying this option causes the exit status of scan-build to be 1
if it found potential bugs and 0 otherwise.
 
+ --exclude 
+
+   Do not run static analyzer against files found in this
+   directory (You can specify this option multiple times).
+   Could be useful when project contains 3rd party libraries.
+
  --use-cc [compiler path]
  --use-cc=[compiler path]
 
@@ -1698,6 +1719,15 @@
   next;
 }
 
+if ($arg eq "--exclude") {
+  shift @$Args;
+  my $arg = shift @$Args;
+  # Remove the trailing slash if any
+  $arg =~ s|/$||;
+  push @{$Options{Excludes}}, $arg;
+  next;
+}
+
 if ($arg eq "-load-plugin") {
   shift @$Args;
   push @{$Options{PluginsToLoad}}, shift @$Args;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52153: scan-build: Add support of the option --exclude like in scan-build-py

2018-09-16 Thread Sylvestre Ledru via Phabricator via cfe-commits

sylvestre.ledru updated this revision to Diff 165691.
sylvestre.ledru added a comment.

Fix the indentation


Repository:
  rC Clang

https://reviews.llvm.org/D52153

Files:
  tools/scan-build/bin/scan-build


Index: tools/scan-build/bin/scan-build
===
--- tools/scan-build/bin/scan-build
+++ tools/scan-build/bin/scan-build
@@ -58,6 +58,7 @@
   KeepEmpty => 0,# Don't remove output directory even with 0 
results.
   EnableCheckers => {},
   DisableCheckers => {},
+  Excludes => [],
   UseCC => undef,# C compiler to use for compilation.
   UseCXX => undef,   # C++ compiler to use for compilation.
   AnalyzerTarget => undef,
@@ -420,6 +421,20 @@
  # The file no longer exists: use the original path.
  $BugFile = $1;
   }
+
+  # Get just the path
+  my $p = dirname($BugFile);
+  # Check if the path is found in the list of exclude
+  if (grep { $p =~ m/$_/ } @{$Options{Excludes}}) {
+ if ($Options{Verbose}) {
+ Diag("File '$BugFile' deleted: part of an ignored directory.\n");
+ }
+
+   # File in an ignored directory. Remove it
+   unlink("$Dir/$FName");
+   return;
+  }
+
   UpdatePrefix($BugFile);
 }
 elsif (/$/) {
@@ -1194,6 +1209,12 @@
command. Specifying this option causes the exit status of scan-build to be 1
if it found potential bugs and 0 otherwise.
 
+ --exclude 
+
+   Do not run static analyzer against files found in this
+   directory (You can specify this option multiple times).
+   Could be useful when project contains 3rd party libraries.
+
  --use-cc [compiler path]
  --use-cc=[compiler path]
 
@@ -1698,6 +1719,15 @@
   next;
 }
 
+if ($arg eq "--exclude") {
+  shift @$Args;
+  my $arg = shift @$Args;
+  # Remove the trailing slash if any
+  $arg =~ s|/$||;
+  push @{$Options{Excludes}}, $arg;
+  next;
+}
+
 if ($arg eq "-load-plugin") {
   shift @$Args;
   push @{$Options{PluginsToLoad}}, shift @$Args;


Index: tools/scan-build/bin/scan-build
===
--- tools/scan-build/bin/scan-build
+++ tools/scan-build/bin/scan-build
@@ -58,6 +58,7 @@
   KeepEmpty => 0,# Don't remove output directory even with 0 results.
   EnableCheckers => {},
   DisableCheckers => {},
+  Excludes => [],
   UseCC => undef,# C compiler to use for compilation.
   UseCXX => undef,   # C++ compiler to use for compilation.
   AnalyzerTarget => undef,
@@ -420,6 +421,20 @@
  # The file no longer exists: use the original path.
  $BugFile = $1;
   }
+
+  # Get just the path
+  my $p = dirname($BugFile);
+  # Check if the path is found in the list of exclude
+  if (grep { $p =~ m/$_/ } @{$Options{Excludes}}) {
+ if ($Options{Verbose}) {
+ Diag("File '$BugFile' deleted: part of an ignored directory.\n");
+ }
+
+   # File in an ignored directory. Remove it
+   unlink("$Dir/$FName");
+   return;
+  }
+
   UpdatePrefix($BugFile);
 }
 elsif (/$/) {
@@ -1194,6 +1209,12 @@
command. Specifying this option causes the exit status of scan-build to be 1
if it found potential bugs and 0 otherwise.
 
+ --exclude 
+
+   Do not run static analyzer against files found in this
+   directory (You can specify this option multiple times).
+   Could be useful when project contains 3rd party libraries.
+
  --use-cc [compiler path]
  --use-cc=[compiler path]
 
@@ -1698,6 +1719,15 @@
   next;
 }
 
+if ($arg eq "--exclude") {
+  shift @$Args;
+  my $arg = shift @$Args;
+  # Remove the trailing slash if any
+  $arg =~ s|/$||;
+  push @{$Options{Excludes}}, $arg;
+  next;
+}
+
 if ($arg eq "-load-plugin") {
   shift @$Args;
   push @{$Options{PluginsToLoad}}, shift @$Args;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r342351 - scan-build: remove trailing whitespaces

2018-09-16 Thread Sylvestre Ledru via cfe-commits

Author: sylvestre
Date: Sun Sep 16 12:51:16 2018
New Revision: 342351

URL: http://llvm.org/viewvc/llvm-project?rev=342351&view=rev
Log:
scan-build: remove trailing whitespaces

Modified:
cfe/trunk/tools/scan-build/bin/scan-build

Modified: cfe/trunk/tools/scan-build/bin/scan-build
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build/bin/scan-build?rev=342351&r1=342350&r2=342351&view=diff
==
--- cfe/trunk/tools/scan-build/bin/scan-build (original)
+++ cfe/trunk/tools/scan-build/bin/scan-build Sun Sep 16 12:51:16 2018
@@ -1813,13 +1813,13 @@ Diag("Using '$Clang' for static analysis
 SetHtmlEnv(\@ARGV, $Options{OutputDir});
 
 my @AnalysesToRun;
-foreach (sort { $Options{EnableCheckers}{$a} <=> $Options{EnableCheckers}{$b} 
} 
- keys %{$Options{EnableCheckers}}) { 
+foreach (sort { $Options{EnableCheckers}{$a} <=> $Options{EnableCheckers}{$b} }
+ keys %{$Options{EnableCheckers}}) {
   # Push checkers in order they were enabled.
   push @AnalysesToRun, "-analyzer-checker", $_;
 }
-foreach (sort { $Options{DisableCheckers}{$a} <=> 
$Options{DisableCheckers}{$b} } 
- keys %{$Options{DisableCheckers}}) { 
+foreach (sort { $Options{DisableCheckers}{$a} <=> 
$Options{DisableCheckers}{$b} }
+ keys %{$Options{DisableCheckers}}) {
   # Push checkers in order they were disabled.
   push @AnalysesToRun, "-analyzer-disable-checker", $_;
 }


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52151: Also manages clang-X as tool for scan-build

2018-09-16 Thread Sylvestre Ledru via Phabricator via cfe-commits

sylvestre.ledru added a comment.

@kristina sorry, I missed your comment. I just followed what we are doing with 
gcc. Do you want me to update it?


Repository:
  rC Clang

https://reviews.llvm.org/D52151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52153: scan-build: Add support of the option --exclude like in scan-build-py

2018-09-16 Thread Jonathan Roelofs via Phabricator via cfe-commits

jroelofs accepted this revision.
jroelofs added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D52153



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D52151: Also manages clang-X as tool for scan-build

2018-09-16 Thread Jon Roelofs via cfe-commits

lgtm

On Sun, Sep 16, 2018 at 12:47 PM Sylvestre Ledru via Phabricator <
revi...@reviews.llvm.org> wrote:

> sylvestre.ledru created this revision.
> sylvestre.ledru added a reviewer: jroelofs.
>
> This will make
> scan-build-7 clang-7 -c foo.c &> /dev/null
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D52151
>
> Files:
>   tools/scan-build/bin/scan-build
>
>
> Index: tools/scan-build/bin/scan-build
> ===
> --- tools/scan-build/bin/scan-build
> +++ tools/scan-build/bin/scan-build
> @@ -1079,7 +1079,7 @@
>if ($Cmd =~ /(.*\/?gcc[^\/]*$)/ or
>$Cmd =~ /(.*\/?cc[^\/]*$)/ or
>$Cmd =~ /(.*\/?llvm-gcc[^\/]*$)/ or
> -  $Cmd =~ /(.*\/?clang$)/ or
> +  $Cmd =~ /(.*\/?clang[^\/]*$)/ or
>$Cmd =~ /(.*\/?ccc-analyzer[^\/]*$)/) {
>
>  if (!($Cmd =~ /ccc-analyzer/) and !defined $ENV{"CCC_CC"}) {
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52151: Also manages clang-X as tool for scan-build

2018-09-16 Thread Kristina Brooks via Phabricator via cfe-commits

kristina added a comment.

Just a minor suggestion, I think it would make it more clear as before LLVM 7, 
Clang did not have a version number with the main executable. GCC is slightly 
less consistent with their formats as they usually ship as host compilers with 
various suffixes, but with Clang it can only be `clang`, `clang-7` or 8 (or 9 
in the future) although it's still generally invoked as `clang` unless you have 
multiple installations.


Repository:
  rC Clang

https://reviews.llvm.org/D52151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52151: Also manages clang-X as tool for scan-build

2018-09-16 Thread Jonathan Roelofs via Phabricator via cfe-commits

jroelofs added a comment.

`$triple-clang`, also


Repository:
  rC Clang

https://reviews.llvm.org/D52151



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52137: Added warning for unary minus used with unsigned type

2018-09-16 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added a comment.

In https://reviews.llvm.org/D52137#1236065, @Quuxplusone wrote:

> In https://reviews.llvm.org/D52137#1236053, @rsmith wrote:
>
> > I think we can and should do better about false positives here. If you move 
> > this check to SemaChecking, you can produce the warning in a context where 
> > you know what the final type is -- I don't think there's any reason to warn 
> > if the final type is signed and no wider than the promoted type of the 
> > negation.
>
>
> I share your general concern about false positives, but in the specific case 
> you mentioned—
>
>   void use(int16_t x)
>   uint8_t u8 = 1;
>   use(-u8);
>   
>
> —I think it'd be surprising to maybe 50% of average programmers that `x`'s 
> received value wasn't `int16_t(255)` but rather `int16_t(-1)`.

You may be right. But 50% is a *far* too high false positive rate for a Clang 
warning, so the conclusion should be that we do not warn on that case. Compare 
that to:

  unsigned int n = //...
  long m = -n; // almost certainly a bug (if long is wider than int)

or

  if (-n < x && x < n) // almost certainly a bug

If 50% of people turn this warning off because it uselessly warns on non-bugs, 
we are doing our users a disservice.

We need to have a clear idea of what classes of bugs we want to catch here. 
Unary negation applied to an unsigned type does not deserve a warning by 
itself, but there are certainly some contexts in which we should warn. The 
interesting part is identifying them.

Comment at: test/Sema/unary-minus-unsigned.c:8
+
+  unsigned int a2 = -a;   // expected-warning {{unary minus operator 
applied to type 'unsigned int', result value is still unsigned}}
+  unsigned long b2 = -b;  // expected-warning {{unary minus operator 
applied to type 'unsigned long', result value is still unsigned}}

It's unreasonable to warn on this. The user clearly meant the result type to be 
unsigned: they wrote that type on the left-hand side.

Comment at: test/Sema/unary-minus-unsigned.c:9-10
+  unsigned int a2 = -a;   // expected-warning {{unary minus operator 
applied to type 'unsigned int', result value is still unsigned}}
+  unsigned long b2 = -b;  // expected-warning {{unary minus operator 
applied to type 'unsigned long', result value is still unsigned}}
+  unsigned long long c2 = -c; // expected-warning {{unary minus operator 
applied to type 'unsigned long long', result value is still unsigned}}
+

For these cases, we should warn that the high order bits are zeroes, since 
that's the surprising thing, not that the result is unsigned (which was 
obviously intended).

Comment at: test/Sema/unary-minus-unsigned.c:12
+
+  int a3 = -a;   // expected-warning {{unary minus operator applied to 
type 'unsigned int', result value is still unsigned}}
+  long b3 = -b;  // expected-warning {{unary minus operator applied to 
type 'unsigned long', result value is still unsigned}}

I don't see any point in warning here. We guarantee that the result is exactly 
the same as that of

```
int a3 = -(int)a;
```

Comment at: test/Sema/unary-minus-unsigned.c:13-14
+  int a3 = -a;   // expected-warning {{unary minus operator applied to 
type 'unsigned int', result value is still unsigned}}
+  long b3 = -b;  // expected-warning {{unary minus operator applied to 
type 'unsigned long', result value is still unsigned}}
+  long long c3 = -c; // expected-warning {{unary minus operator applied to 
type 'unsigned long long', result value is still unsigned}}
+

This is a much less helpful warning than we could give; a better warning would 
be that the resulting value is always non-negative. (With appropriate 
modifications when `sizeof(int)` == `sizeof(long)`.)

Comment at: test/Sema/unary-minus-unsigned.c:16-18
+  unsigned int a4 = -1u; // expected-warning {{unary minus operator 
applied to type 'unsigned int', result value is still unsigned}}
+  unsigned long b4 = -1ul;   // expected-warning {{unary minus operator 
applied to type 'unsigned long', result value is still unsigned}}
+  unsigned long long c4 = -1ull; // expected-warning {{unary minus operator 
applied to type 'unsigned long long', result value is still unsigned}}

Warning on these is unreasonable. These are idiomatic ways of writing certain 
unsigned constants.

https://reviews.llvm.org/D52137

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r342353 - [NFC] Minor refactoring to setup the stage for supporting pointers in ExprMutationAnalyzer

2018-09-16 Thread Shuai Wang via cfe-commits

Author: shuaiwang
Date: Sun Sep 16 14:09:50 2018
New Revision: 342353

URL: http://llvm.org/viewvc/llvm-project?rev=342353&view=rev
Log:
[NFC] Minor refactoring to setup the stage for supporting pointers in 
ExprMutationAnalyzer

Modified:
cfe/trunk/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp

Modified: cfe/trunk/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h?rev=342353&r1=342352&r2=342353&view=diff
==
--- cfe/trunk/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h Sun Sep 16 
14:09:50 2018
@@ -31,13 +31,32 @@ public:
   const Stmt *findMutation(const Expr *Exp);
   const Stmt *findMutation(const Decl *Dec);
 
+  bool isPointeeMutated(const Expr *Exp) {
+return findPointeeMutation(Exp) != nullptr;
+  }
+  bool isPointeeMutated(const Decl *Dec) {
+return findPointeeMutation(Dec) != nullptr;
+  }
+  const Stmt *findPointeeMutation(const Expr *Exp);
+  const Stmt *findPointeeMutation(const Decl *Dec);
+
 private:
+  using MutationFinder = const Stmt *(ExprMutationAnalyzer::*)(const Expr *);
   using ResultMap = llvm::DenseMap;
 
+  const Stmt *findMutationMemoized(const Expr *Exp,
+   llvm::ArrayRef Finders,
+   ResultMap &MemoizedResults);
+  const Stmt *tryEachDeclRef(const Decl *Dec, MutationFinder Finder);
+
   bool isUnevaluated(const Expr *Exp);
 
   const Stmt *findExprMutation(ArrayRef Matches);
   const Stmt *findDeclMutation(ArrayRef Matches);
+  const Stmt *
+  findExprPointeeMutation(ArrayRef Matches);
+  const Stmt *
+  findDeclPointeeMutation(ArrayRef Matches);
 
   const Stmt *findDirectMutation(const Expr *Exp);
   const Stmt *findMemberMutation(const Expr *Exp);
@@ -53,6 +72,7 @@ private:
  std::unique_ptr>
   FuncParmAnalyzer;
   ResultMap Results;
+  ResultMap PointeeResults;
 };
 
 // A convenient wrapper around ExprMutationAnalyzer for analyzing function

Modified: cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp?rev=342353&r1=342352&r2=342353&view=diff
==
--- cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp (original)
+++ cfe/trunk/lib/Analysis/ExprMutationAnalyzer.cpp Sun Sep 16 14:09:50 2018
@@ -64,33 +64,83 @@ const auto isMoveOnly = [] {
unless(isDeleted()));
 };
 
+template  struct NodeID;
+template <> struct NodeID { static const std::string value; };
+template <> struct NodeID { static const std::string value; };
+const std::string NodeID::value = "expr";
+const std::string NodeID::value = "decl";
+
+template 
+const Stmt *tryEachMatch(ArrayRef Matches,
+ ExprMutationAnalyzer *Analyzer, F Finder) {
+  const StringRef ID = NodeID::value;
+  for (const auto &Nodes : Matches) {
+if (const Stmt *S = (Analyzer->*Finder)(Nodes.getNodeAs(ID)))
+  return S;
+  }
+  return nullptr;
+}
+
 } // namespace
 
 const Stmt *ExprMutationAnalyzer::findMutation(const Expr *Exp) {
-  const auto Memoized = Results.find(Exp);
-  if (Memoized != Results.end())
+  return findMutationMemoized(Exp,
+  {&ExprMutationAnalyzer::findDirectMutation,
+   &ExprMutationAnalyzer::findMemberMutation,
+   &ExprMutationAnalyzer::findArrayElementMutation,
+   &ExprMutationAnalyzer::findCastMutation,
+   &ExprMutationAnalyzer::findRangeLoopMutation,
+   &ExprMutationAnalyzer::findReferenceMutation,
+   &ExprMutationAnalyzer::findFunctionArgMutation},
+  Results);
+}
+
+const Stmt *ExprMutationAnalyzer::findMutation(const Decl *Dec) {
+  return tryEachDeclRef(Dec, &ExprMutationAnalyzer::findMutation);
+}
+
+const Stmt *ExprMutationAnalyzer::findPointeeMutation(const Expr *Exp) {
+  return findMutationMemoized(Exp, {/*TODO*/}, PointeeResults);
+}
+
+const Stmt *ExprMutationAnalyzer::findPointeeMutation(const Decl *Dec) {
+  return tryEachDeclRef(Dec, &ExprMutationAnalyzer::findPointeeMutation);
+}
+
+const Stmt *ExprMutationAnalyzer::findMutationMemoized(
+const Expr *Exp, llvm::ArrayRef Finders,
+ResultMap &MemoizedResults) {
+  const auto Memoized = MemoizedResults.find(Exp);
+  if (Memoized != MemoizedResults.end())
 return Memoized->second;
 
   if (isUnevaluated(Exp))
-return Results[Exp] = nullptr;
+return MemoizedResults[Exp] = nullptr;
 
-  for (const auto &Finder : {&ExprMutationAnalyzer::findDirectMutation,

[PATCH] D52136: [clang-tidy] Add modernize-concat-nested-namespaces check

2018-09-16 Thread Wojtek Gumuła via Phabricator via cfe-commits

wgml updated this revision to Diff 165698.
wgml added a comment.

One last typo.


https://reviews.llvm.org/D52136

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ConcatNestedNamespacesCheck.cpp
  clang-tidy/modernize/ConcatNestedNamespacesCheck.h
  clang-tidy/modernize/ModernizeTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
  test/clang-tidy/modernize-concat-nested-namespaces.cpp

Index: test/clang-tidy/modernize-concat-nested-namespaces.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-concat-nested-namespaces.cpp
@@ -0,0 +1,161 @@
+// RUN: %check_clang_tidy %s modernize-concat-nested-namespaces %t -- -- -std=c++17
+
+namespace n1 {}
+
+namespace n2 {
+namespace n3 {
+void t();
+}
+namespace n4 {
+void t();
+}
+} // namespace n2
+
+namespace n5 {
+inline namespace n6 {
+void t();
+}
+} // namespace n5
+
+namespace n7 {
+void t();
+
+namespace n8 {
+void t();
+}
+} // namespace n7
+
+namespace n9 {
+namespace n10 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n9::n10
+void t();
+} // namespace n10
+} // namespace n9
+// CHECK-FIXES: }
+
+namespace n11 {
+namespace n12 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n11::n12
+namespace n13 {
+void t();
+}
+namespace n14 {
+void t();
+}
+} // namespace n12
+} // namespace n11
+// CHECK-FIXES: }
+
+namespace n15 {
+namespace n16 {
+void t();
+}
+
+inline namespace n17 {
+void t();
+}
+
+namespace n18 {
+namespace n19 {
+namespace n20 {
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n18::n19::n20
+void t();
+} // namespace n20
+} // namespace n19
+} // namespace n18
+// CHECK-FIXES: }
+
+namespace n21 {
+void t();
+}
+} // namespace n15
+
+namespace n22 {
+namespace {
+void t();
+}
+} // namespace n22
+
+namespace n23 {
+namespace {
+namespace n24 {
+namespace n25 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n24::n25
+void t();
+} // namespace n25
+} // namespace n24
+// CHECK-FIXES: }
+} // namespace
+} // namespace n23
+
+namespace n26::n27 {
+namespace n28 {
+namespace n29::n30 {
+// CHECK-MESSAGES: :[[@LINE-3]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n26::n27::n28::n29::n30
+void t() {}
+} // namespace n29::n30
+} // namespace n28
+} // namespace n26::n27
+// CHECK-FIXES: }
+
+namespace n31 {
+namespace n32 {}
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+} // namespace n31
+// CHECK-FIXES-EMPTY
+
+namespace n33 {
+namespace n34 {
+namespace n35 {}
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+} // namespace n34
+// CHECK-FIXES-EMPTY
+namespace n36 {
+void t();
+}
+} // namespace n33
+
+namespace n37::n38 {
+void t();
+}
+
+#define IEXIST
+namespace n39 {
+namespace n40 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n39::n40
+#ifdef IEXIST
+void t() {}
+#endif
+} // namespace n40
+} // namespace n39
+// CHECK-FIXES: }
+
+namespace n41 {
+namespace n42 {
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: Nested namespaces can be concatenated [modernize-concat-nested-namespaces]
+// CHECK-FIXES: namespace n41::n42
+#ifdef IDONTEXIST
+void t() {}
+#endif
+} // namespace n42
+} // namespace n41
+// CHECK-FIXES: }
+
+int main() {
+  n26::n27::n28::n29::n30::t();
+#ifdef IEXIST
+  n39::n40::t();
+#endif
+
+#ifdef IDONTEXIST
+  n41::n42::t();
+#endif
+
+  return 0;
+}
Index: docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-concat-nested-namespaces.rst
@@ -0,0 +1,49 @@
+.. title:: clang-tidy - modernize-concat-nested-namespaces
+
+modernize-concat-nested-namespaces
+==
+
+Checks for use of nested namespaces in a form of ``namespace a { namespace b { ... } }``
+and offers change to syntax introduced in C++17: ``namespace a::b { ... }``.
+Inlined namespaces are not modified.
+
+For example:
+
+.. code-block:: c++
+
+  namespace n1 {
+  namespace n2 {
+  void t();
+  }
+  }
+
+  namespace n3 {
+  namespace n4 {
+  namespace n5 {
+  void t();
+  }
+  }
+  namespace n6 {
+  namespace n7 {
+  void t();
+  }
+  }
+  }
+
+Will be modified to:
+
+.. code-block:: c++
+
+  namespace n1::n2 {
+  void t();
+  }
+
+  namespace n3

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

2018-09-16 Thread Tolga Mizrak via Phabricator via cfe-commits

to-miz updated this revision to Diff 165700.
to-miz added a comment.

PPBranchLevel can be negative, while Line->Level is unsigned. Added check to 
ensure that PPBranchLevel is positive before adding to Line->Level.


https://reviews.llvm.org/D52150

Files:
  docs/ClangFormatStyleOptions.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
@@ -1754,8 +1754,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",
@@ -2846,22 +2846,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"
@@ -2871,6 +2874,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"
+   "}";
+EXPECT_EQ(Expected, format(ToFormat, Style));
+EXPECT_EQ(Expected, format(Expected, Style));
+  }
+  {
+

[PATCH] D52157: [ASTMatchers] Let isArrow also support UnresolvedMemberExpr, CXXDependentScopeMemberExpr

2018-09-16 Thread Shuai Wang via Phabricator via cfe-commits

shuaiwang created this revision.
shuaiwang added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D52157

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  unittests/ASTMatchers/Dynamic/RegistryTest.cpp

Index: unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -440,7 +440,8 @@
   Error.get()).isNull());
   EXPECT_EQ("Incorrect type for arg 1. "
 "(Expected = Matcher) != "
-"(Actual = Matcher&Matcher)",
+"(Actual = Matcher&Matcher"
+")",
 Error->toString());
 }
 
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -27,7 +27,7 @@
nullptr));
 
   // Do not accept non-toplevel matchers.
-  EXPECT_FALSE(Finder.addDynamicMatcher(isArrow(), nullptr));
+  EXPECT_FALSE(Finder.addDynamicMatcher(isMain(), nullptr));
   EXPECT_FALSE(Finder.addDynamicMatcher(hasName("x"), nullptr));
 }
 
Index: unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -765,6 +765,11 @@
   memberExpr(isArrow(;
   EXPECT_TRUE(notMatches("class Y { void x() { (*this).y; } int y; };",
  memberExpr(isArrow(;
+  EXPECT_TRUE(matches("template  class Y { void x() { this->m; } };",
+  cxxDependentScopeMemberExpr(isArrow(;
+  EXPECT_TRUE(
+  notMatches("template  class Y { void x() { (*this).m; } };",
+ cxxDependentScopeMemberExpr(isArrow(;
 }
 
 TEST(IsArrow, MatchesStaticMemberVariablesViaArrow) {
@@ -783,6 +788,14 @@
   memberExpr(isArrow(;
   EXPECT_TRUE(notMatches("class Y { void x() { Y y; y.x(); } };",
  memberExpr(isArrow(;
+  EXPECT_TRUE(
+  matches("class Y { template  void x() { this->x(); } };",
+  unresolvedMemberExpr(isArrow(;
+  EXPECT_TRUE(matches("class Y { template  void x() { x(); } };",
+  unresolvedMemberExpr(isArrow(;
+  EXPECT_TRUE(
+  notMatches("class Y { template  void x() { (*this).x(); } };",
+ unresolvedMemberExpr(isArrow(;
 }
 
 TEST(ConversionDeclaration, IsExplicit) {
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -4703,7 +4703,9 @@
 /// \endcode
 /// memberExpr(isArrow())
 ///   matches this->x, x, y.x, a, this->b
-AST_MATCHER(MemberExpr, isArrow) {
+AST_POLYMORPHIC_MATCHER(
+isArrow, AST_POLYMORPHIC_SUPPORTED_TYPES(MemberExpr, UnresolvedMemberExpr,
+ CXXDependentScopeMemberExpr)) {
   return Node.isArrow();
 }
 
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -2235,6 +2235,23 @@
 
 
 
+MatcherCXXDependentScopeMemberExpr>isArrow
+Matches member expressions that are called with '->' as opposed
+to '.'.
+
+Member calls on the implicit this pointer match as called with '->'.
+
+Given
+  class Y {
+void x() { this->x(); x(); Y y; y.x(); a; this->b; Y::b; }
+int a;
+static int b;
+  };
+memberExpr(isArrow())
+  matches this->x, x, y.x, a, this->b
+
+
+
 MatcherCXXMethodDecl>isConst
 Matches if the given method declaration is const.
 
@@ -3886,6 +3903,23 @@
 
 
 
+MatcherUnresolvedMemberExpr>isArrow
+Matches member expressions that are called with '->' as opposed
+to '.'.
+
+Member calls on the implicit this pointer match as called with '->'.
+
+Given
+  class Y {
+void x() { this->x(); x(); Y y; y.x(); a; this->b; Y::b; }
+int a;
+static int b;
+  };
+memberExpr(isArrow())
+  matches this->x, x, y.x, a, this->b
+
+
+
 MatcherVarDecl>hasAutomaticStorageDuration
 Matches a variable declaration that has automatic storage duration.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-b

[PATCH] D51633: [ASTImporter] Added error handling for AST import.

2018-09-16 Thread Aleksei Sidorin via Phabricator via cfe-commits

a_sidorin added a comment.

Hi Gabor,

To make the review proces faster, you can split the review on separate parts 
for Stmts, Decls, Types, etc. Otherwise, the review can last for too long.




Comment at: lib/AST/ASTImporter.cpp:162
+return llvm::make_error();
+return llvm::Error::success();
+  }

Can we get rid from namespace specifier usages on Error and None? As I see, 
even in this patch Error is used without it sometimes.



Comment at: lib/AST/ASTImporter.cpp:417
 
-bool ImportDefinition(RecordDecl *From, RecordDecl *To,
-  ImportDefinitionKind Kind = IDK_Default);
-bool ImportDefinition(VarDecl *From, VarDecl *To,
-  ImportDefinitionKind Kind = IDK_Default);
-bool ImportDefinition(EnumDecl *From, EnumDecl *To,
-  ImportDefinitionKind Kind = IDK_Default);
-bool ImportDefinition(ObjCInterfaceDecl *From, ObjCInterfaceDecl *To,
-  ImportDefinitionKind Kind = IDK_Default);
-bool ImportDefinition(ObjCProtocolDecl *From, ObjCProtocolDecl *To,
-  ImportDefinitionKind Kind = IDK_Default);
-TemplateParameterList *ImportTemplateParameterList(
+Error ImportDefinition(
+RecordDecl *From, RecordDecl *To,

The changes for argument indentation look redundant. Is it a result of 
clang-format?



Comment at: lib/AST/ASTImporter.cpp:840
+template <>
+Expected
+ASTNodeImporter::import(const TemplateArgumentLoc &TALoc) {

Could you please add some newlines to this function? Its control flow is not 
trivial so some blocks need to be separated.


Repository:
  rC Clang

https://reviews.llvm.org/D51633



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52158: [clang-tidy] Remove duplicated logic in UnnecessaryValueParamCheck and use FunctionParmMutationAnalyzer instead.

2018-09-16 Thread Shuai Wang via Phabricator via cfe-commits

shuaiwang created this revision.
shuaiwang added reviewers: alexfh, JonasToth.
Herald added subscribers: cfe-commits, Szelethus, a.sidorin, chrib, 
kristof.beyls, xazax.hun.
Herald added a reviewer: george.karpenkov.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52158

Files:
  clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tidy/performance/UnnecessaryValueParamCheck.h


Index: clang-tidy/performance/UnnecessaryValueParamCheck.h
===
--- clang-tidy/performance/UnnecessaryValueParamCheck.h
+++ clang-tidy/performance/UnnecessaryValueParamCheck.h
@@ -12,6 +12,7 @@
 
 #include "../ClangTidy.h"
 #include "../utils/IncludeInserter.h"
+#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 
 namespace clang {
 namespace tidy {
@@ -29,11 +30,14 @@
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   void registerPPCallbacks(CompilerInstance &Compiler) override;
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void onEndOfTranslationUnit() override;
 
 private:
   void handleMoveFix(const ParmVarDecl &Var, const DeclRefExpr &CopyArgument,
  const ASTContext &Context);
 
+  llvm::DenseMap
+  MutationAnalyzers;
   std::unique_ptr Inserter;
   const utils::IncludeSorter::IncludeStyle IncludeStyle;
 };
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -13,7 +13,6 @@
 #include "../utils/FixItHintUtils.h"
 #include "../utils/Matchers.h"
 #include "../utils/TypeTraits.h"
-#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
@@ -92,21 +91,11 @@
   const auto *Param = Result.Nodes.getNodeAs("param");
   const auto *Function = Result.Nodes.getNodeAs("functionDecl");
 
-  // Do not trigger on non-const value parameters when they are mutated either
-  // within the function body or within init expression(s) when the function is
-  // a ctor.
-  if (ExprMutationAnalyzer(*Function->getBody(), *Result.Context)
-  .isMutated(Param))
+  FunctionParmMutationAnalyzer &Analyzer =
+  MutationAnalyzers.try_emplace(Function, *Function, *Result.Context)
+  .first->second;
+  if (Analyzer.isMutated(Param))
 return;
-  // CXXCtorInitializer might also mutate Param but they're not part of 
function
-  // body, so check them separately here.
-  if (const auto *Ctor = dyn_cast(Function)) {
-for (const auto *Init : Ctor->inits()) {
-  if (ExprMutationAnalyzer(*Init->getInit(), *Result.Context)
-  .isMutated(Param))
-return;
-}
-  }
 
   const bool IsConstQualified =
   Param->getType().getCanonicalType().isConstQualified();
@@ -186,6 +175,10 @@
 utils::IncludeSorter::toString(IncludeStyle));
 }
 
+void UnnecessaryValueParamCheck::onEndOfTranslationUnit() {
+  MutationAnalyzers.clear();
+}
+
 void UnnecessaryValueParamCheck::handleMoveFix(const ParmVarDecl &Var,
const DeclRefExpr &CopyArgument,
const ASTContext &Context) {


Index: clang-tidy/performance/UnnecessaryValueParamCheck.h
===
--- clang-tidy/performance/UnnecessaryValueParamCheck.h
+++ clang-tidy/performance/UnnecessaryValueParamCheck.h
@@ -12,6 +12,7 @@
 
 #include "../ClangTidy.h"
 #include "../utils/IncludeInserter.h"
+#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 
 namespace clang {
 namespace tidy {
@@ -29,11 +30,14 @@
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   void registerPPCallbacks(CompilerInstance &Compiler) override;
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void onEndOfTranslationUnit() override;
 
 private:
   void handleMoveFix(const ParmVarDecl &Var, const DeclRefExpr &CopyArgument,
  const ASTContext &Context);
 
+  llvm::DenseMap
+  MutationAnalyzers;
   std::unique_ptr Inserter;
   const utils::IncludeSorter::IncludeStyle IncludeStyle;
 };
Index: clang-tidy/performance/UnnecessaryValueParamCheck.cpp
===
--- clang-tidy/performance/UnnecessaryValueParamCheck.cpp
+++ clang-tidy/performance/UnnecessaryValueParamCheck.cpp
@@ -13,7 +13,6 @@
 #include "../utils/FixItHintUtils.h"
 #include "../utils/Matchers.h"
 #include "../utils/TypeTraits.h"
-#include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
@@ -92,21 +91,11 @@
   const auto *Param = Result.Nodes.getNodeAs("param");
   const auto

[PATCH] D52159: [Lexer] Add xray_instrument feature

2018-09-16 Thread Petr Hosek via Phabricator via cfe-commits

phosek updated this revision to Diff 165705.
phosek edited the summary of this revision.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D52159

Files:
  clang/include/clang/Basic/Features.def
  clang/test/Lexer/has_feature_xray_instrument.cpp


Index: clang/test/Lexer/has_feature_xray_instrument.cpp
===
--- /dev/null
+++ clang/test/Lexer/has_feature_xray_instrument.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -E -fxray-instrument %s -o - | FileCheck 
--check-prefix=CHECK-XRAY %s
+// RUN: %clang_cc1 -E  %s -o - | FileCheck --check-prefix=CHECK-NO-XRAY %s
+
+#if __has_feature(xray_instrument)
+int XRayInstrumentEnabled();
+#else
+int XRayInstrumentDisabled();
+#endif
+
+// CHECK-XRAY: XRayInstrumentEnabled
+// CHECK-NO-XRAY: XRayInstrumentDisabled
Index: clang/include/clang/Basic/Features.def
===
--- clang/include/clang/Basic/Features.def
+++ clang/include/clang/Basic/Features.def
@@ -37,6 +37,7 @@
 FEATURE(hwaddress_sanitizer,
 LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress |
SanitizerKind::KernelHWAddress))
+FEATURE(xray_instrument, LangOpts.XRayInstrument)
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)


Index: clang/test/Lexer/has_feature_xray_instrument.cpp
===
--- /dev/null
+++ clang/test/Lexer/has_feature_xray_instrument.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -E -fxray-instrument %s -o - | FileCheck --check-prefix=CHECK-XRAY %s
+// RUN: %clang_cc1 -E  %s -o - | FileCheck --check-prefix=CHECK-NO-XRAY %s
+
+#if __has_feature(xray_instrument)
+int XRayInstrumentEnabled();
+#else
+int XRayInstrumentDisabled();
+#endif
+
+// CHECK-XRAY: XRayInstrumentEnabled
+// CHECK-NO-XRAY: XRayInstrumentDisabled
Index: clang/include/clang/Basic/Features.def
===
--- clang/include/clang/Basic/Features.def
+++ clang/include/clang/Basic/Features.def
@@ -37,6 +37,7 @@
 FEATURE(hwaddress_sanitizer,
 LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress |
SanitizerKind::KernelHWAddress))
+FEATURE(xray_instrument, LangOpts.XRayInstrument)
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52160: [Driver] Support XRay on Fuchsia

2018-09-16 Thread Petr Hosek via Phabricator via cfe-commits

phosek created this revision.
phosek added reviewers: mcgrathr, dberris.
Herald added a reviewer: javed.absar.
Herald added a subscriber: cfe-commits.

This enables support for XRay in Fuchsia Clang driver.


Repository:
  rC Clang

https://reviews.llvm.org/D52160

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/XRayArgs.cpp
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/libclang_rt.xray-basic.a
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/libclang_rt.xray.a
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/libclang_rt.xray-basic.a
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/libclang_rt.xray.a
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -135,6 +135,24 @@
 // CHECK-SCUDO-SHARED: "-fsanitize=safe-stack,scudo"
 // CHECK-SCUDO-SHARED: "{{.*[/\\]}}libclang_rt.scudo.so"
 
+// RUN: %clang %s -### --target=x86_64-fuchsia \
+// RUN: -fxray-instrument -fxray-modes=xray-basic \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-XRAY-X86
+// CHECK-XRAY-X86: "-fxray-instrument"
+// CHECK-XRAY-X86: "{{.*[/\\]}}libclang_rt.xray.a"
+// CHECK-XRAY-X86: "{{.*[/\\]}}libclang_rt.xray-basic.a"
+
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fxray-instrument -fxray-modes=xray-basic \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-XRAY-AARCH64
+// CHECK-XRAY-AARCH64: "-fxray-instrument"
+// CHECK-XRAY-AARCH64: "{{.*[/\\]}}libclang_rt.xray.a"
+// CHECK-XRAY-AARCH64: "{{.*[/\\]}}libclang_rt.xray-basic.a"
+
 // RUN: %clang %s -### --target=aarch64-fuchsia \
 // RUN: -O3 -flto -mcpu=cortex-a53 2>&1 \
 // RUN: -fuse-ld=lld \
Index: clang/lib/Driver/XRayArgs.cpp
===
--- clang/lib/Driver/XRayArgs.cpp
+++ clang/lib/Driver/XRayArgs.cpp
@@ -58,6 +58,15 @@
 D.Diag(diag::err_drv_clang_unsupported)
 << (std::string(XRayInstrumentOption) + " on " + Triple.str());
   }
+} else if (Triple.getOS() == llvm::Triple::Fuchsia) {
+  switch (Triple.getArch()) {
+  case llvm::Triple::x86_64:
+  case llvm::Triple::aarch64:
+break;
+  default:
+D.Diag(diag::err_drv_clang_unsupported)
+<< (std::string(XRayInstrumentOption) + " on " + Triple.str());
+  }
 } else {
   D.Diag(diag::err_drv_clang_unsupported)
   << (std::string(XRayInstrumentOption) + " on " + Triple.str());
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -106,8 +106,8 @@
   D.getLTOMode() == LTOK_Thin);
   }
 
-  addSanitizerRuntimes(ToolChain, Args, CmdArgs);
-
+  bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
+  bool NeedsXRayDeps = addXRayRuntime(ToolChain, Args, CmdArgs);
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
   ToolChain.addProfileRTLibs(Args, CmdArgs);
 
@@ -128,6 +128,12 @@
   CmdArgs.push_back("-lm");
 }
 
+if (NeedsSanitizerDeps)
+  linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
+
+if (NeedsXRayDeps)
+  linkXRayRuntimeDeps(ToolChain, CmdArgs);
+
 AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
 
 if (Args.hasArg(options::OPT_pthread) ||


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -135,6 +135,24 @@
 // CHECK-SCUDO-SHARED: "-fsanitize=safe-stack,scudo"
 // CHECK-SCUDO-SHARED: "{{.*[/\\]}}libclang_rt.scudo.so"
 
+// RUN: %clang %s -### --target=x86_64-fuchsia \
+// RUN: -fxray-instrument -fxray-modes=xray-basic \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-XRAY-X86
+// CHECK-XRAY-X86: "-fxray-instrument"
+// CHECK-XRAY-X86: "{{.*[/\\]}}libclang_rt.xray.a"
+// CHECK-XRAY-X86: "{{.*[/\\]}}libclang_rt.xray-basic.a"
+
+// RUN: %clang %s -### --target=aarch64-fuchsia \
+// RUN: -fxray-instrument -fxray-modes=xray-basic \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1 \
+// RUN: | FileCheck %s -check-prefix=CHECK-XRAY-AARCH64
+// CHECK-XRAY-AARCH64: "-fxray-instrument"
+// CHECK-XRAY-AARCH64: "{{.*[/\\]}}libclang_rt.xray.a"
+// CHECK-XRAY-AARCH64: "{{.*[/\\]}}libclang_rt.xray-basic.a"
+
 // RUN: %clang %s -### --target=aarch64-fu

r342358 - [Lexer] Add xray_instrument feature

2018-09-16 Thread Petr Hosek via cfe-commits

Author: phosek
Date: Sun Sep 16 22:25:47 2018
New Revision: 342358

URL: http://llvm.org/viewvc/llvm-project?rev=342358&view=rev
Log:
[Lexer] Add xray_instrument feature

This can be used to detect whether the code is being built with XRay
instrumentation using the __has_feature(xray_instrument) predicate.

Differential Revision: https://reviews.llvm.org/D52159

Added:
cfe/trunk/test/Lexer/has_feature_xray_instrument.cpp
Modified:
cfe/trunk/include/clang/Basic/Features.def

Modified: cfe/trunk/include/clang/Basic/Features.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Features.def?rev=342358&r1=342357&r2=342358&view=diff
==
--- cfe/trunk/include/clang/Basic/Features.def (original)
+++ cfe/trunk/include/clang/Basic/Features.def Sun Sep 16 22:25:47 2018
@@ -37,6 +37,7 @@ FEATURE(address_sanitizer,
 FEATURE(hwaddress_sanitizer,
 LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress |
SanitizerKind::KernelHWAddress))
+FEATURE(xray_instrument, LangOpts.XRayInstrument)
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)

Added: cfe/trunk/test/Lexer/has_feature_xray_instrument.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/has_feature_xray_instrument.cpp?rev=342358&view=auto
==
--- cfe/trunk/test/Lexer/has_feature_xray_instrument.cpp (added)
+++ cfe/trunk/test/Lexer/has_feature_xray_instrument.cpp Sun Sep 16 22:25:47 
2018
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -E -fxray-instrument %s -o - | FileCheck 
--check-prefix=CHECK-XRAY %s
+// RUN: %clang_cc1 -E  %s -o - | FileCheck --check-prefix=CHECK-NO-XRAY %s
+
+#if __has_feature(xray_instrument)
+int XRayInstrumentEnabled();
+#else
+int XRayInstrumentDisabled();
+#endif
+
+// CHECK-XRAY: XRayInstrumentEnabled
+// CHECK-NO-XRAY: XRayInstrumentDisabled


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52159: [Lexer] Add xray_instrument feature

2018-09-16 Thread Petr Hosek via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rC342358: [Lexer] Add xray_instrument feature (authored by 
phosek, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D52159?vs=165705&id=165712#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52159

Files:
  include/clang/Basic/Features.def
  test/Lexer/has_feature_xray_instrument.cpp


Index: include/clang/Basic/Features.def
===
--- include/clang/Basic/Features.def
+++ include/clang/Basic/Features.def
@@ -37,6 +37,7 @@
 FEATURE(hwaddress_sanitizer,
 LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress |
SanitizerKind::KernelHWAddress))
+FEATURE(xray_instrument, LangOpts.XRayInstrument)
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)
Index: test/Lexer/has_feature_xray_instrument.cpp
===
--- test/Lexer/has_feature_xray_instrument.cpp
+++ test/Lexer/has_feature_xray_instrument.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -E -fxray-instrument %s -o - | FileCheck 
--check-prefix=CHECK-XRAY %s
+// RUN: %clang_cc1 -E  %s -o - | FileCheck --check-prefix=CHECK-NO-XRAY %s
+
+#if __has_feature(xray_instrument)
+int XRayInstrumentEnabled();
+#else
+int XRayInstrumentDisabled();
+#endif
+
+// CHECK-XRAY: XRayInstrumentEnabled
+// CHECK-NO-XRAY: XRayInstrumentDisabled


Index: include/clang/Basic/Features.def
===
--- include/clang/Basic/Features.def
+++ include/clang/Basic/Features.def
@@ -37,6 +37,7 @@
 FEATURE(hwaddress_sanitizer,
 LangOpts.Sanitize.hasOneOf(SanitizerKind::HWAddress |
SanitizerKind::KernelHWAddress))
+FEATURE(xray_instrument, LangOpts.XRayInstrument)
 FEATURE(assume_nonnull, true)
 FEATURE(attribute_analyzer_noreturn, true)
 FEATURE(attribute_availability, true)
Index: test/Lexer/has_feature_xray_instrument.cpp
===
--- test/Lexer/has_feature_xray_instrument.cpp
+++ test/Lexer/has_feature_xray_instrument.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -E -fxray-instrument %s -o - | FileCheck --check-prefix=CHECK-XRAY %s
+// RUN: %clang_cc1 -E  %s -o - | FileCheck --check-prefix=CHECK-NO-XRAY %s
+
+#if __has_feature(xray_instrument)
+int XRayInstrumentEnabled();
+#else
+int XRayInstrumentDisabled();
+#endif
+
+// CHECK-XRAY: XRayInstrumentEnabled
+// CHECK-NO-XRAY: XRayInstrumentDisabled
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52137: Added warning for unary minus used with unsigned type

2018-09-16 Thread Dávid Bolvanský via Phabricator via cfe-commits

xbolva00 added a comment.

"High bits are zeros" and "result is always non negative" are good candidatates 
to be catched here. Thanks for detailed review, Richard. Now I will look on it.


https://reviews.llvm.org/D52137



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r342359 - scan-build: Add support of the option --exclude like in scan-build-py

2018-09-16 Thread Sylvestre Ledru via cfe-commits

Author: sylvestre
Date: Sun Sep 16 23:31:46 2018
New Revision: 342359

URL: http://llvm.org/viewvc/llvm-project?rev=342359&view=rev
Log:
scan-build: Add support of the option --exclude like in scan-build-py

Summary:
To exclude thirdparty code.

To test:
With /tmp/foo.c
  
```
void test() {
int x;
x = 1; // warn
}  
```

```
$ scan-build --exclude non-existing/  --exclude /tmp/ -v gcc -c foo.c   




scan-build: Using '/usr/lib/llvm-7/bin/clang' for static analysis
scan-build: Emitting reports for this run to 
'/tmp/scan-build-2018-09-16-214531-8410-1'.
foo.c:3:3: warning: Value stored to 'x' is never read
  x = 1; // warn
  ^   ~
1 warning generated.
scan-build: File '/tmp/foo.c' deleted: part of an ignored directory.
scan-build: 0 bugs found.
```

Reviewers: jroelofs

Reviewed By: jroelofs

Subscribers: whisperity, cfe-commits

Differential Revision: https://reviews.llvm.org/D52153

Modified:
cfe/trunk/tools/scan-build/bin/scan-build

Modified: cfe/trunk/tools/scan-build/bin/scan-build
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build/bin/scan-build?rev=342359&r1=342358&r2=342359&view=diff
==
--- cfe/trunk/tools/scan-build/bin/scan-build (original)
+++ cfe/trunk/tools/scan-build/bin/scan-build Sun Sep 16 23:31:46 2018
@@ -58,6 +58,7 @@ my %Options = (
   KeepEmpty => 0,# Don't remove output directory even with 0 
results.
   EnableCheckers => {},
   DisableCheckers => {},
+  Excludes => [],
   UseCC => undef,# C compiler to use for compilation.
   UseCXX => undef,   # C++ compiler to use for compilation.
   AnalyzerTarget => undef,
@@ -420,6 +421,20 @@ sub ScanFile {
  # The file no longer exists: use the original path.
  $BugFile = $1;
   }
+
+  # Get just the path
+  my $p = dirname($BugFile);
+  # Check if the path is found in the list of exclude
+  if (grep { $p =~ m/$_/ } @{$Options{Excludes}}) {
+ if ($Options{Verbose}) {
+ Diag("File '$BugFile' deleted: part of an ignored directory.\n");
+ }
+
+   # File in an ignored directory. Remove it
+   unlink("$Dir/$FName");
+   return;
+  }
+
   UpdatePrefix($BugFile);
 }
 elsif (/$/) {
@@ -1194,6 +1209,12 @@ OPTIONS:
command. Specifying this option causes the exit status of scan-build to be 1
if it found potential bugs and 0 otherwise.
 
+ --exclude 
+
+   Do not run static analyzer against files found in this
+   directory (You can specify this option multiple times).
+   Could be useful when project contains 3rd party libraries.
+
  --use-cc [compiler path]
  --use-cc=[compiler path]
 
@@ -1698,6 +1719,15 @@ sub ProcessArgs {
   next;
 }
 
+if ($arg eq "--exclude") {
+  shift @$Args;
+  my $arg = shift @$Args;
+  # Remove the trailing slash if any
+  $arg =~ s|/$||;
+  push @{$Options{Excludes}}, $arg;
+  next;
+}
+
 if ($arg eq "-load-plugin") {
   shift @$Args;
   push @{$Options{PluginsToLoad}}, shift @$Args;


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52153: scan-build: Add support of the option --exclude like in scan-build-py

2018-09-16 Thread Phabricator via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342359: scan-build: Add support of the option --exclude like 
in scan-build-py (authored by sylvestre, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D52153

Files:
  cfe/trunk/tools/scan-build/bin/scan-build


Index: cfe/trunk/tools/scan-build/bin/scan-build
===
--- cfe/trunk/tools/scan-build/bin/scan-build
+++ cfe/trunk/tools/scan-build/bin/scan-build
@@ -58,6 +58,7 @@
   KeepEmpty => 0,# Don't remove output directory even with 0 
results.
   EnableCheckers => {},
   DisableCheckers => {},
+  Excludes => [],
   UseCC => undef,# C compiler to use for compilation.
   UseCXX => undef,   # C++ compiler to use for compilation.
   AnalyzerTarget => undef,
@@ -420,6 +421,20 @@
  # The file no longer exists: use the original path.
  $BugFile = $1;
   }
+
+  # Get just the path
+  my $p = dirname($BugFile);
+  # Check if the path is found in the list of exclude
+  if (grep { $p =~ m/$_/ } @{$Options{Excludes}}) {
+ if ($Options{Verbose}) {
+ Diag("File '$BugFile' deleted: part of an ignored directory.\n");
+ }
+
+   # File in an ignored directory. Remove it
+   unlink("$Dir/$FName");
+   return;
+  }
+
   UpdatePrefix($BugFile);
 }
 elsif (/$/) {
@@ -1194,6 +1209,12 @@
command. Specifying this option causes the exit status of scan-build to be 1
if it found potential bugs and 0 otherwise.
 
+ --exclude 
+
+   Do not run static analyzer against files found in this
+   directory (You can specify this option multiple times).
+   Could be useful when project contains 3rd party libraries.
+
  --use-cc [compiler path]
  --use-cc=[compiler path]
 
@@ -1698,6 +1719,15 @@
   next;
 }
 
+if ($arg eq "--exclude") {
+  shift @$Args;
+  my $arg = shift @$Args;
+  # Remove the trailing slash if any
+  $arg =~ s|/$||;
+  push @{$Options{Excludes}}, $arg;
+  next;
+}
+
 if ($arg eq "-load-plugin") {
   shift @$Args;
   push @{$Options{PluginsToLoad}}, shift @$Args;


Index: cfe/trunk/tools/scan-build/bin/scan-build
===
--- cfe/trunk/tools/scan-build/bin/scan-build
+++ cfe/trunk/tools/scan-build/bin/scan-build
@@ -58,6 +58,7 @@
   KeepEmpty => 0,# Don't remove output directory even with 0 results.
   EnableCheckers => {},
   DisableCheckers => {},
+  Excludes => [],
   UseCC => undef,# C compiler to use for compilation.
   UseCXX => undef,   # C++ compiler to use for compilation.
   AnalyzerTarget => undef,
@@ -420,6 +421,20 @@
  # The file no longer exists: use the original path.
  $BugFile = $1;
   }
+
+  # Get just the path
+  my $p = dirname($BugFile);
+  # Check if the path is found in the list of exclude
+  if (grep { $p =~ m/$_/ } @{$Options{Excludes}}) {
+ if ($Options{Verbose}) {
+ Diag("File '$BugFile' deleted: part of an ignored directory.\n");
+ }
+
+   # File in an ignored directory. Remove it
+   unlink("$Dir/$FName");
+   return;
+  }
+
   UpdatePrefix($BugFile);
 }
 elsif (/$/) {
@@ -1194,6 +1209,12 @@
command. Specifying this option causes the exit status of scan-build to be 1
if it found potential bugs and 0 otherwise.
 
+ --exclude 
+
+   Do not run static analyzer against files found in this
+   directory (You can specify this option multiple times).
+   Could be useful when project contains 3rd party libraries.
+
  --use-cc [compiler path]
  --use-cc=[compiler path]
 
@@ -1698,6 +1719,15 @@
   next;
 }
 
+if ($arg eq "--exclude") {
+  shift @$Args;
+  my $arg = shift @$Args;
+  # Remove the trailing slash if any
+  $arg =~ s|/$||;
+  push @{$Options{Excludes}}, $arg;
+  next;
+}
+
 if ($arg eq "-load-plugin") {
   shift @$Args;
   push @{$Options{PluginsToLoad}}, shift @$Args;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45719: [clang-Format] Fix indentation of member call after block

2018-09-16 Thread Anders Karlsson via Phabricator via cfe-commits

ank added a comment.

ping @klimek


Repository:
  rC Clang

https://reviews.llvm.org/D45719



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52160: [Driver] Support XRay on Fuchsia

2018-09-16 Thread Dean Michael Berris via Phabricator via cfe-commits

dberris accepted this revision.
dberris added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D52160



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51990: [DebugInfo] Fix emitting of bit offset for ObjC

2018-09-16 Thread John McCall via Phabricator via cfe-commits

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2369
+  ? CGM.getObjCRuntime().ComputeBitfieldBitOffset(CGM, ID, Field)
+  : 0;
 } else {

JDevlieghere wrote:
> aprantl wrote:
> > JDevlieghere wrote:
> > > JDevlieghere wrote:
> > > > aprantl wrote:
> > > > > aprantl wrote:
> > > > > > It might help to attempt some git blame archeology.
> > > > > > Judging from the comment, it sounds like the debugger is supposed 
> > > > > > to query the runtime for the *byte* offset and then add the bit 
> > > > > > offset from DWARF? Could that make sense?
> > > > > If that is the case, we'd need to relax llvm-dwarfdump --verify to 
> > > > > accept this and make sure LLDB does the right thing instead.
> > > > Ah I see, yeah that sounds reasonable and explains the comment which I 
> > > > interpreted differently. Thanks! 
> > > btw it was added in rL167503. 
> > We should check whether emitting the offsets like this violates the DWARF 
> > spec. If yes, it may be better to emit the static offsets like you are 
> > doing here and then still have LLDB ignore everything but the bit-offsets 
> > from the dynamic byte offset.
> The standard says 
> 
> > The member entry corresponding to a data member that is defined in a 
> > structure,
> > union or class may have either a DW_AT_data_member_location attribute or a
> > DW_AT_data_bit_offset attribute.
> 
> which to me sounds like they should be mutually exclusive. I ran the lldb 
> test suite with my change and there were no new failures, which leads me to 
> believe that the comment from r167503 still holds and lldb ignores this 
> attribute, at least for Objective-C.
In Clang's current code-generation, the ivar offset variable stores the offset 
of the byte containing the first bit of the bit-field, which is not necessarily 
the same as the offset of the byte at which the bit-field's storage begins.  
This is why the debug info `%`s by the number of bits in a `char`: it's the 
correct bit-offset for computing where the bit-field begins relative to the 
value in the ivar offset variable.  See the behavior of 
`CGObjCRuntime::EmitValueForIvarAtOffset`, which does the same `%` when 
computing the layout.

It seems right to me that the `%` is done consistently within the compiler, 
rather than expecting LLDB to do it.  That's the more future-proof design; for 
example, it would allow the compiler to emit a single ObjC ivar record for the 
combined bit-field storage and then emit debug info for the bit-fields at their 
appropriate relative bit-offsets, which could then be `>= 8`.  This would 
arguably be a better code-generation scheme anyway, for a number of reasons — 
at the cost of breaking reflective access to the individual ivars, which I 
don't believe the runtime really supports anyway.

Anyway, the upshot is that I don't think this patch is correct.


Repository:
  rC Clang

https://reviews.llvm.org/D51990



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

2018-09-16 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

It might help if you're more specific about whose review you're asking for.


Repository:
  rC Clang

https://reviews.llvm.org/D50250



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

39 matches

Mail list logo