Re: [PATCH] D18238: [clang-tidy] Fix clang-tidy crashes when using -fdelayed-template-parsing.

[Aaron Ballman via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Commit in r264049, thank you!


http://reviews.llvm.org/D18238



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18238: [clang-tidy] Fix clang-tidy crashes when using -fdelayed-template-parsing.

[Aaron Ballman via cfe-commits]

aaron.ballman added a subscriber: aaron.ballman.


Comment at: cppcoreguidelines/ProTypeMemberInitCheck.cpp:183
@@ +182,3 @@
+  // Skip delayed template instantiation declarations.
+  const auto *Body = Ctor->getBody();
+  if (!Body)

Please do not use auto here since the type isn't spelled out in the 
initializer. (The line directly above it also should not be using auto either.)


Comment at: modernize/RedundantVoidArgCheck.cpp:108
@@ -107,3 +107,3 @@
 SourceLocation End;
-if (Function->hasBody())
-  End = Function->getBody()->getLocStart().getLocWithOffset(-1);
+const auto *Body = Function->getBody();
+if (Body)

Don't use auto here either.


Comment at: modernize/RedundantVoidArgCheck.cpp:109
@@ +108,3 @@
+const auto *Body = Function->getBody();
+if (Body)
+  End = Body->getLocStart().getLocWithOffset(-1);

You could get rid of the if and else and just use it directly in 
removeVoidArgumentTokens().
```
removeVoidArgumentTokens(Result, SourceRange(Start, Body ? 
Body->getLocStart().getLocWithOffset(-1) : Function-getLocEnd()), "function 
definition");
```


http://reviews.llvm.org/D18238



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18238: [clang-tidy] Fix clang-tidy crashes when using -fdelayed-template-parsing.

[Etienne Bergeron via cfe-commits]

etienneb updated this revision to Diff 50940.
etienneb marked 3 inline comments as done.
etienneb added a comment.

fixed. thanks.


http://reviews.llvm.org/D18238

Files:
  cppcoreguidelines/ProTypeMemberInitCheck.cpp
  modernize/RedundantVoidArgCheck.cpp

Index: cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -179,6 +179,11 @@
   const auto *Ctor = Result.Nodes.getNodeAs("ctor");
   const auto  = Ctor->getParent()->fields();
 
+  // Skip delayed template instantiation declarations.
+  const Stmt *Body = Ctor->getBody();
+  if (!Body)
+return;
+
   SmallPtrSet FieldsToInit;
   fieldsRequiringInit(MemberFields, FieldsToInit);
   if (FieldsToInit.empty())
@@ -193,8 +198,8 @@
   continue;
 FieldsToInit.erase(Init->getMember());
   }
-  removeFieldsInitializedInBody(*Ctor->getBody(), *Result.Context,
-FieldsToInit);
+  removeFieldsInitializedInBody(*Body, *Result.Context, FieldsToInit);
+
   if (FieldsToInit.empty())
 return;
 
Index: modernize/RedundantVoidArgCheck.cpp
===
--- modernize/RedundantVoidArgCheck.cpp
+++ modernize/RedundantVoidArgCheck.cpp
@@ -104,13 +104,10 @@
 const MatchFinder::MatchResult , const FunctionDecl *Function) {
   SourceLocation Start = Function->getLocStart();
   if (Function->isThisDeclarationADefinition()) {
-SourceLocation End;
-if (Function->hasBody())
-  End = Function->getBody()->getLocStart().getLocWithOffset(-1);
-else
-  End = Function->getLocEnd();
-removeVoidArgumentTokens(Result, SourceRange(Start, End),
- "function definition");
+const Stmt *Body = Function->getBody();
+SourceRange range(Start, Body ? Body->getLocStart().getLocWithOffset(-1) :
+Function->getLocEnd());
+removeVoidArgumentTokens(Result, range, "function definition");
   } else {
 removeVoidArgumentTokens(Result, Function->getSourceRange(),
  "function declaration");


Index: cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -179,6 +179,11 @@
   const auto *Ctor = Result.Nodes.getNodeAs("ctor");
   const auto  = Ctor->getParent()->fields();
 
+  // Skip delayed template instantiation declarations.
+  const Stmt *Body = Ctor->getBody();
+  if (!Body)
+return;
+
   SmallPtrSet FieldsToInit;
   fieldsRequiringInit(MemberFields, FieldsToInit);
   if (FieldsToInit.empty())
@@ -193,8 +198,8 @@
   continue;
 FieldsToInit.erase(Init->getMember());
   }
-  removeFieldsInitializedInBody(*Ctor->getBody(), *Result.Context,
-FieldsToInit);
+  removeFieldsInitializedInBody(*Body, *Result.Context, FieldsToInit);
+
   if (FieldsToInit.empty())
 return;
 
Index: modernize/RedundantVoidArgCheck.cpp
===
--- modernize/RedundantVoidArgCheck.cpp
+++ modernize/RedundantVoidArgCheck.cpp
@@ -104,13 +104,10 @@
 const MatchFinder::MatchResult , const FunctionDecl *Function) {
   SourceLocation Start = Function->getLocStart();
   if (Function->isThisDeclarationADefinition()) {
-SourceLocation End;
-if (Function->hasBody())
-  End = Function->getBody()->getLocStart().getLocWithOffset(-1);
-else
-  End = Function->getLocEnd();
-removeVoidArgumentTokens(Result, SourceRange(Start, End),
- "function definition");
+const Stmt *Body = Function->getBody();
+SourceRange range(Start, Body ? Body->getLocStart().getLocWithOffset(-1) :
+Function->getLocEnd());
+removeVoidArgumentTokens(Result, range, "function definition");
   } else {
 removeVoidArgumentTokens(Result, Function->getSourceRange(),
  "function declaration");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D18238: [clang-tidy] Fix clang-tidy crashes when using -fdelayed-template-parsing.

[Etienne Bergeron via cfe-commits]

etienneb created this revision.
etienneb added reviewers: rnk, alexfh, emso, bkramer.
etienneb added a subscriber: cfe-commits.
Herald added a subscriber: aemerson.

Fix crashes caused by deferencing null pointer when declarations parsing may be 
delayed.

The body of the declarations may be null.

The crashes were observed on a Windows build.
  command-line switches: -fms-compatibility-version=19 -fms-compatibility
To reproduce them, run clang-tidy over the following basic file:

#include 
int main() {}


Templated functions that aren't used may contains a "null" body even if the 
decl->hasBody() is returning true. 


template  void f() {}


FunctionTemplateDecl 0xd06340  col:28 
f
|-TemplateTypeParmDecl 0xd061e8  col:20 typename T
`-FunctionDecl 0xd062d0  col:28 f 'void (void)'
  `-<<>>




http://reviews.llvm.org/D18238

Files:
  cppcoreguidelines/ProTypeMemberInitCheck.cpp
  modernize/RedundantVoidArgCheck.cpp

Index: cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -179,6 +179,11 @@
   const auto *Ctor = Result.Nodes.getNodeAs("ctor");
   const auto  = Ctor->getParent()->fields();
 
+  // Skip delayed template instantiation declarations.
+  const auto *Body = Ctor->getBody();
+  if (!Body)
+return;
+
   SmallPtrSet FieldsToInit;
   fieldsRequiringInit(MemberFields, FieldsToInit);
   if (FieldsToInit.empty())
@@ -193,7 +198,7 @@
   continue;
 FieldsToInit.erase(Init->getMember());
   }
-  removeFieldsInitializedInBody(*Ctor->getBody(), *Result.Context,
+  removeFieldsInitializedInBody(*Body, *Result.Context,
 FieldsToInit);
   if (FieldsToInit.empty())
 return;
Index: modernize/RedundantVoidArgCheck.cpp
===
--- modernize/RedundantVoidArgCheck.cpp
+++ modernize/RedundantVoidArgCheck.cpp
@@ -105,8 +105,9 @@
   SourceLocation Start = Function->getLocStart();
   if (Function->isThisDeclarationADefinition()) {
 SourceLocation End;
-if (Function->hasBody())
-  End = Function->getBody()->getLocStart().getLocWithOffset(-1);
+const auto *Body = Function->getBody();
+if (Body)
+  End = Body->getLocStart().getLocWithOffset(-1);
 else
   End = Function->getLocEnd();
 removeVoidArgumentTokens(Result, SourceRange(Start, End),


Index: cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -179,6 +179,11 @@
   const auto *Ctor = Result.Nodes.getNodeAs("ctor");
   const auto  = Ctor->getParent()->fields();
 
+  // Skip delayed template instantiation declarations.
+  const auto *Body = Ctor->getBody();
+  if (!Body)
+return;
+
   SmallPtrSet FieldsToInit;
   fieldsRequiringInit(MemberFields, FieldsToInit);
   if (FieldsToInit.empty())
@@ -193,7 +198,7 @@
   continue;
 FieldsToInit.erase(Init->getMember());
   }
-  removeFieldsInitializedInBody(*Ctor->getBody(), *Result.Context,
+  removeFieldsInitializedInBody(*Body, *Result.Context,
 FieldsToInit);
   if (FieldsToInit.empty())
 return;
Index: modernize/RedundantVoidArgCheck.cpp
===
--- modernize/RedundantVoidArgCheck.cpp
+++ modernize/RedundantVoidArgCheck.cpp
@@ -105,8 +105,9 @@
   SourceLocation Start = Function->getLocStart();
   if (Function->isThisDeclarationADefinition()) {
 SourceLocation End;
-if (Function->hasBody())
-  End = Function->getBody()->getLocStart().getLocWithOffset(-1);
+const auto *Body = Function->getBody();
+if (Body)
+  End = Body->getLocStart().getLocWithOffset(-1);
 else
   End = Function->getLocEnd();
 removeVoidArgumentTokens(Result, SourceRange(Start, End),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18238: [clang-tidy] Fix clang-tidy crashes when using -fdelayed-template-parsing.

[Reid Kleckner via cfe-commits]

rnk accepted this revision.
rnk added a comment.

lgtm, thanks!


http://reviews.llvm.org/D18238



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18238: [clang-tidy] Fix clang-tidy crashes when using -fdelayed-template-parsing.

[Richard via cfe-commits]

LegalizeAdulthood added a subscriber: LegalizeAdulthood.
LegalizeAdulthood added a comment.

LGTM


http://reviews.llvm.org/D18238



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18238: [clang-tidy] Fix clang-tidy crashes when using -fdelayed-template-parsing.

[Aaron Ballman via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.

Oops, I take that LGTM back. ;-) Almost LGTM -- this is missing test cases that 
exercise the behavior.


http://reviews.llvm.org/D18238



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18238: [clang-tidy] Fix clang-tidy crashes when using -fdelayed-template-parsing.

[Aaron Ballman via cfe-commits]

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

LGTM now (this time for sure), thank you!


http://reviews.llvm.org/D18238



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits