[PATCH] D44606: [analyzer] Fix the crash in `IteratorChecker.cpp` when `SymbolConjured` has a null Stmt.

[Henry Wong via Phabricator via cfe-commits]

MTC created this revision.
MTC added reviewers: NoQ, baloghadamsoftware, szepet, dcoughlin.
Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, xazax.hun.
Herald added a reviewer: george.karpenkov.

When the loop has a null terminator statement and sets `widen-loops=true`, 
`invalidateRegions()` will constructs the
`SymbolConjured` with null `Stmt`. And this will lead to a crash in 
`IteratorChecker.cpp`.

Given the code below:

  void null_terminator_loop_widen(int *a) {
int c;
for (;;) {
  c = *a;
  a++;
}
  }

I haven't found any problems with `SymbolConjured` containing null `Stmt` for 
the time being. So I just use 
`dyn_cast_or_null<>` instead of `dyn_cast<>` in `IteratorChecker.cpp`, and 
didn't delve into the widen loop part.


Repository:
  rC Clang

https://reviews.llvm.org/D44606

Files:
  lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  test/Analysis/loop-widening.c


Index: test/Analysis/loop-widening.c
===
--- test/Analysis/loop-widening.c
+++ test/Analysis/loop-widening.c
@@ -1,4 +1,5 @@
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 
-analyzer-config widen-loops=true -verify %s
+// RUN: %clang_analyze_cc1 -DTEST_NULL_TERM 
-analyzer-checker=core,unix.Malloc,debug.ExprInspection,alpha.cplusplus.IteratorRange
 -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
 
 void clang_analyzer_eval(int);
 void clang_analyzer_warnIfReached();
@@ -188,3 +189,13 @@
   }
   clang_analyzer_eval(i >= 2); // expected-warning {{TRUE}}
 }
+
+#ifdef TEST_NULL_TERM
+void null_terminator_loop_widen(int *a) {
+  int c;
+  for (;;) {
+c = *a; // no-crash
+a++;
+  }
+}
+#endif
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -604,7 +604,7 @@
   if (const auto *BSE = dyn_cast(SE)) {
 return BSE->getOpcode();
   } else if (const auto *SC = dyn_cast(SE)) {
-const auto *COE = dyn_cast(SC->getStmt());
+const auto *COE = dyn_cast_or_null(SC->getStmt());
 if (!COE)
   return BO_Comma; // Extremal value, neither EQ nor NE
 if (COE->getOperator() == OO_EqualEqual) {


Index: test/Analysis/loop-widening.c
===
--- test/Analysis/loop-widening.c
+++ test/Analysis/loop-widening.c
@@ -1,4 +1,5 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
+// RUN: %clang_analyze_cc1 -DTEST_NULL_TERM -analyzer-checker=core,unix.Malloc,debug.ExprInspection,alpha.cplusplus.IteratorRange -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
 
 void clang_analyzer_eval(int);
 void clang_analyzer_warnIfReached();
@@ -188,3 +189,13 @@
   }
   clang_analyzer_eval(i >= 2); // expected-warning {{TRUE}}
 }
+
+#ifdef TEST_NULL_TERM
+void null_terminator_loop_widen(int *a) {
+  int c;
+  for (;;) {
+c = *a; // no-crash
+a++;
+  }
+}
+#endif
Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
+++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
@@ -604,7 +604,7 @@
   if (const auto *BSE = dyn_cast(SE)) {
 return BSE->getOpcode();
   } else if (const auto *SC = dyn_cast(SE)) {
-const auto *COE = dyn_cast(SC->getStmt());
+const auto *COE = dyn_cast_or_null(SC->getStmt());
 if (!COE)
   return BO_Comma; // Extremal value, neither EQ nor NE
 if (COE->getOperator() == OO_EqualEqual) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44605: [Driver] Default to DWARF 5 for Fuchsia

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added a comment.

We aren't actually using DWARF 5 yet AFAICT.  Our builds don't pass -gdwarf-5.  
So I'm not sure we have yet verified that all the DWARF-consuming tools people 
are using with Fuchsia binaries can handle all of DWARF 5 (which has several 
major format changes).  I'd certainly like 5 to be the default, but I think we 
need to establish a set of consumers we care about and verify their format 
version support before we can be sure about this.


Repository:
  rC Clang

https://reviews.llvm.org/D44605



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

[PATCH] D41102: Setup clang-doc frontend framework

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Hm, or possibly you could just pass the triple to clang?


Repository:
  rL LLVM

https://reviews.llvm.org/D41102



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

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: docs/ReleaseNotes.rst:125
+
+  Flags functions exceeding this number of variables declared in the body.
+

I would rephrase this a little to:

```
 Flags function bodies exceeding this number of declared variables.
```



Comment at: test/clang-tidy/readability-function-size.cpp:118
   }
+  // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 6 variables (threshold 1)
 }

Why is this check here and not on line 100 like the other conditions?



Comment at: test/clang-tidy/readability-function-size.cpp:144
   }
+  // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 4 variables (threshold 1)
 }

same here.



Comment at: test/clang-tidy/readability-function-size.cpp:153
+// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 statements (threshold 0)
+void variables_1(int i) {
+  int j;

Could you please add a test for function having more parameters then allowed 
variables, but no variables?



Comment at: test/clang-tidy/readability-function-size.cpp:180
+// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 2 variables (threshold 1)
+void variables_5() {
+  for (int i;;)

This testfile is not C++17 right now, but do you know how structured bindings 
are handled? Maybe its worth adding another file for C++17 testing it.

Could you add a little test for range based for loops?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44602



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

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 138844.
lebedev.ri marked 3 inline comments as done.
lebedev.ri added a comment.

Address jonastoth's review notes.

- Properly support C++17 structured bindings.
- A few more tests


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44602

Files:
  clang-tidy/readability/FunctionSizeCheck.cpp
  clang-tidy/readability/FunctionSizeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/readability-function-size.rst
  test/clang-tidy/readability-function-size-variables-c++17.cpp
  test/clang-tidy/readability-function-size.cpp

Index: test/clang-tidy/readability-function-size.cpp
===
--- test/clang-tidy/readability-function-size.cpp
+++ test/clang-tidy/readability-function-size.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s readability-function-size %t -- -config='{CheckOptions: [{key: readability-function-size.LineThreshold, value: 0}, {key: readability-function-size.StatementThreshold, value: 0}, {key: readability-function-size.BranchThreshold, value: 0}, {key: readability-function-size.ParameterThreshold, value: 5}, {key: readability-function-size.NestingThreshold, value: 2}]}' -- -std=c++11
+// RUN: %check_clang_tidy %s readability-function-size %t -- -config='{CheckOptions: [{key: readability-function-size.LineThreshold, value: 0}, {key: readability-function-size.StatementThreshold, value: 0}, {key: readability-function-size.BranchThreshold, value: 0}, {key: readability-function-size.ParameterThreshold, value: 5}, {key: readability-function-size.NestingThreshold, value: 2}, {key: readability-function-size.VariableThreshold, value: 1}]}' -- -std=c++11
 
 // Bad formatting is intentional, don't run clang-format over the whole file!
 
@@ -64,7 +64,7 @@
 
 void baz0() { // 1
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'baz0' exceeds recommended size/complexity
-  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 27 lines including whitespace and comments (threshold 0)
+  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 28 lines including whitespace and comments (threshold 0)
   // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 9 statements (threshold 0)
   int a;
   { // 2
@@ -87,14 +87,15 @@
 }
   }
   macro()
-// CHECK-MESSAGES: :[[@LINE-1]]:3: note: nesting level 3 starts here (threshold 2)
-// CHECK-MESSAGES: :[[@LINE-28]]:25: note: expanded from macro 'macro'
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: note: nesting level 3 starts here (threshold 2)
+  // CHECK-MESSAGES: :[[@LINE-28]]:25: note: expanded from macro 'macro'
+  // CHECK-MESSAGES: :[[@LINE-27]]:6: note: 9 variables (threshold 1)
 }
 
 // check that nested if's are not reported. this was broken initially
 void nesting_if() { // 1
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'nesting_if' exceeds recommended size/complexity
-  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 22 lines including whitespace and comments (threshold 0)
+  // CHECK-MESSAGES: :[[@LINE-2]]:6: note: 23 lines including whitespace and comments (threshold 0)
   // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 18 statements (threshold 0)
   // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 6 branches (threshold 0)
   if (true) { // 2
@@ -114,12 +115,13 @@
   } else if (true) { // 2
  int j;
   }
+  // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 6 variables (threshold 1)
 }
 
 // however this should warn
 void bad_if_nesting() { // 1
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'bad_if_nesting' exceeds recommended size/complexity
-// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 22 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-2]]:6: note: 23 lines including whitespace and comments (threshold 0)
 // CHECK-MESSAGES: :[[@LINE-3]]:6: note: 12 statements (threshold 0)
 // CHECK-MESSAGES: :[[@LINE-4]]:6: note: 4 branches (threshold 0)
   if (true) {// 2
@@ -139,4 +141,66 @@
   }
 }
   }
+  // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 4 variables (threshold 1)
 }
