diff mbox

[C++,RFC/Patch] PR c++/71665

Message ID 11b0a778-116d-3249-f39a-75c022df4718@oracle.com
State New
Headers show

Commit Message

Paolo Carlini July 28, 2016, 11:48 a.m. UTC 
Hi,

On 18/07/2016 20:16, Jason Merrill wrote:
> On Tue, Jul 12, 2016 at 10:30 AM, Paolo Carlini
> <paolo.carlini@oracle.com> wrote:
>> On 30/06/2016 19:49, Jason Merrill wrote:
>>> I think we should check the type before calling cxx_constant_value.
>>>
>> Ok, I got the point. I'm not sure however how far we want to go with this
>> and which kind of consistency we want to achieve (vs error messages issued
>> in other similar circumstances). The below certainly passes testing on
>> x86_64-linux.
> I meant the actual type of the expression: that is, check
> INTEGRAL_OR_ENUMERATION_TYPE_P before calling cxx_constant_value.
Ah sorry, I missed the *type* bit. The below passes testing on 
x86_64-linux. I don't think we need to check the type again after 
cxx_constant_value?!?

While finally spending a decent amount of time on this issue I noticed 
that current clang appears to enforce integral or *unscoped* enumeration 
type and tweaking our code in the obvious way doesn't cause regressions, 
we of course reject earlier (ie, not as "could not convert ‘(E)1’ from 
‘E’ to ‘unsigned int’") in build_enumeraror snippets like:

enum class E { e = 1 };

class A
{
   enum { a = E::e };
};

I didn't investigate the issue further, however.

Thanks,
Paolo.

//////////////////////////

Comments

Jason Merrill July 28, 2016, 2:28 p.m. UTC | #1 
On Thu, Jul 28, 2016 at 7:48 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Ah sorry, I missed the *type* bit. The below passes testing on x86_64-linux.
> I don't think we need to check the type again after cxx_constant_value?!?

No, we don't.  The patch is OK.

> While finally spending a decent amount of time on this issue I noticed that
> current clang appears to enforce integral or *unscoped* enumeration type and
> tweaking our code in the obvious way doesn't cause regressions, we of course
> reject earlier (ie, not as "could not convert ‘(E)1’ from ‘E’ to ‘unsigned
> int’") in build_enumerator snippets like:
>
> enum class E { e = 1 };
>
> class A
> {
>   enum { a = E::e };
> };

Sure, that change could improve diagnostic quality a bit.

Jason
diff mbox

Patch

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 238807)
+++ cp/decl.c	(working copy)
@@ -13587,15 +13587,23 @@  build_enumerator (tree name, tree value, tree enum
 
 	  if (value != NULL_TREE)
 	    {
-	      value = cxx_constant_value (value);
-
-	      if (TREE_CODE (value) != INTEGER_CST
-		  || ! INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (value)))
+	      if (! INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (value)))
 		{
-		  error ("enumerator value for %qD is not an integer constant",
-			 name);
+		  error ("enumerator for %qD must have integral or "
+			 "enumeration type", name);
 		  value = NULL_TREE;
 		}
+	      else
+		{
+		  value = cxx_constant_value (value);
+
+		  if (TREE_CODE (value) != INTEGER_CST)
+		    {
+		      error ("enumerator value for %qD is not an integer "
+			     "constant", name);
+		      value = NULL_TREE;
+		    }
+		}
 	    }
 	}
 
Index: testsuite/g++.dg/cpp0x/enum29.C
===================================================================
--- testsuite/g++.dg/cpp0x/enum29.C	(revision 238807)
+++ testsuite/g++.dg/cpp0x/enum29.C	(working copy)
@@ -38,7 +38,7 @@  enum E0 { e0 = X0() };
 enum E1 { e1 = X1() };
 enum E2 { e2 = X2() };
 enum E3 { e3 = X3() };
-enum E4 { e4 = X4() };  // { dg-error "integer constant" }
+enum E4 { e4 = X4() };  // { dg-error "integral" }
 enum E5 { e5 = X5() };  // { dg-error "ambiguous" }
 
 enum F0 : int { f0 = X0() };
Index: testsuite/g++.dg/cpp0x/pr71665.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr71665.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/pr71665.C	(working copy)
@@ -0,0 +1,8 @@ 
+// PR c++/71665
+// { dg-do compile { target c++11 } }
+
+class A 
+{
+  int f ();
+  enum { a = f }; // { dg-error "enumerator" }
+};
Index: testsuite/g++.dg/ext/label10.C
===================================================================
--- testsuite/g++.dg/ext/label10.C	(revision 238807)
+++ testsuite/g++.dg/ext/label10.C	(working copy)
@@ -4,7 +4,7 @@ 
 
 template<int N> struct A
 {
-  enum { M = && N };	// { dg-error "referenced outside|cannot appear in|not an integer constant" }
+  enum { M = && N };	// { dg-error "referenced outside|cannot appear in|integral" }
 };
 
 A<0> a;
@@ -12,6 +12,6 @@  A<0> a;
 void foo ()
 {
   __label__ P;
-  enum { O = && P };	// { dg-error "cannot appear in|not an integer constant" }
+  enum { O = && P };	// { dg-error "cannot appear in|integral" }
   P:;
 }
Index: testsuite/g++.dg/parse/constant5.C
===================================================================
--- testsuite/g++.dg/parse/constant5.C	(revision 238807)
+++ testsuite/g++.dg/parse/constant5.C	(working copy)
@@ -1,7 +1,7 @@ 
 // { dg-options "-std=c++98 -pedantic-errors" }
 
 enum E { 
-  a = 24.2, // { dg-error "constant" }
+  a = 24.2, // { dg-error "integral|constant" }
   b = (int)3.7, 
   c = int(4.2),
   d = (int)(4.2 + 3.7), // { dg-error "constant" }