[PATCH] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

void created this revision.
void added a reviewer: rsmith.
void added a project: clang.
Herald added a subscriber: cfe-commits.

A ConstantExpr class represents a full expression that's in a
context where a constant expression is required. This class reflects
the path the evaluator took to reach the expression rather than
the syntactic context in which the expression occurs.

In the future, the class will be expanded to cache the result of
the evaluated expression so that it's not needlessly re-evaluated.


Repository:
  rC Clang

https://reviews.llvm.org/D53475

Files:
  include/clang/AST/Expr.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/StmtDataCollectors.td
  include/clang/Basic/StmtNodes.td
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/Expr.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  tools/libclang/CXCursor.cpp

Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -343,6 +343,10 @@
 K = CXCursor_CharacterLiteral;
 break;
 
+  case Stmt::ConstantExprClass:
+return MakeCXCursor(cast(S)->getSubExpr(),
+Parent, TU, RegionOfInterest);
+
   case Stmt::ParenExprClass:
 K = CXCursor_ParenExpr;
 break;
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1267,6 +1267,9 @@
 case Stmt::ObjCPropertyRefExprClass:
   llvm_unreachable("These are handled by PseudoObjectExpr");
 
+case Expr::ConstantExprClass:
+  return Visit(cast(S)->getSubExpr(), Pred, DstTop);
+
 case Stmt::GNUNullExprClass: {
   // GNU __null is a pointer-width integer, not an actual pointer.
   ProgramStateRef state = Pred->getState();
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -386,6 +386,12 @@
   Record.push_back(E->getObjectKind());
 }
 
+void ASTStmtWriter::VisitConstantExpr(ConstantExpr *E) {
+  VisitExpr(E);
+  Record.AddStmt(E->getSubExpr());
+  Code = serialization::EXPR_CONSTANT;
+}
+
 void ASTStmtWriter::VisitPredefinedExpr(PredefinedExpr *E) {
   VisitExpr(E);
   Record.AddSourceLocation(E->getLocation());
Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -491,6 +491,11 @@
  "Incorrect expression field count");
 }
 
+void ASTStmtReader::VisitConstantExpr(ConstantExpr *E) {
+  VisitExpr(E);
+  E->setSubExpr(Record.readSubExpr());
+}
+
 void ASTStmtReader::VisitPredefinedExpr(PredefinedExpr *E) {
   VisitExpr(E);
   E->setLocation(ReadSourceLocation());
@@ -2335,6 +2340,10 @@
Record[ASTStmtReader::NumStmtFields]);
   break;
 
+case EXPR_CONSTANT:
+  S = new (Context) ConstantExpr(Empty);
+  break;
+
 case EXPR_PREDEFINED:
   S = new (Context) PredefinedExpr(Empty);
   break;
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -8914,6 +8914,12 @@
 //===--===//
 // Expression transformation
 //===--===//