+
+void variables_0() {
+  int i;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'variables_0' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 2 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 statements (threshold 0)
+void variables_1(int i) {
+  int j;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'variables_1' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 2 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 statements (threshold 0)
+void variables_2(int i, int j) {
+  ;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: function 'variables_2' exceeds recommended size/complexity thresholds [readability-function-size]
+// CHECK-MESSAGES: :[[@LINE-4]]:6: note: 2 lines including whitespace and comments (threshold 0)
+// CHECK-MESSAGES:

r327790 - [dsymutil] Rename llvm-dsymutil -> dsymutil

[Jonas Devlieghere via cfe-commits]

Author: jdevlieghere
Date: Sun Mar 18 04:38:41 2018
New Revision: 327790

URL: http://llvm.org/viewvc/llvm-project?rev=327790&view=rev
Log:
[dsymutil] Rename llvm-dsymutil -> dsymutil

Now that almost all functionality of Apple's dsymutil has been
upstreamed, the open source variant can be used as a drop in
replacement. Hence we feel it's no longer necessary to have the llvm
prefix.

Differential revision: https://reviews.llvm.org/D44527

Modified:
cfe/trunk/cmake/caches/Apple-stage2.cmake
cfe/trunk/cmake/caches/BaremetalARM.cmake
cfe/trunk/cmake/caches/DistributionExample-stage2.cmake
cfe/trunk/cmake/caches/Fuchsia-stage2.cmake

Modified: cfe/trunk/cmake/caches/Apple-stage2.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Apple-stage2.cmake?rev=327790&r1=327789&r2=327790&view=diff
==
--- cfe/trunk/cmake/caches/Apple-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/Apple-stage2.cmake Sun Mar 18 04:38:41 2018
@@ -47,7 +47,7 @@ set(LLVM_CREATE_XCODE_TOOLCHAIN ON CACHE
 # setup toolchain
 set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
 set(LLVM_TOOLCHAIN_TOOLS
-  llvm-dsymutil
+  dsymutil
   llvm-cov
   llvm-dwarfdump
   llvm-profdata

Modified: cfe/trunk/cmake/caches/BaremetalARM.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/BaremetalARM.cmake?rev=327790&r1=327789&r2=327790&view=diff
==
--- cfe/trunk/cmake/caches/BaremetalARM.cmake (original)
+++ cfe/trunk/cmake/caches/BaremetalARM.cmake Sun Mar 18 04:38:41 2018
@@ -24,11 +24,11 @@ set(BUILTINS_armv7em-none-eabi_COMPILER_
 
 set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
 set(LLVM_TOOLCHAIN_TOOLS
+  dsymutil
   llc
   llvm-ar
   llvm-cxxfilt
   llvm-dwarfdump
-  llvm-dsymutil
   llvm-nm
   llvm-objdump
   llvm-ranlib

Modified: cfe/trunk/cmake/caches/DistributionExample-stage2.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/DistributionExample-stage2.cmake?rev=327790&r1=327789&r2=327790&view=diff
==
--- cfe/trunk/cmake/caches/DistributionExample-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/DistributionExample-stage2.cmake Sun Mar 18 04:38:41 
2018
@@ -10,7 +10,7 @@ set(CMAKE_CXX_FLAGS_RELWITHDEBINFO "-O3
 # setup toolchain
 set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
 set(LLVM_TOOLCHAIN_TOOLS
-  llvm-dsymutil
+  dsymutil
   llvm-cov
   llvm-dwarfdump
   llvm-profdata

Modified: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Fuchsia-stage2.cmake?rev=327790&r1=327789&r2=327790&view=diff
==
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake Sun Mar 18 04:38:41 2018
@@ -57,12 +57,12 @@ endforeach()
 # Setup toolchain.
 set(LLVM_INSTALL_TOOLCHAIN_ONLY ON CACHE BOOL "")
 set(LLVM_TOOLCHAIN_TOOLS
+  dsymutil
   llc
   llvm-ar
   llvm-cov
   llvm-cxxfilt
   llvm-dwarfdump
-  llvm-dsymutil
   llvm-lib
   llvm-nm
   llvm-objcopy


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

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Looks fine to me, but Aaron or someone else must accept :)

Just out of curiosity: The decomposition visit is for structured binding and 
the binding visit for range based for? Are there other places where binding 
occurs? For example in some template stuff? Just asking is this hits me in a 
other check :D


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44602



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

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D44602#1041153, @JonasToth wrote:

> Looks fine to me, but Aaron or someone else must accept :)


Thanks for review!

In https://reviews.llvm.org/D44602#1041153, @JonasToth wrote:

> Just out of curiosity: The decomposition visit is for structured binding and 
> the binding visit for range based for? Are there other places where binding 
> occurs, for example in some template stuff? 
>  Just asking if this might hit me in a other check :D


The comment i added is actually wrong.
The range-based for just works anyway.
The `VisitBindingDecl()` is needed to get all the 'variables' declared in 
structured binding.
But as you can see in https://godbolt.org/g/be6Juf, the `BindingDecl`'s are 
wrapped into `DecompositionDecl`.
And `DecompositionDecl` is actually inherited from `VarDecl`, 
https://github.com/llvm-mirror/clang/blob/b031fdc9b7dbd9c7f942d8060a4f00d63f3c9af2/include/clang/AST/DeclCXX.h#L3824-L3825,
so we count the `DecompositionDecl` in `VisitVarDecl()`, thus we need a 
`VisitDecompositionDecl()` that subtracts it.

This may be the wrong/incomplete solution.




Comment at: docs/ReleaseNotes.rst:125
+
+  Flags functions exceeding this number of variables declared in the body.
+

JonasToth wrote:
> I would rephrase this a little to:
> 
> ```
>  Flags function bodies exceeding this number of declared variables.
> ```
I agree that it looks gibberish. Reworded a little. Probably still not ideal.



Comment at: test/clang-tidy/readability-function-size.cpp:118
   }
+  // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 6 variables (threshold 1)
 }

JonasToth wrote:
> Why is this check here and not on line 100 like the other conditions?
It can not be there, the `note: nesting level <> starts here (threshold 2)` are 
outputted before `note: <> variables (threshold 1)`, and filecheck respects the 
order of `// CHECK-MESSAGES:`



Comment at: test/clang-tidy/readability-function-size.cpp:153
+// CHECK-MESSAGES: :[[@LINE-5]]:6: note: 1 statements (threshold 0)
+void variables_1(int i) {
+  int j;

JonasToth wrote:
> Could you please add a test for function having more parameters then allowed 
> variables, but no variables?
It's already checked in `foo7()`, but sure.



Comment at: test/clang-tidy/readability-function-size.cpp:180
+// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 2 variables (threshold 1)
+void variables_5() {
+  for (int i;;)

JonasToth wrote:
> This testfile is not C++17 right now, but do you know how structured bindings 
> are handled? Maybe its worth adding another file for C++17 testing it.
> 
> Could you add a little test for range based for loops?
> This testfile is not C++17 right now, but do you know how structured bindings 
> are handled? Maybe its worth adding another file for C++17 testing it.

Nice catch!

> Could you add a little test for range based for loops?

Added. I expected them to not work (after a quick look at AST), but apparently 
they just work..


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44602



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

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> The comment i added is actually wrong.
>  The range-based for just works anyway.
>  The VisitBindingDecl() is needed to get all the 'variables' declared in 
> structured binding.
>  But as you can see in https://godbolt.org/g/be6Juf, the BindingDecl's are 
> wrapped into DecompositionDecl.
>  And DecompositionDecl is actually inherited from VarDecl, 
> https://github.com/llvm-mirror/clang/blob/b031fdc9b7dbd9c7f942d8060a4f00d63f3c9af2/include/clang/AST/DeclCXX.h#L3824-L3825,
>  so we count the DecompositionDecl in VisitVarDecl(), thus we need a 
> VisitDecompositionDecl() that subtracts it.

Thanks for clarification. It feels very NOT obvious how to handle this case :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44602



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

[PATCH] D44602: [clang-tidy] readability-function-size: add VariableThreshold param.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: docs/ReleaseNotes.rst:125
+
+  Flags functions exceeding this number of variables declared in the body.
+

lebedev.ri wrote:
> JonasToth wrote:
> > I would rephrase this a little to:
> > 
> > ```
> >  Flags function bodies exceeding this number of declared variables.
> > ```
> I agree that it looks gibberish. Reworded a little. Probably still not ideal.
Its better. But the native speakers should give a final thought.



Comment at: test/clang-tidy/readability-function-size.cpp:118
   }
+  // CHECK-MESSAGES: :[[@LINE-22]]:6: note: 6 variables (threshold 1)
 }

lebedev.ri wrote:
> JonasToth wrote:
> > Why is this check here and not on line 100 like the other conditions?
> It can not be there, the `note: nesting level <> starts here (threshold 2)` 
> are outputted before `note: <> variables (threshold 1)`, and filecheck 
> respects the order of `// CHECK-MESSAGES:`
I see, no problems then :)



Comment at: test/clang-tidy/readability-function-size.cpp:180
+// CHECK-MESSAGES: :[[@LINE-6]]:6: note: 2 variables (threshold 1)
+void variables_5() {
+  for (int i;;)

lebedev.ri wrote:
> JonasToth wrote:
> > This testfile is not C++17 right now, but do you know how structured 
> > bindings are handled? Maybe its worth adding another file for C++17 testing 
> > it.
> > 
> > Could you add a little test for range based for loops?
> > This testfile is not C++17 right now, but do you know how structured 
> > bindings are handled? Maybe its worth adding another file for C++17 testing 
> > it.
> 
> Nice catch!
> 
> > Could you add a little test for range based for loops?
> 
> Added. I expected them to not work (after a quick look at AST), but 
> apparently they just work..
> Added. I expected them to not work (after a quick look at AST), but 
> apparently they just work..

Life would be so much easier if this is always the case :D


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44602



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

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a subscriber: rsmith.
JonasToth added a comment.

Good news: The stack overflow seems to be fixed, by the patch r326624 from 
@rsmith (Thank you very much!).
Bad news: The check does not do its job anymore I try to fix the 
functionality problems.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737



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

[PATCH] D44607: Recompute invalidated iterator in insertTargetAndModeArgs

[Hector Martin via Phabricator via cfe-commits]

marcan created this revision.
marcan added reviewers: sepavloff, echristo.
Herald added a subscriber: cfe-commits.

Doing an `.insert()` can potentially invalidate iterators by reallocating the 
vector's storage. When all the stars align just right, this causes segfaults or 
glibc aborts. Fix this by recomputing the position before each insert.

Gentoo Linux bug (crashes while building Chromium): 
https://bugs.gentoo.org/650082


Repository:
  rC Clang

https://reviews.llvm.org/D44607

Files:
  tools/driver/driver.cpp


Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -209,20 +209,24 @@
 static void insertTargetAndModeArgs(const ParsedClangName &NameParts,
 SmallVectorImpl &ArgVector,
 std::set &SavedStrings) {
-  // Put target and mode arguments at the start of argument list so that
-  // arguments specified in command line could override them. Avoid putting
-  // them at index 0, as an option like '-cc1' must remain the first.
-  auto InsertionPoint = ArgVector.begin();
-  if (InsertionPoint != ArgVector.end())
-++InsertionPoint;
-
   if (NameParts.DriverMode) {
+// Put target and mode arguments at the start of argument list so that
+// arguments specified in command line could override them. Avoid putting
+// them at index 0, as an option like '-cc1' must remain the first.
+auto InsertionPoint = ArgVector.begin();
+if (InsertionPoint != ArgVector.end())
+++InsertionPoint;
+
 // Add the mode flag to the arguments.
 ArgVector.insert(InsertionPoint,
  GetStableCStr(SavedStrings, NameParts.DriverMode));
   }
 
   if (NameParts.TargetIsValid) {
+auto InsertionPoint = ArgVector.begin();
+if (InsertionPoint != ArgVector.end())
+++InsertionPoint;
+
 const char *arr[] = {"-target", GetStableCStr(SavedStrings,
   NameParts.TargetPrefix)};
 ArgVector.insert(InsertionPoint, std::begin(arr), std::end(arr));


Index: tools/driver/driver.cpp
===
--- tools/driver/driver.cpp
+++ tools/driver/driver.cpp
@@ -209,20 +209,24 @@
 static void insertTargetAndModeArgs(const ParsedClangName &NameParts,
 SmallVectorImpl &ArgVector,
 std::set &SavedStrings) {
-  // Put target and mode arguments at the start of argument list so that
-  // arguments specified in command line could override them. Avoid putting
-  // them at index 0, as an option like '-cc1' must remain the first.
-  auto InsertionPoint = ArgVector.begin();
-  if (InsertionPoint != ArgVector.end())
-++InsertionPoint;
-
   if (NameParts.DriverMode) {
+// Put target and mode arguments at the start of argument list so that
+// arguments specified in command line could override them. Avoid putting
+// them at index 0, as an option like '-cc1' must remain the first.
+auto InsertionPoint = ArgVector.begin();
+if (InsertionPoint != ArgVector.end())
+++InsertionPoint;
+
 // Add the mode flag to the arguments.
 ArgVector.insert(InsertionPoint,
  GetStableCStr(SavedStrings, NameParts.DriverMode));
   }
 
   if (NameParts.TargetIsValid) {
+auto InsertionPoint = ArgVector.begin();
+if (InsertionPoint != ArgVector.end())
+++InsertionPoint;
+
 const char *arr[] = {"-target", GetStableCStr(SavedStrings,
   NameParts.TargetPrefix)};
 ArgVector.insert(InsertionPoint, std::begin(arr), std::end(arr));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44607: Recompute invalidated iterator in insertTargetAndModeArgs

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Tests?


Repository:
  rC Clang

https://reviews.llvm.org/D44607



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

Re: r327738 - [MS] Don't escape MS C++ names with \01

[Reid Kleckner via cfe-commits]

Yes: https://reviews.llvm.org/D7775

This side is very mechanical, so I did not send it for review.


On Sun, Mar 18, 2018, 3:54 AM Nico Weber  wrote:

> Was this discussed or reviewed somewhere? (It looks like a good change to
> me, I'm just wondering if there was something that triggered this and would
> like to read about the background if there's anything to read.)
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 138849.
JonasToth added a comment.

- get the check working again
- enable the big test


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737

Files:
  clang-tidy/hicpp/CMakeLists.txt
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
  test/clang-tidy/hicpp-multiway-paths-covered.cpp

Index: test/clang-tidy/hicpp-multiway-paths-covered.cpp
===
--- /dev/null
+++ test/clang-tidy/hicpp-multiway-paths-covered.cpp
@@ -0,0 +1,468 @@
+// RUN: %check_clang_tidy %s hicpp-multiway-paths-covered %t
+
+enum OS { Mac,
+  Windows,
+  Linux };
+
+struct Bitfields {
+  unsigned UInt : 3;
+  int SInt : 1;
+};
+
+int return_integer() { return 42; }
+
+void bad_switch(int i) {
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case 0:
+break;
+  }
+  // No default in this switch
+  switch (i) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+break;
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  // degenerate, maybe even warning
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch without labels
+  }
+
+  switch (int j = return_integer()) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+  case 2:
+break;
+  }
+
+  // Degenerated, only default case.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+break;
+  }
+
+  // Degenerated, only one case label and default case -> Better as if-stmt.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch could be better written as an if/else statement
+  case 0:
+break;
+  default:
+break;
+  }
+
+  unsigned long long BigNumber = 0;
+  switch (BigNumber) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+
+  const int &IntRef = i;
+  switch (IntRef) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+
+  char C = 'A';
+  switch (C) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 'A':
+break;
+  case 'B':
+break;
+  }
+
+  Bitfields Bf;
+  // UInt has 3 bits size.
+  switch (Bf.UInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.UInt) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+break;
+  }
+  // SInt has 1 bit size, so this is somewhat degenerated.
+  switch (Bf.SInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case 0:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.SInt) {
+  case 0:
+  case 1:
+break;
+  }
+
+  bool Flag = false;
+  switch (Flag) {
+// CHECK-MESSAGES:[[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case true:
+break;
+  }
+
+  switch (Flag) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+break;
+  }
+
+  // This `switch` will create a frontend warning from '-Wswitch-bool' but is
+  // ok for this check.
+  switch (Flag) {
+  case true:
+break;
+  case false:
+break;
+  }
+}
+
+void unproblematic_switch(unsigned char c) {
+  //
+  switch (c) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+  case 8:
+  case 9:
+  case 10:
+  case 11:
+  case 12:
+  case 13:
+  case 14:
+  case 15:
+  case 16:
+  case 17:
+  case 18:
+  case 19:
+  case 20:
+  case 21:
+  case 22:
+  case 23:
+  case 24:
+  case 25:
+  case 26:
+  case 27:
+  case 28:
+  case 29:
+  case 30:
+  case 31:
+  case 32:
+  case 33:
+  case 34:
+  case 35:
+  case 36:
+  case 37:
+  case 38:
+  case 39:
+  case 40:
+  case 41:
+  case 42:
+  case 43:
+  case 44:
+  case 45:
+  case 46:
+  case 47:
+  case 48:
+  case 49:
+  case 50:
+  case 51:
+  case 52:
+  case 53:
+  case 54:
+  case 55:
+  case 56:
+  case 57:
+  case 58:
+  case 59:
+  case 60:
+  case 61:
+  case 62:
+  case 63:
+  case 64:
+  case 65:
+  case 66:
+  case 67:
+  case 68:
+  case 69:
+  case 70:
+  case 71:
+  case 72:
+  case 73:
+  case 74:
+  case 75:
+  case 76:
+  case 77:
+  case 78:
+  case 79:
+  case 80:
+  case 81:
+  case 82:
+  case 83:
+  case 84:
+  c

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 138850.
JonasToth added a comment.

- reorder check in release notes to get it in alphabetical order


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737

Files:
  clang-tidy/hicpp/CMakeLists.txt
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
  test/clang-tidy/hicpp-multiway-paths-covered.cpp

Index: test/clang-tidy/hicpp-multiway-paths-covered.cpp
===
--- /dev/null
+++ test/clang-tidy/hicpp-multiway-paths-covered.cpp
@@ -0,0 +1,468 @@
+// RUN: %check_clang_tidy %s hicpp-multiway-paths-covered %t
+
+enum OS { Mac,
+  Windows,
+  Linux };
+
+struct Bitfields {
+  unsigned UInt : 3;
+  int SInt : 1;
+};
+
+int return_integer() { return 42; }
+
+void bad_switch(int i) {
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case 0:
+break;
+  }
+  // No default in this switch
+  switch (i) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+break;
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  // degenerate, maybe even warning
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch without labels
+  }
+
+  switch (int j = return_integer()) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+  case 2:
+break;
+  }
+
+  // Degenerated, only default case.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+break;
+  }
+
+  // Degenerated, only one case label and default case -> Better as if-stmt.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch could be better written as an if/else statement
+  case 0:
+break;
+  default:
+break;
+  }
+
+  unsigned long long BigNumber = 0;
+  switch (BigNumber) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+
+  const int &IntRef = i;
+  switch (IntRef) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+
+  char C = 'A';
+  switch (C) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 'A':
+break;
+  case 'B':
+break;
+  }
+
+  Bitfields Bf;
+  // UInt has 3 bits size.
+  switch (Bf.UInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.UInt) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+break;
+  }
+  // SInt has 1 bit size, so this is somewhat degenerated.
+  switch (Bf.SInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case 0:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.SInt) {
+  case 0:
+  case 1:
+break;
+  }
+
+  bool Flag = false;
+  switch (Flag) {
+// CHECK-MESSAGES:[[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case true:
+break;
+  }
+
+  switch (Flag) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+break;
+  }
+
+  // This `switch` will create a frontend warning from '-Wswitch-bool' but is
+  // ok for this check.
+  switch (Flag) {
+  case true:
+break;
+  case false:
+break;
+  }
+}
+
+void unproblematic_switch(unsigned char c) {
+  //
+  switch (c) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+  case 8:
+  case 9:
+  case 10:
+  case 11:
+  case 12:
+  case 13:
+  case 14:
+  case 15:
+  case 16:
+  case 17:
+  case 18:
+  case 19:
+  case 20:
+  case 21:
+  case 22:
+  case 23:
+  case 24:
+  case 25:
+  case 26:
+  case 27:
+  case 28:
+  case 29:
+  case 30:
+  case 31:
+  case 32:
+  case 33:
+  case 34:
+  case 35:
+  case 36:
+  case 37:
+  case 38:
+  case 39:
+  case 40:
+  case 41:
+  case 42:
+  case 43:
+  case 44:
+  case 45:
+  case 46:
+  case 47:
+  case 48:
+  case 49:
+  case 50:
+  case 51:
+  case 52:
+  case 53:
+  case 54:
+  case 55:
+  case 56:
+  case 57:
+  case 58:
+  case 59:
+  case 60:
+  case 61:
+  case 62:
+  case 63:
+  case 64:
+  case 65:
+  case 66:
+  case 67:
+  case 68:
+  case 69:
+  case 70:
+  case 71:
+  case 72:
+  case 73:
+  case 74:
+  case 75:
+  case 76:
+  case 77:
+  case 78:
+  case 79:
+  case 80:
+  case 81:
+  case 82:
+  case 83:
+  

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

I think the check is ready to land. I did check run it over LLVM and found some 
interesting code parts that could benefit. I extracted all the warnings and 
sorted them into the categories `missing default/covered 
codepath(uncovered.txt)`, `better use if/else(better_if.txt)` and `use 
if(mandatory_if.txt)`. Some warnings come from generated files but some live in 
usercode. In total there are 540 warnings.

Please note, that i identified a common pattern to leave out the `default` 
label and add an ending `llvm_unreachable` that is outside the switch but 
ensures there is no unwanted fall_through. Most of the warnings for the first 
category are like this, but i could not check all of them!

F5899267: uncovered.txt 

F5899266: better_if.txt 

F5899265: mandatory_if.txt 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737



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

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 138851.
JonasToth added a comment.

- remove spurious debug iostream


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737

Files:
  clang-tidy/hicpp/CMakeLists.txt
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
  test/clang-tidy/hicpp-multiway-paths-covered.cpp

Index: test/clang-tidy/hicpp-multiway-paths-covered.cpp
===
--- /dev/null
+++ test/clang-tidy/hicpp-multiway-paths-covered.cpp
@@ -0,0 +1,468 @@
+// RUN: %check_clang_tidy %s hicpp-multiway-paths-covered %t
+
+enum OS { Mac,
+  Windows,
+  Linux };
+
+struct Bitfields {
+  unsigned UInt : 3;
+  int SInt : 1;
+};
+
+int return_integer() { return 42; }
+
+void bad_switch(int i) {
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case 0:
+break;
+  }
+  // No default in this switch
+  switch (i) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+break;
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  // degenerate, maybe even warning
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch without labels
+  }
+
+  switch (int j = return_integer()) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+  case 2:
+break;
+  }
+
+  // Degenerated, only default case.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+break;
+  }
+
+  // Degenerated, only one case label and default case -> Better as if-stmt.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch could be better written as an if/else statement
+  case 0:
+break;
+  default:
+break;
+  }
+
+  unsigned long long BigNumber = 0;
+  switch (BigNumber) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+
+  const int &IntRef = i;
+  switch (IntRef) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+
+  char C = 'A';
+  switch (C) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 'A':
+break;
+  case 'B':
+break;
+  }
+
+  Bitfields Bf;
+  // UInt has 3 bits size.
+  switch (Bf.UInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered code path; add a default label
+  case 0:
+  case 1:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.UInt) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+break;
+  }
+  // SInt has 1 bit size, so this is somewhat degenerated.
+  switch (Bf.SInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case 0:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.SInt) {
+  case 0:
+  case 1:
+break;
+  }
+
+  bool Flag = false;
+  switch (Flag) {
+// CHECK-MESSAGES:[[@LINE-1]]:3: warning: switch with only one case; use an if statement
+  case true:
+break;
+  }
+
+  switch (Flag) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+break;
+  }
+
+  // This `switch` will create a frontend warning from '-Wswitch-bool' but is
+  // ok for this check.
+  switch (Flag) {
+  case true:
+break;
+  case false:
+break;
+  }
+}
+
+void unproblematic_switch(unsigned char c) {
+  //
+  switch (c) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+  case 8:
+  case 9:
+  case 10:
+  case 11:
+  case 12:
+  case 13:
+  case 14:
+  case 15:
+  case 16:
+  case 17:
+  case 18:
+  case 19:
+  case 20:
+  case 21:
+  case 22:
+  case 23:
+  case 24:
+  case 25:
+  case 26:
+  case 27:
+  case 28:
+  case 29:
+  case 30:
+  case 31:
+  case 32:
+  case 33:
+  case 34:
+  case 35:
+  case 36:
+  case 37:
+  case 38:
+  case 39:
+  case 40:
+  case 41:
+  case 42:
+  case 43:
+  case 44:
+  case 45:
+  case 46:
+  case 47:
+  case 48:
+  case 49:
+  case 50:
+  case 51:
+  case 52:
+  case 53:
+  case 54:
+  case 55:
+  case 56:
+  case 57:
+  case 58:
+  case 59:
+  case 60:
+  case 61:
+  case 62:
+  case 63:
+  case 64:
+  case 65:
+  case 66:
+  case 67:
+  case 68:
+  case 69:
+  case 70:
+  case 71:
+  case 72:
+  case 73:
+  case 74:
+  case 75:
+  case 76:
+  case 77:
+  case 78:
+  case 79:
+  case 80:
+  case 81:
+  case 82:
+  case 83:
+  case 84:
+  case 85:
+  case 86:

[PATCH] D44607: Recompute invalidated iterator in insertTargetAndModeArgs

[Hector Martin via Phabricator via cfe-commits]

marcan added a comment.

I'm not sure how to test this. Getting the bug to manifest involves all of 
having a lot of command line arguments, getting unlucky with the way 
SmallVectorImpl decides to allocate, and getting unlucky with the glibc 
allocator deciding to move the data block on realloc (or running under 
valgrind).

The easiest reproducer I have is `valgrind x86_64-pc-linux-gnu-clang++ $(seq 1 
512)` with a debug build, but I'm not sure that that guarantees an abort in 
every case. 511 arguments is not sufficient to trigger it on my system, but 512 
is.


Repository:
  rC Clang

https://reviews.llvm.org/D44607



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

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis updated this revision to Diff 138858.
zinovy.nis edited the summary of this revision.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44295

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/ParentVirtualCallCheck.cpp
  clang-tidy/bugprone/ParentVirtualCallCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-parent-virtual-call.cpp

Index: test/clang-tidy/bugprone-parent-virtual-call.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-parent-virtual-call.cpp
@@ -0,0 +1,138 @@
+// RUN: %check_clang_tidy %s bugprone-parent-virtual-call %t
+
+extern int foo();
+
+class A {
+public:
+  A() = default;
+  virtual ~A() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class B : public A {
+public:
+  B() = default;
+
+  // Nothing to fix: calls to direct parent.
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+
+class C : public B {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name A::virt_1 refers to a member which was overridden in subclass 'B' [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return B::virt_1() + B::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name A::virt_1 refers to a member which was overridden in subclass 'B' [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return B::virt_1() + B::virt_1(); }
+
+  // Test that non-virtual and static methods are not affected by this cherker.
+  int method_c() { return A::stat() + A::non_virt(); }
+};
+
+// Test that the check affects grand-grand..-parent calls too.
+class D : public C {
+public:
+  int virt_1() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name A::virt_1 refers to a member which was overridden in subclass 'C' [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified name B::virt_1 refers to a member which was overridden in subclass 'C' [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+  int virt_2() override { return A::virt_1() + B::virt_1() + D::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name A::virt_1 refers to a member which was overridden in subclass 'C' [bugprone-parent-virtual-call]
+  // CHECK-MESSAGES: :[[@LINE-2]]:48: warning: qualified name B::virt_1 refers to a member which was overridden in subclass 'C' [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return C::virt_1() + C::virt_1() + D::virt_1(); }
+};
+
+// Test classes in anonymous namespaces.
+namespace {
+class BN : public A {
+public:
+  int virt_1() override { return A::virt_1() + 3; }
+  int virt_2() override { return A::virt_2() + 4; }
+};
+} // namespace N
+
+class CN : public BN {
+public:
+  int virt_1() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name A::virt_1 refers to a member which was overridden in subclass 'BN' [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return BN::virt_1() + BN::virt_1(); }
+  int virt_2() override { return A::virt_1() + BN::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name A::virt_1 refers to a member which was overridden in subclass 'BN' [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_2() override { return BN::virt_1() + BN::virt_1(); }
+};
+
+// Test multiple inheritance fixes
+class AA {
+public:
+  AA() = default;
+  virtual ~AA() = default;
+
+  virtual int virt_1() { return foo() + 1; }
+  virtual int virt_2() { return foo() + 2; }
+
+  int non_virt() { return foo() + 3; }
+  static int stat() { return foo() + 4; }
+};
+
+class BB_1 : virtual public AA {
+public:
+  BB_1() = default;
+
+  // Nothing to fix: calls to parent.
+  int virt_1() override { return AA::virt_1() + 3; }
+  int virt_2() override { return AA::virt_2() + 4; }
+};
+
+class BB_2 : virtual public AA {
+public:
+  BB_2() = default;
+  int virt_1() override { return AA::virt_1() + 3; }
+};
+
+class CC : public BB_1, public BB_2 {
+public:
+  int virt_1() override { return AA::virt_1() + 3; }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified name AA::virt_1 refers to a member which was overridden in subclasses 'BB_1' and 'BB_2' [bugprone-parent-virtual-call]
+  // No fix available due to multiple choice of parent class.
+};
+
+// Test 

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis marked 5 inline comments as done.
zinovy.nis added inline comments.



Comment at: test/clang-tidy/bugprone-parent-virtual-call.cpp:113
+  int virt_1() override { return A::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name 
A::virt_1 refers to a function not from a direct base class; did you mean 'BI'? 
[bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return BI::virt_1(); }

aaron.ballman wrote:
> zinovy.nis wrote:
> > aaron.ballman wrote:
> > > zinovy.nis wrote:
> > > > aaron.ballman wrote:
> > > > > This seems like a false positive to me. Yes, the virtual function is 
> > > > > technically exposed in `BI`, but why is the programmer obligated to 
> > > > > call that one rather than the one from `A`, which is written in the 
> > > > > source?
> > > > IMHO it's a matter of safety. Today virt_1() is not overridden in BI, 
> > > > but tomorrow someone will implement BI::virt_1() and it will silently 
> > > > lead to bugs or whatever.
> > > If tomorrow someone implements `BI::virt_1()`, then the check will start 
> > > diagnosing at that point.
> > Correct, but anyway I don't think it's a problem.
> I'd prefer to see this changed to not diagnose. I don't relish the idea of 
> explaining why that code diagnoses as it stands.
Done. Now the check looks for a most recent class with the overridden method. 
Please review this change.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44295



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

[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis marked 6 inline comments as done.
zinovy.nis added inline comments.



Comment at: clang-tidy/bugprone/ParentVirtualCallCheck.cpp:128
+  diag(Member->getQualifierLoc().getSourceRange().getBegin(),
+   "'%0' is a grand-parent's method, not parent's. Did you mean %1?")
+  << CalleeName << ParentsStr;

aaron.ballman wrote:
> zinovy.nis wrote:
> > aaron.ballman wrote:
> > > The diagnostic should not contain punctuation aside from the question 
> > > mark.
> > > 
> > > Also, the diagnostic uses some novel terminology in "grandparent's 
> > > method". How about: "qualified function name %0 refers to a function that 
> > > is not defined by a direct base class; did you mean %1?"
> > "is not defined by a direct base class" is not a correct proposal. Issues 
> > that this check finds are all about calling a grand-parent's method instead 
> > of parent's whether base class defines this method or not.
> > 
> > How about 
> > 
> > > Qualified function name %0 refers to a function not from a direct base 
> > > class; did you mean %1?
> > 
> > ?
> Not keen on "from", but we could use `"qualified function name %q0 refers to 
> a function that is not explicitly declared by a direct base class; did you 
> mean %1?"`
Changed to  

> qualified name %0 refers to a member which was overridden in subclass%1"




Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44295



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

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

[Francois JEAN via Phabricator via cfe-commits]

Wawha created this revision.
Herald added subscribers: cfe-commits, klimek.

This is a patch to fix the problem identify by this bug: 
https://bugs.llvm.org/show_bug.cgi?id=27640.

When formatting this code with option "BreakBeforeBraces: Allman", the lambda 
body are not put entirely on new lines if the lambda is inside a function call.
Here an example of the current formatting in "allman":

  connect([]() {
foo();
bar();
  });

We the new option, the formatting is like that:

  connect(
[]()
{
  foo();
  bar();
});

It's my first patch for this project, so if I made some mistake, do not 
hesitate to explain me that is wrong.
I write few test to check that the new option work and check that there is no 
regression.

This patch should also meet the request of this bug/request: 
https://bugs.llvm.org//show_bug.cgi?id=32151


Repository:
  rC Clang

https://reviews.llvm.org/D44609

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/TokenAnnotator.h

Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -872,6 +872,24 @@
   /// \endcode
   bool BreakBeforeInheritanceComma;
 
+  /// \brief Wrap lambda block inside function parameters list.
+  /// \code
+  ///   true:
+  ///   connect(
+  /// []()
+  /// {
+  ///   foo();
+  ///   bar();
+  /// });
+  ///
+  ///   false:
+  ///   connect([]() {
+  /// foo();
+  /// bar();
+  ///   });
+  /// \endcode
+  bool BreakBeforeLambdaBody;
+
   /// \brief If ``true``, consecutive namespace declarations will be on the same
   /// line. If ``false``, each namespace is declared on a new line.
   /// \code
@@ -1723,6 +1741,7 @@
BreakStringLiterals == R.BreakStringLiterals &&
ColumnLimit == R.ColumnLimit && CommentPragmas == R.CommentPragmas &&
BreakBeforeInheritanceComma == R.BreakBeforeInheritanceComma &&
+   BreakBeforeLambdaBody == R.BreakBeforeLambdaBody &&
ConstructorInitializerAllOnOneLineOrOnePerLine ==
R.ConstructorInitializerAllOnOneLineOrOnePerLine &&
ConstructorInitializerIndentWidth ==
Index: lib/Format/TokenAnnotator.h
===
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -94,6 +94,28 @@
   template  bool endsWith(Ts... Tokens) const {
 return Last && Last->endsSequence(Tokens...);
   }
+  /// \c true if this line, starting at token 'Start', contains the given tokens in reverse order,
+  /// ignoring comments.
+  template  bool containsBefore(const FormatToken& Start, Ts... Tokens) const {
+const FormatToken *Tok = &Start;
+while (Tok) {
+if(Tok->startsSequence(Tokens...))
+return true;
+Tok = Tok->Previous;
+}
+return false;
+  }
+  /// \c true if this line, starting at token 'Start', contains the given tokens in order,
+  /// ignoring comments.
+  template  bool containsAfter(const FormatToken& Start, Ts... Tokens) const {
+const FormatToken *Tok = &Start;
+while (Tok) {
+if(Tok->startsSequence(Tokens...))
+return true;
+Tok = Tok->Next;
+}
+return false;
+  }
 
   /// \c true if this line looks like a function definition instead of a
   /// function declaration. Asserts MightBeFunctionDecl.
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2841,7 +2841,8 @@
   if (Right.is(TT_InlineASMBrace))
 return Right.HasUnescapedNewline;
   if (isAllmanBrace(Left) || isAllmanBrace(Right))
-return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
+return (Line.containsBefore(Right, TT_LambdaLSquare) && Style.BreakBeforeLambdaBody) ||
+   (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
(Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
 Style.BraceWrapping.AfterEnum) ||
(Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) ||
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -337,6 +337,7 @@
 IO.mapOptional("BreakBeforeBraces", Style.BreakBeforeBraces);
 IO.mapOptional("BreakBeforeInheritanceComma",
Style.BreakBeforeInheritanceComma);
+IO.mapOptional("BreakBeforeLambdaBody", Style.BreakBeforeLambdaBody);
 IO.mapOptional("BreakBeforeTernaryOperators",
Style.BreakBeforeTernaryOperators);
 
@@ -623,6 +624,7 @@
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
   LLVMStyle.BreakBeforeInheritanc

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Please always upload all patches with full context (`-U99`)


Repository:
  rC Clang

https://reviews.llvm.org/D44609



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

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Also, tests?


Repository:
  rC Clang

https://reviews.llvm.org/D44609



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

[PATCH] D43322: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

@rtrieu ping! I've rebased this on the pure refactoring you committed for me 
last week.


Repository:
  rC Clang

https://reviews.llvm.org/D43322



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

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

[Francois JEAN via Phabricator via cfe-commits]

Wawha updated this revision to Diff 138859.
Wawha added a comment.

I upload the full context for these files.


Repository:
  rC Clang

https://reviews.llvm.org/D44609

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  lib/Format/TokenAnnotator.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11357,6 +11357,42 @@
   "> {\n"
   "  //\n"
   "});");
+
+  // Check option "BreakBeforeLambdaBody"
+  FormatStyle LLVMWithBreakBeforeLambdaBody = getLLVMStyle();
+  LLVMWithBreakBeforeLambdaBody.BreakBeforeLambdaBody = true;
+  verifyFormat("FunctionWithOneParam(\n"
+   "[]()\n"
+   "{\n"
+   "  // A cool function...\n"
+   "  return 43;\n"
+   "},\n"
+   "87);",
+   LLVMWithBreakBeforeLambdaBody);
+  verifyFormat("FunctionWithTwoParams(\n"
+   "[]()\n"
+   "{\n"
+   "  // A cool function...\n"
+   "  return 43;\n"
+   "},\n"
+   "87);",
+   LLVMWithBreakBeforeLambdaBody);
+  verifyFormat("TwoNestedLambdas(\n"
+   "[]()\n"
+   "{\n"
+   "  return Call(\n"
+   "  []()\n"
+   "  {\n"
+   "return 17;\n"
+   "  });\n"
+   "});",
+   LLVMWithBreakBeforeLambdaBody);
+  verifyFormat("auto array = {[]()\n"
+   "  {\n"
+   "return 43;\n"
+   "  },\n"
+   "  MyFunctor};",
+   LLVMWithBreakBeforeLambdaBody);
 }
 
 TEST_F(FormatTest, EmptyLinesInLambdas) {
Index: lib/Format/TokenAnnotator.h
===
--- lib/Format/TokenAnnotator.h
+++ lib/Format/TokenAnnotator.h
@@ -94,6 +94,28 @@
   template  bool endsWith(Ts... Tokens) const {
 return Last && Last->endsSequence(Tokens...);
   }
+  /// \c true if this line, starting at token 'Start', contains the given tokens in reverse order,
+  /// ignoring comments.
+  template  bool containsBefore(const FormatToken& Start, Ts... Tokens) const {
+const FormatToken *Tok = &Start;
+while (Tok) {
+if(Tok->startsSequence(Tokens...))
+return true;
+Tok = Tok->Previous;
+}
+return false;
+  }
+  /// \c true if this line, starting at token 'Start', contains the given tokens in order,
+  /// ignoring comments.
+  template  bool containsAfter(const FormatToken& Start, Ts... Tokens) const {
+const FormatToken *Tok = &Start;
+while (Tok) {
+if(Tok->startsSequence(Tokens...))
+return true;
+Tok = Tok->Next;
+}
+return false;
+  }
 
   /// \c true if this line looks like a function definition instead of a
   /// function declaration. Asserts MightBeFunctionDecl.
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2841,7 +2841,8 @@
   if (Right.is(TT_InlineASMBrace))
 return Right.HasUnescapedNewline;
   if (isAllmanBrace(Left) || isAllmanBrace(Right))
-return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
+return (Line.containsBefore(Right, TT_LambdaLSquare) && Style.BreakBeforeLambdaBody) ||
+   (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
(Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
 Style.BraceWrapping.AfterEnum) ||
(Line.startsWith(tok::kw_class) && Style.BraceWrapping.AfterClass) ||
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -337,6 +337,7 @@
 IO.mapOptional("BreakBeforeBraces", Style.BreakBeforeBraces);
 IO.mapOptional("BreakBeforeInheritanceComma",
Style.BreakBeforeInheritanceComma);
+IO.mapOptional("BreakBeforeLambdaBody", Style.BreakBeforeLambdaBody);
 IO.mapOptional("BreakBeforeTernaryOperators",
Style.BreakBeforeTernaryOperators);
 
@@ -623,6 +624,7 @@
   LLVMStyle.BreakAfterJavaFieldAnnotations = false;
   LLVMStyle.BreakConstructorInitializers = FormatStyle::BCIS_BeforeColon;
   LLVMStyle.BreakBeforeInheritanceComma = false;
+  LLVMStyle.BreakBeforeLambdaBody = false;
   LLVMStyle.BreakStringLiterals = true;
   LLVMStyle.ColumnLimit = 80;
   LLVMStyle.CommentPragmas = "^ IWYU pragma:";
Index: lib/Format/ContinuationIndenter.cpp
===

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

[Francois JEAN via Phabricator via cfe-commits]

Wawha added a comment.

In https://reviews.llvm.org/D44609#1041260, @lebedev.ri wrote:

> Also, tests?


Sorry, the test files was missing with the first patch. I make mistake using 
svn... I create the new patch with git, it's easier for me.
There is 4 small tests inside unittests/Format/FormatTest.cpp. The first 3 are 
testing the new options, and the last one is here to check that there is no 
regression with initialiser list which contain lambda.


Repository:
  rC Clang

https://reviews.llvm.org/D44609



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

[libcxx] r327806 - Updated C++2a status with changes from Jacksonville WG21 meeting

[Marshall Clow via cfe-commits]

Author: marshall
Date: Sun Mar 18 12:29:21 2018
New Revision: 327806

URL: http://llvm.org/viewvc/llvm-project?rev=327806&view=rev
Log:
Updated C++2a status with changes from Jacksonville WG21 meeting


Modified:
libcxx/trunk/www/cxx2a_status.html

Modified: libcxx/trunk/www/cxx2a_status.html
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/cxx2a_status.html?rev=327806&r1=327805&r2=327806&view=diff
==
--- libcxx/trunk/www/cxx2a_status.html (original)
+++ libcxx/trunk/www/cxx2a_status.html Sun Mar 18 12:29:21 2018
@@ -72,6 +72,15 @@
https://wg21.link/P0767R1";>P0767R1CWGDeprecate 
PODAlbuquerqueComplete7.0
https://wg21.link/P0768R1";>P0768R1CWGLibrary 
Support for the Spaceship (Comparison) 
OperatorAlbuquerque
https://wg21.link/P0777R1";>P0777R1LWGTreating 
Unnecessary 
decayAlbuquerqueComplete7.0
+   https://wg21.link/P0122R7";>P0122R7LWGJacksonville
+   https://wg21.link/P0355R7";>P0355R7LWGExtending 
chrono to Calendars and Time 
ZonesJacksonville
+   https://wg21.link/P0551R3";>P0551R3LWGThou Shalt Not 
Specialize std Function 
Templates!Jacksonville
+   https://wg21.link/P0753R2";>P0753R2LWGManipulators 
for C++ Synchronized Buffered 
OstreamJacksonville
+   https://wg21.link/P0754R2";>P0754R2LWGJacksonville
+   https://wg21.link/P0809R0";>P0809R0LWGComparing 
Unordered ContainersJacksonville
+   https://wg21.link/P0858R0";>P0858R0LWGConstexpr 
iterator requirementsJacksonville
+   https://wg21.link/P0905R1";>P0905R1CWGSymmetry for 
spaceshipJacksonville
+   https://wg21.link/P0966R1";>P0966R1LWGstring::reserve
 Should Not ShrinkJacksonville
 
 
   
@@ -132,10 +141,46 @@
https://wg21.link/LWG3001";>3001weak_ptr::element_type needs 
remove_extent_tAlbuquerque
https://wg21.link/LWG3024";>3024variant's 
copies must be deleted instead of disabled via 
SFINAEAlbuquerque
 
+   
+   https://wg21.link/LWG2164";>2164What are 
the semantics of vector.emplace(vector.begin(), 
vector.back())?Jacksonville
+   https://wg21.link/LWG2243";>2243istream::putback 
problemJacksonvilleComplete
+   https://wg21.link/LWG2816";>2816resize_file has 
impossible postconditionJacksonvilleNothing to 
do
+   https://wg21.link/LWG2843";>2843Unclear 
behavior of 
std::pmr::memory_resource::do_allocate()Jacksonville
+   https://wg21.link/LWG2849";>2849Why does 
!is_regular_file(from) cause copy_file to report a "file 
already exists" error?JacksonvilleNothing to 
do
+   https://wg21.link/LWG2851";>2851std::filesystem enum 
classes are now underspecifiedJacksonvilleNothing to 
do
+   https://wg21.link/LWG2946";>2946LWG 2758's 
resolution missed further correctionsJacksonville
+   https://wg21.link/LWG2969";>2969polymorphic_allocator::construct()
 shouldn't pass resource()Jacksonville
+   https://wg21.link/LWG2975";>2975Missing 
case for pair construction in scoped and polymorphic 
allocatorsJacksonville
+   https://wg21.link/LWG2989";>2989path's stream 
insertion operator lets you insert everything under the 
sunJacksonvilleCompleted
+   https://wg21.link/LWG3000";>3000monotonic_memory_resource::do_is_equal
 uses dynamic_cast 
unnecessarilyJacksonville
+   https://wg21.link/LWG3002";>3002[networking.ts] 
basic_socket_acceptor::is_open() isn't 
noexceptJacksonville
+   https://wg21.link/LWG3004";>3004§[string.capacity] and 
§[vector.capacity] should specify time complexity for 
capacity()JacksonvilleNothing to do
+   https://wg21.link/LWG3005";>3005Destruction order of arrays 
by make_shared/allocate_shared only 
recommended?Jacksonville
+   https://wg21.link/LWG3007";>3007allocate_shared 
should rebind allocator to cv-unqualified value_type for 
constructionJacksonville
+   https://wg21.link/LWG3009";>3009Including 
 doesn't provide 
std::size/empty/dataJacksonvilleComplete
+   https://wg21.link/LWG3010";>3010[networking.ts] 
uses_executor says "if a type T::executor_type 
exists"Jacksonville
+   https://wg21.link/LWG3013";>3013(recursive_)directory_iterator
 construction and traversal should not be 
noexceptJacksonvilleComplete
+   https://wg21.link/LWG3014";>3014More 
noexcept issues with filesystem 
operationsJacksonvilleComplete
+   https://wg21.link/LWG3015";>3015copy_options::unspecified
 underspecifiedJacksonvilleNothing to do
+   https://wg21.link/LWG3017";>3017list 
splice functions should use 
addressofJacksonville
+   https://wg21.link/LWG3020";>3020[networking.ts] Remove 
spurious nested value_type buffer sequence 
requirementJacksonville
+   https://wg21.link/LWG3026";>3026filesystem::weakly_canonical
 still defined in terms of canonical(p, 
base)Jacksonville
+   https://wg21.link/LWG3030";>3030Who shall 
meet the requirements of 
try_lock?JacksonvilleNothing to do
+   https://wg21.link/LWG3034";>3034P0767R1 
breaks previously-st

[PATCH] D38216: [C++17] Fix class template argument deduction for default constructors without an initializer

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete updated this revision to Diff 138862.
Rakete added a comment.

@lichray Ok done :) Thanks for reviewing


https://reviews.llvm.org/D38216

Files:
  lib/Sema/SemaDecl.cpp
  test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.class.deduct/p1.cpp
  test/Parser/cxx1z-class-template-argument-deduction.cpp


Index: test/Parser/cxx1z-class-template-argument-deduction.cpp
===
--- test/Parser/cxx1z-class-template-argument-deduction.cpp
+++ test/Parser/cxx1z-class-template-argument-deduction.cpp
@@ -137,7 +137,6 @@
 (void)A{n};
 (void)new A(n);
 (void)new A{n};
-// FIXME: We should diagnose the lack of an initializer here.
 (void)new A;
   }
 }
@@ -150,7 +149,7 @@
 
   auto k() -> A; // expected-error{{requires template arguments}}
 
-  A a; // expected-error {{declaration of variable 'a' with deduced type 'A' 
requires an initializer}}
+  A a;
   A b = 0;
   const A c = 0;
   A (parens) = 0; // expected-error {{cannot use parentheses when declaring 
variable with deduced class template specialization type}}
Index: test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.class.deduct/p1.cpp
===
--- test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.class.deduct/p1.cpp
+++ test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.class.deduct/p1.cpp
@@ -5,8 +5,7 @@
 A(int) -> A;
 
 static constexpr inline const volatile A a = {}; // ok, specifiers are 
permitted
-// FIXME: There isn't really a good reason to reject this.
-A b; // expected-error {{requires an initializer}}
+A b;
 A c [[]] {};
 
 A d = {}, e = {};
@@ -16,3 +15,5 @@
   static A a; // expected-error {{requires an initializer}}
 };
 extern A x; // expected-error {{requires an initializer}}
+static A y;
+
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -10396,12 +10396,22 @@
   // C++11 [dcl.spec.auto]p3
   if (!Init) {
 assert(VDecl && "no init for init capture deduction?");
-Diag(VDecl->getLocation(), diag::err_auto_var_requires_init)
-  << VDecl->getDeclName() << Type;
-return QualType();
+
+// Except for class argument deduction, and then for an initializing
+// declaration only, i.e. no static at class scope or extern.
+if (!isa(Deduced) ||
+VDecl->hasExternalStorage() ||
+VDecl->isStaticDataMember()) {
+  Diag(VDecl->getLocation(), diag::err_auto_var_requires_init)
+<< VDecl->getDeclName() << Type;
+  return QualType();
+}
   }
 
-  ArrayRef DeduceInits = Init;
+  ArrayRef DeduceInits;
+  if (Init)
+DeduceInits = Init;
+
   if (DirectInit) {
 if (auto *PL = dyn_cast_or_null(Init))
   DeduceInits = PL->exprs();


Index: test/Parser/cxx1z-class-template-argument-deduction.cpp
===
--- test/Parser/cxx1z-class-template-argument-deduction.cpp
+++ test/Parser/cxx1z-class-template-argument-deduction.cpp
@@ -137,7 +137,6 @@
 (void)A{n};
 (void)new A(n);
 (void)new A{n};
-// FIXME: We should diagnose the lack of an initializer here.
 (void)new A;
   }
 }
@@ -150,7 +149,7 @@
 
   auto k() -> A; // expected-error{{requires template arguments}}
 
-  A a; // expected-error {{declaration of variable 'a' with deduced type 'A' requires an initializer}}
+  A a;
   A b = 0;
   const A c = 0;
   A (parens) = 0; // expected-error {{cannot use parentheses when declaring variable with deduced class template specialization type}}
Index: test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.class.deduct/p1.cpp
===
--- test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.class.deduct/p1.cpp
+++ test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.type.class.deduct/p1.cpp
@@ -5,8 +5,7 @@
 A(int) -> A;
 
 static constexpr inline const volatile A a = {}; // ok, specifiers are permitted
-// FIXME: There isn't really a good reason to reject this.
-A b; // expected-error {{requires an initializer}}
+A b;
 A c [[]] {};
 
 A d = {}, e = {};
@@ -16,3 +15,5 @@
   static A a; // expected-error {{requires an initializer}}
 };
 extern A x; // expected-error {{requires an initializer}}
+static A y;
+
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -10396,12 +10396,22 @@
   // C++11 [dcl.spec.auto]p3
   if (!Init) {
 assert(VDecl && "no init for init capture deduction?");
-Diag(VDecl->getLocation(), diag::err_auto_var_requires_init)
-  << VDecl->getDeclName() << Type;
-return QualType();
+
+// Except for class argument deduction, and then for an initializing
+// declaration only, i.e. no static at class scope or extern.
+if (!isa(Deduced) ||
+VDecl->hasExternalStorage() ||
+VDecl->isStaticDataMember()) {
+  Diag(VDecl->getLocation(),

Re: r327738 - [MS] Don't escape MS C++ names with \01

[Nico Weber via cfe-commits]

Awesome, thanks!

On Sun, Mar 18, 2018, 3:05 PM Reid Kleckner  wrote:

> Yes: https://reviews.llvm.org/D7775
>
> This side is very mechanical, so I did not send it for review.
>
>
> On Sun, Mar 18, 2018, 3:54 AM Nico Weber  wrote:
>
>> Was this discussed or reviewed somewhere? (It looks like a good change to
>> me, I'm just wondering if there was something that triggered this and would
>> like to read about the background if there's anything to read.)
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44580: Sema: allow comparison between blocks & block-compatible objc types

[Dustin L. Howett via Phabricator via cfe-commits]

DHowett-MSFT planned changes to this revision.
DHowett-MSFT marked an inline comment as done.
DHowett-MSFT added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:7749
+// id (or strictly compatible object type) -> T^
+if (getLangOpts().ObjC1 && 
RHSType->isBlockCompatibleObjCPointerType(Context)) {
   Kind = CK_AnyPointerToBlockPointerCast;

theraven wrote:
> Do we want to allow implicit casts for all object types to block types for 
> assignment, or only for null pointers?  We definitely want to allow `nil` to 
> be assigned to a block type, but I would lean slightly to requiring an 
> implicit cast.
> 
> Ideally, I think we'd allow this but warn, because casting from an arbitrary 
> ObjC type to a block incorrectly can cause exciting security vulnerabilities 
> if it's done incorrectly and we should encourage people to check these casts 
> (`nil` is always safe though - as long as somewhere else checks the 
> nullability attributes).
I don't actually have a compelling use case for widening assignability here. 
I'm dropping this part of the patch.

I do think it should be valid, perhaps with a warning. It feels incorrect for 
there to be valid comparison cases that are not valid assignment cases (here), 
but that position doesn't hold up under scrutiny.


Repository:
  rC Clang

https://reviews.llvm.org/D44580



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

[PATCH] D44580: Sema: allow comparison between blocks & block-compatible objc types

[Dustin L. Howett via Phabricator via cfe-commits]

DHowett-MSFT updated this revision to Diff 138868.
DHowett-MSFT marked an inline comment as done.
DHowett-MSFT added a comment.

Backed out changes to block type assignment.


Repository:
  rC Clang

https://reviews.llvm.org/D44580

Files:
  lib/Sema/SemaExpr.cpp
  test/SemaObjC/block-compare.mm


Index: test/SemaObjC/block-compare.mm
===
--- /dev/null
+++ test/SemaObjC/block-compare.mm
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -S -o - -triple i686-windows -verify -fblocks \
+// RUN: -Wno-unused-comparison %s
+
+#pragma clang diagnostic ignored "-Wunused-comparison"
+
+#define nil ((id)nullptr)
+
+@protocol NSObject
+@end
+
+@protocol NSCopying
+@end
+
+@protocol OtherProtocol
+@end
+
+__attribute__((objc_root_class))
+@interface NSObject 
+@end
+
+__attribute__((objc_root_class))
+@interface Test
+@end
+
+int main() {
+  void (^block)() = ^{};
+  NSObject *object;
+  id qualifiedId;
+
+  id poorlyQualified1;
+  Test *objectOfWrongType;
+
+  block == nil;
+  block == object;
+  block == qualifiedId;
+
+  nil == block;
+  object == block;
+  qualifiedId == block;
+
+  // these are still not valid: blocks must be compared with id, NSObject*, or 
a protocol-qualified id
+  // conforming to NSCopying or NSObject.
+
+  block == poorlyQualified1; // expected-error {{invalid operands to binary 
expression ('void (^)()' and 'id')}}
+  block == objectOfWrongType; // expected-error {{invalid operands to binary 
expression ('void (^)()' and 'Test *')}}
+
+  poorlyQualified1 == block; // expected-error {{invalid operands to binary 
expression ('id' and 'void (^)()')}}
+  objectOfWrongType == block; // expected-error {{invalid operands to binary 
expression ('Test *' and 'void (^)()')}}
+
+  return 0;
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -10028,6 +10028,18 @@
 RHS = ImpCastExprToType(RHS.get(), LHSType, CK_BitCast);
   return ResultTy;
 }
