https://github.com/PiotrZSL commented:

Release notes entry missing.

Now, to be honest I do not like idea behind those changes.
Simply because if diagnostic is raised for example for an surrogate type, there 
is no way to apply fixes, and as this check is "modernize", main purpose of it 
is to apply fixes.

Personally I run into situation in my code base where this check is too 
aggressive and should be relaxed.

Example:
```
#include <type_traits>

template <typename T, bool = std::is_same_v<int, T>> void f(T &&value) {}

void test() {
  int t[10];
  f(t);
}
```
```
/root/1.cpp:3:50: warning: do not declare C-style arrays, use 'std::array' 
instead 
[cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays]
    3 | template <typename T, bool = std::is_same_v<int, T>> void f(T &&value) 
{}
      |                                                  ^
/root/1.cpp:6:3: warning: do not declare C-style arrays, use 'std::array' 
instead 
[cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays]
    6 |   int t[10];
      |   ^
```

In above code check is executed in template instance even that 
TK_IgnoreUnlessSpelledInSource is set. With your change it will be even worst.

Thing is that if check would properly handled implicit code then "template 
parameters" would never be catch. At the end check should point places in code 
where actually arrays are defined, otherwise if you would put array into type 
alias, then you would get warning in every place that type alias is used, and 
that's stupid as there is only one place where such array can be fixed, and 
that is a definition of that type alias.

Due to above I do not like this change.

https://github.com/llvm/llvm-project/pull/131468
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to