+template
+ExprResult
+TreeTransform::TransformConstantExpr(ConstantExpr *E) {
+  return TransformExpr(E->getSubExpr());
+}
+
 template
 ExprResult
 TreeTransform::TransformPredefinedExpr(PredefinedExpr *E) {
Index: lib/Sema/SemaExceptionSpec.cpp
===
--- lib/Sema/SemaExceptionSpec.cpp
+++ lib/Sema/SemaExceptionSpec.cpp
@@ -1051,6 +1051,9 @@
   //   [Can throw] if in a potentially-evaluated context the expression would
   //   contain:
   switch (E->getStmtClass()) {
+  case Expr::ConstantExprClass:
+return canThrow(cast(E)->getSubExpr());
+
   case Expr::CXXThrowExprClass:
 //   - a potentially evaluated throw-expression
 return CT_Can;
Index: lib/AST/StmtProfile.cpp
===
--- lib/AST/StmtProfile.cpp
+++ lib/AST/StmtProfile.cpp
@@ -996,6 +996,11 @@
   VisitStmt(S);
 }
 
+void StmtProfiler::VisitConstantExpr(const ConstantExpr *S) {
+  VisitExpr(S);
+  VisitExpr(S->getSubExpr());
+}
+
 void StmtProfiler::VisitDeclRefExpr(const DeclRefExpr *S) {
   VisitExpr(S);
   

[PATCH] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 170321.

Repository:
  rC Clang

https://reviews.llvm.org/D53475

Files:
  include/clang/AST/Expr.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/StmtDataCollectors.td
  include/clang/Basic/StmtNodes.td
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/Expr.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  tools/libclang/CXCursor.cpp

Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -343,6 +343,10 @@
 K = CXCursor_CharacterLiteral;
 break;
 
+  case Stmt::ConstantExprClass:
+return MakeCXCursor(cast(S)->getSubExpr(),
+Parent, TU, RegionOfInterest);
+
   case Stmt::ParenExprClass:
 K = CXCursor_ParenExpr;
 break;
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1267,6 +1267,9 @@
 case Stmt::ObjCPropertyRefExprClass:
   llvm_unreachable("These are handled by PseudoObjectExpr");
 
+case Expr::ConstantExprClass:
+  return Visit(cast(S)->getSubExpr(), Pred, DstTop);
+
 case Stmt::GNUNullExprClass: {
   // GNU __null is a pointer-width integer, not an actual pointer.
   ProgramStateRef state = Pred->getState();
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -386,6 +386,12 @@
   Record.push_back(E->getObjectKind());
 }
 
+void ASTStmtWriter::VisitConstantExpr(ConstantExpr *E) {
+  VisitExpr(E);
+  Record.AddStmt(E->getSubExpr());
+  Code = serialization::EXPR_CONSTANT;
+}
+
 void ASTStmtWriter::VisitPredefinedExpr(PredefinedExpr *E) {
   VisitExpr(E);
   Record.AddSourceLocation(E->getLocation());
Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -491,6 +491,11 @@
  "Incorrect expression field count");
 }
 
+void ASTStmtReader::VisitConstantExpr(ConstantExpr *E) {
+  VisitExpr(E);
+  E->setSubExpr(Record.readSubExpr());
+}
+
 void ASTStmtReader::VisitPredefinedExpr(PredefinedExpr *E) {
   VisitExpr(E);
   E->setLocation(ReadSourceLocation());
@@ -2335,6 +2340,10 @@
Record[ASTStmtReader::NumStmtFields]);
   break;
 
+case EXPR_CONSTANT:
+  S = new (Context) ConstantExpr(Empty);
+  break;
+
 case EXPR_PREDEFINED:
   S = new (Context) PredefinedExpr(Empty);
   break;
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -8914,6 +8914,12 @@
 //===--===//
 // Expression transformation
 //===--===//
+template
+ExprResult
+TreeTransform::TransformConstantExpr(ConstantExpr *E) {
+  return TransformExpr(E->getSubExpr());
+}
+
 template
 ExprResult
 TreeTransform::TransformPredefinedExpr(PredefinedExpr *E) {
Index: lib/Sema/SemaExceptionSpec.cpp
===
--- lib/Sema/SemaExceptionSpec.cpp
+++ lib/Sema/SemaExceptionSpec.cpp
@@ -1051,6 +1051,9 @@
   //   [Can throw] if in a potentially-evaluated context the expression would
   //   contain:
   switch (E->getStmtClass()) {
+  case Expr::ConstantExprClass:
+return canThrow(cast(E)->getSubExpr());
+
   case Expr::CXXThrowExprClass:
 //   - a potentially evaluated throw-expression
 return CT_Can;
Index: lib/AST/StmtProfile.cpp
===
--- lib/AST/StmtProfile.cpp
+++ lib/AST/StmtProfile.cpp
@@ -996,6 +996,11 @@
   VisitStmt(S);
 }
 
+void StmtProfiler::VisitConstantExpr(const ConstantExpr *S) {
+  VisitExpr(S);
+  VisitExpr(S->getSubExpr());
+}
+
 void StmtProfiler::VisitDeclRefExpr(const DeclRefExpr *S) {
   VisitExpr(S);
   if (!Canonical)
Index: lib/AST/StmtPrinter.cpp
===
--- lib/AST/StmtPrinter.cpp
+++ lib/AST/StmtPrinter.cpp
@@ -906,6 +906,10 @@
 //  Expr printing methods.
 //===--===//
 
+void StmtPrinter::VisitConstantExpr(ConstantExpr *Node) {
+  VisitExpr(Node->getSubExpr());
+}
+
 void StmtPrinter::VisitDeclRefExpr(DeclRefExpr *Node) {
   if (con

[PATCH] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 170324.
void marked an inline comment as done.
void added a comment.

Create a "FullExpression" parent class and skip full expressions in all places 
we skip with ExprWithCleanups.


Repository:
  rC Clang

https://reviews.llvm.org/D53475

Files:
  include/clang/AST/Expr.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/StmtDataCollectors.td
  include/clang/Basic/StmtNodes.td
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/Expr.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  tools/libclang/CXCursor.cpp

Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -343,6 +343,10 @@
 K = CXCursor_CharacterLiteral;
 break;
 
+  case Stmt::ConstantExprClass:
+return MakeCXCursor(cast(S)->getSubExpr(),
+Parent, TU, RegionOfInterest);
+
   case Stmt::ParenExprClass:
 K = CXCursor_ParenExpr;
 break;
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1267,6 +1267,9 @@
 case Stmt::ObjCPropertyRefExprClass:
   llvm_unreachable("These are handled by PseudoObjectExpr");
 
+case Expr::ConstantExprClass:
+  return Visit(cast(S)->getSubExpr(), Pred, DstTop);
+
 case Stmt::GNUNullExprClass: {
   // GNU __null is a pointer-width integer, not an actual pointer.
   ProgramStateRef state = Pred->getState();
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -386,6 +386,12 @@
   Record.push_back(E->getObjectKind());
 }
 
+void ASTStmtWriter::VisitConstantExpr(ConstantExpr *E) {
+  VisitExpr(E);
+  Record.AddStmt(E->getSubExpr());
+  Code = serialization::EXPR_CONSTANT;
+}
+
 void ASTStmtWriter::VisitPredefinedExpr(PredefinedExpr *E) {
   VisitExpr(E);
   Record.AddSourceLocation(E->getLocation());
Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -491,6 +491,11 @@
  "Incorrect expression field count");
 }
 
+void ASTStmtReader::VisitConstantExpr(ConstantExpr *E) {
+  VisitExpr(E);
+  E->setSubExpr(Record.readSubExpr());
+}
+
 void ASTStmtReader::VisitPredefinedExpr(PredefinedExpr *E) {
   VisitExpr(E);
   E->setLocation(ReadSourceLocation());
@@ -2335,6 +2340,10 @@
Record[ASTStmtReader::NumStmtFields]);
   break;
 
+case EXPR_CONSTANT:
+  S = new (Context) ConstantExpr(Empty);
+  break;
+
 case EXPR_PREDEFINED:
   S = new (Context) PredefinedExpr(Empty);
   break;
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -8914,6 +8914,12 @@
 //===--===//
 // Expression transformation
 //===--===//
+template
+ExprResult
+TreeTransform::TransformConstantExpr(ConstantExpr *E) {
+  return TransformExpr(E->getSubExpr());
+}
+
 template
 ExprResult
 TreeTransform::TransformPredefinedExpr(PredefinedExpr *E) {
Index: lib/Sema/SemaExceptionSpec.cpp
===
--- lib/Sema/SemaExceptionSpec.cpp
+++ lib/Sema/SemaExceptionSpec.cpp
@@ -1051,6 +1051,9 @@
   //   [Can throw] if in a potentially-evaluated context the expression would
   //   contain:
   switch (E->getStmtClass()) {
+  case Expr::ConstantExprClass:
+return canThrow(cast(E)->getSubExpr());
+
   case Expr::CXXThrowExprClass:
 //   - a potentially evaluated throw-expression
 return CT_Can;
Index: lib/AST/StmtProfile.cpp
===
--- lib/AST/StmtProfile.cpp
+++ lib/AST/StmtProfile.cpp
@@ -996,6 +996,11 @@
   VisitStmt(S);
 }
 
+void StmtProfiler::VisitConstantExpr(const ConstantExpr *S) {
+  VisitExpr(S);
+  VisitExpr(S->getSubExpr());
+}
+
 void StmtProfiler::VisitDeclRefExpr(const DeclRefExpr *S) {
   VisitExpr(S);
   if (!Canonical)
Index: lib/AST/StmtPrinter.cpp
===
--- lib/AST/StmtPrinter.cpp
+++ lib/AST/StmtPrinter.cpp
@@ -906,6 +906,10 @@
 //  Expr printing methods.
 //===-

[PATCH] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 170325.

Repository:
  rC Clang

https://reviews.llvm.org/D53475

Files:
  include/clang/AST/Expr.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/StmtDataCollectors.td
  include/clang/Basic/StmtNodes.td
  include/clang/Serialization/ASTBitCodes.h
  lib/ARCMigrate/TransAutoreleasePool.cpp
  lib/ARCMigrate/TransRetainReleaseDealloc.cpp
  lib/ARCMigrate/TransUnbridgedCasts.cpp
  lib/ARCMigrate/Transforms.cpp
  lib/AST/Decl.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprCXX.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/ParentMap.cpp
  lib/AST/Stmt.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/Analysis/LiveVariables.cpp
  lib/Analysis/ThreadSafety.cpp
  lib/Analysis/ThreadSafetyCommon.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGStmt.cpp
  lib/CodeGen/CGStmtOpenMP.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  tools/libclang/CXCursor.cpp

Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -343,6 +343,10 @@
 K = CXCursor_CharacterLiteral;
 break;
 
+  case Stmt::ConstantExprClass:
+return MakeCXCursor(cast(S)->getSubExpr(),
+Parent, TU, RegionOfInterest);
+
   case Stmt::ParenExprClass:
 K = CXCursor_ParenExpr;
 break;
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1267,6 +1267,9 @@
 case Stmt::ObjCPropertyRefExprClass:
   llvm_unreachable("These are handled by PseudoObjectExpr");
 
+case Expr::ConstantExprClass:
+  return Visit(cast(S)->getSubExpr(), Pred, DstTop);
+
 case Stmt::GNUNullExprClass: {
   // GNU __null is a pointer-width integer, not an actual pointer.
   ProgramStateRef state = Pred->getState();
Index: lib/StaticAnalyzer/Core/Environment.cpp
===
--- lib/StaticAnalyzer/Core/Environment.cpp
+++ lib/StaticAnalyzer/Core/Environment.cpp
@@ -44,6 +44,9 @@
   case Stmt::ExprWithCleanupsClass:
 E = cast(E)->getSubExpr();
 break;
+  case Stmt::ConstantExprClass:
+E = cast(E)->getSubExpr();
+break;
   case Stmt::CXXBindTemporaryExprClass:
 E = cast(E)->getSubExpr();
 break;
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -142,8 +142,8 @@
   E = AE->getBase();
 } else if (const auto *PE = dyn_cast(E)) {
   E = PE->getSubExpr();
-} else if (const auto *EWC = dyn_cast(E)) {
-  E = EWC->getSubExpr();
+} else if (const auto *FE = dyn_cast(E)) {
+  E = FE->getSubExpr();
 } else {
   // Other arbitrary stuff.
   break;
@@ -1515,8 +1515,8 @@
 static const Expr *peelOffOuterExpr(const Expr *Ex,
 const ExplodedNode *N) {
   Ex = Ex->IgnoreParenCasts();
-  if (const auto *EWC = dyn_cast(Ex))
-return peelOffOuterExpr(EWC->getSubExpr(), N);
+  if (const auto *FE = dyn_cast(Ex))
+return peelOffOuterExpr(FE->getSubExpr(), N);
   if (const auto *OVE = dyn_cast(Ex))
 return peelOffOuterExpr(OVE->getSourceExpr(), N);
   if (const auto *POE = dyn_cast(Ex)) {
Index: lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- lib/StaticAnalyzer/Core/BugReporter.cpp
+++ lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1265,7 +1265,7 @@
 if (!S)
   break;
 
-if (isa(S) ||
+if (isa(S) ||
 isa(S) ||
 isa(S))
   continue;
Index: lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -329,9 +329,8 @@
 return;
 
   if (const Expr *E = V->getInit()) {
-while (const ExprWithCleanups *exprClean =
-dyn_cast(E))
-  E = exprClean->getSubExpr();
+while (const FullExpression *FE = dyn_cast(E))
+  E = FE->getSubExpr();
 
 // Look through transitive assignments, e.g.:
 // int x = y = 0;
Index: lib/Serial

[PATCH] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 171417.
void marked an inline comment as done.

Repository:
  rC Clang

https://reviews.llvm.org/D53475

Files:
  include/clang/AST/Expr.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/StmtDataCollectors.td
  include/clang/Basic/StmtNodes.td
  include/clang/Serialization/ASTBitCodes.h
  lib/ARCMigrate/TransAutoreleasePool.cpp
  lib/ARCMigrate/TransRetainReleaseDealloc.cpp
  lib/ARCMigrate/TransUnbridgedCasts.cpp
  lib/ARCMigrate/Transforms.cpp
  lib/AST/Decl.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprCXX.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/ParentMap.cpp
  lib/AST/Stmt.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/Analysis/LiveVariables.cpp
  lib/Analysis/ThreadSafety.cpp
  lib/Analysis/ThreadSafetyCommon.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGStmt.cpp
  lib/CodeGen/CGStmtOpenMP.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  tools/libclang/CXCursor.cpp

Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -343,6 +343,10 @@
 K = CXCursor_CharacterLiteral;
 break;
 
+  case Stmt::ConstantExprClass:
+return MakeCXCursor(cast(S)->getSubExpr(),
+Parent, TU, RegionOfInterest);
+
   case Stmt::ParenExprClass:
 K = CXCursor_ParenExpr;
 break;
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1267,6 +1267,9 @@
 case Stmt::ObjCPropertyRefExprClass:
   llvm_unreachable("These are handled by PseudoObjectExpr");
 
+case Expr::ConstantExprClass:
+  return Visit(cast(S)->getSubExpr(), Pred, DstTop);
+
 case Stmt::GNUNullExprClass: {
   // GNU __null is a pointer-width integer, not an actual pointer.
   ProgramStateRef state = Pred->getState();
Index: lib/StaticAnalyzer/Core/Environment.cpp
===
--- lib/StaticAnalyzer/Core/Environment.cpp
+++ lib/StaticAnalyzer/Core/Environment.cpp
@@ -44,6 +44,9 @@
   case Stmt::ExprWithCleanupsClass:
 E = cast(E)->getSubExpr();
 break;
+  case Stmt::ConstantExprClass:
+E = cast(E)->getSubExpr();
+break;
   case Stmt::CXXBindTemporaryExprClass:
 E = cast(E)->getSubExpr();
 break;
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -136,8 +136,8 @@
   E = AE->getBase();
 } else if (const auto *PE = dyn_cast(E)) {
   E = PE->getSubExpr();
-} else if (const auto *EWC = dyn_cast(E)) {
-  E = EWC->getSubExpr();
+} else if (const auto *FE = dyn_cast(E)) {
+  E = FE->getSubExpr();
 } else {
   // Other arbitrary stuff.
   break;
@@ -1494,8 +1494,8 @@
 static const Expr *peelOffOuterExpr(const Expr *Ex,
 const ExplodedNode *N) {
   Ex = Ex->IgnoreParenCasts();
-  if (const auto *EWC = dyn_cast(Ex))
-return peelOffOuterExpr(EWC->getSubExpr(), N);
+  if (const auto *FE = dyn_cast(Ex))
+return peelOffOuterExpr(FE->getSubExpr(), N);
   if (const auto *OVE = dyn_cast(Ex))
 return peelOffOuterExpr(OVE->getSourceExpr(), N);
   if (const auto *POE = dyn_cast(Ex)) {
Index: lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- lib/StaticAnalyzer/Core/BugReporter.cpp
+++ lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1265,7 +1265,7 @@
 if (!S)
   break;
 
-if (isa(S) ||
+if (isa(S) ||
 isa(S) ||
 isa(S))
   continue;
Index: lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -329,9 +329,8 @@
 return;
 
   if (const Expr *E = V->getInit()) {
-while (const ExprWithCleanups *exprClean =
-dyn_cast(E))
-  E = exprClean->getSubExpr();
+while (const FullExpr *FE = dyn_cast(E))
+  E = FE->getSubExpr();
 
 // Look through transitive assignments, e.g.:
 //

[PATCH] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

void marked 6 inline comments as done.
void added inline comments.



Comment at: lib/AST/StmtProfile.cpp:1001
+  VisitExpr(S);
+  VisitExpr(S->getSubExpr());
+}

rsmith wrote:
> This is unnecessary: this visitor visits the children of the node for you.
If I don't have this then the link fails because it cannot find this.


Repository:
  rC Clang

https://reviews.llvm.org/D53475



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

[PATCH] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Ping?


Repository:
  rC Clang

https://reviews.llvm.org/D53475



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

[PATCH] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Thanks, Richard! :-)


Repository:
  rC Clang

https://reviews.llvm.org/D53475



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

[PATCH] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

void closed this revision.
void marked 2 inline comments as done.
void added inline comments.



Comment at: include/clang/AST/Expr.h:898
+class ConstantExpr : public FullExpr {
+  Stmt *Val;
+public:

rsmith wrote:
> I think it'd be cleaner and simpler to move this field into the base class, 
> merging it with the corresponding member of `ExprWithCleanups`.
Oh good. I was wanting to do that. Done. :-)


Repository:
  rC Clang

https://reviews.llvm.org/D53475



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

[PATCH] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

void marked 4 inline comments as done.
void added a comment.

That change was intentional. But I guess it's causing issues. I'll change it to 
a `class`.


Repository:
  rC Clang

https://reviews.llvm.org/D53475



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

[PATCH] D53475: Create ConstantExpr class

[Bill Wendling via Phabricator via cfe-commits]

void added a subscriber: rsmith.
void added a comment.

I didn't see that during my tests. Probably different -W flags or
something. I reverted that bit. I hope it fixes the problem.

-bw


Repository:
  rC Clang

https://reviews.llvm.org/D53475



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

[PATCH] D53921: Compound literals and enums require constant inits

[Bill Wendling via Phabricator via cfe-commits]

void created this revision.
void added a reviewer: rsmith.
Herald added a subscriber: cfe-commits.

Both compound literals and enums require their initializers to be
constant. Wrap the initializer expressions in a ConstantExpr so that we
can easily check for this later on.


Repository:
  rC Clang

https://reviews.llvm.org/D53921

Files:
  lib/ARCMigrate/ObjCMT.cpp
  lib/AST/ASTImporter.cpp
  lib/AST/ExprConstant.cpp
  lib/Analysis/CFG.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Misc/ast-dump-decl.c
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -37,28 +37,32 @@
   void test() {
 (void)(POD){1, 2};
 // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-// CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+// CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
+// CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
 // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
 // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
 // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
 
 (void)(HasDtor){1, 2};
 // CHECK: CXXBindTemporaryExpr {{.*}} 'brace_initializers::HasDtor'
+// CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::HasDtor'
 // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::HasDtor'
 // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::HasDtor'
 // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
 // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
 
 #if __cplusplus >= 201103L
 (void)(HasCtor){1, 2};
 // CHECK-CXX11-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::HasCtor'
-// CHECK-CXX11: CompoundLiteralExpr {{.*}} 'brace_initializers::HasCtor'
+// CHECK-CXX11: ConstantExpr {{.*}} 'brace_initializers::HasCtor'
+// CHECK-CXX11-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::HasCtor'
 // CHECK-CXX11-NEXT: CXXTemporaryObjectExpr {{.*}} 'brace_initializers::HasCtor'
 // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 1{{$}}
 // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 2{{$}}
 
 (void)(HasCtorDtor){1, 2};
 // CHECK-CXX11: CXXBindTemporaryExpr {{.*}} 'brace_initializers::HasCtorDtor'
+// CHECK-CXX11-NEXT: ConstantExpr {{.*}} 'brace_initializers::HasCtorDtor'
 // CHECK-CXX11-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::HasCtorDtor'
 // CHECK-CXX11-NEXT: CXXTemporaryObjectExpr {{.*}} 'brace_initializers::HasCtorDtor'
 // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 1{{$}}
Index: test/Misc/ast-dump-decl.c
===
--- test/Misc/ast-dump-decl.c
+++ test/Misc/ast-dump-decl.c
@@ -96,6 +96,7 @@
 };
 // CHECK:  EnumConstantDecl{{.*}} TestEnumConstantDecl 'int'
 // CHECK:  EnumConstantDecl{{.*}} TestEnumConstantDeclInit 'int'
+// CHECK-NEXT:   ConstantExpr
 // CHECK-NEXT:   IntegerLiteral
 
 struct testIndirectFieldDecl {
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1267,9 +1267,6 @@
 case Stmt::ObjCPropertyRefExprClass:
   llvm_unreachable("These are handled by PseudoObjectExpr");
 
-case Expr::ConstantExprClass:
-  return Visit(cast(S)->getSubExpr(), Pred, DstTop);
-
 case Stmt::GNUNullExprClass: {
   // GNU __null is a pointer-width integer, not an actual pointer.
   ProgramStateRef state = Pred->getState();
@@ -1285,6 +1282,10 @@
   Bldr.addNodes(Dst);
   break;
 
+case Expr::ConstantExprClass:
+  // Handled due to it being a wrapper class.
+  break;
+
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/StaticAnalyzer/Core/Environment.cpp
===
--- lib/StaticAnalyzer/Core/Environment.cpp
+++ lib/StaticAnalyzer/Core/Environment.cpp
@@ -92,6 +92,7 @@
   case Stmt::ExprWithCleanupsClass:
   case Stmt::GenericSelectionExprClass:
   case Stmt::OpaqueValueExprClass:
+  case Stmt::ConstantExprClass:
   case Stmt::ParenExprClass:
   case Stmt::SubstNonTypeTemplateParmExprClass:
 llvm_unreachable("Should have been handled by ignoreTransparentExprs");
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -5503,7 +5503,8 @@
 // array from a compound literal that creates an array of the same
 // type, so long as the initializer has no s

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

> Apologies - the value seems to indeed overflow, but I'm still very confused 
> how this was affected by this change.

What's different is that `setRequiresImmediate` is being set on the constraint 
where before it wasn't. If no range values are supplied (like in this patch), 
it defaults to `INT_MIN` and `INT_MAX`. From GNU's documentation, `n` accepts 
an integer, which we're treating as signed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D57114: Remove Expr sugar decorating the CXXUuidofExpr node.

[Bill Wendling via Phabricator via cfe-commits]

void created this revision.
void added a reviewer: rsmith.
Herald added a subscriber: cfe-commits.

Sugar, like ConstantExpr, causes an infinite expansion of the template object.


Repository:
  rC Clang

https://reviews.llvm.org/D57114

Files:
  lib/Sema/SemaTemplate.cpp
  test/CodeGenCXX/pr40395.cpp


Index: test/CodeGenCXX/pr40395.cpp
===
--- /dev/null
+++ test/CodeGenCXX/pr40395.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++17 -fms-extensions -emit-llvm %s -o - 
-triple=x86_64-pc-win32
+
+struct _GUID {};
+struct __declspec(uuid("{----}")) B {};
+
+template 
+struct A {
+  virtual void baz() { A(); }
+};
+
+void f() {
+  A<&__uuidof(B)>();
+}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -6308,7 +6308,7 @@
   // -- a predefined __func__ variable
   if (auto *E = Value.getLValueBase().dyn_cast()) {
 if (isa(E)) {
-  Converted = TemplateArgument(ArgResult.get());
+  Converted = TemplateArgument(ArgResult.get()->IgnoreImpCasts());
   break;
 }
 Diag(Arg->getBeginLoc(), diag::err_template_arg_not_decl_ref)


Index: test/CodeGenCXX/pr40395.cpp
===
--- /dev/null
+++ test/CodeGenCXX/pr40395.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++17 -fms-extensions -emit-llvm %s -o - -triple=x86_64-pc-win32
+
+struct _GUID {};
+struct __declspec(uuid("{----}")) B {};
+
+template 
+struct A {
+  virtual void baz() { A(); }
+};
+
+void f() {
+  A<&__uuidof(B)>();
+}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -6308,7 +6308,7 @@
   // -- a predefined __func__ variable
   if (auto *E = Value.getLValueBase().dyn_cast()) {
 if (isa(E)) {
-  Converted = TemplateArgument(ArgResult.get());
+  Converted = TemplateArgument(ArgResult.get()->IgnoreImpCasts());
   break;
 }
 Diag(Arg->getBeginLoc(), diag::err_template_arg_not_decl_ref)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57114: Remove Expr sugar decorating the CXXUuidofExpr node.

[Bill Wendling via Phabricator via cfe-commits]

void marked 2 inline comments as done.
void added inline comments.



Comment at: lib/Sema/SemaTemplate.cpp:6311
 if (isa(E)) {
-  Converted = TemplateArgument(ArgResult.get());
+  Converted = TemplateArgument(ArgResult.get()->IgnoreImpCasts());
   break;

aaron.ballman wrote:
> `IgnoreParenImpCasts()` or are the paren expressions of value?
I want to make this a minimal change. Richard was okay with just the implicit 
casts. I'm not relaly qualified in this part of the code to say that we should 
also include parentheses in this. I'll leave that up to you and Richard.

  https://bugs.llvm.org/show_bug.cgi?id=40395


Repository:
  rC Clang

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

https://reviews.llvm.org/D57114



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

[PATCH] D57114: Remove Expr sugar decorating the CXXUuidofExpr node.

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 183180.
void added a comment.

Move test to proper place and add "-verify" flag.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57114

Files:
  lib/Sema/SemaTemplate.cpp
  test/SemaCXX/PR40395.cpp


Index: test/SemaCXX/PR40395.cpp
===
--- /dev/null
+++ test/SemaCXX/PR40395.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -std=c++17 -fms-extensions -triple=x86_64-pc-win32 -verify 
%s
+// expected-no-diagnostics
+
+// PR40395 - ConstantExpr shouldn't cause the template object to infinitely
+// expand.
+struct _GUID {};
+struct __declspec(uuid("{----}")) B {};
+
+template 
+struct A {
+  virtual void baz() { A(); }
+};
+
+void f() {
+  A<&__uuidof(B)>();
+}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -6308,7 +6308,7 @@
   // -- a predefined __func__ variable
   if (auto *E = Value.getLValueBase().dyn_cast()) {
 if (isa(E)) {
-  Converted = TemplateArgument(ArgResult.get());
+  Converted = TemplateArgument(ArgResult.get()->IgnoreImpCasts());
   break;
 }
 Diag(Arg->getBeginLoc(), diag::err_template_arg_not_decl_ref)


Index: test/SemaCXX/PR40395.cpp
===
--- /dev/null
+++ test/SemaCXX/PR40395.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -std=c++17 -fms-extensions -triple=x86_64-pc-win32 -verify %s
+// expected-no-diagnostics
+
+// PR40395 - ConstantExpr shouldn't cause the template object to infinitely
+// expand.
+struct _GUID {};
+struct __declspec(uuid("{----}")) B {};
+
+template 
+struct A {
+  virtual void baz() { A(); }
+};
+
+void f() {
+  A<&__uuidof(B)>();
+}
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -6308,7 +6308,7 @@
   // -- a predefined __func__ variable
   if (auto *E = Value.getLValueBase().dyn_cast()) {
 if (isa(E)) {
-  Converted = TemplateArgument(ArgResult.get());
+  Converted = TemplateArgument(ArgResult.get()->IgnoreImpCasts());
   break;
 }
 Diag(Arg->getBeginLoc(), diag::err_template_arg_not_decl_ref)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57114: Remove Expr sugar decorating the CXXUuidofExpr node.

[Bill Wendling via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC352307: Remove Expr sugar decorating the CXXUuidofExpr node. 
(authored by void, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57114?vs=183180&id=183750#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57114

Files:
  lib/Sema/SemaTemplate.cpp
  test/SemaCXX/PR40395.cpp


Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -6308,7 +6308,7 @@
   // -- a predefined __func__ variable
   if (auto *E = Value.getLValueBase().dyn_cast()) {
 if (isa(E)) {
-  Converted = TemplateArgument(ArgResult.get());
+  Converted = TemplateArgument(ArgResult.get()->IgnoreImpCasts());
   break;
 }
 Diag(Arg->getBeginLoc(), diag::err_template_arg_not_decl_ref)
Index: test/SemaCXX/PR40395.cpp
===
--- test/SemaCXX/PR40395.cpp
+++ test/SemaCXX/PR40395.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -std=c++17 -fms-extensions -triple=x86_64-pc-win32 -verify 
%s
+// expected-no-diagnostics
+
+// PR40395 - ConstantExpr shouldn't cause the template object to infinitely
+// expand.
+struct _GUID {};
+struct __declspec(uuid("{----}")) B {};
+
+template 
+struct A {
+  virtual void baz() { A(); }
+};
+
+void f() {
+  A<&__uuidof(B)>();
+}


Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -6308,7 +6308,7 @@
   // -- a predefined __func__ variable
   if (auto *E = Value.getLValueBase().dyn_cast()) {
 if (isa(E)) {
-  Converted = TemplateArgument(ArgResult.get());
+  Converted = TemplateArgument(ArgResult.get()->IgnoreImpCasts());
   break;
 }
 Diag(Arg->getBeginLoc(), diag::err_template_arg_not_decl_ref)
Index: test/SemaCXX/PR40395.cpp
===
--- test/SemaCXX/PR40395.cpp
+++ test/SemaCXX/PR40395.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -std=c++17 -fms-extensions -triple=x86_64-pc-win32 -verify %s
+// expected-no-diagnostics
+
+// PR40395 - ConstantExpr shouldn't cause the template object to infinitely
+// expand.
+struct _GUID {};
+struct __declspec(uuid("{----}")) B {};
+
+template 
+struct A {
+  virtual void baz() { A(); }
+};
+
+void f() {
+  A<&__uuidof(B)>();
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57427: [CodeGen][ObjC] Fix assert on calling `__builtin_constant_p` with ObjC objects.

[Bill Wendling via Phabricator via cfe-commits]

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

LGTM, but may want to wait for other reviewers.


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

https://reviews.llvm.org/D57427



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

[PATCH] D53921: Compound literals, global array decls, and enums require constant inits

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 171976.
void retitled this revision from "Compound literals and enums require constant 
inits" to "Compound literals, global array decls, and enums require constant 
inits".
void edited the summary of this revision.

Repository:
  rC Clang

https://reviews.llvm.org/D53921

Files:
  lib/ARCMigrate/ObjCMT.cpp
  lib/AST/ASTImporter.cpp
  lib/AST/ExprConstant.cpp
  lib/Analysis/CFG.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Misc/ast-dump-decl.c
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -37,28 +37,32 @@
   void test() {
 (void)(POD){1, 2};
 // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-// CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+// CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
+// CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
 // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
 // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
 // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
 
 (void)(HasDtor){1, 2};
 // CHECK: CXXBindTemporaryExpr {{.*}} 'brace_initializers::HasDtor'
+// CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::HasDtor'
 // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::HasDtor'
 // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::HasDtor'
 // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
 // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
 
 #if __cplusplus >= 201103L
 (void)(HasCtor){1, 2};
 // CHECK-CXX11-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::HasCtor'
-// CHECK-CXX11: CompoundLiteralExpr {{.*}} 'brace_initializers::HasCtor'
+// CHECK-CXX11: ConstantExpr {{.*}} 'brace_initializers::HasCtor'
+// CHECK-CXX11-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::HasCtor'
 // CHECK-CXX11-NEXT: CXXTemporaryObjectExpr {{.*}} 'brace_initializers::HasCtor'
 // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 1{{$}}
 // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 2{{$}}
 
 (void)(HasCtorDtor){1, 2};
 // CHECK-CXX11: CXXBindTemporaryExpr {{.*}} 'brace_initializers::HasCtorDtor'
+// CHECK-CXX11-NEXT: ConstantExpr {{.*}} 'brace_initializers::HasCtorDtor'
 // CHECK-CXX11-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::HasCtorDtor'
 // CHECK-CXX11-NEXT: CXXTemporaryObjectExpr {{.*}} 'brace_initializers::HasCtorDtor'
 // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 1{{$}}
Index: test/Misc/ast-dump-decl.c
===
--- test/Misc/ast-dump-decl.c
+++ test/Misc/ast-dump-decl.c
@@ -96,6 +96,7 @@
 };
 // CHECK:  EnumConstantDecl{{.*}} TestEnumConstantDecl 'int'
 // CHECK:  EnumConstantDecl{{.*}} TestEnumConstantDeclInit 'int'
+// CHECK-NEXT:   ConstantExpr
 // CHECK-NEXT:   IntegerLiteral
 
 struct testIndirectFieldDecl {
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1267,9 +1267,6 @@
 case Stmt::ObjCPropertyRefExprClass:
   llvm_unreachable("These are handled by PseudoObjectExpr");
 
-case Expr::ConstantExprClass:
-  return Visit(cast(S)->getSubExpr(), Pred, DstTop);
-
 case Stmt::GNUNullExprClass: {
   // GNU __null is a pointer-width integer, not an actual pointer.
   ProgramStateRef state = Pred->getState();
@@ -1285,6 +1282,10 @@
   Bldr.addNodes(Dst);
   break;
 
+case Expr::ConstantExprClass:
+  // Handled due to it being a wrapper class.
+  break;
+
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/StaticAnalyzer/Core/Environment.cpp
===
--- lib/StaticAnalyzer/Core/Environment.cpp
+++ lib/StaticAnalyzer/Core/Environment.cpp
@@ -92,6 +92,7 @@
   case Stmt::ExprWithCleanupsClass:
   case Stmt::GenericSelectionExprClass:
   case Stmt::OpaqueValueExprClass:
+  case Stmt::ConstantExprClass:
   case Stmt::ParenExprClass:
   case Stmt::SubstNonTypeTemplateParmExprClass:
 llvm_unreachable("Should have been handled by ignoreTransparentExprs");
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,6 +2233,10 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
+  if (ArraySize && !CurContext->isFunctionOrMeth

[PATCH] D53921: Compound literals, global array decls, and enums require constant inits

[Bill Wendling via Phabricator via cfe-commits]

void added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:16411
 // this constant.  Skip this enum since it may be ill-formed.
-if (!ECD) {
-  return;
-}
+if (!ECD) return;
 

nickdesaulniers wrote:
> probably don't need to adjust this line
I kinda felt compelled to do it when I saw it. Became twitchy. :-)


Repository:
  rC Clang

https://reviews.llvm.org/D53921



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

[PATCH] D53921: Compound literals, global array decls, and enums require constant inits

[Bill Wendling via Phabricator via cfe-commits]

void marked 3 inline comments as done.
void added inline comments.
Herald added a reviewer: shafik.



Comment at: lib/Sema/SemaDecl.cpp:16086-16094
   CheckConvertedConstantExpression(Val, EltTy, EnumVal,
CCEK_Enumerator);
 if (Converted.isInvalid())
   Val = nullptr;
 else
   Val = Converted.get();
   } else if (!Val->isValueDependent() &&

rsmith wrote:
> I think it would make sense to create the `ConstantExpr` wrapper node in 
> `CheckConvertedConstantExpr` / `VerifyIntegerConstantExpr`. (That's also 
> where it'd make sense to cache the evaluated value on the wrapper node once 
> we start doing so.)
I thought the purpose of `ConstantExpr` is to specify those places where a 
constant expression is required. I.e., we can't have something like:

```
int z;
foo y = (foo){z + 2};
```

In this case, `z + 2` would be wrapped by the `ConstantExpr` class. But in a 
function or module scope, then it would be fine:

```
void x(int z) {
  foo y = (foo){z + 2};
}
```

So `z + 2` wouldn't be wrapped. If I perform the wrapping in 
`CheckConvertedConstantExpr`, et al, then it doesn't seem like I have the 
context to say that it's a) a compound literal, and b) in file scope. So how 
can I correctly wrap it?



Comment at: lib/Sema/SemaExpr.cpp:5752
 !literalType->isDependentType()) // C99 6.5.2.5p3
   if (CheckForConstantInitializer(LiteralExpr, literalType))
 return ExprError();

rsmith wrote:
> Can we create the `ConstantExpr` in here instead (assuming we need it at all)?
> 
> (The reason it's not clear to me that we need it at all is that file-scope 
> compound literals can only appear in contexts where a constant expression is 
> required *anyway* -- be they in array bounds, enumerators, or the initializer 
> of a global variable.)
I changed the code to wrap around here. But see my comment above. I think we 
might be thinking about `ConstantExpr` differently.


Repository:
  rC Clang

https://reviews.llvm.org/D53921



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

[PATCH] D53921: Compound literals, global array decls, and enums require constant inits

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 172082.

Repository:
  rC Clang

https://reviews.llvm.org/D53921

Files:
  include/clang/AST/Expr.h
  include/clang/AST/ExprCXX.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/StmtDataCollectors.td
  include/clang/Basic/StmtNodes.td
  include/clang/Serialization/ASTBitCodes.h
  lib/ARCMigrate/TransAutoreleasePool.cpp
  lib/ARCMigrate/TransRetainReleaseDealloc.cpp
  lib/ARCMigrate/TransUnbridgedCasts.cpp
  lib/ARCMigrate/Transforms.cpp
  lib/AST/Decl.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprCXX.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/ParentMap.cpp
  lib/AST/Stmt.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/Analysis/LiveVariables.cpp
  lib/Analysis/ThreadSafety.cpp
  lib/Analysis/ThreadSafetyCommon.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGStmt.cpp
  lib/CodeGen/CGStmtOpenMP.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  lib/StaticAnalyzer/Core/BugReporter.cpp
  lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  tools/libclang/CXCursor.cpp

Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -343,6 +343,10 @@
 K = CXCursor_CharacterLiteral;
 break;
 
+  case Stmt::ConstantExprClass:
+return MakeCXCursor(cast(S)->getSubExpr(),
+Parent, TU, RegionOfInterest);
+
   case Stmt::ParenExprClass:
 K = CXCursor_ParenExpr;
 break;
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1267,6 +1267,9 @@
 case Stmt::ObjCPropertyRefExprClass:
   llvm_unreachable("These are handled by PseudoObjectExpr");
 
+case Expr::ConstantExprClass:
+  return Visit(cast(S)->getSubExpr(), Pred, DstTop);
+
 case Stmt::GNUNullExprClass: {
   // GNU __null is a pointer-width integer, not an actual pointer.
   ProgramStateRef state = Pred->getState();
Index: lib/StaticAnalyzer/Core/Environment.cpp
===
--- lib/StaticAnalyzer/Core/Environment.cpp
+++ lib/StaticAnalyzer/Core/Environment.cpp
@@ -44,6 +44,9 @@
   case Stmt::ExprWithCleanupsClass:
 E = cast(E)->getSubExpr();
 break;
+  case Stmt::ConstantExprClass:
+E = cast(E)->getSubExpr();
+break;
   case Stmt::CXXBindTemporaryExprClass:
 E = cast(E)->getSubExpr();
 break;
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -136,8 +136,8 @@
   E = AE->getBase();
 } else if (const auto *PE = dyn_cast(E)) {
   E = PE->getSubExpr();
-} else if (const auto *EWC = dyn_cast(E)) {
-  E = EWC->getSubExpr();
+} else if (const auto *FE = dyn_cast(E)) {
+  E = FE->getSubExpr();
 } else {
   // Other arbitrary stuff.
   break;
@@ -1494,8 +1494,8 @@
 static const Expr *peelOffOuterExpr(const Expr *Ex,
 const ExplodedNode *N) {
   Ex = Ex->IgnoreParenCasts();
-  if (const auto *EWC = dyn_cast(Ex))
-return peelOffOuterExpr(EWC->getSubExpr(), N);
+  if (const auto *FE = dyn_cast(Ex))
+return peelOffOuterExpr(FE->getSubExpr(), N);
   if (const auto *OVE = dyn_cast(Ex))
 return peelOffOuterExpr(OVE->getSourceExpr(), N);
   if (const auto *POE = dyn_cast(Ex)) {
Index: lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- lib/StaticAnalyzer/Core/BugReporter.cpp
+++ lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -1265,7 +1265,7 @@
 if (!S)
   break;
 
-if (isa(S) ||
+if (isa(S) ||
 isa(S) ||
 isa(S))
   continue;
Index: lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -329,9 +329,8 @@
 return;
 
   if (const Expr *E = V->getInit()) {
-while (const ExprWithCleanups *exprClean =
-dyn_cast(E))
-  E = exprClean->getSubExpr();
+while (const FullExpr *FE = dyn_cast(E))
+  E = FE->getSubExpr();
 
 // Look through transitive assignments, e.g.:
 // int x = y = 0;
Index: lib/Serializatio

[PATCH] D53921: Compound literals, global array decls, and enums require constant inits

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 172194.

Repository:
  rC Clang

https://reviews.llvm.org/D53921

Files:
  lib/ARCMigrate/ObjCMT.cpp
  lib/AST/ASTImporter.cpp
  lib/AST/ExprConstant.cpp
  lib/Analysis/CFG.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Misc/ast-dump-decl.c
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -34,9 +34,18 @@
 ~HasCtorDtor();
   };
 
+  POD p = (POD){1, 2};
+  // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
+  // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
+
   void test() {
 (void)(POD){1, 2};
 // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
+// CHECK-NOT: ConstantExpr {{.*}} 'brace_initializers::POD'
 // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
 // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
 // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
@@ -52,14 +61,16 @@
 #if __cplusplus >= 201103L
 (void)(HasCtor){1, 2};
 // CHECK-CXX11-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::HasCtor'
+// CHECK-CXX11-NOT: ConstantExpr {{.*}} 'brace_initializers::HasCtor'
 // CHECK-CXX11: CompoundLiteralExpr {{.*}} 'brace_initializers::HasCtor'
 // CHECK-CXX11-NEXT: CXXTemporaryObjectExpr {{.*}} 'brace_initializers::HasCtor'
 // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 1{{$}}
 // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 2{{$}}
 
 (void)(HasCtorDtor){1, 2};
 // CHECK-CXX11: CXXBindTemporaryExpr {{.*}} 'brace_initializers::HasCtorDtor'
-// CHECK-CXX11-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::HasCtorDtor'
+// CHECK-CXX11-NOT: ConstantExpr {{.*}} 'brace_initializers::HasCtorDtor'
+// CHECK-CXX11: CompoundLiteralExpr {{.*}} 'brace_initializers::HasCtorDtor'
 // CHECK-CXX11-NEXT: CXXTemporaryObjectExpr {{.*}} 'brace_initializers::HasCtorDtor'
 // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 1{{$}}
 // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Misc/ast-dump-decl.c
===
--- test/Misc/ast-dump-decl.c
+++ test/Misc/ast-dump-decl.c
@@ -96,6 +96,7 @@
 };
 // CHECK:  EnumConstantDecl{{.*}} TestEnumConstantDecl 'int'
 // CHECK:  EnumConstantDecl{{.*}} TestEnumConstantDeclInit 'int'
+// CHECK-NEXT:   ConstantExpr
 // CHECK-NEXT:   IntegerLiteral
 
 struct testIndirectFieldDecl {
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1267,9 +1267,6 @@
 case Stmt::ObjCPropertyRefExprClass:
   llvm_unreachable("These are handled by PseudoObjectExpr");
 
-case Expr::ConstantExprClass:
-  return Visit(cast(S)->getSubExpr(), Pred, DstTop);
-
 case Stmt::GNUNullExprClass: {
   // GNU __null is a pointer-width integer, not an actual pointer.
   ProgramStateRef state = Pred->getState();
@@ -1285,6 +1282,10 @@
   Bldr.addNodes(Dst);
   break;
 
+case Expr::ConstantExprClass:
+  // Handled due to it being a wrapper class.
+  break;
+
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/StaticAnalyzer/Core/Environment.cpp
===
--- lib/StaticAnalyzer/Core/Environment.cpp
+++ lib/StaticAnalyzer/Core/Environment.cpp
@@ -92,6 +92,7 @@
   case Stmt::ExprWithCleanupsClass:
   case Stmt::GenericSelectionExprClass:
   case Stmt::OpaqueValueExprClass:
+  case Stmt::ConstantExprClass:
   case Stmt::ParenExprClass:
   case Stmt::SubstNonTypeTemplateParmExprClass:
 llvm_unreachable("Should have been handled by ignoreTransparentExprs");
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,6 +2233,10 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
+  if (ArraySize && !CurContext->isFunctionOrMethod())
+// A file-scoped array must have a constant array size.
+ArraySize = new (Context) ConstantExpr(ArraySize);
+
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {

[PATCH] D53921: Compound literals, enums, et al require const expr

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 172279.
void retitled this revision from "Compound literals, global array decls, and 
enums require constant inits" to "Compound literals, enums, et al require const 
expr".
void edited the summary of this revision.
void added a comment.

I believe this patch should address most, if not all, of your concerns. PTAL.


Repository:
  rC Clang

https://reviews.llvm.org/D53921

Files:
  lib/ARCMigrate/ObjCMT.cpp
  lib/AST/ASTImporter.cpp
  lib/AST/ExprConstant.cpp
  lib/Analysis/CFG.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Misc/ast-dump-decl.c
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -34,9 +34,18 @@
 ~HasCtorDtor();
   };
 
+  POD p = (POD){1, 2};
+  // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
+  // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
+
   void test() {
 (void)(POD){1, 2};
 // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
+// CHECK-NOT: ConstantExpr {{.*}} 'brace_initializers::POD'
 // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
 // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
 // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
@@ -52,14 +61,16 @@
 #if __cplusplus >= 201103L
 (void)(HasCtor){1, 2};
 // CHECK-CXX11-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::HasCtor'
+// CHECK-CXX11-NOT: ConstantExpr {{.*}} 'brace_initializers::HasCtor'
 // CHECK-CXX11: CompoundLiteralExpr {{.*}} 'brace_initializers::HasCtor'
 // CHECK-CXX11-NEXT: CXXTemporaryObjectExpr {{.*}} 'brace_initializers::HasCtor'
 // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 1{{$}}
 // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 2{{$}}
 
 (void)(HasCtorDtor){1, 2};
 // CHECK-CXX11: CXXBindTemporaryExpr {{.*}} 'brace_initializers::HasCtorDtor'
-// CHECK-CXX11-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::HasCtorDtor'
+// CHECK-CXX11-NOT: ConstantExpr {{.*}} 'brace_initializers::HasCtorDtor'
+// CHECK-CXX11: CompoundLiteralExpr {{.*}} 'brace_initializers::HasCtorDtor'
 // CHECK-CXX11-NEXT: CXXTemporaryObjectExpr {{.*}} 'brace_initializers::HasCtorDtor'
 // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 1{{$}}
 // CHECK-CXX11-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Misc/ast-dump-decl.c
===
--- test/Misc/ast-dump-decl.c
+++ test/Misc/ast-dump-decl.c
@@ -96,6 +96,7 @@
 };
 // CHECK:  EnumConstantDecl{{.*}} TestEnumConstantDecl 'int'
 // CHECK:  EnumConstantDecl{{.*}} TestEnumConstantDeclInit 'int'
+// CHECK-NEXT:   ConstantExpr
 // CHECK-NEXT:   IntegerLiteral
 
 struct testIndirectFieldDecl {
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1267,9 +1267,6 @@
 case Stmt::ObjCPropertyRefExprClass:
   llvm_unreachable("These are handled by PseudoObjectExpr");
 
-case Expr::ConstantExprClass:
-  return Visit(cast(S)->getSubExpr(), Pred, DstTop);
-
 case Stmt::GNUNullExprClass: {
   // GNU __null is a pointer-width integer, not an actual pointer.
   ProgramStateRef state = Pred->getState();
@@ -1285,6 +1282,10 @@
   Bldr.addNodes(Dst);
   break;
 
+case Expr::ConstantExprClass:
+  // Handled due to it being a wrapper class.
+  break;
+
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/StaticAnalyzer/Core/Environment.cpp
===
--- lib/StaticAnalyzer/Core/Environment.cpp
+++ lib/StaticAnalyzer/Core/Environment.cpp
@@ -92,6 +92,7 @@
   case Stmt::ExprWithCleanupsClass:
   case Stmt::GenericSelectionExprClass:
   case Stmt::OpaqueValueExprClass:
+  case Stmt::ConstantExprClass:
   case Stmt::ParenExprClass:
   case Stmt::SubstNonTypeTemplateParmExprClass:
 llvm_unreachable("Should have been handled by ignoreTransparentExprs");
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,6 +2233,10 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
  

[PATCH] D53921: Compound literals, enums, et al require const expr

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 172297.

Repository:
  rC Clang

https://reviews.llvm.org/D53921

Files:
  include/clang/AST/Expr.h
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  lib/Analysis/CFG.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Import/switch-stmt/test.cpp
  test/Misc/ast-dump-attr.cpp
  test/Misc/ast-dump-c-attr.c
  test/Misc/ast-dump-color.cpp
  test/Misc/ast-dump-decl.c
  test/Misc/ast-dump-decl.cpp
  test/SemaCXX/compound-literal.cpp
  test/Tooling/clang-check-ast-dump.cpp
  unittests/AST/ASTImporterTest.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1574,13 +1574,16 @@
 ifStmt(has(switchStmt(forEachSwitchCase(defaultStmt()));
   EXPECT_TRUE(matches("void x() { switch(42) { case 1+1: case 4:; } }",
   switchStmt(forEachSwitchCase(
-caseStmt(hasCaseConstant(integerLiteral()));
+caseStmt(hasCaseConstant(
+constantExpr(has(integerLiteral();
   EXPECT_TRUE(notMatches("void x() { switch(42) { case 1+1: case 2+2:; } }",
  switchStmt(forEachSwitchCase(
-   caseStmt(hasCaseConstant(integerLiteral()));
+   caseStmt(hasCaseConstant(
+   constantExpr(has(integerLiteral();
   EXPECT_TRUE(notMatches("void x() { switch(42) { case 1 ... 2:; } }",
  switchStmt(forEachSwitchCase(
-   caseStmt(hasCaseConstant(integerLiteral()));
+   caseStmt(hasCaseConstant(
+   constantExpr(has(integerLiteral();
   EXPECT_TRUE(matchAndVerifyResultTrue(
 "void x() { switch (42) { case 1: case 2: case 3: default:; } }",
 switchStmt(forEachSwitchCase(caseStmt().bind("x"))),
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -760,23 +760,23 @@
has(
  designatedInitExpr(
designatorCountIs(2),
-   has(floatLiteral(
+   hasDescendant(floatLiteral(
  equals(1.0))),
-   has(integerLiteral(
+   hasDescendant(integerLiteral(
  equals(2),
has(
  designatedInitExpr(
designatorCountIs(2),
-   has(floatLiteral(
+   hasDescendant(floatLiteral(
  equals(2.0))),
-   has(integerLiteral(
+   hasDescendant(integerLiteral(
  equals(2),
has(
  designatedInitExpr(
designatorCountIs(2),
-   has(floatLiteral(
+   hasDescendant(floatLiteral(
  equals(1.0))),
-   has(integerLiteral(
+   hasDescendant(integerLiteral(
  equals(0)
  );
 }
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -140,6 +140,7 @@
 if (!Imported)
   return testing::AssertionFailure() << "Import failed, nullptr returned!";
 
+
 return Verifier.match(Imported, WrapperMatcher);
   }
 
@@ -502,7 +503,6 @@
   EXPECT_THAT(RedeclsD1, ::testing::ContainerEq(RedeclsD2));
 }
 
-
 TEST_P(ImportExpr, ImportStringLiteral) {
   MatchVerifier Verifier;
   testImport(
@@ -719,19 +719,18 @@
   initListExpr(
   has(designatedInitExpr(
 

[PATCH] D53921: Compound literals, enums, et al require const expr

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 172313.

Repository:
  rC Clang

https://reviews.llvm.org/D53921

Files:
  include/clang/AST/Expr.h
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  lib/Analysis/CFG.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Import/switch-stmt/test.cpp
  test/Misc/ast-dump-attr.cpp
  test/Misc/ast-dump-c-attr.c
  test/Misc/ast-dump-color.cpp
  test/Misc/ast-dump-decl.c
  test/Misc/ast-dump-decl.cpp
  test/SemaCXX/compound-literal.cpp
  test/Tooling/clang-check-ast-dump.cpp
  unittests/AST/ASTImporterTest.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1574,13 +1574,16 @@
 ifStmt(has(switchStmt(forEachSwitchCase(defaultStmt()));
   EXPECT_TRUE(matches("void x() { switch(42) { case 1+1: case 4:; } }",
   switchStmt(forEachSwitchCase(
-caseStmt(hasCaseConstant(integerLiteral()));
+caseStmt(hasCaseConstant(
+constantExpr(has(integerLiteral();
   EXPECT_TRUE(notMatches("void x() { switch(42) { case 1+1: case 2+2:; } }",
  switchStmt(forEachSwitchCase(
-   caseStmt(hasCaseConstant(integerLiteral()));
+   caseStmt(hasCaseConstant(
+   constantExpr(has(integerLiteral();
   EXPECT_TRUE(notMatches("void x() { switch(42) { case 1 ... 2:; } }",
  switchStmt(forEachSwitchCase(
-   caseStmt(hasCaseConstant(integerLiteral()));
+   caseStmt(hasCaseConstant(
+   constantExpr(has(integerLiteral();
   EXPECT_TRUE(matchAndVerifyResultTrue(
 "void x() { switch (42) { case 1: case 2: case 3: default:; } }",
 switchStmt(forEachSwitchCase(caseStmt().bind("x"))),
Index: unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -760,23 +760,23 @@
has(
  designatedInitExpr(
designatorCountIs(2),
-   has(floatLiteral(
+   hasDescendant(floatLiteral(
  equals(1.0))),
-   has(integerLiteral(
+   hasDescendant(integerLiteral(
  equals(2),
has(
  designatedInitExpr(
designatorCountIs(2),
-   has(floatLiteral(
+   hasDescendant(floatLiteral(
  equals(2.0))),
-   has(integerLiteral(
+   hasDescendant(integerLiteral(
  equals(2),
has(
  designatedInitExpr(
designatorCountIs(2),
-   has(floatLiteral(
+   hasDescendant(floatLiteral(
  equals(1.0))),
-   has(integerLiteral(
+   hasDescendant(integerLiteral(
  equals(0)
  );
 }
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -140,6 +140,7 @@
 if (!Imported)
   return testing::AssertionFailure() << "Import failed, nullptr returned!";
 
+
 return Verifier.match(Imported, WrapperMatcher);
   }
 
@@ -502,7 +503,6 @@
   EXPECT_THAT(RedeclsD1, ::testing::ContainerEq(RedeclsD2));
 }
 
-
 TEST_P(ImportExpr, ImportStringLiteral) {
   MatchVerifier Verifier;
   testImport(
@@ -719,19 +719,18 @@
   initListExpr(
   has(designatedInitExpr(
   design

[PATCH] D53921: Compound literals, enums, et al require const expr

[Bill Wendling via Phabricator via cfe-commits]

void marked 2 inline comments as done.
void added inline comments.



Comment at: include/clang/AST/Expr.h:3073-3074
+  e = ice->getSubExpr();
+else if (ConstantExpr *ce = dyn_cast(e))
+  e = ce->getSubExpr();
+else

rsmith wrote:
> Should we skip an arbitrary `FullExpr` here (and in the other `Ignore` 
> functions below)?
Sure. That seems reasonable.



Comment at: lib/Sema/SemaType.cpp:2236-2238
+  if (ArraySize && !CurContext->isFunctionOrMethod())
+// A file-scoped array must have a constant array size.
+ArraySize = new (Context) ConstantExpr(ArraySize);

rsmith wrote:
> As noted above, I'd prefer for `VerifyIntegerConstantExpression` to create 
> this. But if we need to do it here, we should do it on the code path that 
> creates a `ConstantArrayType`, not based on whether the array type appears at 
> file scope.
I think my next patch takes care of this (moves it to 
`VerifyIntegerConstantExpression`). There may be conflicts between the two 
patches though, so I'd like to move this there in the follow-up patch.


Repository:
  rC Clang

https://reviews.llvm.org/D53921



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

[PATCH] D53921: Compound literals, enums, et al require const expr

[Bill Wendling via Phabricator via cfe-commits]

void marked an inline comment as done.
void added a comment.

I'm using "arcanist" for the first time, and I don't want to mess things up. 
I'll address the comments here in the next patch.


Repository:
  rC Clang

https://reviews.llvm.org/D53921



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

[PATCH] D53921: Compound literals, enums, et al require const expr

[Bill Wendling via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC346455: Compound literals, enums, et al require const expr 
(authored by void, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D53921?vs=172313&id=173248#toc

Repository:
  rC Clang

https://reviews.llvm.org/D53921

Files:
  include/clang/AST/Expr.h
  include/clang/ASTMatchers/ASTMatchers.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/ASTMatchers/ASTMatchersInternal.cpp
  lib/ASTMatchers/Dynamic/Registry.cpp
  lib/Analysis/CFG.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/Environment.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Import/switch-stmt/test.cpp
  test/Misc/ast-dump-attr.cpp
  test/Misc/ast-dump-c-attr.c
  test/Misc/ast-dump-color.cpp
  test/Misc/ast-dump-decl.c
  test/Misc/ast-dump-decl.cpp
  test/SemaCXX/compound-literal.cpp
  test/Tooling/clang-check-ast-dump.cpp
  unittests/AST/ASTImporterTest.cpp
  unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -3024,8 +3024,7 @@
 private:
   ImplicitCastExpr(QualType ty, CastKind kind, Expr *op,
unsigned BasePathLength, ExprValueKind VK)
-: CastExpr(ImplicitCastExprClass, ty, VK, kind, op, BasePathLength) {
-  }
+: CastExpr(ImplicitCastExprClass, ty, VK, kind, op, BasePathLength) { }
 
   /// Construct an empty implicit cast.
   explicit ImplicitCastExpr(EmptyShell Shell, unsigned PathSize)
@@ -3068,8 +3067,13 @@
 
 inline Expr *Expr::IgnoreImpCasts() {
   Expr *e = this;
-  while (ImplicitCastExpr *ice = dyn_cast(e))
-e = ice->getSubExpr();
+  while (true)
+if (ImplicitCastExpr *ice = dyn_cast(e))
+  e = ice->getSubExpr();
+else if (ConstantExpr *ce = dyn_cast(e))
+  e = ce->getSubExpr();
+else
+  break;
   return e;
 }
 
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -1574,6 +1574,18 @@
UnresolvedUsingTypenameDecl>
 unresolvedUsingTypenameDecl;
 
+/// Matches a constant expression wrapper.
+///
+/// Example matches the constant in the case statement:
+/// (matcher = constantExpr())
+/// \code
+///   switch (a) {
+///   case 37: break;
+///   }
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher
+constantExpr;
+
 /// Matches parentheses used in expressions.
 ///
 /// Example matches (foo() + 1)
Index: test/Misc/ast-dump-color.cpp
===
--- test/Misc/ast-dump-color.cpp
+++ test/Misc/ast-dump-color.cpp
@@ -49,12 +49,14 @@
 //CHECK: {{^}}[[Blue]]| |   |-[[RESET]][[MAGENTA]]IntegerLiteral[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:10:11[[RESET]]> [[Green]]'int'[[RESET]][[Cyan:.\[0;36m]][[RESET]][[Cyan]][[RESET]][[CYAN]] 1[[RESET]]{{$}}
 //CHECK: {{^}}[[Blue]]| |   `-[[RESET]][[MAGENTA]]CompoundStmt[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:14[[RESET]], [[Yellow]]line:15:3[[RESET]]>{{$}}
 //CHECK: {{^}}[[Blue]]| | |-[[RESET]][[MAGENTA]]CaseStmt[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:11:3[[RESET]], [[Yellow]]line:12:27[[RESET]]>{{$}}
-//CHECK: {{^}}[[Blue]]| | | |-[[RESET]][[MAGENTA]]IntegerLiteral[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:11:8[[RESET]]> [[Green]]'int'[[RESET]][[Cyan]][[RESET]][[Cyan]][[RESET]][[CYAN]] 1[[RESET]]{{$}}
+//CHECK: {{^}}[[Blue]]| | | |-[[RESET]][[MAGENTA]]ConstantExpr[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:11:8[[RESET]]> [[Green]]'int'[[RESET]][[Cyan]][[RESET]][[Cyan]][[RESET]]{{$}}
+//CHECK: {{^}}[[Blue]]| | | | `-[[RESET]][[MAGENTA]]IntegerLiteral[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:8[[RESET]]> [[Green]]'int'[[RESET]][[Cyan]][[RESET]][[Cyan]][[RESET]][[CYAN]] 1[[RESET]]{{$}}
 //CHECK: {{^}}[[Blue]]| | | `-[[RESET]][[MAGENTA]]AttributedStmt[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]line:12:5[[RESET]], [[Yellow]]col:27[[RESET]]>{{$}}
 //CHECK: {{^}}[[Blue]]| | |   |-[[RESET]][[BLUE]]FallThroughAttr[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:7[[RESET]], [[Yellow]]col:14[[RESET]]>{{$}}
 //CHECK: {{^}}[[Blue]]| | |   `-[[RESET]][[MAGENTA]]NullStmt[[RESET]][[Yellow]] 0x{{[0-9a-fA-F]*}}[[RESET]] <[[Yellow]]col:27[[RESET]]>{{$}}
 //CHECK: {{^}}[[Blue]]| | `-[[RESET]][[MAGENTA]]CaseStmt[[RESET]][[Yellow]

[PATCH] D53921: Compound literals, enums, et al require const expr

[Bill Wendling via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346455: Compound literals, enums, et al require const expr 
(authored by void, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53921?vs=172313&id=173249#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53921

Files:
  cfe/trunk/include/clang/AST/Expr.h
  cfe/trunk/include/clang/ASTMatchers/ASTMatchers.h
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/lib/AST/Expr.cpp
  cfe/trunk/lib/AST/ExprConstant.cpp
  cfe/trunk/lib/ASTMatchers/ASTMatchersInternal.cpp
  cfe/trunk/lib/ASTMatchers/Dynamic/Registry.cpp
  cfe/trunk/lib/Analysis/CFG.cpp
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CGExprAgg.cpp
  cfe/trunk/lib/CodeGen/CGExprComplex.cpp
  cfe/trunk/lib/CodeGen/CGExprConstant.cpp
  cfe/trunk/lib/CodeGen/CGExprScalar.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/lib/Sema/SemaInit.cpp
  cfe/trunk/lib/Sema/SemaOverload.cpp
  cfe/trunk/lib/Sema/SemaType.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/Environment.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
  cfe/trunk/test/Import/switch-stmt/test.cpp
  cfe/trunk/test/Misc/ast-dump-attr.cpp
  cfe/trunk/test/Misc/ast-dump-c-attr.c
  cfe/trunk/test/Misc/ast-dump-color.cpp
  cfe/trunk/test/Misc/ast-dump-decl.c
  cfe/trunk/test/Misc/ast-dump-decl.cpp
  cfe/trunk/test/SemaCXX/compound-literal.cpp
  cfe/trunk/test/Tooling/clang-check-ast-dump.cpp
  cfe/trunk/unittests/AST/ASTImporterTest.cpp
  cfe/trunk/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
  cfe/trunk/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -1260,6 +1260,8 @@
 return EmitVAArgExprLValue(cast(E));
   case Expr::DeclRefExprClass:
 return EmitDeclRefLValue(cast(E));
+  case Expr::ConstantExprClass:
+return EmitLValue(cast(E)->getSubExpr());
   case Expr::ParenExprClass:
 return EmitLValue(cast(E)->getSubExpr());
   case Expr::GenericSelectionExprClass:
Index: cfe/trunk/lib/CodeGen/CGExprAgg.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprAgg.cpp
+++ cfe/trunk/lib/CodeGen/CGExprAgg.cpp
@@ -125,6 +125,10 @@
 return Visit(E->getReplacement());
   }
 
+  void VisitConstantExpr(ConstantExpr *E) {
+return Visit(E->getSubExpr());
+  }
+
   // l-values.
   void VisitDeclRefExpr(DeclRefExpr *E) { EmitAggLoadOfLValue(E); }
   void VisitMemberExpr(MemberExpr *ME) { EmitAggLoadOfLValue(ME); }
Index: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprConstant.cpp
+++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp
@@ -723,6 +723,10 @@
 return nullptr;
   }
 
+  llvm::Constant *VisitConstantExpr(ConstantExpr *CE, QualType T) {
+return Visit(CE->getSubExpr(), T);
+  }
+
   llvm::Constant *VisitParenExpr(ParenExpr *PE, QualType T) {
 return Visit(PE->getSubExpr(), T);
   }
@@ -1601,6 +1605,7 @@
   ConstantLValue tryEmitBase(const APValue::LValueBase &base);
 
   ConstantLValue VisitStmt(const Stmt *S) { return nullptr; }
+  ConstantLValue VisitConstantExpr(const ConstantExpr *E);
   ConstantLValue VisitCompoundLiteralExpr(const CompoundLiteralExpr *E);
   ConstantLValue VisitStringLiteral(const StringLiteral *E);
   ConstantLValue VisitObjCEncodeExpr(const ObjCEncodeExpr *E);
@@ -1756,6 +1761,11 @@
 }
 
 ConstantLValue
+ConstantLValueEmitter::VisitConstantExpr(const ConstantExpr *E) {
+  return Visit(E->getSubExpr());
+}
+
+ConstantLValue
 ConstantLValueEmitter::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) {
   return tryEmitGlobalCompoundLiteral(CGM, Emitter.CGF, E);
 }
Index: cfe/trunk/lib/CodeGen/CGExprComplex.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprComplex.cpp
+++ cfe/trunk/lib/CodeGen/CGExprComplex.cpp
@@ -101,6 +101,9 @@
 llvm_unreachable("Stmt can't have complex result type!");
   }
   ComplexPairTy VisitExpr(Expr *S);
+  ComplexPairTy VisitConstantExpr(ConstantExpr *E) {
+return Visit(E->getSubExpr());
+  }
   ComplexPairTy VisitParenExpr(ParenExpr *PE) { return Visit(PE->getSubExpr());}
   ComplexPairTy VisitGenericSelectionExpr(GenericSelectionExpr *GE) {
 return Visit(GE->getResultExpr());
Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp
@@ -398,6 +398,9 @@
   }
   Value *VisitExpr(Expr *S);
 
+  Value *VisitConstantExpr(ConstantExpr *E) {
+return Visit(E->getSubExpr());
+  }
   Value *VisitParenExpr(ParenExpr *PE) {
 return Visit(PE->getSubExpr());
   }
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void created this revision.
void added a reviewer: rsmith.
Herald added subscribers: cfe-commits, kristina.
Herald added a reviewer: shafik.

A __builtin_constant_p may end up with a constant after inlining. Use
the is.constant intrinsic if it's a variable that's in a context where
it may resolve to a constant, e.g., an argument to a function after
inlining.


Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1283,9 +1283,6 @@
   break;
 
 case Expr::ConstantExprClass:
-  // Handled due to it being a wrapper class.
-  break;
-
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,10 +2233,6 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-// A file-scoped array must have a constant array size.
-ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
 Diag(Loc, diag::err_opencl_vla);
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -178,6 +178,8 @@
   while (true) {
 if (ImplicitCastExpr *IC = dyn_cast(E))
   E = IC->getSubExpr();
+else if (ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
@@ -5225,6 +5227,8 @@
   while (true) {
 if (const ImplicitCastExpr *ICE = dyn_cast(E))
   E = ICE->getSubExpr();
+else if (const ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (const SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sem

[PATCH] D54356: Convert CheckICE into a statment visitor

[Bill Wendling via Phabricator via cfe-commits]

void created this revision.
void added reviewers: rsmith, shafik.
Herald added a subscriber: jfb.

This cleans up the code somewhat and allows us conditionally to act on
different types of nodes depending on their context. E.g., if we're
checking for an ICE in a constant context.


Repository:
  rC Clang

https://reviews.llvm.org/D54356

Files:
  lib/AST/ExprConstant.cpp

Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -10956,401 +10956,372 @@
 
 static ICEDiag Worst(ICEDiag A, ICEDiag B) { return A.Kind >= B.Kind ? A : B; }
 
-static ICEDiag CheckEvalInICE(const Expr* E, const ASTContext &Ctx) {
+static ICEDiag CheckEvalInICE(const Expr *E, const ASTContext &Ctx,
+  EvalInfo &Info) {
   Expr::EvalResult EVResult;
-  if (!E->EvaluateAsRValue(EVResult, Ctx) || EVResult.HasSideEffects ||
+  if (!::EvaluateAsRValue(Info, E, EVResult.Val) || EVResult.HasSideEffects ||
   !EVResult.Val.isInt())
 return ICEDiag(IK_NotICE, E->getBeginLoc());
 
   return NoDiag();
 }
 
+class ICEChecker
+  : public ConstStmtVisitor {
+  const ASTContext &Ctx;
+  EvalInfo &Info;
+
+  typedef ConstStmtVisitor StmtVisitorTy;
+
+public:
+  ICEChecker(const ASTContext &Ctx, EvalInfo &Info)
+  : Ctx(Ctx), Info(Info) {}
+
+  ICEDiag VisitStmt(const Stmt *S) {
+return ICEDiag(IK_NotICE, S->getBeginLoc());
+  }
+
+  ICEDiag VisitExpr(const Expr *E);
+  ICEDiag VisitInitListExpr(const InitListExpr *E);
+  ICEDiag VisitSubstNonTypeTemplateParmExpr(
+  const SubstNonTypeTemplateParmExpr *E);
+  ICEDiag VisitParenExpr(const ParenExpr *E);
+  ICEDiag VisitGenericSelectionExpr(const GenericSelectionExpr *E);
+  ICEDiag VisitCallExpr(const CallExpr *E);
+  ICEDiag VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *E);
+  ICEDiag VisitDeclRefExpr(const DeclRefExpr *E);
+  ICEDiag VisitBinaryOperator(const BinaryOperator *E);
+  ICEDiag VisitUnaryOperator(const UnaryOperator *E);
+  ICEDiag VisitOffsetOfExpr(const OffsetOfExpr *E);
+  ICEDiag VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
+  ICEDiag VisitCastExpr(const CastExpr *E);
+  ICEDiag VisitBinaryConditionalOperator(const BinaryConditionalOperator *E);
+  ICEDiag VisitConditionalOperator(const ConditionalOperator *E);
+  ICEDiag VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *E);
+  ICEDiag VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *E);
+  ICEDiag VisitChooseExpr(const ChooseExpr *E);
+};
+
+ICEDiag ICEChecker::VisitExpr(const Expr *E) {
+  switch (E->getStmtClass()) {
+  default:
+return ICEDiag(IK_NotICE, E->getBeginLoc());
+  case Expr::ArrayTypeTraitExprClass:
+  case Expr::CharacterLiteralClass:
+  case Expr::CXXBoolLiteralExprClass:
+  case Expr::CXXNoexceptExprClass:
+  case Expr::CXXScalarValueInitExprClass:
+  case Expr::ExpressionTraitExprClass:
+  case Expr::FixedPointLiteralClass:
+  case Expr::GNUNullExprClass:
+// GCC considers the GNU __null value to be an integral constant expression.
+  case Expr::IntegerLiteralClass:
+  case Expr::ObjCBoolLiteralExprClass:
+  case Expr::SizeOfPackExprClass:
+  case Expr::TypeTraitExprClass:
+return NoDiag();
+  }
+}
+
+ICEDiag ICEChecker::VisitInitListExpr(const InitListExpr *E) {
+  // C++03 [dcl.init]p13: If T is a scalar type, then a declaration of the form
+  // "T x = { a };" is equivalent to "T x = a;".
+  // Unless we're initializing a reference, T is a scalar as it is known to be
+  // of integral or enumeration type.
+  if (E->isRValue())
+if (E->getNumInits() == 1)
+  return StmtVisitorTy::Visit(E->getInit(0));
+  return ICEDiag(IK_NotICE, E->getBeginLoc());
+}
+
+ICEDiag ICEChecker::VisitSubstNonTypeTemplateParmExpr(
+const SubstNonTypeTemplateParmExpr *E) {
+  return StmtVisitorTy::Visit(E->getReplacement());
+}
+
+ICEDiag ICEChecker::VisitParenExpr(const ParenExpr *E) {
+  return StmtVisitorTy::Visit(E->getSubExpr());
+}
+
+ICEDiag ICEChecker::VisitGenericSelectionExpr(const GenericSelectionExpr *E) {
+  return StmtVisitorTy::Visit(E->getResultExpr());
+}
+
+ICEDiag ICEChecker::VisitCallExpr(const CallExpr *E) {
+  // C99 6.6/3 allows function calls within unevaluated subexpressions of
+  // constant expressions, but they can never be ICEs because an ICE cannot
+  // contain an operand of (pointer to) function type.
+  if (E->getBuiltinCallee())
+return CheckEvalInICE(E, Ctx, Info);
+  return ICEDiag(IK_NotICE, E->getBeginLoc());
+}
+
+ICEDiag ICEChecker::VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *E) {
+  return VisitCallExpr(E);
+}
+
+ICEDiag ICEChecker::VisitDeclRefExpr(const DeclRefExpr *E) {
+  if (isa(E->getDecl()))
+return NoDiag();
+
+  const ValueDecl *D = E->getDecl();
+  if (Ctx.getLangOpts().CPlusPlus &&
+  D && IsConstNonVolatile(D->getType())) {
+// Parameter variables are never constants.  Without this check,
+// getAnyInitializer() can find a default argument, which leads
+  

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 173445.
void added a comment.
Herald added a subscriber: jfb.

Adding ConstantExpr visitor.


Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1283,9 +1283,6 @@
   break;
 
 case Expr::ConstantExprClass:
-  // Handled due to it being a wrapper class.
-  break;
-
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,10 +2233,6 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-// A file-scoped array must have a constant array size.
-ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
 Diag(Loc, diag::err_opencl_vla);
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -178,6 +178,8 @@
   while (true) {
 if (ImplicitCastExpr *IC = dyn_cast(E))
   E = IC->getSubExpr();
+else if (ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
@@ -5225,6 +5227,8 @@
   while (true) {
 if (const ImplicitCastExpr *ICE = dyn_cast(E))
   E = ICE->getSubExpr();
+else if (const ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (const SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5444,7 +5444,7 @@
 
 if (Notes.empty()) {
   // It's a constant expression.
-  return new (S.Context) ConstantExpr(Result.get());
+  return ConstantExpr::Create(S.Context, Result.get());
 }
 

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

This has https://reviews.llvm.org/D54356 integrated into it. 
https://reviews.llvm.org/D54356 should be reviewed and submitted first, even 
though it's out of order.


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D54356: Convert CheckICE into a statment visitor

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Ping?


Repository:
  rC Clang

https://reviews.llvm.org/D54356



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Ping?


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D54356: Convert CheckICE into a statment visitor

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

This code is called in over 90 places, so it's hard to tell if they all are in 
a constant context. Though I suppose that what this code is supposed to check 
for would work the same in a constant context as without one. I can revert this 
if you want, but to be honest the original function was terrible--it's huge and 
hard to understand what's going on. As for adding new expressions, this isn't 
the only place where a `StmtVisitor` is used. One still needs to go through all 
of those visitors to see if they need to add their expression.


Repository:
  rC Clang

https://reviews.llvm.org/D54356



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

[PATCH] D54356: Convert CheckICE into a statment visitor

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Okay. I'll revert this then.


Repository:
  rC Clang

https://reviews.llvm.org/D54356



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

[PATCH] D54356: Convert CheckICE into a statment visitor

[Bill Wendling via Phabricator via cfe-commits]

void abandoned this revision.
void added a comment.

This isn't necessary. We can assume it's in a constant context because it's 
checking for an ICE.


Repository:
  rC Clang

https://reviews.llvm.org/D54356



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

[PATCH] D54356: Convert CheckICE into a statment visitor

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In https://reviews.llvm.org/D54356#1297543, @rsmith wrote:

> In https://reviews.llvm.org/D54356#1297522, @void wrote:
>
> > Okay. I'll revert this then.
>
>
> I don't think we necessarily need the change in the other patch that's based 
> on this one, but I still think this refactoring is an improvement :)


Thanks. I can resurrect this after these changes go in. This will keep the 
resulting changes small. :-)


Repository:
  rC Clang

https://reviews.llvm.org/D54356



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 173922.
void marked an inline comment as done.
void added a comment.

Remove some extraneous checks.


Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1283,9 +1283,6 @@
   break;
 
 case Expr::ConstantExprClass:
-  // Handled due to it being a wrapper class.
-  break;
-
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,10 +2233,6 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-// A file-scoped array must have a constant array size.
-ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
 Diag(Loc, diag::err_opencl_vla);
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -178,6 +178,8 @@
   while (true) {
 if (ImplicitCastExpr *IC = dyn_cast(E))
   E = IC->getSubExpr();
+else if (ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
@@ -5225,6 +5227,8 @@
   while (true) {
 if (const ImplicitCastExpr *ICE = dyn_cast(E))
   E = ICE->getSubExpr();
+else if (const ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (const SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -5444,7 +5444,7 @@
 
 if (Notes.empty()) {
   // It's a constant expression.
-  return new (S.Context) ConstantExpr(Result.get());
+  return ConstantExpr::Crea

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added inline comments.



Comment at: include/clang/AST/Expr.h:908-912
+  static ConstantExpr *Create(const ASTContext &Context, Expr *E) {
+if (ConstantExpr *CE = dyn_cast(E))
+  return CE;
+return new (Context) ConstantExpr(E);
+  }

rsmith wrote:
> If we're creating two `ConstantExpr` wrappers for the same expression, that 
> seems like a bug in the caller to me. When does this happen?
I saw a place that was trying to do that, but it may no longer be valid. I can 
remove the check for now.



Comment at: lib/AST/Expr.cpp:2915-2916
+  case ConstantExprClass: {
+const Expr *Exp = cast(this)->getSubExpr();
+return Exp->isConstantInitializer(Ctx, false, Culprit);
+  }

rsmith wrote:
> Can we just return `true` here? If not, please add a FIXME saying that we 
> should be able to.
To be honest, I don't know. Theoretically we should be able to. Let me give it 
a try and see how it goes.



Comment at: lib/Sema/SemaExpr.cpp:14156-14157
 
+  if (!CurContext->isFunctionOrMethod())
+E = ConstantExpr::Create(Context, E);
+

rsmith wrote:
> I don't understand why `CurContext` matters here. Can you explain the intent 
> of this check?
It may have been an artifact of development. I can remove it.


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void marked an inline comment as not done.
void added inline comments.



Comment at: lib/AST/Expr.cpp:2915-2916
+  case ConstantExprClass: {
+const Expr *Exp = cast(this)->getSubExpr();
+return Exp->isConstantInitializer(Ctx, false, Culprit);
+  }

void wrote:
> rsmith wrote:
> > Can we just return `true` here? If not, please add a FIXME saying that we 
> > should be able to.
> To be honest, I don't know. Theoretically we should be able to. Let me give 
> it a try and see how it goes.
I tested this and it looks like it could lead to an extra warning like this:

```
File /sandbox/morbo/llvm/llvm-mirror/tools/clang/test/Sema/array-init.c Line 
285: cannot initialize array of type 'int [5]' with non-constant array of type 
'int [5]'
```

(Similar in `Sema/compound-literal.c`.) I'll add a comment.



Comment at: lib/Sema/SemaExpr.cpp:4973-4974
 
+if ((ICEArguments & (1 << (ArgIx - 1))) != 0)
+  Arg = ConstantExpr::Create(Context, Arg);
+

rsmith wrote:
> We should create this node as part of checking that the argument is an ICE 
> (in `SemaBuiltinConstantArg`).
It turns out that `SemaBuiltinConstantArg` isn't called for 
`__builtin_constant_p()`. Its argument type is `.`, which...I'm not sure what 
that means. It looks like a vararg, but that doesn't make sense for the builtin.

Should I change its signature in `Builtins.def`?


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 173970.
void marked an inline comment as not done.
void added a comment.

Updated to address comments.


Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/Sema/builtins.c
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -122,6 +122,14 @@
  __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1283,9 +1283,6 @@
   break;
 
 case Expr::ConstantExprClass:
-  // Handled due to it being a wrapper class.
-  break;
-
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,10 +2233,6 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-// A file-scoped array must have a constant array size.
-ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
 Diag(Loc, diag::err_opencl_vla);
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -178,6 +178,8 @@
   while (true) {
 if (ImplicitCastExpr *IC = dyn_cast(E))
   E = IC->getSubExpr();
+else if (ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
@@ -52

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

I think this is ready now. PTAL.




Comment at: lib/CodeGen/CGBuiltin.cpp:1930-1931
+if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+  // At -O0, we don't perform inlining, so we don't need to delay the
+  // processing.
+  return RValue::get(ConstantInt::get(Int32Ty, 0));

rsmith wrote:
> Are there cases where a call to an `always_inline` function could change the 
> outcome?
Yes, but the result of bcp isn't guaranteed to be the same for optimized and 
unoptimized code. GCC's documentation says that it won't return 1 unless `-O` 
is used. Their code appears to back this up somewhat:

```
  switch (fcode)
{
case BUILT_IN_CONSTANT_P:
  {
tree val = fold_builtin_constant_p (arg0);

/* Gimplification will pull the CALL_EXPR for the builtin out of
   an if condition.  When not optimizing, we'll not CSE it back.
   To avoid link error types of regressions, return false now.  */
if (!val && !optimize)
  val = integer_zero_node;

return val;
  }
...
}
```

Our inlining is different of course, and I'm not sure when GCC performs their 
`always_inline` inlining. It may be before the above code is called. But I 
think we're within reasonable bounds with respect to the documentation.

(That said, GCC has a tendency to violate its own documentation. So caveat 
emptor, etc.)



Comment at: lib/Sema/SemaExpr.cpp:4973-4974
 
+if ((ICEArguments & (1 << (ArgIx - 1))) != 0)
+  Arg = ConstantExpr::Create(Context, Arg);
+

rsmith wrote:
> void wrote:
> > rsmith wrote:
> > > We should create this node as part of checking that the argument is an 
> > > ICE (in `SemaBuiltinConstantArg`).
> > It turns out that `SemaBuiltinConstantArg` isn't called for 
> > `__builtin_constant_p()`. Its argument type is `.`, which...I'm not sure 
> > what that means. It looks like a vararg, but that doesn't make sense for 
> > the builtin.
> > 
> > Should I change its signature in `Builtins.def`?
> It's also marked `t`, which means "signature is meaningless, uses custom type 
> checking".
> 
> The argument to `__builtin_constant_p` isn't required to be an ICE, though, 
> so we shouldn't be calling `SemaBuiltinConstantArg` for it / creating a 
> `ConstantExpr` wrapping it, as far as I can tell.
Right. I think I was looking at something else. I changed it.


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 173974.
void added a comment.

Fix overflow flag.


Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/Sema/builtins.c
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -122,6 +122,14 @@
  __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1283,9 +1283,6 @@
   break;
 
 case Expr::ConstantExprClass:
-  // Handled due to it being a wrapper class.
-  break;
-
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,10 +2233,6 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-// A file-scoped array must have a constant array size.
-ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
 Diag(Loc, diag::err_opencl_vla);
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -178,6 +178,8 @@
   while (true) {
 if (ImplicitCastExpr *IC = dyn_cast(E))
   E = IC->getSubExpr();
+else if (ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
@@ -5225,6 +5227,8 @@
   while (true) {
 if (const Impl

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 173986.
void added a comment.

ImpCast.


Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/Sema/builtins.c
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -122,6 +122,14 @@
  __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1283,9 +1283,6 @@
   break;
 
 case Expr::ConstantExprClass:
-  // Handled due to it being a wrapper class.
-  break;
-
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,10 +2233,6 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-// A file-scoped array must have a constant array size.
-ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
 Diag(Loc, diag::err_opencl_vla);
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -178,6 +178,8 @@
   while (true) {
 if (ImplicitCastExpr *IC = dyn_cast(E))
   E = IC->getSubExpr();
+else if (ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
@@ -5225,6 +5227,8 @@
   while (true) {
 if (const ImplicitCastEx

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Ping? :-)


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Ping? I really don't want this review to go on forever.


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 174529.
void added a comment.

Don't re-wrap a ConstExpr.


Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/Sema/builtins.c
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -122,6 +122,14 @@
  __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1283,9 +1283,6 @@
   break;
 
 case Expr::ConstantExprClass:
-  // Handled due to it being a wrapper class.
-  break;
-
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,10 +2233,6 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-// A file-scoped array must have a constant array size.
-ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
 Diag(Loc, diag::err_opencl_vla);
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -178,6 +178,8 @@
   while (true) {
 if (ImplicitCastExpr *IC = dyn_cast(E))
   E = IC->getSubExpr();
+else if (ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
@@ -5225,6 +5227,8 @@
   while (true) {
 if (co

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 174551.
void added a comment.

No function pointers


Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/Sema/builtins.c
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -122,6 +122,14 @@
  __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1283,9 +1283,6 @@
   break;
 
 case Expr::ConstantExprClass:
-  // Handled due to it being a wrapper class.
-  break;
-
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,10 +2233,6 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-// A file-scoped array must have a constant array size.
-ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
 Diag(Loc, diag::err_opencl_vla);
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -178,6 +178,8 @@
   while (true) {
 if (ImplicitCastExpr *IC = dyn_cast(E))
   E = IC->getSubExpr();
+else if (ConstantExpr *CE = dyn_cast(E))
+  E = CE->getSubExpr();
 else if (SubstNonTypeTemplateParmExpr *Subst =
dyn_cast(E))
   E = Subst->getReplacement();
@@ -5225,6 +5227,8 @@
   while (true) {
 if (const Im

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 174692.
void added a comment.

Remove ODR violation.


Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/Sema/builtins.c
  test/SemaCXX/compound-literal.cpp

Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -122,6 +122,14 @@
  __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
@@ -161,6 +169,7 @@
   F(&test17_d);
   F((struct Aggregate){0, 1});
   F((IntVector){0, 1, 2, 3});
+  F(test17);
 
   // Ensure that a technique used in glibc is handled correctly.
 #define OPT(...) (__builtin_constant_p(__VA_ARGS__) && strlen(__VA_ARGS__) < 4)
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1283,9 +1283,6 @@
   break;
 
 case Expr::ConstantExprClass:
-  // Handled due to it being a wrapper class.
-  break;
-
 case Stmt::ExprWithCleanupsClass:
   // Handled due to fully linearised CFG.
   break;
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -2233,10 +2233,6 @@
 T = Context.getConstantArrayType(T, ConstVal, ASM, Quals);
   }
 
-  if (ArraySize && !CurContext->isFunctionOrMethod())
-// A file-scoped array must have a constant array size.
-ArraySize = new (Context) ConstantExpr(ArraySize);
-
   // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
   if (getLangOpts().OpenCL && T->isVariableArrayType()) {
 Diag(Loc, diag::err_opencl_vla);
Index: lib/Sema/SemaTemplateDeduction.cpp
===
--- lib/Sema/SemaTemplateDeduction.cpp
+++ lib/Sema/SemaTemplateDeduction.cpp
@@ -178,6 +178,8 @@
   while (true) {
 if (ImplicitCastExpr *IC = dyn_cast(E))
   E = IC-

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Should be okay now. PTAL. :-)


Repository:
  rC Clang

https://reviews.llvm.org/D54355



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC347294: Use is.constant intrinsic for __builtin_constant_p 
(authored by void, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54355?vs=174692&id=174731#toc

Repository:
  rC Clang

https://reviews.llvm.org/D54355

Files:
  include/clang/AST/Expr.h
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaType.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/Analysis/builtin-functions.cpp
  test/Sema/builtins.c
  test/SemaCXX/compound-literal.cpp

Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -901,10 +901,15 @@
 
 /// ConstantExpr - An expression that occurs in a constant context.
 class ConstantExpr : public FullExpr {
-public:
   ConstantExpr(Expr *subexpr)
 : FullExpr(ConstantExprClass, subexpr) {}
 
+public:
+  static ConstantExpr *Create(const ASTContext &Context, Expr *E) {
+assert(!isa(E));
+return new (Context) ConstantExpr(E);
+  }
+
   /// Build an empty constant expression wrapper.
   explicit ConstantExpr(EmptyShell Empty)
 : FullExpr(ConstantExprClass, Empty) {}
@@ -3091,8 +3096,8 @@
   while (true)
 if (ImplicitCastExpr *ice = dyn_cast(e))
   e = ice->getSubExpr();
-else if (ConstantExpr *ce = dyn_cast(e))
-  e = ce->getSubExpr();
+else if (FullExpr *fe = dyn_cast(e))
+  e = fe->getSubExpr();
 else
   break;
   return e;
Index: test/SemaCXX/compound-literal.cpp
===
--- test/SemaCXX/compound-literal.cpp
+++ test/SemaCXX/compound-literal.cpp
@@ -36,8 +36,8 @@
 
   POD p = (POD){1, 2};
   // CHECK-NOT: CXXBindTemporaryExpr {{.*}} 'brace_initializers::POD'
-  // CHECK: ConstantExpr {{.*}} 'brace_initializers::POD'
-  // CHECK-NEXT: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK: CompoundLiteralExpr {{.*}} 'brace_initializers::POD'
+  // CHECK-NEXT: ConstantExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: InitListExpr {{.*}} 'brace_initializers::POD'
   // CHECK-NEXT: IntegerLiteral {{.*}} 1{{$}}
   // CHECK-NEXT: IntegerLiteral {{.*}} 2{{$}}
Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -122,6 +122,14 @@
  __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
@@ -161,6 +169,7 @@
   F(&test17_d);
   F((struct Aggregate){0, 1});
   F((IntVector){0, 1, 2, 3});
+  F(test17);
 
   // Ensure that a technique used in glibc is handled correctly.
 #define OPT(...) (__builtin_constant_p(__VA_ARGS__) && strlen(__VA_ARGS__) < 4)
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/CodeGen/CGBuiltin.cpp
==

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Doh! I'll take a look at it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54355



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Fixed in r347531.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54355



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D54355#1326494 , @craig.topper 
wrote:

> I'm seeing a case where an inline assembly 'n' constraint is behaving 
> differently in -O0 now. Previously we would evaluate the __builtin_constant_p 
> as part of the EvaluateAsInt call in CodeGenFunction::EmitAsmInput, but I 
> think we're not doing that now. This causes us to fail later because the 
> constraint wasn't satisfied since we were in -O0 and thus we didn't optimize 
> the rest of the expression. The test case we have is a nasty mix of 
> __builtin_constant_p, __builtin_choose_expr, and __builtin_classify_type. I 
> can see about sharing it if needed.


`__builtin_constant_p()` is known to have different semantics at `-O0` than it 
does during optimizations. That said, we shouldn't be intentionally generating 
incorrect code. Could you test it against gcc to see if it has the same 
semantics? Otherwise, please add a testcase. :-)


Repository:
  rC Clang

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

https://reviews.llvm.org/D54355



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

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

My opinion doesn't carry as much weight as others who are more familiar with 
the front-end code, but LGTM.

One question, the code you added looks similar. Is there a way to extrapolate 
it into its own function? Maybe yet another `EvaluateAs*` method?




Comment at: clang/lib/Sema/SemaStmtAsm.cpp:389
+
+// For compatibility with GCC, we also allows pointers that would be
+// integral constant expressions if they were cast to int.

s/allows/allow/


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

https://reviews.llvm.org/D58821



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.



  ; ModuleID = 'arch_static_branch.bc'
  source_filename = "arch/x86/entry/vsyscall/vsyscall_64.c"
  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
  target triple = "x86_64-grtev4-linux-gnu"
  
  %struct.atomic_t = type { i32 }
  %struct.jump_entry = type { i64, i64, i64 }
  %struct.tracepoint_func = type { i8*, i8*, i32 }
  %struct.static_key = type { %struct.atomic_t, %union.anon }
  %union.anon = type { i64 }
  
  @__tracepoint_emulate_vsyscall = external hidden global { i8*, { 
%struct.atomic_t, { %struct.jump_entry* } }, i32 ()*, void ()*, 
%struct.tracepoint_func* }, section "__tracepoints", align 8
  
  ; Function Attrs: alwaysinline noredzone nounwind sspstrong
  define zeroext i1 @arch_static_branch(%struct.static_key* nocapture readnone 
%key, i1 zeroext %branch) {
  entry:
callbr void asm sideeffect "1:.byte 0x0f,0x1f,0x44,0x00,0\0A\09.pushsection 
__jump_table,  \22aw\22 \0A\09 .balign 8 \0A\09 .quad 1b, ${2:l}, ${0:c} + 
${1:c} \0A\09.popsection \0A\09", 
"i,i,X,~{dirflag},~{fpsr},~{flags}"(%struct.static_key* bitcast (i32* 
getelementptr inbounds ({ i8*, { %struct.atomic_t, { %struct.jump_entry* } }, 
i32 ()*, void ()*, %struct.tracepoint_func* }, { i8*, { %struct.atomic_t, { 
%struct.j
  ump_entry* } }, i32 ()*, void ()*, %struct.tracepoint_func* }* 
@__tracepoint_emulate_vsyscall, i64 0, i32 1, i32 0, i32 0) to 
%struct.static_key*), i1 false, i8* blockaddress(@arch_static_branch, %return))
to label %asm.fallthrough [label %return]
  
  asm.fallthrough:  ; preds = %entry
call void asm sideeffect "", "~{dirflag},~{fpsr},~{flags}"()
br label %return
  
  return:   ; preds = %asm.fallthrough, 
%entry
%retval.0 = phi i1 [ false, %asm.fallthrough ], [ true, %entry ]
ret i1 %retval.0
  }


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

https://reviews.llvm.org/D56571



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

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D56571#1467358 , @craig.topper 
wrote:

> I think things don't work right unless you disable the integrated assembler. 
> I'm not sure why though.


Using `-no-integrated-as` does allow it to compile, but it doesn't link (with 
ld.lld) if LTO is specified. Is there an equivalent flag / plugin-opt for lld?


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

https://reviews.llvm.org/D56571



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

[PATCH] D60943: Delay diagnosing "n" constraint until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

An inline asm call may result in an immediate input value after inlining.
Therefore, don't emit a diagnostic here if the input isn't an immediate.


Repository:
  rC Clang

https://reviews.llvm.org/D60943

Files:
  lib/CodeGen/CGStmt.cpp
  lib/Sema/SemaStmtAsm.cpp


Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -381,7 +381,8 @@
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
 Expr::EvalResult EVResult;
-if (!InputExpr->EvaluateAsRValue(EVResult, Context, true))
+if (!InputExpr->EvaluateAsRValue(EVResult, Context, true) &&
+Literal->getString() != "n")
   return StmtError(
   Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
   << Info.getConstraintStr() << InputExpr->getSourceRange());
@@ -395,7 +396,7 @@
   Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
   << Info.getConstraintStr() << InputExpr->getSourceRange());
 
-if (!Info.isValidAsmImmediate(IntResult))
+if (!Info.isValidAsmImmediate(IntResult) && Literal->getString() != 
"n")
   return StmtError(Diag(InputExpr->getBeginLoc(),
 diag::err_invalid_asm_value_for_constraint)
<< IntResult.toString(10) << Info.getConstraintStr()
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1837,7 +1837,8 @@
   // If this can't be a register or memory, i.e., has to be a constant
   // (immediate or symbolic), try to emit it as such.
   if (!Info.allowsRegister() && !Info.allowsMemory()) {
-if (Info.requiresImmediateConstant()) {
+// We can delay diagnosing the "n" constraint until after inlining.
+if (Info.requiresImmediateConstant() && ConstraintStr != "n") {
   Expr::EvalResult EVResult;
   InputExpr->EvaluateAsRValue(EVResult, getContext(), true);
 


Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -381,7 +381,8 @@
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
 Expr::EvalResult EVResult;
-if (!InputExpr->EvaluateAsRValue(EVResult, Context, true))
+if (!InputExpr->EvaluateAsRValue(EVResult, Context, true) &&
+Literal->getString() != "n")
   return StmtError(
   Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
   << Info.getConstraintStr() << InputExpr->getSourceRange());
@@ -395,7 +396,7 @@
   Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
   << Info.getConstraintStr() << InputExpr->getSourceRange());
 
-if (!Info.isValidAsmImmediate(IntResult))
+if (!Info.isValidAsmImmediate(IntResult) && Literal->getString() != "n")
   return StmtError(Diag(InputExpr->getBeginLoc(),
 diag::err_invalid_asm_value_for_constraint)
<< IntResult.toString(10) << Info.getConstraintStr()
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1837,7 +1837,8 @@
   // If this can't be a register or memory, i.e., has to be a constant
   // (immediate or symbolic), try to emit it as such.
   if (!Info.allowsRegister() && !Info.allowsMemory()) {
-if (Info.requiresImmediateConstant()) {
+// We can delay diagnosing the "n" constraint until after inlining.
+if (Info.requiresImmediateConstant() && ConstraintStr != "n") {
   Expr::EvalResult EVResult;
   InputExpr->EvaluateAsRValue(EVResult, getContext(), true);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60943: Delay diagnosing "n" constraint until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 195992.
void added a comment.

Put constraint string check in the correct place.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943

Files:
  lib/CodeGen/CGStmt.cpp
  lib/Sema/SemaStmtAsm.cpp


Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -379,7 +379,7 @@
  << Info.getConstraintStr()
  << InputExpr->getSourceRange());
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
-  if (!InputExpr->isValueDependent()) {
+  if (!InputExpr->isValueDependent() && Literal->getString() != "n") {
 Expr::EvalResult EVResult;
 if (!InputExpr->EvaluateAsRValue(EVResult, Context, true))
   return StmtError(
@@ -401,7 +401,6 @@
<< IntResult.toString(10) << Info.getConstraintStr()
<< InputExpr->getSourceRange());
   }
-
 } else {
   ExprResult Result = DefaultFunctionArrayLvalueConversion(Exprs[i]);
   if (Result.isInvalid())
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1842,11 +1842,13 @@
   InputExpr->EvaluateAsRValue(EVResult, getContext(), true);
 
   llvm::APSInt IntResult;
-  if (!EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
-   getContext()))
-llvm_unreachable("Invalid immediate constant!");
+  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
+  getContext()))
+return llvm::ConstantInt::get(getLLVMContext(), IntResult);
 
-  return llvm::ConstantInt::get(getLLVMContext(), IntResult);
+  if (Info.getConstraintStr() != "n")
+// We can delay diagnosing the "n" constraint until after inlining.
+llvm_unreachable("Invalid immediate constant!");
 }
 
 Expr::EvalResult Result;


Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -379,7 +379,7 @@
  << Info.getConstraintStr()
  << InputExpr->getSourceRange());
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
-  if (!InputExpr->isValueDependent()) {
+  if (!InputExpr->isValueDependent() && Literal->getString() != "n") {
 Expr::EvalResult EVResult;
 if (!InputExpr->EvaluateAsRValue(EVResult, Context, true))
   return StmtError(
@@ -401,7 +401,6 @@
<< IntResult.toString(10) << Info.getConstraintStr()
<< InputExpr->getSourceRange());
   }
-
 } else {
   ExprResult Result = DefaultFunctionArrayLvalueConversion(Exprs[i]);
   if (Result.isInvalid())
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1842,11 +1842,13 @@
   InputExpr->EvaluateAsRValue(EVResult, getContext(), true);
 
   llvm::APSInt IntResult;
-  if (!EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
-   getContext()))
-llvm_unreachable("Invalid immediate constant!");
+  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
+  getContext()))
+return llvm::ConstantInt::get(getLLVMContext(), IntResult);
 
-  return llvm::ConstantInt::get(getLLVMContext(), IntResult);
+  if (Info.getConstraintStr() != "n")
+// We can delay diagnosing the "n" constraint until after inlining.
+llvm_unreachable("Invalid immediate constant!");
 }
 
 Expr::EvalResult Result;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60943: Delay diagnosing "n" constraint until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 195994.
void added a comment.
Herald added a subscriber: eraman.

Fix test. Use "e" instead of "n".


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943

Files:
  lib/CodeGen/CGStmt.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/Sema/inline-asm-validate-x86.c


Index: test/Sema/inline-asm-validate-x86.c
===
--- test/Sema/inline-asm-validate-x86.c
+++ test/Sema/inline-asm-validate-x86.c
@@ -147,9 +147,9 @@
   // This offset-from-null pointer can be used as an integer constant 
expression.
   __asm__ __volatile__("\n#define S_B abcd%0\n" : : "n"(&((struct s*)0)->b));
   // This pointer cannot be used as an integer constant expression.
-  __asm__ __volatile__("\n#define GLOBAL_A abcd%0\n" : : "n"(&s.a)); // 
expected-error{{constraint 'n' expects an integer constant expression}}
+  __asm__ __volatile__("\n#define GLOBAL_A abcd%0\n" : : "e"(&s.a)); // 
expected-error{{constraint 'e' expects an integer constant expression}}
   // Floating-point is also not okay.
-  __asm__ __volatile__("\n#define PI abcd%0\n" : : "n"(3.14f)); // 
expected-error{{constraint 'n' expects an integer constant expression}}
+  __asm__ __volatile__("\n#define PI abcd%0\n" : : "e"(3.14f)); // 
expected-error{{constraint 'e' expects an integer constant expression}}
 #ifdef AMD64
   // This arbitrary pointer is fine.
   __asm__ __volatile__("\n#define BEEF abcd%0\n" : : 
"n"((int*)0xdeadbeef));
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -379,7 +379,7 @@
  << Info.getConstraintStr()
  << InputExpr->getSourceRange());
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
-  if (!InputExpr->isValueDependent()) {
+  if (!InputExpr->isValueDependent() && Literal->getString() != "n") {
 Expr::EvalResult EVResult;
 if (!InputExpr->EvaluateAsRValue(EVResult, Context, true))
   return StmtError(
@@ -401,7 +401,6 @@
<< IntResult.toString(10) << Info.getConstraintStr()
<< InputExpr->getSourceRange());
   }
-
 } else {
   ExprResult Result = DefaultFunctionArrayLvalueConversion(Exprs[i]);
   if (Result.isInvalid())
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1842,11 +1842,13 @@
   InputExpr->EvaluateAsRValue(EVResult, getContext(), true);
 
   llvm::APSInt IntResult;
-  if (!EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
-   getContext()))
-llvm_unreachable("Invalid immediate constant!");
+  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
+  getContext()))
+return llvm::ConstantInt::get(getLLVMContext(), IntResult);
 
-  return llvm::ConstantInt::get(getLLVMContext(), IntResult);
+  if (Info.getConstraintStr() != "n")
+// We can delay diagnosing the "n" constraint until after inlining.
+llvm_unreachable("Invalid immediate constant!");
 }
 
 Expr::EvalResult Result;


Index: test/Sema/inline-asm-validate-x86.c
===
--- test/Sema/inline-asm-validate-x86.c
+++ test/Sema/inline-asm-validate-x86.c
@@ -147,9 +147,9 @@
   // This offset-from-null pointer can be used as an integer constant expression.
   __asm__ __volatile__("\n#define S_B abcd%0\n" : : "n"(&((struct s*)0)->b));
   // This pointer cannot be used as an integer constant expression.
-  __asm__ __volatile__("\n#define GLOBAL_A abcd%0\n" : : "n"(&s.a)); // expected-error{{constraint 'n' expects an integer constant expression}}
+  __asm__ __volatile__("\n#define GLOBAL_A abcd%0\n" : : "e"(&s.a)); // expected-error{{constraint 'e' expects an integer constant expression}}
   // Floating-point is also not okay.
-  __asm__ __volatile__("\n#define PI abcd%0\n" : : "n"(3.14f)); // expected-error{{constraint 'n' expects an integer constant expression}}
+  __asm__ __volatile__("\n#define PI abcd%0\n" : : "e"(3.14f)); // expected-error{{constraint 'e' expects an integer constant expression}}
 #ifdef AMD64
   // This arbitrary pointer is fine.
   __asm__ __volatile__("\n#define BEEF abcd%0\n" : : "n"((int*)0xdeadbeef));
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -379,7 +379,7 @@
  << Info.getConstraintStr()
  << InputExpr->getSourceRange());
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
-  if (!InputExpr

[PATCH] D60943: Delay diagnosing "n" constraint until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

This relies upon D60943 .


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943



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

[PATCH] D60943: Delay diagnosing "n" constraint until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D60943#1474091 , @joerg wrote:

> I'm in the process of testing this, but feedback will take a bit.
>
> On the more meaty parts of this change, I think further iterations will be 
> necessary in-tree to extend this to the other constraints.


Thanks, Joerg. I was wondering about other constraints. This can be easily 
extended to encompass them. I just didn't want to jump the gun and come up with 
a "fix" that wasn't necessary.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943



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

[PATCH] D60943: Delay diagnosing "n" constraint until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Here's the motivating bug report: https://bugs.llvm.org/show_bug.cgi?id=41027

In general, I agree with you that diagnostics shouldn't depend on optimization 
levels, but inline assembly subverts this paradigm because it was originally a 
gcc extension. :-( That said, I don't know if looking at the `always_inline` 
attribute would work. I'll let @joerg and others comment on that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943



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

[PATCH] D60943: Delay diagnosing "n" constraint until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D60943#1474926 , @rsmith wrote:

> In D60943#1474899 , @void wrote:
>
> > Here's the motivating bug report: 
> > https://bugs.llvm.org/show_bug.cgi?id=41027
>
>
> Thanks, that's illuminating. OK, if we want to support that code, then 
> there's really not much validation we can do in the frontend for constraints 
> that require immediate constants. Is `"n"` really special in this regard, or 
> is it just the first one that we've encountered?


I think it's just the first one we've encountered. There could be other 
constraints which require immediates, but they're probably target-specific. 
E.g. `e` and `K` for x86.

>> In general, I agree with you that diagnostics shouldn't depend on 
>> optimization levels, but inline assembly subverts this paradigm because it 
>> was originally a gcc extension. :-(
> 
> There is a question of how much weird stuff we should accept in the name of 
> GCC compatibility. In this case, the fact that the frontend rejects seems 
> like the tip of an iceberg: the code in the PR is also relying on the control 
> flow of the `&&` inside the `if` to be emitted in a very particular way (or 
> to be optimized to that form), and for dead-code elimination to be considered 
> and to actually fire, and probably more things besides. Do we at least still 
> produce a nice diagnostic if the value ends up not being a constant?

It does, but for the wrong line:

  [morbo@fawn:llvm] cat bad.c
  static __attribute__((always_inline)) void outl(unsigned port, unsigned data) 
{
asm volatile("outl %0,%w1" : : "a"(data), "n"(port));
  }
  
  void f(unsigned port) {
outl(port, 1);
  }
  [morbo@fawn:llvm] ./llvm.opt.install/bin/clang -c bad.c
  bad.c:2:16: error: constraint 'n' expects an integer constant expression
asm volatile("outl %0,%w1" : : "a"(data), "n"(port));
 ^
  1 error generated.

I'll need to modify the LLVM patch to output the correct location.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943



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

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void created this revision.
void added reviewers: jyknight, eli.friedman, resistor, janusz.
Herald added subscribers: cfe-commits, kristina.

A __builtin_constant_p may end up with a constant after inlining. Use
the is.constant intrinsic if we're unable to determine if it's a
constant.

See: https://reviews.llvm.org/D4276


Repository:
  rC Clang

https://reviews.llvm.org/D52854

Files:
  include/clang/AST/Expr.h
  include/clang/Sema/Sema.h
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaType.cpp
  test/Analysis/builtin-functions.cpp
  test/Sema/builtins.c

Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -122,6 +122,14 @@
  __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
@@ -168,14 +176,17 @@
   // a builtin.
   ASSERT(OPT("abc"));
   ASSERT(!OPT("abcd"));
+
   // In these cases, the strlen is non-constant, but the __builtin_constant_p
-  // is 0: the array size is not an ICE but is foldable.
-  ASSERT(!OPT(test17_c));// expected-warning {{folded}}
+  // is 0: the array size is not an ICE.
+  ASSERT(!OPT(test17_c));// no warning expected
+  ASSERT(!OPT((char*)test17_c)); // no warning expected
+  ASSERT(!OPT(test17_d));// no warning expected
+  ASSERT(!OPT((char*)test17_d)); // no warning expected
+
+  // These are foldable.
   ASSERT(!OPT(&test17_c[0]));// expected-warning {{folded}}
-  ASSERT(!OPT((char*)test17_c)); // expected-warning {{folded}}
-  ASSERT(!OPT(test17_d));// expected-warning {{folded}}
   ASSERT(!OPT(&test17_d[0]));// expected-warning {{folded}}
-  ASSERT(!OPT((char*)test17_d)); // expected-warning {{folded}}
 
 #undef OPT
 #undef T
@@ -287,3 +298,18 @@
   memcpy(buf, src, 11); // expected-warning{{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
   my_memcpy(buf, src, 11); // expected-warning{{'__builtin___memcpy_chk' will always overflow; destination buffer has size 10, but size argument is 11}}
 }
+
+size_t test24(int a) {
+  char x[__builtin_constant_p(a) ? -1 : 1]; // no warning expected
+  return strlen(x);
+}
+
+__attribute__((always_inline))
+size_t test25(int a) {
+  char x[__builtin_constant_p(a) ? 1 : -1]; // no warning expected
+  return strlen(x);
+}
+
+size_t test26() {
+  return test25(2);
+}
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -4435,6 +4435,11 @@
 checkNullabilityConsistency(S, SimplePointerKind::Array, DeclType.Loc);
   }
 
+  if (ArraySize && !D.isExpressionContext())
+// Mark all __builtin_constant_p() calls in an array size expression as
+// needing to be evaluated early.
+S.MarkBuiltinConstantPCannotDelayEvaluation(ArraySize);
+
   T = S.BuildArrayType(T, ASM, ArraySize, ATI.TypeQuals,
SourceRange(DeclType.Loc, DeclType.EndLoc), Name);
   break;
Index: lib/Sema/SemaExpr.cpp
=

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

This is related to https://reviews.llvm.org/D4276, which I believe has been 
deleted. However, I believe that a modified version of that patch will work.

The idea of this patch is *not* to be fully compatible with gcc's 
implementation, but to expand clang's implementation of `__builtin_constant_p` 
without breaking existing code.

This has been tested by successfully building (and running) the Linux kernel 
for both ARM and X86.


Repository:
  rC Clang

https://reviews.llvm.org/D52854



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

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 168356.
void added a comment.

Removed unused field.


https://reviews.llvm.org/D52854

Files:
  include/clang/AST/Expr.h
  include/clang/Sema/Sema.h
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaType.cpp
  test/Analysis/builtin-functions.cpp
  test/Sema/builtins.c

Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -122,6 +122,14 @@
  __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
@@ -168,14 +176,17 @@
   // a builtin.
   ASSERT(OPT("abc"));
   ASSERT(!OPT("abcd"));
+
   // In these cases, the strlen is non-constant, but the __builtin_constant_p
-  // is 0: the array size is not an ICE but is foldable.
-  ASSERT(!OPT(test17_c));// expected-warning {{folded}}
+  // is 0: the array size is not an ICE.
+  ASSERT(!OPT(test17_c));// no warning expected
+  ASSERT(!OPT((char*)test17_c)); // no warning expected
+  ASSERT(!OPT(test17_d));// no warning expected
+  ASSERT(!OPT((char*)test17_d)); // no warning expected
+
+  // These are foldable.
   ASSERT(!OPT(&test17_c[0]));// expected-warning {{folded}}
-  ASSERT(!OPT((char*)test17_c)); // expected-warning {{folded}}
-  ASSERT(!OPT(test17_d));// expected-warning {{folded}}
   ASSERT(!OPT(&test17_d[0]));// expected-warning {{folded}}
-  ASSERT(!OPT((char*)test17_d)); // expected-warning {{folded}}
 
 #undef OPT
 #undef T
@@ -287,3 +298,18 @@
   memcpy(buf, src, 11); // expected-warning{{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
   my_memcpy(buf, src, 11); // expected-warning{{'__builtin___memcpy_chk' will always overflow; destination buffer has size 10, but size argument is 11}}
 }
+
+size_t test24(int a) {
+  char x[__builtin_constant_p(a) ? -1 : 1]; // no warning expected
+  return strlen(x);
+}
+
+__attribute__((always_inline))
+size_t test25(int a) {
+  char x[__builtin_constant_p(a) ? 1 : -1]; // no warning expected
+  return strlen(x);
+}
+
+size_t test26() {
+  return test25(2);
+}
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -4435,6 +4435,11 @@
 checkNullabilityConsistency(S, SimplePointerKind::Array, DeclType.Loc);
   }
 
+  if (ArraySize && !D.isExpressionContext())
+// Mark all __builtin_constant_p() calls in an array size expression as
+// needing to be evaluated early.
+S.MarkBuiltinConstantPCannotDelayEvaluation(ArraySize);
+
   T = S.BuildArrayType(T, ASM, ArraySize, ATI.TypeQuals,
SourceRange(DeclType.Loc, DeclType.EndLoc), Name);
   break;
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5691,6 +5691,7 @@
   if (!TInfo)
 TInfo = Context.getTrivialTypeSourceInfo(literalType);
 
+  MarkBuiltinConstantPCannotDelayEvaluation(InitExpr);
   return BuildCompoundLiteralExpr(LParenL

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:15621-15623
+for (InitListExpr::iterator II = E->begin(), IE = E->end();
+ II != IE; ++II)
+  Visit(*II);

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > `std::for_each`?
> Sorry, posted that comment as you uploaded the next version.  This should be 
> highlighting L15618 to L15620.
That's no used anywhere else in the source code. Maybe there's another 
mechanism that they use?


https://reviews.llvm.org/D52854



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

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void marked 9 inline comments as done.
void added a comment.

In https://reviews.llvm.org/D52854#1255755, @rsmith wrote:

> Essentially, you would add a `ConstExpr` node (or similar), teach 
> `IgnoreImplicit` and friends to step over it


I've been trying to do this to no avail. Here's the code I was working with:

  
//===--===//
  // Wrapper Expressions.
  
//===--===//
  
  // ConstExpr is a wrapper class indicating that the expression must be 
constant.
  // This is useful for something like __builtin_constant_p(), which in some
  // contexts is required to be constant before generating LLVM IR.
  template 
  class ConstExpr : public Expr {
T *E;
  
   public:
ConstExpr(Expr *E) : E(E) {}
  
T *getSubExpr() { return E; }
const T *getSubExpr() const { return E; }
  
static bool classof(const Stmt *T) {
  return T->getStmtClass() == ConstExprClass;
}
  };

I did it this way because otherwise I would have to forward all calls to 
individual expression, which is ugly and made me want to cry. The issue is 
getting the `ConstExprClass` variable. Those variables are automatically 
generated via macro magick, but those macros aren't set up to handle templates.

Do I need to mangle the macros to support templates, or is there another way to 
achieve this?




Comment at: lib/AST/ExprConstant.cpp:8162
+  return Success(true, E);
+if (isa(Arg->IgnoreParenCasts()) &&
+E->getCanDelayEvaluation())

rsmith wrote:
> rsmith wrote:
> > Your `canDelayEvaluation` check does not appear to cover several of the 
> > cases we'd need to check for here. Eg:
> > 
> > ```
> > void f(int n) {
> >   enum E { a = __builtin_constant_p(n) }; // ok, a == 0, not an error 
> > because a's value is non-constant
> > ```
> > 
> > 
> Hmm, OK. Per https://bugs.llvm.org/show_bug.cgi?id=4898#c38, the correct 
> check would be whether the reason we found the expression to be non-constant 
> was that it tried to read the value of a local variable or function 
> parameter. Eg, for:
> 
> ```
> void f(int x, int y) {
>   bool k = __builtin_constant_p(3 + x < 5 * y);
> ```
> 
> ... we should defer the `__builtin_constant_p` until after optimization. (And 
> we should never do this if the expression has side-effects.) But given that 
> your goal is merely to improve the status quo, not to exactly match GCC, this 
> may be sufficient.
> 
> You should at least check that the variable is non-volatile, though. (More 
> generally, check that `Arg` doesn't have side-effects.)
I had the side effect check in there before. I removed it for some reason. I'll 
re-add it.



Comment at: lib/Sema/SemaExpr.cpp:5694
 
+  MarkBuiltinConstantPCannotDelayEvaluation(InitExpr);
   return BuildCompoundLiteralExpr(LParenLoc, TInfo, RParenLoc, InitExpr);

rsmith wrote:
> Why are you marking this? This isn't (necessarily) a context where a constant 
> expression is required. (In C, the overall initializer for a global would be, 
> but a local-scope compound literal would not.)
> 
> Also, this should be in the `Build` function, not in the `ActOn` function.
The name may be misleading. It's looking to see if it should mark whether it 
can delay evaluation or not. I'll change its name to be clearer.



Comment at: lib/Sema/SemaExpr.cpp:5799
   E->setType(Context.VoidTy); // FIXME: just a place holder for now.
+  MarkBuiltinConstantPCannotDelayEvaluation(E);
   return E;

rsmith wrote:
> Likewise here, I don't see the justification for this.
I can't find an equivalent "Build..." for `InitList` expressions. Could you 
point me to it?


https://reviews.llvm.org/D52854



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

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 168395.
void marked an inline comment as done.

Repository:
  rC Clang

https://reviews.llvm.org/D52854

Files:
  include/clang/AST/Expr.h
  include/clang/Sema/Sema.h
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaType.cpp
  test/Analysis/builtin-functions.cpp
  test/Sema/builtins.c

Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -122,6 +122,14 @@
  __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants such as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
@@ -168,14 +176,17 @@
   // a builtin.
   ASSERT(OPT("abc"));
   ASSERT(!OPT("abcd"));
+
   // In these cases, the strlen is non-constant, but the __builtin_constant_p
-  // is 0: the array size is not an ICE but is foldable.
-  ASSERT(!OPT(test17_c));// expected-warning {{folded}}
+  // is 0: the array size is not an ICE.
+  ASSERT(!OPT(test17_c));// no warning expected
+  ASSERT(!OPT((char*)test17_c)); // no warning expected
+  ASSERT(!OPT(test17_d));// no warning expected
+  ASSERT(!OPT((char*)test17_d)); // no warning expected
+
+  // These are foldable.
   ASSERT(!OPT(&test17_c[0]));// expected-warning {{folded}}
-  ASSERT(!OPT((char*)test17_c)); // expected-warning {{folded}}
-  ASSERT(!OPT(test17_d));// expected-warning {{folded}}
   ASSERT(!OPT(&test17_d[0]));// expected-warning {{folded}}
-  ASSERT(!OPT((char*)test17_d)); // expected-warning {{folded}}
 
 #undef OPT
 #undef T
@@ -287,3 +298,18 @@
   memcpy(buf, src, 11); // expected-warning{{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
   my_memcpy(buf, src, 11); // expected-warning{{'__builtin___memcpy_chk' will always overflow; destination buffer has size 10, but size argument is 11}}
 }
+
+size_t test24(int a) {
+  char x[__builtin_constant_p(a) ? -1 : 1]; // no warning expected
+  return strlen(x);
+}
+
+__attribute__((always_inline))
+size_t test25(int a) {
+  char x[__builtin_constant_p(a) ? 1 : -1]; // no warning expected
+  return strlen(x);
+}
+
+size_t test26() {
+  return test25(2);
+}
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -4435,6 +4435,11 @@
 checkNullabilityConsistency(S, SimplePointerKind::Array, DeclType.Loc);
   }
 
+  if (ArraySize && !D.isExpressionContext())
+// Mark all __builtin_constant_p() calls in an array size expression as
+// needing to be evaluated early.
+S.MaybeMarkBuiltinConstantPCannotDelayEvaluation(ArraySize);
+
   T = S.BuildArrayType(T, ASM, ArraySize, ATI.TypeQuals,
SourceRange(DeclType.Loc, DeclType.EndLoc), Name);
   break;
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5767,9 +5767,10 @@
   ? VK_RValue
   : VK_LValue;
 
-  return MaybeBindToTemporary(
-  new (Context) CompoundLiteralExpr(LParenLoc, TInfo, literal

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

@rsmith I've been implementing your suggestion, but it's quickly becoming 
ridiculous. Just to implement a `ConstExpr` wrapper class, I need to touch ~23 
files, virtually all changes being boilerplate code to forward the action to 
the wrapped expression. And this is before I add the code in this patch that 
implements the feature. While it would be nice to use LLVM's type system to 
determine if an expression is expected to be constant, it appears that doing 
that is much worse than adding the information to the bits field you mentioned.


Repository:
  rC Clang

https://reviews.llvm.org/D52854



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

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

No, I understood that. But the issue is that you then need to specify this new 
expression class in over 23 different files: various macros, switch statements, 
etc.


Repository:
  rC Clang

https://reviews.llvm.org/D52854



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

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Here's a WIP patch to show what I'm talking about.F7375980: example.diff 



Repository:
  rC Clang

https://reviews.llvm.org/D52854



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

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 168708.
void added a comment.

I updated the patch that implements putting the "can delay evaluation" bit into 
the CallExprBitfields. PTAL.


Repository:
  rC Clang

https://reviews.llvm.org/D52854

Files:
  include/clang/AST/Expr.h
  include/clang/AST/Stmt.h
  include/clang/Sema/Sema.h
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseInit.cpp
  lib/Sema/SemaExpr.cpp
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/Analysis/builtin-functions.cpp
  test/Sema/builtins.c

Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -122,6 +122,14 @@
  __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants such as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
@@ -168,14 +176,17 @@
   // a builtin.
   ASSERT(OPT("abc"));
   ASSERT(!OPT("abcd"));
+
   // In these cases, the strlen is non-constant, but the __builtin_constant_p
-  // is 0: the array size is not an ICE but is foldable.
-  ASSERT(!OPT(test17_c));// expected-warning {{folded}}
+  // is 0: the array size is not an ICE.
+  ASSERT(!OPT(test17_c));// no warning expected
+  ASSERT(!OPT((char*)test17_c)); // no warning expected
+  ASSERT(!OPT(test17_d));// no warning expected
+  ASSERT(!OPT((char*)test17_d)); // no warning expected
+
+  // These are foldable.
   ASSERT(!OPT(&test17_c[0]));// expected-warning {{folded}}
-  ASSERT(!OPT((char*)test17_c)); // expected-warning {{folded}}
-  ASSERT(!OPT(test17_d));// expected-warning {{folded}}
   ASSERT(!OPT(&test17_d[0]));// expected-warning {{folded}}
-  ASSERT(!OPT((char*)test17_d)); // expected-warning {{folded}}
 
 #undef OPT
 #undef T
@@ -287,3 +298,22 @@
   memcpy(buf, src, 11); // expected-warning{{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
   my_memcpy(buf, src, 11); // expected-warning{{'__builtin___memcpy_chk' will always overflow; destination buffer has size 10, but size argument is 11}}
 }
+
+size_t test24(int a) {
+  char x[__builtin_constant_p(a) ? -1 : 1]; // no warning expected
+  return strlen(x);
+}
+
+__attribute__((always_inline))
+size_t test25(int a) {
+  char x[__builtin_constant_p(a) ? 1 : -1]; // no warning expected
+  return strlen(x);
+}
+
+size_t test26() {
+  return test25(2);
+}
+
+void f(int n) {
+  enum E { a = __builtin_constant_p(n) }; // ok, a == 0, not an error because a's value is non-constant
+}
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,7 +70,7 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
@@ -77,7 +77,7 @@
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -590,6 +590,7 @@
 void ASTStmtWriter::VisitCallExpr(CallExpr *E) {
   VisitExpr(E);
   Record.push_back(E->getNumArgs());
+  Record.push_back(E->getCanDelayEvaluation());
   Record.AddSourceLocation(E->getRParenLoc());
   Record.AddStmt(E->getCallee());
   for (CallExpr::arg_iterator Arg = E->arg_begin(), ArgEnd = E->arg_end();
Index: lib/Serialization/ASTReaderStmt.cpp

[PATCH] D52854: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 168712.

Repository:
  rC Clang

https://reviews.llvm.org/D52854

Files:
  include/clang/AST/Expr.h
  include/clang/AST/Stmt.h
  include/clang/Sema/Sema.h
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseInit.cpp
  lib/Sema/SemaExpr.cpp
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/Analysis/builtin-functions.cpp
  test/Sema/builtins.c

Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -122,6 +122,14 @@
  __builtin_constant_p(1, 2); // expected-error {{too many arguments}}
 }
 
+// __builtin_constant_p cannot resolve non-constants such as a file scoped array.
+int expr;
+char y[__builtin_constant_p(expr) ? -1 : 1]; // no warning, the builtin is false.
+
+// no warning, the builtin is false.
+struct foo { int a; };
+struct foo x = (struct foo) { __builtin_constant_p(42) ? 37 : 927 };
+
 const int test17_n = 0;
 const char test17_c[] = {1, 2, 3, 0};
 const char test17_d[] = {1, 2, 3, 4};
@@ -168,14 +176,17 @@
   // a builtin.
   ASSERT(OPT("abc"));
   ASSERT(!OPT("abcd"));
+
   // In these cases, the strlen is non-constant, but the __builtin_constant_p
-  // is 0: the array size is not an ICE but is foldable.
-  ASSERT(!OPT(test17_c));// expected-warning {{folded}}
+  // is 0: the array size is not an ICE.
+  ASSERT(!OPT(test17_c));// no warning expected
+  ASSERT(!OPT((char*)test17_c)); // no warning expected
+  ASSERT(!OPT(test17_d));// no warning expected
+  ASSERT(!OPT((char*)test17_d)); // no warning expected
+
+  // These are foldable.
   ASSERT(!OPT(&test17_c[0]));// expected-warning {{folded}}
-  ASSERT(!OPT((char*)test17_c)); // expected-warning {{folded}}
-  ASSERT(!OPT(test17_d));// expected-warning {{folded}}
   ASSERT(!OPT(&test17_d[0]));// expected-warning {{folded}}
-  ASSERT(!OPT((char*)test17_d)); // expected-warning {{folded}}
 
 #undef OPT
 #undef T
@@ -287,3 +298,22 @@
   memcpy(buf, src, 11); // expected-warning{{'memcpy' will always overflow; destination buffer has size 10, but size argument is 11}}
   my_memcpy(buf, src, 11); // expected-warning{{'__builtin___memcpy_chk' will always overflow; destination buffer has size 10, but size argument is 11}}
 }
+
+size_t test24(int a) {
+  char x[__builtin_constant_p(a) ? -1 : 1]; // no warning expected
+  return strlen(x);
+}
+
+__attribute__((always_inline))
+size_t test25(int a) {
+  char x[__builtin_constant_p(a) ? 1 : -1]; // no warning expected
+  return strlen(x);
+}
+
+size_t test26() {
+  return test25(2);
+}
+
+void f(int n) {
+  enum E { a = __builtin_constant_p(n) }; // ok, a == 0, not an error because a's value is non-constant
+}
Index: test/Analysis/builtin-functions.cpp
===
--- test/Analysis/builtin-functions.cpp
+++ test/Analysis/builtin-functions.cpp
@@ -70,14 +70,14 @@
   const int j = 2;
   constexpr int k = 3;
   clang_analyzer_eval(__builtin_constant_p(42) == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(i) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(j) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(i + 42) == 0); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(j + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(k + 42) == 1); // expected-warning {{TRUE}}
   clang_analyzer_eval(__builtin_constant_p(" ") == 1); // expected-warning {{TRUE}}
-  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{TRUE}}
+  clang_analyzer_eval(__builtin_constant_p(test_constant_p) == 0); // expected-warning {{UNKNOWN}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 0); // expected-warning {{FALSE}}
   clang_analyzer_eval(__builtin_constant_p(k - 3) == 1); // expected-warning {{TRUE}}
 }
Index: lib/Serialization/ASTWriterStmt.cpp
===
--- lib/Serialization/ASTWriterStmt.cpp
+++ lib/Serialization/ASTWriterStmt.cpp
@@ -590,6 +590,7 @@
 void ASTStmtWriter::VisitCallExpr(CallExpr *E) {
   VisitExpr(E);
   Record.push_back(E->getNumArgs());
+  Record.push_back(E->getCanDelayEvaluation());
   Record.AddSourceLocation(E->getRParenLoc());
   Record.AddStmt(E->getCallee());
   for (CallExpr::arg_iterator Arg = E->arg_begin(), ArgEnd = E->arg_end();
Index: lib/Serialization/ASTReaderStmt.cpp
===
--- lib/Serialization/ASTReaderStmt.cpp
+++ lib/Serialization/ASTReaderStmt.cpp
@@ -679,6 +679,7 @@
 void ASTStmtReader::VisitCall

[PATCH] D57267: [AST] Factor out the logic of the various Expr::Ignore*

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D57267#1390484 , @riccibruno wrote:

> @void @rnk Perhaps you can comment on this: currently `Expr::IgnoreImpCasts` 
> skips `FullExpr`s, but `Expr::IgnoreParenImpCasts` only skips (via 
> `IgnoreParens`) `ConstantExpr`s. Is there any reason for this inconsistency ? 
> I would like to add `FullExpr` to the nodes skipped by `IgnoreParenImpCasts` 
> for consistency but I am worried about unexpected issues even though all 
> tests pass.


I don't think there was an explicit reason beyond "I didn't need to do it at 
the time". So probably just an oversight on my part. I don't know the code 
nearly as well as @rnk, so I could be wrong, but I think the existing tests 
should tell you if something went haywire if you skip `FullExpr`s.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57267



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

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

That was intentional. We want to do this for more than just the `n` constraint.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943



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

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 212072.
void added a comment.

Save checking of immediates until code generation.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943

Files:
  lib/CodeGen/CGStmt.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/Sema/inline-asm-validate-x86.c


Index: test/Sema/inline-asm-validate-x86.c
===
--- test/Sema/inline-asm-validate-x86.c
+++ test/Sema/inline-asm-validate-x86.c
@@ -147,9 +147,9 @@
   // This offset-from-null pointer can be used as an integer constant 
expression.
   __asm__ __volatile__("\n#define S_B abcd%0\n" : : "n"(&((struct s*)0)->b));
   // This pointer cannot be used as an integer constant expression.
-  __asm__ __volatile__("\n#define GLOBAL_A abcd%0\n" : : "n"(&s.a)); // 
expected-error{{constraint 'n' expects an integer constant expression}}
+  __asm__ __volatile__("\n#define GLOBAL_A abcd%0\n" : : "e"(&s.a)); // 
expected-error{{constraint 'e' expects an integer constant expression}}
   // Floating-point is also not okay.
-  __asm__ __volatile__("\n#define PI abcd%0\n" : : "n"(3.14f)); // 
expected-error{{constraint 'n' expects an integer constant expression}}
+  __asm__ __volatile__("\n#define PI abcd%0\n" : : "e"(3.14f)); // 
expected-error{{constraint 'e' expects an integer constant expression}}
 #ifdef AMD64
   // This arbitrary pointer is fine.
   __asm__ __volatile__("\n#define BEEF abcd%0\n" : : 
"n"((int*)0xdeadbeef));
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -383,25 +383,19 @@
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
 Expr::EvalResult EVResult;
-if (!InputExpr->EvaluateAsRValue(EVResult, Context, true))
-  return StmtError(
-  Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
-  << Info.getConstraintStr() << InputExpr->getSourceRange());
-
-// For compatibility with GCC, we also allow pointers that would be
-// integral constant expressions if they were cast to int.
-llvm::APSInt IntResult;
-if (!EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
- Context))
-  return StmtError(
-  Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
-  << Info.getConstraintStr() << InputExpr->getSourceRange());
-
-if (!Info.isValidAsmImmediate(IntResult))
-  return StmtError(Diag(InputExpr->getBeginLoc(),
-diag::err_invalid_asm_value_for_constraint)
-   << IntResult.toString(10) << Info.getConstraintStr()
-   << InputExpr->getSourceRange());
+if (InputExpr->EvaluateAsRValue(EVResult, Context, true)) {
+  // For compatibility with GCC, we also allow pointers that would be
+  // integral constant expressions if they were cast to int.
+  llvm::APSInt IntResult;
+  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
+   Context))
+if (!Info.isValidAsmImmediate(IntResult))
+  return StmtError(Diag(InputExpr->getBeginLoc(),
+diag::err_invalid_asm_value_for_constraint)
+   << IntResult.toString(10)
+   << Info.getConstraintStr()
+   << InputExpr->getSourceRange());
+}
   }
 
 } else {
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1846,11 +1846,9 @@
   InputExpr->EvaluateAsRValue(EVResult, getContext(), true);
 
   llvm::APSInt IntResult;
-  if (!EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
-   getContext()))
-llvm_unreachable("Invalid immediate constant!");
-
-  return llvm::ConstantInt::get(getLLVMContext(), IntResult);
+  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
+  getContext()))
+return llvm::ConstantInt::get(getLLVMContext(), IntResult);
 }
 
 Expr::EvalResult Result;


Index: test/Sema/inline-asm-validate-x86.c
===
--- test/Sema/inline-asm-validate-x86.c
+++ test/Sema/inline-asm-validate-x86.c
@@ -147,9 +147,9 @@
   // This offset-from-null pointer can be used as an integer constant expression.
   __asm__ __volatile__("\n#define S_B abcd%0\n" : : "n"(&((struct s*)0)->b));
   // This pointer cannot be used as an integer constant expression.
-  __asm__ __vo

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 212079.
void added a comment.

Don't emit errors here. Wait until codegen.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943

Files:
  lib/CodeGen/CGStmt.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/Sema/inline-asm-validate-x86.c


Index: test/Sema/inline-asm-validate-x86.c
===
--- test/Sema/inline-asm-validate-x86.c
+++ test/Sema/inline-asm-validate-x86.c
@@ -147,9 +147,9 @@
   // This offset-from-null pointer can be used as an integer constant 
expression.
   __asm__ __volatile__("\n#define S_B abcd%0\n" : : "n"(&((struct s*)0)->b));
   // This pointer cannot be used as an integer constant expression.
-  __asm__ __volatile__("\n#define GLOBAL_A abcd%0\n" : : "n"(&s.a)); // 
expected-error{{constraint 'n' expects an integer constant expression}}
+  __asm__ __volatile__("\n#define GLOBAL_A abcd%0\n" : : "e"(&s.a)); // 
expected-error{{constraint 'e' expects an integer constant expression}}
   // Floating-point is also not okay.
-  __asm__ __volatile__("\n#define PI abcd%0\n" : : "n"(3.14f)); // 
expected-error{{constraint 'n' expects an integer constant expression}}
+  __asm__ __volatile__("\n#define PI abcd%0\n" : : "e"(3.14f)); // 
expected-error{{constraint 'e' expects an integer constant expression}}
 #ifdef AMD64
   // This arbitrary pointer is fine.
   __asm__ __volatile__("\n#define BEEF abcd%0\n" : : 
"n"((int*)0xdeadbeef));
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -383,25 +383,19 @@
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
 Expr::EvalResult EVResult;
-if (!InputExpr->EvaluateAsRValue(EVResult, Context, true))
-  return StmtError(
-  Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
-  << Info.getConstraintStr() << InputExpr->getSourceRange());
-
-// For compatibility with GCC, we also allow pointers that would be
-// integral constant expressions if they were cast to int.
-llvm::APSInt IntResult;
-if (!EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
- Context))
-  return StmtError(
-  Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
-  << Info.getConstraintStr() << InputExpr->getSourceRange());
-
-if (!Info.isValidAsmImmediate(IntResult))
-  return StmtError(Diag(InputExpr->getBeginLoc(),
-diag::err_invalid_asm_value_for_constraint)
-   << IntResult.toString(10) << Info.getConstraintStr()
-   << InputExpr->getSourceRange());
+if (InputExpr->EvaluateAsRValue(EVResult, Context, true)) {
+  // For compatibility with GCC, we also allow pointers that would be
+  // integral constant expressions if they were cast to int.
+  llvm::APSInt IntResult;
+  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
+   Context))
+if (!Info.isValidAsmImmediate(IntResult))
+  return StmtError(Diag(InputExpr->getBeginLoc(),
+diag::err_invalid_asm_value_for_constraint)
+   << IntResult.toString(10)
+   << Info.getConstraintStr()
+   << InputExpr->getSourceRange());
+}
   }
 
 } else {
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1846,11 +1846,9 @@
   InputExpr->EvaluateAsRValue(EVResult, getContext(), true);
 
   llvm::APSInt IntResult;
-  if (!EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
-   getContext()))
-llvm_unreachable("Invalid immediate constant!");
-
-  return llvm::ConstantInt::get(getLLVMContext(), IntResult);
+  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
+  getContext()))
+return llvm::ConstantInt::get(getLLVMContext(), IntResult);
 }
 
 Expr::EvalResult Result;


Index: test/Sema/inline-asm-validate-x86.c
===
--- test/Sema/inline-asm-validate-x86.c
+++ test/Sema/inline-asm-validate-x86.c
@@ -147,9 +147,9 @@
   // This offset-from-null pointer can be used as an integer constant expression.
   __asm__ __volatile__("\n#define S_B abcd%0\n" : : "n"(&((struct s*)0)->b));
   // This pointer cannot be used as an integer constant expression.
-  __asm__ __volatile_

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 212085.
void added a comment.
Herald added subscribers: s.egerton, jocewei, PkmX, the_o, brucehoult, 
MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, 
apazos, simoncook, johnrusso, rbar, asb.

Move a few more tests around. Some go to the LLVM side.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943

Files:
  lib/CodeGen/CGStmt.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/pr41027.c
  test/Sema/inline-asm-validate-riscv.c
  test/Sema/inline-asm-validate-x86.c
  test/Sema/pr41027.c

Index: test/Sema/pr41027.c
===
--- test/Sema/pr41027.c
+++ /dev/null
@@ -1,10 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64 -fsyntax-only %s
-// XFAIL: *
-
-inline void pr41027(unsigned a, unsigned b) {
-  if (__builtin_constant_p(a)) {
-__asm__ volatile("outl %0,%w1" : : "a"(b), "n"(a));
-  } else {
-__asm__ volatile("outl %0,%w1" : : "a"(b), "d"(a));
-  }
-}
Index: test/Sema/inline-asm-validate-x86.c
===
--- test/Sema/inline-asm-validate-x86.c
+++ test/Sema/inline-asm-validate-x86.c
@@ -4,9 +4,6 @@
 void I(int i, int j) {
   static const int BelowMin = -1;
   static const int AboveMax = 32;
-  __asm__("xorl %0,%2"
-  : "=r"(i)
-  : "0"(i), "I"(j)); // expected-error{{constraint 'I' expects an integer constant expression}}
   __asm__("xorl %0,%2"
   : "=r"(i)
   : "0"(i), "I"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'I'}}
@@ -21,9 +18,6 @@
 void J(int i, int j) {
   static const int BelowMin = -1;
   static const int AboveMax = 64;
-  __asm__("xorl %0,%2"
-  : "=r"(i)
-  : "0"(i), "J"(j)); // expected-error{{constraint 'J' expects an integer constant expression}}
   __asm__("xorl %0,%2"
   : "=r"(i)
   : "0"(i), "J"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'J'}}
@@ -38,9 +32,6 @@
 void K(int i, int j) {
   static const int BelowMin = -129;
   static const int AboveMax = 128;
-  __asm__("xorl %0,%2"
-  : "=r"(i)
-  : "0"(i), "K"(j)); // expected-error{{constraint 'K' expects an integer constant expression}}
   __asm__("xorl %0,%2"
   : "=r"(i)
   : "0"(i), "K"(BelowMin)); // expected-error{{value '-129' out of range for constraint 'K'}}
@@ -60,9 +51,6 @@
   static const int Valid1 = 0xff;
   static const int Valid2 = 0x;
   static const int Valid3 = 0x;
-  __asm__("xorl %0,%2"
-  : "=r"(i)
-  : "0"(i), "L"(j)); // expected-error{{constraint 'L' expects an integer constant expression}}
   __asm__("xorl %0,%2"
   : "=r"(i)
   : "0"(i), "L"(Invalid1)); // expected-error{{value '1' out of range for constraint 'L'}}
@@ -89,9 +77,6 @@
 void M(int i, int j) {
   static const int BelowMin = -1;
   static const int AboveMax = 4;
-  __asm__("xorl %0,%2"
-  : "=r"(i)
-  : "0"(i), "M"(j)); // expected-error{{constraint 'M' expects an integer constant expression}}
   __asm__("xorl %0,%2"
   : "=r"(i)
   : "0"(i), "M"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'M'}}
@@ -106,9 +91,6 @@
 void N(int i, int j) {
   static const int BelowMin = -1;
   static const int AboveMax = 256;
-  __asm__("xorl %0,%2"
-  : "=r"(i)
-  : "0"(i), "N"(j)); // expected-error{{constraint 'N' expects an integer constant expression}}
   __asm__("xorl %0,%2"
   : "=r"(i)
   : "0"(i), "N"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'N'}}
@@ -123,9 +105,6 @@
 void O(int i, int j) {
   static const int BelowMin = -1;
   static const int AboveMax = 128;
-  __asm__("xorl %0,%2"
-  : "=r"(i)
-  : "0"(i), "O"(j)); // expected-error{{constraint 'O' expects an integer constant expression}}
   __asm__("xorl %0,%2"
   : "=r"(i)
   : "0"(i), "O"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'O'}}
@@ -146,10 +125,6 @@
   __asm__ __volatile__("\n#define S_A abcd%0\n" : : "n"(&((struct s*)0)->a));
   // This offset-from-null pointer can be used as an integer constant expression.
   __asm__ __volatile__("\n#define S_B abcd%0\n" : : "n"(&((struct s*)0)->b));
-  // This pointer cannot be used as an integer constant expression.
-  __asm__ __volatile__("\n#define GLOBAL_A abcd%0\n" : : "n"(&s.a)); // expected-error{{constraint 'n' expects an integer constant expression}}
-  // Floating-point is also not okay.
-  __asm__ __volatile__("\n#define PI abcd%0\n" : : "n"(3.14f)); // expected-error{{constraint 'n' expects an integer constant expression}}
 #ifdef AMD64
   // This arbitrary pointer is fine.
   __asm__ __volatile__("\n#define BEEF abcd%0\n" : : "n"((int*)0xdeadbeef));
Index: test/Sema/inline-asm-validate-riscv.c
==

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Friendly ping. :-)


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943



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

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D60943#1612083 , @joerg wrote:

> The combination of D60942 , D06943 and 
> D65280  solves the problems for me on all 
> targets I have.


Excellent! I submitted D60942 . Go ahead and 
LGTM please. :-) Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943



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

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[Bill Wendling via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL368104: Delay diagnosing asm constraints that require 
immediates until after inlining (authored by void, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60943?vs=212085&id=213738#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60943

Files:
  cfe/trunk/lib/CodeGen/CGStmt.cpp
  cfe/trunk/lib/Sema/SemaStmtAsm.cpp
  cfe/trunk/test/CodeGen/pr41027.c
  cfe/trunk/test/Sema/inline-asm-validate-riscv.c
  cfe/trunk/test/Sema/inline-asm-validate-x86.c
  cfe/trunk/test/Sema/pr41027.c

Index: cfe/trunk/lib/CodeGen/CGStmt.cpp
===
--- cfe/trunk/lib/CodeGen/CGStmt.cpp
+++ cfe/trunk/lib/CodeGen/CGStmt.cpp
@@ -1846,11 +1846,9 @@
   InputExpr->EvaluateAsRValue(EVResult, getContext(), true);
 
   llvm::APSInt IntResult;
-  if (!EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
-   getContext()))
-llvm_unreachable("Invalid immediate constant!");
-
-  return llvm::ConstantInt::get(getLLVMContext(), IntResult);
+  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
+  getContext()))
+return llvm::ConstantInt::get(getLLVMContext(), IntResult);
 }
 
 Expr::EvalResult Result;
Index: cfe/trunk/lib/Sema/SemaStmtAsm.cpp
===
--- cfe/trunk/lib/Sema/SemaStmtAsm.cpp
+++ cfe/trunk/lib/Sema/SemaStmtAsm.cpp
@@ -383,25 +383,19 @@
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
 Expr::EvalResult EVResult;
-if (!InputExpr->EvaluateAsRValue(EVResult, Context, true))
-  return StmtError(
-  Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
-  << Info.getConstraintStr() << InputExpr->getSourceRange());
-
-// For compatibility with GCC, we also allow pointers that would be
-// integral constant expressions if they were cast to int.
-llvm::APSInt IntResult;
-if (!EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
- Context))
-  return StmtError(
-  Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
-  << Info.getConstraintStr() << InputExpr->getSourceRange());
-
-if (!Info.isValidAsmImmediate(IntResult))
-  return StmtError(Diag(InputExpr->getBeginLoc(),
-diag::err_invalid_asm_value_for_constraint)
-   << IntResult.toString(10) << Info.getConstraintStr()
-   << InputExpr->getSourceRange());
+if (InputExpr->EvaluateAsRValue(EVResult, Context, true)) {
+  // For compatibility with GCC, we also allow pointers that would be
+  // integral constant expressions if they were cast to int.
+  llvm::APSInt IntResult;
+  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
+   Context))
+if (!Info.isValidAsmImmediate(IntResult))
+  return StmtError(Diag(InputExpr->getBeginLoc(),
+diag::err_invalid_asm_value_for_constraint)
+   << IntResult.toString(10)
+   << Info.getConstraintStr()
+   << InputExpr->getSourceRange());
+}
   }
 
 } else {
Index: cfe/trunk/test/Sema/inline-asm-validate-riscv.c
===
--- cfe/trunk/test/Sema/inline-asm-validate-riscv.c
+++ cfe/trunk/test/Sema/inline-asm-validate-riscv.c
@@ -4,7 +4,6 @@
 void I(int i) {
   static const int BelowMin = -2049;
   static const int AboveMax = 2048;
-  asm volatile ("" :: "I"(i)); // expected-error{{constraint 'I' expects an integer constant expression}}
   asm volatile ("" :: "I"(BelowMin)); // expected-error{{value '-2049' out of range for constraint 'I'}}
   asm volatile ("" :: "I"(AboveMax)); // expected-error{{value '2048' out of range for constraint 'I'}}
 }
@@ -12,7 +11,6 @@
 void J(int j) {
   static const int BelowMin = -1;
   static const int AboveMax = 1;
-  asm volatile ("" :: "J"(j)); // expected-error{{constraint 'J' expects an integer constant expression}}
   asm volatile ("" :: "J"(BelowMin)); // expected-error{{value '-1' out of range for constraint 'J'}}
   asm volatile ("" :: "J"(AboveMax)); // expected-error{{value '1' out of range for constraint 'J'}}
 }
@@ -20,7 +18,6 @@
 void K(int k) {

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D60943#1619579 , @JamesNagurne 
wrote:

> In case you haven't seen, this commit breaks non-x86 build bots due to the 
> combination of '-triple x86_64*' and '-S'. Some tests that use this target 
> are only looking for AST dumps, and do not actually require such a target. 
> This is not one of those tests, as it's inspecting assembly.
>  See clang/test/CodeGen/addrsig.c to see how that is handled (via REQUIRES: 
> x86-registered-target).
>
> Oddly enough, other tests that use -triple x86_64* and only inspect the AST 
> don't require the registered target.


Sorry about that. Fixed in r368202.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60943



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

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

This should be ready for review now. PTAL.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943



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

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 206587.
void added a comment.

Modify so that no diagnistics are emitted until we can determine if it's an
immediate or not.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943

Files:
  lib/CodeGen/CGStmt.cpp
  test/Sema/inline-asm-validate-x86.c


Index: test/Sema/inline-asm-validate-x86.c
===
--- test/Sema/inline-asm-validate-x86.c
+++ test/Sema/inline-asm-validate-x86.c
@@ -147,9 +147,9 @@
   // This offset-from-null pointer can be used as an integer constant 
expression.
   __asm__ __volatile__("\n#define S_B abcd%0\n" : : "n"(&((struct s*)0)->b));
   // This pointer cannot be used as an integer constant expression.
-  __asm__ __volatile__("\n#define GLOBAL_A abcd%0\n" : : "n"(&s.a)); // 
expected-error{{constraint 'n' expects an integer constant expression}}
+  __asm__ __volatile__("\n#define GLOBAL_A abcd%0\n" : : "e"(&s.a)); // 
expected-error{{constraint 'e' expects an integer constant expression}}
   // Floating-point is also not okay.
-  __asm__ __volatile__("\n#define PI abcd%0\n" : : "n"(3.14f)); // 
expected-error{{constraint 'n' expects an integer constant expression}}
+  __asm__ __volatile__("\n#define PI abcd%0\n" : : "e"(3.14f)); // 
expected-error{{constraint 'e' expects an integer constant expression}}
 #ifdef AMD64
   // This arbitrary pointer is fine.
   __asm__ __volatile__("\n#define BEEF abcd%0\n" : : 
"n"((int*)0xdeadbeef));
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1842,11 +1842,9 @@
   InputExpr->EvaluateAsRValue(EVResult, getContext(), true);
 
   llvm::APSInt IntResult;
-  if (!EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
-   getContext()))
-llvm_unreachable("Invalid immediate constant!");
-
-  return llvm::ConstantInt::get(getLLVMContext(), IntResult);
+  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
+  getContext()))
+return llvm::ConstantInt::get(getLLVMContext(), IntResult);
 }
 
 Expr::EvalResult Result;


Index: test/Sema/inline-asm-validate-x86.c
===
--- test/Sema/inline-asm-validate-x86.c
+++ test/Sema/inline-asm-validate-x86.c
@@ -147,9 +147,9 @@
   // This offset-from-null pointer can be used as an integer constant expression.
   __asm__ __volatile__("\n#define S_B abcd%0\n" : : "n"(&((struct s*)0)->b));
   // This pointer cannot be used as an integer constant expression.
-  __asm__ __volatile__("\n#define GLOBAL_A abcd%0\n" : : "n"(&s.a)); // expected-error{{constraint 'n' expects an integer constant expression}}
+  __asm__ __volatile__("\n#define GLOBAL_A abcd%0\n" : : "e"(&s.a)); // expected-error{{constraint 'e' expects an integer constant expression}}
   // Floating-point is also not okay.
-  __asm__ __volatile__("\n#define PI abcd%0\n" : : "n"(3.14f)); // expected-error{{constraint 'n' expects an integer constant expression}}
+  __asm__ __volatile__("\n#define PI abcd%0\n" : : "e"(3.14f)); // expected-error{{constraint 'e' expects an integer constant expression}}
 #ifdef AMD64
   // This arbitrary pointer is fine.
   __asm__ __volatile__("\n#define BEEF abcd%0\n" : : "n"((int*)0xdeadbeef));
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1842,11 +1842,9 @@
   InputExpr->EvaluateAsRValue(EVResult, getContext(), true);
 
   llvm::APSInt IntResult;
-  if (!EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
-   getContext()))
-llvm_unreachable("Invalid immediate constant!");
-
-  return llvm::ConstantInt::get(getLLVMContext(), IntResult);
+  if (EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
+  getContext()))
+return llvm::ConstantInt::get(getLLVMContext(), IntResult);
 }
 
 Expr::EvalResult Result;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Friendly ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60943



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

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D38479#1507435 , @phosek wrote:

> We (Fuchsia) would like to see this landed as well so we can start using this 
> in our kernel.


I get the feeling that this patch has been abandoned by the author. Would 
someone like to resurrect it?


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

https://reviews.llvm.org/D38479



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

[PATCH] D38479: Make -mgeneral-regs-only more like GCC's

[Bill Wendling via Phabricator via cfe-commits]

void added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.def:143
 LANGOPT(RelaxedTemplateTemplateArgs, 1, 0, "C++17 relaxed matching of template 
template arguments")
+LANGOPT(GeneralOpsOnly, 1, 0, "Whether to diagnose the use of 
floating-point or vector operations")
 

Everywhere else you use "general regs only" instead of "ops". Should that be 
done here?


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

https://reviews.llvm.org/D38479



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

The issue is that "`n`" is expecting an immediate value, but the result of the 
ternary operator isn't calculated by the front-end, because it doesn't "know" 
that the evaluation of `__builtin_constant_p` shouldn't be delayed (it being 
compiled at `-O0`). I suspect that this issue will also happen with the "`i`" 
constraint.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54355



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

As a side note, the number of ways to evaluate a constant expression are legion 
in clang. They should really be unified...


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void created this revision.
void added a reviewer: rsmith.
Herald added a subscriber: cfe-commits.
void added subscribers: jyknight, craig.topper.
void added a comment.

As a side note, the number of ways to evaluate a constant expression are legion 
in clang. They should really be unified...


Some ASM input constraints (e.g., "i" and "n") require immediate values. At O0,
very few code transformations are performed. So if we cannot resolve to an
immediate when emitting the ASM input we shouldn't delay its processing.


Repository:
  rC Clang

https://reviews.llvm.org/D55616

Files:
  include/clang/AST/Expr.h
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGStmt.cpp
  test/CodeGen/builtin-constant-p.c

Index: test/CodeGen/builtin-constant-p.c
===
--- test/CodeGen/builtin-constant-p.c
+++ test/CodeGen/builtin-constant-p.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | FileCheck --check-prefix=O2 %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | FileCheck --check-prefix=O0 %s
 
 int a = 42;
 
@@ -14,8 +15,8 @@
 struct foo f = (struct foo){ __builtin_constant_p(y), 42 };
 
 struct foo test0(int expr) {
-  // CHECK: define i64 @test0(i32 %expr)
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %expr)
+  // O2: define i64 @test0(i32 %expr)
+  // O2: call i1 @llvm.is.constant.i32(i32 %expr)
   struct foo f = (struct foo){ __builtin_constant_p(expr), 42 };
   return f;
 }
@@ -27,15 +28,15 @@
 }
 
 int test1() {
-  // CHECK: define i32 @test1
-  // CHECK: add nsw i32 %0, -13
-  // CHECK-NEXT: call i1 @llvm.is.constant.i32(i32 %sub)
+  // O2: define i32 @test1
+  // O2: add nsw i32 %0, -13
+  // O2-NEXT: call i1 @llvm.is.constant.i32(i32 %sub)
   return bcp(test1_i(&a) - 13);
 }
 
 int test2() {
-  // CHECK: define i32 @test2
-  // CHECK: ret i32 0
+  // O2: define i32 @test2
+  // O2: ret i32 0
   return __builtin_constant_p(&a - 13);
 }
 
@@ -44,8 +45,8 @@
 }
 
 int test3() {
-  // CHECK: define i32 @test3
-  // CHECK: ret i32 1
+  // O2: define i32 @test3
+  // O2: ret i32 1
   return bcp(test3_i(&a) - 13);
 }
 
@@ -54,16 +55,16 @@
 int b[] = {1, 2, 3};
 
 int test4() {
-  // CHECK: define i32 @test4
-  // CHECK: ret i32 0
+  // O2: define i32 @test4
+  // O2: ret i32 0
   return __builtin_constant_p(b);
 }
 
 const char test5_c[] = {1, 2, 3, 0};
 
 int test5() {
-  // CHECK: define i32 @test5
-  // CHECK: ret i32 0
+  // O2: define i32 @test5
+  // O2: ret i32 0
   return __builtin_constant_p(test5_c);
 }
 
@@ -72,16 +73,16 @@
 }
 
 int test6() {
-  // CHECK: define i32 @test6
-  // CHECK: ret i32 0
+  // O2: define i32 @test6
+  // O2: ret i32 0
   return __builtin_constant_p(test6_i(test5_c));
 }
 
 /* --- Non-constant global variables */
 
 int test7() {
-  // CHECK: define i32 @test7
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %0)
+  // O2: define i32 @test7
+  // O2: call i1 @llvm.is.constant.i32(i32 %0)
   return bcp(a);
 }
 
@@ -90,8 +91,8 @@
 const int c = 42;
 
 int test8() {
-  // CHECK: define i32 @test8
-  // CHECK: ret i32 1
+  // O2: define i32 @test8
+  // O2: ret i32 1
   return bcp(c);
 }
 
@@ -101,34 +102,34 @@
 const int c_arr[] = { 1, 2, 3 };
 
 int test9() {
-  // CHECK: define i32 @test9
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %0)
+  // O2: define i32 @test9
+  // O2: call i1 @llvm.is.constant.i32(i32 %0)
   return __builtin_constant_p(arr[2]);
 }
 
 int test10() {
-  // CHECK: define i32 @test10
-  // CHECK: ret i32 1
+  // O2: define i32 @test10
+  // O2: ret i32 1
   return __builtin_constant_p(c_arr[2]);
 }
 
 int test11() {
-  // CHECK: define i32 @test11
-  // CHECK: ret i32 0
+  // O2: define i32 @test11
+  // O2: ret i32 0
   return __builtin_constant_p(c_arr);
 }
 
 /* --- Function pointers */
 
 int test12() {
-  // CHECK: define i32 @test12
-  // CHECK: ret i32 0
+  // O2: define i32 @test12
+  // O2: ret i32 0
   return __builtin_constant_p(&test10);
 }
 
 int test13() {
-  // CHECK: define i32 @test13
-  // CHECK: ret i32 1
+  // O2: define i32 @test13
+  // O2: ret i32 1
   return __builtin_constant_p(&test10 != 0);
 }
 
@@ -166,3 +167,13 @@
 
 extern char test16_v;
 struct { int a; } test16 = { __builtin_constant_p(test16_v) };
+
+extern unsigned long long test17_v;
+
+void test17() {
+  // O0: define void @test17
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : -1));
+  __asm__ __volatile__("" :: "i"( (__builtin_constant_p(test17_v) || 0) ? 1 : -1));
+}
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1821,7 +1821,12 @@
   // (immediate or symbolic), try to emit it as such.
   if (!Info.allowsRegister() && !

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D54355#1328786 , @jyknight wrote:

> In D54355#1328557 , @void wrote:
>
> > The issue is that "`n`" is expecting an immediate value, but the result of 
> > the ternary operator isn't calculated by the front-end, because it doesn't 
> > "know" that the evaluation of `__builtin_constant_p` shouldn't be delayed 
> > (it being compiled at `-O0`). I suspect that this issue will also happen 
> > with the "`i`" constraint.
>
>
> Indeed. We _do_ actually already know that in clang too, however -- clang 
> already knows that "n" requires an immediate, and does constant expression 
> evaluation for it, e.g. to expand constexpr functions. Grep for 
> "requiresImmediateConstant". I guess it's just not hooked up to the __b_c_p 
> correctly, though.
>
> (Note, as a workaround, you could use `|` instead of `||`, or put the 
> expression as the value of an enumeration constant).


I looked at `requiresImmediateConstant`, but when I set it for `n` and `i` 
things broke all over the place. I sent out a patch for review. It's limited to 
`-O0`, since we can compile with optimizations without complaint. And there may 
be instances where it's necessary:

  inline void foo(int x) {
asm("" : : "n" (__builtin_constant_p(n) ? 1 : -1));
  }
  
  void bar() {
foo(37);
  }


Repository:
  rC Clang

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

https://reviews.llvm.org/D54355



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 177936.
void marked an inline comment as done.
void added a comment.

Don't look at the optimization level here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616

Files:
  include/clang/AST/Expr.h
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGStmt.cpp
  test/CodeGen/builtin-constant-p.c

Index: test/CodeGen/builtin-constant-p.c
===
--- test/CodeGen/builtin-constant-p.c
+++ test/CodeGen/builtin-constant-p.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | FileCheck --check-prefix=O2 %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | FileCheck --check-prefix=O0 %s
 
 int a = 42;
 
@@ -14,8 +15,8 @@
 struct foo f = (struct foo){ __builtin_constant_p(y), 42 };
 
 struct foo test0(int expr) {
-  // CHECK: define i64 @test0(i32 %expr)
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %expr)
+  // O2: define i64 @test0(i32 %expr)
+  // O2: call i1 @llvm.is.constant.i32(i32 %expr)
   struct foo f = (struct foo){ __builtin_constant_p(expr), 42 };
   return f;
 }
@@ -27,15 +28,15 @@
 }
 
 int test1() {
-  // CHECK: define i32 @test1
-  // CHECK: add nsw i32 %0, -13
-  // CHECK-NEXT: call i1 @llvm.is.constant.i32(i32 %sub)
+  // O2: define i32 @test1
+  // O2: add nsw i32 %0, -13
+  // O2-NEXT: call i1 @llvm.is.constant.i32(i32 %sub)
   return bcp(test1_i(&a) - 13);
 }
 
 int test2() {
-  // CHECK: define i32 @test2
-  // CHECK: ret i32 0
+  // O2: define i32 @test2
+  // O2: ret i32 0
   return __builtin_constant_p(&a - 13);
 }
 
@@ -44,8 +45,8 @@
 }
 
 int test3() {
-  // CHECK: define i32 @test3
-  // CHECK: ret i32 1
+  // O2: define i32 @test3
+  // O2: ret i32 1
   return bcp(test3_i(&a) - 13);
 }
 
@@ -54,16 +55,16 @@
 int b[] = {1, 2, 3};
 
 int test4() {
-  // CHECK: define i32 @test4
-  // CHECK: ret i32 0
+  // O2: define i32 @test4
+  // O2: ret i32 0
   return __builtin_constant_p(b);
 }
 
 const char test5_c[] = {1, 2, 3, 0};
 
 int test5() {
-  // CHECK: define i32 @test5
-  // CHECK: ret i32 0
+  // O2: define i32 @test5
+  // O2: ret i32 0
   return __builtin_constant_p(test5_c);
 }
 
@@ -72,16 +73,16 @@
 }
 
 int test6() {
-  // CHECK: define i32 @test6
-  // CHECK: ret i32 0
+  // O2: define i32 @test6
+  // O2: ret i32 0
   return __builtin_constant_p(test6_i(test5_c));
 }
 
 /* --- Non-constant global variables */
 
 int test7() {
-  // CHECK: define i32 @test7
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %0)
+  // O2: define i32 @test7
+  // O2: call i1 @llvm.is.constant.i32(i32 %0)
   return bcp(a);
 }
 
@@ -90,8 +91,8 @@
 const int c = 42;
 
 int test8() {
-  // CHECK: define i32 @test8
-  // CHECK: ret i32 1
+  // O2: define i32 @test8
+  // O2: ret i32 1
   return bcp(c);
 }
 
@@ -101,34 +102,34 @@
 const int c_arr[] = { 1, 2, 3 };
 
 int test9() {
-  // CHECK: define i32 @test9
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %0)
+  // O2: define i32 @test9
+  // O2: call i1 @llvm.is.constant.i32(i32 %0)
   return __builtin_constant_p(arr[2]);
 }
 
 int test10() {
-  // CHECK: define i32 @test10
-  // CHECK: ret i32 1
+  // O2: define i32 @test10
+  // O2: ret i32 1
   return __builtin_constant_p(c_arr[2]);
 }
 
 int test11() {
-  // CHECK: define i32 @test11
-  // CHECK: ret i32 0
+  // O2: define i32 @test11
+  // O2: ret i32 0
   return __builtin_constant_p(c_arr);
 }
 
 /* --- Function pointers */
 
 int test12() {
-  // CHECK: define i32 @test12
-  // CHECK: ret i32 0
+  // O2: define i32 @test12
+  // O2: ret i32 0
   return __builtin_constant_p(&test10);
 }
 
 int test13() {
-  // CHECK: define i32 @test13
-  // CHECK: ret i32 1
+  // O2: define i32 @test13
+  // O2: ret i32 1
   return __builtin_constant_p(&test10 != 0);
 }
 
@@ -166,3 +167,13 @@
 
 extern char test16_v;
 struct { int a; } test16 = { __builtin_constant_p(test16_v) };
+
+extern unsigned long long test17_v;
+
+void test17() {
+  // O0: define void @test17
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : -1));
+  __asm__ __volatile__("" :: "i"( (__builtin_constant_p(test17_v) || 0) ? 1 : -1));
+}
Index: lib/CodeGen/CGStmt.cpp
===
--- lib/CodeGen/CGStmt.cpp
+++ lib/CodeGen/CGStmt.cpp
@@ -1821,7 +1821,7 @@
   // (immediate or symbolic), try to emit it as such.
   if (!Info.allowsRegister() && !Info.allowsMemory()) {
 Expr::EvalResult Result;
-if (InputExpr->EvaluateAsInt(Result, getContext()))
+if (InputExpr->EvaluateAsIntInConstantContext(Result, getContext()))
   return llvm::ConstantInt::get(getLLVMContext(), Result.Val.getInt());
 assert(!Info.requiresImmediateConstant() &&
 

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Addressed Eli's comment.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 178158.
void added a comment.

The `n` constriant *requires* a known integer. Therefore, enforce this in both
Sema and CodeGen by setting the "requires immediate" flag and evaluating to a
known integer instead of random "int"..

This doesn't so much address `i`, which has other semantics. However, it
shouldn't regress how the `i` constraint is currently used.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616

Files:
  lib/Basic/TargetInfo.cpp
  lib/CodeGen/CGStmt.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/builtin-constant-p.c

Index: test/CodeGen/builtin-constant-p.c
===
--- test/CodeGen/builtin-constant-p.c
+++ test/CodeGen/builtin-constant-p.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O2 | FileCheck --check-prefix=O2 %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -o - %s -O0 | FileCheck --check-prefix=O0 %s
 
 int a = 42;
 
@@ -14,8 +15,8 @@
 struct foo f = (struct foo){ __builtin_constant_p(y), 42 };
 
 struct foo test0(int expr) {
-  // CHECK: define i64 @test0(i32 %expr)
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %expr)
+  // O2: define i64 @test0(i32 %expr)
+  // O2: call i1 @llvm.is.constant.i32(i32 %expr)
   struct foo f = (struct foo){ __builtin_constant_p(expr), 42 };
   return f;
 }
@@ -27,15 +28,15 @@
 }
 
 int test1() {
-  // CHECK: define i32 @test1
-  // CHECK: add nsw i32 %0, -13
-  // CHECK-NEXT: call i1 @llvm.is.constant.i32(i32 %sub)
+  // O2: define i32 @test1
+  // O2: add nsw i32 %0, -13
+  // O2-NEXT: call i1 @llvm.is.constant.i32(i32 %sub)
   return bcp(test1_i(&a) - 13);
 }
 
 int test2() {
-  // CHECK: define i32 @test2
-  // CHECK: ret i32 0
+  // O2: define i32 @test2
+  // O2: ret i32 0
   return __builtin_constant_p(&a - 13);
 }
 
@@ -44,8 +45,8 @@
 }
 
 int test3() {
-  // CHECK: define i32 @test3
-  // CHECK: ret i32 1
+  // O2: define i32 @test3
+  // O2: ret i32 1
   return bcp(test3_i(&a) - 13);
 }
 
@@ -54,16 +55,16 @@
 int b[] = {1, 2, 3};
 
 int test4() {
-  // CHECK: define i32 @test4
-  // CHECK: ret i32 0
+  // O2: define i32 @test4
+  // O2: ret i32 0
   return __builtin_constant_p(b);
 }
 
 const char test5_c[] = {1, 2, 3, 0};
 
 int test5() {
-  // CHECK: define i32 @test5
-  // CHECK: ret i32 0
+  // O2: define i32 @test5
+  // O2: ret i32 0
   return __builtin_constant_p(test5_c);
 }
 
@@ -72,16 +73,16 @@
 }
 
 int test6() {
-  // CHECK: define i32 @test6
-  // CHECK: ret i32 0
+  // O2: define i32 @test6
+  // O2: ret i32 0
   return __builtin_constant_p(test6_i(test5_c));
 }
 
 /* --- Non-constant global variables */
 
 int test7() {
-  // CHECK: define i32 @test7
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %0)
+  // O2: define i32 @test7
+  // O2: call i1 @llvm.is.constant.i32(i32 %0)
   return bcp(a);
 }
 
@@ -90,8 +91,8 @@
 const int c = 42;
 
 int test8() {
-  // CHECK: define i32 @test8
-  // CHECK: ret i32 1
+  // O2: define i32 @test8
+  // O2: ret i32 1
   return bcp(c);
 }
 
@@ -101,34 +102,34 @@
 const int c_arr[] = { 1, 2, 3 };
 
 int test9() {
-  // CHECK: define i32 @test9
-  // CHECK: call i1 @llvm.is.constant.i32(i32 %0)
+  // O2: define i32 @test9
+  // O2: call i1 @llvm.is.constant.i32(i32 %0)
   return __builtin_constant_p(arr[2]);
 }
 
 int test10() {
-  // CHECK: define i32 @test10
-  // CHECK: ret i32 1
+  // O2: define i32 @test10
+  // O2: ret i32 1
   return __builtin_constant_p(c_arr[2]);
 }
 
 int test11() {
-  // CHECK: define i32 @test11
-  // CHECK: ret i32 0
+  // O2: define i32 @test11
+  // O2: ret i32 0
   return __builtin_constant_p(c_arr);
 }
 
 /* --- Function pointers */
 
 int test12() {
-  // CHECK: define i32 @test12
-  // CHECK: ret i32 0
+  // O2: define i32 @test12
+  // O2: ret i32 0
   return __builtin_constant_p(&test10);
 }
 
 int test13() {
-  // CHECK: define i32 @test13
-  // CHECK: ret i32 1
+  // O2: define i32 @test13
+  // O2: ret i32 1
   return __builtin_constant_p(&test10 != 0);
 }
 
@@ -166,3 +167,13 @@
 
 extern char test16_v;
 struct { int a; } test16 = { __builtin_constant_p(test16_v) };
+
+extern unsigned long long test17_v;
+
+void test17() {
+  // O0: define void @test17
+  // O0: call void asm sideeffect "", {{.*}}(i32 -1) 
+  // O2: define void @test17
+  // O2: call void asm sideeffect "", {{.*}}(i32 -1) 
+  __asm__ __volatile__("" :: "n"( (__builtin_constant_p(test17_v) || 0) ? 1 : -1));
+}
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -378,17 +378,17 @@
  << InputExpr->getSourceRange());
 } else if (Info.requiresImmediateConstant() && !Info.allowsRegister()) {
   if (!InputExpr->isValueDependent()) {
-Expr::Eva

[PATCH] D55616: Emit ASM input in a constant context

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D55616#1329301 , @jyknight wrote:

> This seems like a good start, but not complete.
>
> "n" and "i" both should require that their argument is a constant expression. 
> For "n", it actually must be an immediate constant integer, so 
> setRequiresImmediate() should be used there. For "i", you may use an lvalue 
> constant as well. The way we seem to indicate that, today, is with `if 
> (!Info.allowsRegister() && !Info.allowsMemory())`. However the code in that 
> block does not today *require* that the evaluation as a constant 
> succeed...and it should.
>
> It should also only require that the result is an integer when 
> `requiresImmediateConstant()`.
>
> Additionally, the Sema code needs to have the same conditions as the CodeGen 
> code for when a constant expression is required, but it doesn't. (before or 
> after your change).


Have I mentioned how much I *love* inline asm? :-)

I think I've addressed all of your concerns. PTAL.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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