+
+if(!IsRelational &&
+   LHSType->isBlockPointerType() &&
+   RHSType->isBlockCompatibleObjCPointerType(Context)) {
+  LHS = ImpCastExprToType(LHS.get(), RHSType, 
CK_BlockPointerToObjCPointerCast);
+  return ResultTy;
+} else if(!IsRelational &&
+  LHSType->isBlockCompatibleObjCPointerType(Context) &&
+  RHSType->isBlockPointerType()) {
+  RHS = ImpCastExprToType(RHS.get(), LHSType, 
CK_BlockPointerToObjCPointerCast);
+  return ResultTy;
+}
   }
   if ((LHSType->isAnyPointerType() && RHSType->isIntegerType()) ||
   (LHSType->isIntegerType() && RHSType->isAnyPointerType())) {


Index: test/SemaObjC/block-compare.mm
===
--- /dev/null
+++ test/SemaObjC/block-compare.mm
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -S -o - -triple i686-windows -verify -fblocks \
+// RUN: -Wno-unused-comparison %s
+
+#pragma clang diagnostic ignored "-Wunused-comparison"
+
+#define nil ((id)nullptr)
+
+@protocol NSObject
+@end
+
+@protocol NSCopying
+@end
+
+@protocol OtherProtocol
+@end
+
+__attribute__((objc_root_class))
+@interface NSObject 
+@end
+
+__attribute__((objc_root_class))
+@interface Test
+@end
+
+int main() {
+  void (^block)() = ^{};
+  NSObject *object;
+  id qualifiedId;
+
+  id poorlyQualified1;
+  Test *objectOfWrongType;
+
+  block == nil;
+  block == object;
+  block == qualifiedId;
+
+  nil == block;
+  object == block;
+  qualifiedId == block;
+
+  // these are still not valid: blocks must be compared with id, NSObject*, or a protocol-qualified id
+  // conforming to NSCopying or NSObject.
+
+  block == poorlyQualified1; // expected-error {{invalid operands to binary expression ('void (^)()' and 'id')}}
+  block == objectOfWrongType; // expected-error {{invalid operands to binary expression ('void (^)()' and 'Test *')}}
+
+  poorlyQualified1 == block; // expected-error {{invalid operands to binary expression ('id' and 'void (^)()')}}
+  objectOfWrongType == block; // expected-error {{invalid operands to binary expression ('Test *' and 'void (^)()')}}
+
+  return 0;
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -10028,6 +10028,18 @@
 RHS = ImpCastExprToType(RHS.get(), LHSType, CK_BitCast);
   return ResultTy;
 }
+
+if(!IsRelational &&
+   LHSType->isBlockPointerType() &&
+   RHSType->isBlockCompatibleObjCPointerType(Context)) {
+  LHS = ImpCastExprToType(LHS.get(), RHSType, CK_BlockPointerToObjCPointerCast);
+  return ResultTy;
+} else if(!IsRelational &&
+  LHSType->isBlockCompatibleObjCPointerType(Context) &&
+  RHSType->isBlockPointerType()) {
+  RHS = ImpCastExprToType(RHS.get(), LHSType, CK_BlockPointerToObjCPointerCast);
+  return ResultTy;
+}
   }
   if ((LHSType->isAnyPointerTy

[PATCH] D44616: Update existed CodeGen TBAA tests

[Danil Malyshev via Phabricator via cfe-commits]

DanilM created this revision.
DanilM added reviewers: hfinkel, kosarev.
Herald added a subscriber: cfe-commits.

Just added testing of the new TBAA format to existed tests for old TBAA.


Repository:
  rC Clang

https://reviews.llvm.org/D44616

Files:
  test/CodeGen/tbaa-base.cpp
  test/CodeGen/tbaa-cast.cpp
  test/CodeGen/tbaa-class.cpp
  test/CodeGen/tbaa-for-vptr.cpp
  test/CodeGen/tbaa-ms-abi.cpp
  test/CodeGen/tbaa-reference.cpp

Index: test/CodeGen/tbaa-reference.cpp
===
--- test/CodeGen/tbaa-reference.cpp
+++ test/CodeGen/tbaa-reference.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-linux -O1 -disable-llvm-passes %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux -O1 -disable-llvm-passes %s -emit-llvm -o - | FileCheck %s  -check-prefixes=CHECK,OLD-PATH
+// RUN: %clang_cc1 -triple x86_64-linux -O1 -disable-llvm-passes %s -emit-llvm -new-struct-path-tbaa -o - | FileCheck %s -check-prefixes=CHECK,NEW-PATH
 //
 // Check that we generate correct TBAA information for reference accesses.
 
@@ -29,9 +30,16 @@
   return s;
 }
 
-// CHECK-DAG: [[TAG_pointer]] = !{[[TYPE_pointer:!.*]], [[TYPE_pointer]], i64 0}
-// CHECK-DAG: [[TAG_B_s]] = !{[[TYPE_B:!.*]], [[TYPE_pointer]], i64 0}
+// OLD-PATH-DAG: [[TAG_pointer]] = !{[[TYPE_pointer:!.*]], [[TYPE_pointer]], i64 0}
+// OLD-PATH-DAG: [[TAG_B_s]] = !{[[TYPE_B:!.*]], [[TYPE_pointer]], i64 0}
 //
-// CHECK-DAG: [[TYPE_B]] = !{!"_ZTS1B", [[TYPE_pointer]], i64 0}
-// CHECK-DAG: [[TYPE_pointer]] = !{!"any pointer", [[TYPE_char:!.*]], i64 0}
-// CHECK-DAG: [[TYPE_char]] = !{!"omnipotent char", {{!.*}}, i64 0}
+// OLD-PATH-DAG: [[TYPE_B]] = !{!"_ZTS1B", [[TYPE_pointer]], i64 0}
+// OLD-PATH-DAG: [[TYPE_pointer]] = !{!"any pointer", [[TYPE_char:!.*]], i64 0}
+// OLD-PATH-DAG: [[TYPE_char]] = !{!"omnipotent char", {{!.*}}, i64 0}
+
+// NEW-PATH-DAG: [[TAG_pointer]] = !{[[TYPE_pointer:!.*]], [[TYPE_pointer]], i64 0, i64 8}
+// NEW-PATH-DAG: [[TAG_B_s]] = !{[[TYPE_B:!.*]], [[TYPE_pointer]], i64 0, i64 8}
+//
+// NEW-PATH-DAG: [[TYPE_B]] = !{[[TYPE_char:!.*]], i64 8, !"_ZTS1B", [[TYPE_pointer]], i64 0, i64 8}
+// NEW-PATH-DAG: [[TYPE_pointer]] = !{[[TYPE_char:!.*]], i64 8, !"any pointer"}
+// NEW-PATH-DAG: [[TYPE_char]] = !{{{!.*}}, i64 1, !"omnipotent char"}
Index: test/CodeGen/tbaa-ms-abi.cpp
===
--- test/CodeGen/tbaa-ms-abi.cpp
+++ test/CodeGen/tbaa-ms-abi.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple i686-pc-win32 -disable-llvm-passes -emit-llvm -o - -O1 %s | FileCheck %s
+// RUN: %clang_cc1 -triple i686-pc-win32 -disable-llvm-passes -emit-llvm -o - -O1 %s | FileCheck %s -check-prefixes=CHECK,OLD-PATH
+// RUN: %clang_cc1 -triple i686-pc-win32 -disable-llvm-passes -emit-llvm -new-struct-path-tbaa -o - -O1 %s | FileCheck %s -check-prefixes=CHECK,NEW-PATH
 //
 // Test that TBAA works in the Microsoft C++ ABI.  We used to error out while
 // attempting to mangle RTTI.
@@ -16,7 +17,11 @@
 // CHECK: store i32 42, i32* {{.*}}, !tbaa [[TAG_A_i32:!.*]]
 }
 
