diff mbox

[C++] PR 51312

Message ID 53E3AC51.60109@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Aug. 7, 2014, 4:41 p.m. UTC 
Hi,

On 08/07/2014 06:32 PM, Jason Merrill wrote:
> On 08/07/2014 11:28 AM, Jason Merrill wrote:
>> -Wno-narrowing should prevent an error on enum27
>
> Er, should *not*.
Ok. Probably at some point I should review which kind of diagnostics the 
various implementations emit: for example *for enum27.C* currently clang 
talks about *narrowing*, the kind of of diagnostics we emit from 
chech_narrowing; EDG instead produces a something closer to our error.

Anyway, if that's what we want, at least for the time being, I guess 
it's just matter of sending you what I essentially had yesterday, 
shorter, with the final check unmoved and the those two testcases not 
tweaked. Passes testing.

Thanks!
Paolo.

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

Comments

Jason Merrill Aug. 7, 2014, 5:44 p.m. UTC | #1 
On 08/07/2014 12:41 PM, Paolo Carlini wrote:
> Ok. Probably at some point I should review which kind of diagnostics the various implementations emit: for example *for enum27.C* currently clang talks about *narrowing*, the kind of of diagnostics we emit from chech_narrowing; EDG instead produces a something closer to our error.

I saw that, too.  There used to be a separate constraint "if the 
initializing value of an enumerator cannot be represented by the 
underlying type, the program is ill-formed" that was removed because 
Richard Smith suggested it was redundant with the requirements for a 
converted constant expression, but clang's current behavior suggests 
that perhaps it wasn't so redundant...

The patch is OK, thanks.
Paolo Carlini Aug. 8, 2014, 8:26 a.m. UTC | #2 
Hi,

On 08/07/2014 07:44 PM, Jason Merrill wrote:
> On 08/07/2014 12:41 PM, Paolo Carlini wrote:
>> Ok. Probably at some point I should review which kind of diagnostics 
>> the various implementations emit: for example *for enum27.C* 
>> currently clang talks about *narrowing*, the kind of of diagnostics 
>> we emit from chech_narrowing; EDG instead produces a something closer 
>> to our error.
>
> I saw that, too.  There used to be a separate constraint "if the 
> initializing value of an enumerator cannot be represented by the 
> underlying type, the program is ill-formed" that was removed because 
> Richard Smith suggested it was redundant with the requirements for a 
> converted constant expression, but clang's current behavior suggests 
> that perhaps it wasn't so redundant...
I see, thanks. I wasn't aware of those events. Thus, does that mean that 
maybe we should not use LOOKUP_NO_NARROWING outside, essentially, the 
braced initializers context proper? However, a 
LOOKUP_NO_NARROWING_OUTSIDE_BRACED_INITIALIZER would lead exactly to the 
same code path and checks, only the actual diagnostic at the end of 
check_narrowing would be different, an hard error instead of a pedwarn 
and a wording like the one we have got now in the error message in 
build_enumerator? Something I'm also thinking is that consistency in 
this area is easier for clang, because, if I remember correctly, they 
always have hard errors for narrowings and they don't print, as we do, 
the final '... inside { }' text.

Thus, can you see a relatively easy way for us to have a better 
consistency in the error messages, thus eg, same error/warning for:

struct X
{
   constexpr operator int() const { return 768; }
   constexpr int f() const { return 768; }
};

enum E : signed char { e1 = X(), e2 = X().f() };

???

Thanks!
Paolo.

PS: Finally, when/if we emit a clear hard error up front we should 
probably also avoid the "overflow in implicit constant conversion" 
warning. Certainly we don't emit it together with the error in 
build_enumerator...
diff mbox

Patch

Index: cp/cvt.c
===================================================================
--- cp/cvt.c	(revision 213713)
+++ cp/cvt.c	(working copy)
@@ -1658,8 +1658,9 @@  build_expr_type_conversion (int desires, tree expr
 		    {
 		      error ("ambiguous default type conversion from %qT",
 			     basetype);
-		      error ("  candidate conversions include %qD and %qD",
-			     winner, cand);
+		      inform (input_location,
+			      "  candidate conversions include %qD and %qD",
+			      winner, cand);
 		    }
 		  return error_mark_node;
 		}
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 213713)
+++ cp/decl.c	(working copy)
@@ -12966,14 +12966,32 @@  build_enumerator (tree name, tree value, tree enum
       /* Validate and default VALUE.  */
       if (value != NULL_TREE)
 	{
-	  value = cxx_constant_value (value);
+	  if (!ENUM_UNDERLYING_TYPE (enumtype))
+	    {
+	      tree tmp_value = build_expr_type_conversion (WANT_INT | WANT_ENUM,
+							   value, true);
+	      if (tmp_value)
+		value = tmp_value;
+	    }
+	  else if (! INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (value)))
+	    value = perform_implicit_conversion_flags
+	      (ENUM_UNDERLYING_TYPE (enumtype), value, tf_warning_or_error,
+	       LOOKUP_IMPLICIT | LOOKUP_NO_NARROWING);
 
-	  if (TREE_CODE (value) != INTEGER_CST
-	      || ! INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (value)))
+	  if (value == error_mark_node)
+	    value = NULL_TREE;
+
+	  if (value != NULL_TREE)
 	    {
-	      error ("enumerator value for %qD is not an integer constant",
-		     name);
-	      value = NULL_TREE;
+	      value = cxx_constant_value (value);
+
+	      if (TREE_CODE (value) != INTEGER_CST
+		  || ! INTEGRAL_OR_ENUMERATION_TYPE_P (TREE_TYPE (value)))
+		{
+		  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 0)
+++ testsuite/g++.dg/cpp0x/enum29.C	(working copy)
@@ -0,0 +1,58 @@ 
+// PR c++/51312
+// { dg-do compile { target c++11 } }
+
+struct X0
+{
+  constexpr operator int() const { return 1; }
+};
+
+struct X1
+{
+  enum RE1 { re1 = 1 };
+  constexpr operator RE1() const { return re1; }
+};
+
+struct X2
+{
+  constexpr operator int() const { return __INT_MAX__; }
+};
+
+struct X3
+{
+  enum RE3 { re3 = __INT_MAX__ };
+  constexpr operator RE3() const { return re3; }
+};
+
+struct X4
+{
+  constexpr operator double() const { return 1.0; }
+};
+
+struct X5
+{
+  constexpr operator int() const { return __INT_MAX__; }
+  constexpr operator unsigned() const { return __INT_MAX__ * 2U + 1; }
+};
+
+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 E5 { e5 = X5() };  // { dg-error "ambiguous" }
+
+enum F0 : int { f0 = X0() };
+enum F1 : int { f1 = X1() };
+enum F2 : int { f2 = X2() };
+enum F3 : int { f3 = X3() };
+enum F4 : int { f4 = X4() };  // { dg-error "narrowing" }
+enum F5 : int { f5 = X5() };
+
+enum G0 : signed char { g0 = X0() };
+enum G1 : signed char { g1 = X1() };
+enum G2 : signed char { g2 = X2() };  // { dg-error "narrowing" }
+// { dg-warning "overflow" "" { target *-*-* } 53 }
+enum G3 : signed char { g3 = X3() };  // { dg-error "narrowing" }
+// { dg-warning "overflow" "" { target *-*-* } 55 }
+enum G4 : signed char { g4 = X4() };  // { dg-error "narrowing" }
+enum G5 : signed char { g5 = X5() };  // { dg-error "ambiguous" }