aaron.ballman added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:76
+ else if (TypePtr->isFloatingType()) {
+ InitializationString = " = NAN";
+ AddMathInclude = true;
----------------
jpakkane wrote:
> aaron.ballman wrote:
> > In C++11 mode, I think this should recommend
> > `std::numeric_limits<type>::quiet_NaN()` if possible, from `<limits>`
> > rather than `<math.h>`.
> As a stylistic point I would disagree with this. In this particular case the
> "C++ way" looks very verbose and confusing. For a single variable this is not
> a huge problem, but it gets very ugly when you have something like this
> (which is common in real world code bases):
>
>
> ```
> double d1, d2, d3, d4, d5, d6, d7;
> ```
>
> This just explodes and is very ugly and unpleasant to read when every one of
> these gets the `std::numeric_limits` template value. I would recommend using
> plain `NAN` for terseness and readability, but can update the PR if the
> template form is deemed the better choice.
Let's go with `NAN` for now. We can always add an option letting the user pick
which style to replace with later.
================
Comment at:
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:66
+ // not come from a macro expansion.
+ if (MatchedDecl->getEndLoc().isMacroID()) {
+ return;
----------------
Elide braces.
================
Comment at:
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst:33
+ char *txt = nullptr;
+ double d = (0.0/0.0);
+
----------------
This looks incorrect.
================
Comment at:
clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp:3
+
+#define DO_NOTHING(x) ((void)x)
+
----------------
You can drop this and its uses, we don't care about warnings produced by Clang
about the variables not being used.
================
Comment at:
clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp:36
+void init_unit_tests() {
+ int x;
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'x' is not initialized
[cppcoreguidelines-init-variables]
----------------
I'd like to see a test using a local static (which should not diagnose, as it's
zero-initialized by default) and a local extern (which should not diagnose
because it cannot have an initializer).
================
Comment at:
clang-tools-extra/test/clang-tidy/cppcoreguidelines-init-variables.cpp:55
+ // CHECK-FIXES: {{^}} int y0 = 0, y1 = 1, y2 = 0;{{$}}
+ int hasval = 42;
+
----------------
I'd like to see some tests for other initialization forms as well:
```
int braces{42};
int parens(42);
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64671/new/
https://reviews.llvm.org/D64671
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