-// CHECK: [[TYPE_INT:!.*]] = !{!"int", [[TYPE_CHAR:!.*]], i64 0}
-// CHECK: [[TYPE_CHAR]] = !{!"omnipotent char", !
-// CHECK: [[TAG_A_i32]] = !{[[TYPE_A:!.*]], [[TYPE_INT]], i64 0}
-// CHECK: [[TYPE_A]] = !{!"?AUStructA@@", [[TYPE_INT]], i64 0}
+// OLD-PATH: [[TYPE_INT:!.*]] = !{!"int", [[TYPE_CHAR:!.*]], i64 0}
+// OLD-PATH: [[TYPE_CHAR]] = !{!"omnipotent char", !
+// OLD-PATH: [[TAG_A_i32]] = !{[[TYPE_A:!.*]], [[TYPE_INT]], i64 0}
+// OLD-PATH: [[TYPE_A]] = !{!"?AUStructA@@", [[TYPE_INT]], i64 0}
+// NEW-PATH: [[TYPE_INT:!.*]] = !{[[TYPE_CHAR:!.*]], i64 4, !"int"}
+// NEW-PATH: [[TYPE_CHAR]] = !{{{.*}}, i64 1, !"omnipotent char"}
+// NEW-PATH: [[TAG_A_i32]] = !{[[TYPE_A:!.*]], [[TYPE_INT]], i64 0, i64 4}
+// NEW-PATH: [[TYPE_A]] = !{[[TYPE_CHAR]], i64 4, !"?AUStructA@@", [[TYPE_INT]], i64 0, i64 4}
Index: test/CodeGen/tbaa-for-vptr.cpp
===
--- test/CodeGen/tbaa-for-vptr.cpp
+++ test/CodeGen/tbaa-for-vptr.cpp
@@ -1,6 +1,10 @@
-// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - -fsanitize=thread %s | FileCheck %s
-// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - -O1 %s | FileCheck %s
-// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - -O1  -relaxed-aliasing -fsanitize=thread %s | FileCheck %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - -fsanitize=thread %s | FileCheck %s --check-prefixes=CHECK,OLD-PATH
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - -O1 %s | FileCheck %s --check-prefixes=CHECK,OLD-PATH
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - -O1  -relaxed-aliasing -fsanitize=thread %s | FileCheck %s --check-prefixes=CHECK,OLD-PATH
+//
+// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -new-struct-path-tbaa -o - -fsanitize=thread %s | FileCheck %s --check-prefixes=CHECK,NEW-PATH
+// RUN: %clang_cc1 -

