[PATCH] D138655: [clang-tidy] Fix `cppcoreguidelines-init-variables` for invalid vardecl

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 21 12:38:25 PST 2022


njames93 added a comment.

In D138655#4007822 <https://reviews.llvm.org/D138655#4007822>, @carlosgalvezp wrote:

> Now that I think a bit better about this I wonder - does it really make sense that we increase the complexity of the check to cover for cases where code does not compile? If it fails to include a header, there's many other things that can possibly go wrong - should clang-tidy checks in general really be defensive against that? @njames93 WDYT?

It's not a hard rule that we should be defensive against clang-tidy emitting false positives due to a previous compilation error. However for simple cases, like this, where its just ensuring a declaration is valid before we try to emit diagnostic, It does make sense to handle this.
With this change the main thing would be a little less noise while trying to debug the cause of the compilation error which is always a good thing.



================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp:4-13
+// Header file error causes stl container variable to be invalid int vardecl
+#include "unknown.h" 
+// CHECK-MESSAGES: :[[@LINE-1]]:10: error: 'unknown.h' file not found [clang-diagnostic-error]
+
+namespace std {
+template <typename T>
+struct vector {
----------------
All this is unnecessary there is a much nicer way to test this, see below


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp:139-141
+  int a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
----------------
This test line is not testing the behaviour described in this patch and can be removed.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp:144
+  // The stl object has been initialized
+  std::vector<int> arr;
+  // CHECK-FIXES-NOT: {{^}}  std::vector<int> arr = 0;{{$}}
----------------
Just use this and the declaration will be invalid and you can test the expected behaviour


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

https://reviews.llvm.org/D138655



More information about the cfe-commits mailing list