[PATCH] D40398: Remove ValueDependent assertions on ICE checks.

[Matt Davis via Phabricator via cfe-commits]

mattd created this revision.

In the case of ill-formed templates, a value dependent name can reach the integral constant expression (ICE) check. After an error occurs following an ill-formed template, an ICE check can be performed on a template definition. That definition might contain an expression, such as "const a = sizeof(T);" where T is a template type argument. In this case, the latter expression is ValueDependent, T is type dependent, and the sizeof expression is therefore considered Value Dependent.

The assertions will trigger, if the implementation of a templated member function occurs in a separate namespace from its definition (bad code). If such a case occurs, an ODR marking of a variable decl is performed, which happens to check that a particular variable is constant.

Additionally, ValueDependent items can be ICEs, but we never check that case, and the assertions assume we never see them in the first place.  In general, I don't like removing assertions, for obvious reasons.  But in this case, I am not sure they are necessary, and can be problematic as seen by the included test case.


https://reviews.llvm.org/D40398

Files:
  lib/AST/Decl.cpp
  lib/AST/ExprConstant.cpp
  test/SemaTemplate/illformed-template-ice.cpp


Index: test/SemaTemplate/illformed-template-ice.cpp
===================================================================
--- test/SemaTemplate/illformed-template-ice.cpp
+++ test/SemaTemplate/illformed-template-ice.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+class A {
+public:
+  template <typename T> void B();
+};
+
+namespace {
+  template <typename T> void A::B() { // expected-error {{cannot define or redeclare 'B' here because namespace '' does not enclose namespace 'A'}}
+    const int a = sizeof(T);
+    int  b = a;
+  }
+}
Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -10250,7 +10250,6 @@
 }
 
 static ICEDiag CheckICE(const Expr* E, const ASTContext &Ctx) {
-  assert(!E->isValueDependent() && "Should not see value dependent exprs!");
   if (!E->getType()->isIntegralOrEnumerationType())
     return ICEDiag(IK_NotICE, E->getLocStart());
 
Index: lib/AST/Decl.cpp
===================================================================
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2216,7 +2216,6 @@
     return Eval->Evaluated.isUninit() ? nullptr : &Eval->Evaluated;
 
   const auto *Init = cast<Expr>(Eval->Value);
-  assert(!Init->isValueDependent());
 
   if (Eval->IsEvaluating) {
     // FIXME: Produce a diagnostic for self-initialization.
@@ -2284,7 +2283,6 @@
     return Eval->IsICE;
 
   const auto *Init = cast<Expr>(Eval->Value);
-  assert(!Init->isValueDependent());
 
   // In C++11, evaluate the initializer to check whether it's a constant
   // expression.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40398.124102.patch
Type: text/x-patch
Size: 1627 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171123/fbe07268/attachment.bin>