[libclc] r327818 - remainder: Port from amd builtins

[Jan Vesely via cfe-commits]

Author: jvesely
Date: Sun Mar 18 18:01:10 2018
New Revision: 327818

URL: http://llvm.org/viewvc/llvm-project?rev=327818&view=rev
Log:
remainder: Port from amd builtins

Mostly ported from amd_builtins, uses only denormal path for fp32.
Passes CTS on carrizo and turks

Reviewer: Aaron Watry 
Signed-off-by: Jan Vesely 

Added:
libclc/trunk/generic/include/clc/math/remainder.h
libclc/trunk/generic/include/math/clc_remainder.h
libclc/trunk/generic/lib/math/clc_remainder.cl
libclc/trunk/generic/lib/math/remainder.cl
Modified:
libclc/trunk/generic/include/clc/clc.h
libclc/trunk/generic/lib/SOURCES

Modified: libclc/trunk/generic/include/clc/clc.h
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/include/clc/clc.h?rev=327818&r1=327817&r2=327818&view=diff
==
--- libclc/trunk/generic/include/clc/clc.h (original)
+++ libclc/trunk/generic/include/clc/clc.h Sun Mar 18 18:01:10 2018
@@ -103,6 +103,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 

Added: libclc/trunk/generic/include/clc/math/remainder.h
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/include/clc/math/remainder.h?rev=327818&view=auto
==
--- libclc/trunk/generic/include/clc/math/remainder.h (added)
+++ libclc/trunk/generic/include/clc/math/remainder.h Sun Mar 18 18:01:10 2018
@@ -0,0 +1,4 @@
+#define __CLC_FUNCTION remainder
+#define __CLC_BODY 
+#include 
+#undef __CLC_FUNCTION

