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

[gehry via Phabricator via cfe-commits]

Sockke added a comment.

@carlosgalvezp Thanks for your review! I committed it myself.


Repository:
  rG LLVM Github Monorepo

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

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

[gehry via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGce6de98b80ad: [clang-tidy] Fix 
`cppcoreguidelines-init-variables` for invalid vardecl (authored by Sockke).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138655

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -- -- 
-fno-delayed-template-parsing -fexceptions
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables -fix-errors %t 
-- -- -fno-delayed-template-parsing -fexceptions
 // CHECK-FIXES: {{^}}#include 
 
 // Ensure that function declarations are not changed.
@@ -124,3 +124,13 @@
   Fruit fruit;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable 'fruit' is not 
initialized [cppcoreguidelines-init-variables]
 }
+
+void test_clang_diagnostic_error() {
+  int a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' is not initialized 
[cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+
+  UnknownType b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: error: unknown type name 'UnknownType' 
[clang-diagnostic-error]
+  // CHECK-FIXES-NOT: {{^}}  UnknownType b = 0;{{$}}
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -59,6 +59,10 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  // Clang diagnostic error may cause the variable to be an invalid int vardecl
+  if (MatchedDecl->isInvalidDecl())
+return;
+
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -- -- -fno-delayed-template-parsing -fexceptions
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables -fix-errors %t -- -- -fno-delayed-template-parsing -fexceptions
 // CHECK-FIXES: {{^}}#include 
 
 // Ensure that function declarations are not changed.
@@ -124,3 +124,13 @@
   Fruit fruit;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable 'fruit' is not initialized [cppcoreguidelines-init-variables]
 }
+
+void test_clang_diagnostic_error() {
+  int a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+
+  UnknownType b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: error: unknown type name 'UnknownType' [clang-diagnostic-error]
+  // CHECK-FIXES-NOT: {{^}}  UnknownType b = 0;{{$}}
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -59,6 +59,10 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  // Clang diagnostic error may cause the variable to be an invalid int vardecl
+  if (MatchedDecl->isInvalidDecl())
+return;
+
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[gehry via Phabricator via cfe-commits]

Sockke updated this revision to Diff 495413.
Sockke added a comment.

Rebased.


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

https://reviews.llvm.org/D138655

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -- -- 
-fno-delayed-template-parsing -fexceptions
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables -fix-errors %t 
-- -- -fno-delayed-template-parsing -fexceptions
 // CHECK-FIXES: {{^}}#include 
 
 // Ensure that function declarations are not changed.
@@ -124,3 +124,13 @@
   Fruit fruit;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable 'fruit' is not 
initialized [cppcoreguidelines-init-variables]
 }
+
+void test_clang_diagnostic_error() {
+  int a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' is not initialized 
[cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+
+  UnknownType b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: error: unknown type name 'UnknownType' 
[clang-diagnostic-error]
+  // CHECK-FIXES-NOT: {{^}}  UnknownType b = 0;{{$}}
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -59,6 +59,10 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  // Clang diagnostic error may cause the variable to be an invalid int vardecl
+  if (MatchedDecl->isInvalidDecl())
+return;
+
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -- -- -fno-delayed-template-parsing -fexceptions
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables -fix-errors %t -- -- -fno-delayed-template-parsing -fexceptions
 // CHECK-FIXES: {{^}}#include 
 
 // Ensure that function declarations are not changed.
@@ -124,3 +124,13 @@
   Fruit fruit;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable 'fruit' is not initialized [cppcoreguidelines-init-variables]
 }
+
+void test_clang_diagnostic_error() {
+  int a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+
+  UnknownType b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: error: unknown type name 'UnknownType' [clang-diagnostic-error]
+  // CHECK-FIXES-NOT: {{^}}  UnknownType b = 0;{{$}}
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -59,6 +59,10 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  // Clang diagnostic error may cause the variable to be an invalid int vardecl
+  if (MatchedDecl->isInvalidDecl())
+return;
+
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

@Sockke ping, can you land this or should we do it for you? If so, please let 
us know what name and email we should use for attribution.


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

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

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

@Sockke do you need help landing the patch? If so, let me know user and email 
for attribution :)


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

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

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp accepted this revision.
carlosgalvezp added a comment.
This revision is now accepted and ready to land.

LGTM, thanks for the fix!


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

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

[gehry via Phabricator via cfe-commits]

Sockke added a comment.

Hi, Could anyone please review this diff?


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

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

[gehry via Phabricator via cfe-commits]

Sockke added a comment.

Friendly ping.


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

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

[gehry via Phabricator via cfe-commits]

Sockke added a comment.

Friendly ping.


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

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

[gehry via Phabricator via cfe-commits]

Sockke marked 6 inline comments as done.
Sockke added a comment.

Mark comments.


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

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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D138655#4012557 , @Sockke wrote:

> Improved the test file.

Just a general workflow comment, can you please mark comments as done when they 
are addressed, or, if you feel the comment in unjustified, explain why.


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

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

[gehry via Phabricator via cfe-commits]

Sockke updated this revision to Diff 484765.
Sockke added a comment.

Improved the test file.


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

https://reviews.llvm.org/D138655

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -- -- 
-fno-delayed-template-parsing -fexceptions
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables -fix-errors %t 
-- -- -fno-delayed-template-parsing -fexceptions
 // CHECK-FIXES: {{^}}#include 
 
 // Ensure that function declarations are not changed.
@@ -124,3 +124,13 @@
   Fruit fruit;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable 'fruit' is not 
initialized [cppcoreguidelines-init-variables]
 }
+
+void test_clang_diagnostic_error() {
+  int a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' is not initialized 
[cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+
+  UnknownType b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: error: unknown type name 'UnknownType' 
[clang-diagnostic-error]
+  // CHECK-FIXES-NOT: {{^}}  UnknownType b = 0;{{$}}
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -60,6 +60,10 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  // Clang diagnostic error may cause the variable to be an invalid int vardecl
+  if (MatchedDecl->isInvalidDecl())
+return;
+
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -- -- -fno-delayed-template-parsing -fexceptions
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables -fix-errors %t -- -- -fno-delayed-template-parsing -fexceptions
 // CHECK-FIXES: {{^}}#include 
 
 // Ensure that function declarations are not changed.
@@ -124,3 +124,13 @@
   Fruit fruit;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable 'fruit' is not initialized [cppcoreguidelines-init-variables]
 }
+
+void test_clang_diagnostic_error() {
+  int a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+
+  UnknownType b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: error: unknown type name 'UnknownType' [clang-diagnostic-error]
+  // CHECK-FIXES-NOT: {{^}}  UnknownType b = 0;{{$}}
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -60,6 +60,10 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  // Clang diagnostic error may cause the variable to be an invalid int vardecl
+  if (MatchedDecl->isInvalidDecl())
+return;
+
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In 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 
+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 arr;
+  // CHECK-FIXES-NOT: {{^}}  std::vector 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

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

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

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?


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

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

[gehry via Phabricator via cfe-commits]

Sockke updated this revision to Diff 484210.
Sockke added a comment.

Added comments


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

https://reviews.llvm.org/D138655

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
@@ -1,6 +1,16 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -- -- 
-fno-delayed-template-parsing -fexceptions
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables -fix-errors %t 
-- -- -fno-delayed-template-parsing -fexceptions
 // CHECK-FIXES: {{^}}#include 
 
+// 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 
+struct vector {
+  vector();
+};
+
 // Ensure that function declarations are not changed.
 void some_func(int x, double d, bool b, const char *p);
 
@@ -124,3 +134,13 @@
   Fruit fruit;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable 'fruit' is not 
initialized [cppcoreguidelines-init-variables]
 }
+
+void test_clang_diagnostic_error() {
+  int a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' is not initialized 
[cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+
+  // The stl object has been initialized
+  std::vector arr;
+  // CHECK-FIXES-NOT: {{^}}  std::vector arr = 0;{{$}}
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -60,6 +60,9 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  // Header file error causes the stl container variable to be an invalid int 
vardecl
+  if (MatchedDecl->isInvalidDecl())
+return;
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
@@ -1,6 +1,16 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -- -- -fno-delayed-template-parsing -fexceptions
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables -fix-errors %t -- -- -fno-delayed-template-parsing -fexceptions
 // CHECK-FIXES: {{^}}#include 
 
+// 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 
+struct vector {
+  vector();
+};
+
 // Ensure that function declarations are not changed.
 void some_func(int x, double d, bool b, const char *p);
 
@@ -124,3 +134,13 @@
   Fruit fruit;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable 'fruit' is not initialized [cppcoreguidelines-init-variables]
 }
+
+void test_clang_diagnostic_error() {
+  int a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+
+  // The stl object has been initialized
+  std::vector arr;
+  // CHECK-FIXES-NOT: {{^}}  std::vector arr = 0;{{$}}
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -60,6 +60,9 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  // Header file error causes the stl container variable to be an invalid int vardecl
+  if (MatchedDecl->isInvalidDecl())
+return;
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Looks good in general, IMHO it would be good with a couple more comments to 
make the code and test easier to understand.




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:63
 
+  if (MatchedDecl->isInvalidDecl())
+return;

Add a comment as to why this is needed? I haven't seen this pattern often in 
checks.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp:4
 
+#include "unknown.h"
+// CHECK-MESSAGES: :[[@LINE-1]]:10: error: 'unknown.h' file not found 
[clang-diagnostic-error]

Add a comment explaining the purpose of introducing this error? At first sight, 
it's not obvious to me why this is needed.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp:141
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+  std::vector arr;
+}

Perhaps add a comment explaining that clang-tidy should not warn here, and why.


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

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

[gehry via Phabricator via cfe-commits]

Sockke added a comment.

Friendly ping.


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

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

[gehry via Phabricator via cfe-commits]

Sockke added a comment.

Friendly ping.


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

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

[gehry via Phabricator via cfe-commits]

Sockke updated this revision to Diff 478529.
Sockke added a comment.

Added test in existing `init-variables.cpp` file.


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

https://reviews.llvm.org/D138655

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
@@ -1,6 +1,15 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -- -- 
-fno-delayed-template-parsing -fexceptions
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables -fix-errors %t 
-- -- -fno-delayed-template-parsing -fexceptions
 // CHECK-FIXES: {{^}}#include 
 
+#include "unknown.h"
+// CHECK-MESSAGES: :[[@LINE-1]]:10: error: 'unknown.h' file not found 
[clang-diagnostic-error]
+
+namespace std {
+template 
+struct vector {
+  vector();
+};
+
 // Ensure that function declarations are not changed.
 void some_func(int x, double d, bool b, const char *p);
 
@@ -124,3 +133,10 @@
   Fruit fruit;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable 'fruit' is not 
initialized [cppcoreguidelines-init-variables]
 }
+
+void test_clang_diagnostic_error() {
+  int a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' is not initialized 
[cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+  std::vector arr;
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -60,6 +60,8 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  if (MatchedDecl->isInvalidDecl())
+return;
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp
@@ -1,6 +1,15 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -- -- -fno-delayed-template-parsing -fexceptions
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables -fix-errors %t -- -- -fno-delayed-template-parsing -fexceptions
 // CHECK-FIXES: {{^}}#include 
 
+#include "unknown.h"
+// CHECK-MESSAGES: :[[@LINE-1]]:10: error: 'unknown.h' file not found [clang-diagnostic-error]
+
+namespace std {
+template 
+struct vector {
+  vector();
+};
+
 // Ensure that function declarations are not changed.
 void some_func(int x, double d, bool b, const char *p);
 
@@ -124,3 +133,10 @@
   Fruit fruit;
   // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable 'fruit' is not initialized [cppcoreguidelines-init-variables]
 }
+
+void test_clang_diagnostic_error() {
+  int a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+  std::vector arr;
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -60,6 +60,8 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  if (MatchedDecl->isInvalidDecl())
+return;
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

It'll be better to expand existing `init-variables.cpp` test.


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

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

[gehry via Phabricator via cfe-commits]

Sockke updated this revision to Diff 477861.
Sockke added a comment.

Added test.


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

https://reviews.llvm.org/D138655

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables-clang-diagnostic-error.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables-clang-diagnostic-error.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables-clang-diagnostic-error.cpp
@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -fix-errors --
+
+#include "unknown.h"
+// CHECK-MESSAGES: :[[@LINE-1]]:10: error: 'unknown.h' file not found 
[clang-diagnostic-error]
+
+namespace std {
+template 
+struct vector {
+  vector();
+};
+
+void test() {
+  int a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' is not initialized 
[cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+  std::vector arr;
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -60,6 +60,8 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  if (MatchedDecl->isInvalidDecl())
+return;
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables-clang-diagnostic-error.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables-clang-diagnostic-error.cpp
@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables %t -fix-errors --
+
+#include "unknown.h"
+// CHECK-MESSAGES: :[[@LINE-1]]:10: error: 'unknown.h' file not found [clang-diagnostic-error]
+
+namespace std {
+template 
+struct vector {
+  vector();
+};
+
+void test() {
+  int a;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'a' is not initialized [cppcoreguidelines-init-variables]
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+  std::vector arr;
+}
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -60,6 +60,8 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  if (MatchedDecl->isInvalidDecl())
+return;
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Will be good idea to add test for this situation.


Repository:
  rG LLVM Github Monorepo

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

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

[gehry via Phabricator via cfe-commits]

Sockke created this revision.
Sockke added a reviewer: aaron.ballman.
Herald added subscribers: carlosgalvezp, shchenz, kbarton, xazax.hun, nemanjai.
Herald added a reviewer: njames93.
Herald added a project: All.
Sockke requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

https://godbolt.org/z/n4cK4fo3o
The checker missed a check for invalid vardecl and this could cause a false 
positive.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138655

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp


Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -60,6 +60,8 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  if (MatchedDecl->isInvalidDecl())
+return;
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //


Index: clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
@@ -60,6 +60,8 @@
   const ASTContext  = *Result.Context;
   const SourceManager  = Context.getSourceManager();
 
+  if (MatchedDecl->isInvalidDecl())
+return;
   // We want to warn about cases where the type name
   // comes from a macro like this:
   //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits