[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash

2017-10-25 Thread Alexander Kornienko via Phabricator via cfe-commits 
alexfh added inline comments.



Comment at: cfe/trunk/lib/Analysis/BodyFarm.cpp:366
+M.makeLvalueToRvalue(D->getParamDecl(i),
+ /* RefersToEnclosingVariableOrCapture= */ false));
 

george.karpenkov wrote:
> alexfh wrote:
> > Remove the spaces inside the comment. `/*ParamName=*/value` is the format 
> > that is understood by clang-format and the 
> > https://clang.llvm.org/extra/clang-tidy/checks/misc-argument-comment.html 
> > clang-tidy check. Same elsewhere in this patch.
> yeah, i can do that.
> can we also consider changing `clang-tidy` to ignore those spaces?
I've just looked at the source code, and, apparently, clang-tidy already 
understands spaces in argument comments already. Sorry for misinforming you. 
However, clang-format is more strict here and there's also the consistency 
argument (see the comment on r316539), which I find to be a good motivation to 
change this (or at least to not introduce more inconsistency).


Repository:
  rL LLVM

https://reviews.llvm.org/D39015



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

[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash

2017-10-24 Thread George Karpenkov via Phabricator via cfe-commits 
george.karpenkov added inline comments.



Comment at: cfe/trunk/lib/Analysis/BodyFarm.cpp:139
 
-DeclRefExpr *ASTMaker::makeDeclRefExpr(const VarDecl *D,
-   bool RefersToEnclosingVariableOrCapture,
-   bool GetNonReferenceType) {
-  auto Type = D->getType();
-  if (GetNonReferenceType)
-Type = Type.getNonReferenceType();
+DeclRefExpr *ASTMaker::makeDeclRefExpr(
+const VarDecl *D,

danielmarjamaki wrote:
> this looks strange, did clang-format do this?
yes i believe so.



Comment at: cfe/trunk/lib/Analysis/BodyFarm.cpp:366
+M.makeLvalueToRvalue(D->getParamDecl(i),
+ /* RefersToEnclosingVariableOrCapture= */ false));
 

alexfh wrote:
> Remove the spaces inside the comment. `/*ParamName=*/value` is the format 
> that is understood by clang-format and the 
> https://clang.llvm.org/extra/clang-tidy/checks/misc-argument-comment.html 
> clang-tidy check. Same elsewhere in this patch.
yeah, i can do that.
can we also consider changing `clang-tidy` to ignore those spaces?


Repository:
  rL LLVM

https://reviews.llvm.org/D39015



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

[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash

2017-10-24 Thread Alexander Kornienko via Phabricator via cfe-commits 
alexfh added inline comments.



Comment at: cfe/trunk/lib/Analysis/BodyFarm.cpp:366
+M.makeLvalueToRvalue(D->getParamDecl(i),
+ /* RefersToEnclosingVariableOrCapture= */ false));
 

Remove the spaces inside the comment. `/*ParamName=*/value` is the format that 
is understood by clang-format and the 
https://clang.llvm.org/extra/clang-tidy/checks/misc-argument-comment.html 
clang-tidy check. Same elsewhere in this patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D39015



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

[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash

2017-10-19 Thread Daniel Marjamäki via Phabricator via cfe-commits 
danielmarjamaki added inline comments.



Comment at: cfe/trunk/lib/Analysis/BodyFarm.cpp:139
 
-DeclRefExpr *ASTMaker::makeDeclRefExpr(const VarDecl *D,
-   bool RefersToEnclosingVariableOrCapture,
-   bool GetNonReferenceType) {
-  auto Type = D->getType();
-  if (GetNonReferenceType)
-Type = Type.getNonReferenceType();
+DeclRefExpr *ASTMaker::makeDeclRefExpr(
+const VarDecl *D,

this looks strange, did clang-format do this?


Repository:
  rL LLVM

https://reviews.llvm.org/D39015



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

[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash

2017-10-17 Thread George Karpenkov via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316041: [Analyzer] Always use non-reference types when 
creating expressions in BodyFarm. (authored by george.karpenkov).

Changed prior to commit:
  https://reviews.llvm.org/D39015?vs=119360=119392#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39015

Files:
  cfe/trunk/lib/Analysis/BodyFarm.cpp
  cfe/trunk/test/Analysis/call_once.cpp


Index: cfe/trunk/lib/Analysis/BodyFarm.cpp
===
--- cfe/trunk/lib/Analysis/BodyFarm.cpp
+++ cfe/trunk/lib/Analysis/BodyFarm.cpp
@@ -63,8 +63,7 @@
   
   /// Create a new DeclRefExpr for the referenced variable.
   DeclRefExpr *makeDeclRefExpr(const VarDecl *D,
-   bool RefersToEnclosingVariableOrCapture = false,
-   bool GetNonReferenceType = false);
+   bool RefersToEnclosingVariableOrCapture = 
false);
   
   /// Create a new UnaryOperator representing a dereference.
   UnaryOperator *makeDereference(const Expr *Arg, QualType Ty);
@@ -82,8 +81,7 @@
   /// DeclRefExpr in the process.
   ImplicitCastExpr *
   makeLvalueToRvalue(const VarDecl *Decl,
- bool RefersToEnclosingVariableOrCapture = false,
- bool GetNonReferenceType = false);
+ bool RefersToEnclosingVariableOrCapture = false);
 
   /// Create an implicit cast of the given type.
   ImplicitCastExpr *makeImplicitCast(const Expr *Arg, QualType Ty,
@@ -138,12 +136,10 @@
   return new (C) CompoundStmt(C, Stmts, SourceLocation(), SourceLocation());
 }
 
-DeclRefExpr *ASTMaker::makeDeclRefExpr(const VarDecl *D,
-   bool RefersToEnclosingVariableOrCapture,
-   bool GetNonReferenceType) {
-  auto Type = D->getType();
-  if (GetNonReferenceType)
-Type = Type.getNonReferenceType();
+DeclRefExpr *ASTMaker::makeDeclRefExpr(
+const VarDecl *D,
+bool RefersToEnclosingVariableOrCapture) {
+  QualType Type = D->getType().getNonReferenceType();
 
   DeclRefExpr *DR = DeclRefExpr::Create(
   C, NestedNameSpecifierLoc(), SourceLocation(), const_cast(D),
@@ -162,14 +158,10 @@
 
 ImplicitCastExpr *
 ASTMaker::makeLvalueToRvalue(const VarDecl *Arg,
- bool RefersToEnclosingVariableOrCapture,
- bool GetNonReferenceType) {
-  auto Type = Arg->getType();
-  if (GetNonReferenceType)
-Type = Type.getNonReferenceType();
+ bool RefersToEnclosingVariableOrCapture) {
+  QualType Type = Arg->getType().getNonReferenceType();
   return makeLvalueToRvalue(makeDeclRefExpr(Arg,
-RefersToEnclosingVariableOrCapture,
-GetNonReferenceType),
+
RefersToEnclosingVariableOrCapture),
 Type);
 }
 
@@ -365,12 +357,13 @@
 // Lambda requires callback itself inserted as a first parameter.
 CallArgs.push_back(
 M.makeDeclRefExpr(Callback,
-  /* RefersToEnclosingVariableOrCapture= */ true,
-  /* GetNonReferenceType= */ true));
+  /* RefersToEnclosingVariableOrCapture= */ true));
 
   // All arguments past first two ones are passed to the callback.
   for (unsigned int i = 2; i < D->getNumParams(); i++)
-CallArgs.push_back(M.makeLvalueToRvalue(D->getParamDecl(i)));
+CallArgs.push_back(
+M.makeLvalueToRvalue(D->getParamDecl(i),
+ /* RefersToEnclosingVariableOrCapture= */ false));
 
   CallExpr *CallbackCall;
   if (isLambdaCall) {
@@ -385,8 +378,7 @@
 
   DeclRefExpr *FlagDecl =
   M.makeDeclRefExpr(Flag,
-/* RefersToEnclosingVariableOrCapture=*/true,
-/* GetNonReferenceType=*/true);
+/* RefersToEnclosingVariableOrCapture=*/true);
 
 
   MemberExpr *Deref = M.makeMemberExpression(FlagDecl, FlagFieldDecl);
Index: cfe/trunk/test/Analysis/call_once.cpp
===
--- cfe/trunk/test/Analysis/call_once.cpp
+++ cfe/trunk/test/Analysis/call_once.cpp
@@ -17,7 +17,7 @@
 #endif
 
 template 
-void call_once(once_flag , Callable func, Args... args) {};
+void call_once(once_flag , Callable&& func, Args&&... args) {};
 
 } // namespace std
 


Index: cfe/trunk/lib/Analysis/BodyFarm.cpp
===
--- cfe/trunk/lib/Analysis/BodyFarm.cpp
+++ cfe/trunk/lib/Analysis/BodyFarm.cpp
@@ -63,8 +63,7 @@
   
   /// Create a new DeclRefExpr for the referenced variable.
   DeclRefExpr *makeDeclRefExpr(const VarDecl *D,
-   bool RefersToEnclosingVariableOrCapture = false,
-   bool

[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash

2017-10-17 Thread Devin Coughlin via Phabricator via cfe-commits 
dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

Looks good to me.

This comment is unrelated to this particular patch, but since I know you're 
doing other work in the area I think it would also be good to have test cases 
for the two other forms of call_once defined in . It looks to me like 
those will be used for projects that that use pre-C++11 -- so we should have 
test coverage for them.


https://reviews.llvm.org/D39015



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

[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash

2017-10-17 Thread George Karpenkov via Phabricator via cfe-commits 
george.karpenkov updated this revision to Diff 119360.

https://reviews.llvm.org/D39015

Files:
  lib/Analysis/BodyFarm.cpp
  test/Analysis/call_once.cpp


Index: test/Analysis/call_once.cpp
===
--- test/Analysis/call_once.cpp
+++ test/Analysis/call_once.cpp
@@ -17,7 +17,7 @@
 #endif
 
 template 
-void call_once(once_flag , Callable func, Args... args) {};
+void call_once(once_flag , Callable&& func, Args&&... args) {};
 
 } // namespace std
 
Index: lib/Analysis/BodyFarm.cpp
===
--- lib/Analysis/BodyFarm.cpp
+++ lib/Analysis/BodyFarm.cpp
@@ -63,8 +63,7 @@
   
   /// Create a new DeclRefExpr for the referenced variable.
   DeclRefExpr *makeDeclRefExpr(const VarDecl *D,
-   bool RefersToEnclosingVariableOrCapture = false,
-   bool GetNonReferenceType = false);
+   bool RefersToEnclosingVariableOrCapture = 
false);
   
   /// Create a new UnaryOperator representing a dereference.
   UnaryOperator *makeDereference(const Expr *Arg, QualType Ty);
@@ -82,8 +81,7 @@
   /// DeclRefExpr in the process.
   ImplicitCastExpr *
   makeLvalueToRvalue(const VarDecl *Decl,
- bool RefersToEnclosingVariableOrCapture = false,
- bool GetNonReferenceType = false);
+ bool RefersToEnclosingVariableOrCapture = false);
 
   /// Create an implicit cast of the given type.
   ImplicitCastExpr *makeImplicitCast(const Expr *Arg, QualType Ty,
@@ -138,12 +136,10 @@
   return new (C) CompoundStmt(C, Stmts, SourceLocation(), SourceLocation());
 }
 
-DeclRefExpr *ASTMaker::makeDeclRefExpr(const VarDecl *D,
-   bool RefersToEnclosingVariableOrCapture,
-   bool GetNonReferenceType) {
-  auto Type = D->getType();
-  if (GetNonReferenceType)
-Type = Type.getNonReferenceType();
+DeclRefExpr *ASTMaker::makeDeclRefExpr(
+const VarDecl *D,
+bool RefersToEnclosingVariableOrCapture) {
+  QualType Type = D->getType().getNonReferenceType();
 
   DeclRefExpr *DR = DeclRefExpr::Create(
   C, NestedNameSpecifierLoc(), SourceLocation(), const_cast(D),
@@ -162,14 +158,10 @@
 
 ImplicitCastExpr *
 ASTMaker::makeLvalueToRvalue(const VarDecl *Arg,
- bool RefersToEnclosingVariableOrCapture,
- bool GetNonReferenceType) {
-  auto Type = Arg->getType();
-  if (GetNonReferenceType)
-Type = Type.getNonReferenceType();
+ bool RefersToEnclosingVariableOrCapture) {
+  QualType Type = Arg->getType().getNonReferenceType();
   return makeLvalueToRvalue(makeDeclRefExpr(Arg,
-RefersToEnclosingVariableOrCapture,
-GetNonReferenceType),
+
RefersToEnclosingVariableOrCapture),
 Type);
 }
 
@@ -365,12 +357,13 @@
 // Lambda requires callback itself inserted as a first parameter.
 CallArgs.push_back(
 M.makeDeclRefExpr(Callback,
-  /* RefersToEnclosingVariableOrCapture= */ true,
-  /* GetNonReferenceType= */ true));
+  /* RefersToEnclosingVariableOrCapture= */ true));
 
   // All arguments past first two ones are passed to the callback.
   for (unsigned int i = 2; i < D->getNumParams(); i++)
-CallArgs.push_back(M.makeLvalueToRvalue(D->getParamDecl(i)));
+CallArgs.push_back(
+M.makeLvalueToRvalue(D->getParamDecl(i),
+ /* RefersToEnclosingVariableOrCapture= */ false));
 
   CallExpr *CallbackCall;
   if (isLambdaCall) {
@@ -385,8 +378,7 @@
 
   DeclRefExpr *FlagDecl =
   M.makeDeclRefExpr(Flag,
-/* RefersToEnclosingVariableOrCapture=*/true,
-/* GetNonReferenceType=*/true);
+/* RefersToEnclosingVariableOrCapture=*/true);
 
 
   MemberExpr *Deref = M.makeMemberExpression(FlagDecl, FlagFieldDecl);


Index: test/Analysis/call_once.cpp
===
--- test/Analysis/call_once.cpp
+++ test/Analysis/call_once.cpp
@@ -17,7 +17,7 @@
 #endif
 
 template 
-void call_once(once_flag , Callable func, Args... args) {};
+void call_once(once_flag , Callable&& func, Args&&... args) {};
 
 } // namespace std
 
Index: lib/Analysis/BodyFarm.cpp
===
--- lib/Analysis/BodyFarm.cpp
+++ lib/Analysis/BodyFarm.cpp
@@ -63,8 +63,7 @@
   
   /// Create a new DeclRefExpr for the referenced variable.
   DeclRefExpr *makeDeclRefExpr(const VarDecl *D,
-   bool RefersToEnclosingVariableOrCapture = false,
-   bool GetNonReferenceType = false);
+

[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash

2017-10-17 Thread George Karpenkov via Phabricator via cfe-commits 
george.karpenkov created this revision.
Herald added subscribers: szepet, kristof.beyls, xazax.hun, javed.absar, 
aemerson.

Remove an option to use a reference type (on by default!) since a non-reference 
type is always needed for creating expressions, functions with multiple boolean 
parameters are very hard to use, and in general it was just a booby trap for 
further crashes.
Furthermore, generalize `call_once` test case to fix some of the crashes 
mentioned https://bugs.llvm.org/show_bug.cgi?id=34869


https://reviews.llvm.org/D39015

Files:
  lib/Analysis/BodyFarm.cpp
  test/Analysis/call_once.cpp

Index: test/Analysis/call_once.cpp
===
--- test/Analysis/call_once.cpp
+++ test/Analysis/call_once.cpp
@@ -17,7 +17,7 @@
 #endif
 
 template 
-void call_once(once_flag , Callable func, Args... args) {};
+void call_once(once_flag , Callable&& func, Args&&... args) {};
 
 } // namespace std
 
Index: lib/Analysis/BodyFarm.cpp
===
--- lib/Analysis/BodyFarm.cpp
+++ lib/Analysis/BodyFarm.cpp
@@ -63,8 +63,7 @@
   
   /// Create a new DeclRefExpr for the referenced variable.
   DeclRefExpr *makeDeclRefExpr(const VarDecl *D,
-   bool RefersToEnclosingVariableOrCapture = false,
-   bool GetNonReferenceType = false);
+   bool RefersToEnclosingVariableOrCapture = false);
   
   /// Create a new UnaryOperator representing a dereference.
   UnaryOperator *makeDereference(const Expr *Arg, QualType Ty);
@@ -82,8 +81,7 @@
   /// DeclRefExpr in the process.
   ImplicitCastExpr *
   makeLvalueToRvalue(const VarDecl *Decl,
- bool RefersToEnclosingVariableOrCapture = false,
- bool GetNonReferenceType = false);
+ bool RefersToEnclosingVariableOrCapture = false);
 
   /// Create an implicit cast of the given type.
   ImplicitCastExpr *makeImplicitCast(const Expr *Arg, QualType Ty,
@@ -138,12 +136,10 @@
   return new (C) CompoundStmt(C, Stmts, SourceLocation(), SourceLocation());
 }
 
-DeclRefExpr *ASTMaker::makeDeclRefExpr(const VarDecl *D,
-   bool RefersToEnclosingVariableOrCapture,
-   bool GetNonReferenceType) {
-  auto Type = D->getType();
-  if (GetNonReferenceType)
-Type = Type.getNonReferenceType();
+DeclRefExpr *ASTMaker::makeDeclRefExpr(
+const VarDecl *D,
+bool RefersToEnclosingVariableOrCapture) {
+  QualType Type = D->getType().getNonReferenceType();
 
   DeclRefExpr *DR = DeclRefExpr::Create(
   C, NestedNameSpecifierLoc(), SourceLocation(), const_cast(D),
@@ -162,14 +158,10 @@
 
 ImplicitCastExpr *
 ASTMaker::makeLvalueToRvalue(const VarDecl *Arg,
- bool RefersToEnclosingVariableOrCapture,
- bool GetNonReferenceType) {
-  auto Type = Arg->getType();
-  if (GetNonReferenceType)
-Type = Type.getNonReferenceType();
+ bool RefersToEnclosingVariableOrCapture) {
+  QualType Type = Arg->getType().getNonReferenceType();
   return makeLvalueToRvalue(makeDeclRefExpr(Arg,
-RefersToEnclosingVariableOrCapture,
-GetNonReferenceType),
+RefersToEnclosingVariableOrCapture),
 Type);
 }
 
@@ -263,7 +255,7 @@
 
   return new (C) CallExpr(
   /*ASTContext=*/C,
-  /*StmtClass=*/M.makeLvalueToRvalue(/*Expr=*/Callback),
+  /*StmtClass=*/M.makeLvalueToRvalue(/*Expr=*/Callback), // TODO: this is a crashling line w/ reference type
   /*args=*/CallArgs,
   /*QualType=*/C.VoidTy,
   /*ExprValueType=*/VK_RValue,
@@ -365,12 +357,13 @@
 // Lambda requires callback itself inserted as a first parameter.
 CallArgs.push_back(
 M.makeDeclRefExpr(Callback,
-  /* RefersToEnclosingVariableOrCapture= */ true,
-  /* GetNonReferenceType= */ true));
+  /* RefersToEnclosingVariableOrCapture= */ true));
 
   // All arguments past first two ones are passed to the callback.
   for (unsigned int i = 2; i < D->getNumParams(); i++)
-CallArgs.push_back(M.makeLvalueToRvalue(D->getParamDecl(i)));
+CallArgs.push_back(
+M.makeLvalueToRvalue(D->getParamDecl(i),
+ /* RefersToEnclosingVariableOrCapture= */ false));
 
   CallExpr *CallbackCall;
   if (isLambdaCall) {
@@ -385,8 +378,7 @@
 
   DeclRefExpr *FlagDecl =
   M.makeDeclRefExpr(Flag,
-/* RefersToEnclosingVariableOrCapture=*/true,
-/* GetNonReferenceType=*/true);
+/* RefersToEnclosingVariableOrCapture=*/true);
 
 
   MemberExpr *Deref = M.makeMemberExpression(FlagDecl, FlagFieldDecl);