Added: libclc/trunk/generic/include/math/clc_remainder.h
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/include/math/clc_remainder.h?rev=327818&view=auto
==
--- libclc/trunk/generic/include/math/clc_remainder.h (added)
+++ libclc/trunk/generic/include/math/clc_remainder.h Sun Mar 18 18:01:10 2018
@@ -0,0 +1,4 @@
+#define __CLC_FUNCTION __clc_remainder
+#define __CLC_BODY 
+#include 
+#undef __CLC_FUNCTION

Modified: libclc/trunk/generic/lib/SOURCES
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/SOURCES?rev=327818&r1=327817&r2=327818&view=diff
==
--- libclc/trunk/generic/lib/SOURCES (original)
+++ libclc/trunk/generic/lib/SOURCES Sun Mar 18 18:01:10 2018
@@ -158,6 +158,8 @@ math/clc_pown.cl
 math/pown.cl
 math/clc_powr.cl
 math/powr.cl
+math/clc_remainder.cl
+math/remainder.cl
 math/clc_rootn.cl
 math/rootn.cl
 math/sin.cl

Added: libclc/trunk/generic/lib/math/clc_remainder.cl
URL: 
http://llvm.org/viewvc/llvm-project/libclc/trunk/generic/lib/math/clc_remainder.cl?rev=327818&view=auto
==
--- libclc/trunk/generic/lib/math/clc_remainder.cl (added)
+++ libclc/trunk/generic/lib/math/clc_remainder.cl Sun Mar 18 18:01:10 2018
@@ -0,0 +1,218 @@
+/*
+ * Copyright (c) 2014 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include 
+
+#include 
+#include "../clcmacro.h"
+#include "config.h"
+#include "math.h"
+
+_CLC_DEF _CLC_OVERLOAD float __clc_remainder(float x, float y)
+{
+int ux = as_int(x);
+int ax = ux & EXSIGNBIT_SP32;
+float xa = as_float(ax);
+int sx = ux ^ ax;
+int ex = ax >> EXPSHIFTBITS_SP32;
+
+int uy = as_int(y);
+int ay = uy & EXSIGNBIT_SP32;
+float ya = as_float(ay);
+int ey = ay >> EXPSHIFTBITS_SP32;
+
+float xr = as_float(0x3f80 | (ax & 0x007f));
+float yr = as_float(0x3f80 | (ay & 0x007f));
+int c;
+int k = ex - ey;
+
+uint q = 0;
+
+while (k > 0) {
+c = xr >= yr;
+q = (q << 1) | c;
+xr -= c ? yr : 0.0f;
+xr += xr;
+   --k;
+}
+
+c = xr > yr;
+q = (q << 1) | c;
+xr -= c ? yr : 0.0f;
+
+

[PATCH] D44617: [Transforms] Keep TBAA tags in SimplifyCFG transformations

[Danil Malyshev via Phabricator via cfe-commits]

DanilM created this revision.
DanilM added reviewers: hfinkel, rjmccall, kosarev.
Herald added a subscriber: cfe-commits.

If SimplifyCFG attaches one block to another it removes all metadata from 
instructions of a block that should be attached because it can be depended on 
conditions that are changed during transformation.
In case of TBAA metadata, it looks too conservatively: data types of load/store 
instructions cannot be changed during such transformations even if the pointed 
data was changed.
So attached the patch saves TBAA tags from removing during SimplifyCFG 
transformations.


Repository:
  rC Clang

https://reviews.llvm.org/D44617

Files:
  lib/Transforms/Utils/SimplifyCFG.cpp
  test/Transforms/SimplifyCFG/basictest.ll
  test/Transforms/SimplifyCFG/fold-two-entry-phi-tbaa.ll
  test/Transforms/SimplifyCFG/speculate-tbaa.ll

Index: test/Transforms/SimplifyCFG/speculate-tbaa.ll
===
--- test/Transforms/SimplifyCFG/speculate-tbaa.ll
+++ test/Transforms/SimplifyCFG/speculate-tbaa.ll
@@ -0,0 +1,53 @@
+; RUN: opt < %s -S -simplifycfg | FileCheck %s
+
+; This test case was generated from speculate-tbaa.c:
+;
+; void foo(int *p1, int p2, int condition) {
+;   *p1 = 100;
+;   if (condition)
+; *p1 = p2;
+; }
+;
+; using
+;   clang -cc1 -triple x86_64-linux-gnu -new-struct-path-tbaa -emit-llvm -o - -O3 -disable-llvm-passes speculate-tbaa.c | opt -mem2reg -S
+
+; ModuleID = ''
+source_filename = "speculate-tbaa.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--linux-gnu"
+
+; Function Attrs: nounwind
+define void @foo(i32* %p1, i32 %p2, i32 %condition) #0 {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:store i32 100, i32* %p1, align 4, !tbaa [[TBAA_Tag:![0-9]+]]
+; CHECK-NEXT:%tobool = icmp ne i32 %condition, 0
+; CHECK-NEXT:%spec.store.select = select i1 %tobool, i32 %p2, i32 100
+; CHECK-NEXT:store i32 %spec.store.select, i32* %p1, align 4, !tbaa [[TBAA_Tag]]
+entry:
+  store i32 100, i32* %p1, align 4, !tbaa !2
+  %tobool = icmp ne i32 %condition, 0
+  br i1 %tobool, label %if.then, label %if.end
+
+if.then:  ; preds = %entry
+  store i32 %p2, i32* %p1, align 4, !tbaa !2
+  br label %if.end
+
+if.end:   ; preds = %if.then, %entry
+  ret void
+}
+
+attributes #0 = { nounwind "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.module.flags = !{!0}
+!llvm.ident = !{!1}
+
+; CHECK-DAG: [[TBAA_Tag]] = !{[[TBAA_Type:![0-9]+]], [[TBAA_Type]], i64 0, i64 4}
+; CHECK-DAG: [[TBAA_Type]] = !{!{{[0-9]+}}, i64 4, !"int"}
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{!"clang version 7.0.0 (trunk 326065)"}
+!2 = !{!3, !3, i64 0, i64 4}
+!3 = !{!4, i64 4, !"int"}
+!4 = !{!5, i64 1, !"omnipotent char"}
+!5 = !{!"Simple C/C++ TBAA"}
+
Index: test/Transforms/SimplifyCFG/fold-two-entry-phi-tbaa.ll
===
--- test/Transforms/SimplifyCFG/fold-two-entry-phi-tbaa.ll
+++ test/Transforms/SimplifyCFG/fold-two-entry-phi-tbaa.ll
@@ -0,0 +1,75 @@
+; RUN: opt < %s -S -simplifycfg | FileCheck %s
+
+; This test case was generated from fold-two-entry-phi-tbaa.c:
+;
+; int foo(int p1, int p2, int p3) {
+;   return p1 ? p1 : p2 ? p2 : p3;
+; }
+;
+; using
+;   clang -cc1 -triple x86_64-linux-gnu -new-struct-path-tbaa -emit-llvm -o - -O3 -disable-llvm-passes fold-two-entry-phi-tbaa.c
+
+; ModuleID = 'fold-two-entry-phi-tbaa.c'
+source_filename = "fold-two-entry-phi-tbaa.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--linux-gnu"
+
+; Function Attrs: nounwind
+define i32 @foo(i32 %p1, i32 %p2, i32 %p3) #0 {
+entry:
+  %p1.addr = alloca i32, align 4
+  %p2.addr = alloca i32, align 4
+  %p3.addr = alloca i32, align 4
+  store i32 %p1, i32* %p1.addr, align 4, !tbaa !2
+  store i32 %p2, i32* %p2.addr, align 4, !tbaa !2
+  store i32 %p3, i32* %p3.addr, align 4, !tbaa !2
+  %0 = load i32, i32* %p1.addr, align 4, !tbaa !2
+  %tobool = icmp ne i32 %0, 0
+  br i1 %tobool, label %cond.true, label %cond.false
+
+cond.true:; preds = %entry
+  %1 = load i32, i32* %p1.addr, align 4, !tbaa !2
+  br label %cond.end4
+
+; CHECK: cond.false:
+; CHECK-NEXT:  %2 = load i32, i32* %p2.addr, align 4, !tbaa [[TBAA_Tag:![0-9]+]]
+; CHECK-NEXT:  %tobool1 = icmp ne i32 %2, 0
+; CHECK-NEXT:  %3 = load i32, i32* %p2.addr, align 4, !tbaa [[TBAA_Tag]]
+; CHECK-NEXT:  %4 = load i32, i32* %p3.addr, align 4, !tbaa [[TBAA_Tag]]
+; CHECK-NEXT:  %cond = select i1 %tobool

[PATCH] D39050: Add index-while-building support to Clang

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added a comment.

In https://reviews.llvm.org/D39050#1040501, @akyrtzi wrote:

> > That would be good. How would one go about asking Clang to generate this 
> > extra information? Would a Clang Plugin be suitable for this?
>
> That's an interesting idea that we could explore, but I don't have much 
> experience with that mechanism to comment on.
>
> > Only the lexer was needed to get the end of the token
>
> Ok, that's interesting, not sure why Xcode is so fast to highlight, did you 
> reuse same SourceManager/Lexer/buffers for occurrences from same file ? We'd 
> definitely add the end-loc if we cannot come up with a mechanism to highlight 
> fast enough without it.


I don't think Xcode is quite fast, it's about 10 times slower (although I'm not 
sure it really finished) than when I use my branch that has the end-loc. I 
would try end-locs in Xcode if I could, to compare :) So I don't really know 
where the bottleneck is in Xcode. Comparing oranges to oranges, it's 4 times 
slower without end-locs compared to with end-locs on my branch. I does use the 
same SourceManager for the 46K references and I verified that it uses the same 
buffers, etc.
I'll put the numbers here again for readability.

> For my little benchmark, I used the LLVM/Clang/Clangd code base which I 
> queried for all references of "std" (the namespace) which is around 46K 
> references in the index.
> 
> With end-loc in index: 3.45s on average (20 samples)
>  With end-loc computed on the fly: 11.33s on average (20 samples)
>  I also tried with Xcode but without too much success: it took about 30 secs 
> to reach 45K results and then carried on for a long time and hung (although I 
> didn't try to leave it for hours to see if it finished).



>> I think it's useful to highlight something even when the name is not there. 
>> For example in "MyClass o1, o2;" it feels natural that o1 and o2 would get 
>> highlighted.
> 
> To clarify, even with implicit references the start loc points to something. 
> In this case the implicit references can have start locs for the o1 and o2 
> identifiers and the end result for the UI will be the same (o1 and o2 get 
> highlighted) even without having end-locs for all references.

It's the same but slower. IMO, the trade off is not great. It's entirely 
subjective but I think 4-10 times slower in order to save an integer per 
occurrence is not worth it from my point of view.

> Even without end-loc, the start loc could point to MyStringRef and you could 
> highlight it.

(Same here, it's doable but faster if already in the index.)

>> It does? I can only seem to do a textual search.
> 
> The example I tried is the following. If you could file a bug report for the 
> test case that did not work as you expected it would be much appreciated!

Sure, I'll give that a try and isolate it as much as I can. BTW, does it work 
for you on the LLVM code base?


https://reviews.llvm.org/D39050



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

[PATCH] D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai created this revision.
smeenai added reviewers: rnk, rsmith.

EmitInlinedInheritingCXXConstructorCall may result in a CallBaseDtor
cleanup being pushed. That cleanup would then be popped when the CGF's
CurCodeDecl no longer points to the method which triggered the cleanup,
leading to a failed cast. Create a new cleanup scope to ensure that the
cleanup gets popped while the CurCodeDecl still points to the method.
Fixes PR36748.


Repository:
  rC Clang

https://reviews.llvm.org/D44619

Files:
  lib/CodeGen/CGClass.cpp
  test/CodeGenCXX/inheriting-constructor-cleanup.cpp


Index: test/CodeGenCXX/inheriting-constructor-cleanup.cpp
===
--- /dev/null
+++ test/CodeGenCXX/inheriting-constructor-cleanup.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-linux -fexceptions -emit-llvm -o - %s | 
FileCheck %s
+
+// PR36748
+struct S {
+  ~S() {}
+};
+
+struct T {
+  T(int, ...) {}
+};
+
+struct U : public S, public T {
+  using T::T;
+};
+
+void f() {
+  U(0);
+}
+
+// ~S cleanup should be emitted rather than crashing
+// CHECK-LABEL: define void @_Z1fv
+// CHECK: call void @_ZN1SD2Ev
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2180,6 +2180,7 @@
   GlobalDecl GD(Ctor, CtorType);
   InlinedInheritingConstructorScope Scope(*this, GD);
   ApplyInlineDebugLocation DebugScope(*this, GD);
+  RunCleanupsScope CleanupScope(*this);
 
   // Save the arguments to be passed to the inherited constructor.
   CXXInheritedCtorInitExprArgs = Args;


Index: test/CodeGenCXX/inheriting-constructor-cleanup.cpp
===
--- /dev/null
+++ test/CodeGenCXX/inheriting-constructor-cleanup.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-linux -fexceptions -emit-llvm -o - %s | FileCheck %s
+
+// PR36748
+struct S {
+  ~S() {}
+};
+
+struct T {
+  T(int, ...) {}
+};
+
+struct U : public S, public T {
+  using T::T;
+};
+
+void f() {
+  U(0);
+}
+
+// ~S cleanup should be emitted rather than crashing
+// CHECK-LABEL: define void @_Z1fv
+// CHECK: call void @_ZN1SD2Ev
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2180,6 +2180,7 @@
   GlobalDecl GD(Ctor, CtorType);
   InlinedInheritingConstructorScope Scope(*this, GD);
   ApplyInlineDebugLocation DebugScope(*this, GD);
+  RunCleanupsScope CleanupScope(*this);
 
   // Save the arguments to be passed to the inherited constructor.
   CXXInheritedCtorInitExprArgs = Args;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Argh, this fixes my test case, but causes failures in some other ones. Back to 
the drawing board, I guess.


Repository:
  rC Clang

https://reviews.llvm.org/D44619



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