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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
