[PATCH] D125137: [clang-format] Don't remove braces if a 1-statement body would wrap

2022-05-06 Thread Jeroen Van Antwerpen via Phabricator via cfe-commits
Jeroen added a comment.

Will it also add braces if they where not there yet?




Comment at: clang/unittests/Format/FormatTest.cpp:25374
   verifyFormat("if (a) {\n"
"  b = c >= 0 ? d\n"
" : e;\n"

Will it also add braces if they where not there yet?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125137/new/

https://reviews.llvm.org/D125137

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


[PATCH] D114317: [clang-tidy][WIP] Do not run perfect alias checks

2022-01-08 Thread Jeroen Van Antwerpen via Phabricator via cfe-commits
Jeroen added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:442
+if (PrimaryCheckValue != Value) {
+  std::cout << "Alias check \"" << CheckName.str() << "\" of \""
+<< PrimaryCheckName.str() << "\""

I'm not sure if you have this in for testing purpose or not, though I really 
like to have this message. If I would be having 2 aliases active with different 
options, it would be nice to know about that. So please keep this in!
Preferably, we would only configure it once, though that's dreaming.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114317/new/

https://reviews.llvm.org/D114317

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


[PATCH] D14484: [clang-format] Formatting constructor initializer lists by putting them always on different lines

2021-05-31 Thread Jeroen Van Antwerpen via Phabricator via cfe-commits
Jeroen added a comment.

In D14484#2788953 , @FStefanni wrote:

> Hi,
>
> in case of any doubt: yet it is of interest.
> 6 years old means only that it has not been "fixed" in the meanwhile...
>
> Please consider that there are not many code formatters for C++, since it is 
> quite a complex language (and a lot of different formatting conventions!).
> So we are just waiting to have integrated the formatting rules that we need.
>
> Regards.

Feel free to revive my patch, I'll be very happy to finally start using this.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D14484/new/

https://reviews.llvm.org/D14484

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


[PATCH] D87244: [clang] Add fix-it for -Wreorder-ctor.

2021-03-06 Thread Jeroen Van Antwerpen via Phabricator via cfe-commits
Jeroen added inline comments.



Comment at: clang/test/SemaCXX/warn-reorder-ctor-initialization.cpp:171
+  // code can't do this.
+  Bar(int x) : c(x), /*foo*/ a(2), b(3) {} // expected-warning {{field 'c' 
will be initialized after field 'a'}}
+  // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]

What would be the behavior for `:c(x), b(3), a(2)`?
Does that get fixed in 1 go, or should this be done in 2 steps?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87244/new/

https://reviews.llvm.org/D87244

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


[PATCH] D66040: [Sema] Implement DR2386 for C++17 structured binding

2019-08-10 Thread Jeroen Van Antwerpen via Phabricator via cfe-commits
Jeroen added a comment.

In the reproduction of https://bugs.llvm.org/show_bug.cgi?id=33236 there is 
explicit mentioning of `const T`. It would be nice if the test cases for this 
fix would also have coverage for that.




Comment at: clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp:15
 template<> struct std::tuple_size {};
-void no_tuple_size_3() { auto [x, y] = Bad1(); } // expected-error {{cannot 
decompose this type; 'std::tuple_size::value' is not a valid integral 
constant expression}}
+void no_tuple_size_3() { auto [x, y] = Bad1(); } // ok, omitting value is 
valid after DR2386
 

In the reproduction of https://bugs.llvm.org/show_bug.cgi?id=33236 there is 
explicit mentioning of `const T`. It would be nice if the test cases for this 
fix would also have coverage for that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66040/new/

https://reviews.llvm.org/D66040



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