diff mbox series

[C++] PR 82307

Message ID 906899ba-66bc-a8c7-1554-5c3b1a330098@oracle.com
State New
Headers show
Series [C++] PR 82307 | expand

Commit Message

Mukesh Kapoor Oct. 9, 2017, 7:20 p.m. UTC 
Hi,

This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82307.
For an unscoped enum with a fixed underlying type, the function 
type_promotes_to() does not always return the same type as the 
underlying type. The fix is to use the underlying type of the enum 
instead of creating a new one by calling c_common_type_for_size().

Bootstrapped and tested with 'make check' on x86_64-linux. New test case 
added.

Mukesh

Comments

Mukesh Kapoor Oct. 18, 2017, 4:17 p.m. UTC | #1 
On 10/9/2017 12:20 PM, Mukesh Kapoor wrote:
> Hi,
>
> This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82307.
> For an unscoped enum with a fixed underlying type, the function 
> type_promotes_to() does not always return the same type as the 
> underlying type. The fix is to use the underlying type of the enum 
> instead of creating a new one by calling c_common_type_for_size().
>
> Bootstrapped and tested with 'make check' on x86_64-linux. New test 
> case added.
>
> Mukesh
>
Nathan Sidwell Oct. 18, 2017, 8:10 p.m. UTC | #2 
On 10/18/2017 12:17 PM, Mukesh Kapoor wrote:
> On 10/9/2017 12:20 PM, Mukesh Kapoor wrote:
>> Hi,
>>
>> This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82307.
>> For an unscoped enum with a fixed underlying type, the function 
>> type_promotes_to() does not always return the same type as the 
>> underlying type. The fix is to use the underlying type of the enum 
>> instead of creating a new one by calling c_common_type_for_size().


The diff looks wrong.  Just before the changed piece it (attempts to) 
deal with enum types.  Why is that failing?

nathan
Mukesh Kapoor Oct. 18, 2017, 8:32 p.m. UTC | #3 
The bug happens only for enum types with a fixed underlying type. The 
existing code tries to create another type based on it's precision by 
calling c_common_type_for_size(). For the precision value of an unsigned 
long long type, the call to c_common_type_for_size() returns an unsigned 
long type and this causes compilation errors later on. The fix is to 
simply use the fixed underlying type of the enum instead of creating a 
new one.

Mukesh

On 10/18/2017 1:10 PM, Nathan Sidwell wrote:
> On 10/18/2017 12:17 PM, Mukesh Kapoor wrote:
>> On 10/9/2017 12:20 PM, Mukesh Kapoor wrote:
>>> Hi,
>>>
>>> This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82307.
>>> For an unscoped enum with a fixed underlying type, the function 
>>> type_promotes_to() does not always return the same type as the 
>>> underlying type. The fix is to use the underlying type of the enum 
>>> instead of creating a new one by calling c_common_type_for_size().
>
>
> The diff looks wrong.  Just before the changed piece it (attempts to) 
> deal with enum types.  Why is that failing?
>
> nathan
>
Paolo Carlini Oct. 23, 2017, 1:15 p.m. UTC | #4 
Hi,

following up to a short off-line exchange with Nathan, I'm sending a 
reworked patch which - among other things - avoids regressing on the 
second testcase (cpp0x/enum36.C). Tested x86_64-linux.

Thanks,
Paolo.

////////////////////
/cp
2017-10-23  Mukesh Kapoor  <mukesh.kapoor@oracle.com>
	    Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/82307
	* cvt.c (type_promotes_to): Implement C++17, 7.6/4, about unscoped
	enumeration type whose underlying type is fixed.

/testsuite
2017-10-23  Mukesh Kapoor  <mukesh.kapoor@oracle.com>
	    Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/82307
	* g++.dg/cpp0x/enum35.C: New.
	* g++.dg/cpp0x/enum36.C: Likewise.
Index: cp/cvt.c
===================================================================
--- cp/cvt.c	(revision 254005)
+++ cp/cvt.c	(working copy)
@@ -1834,12 +1834,27 @@ type_promotes_to (tree type)
 	   || type == char32_type_node
 	   || type == wchar_type_node)
     {
+      tree prom = type;
+
+      if (TREE_CODE (type) == ENUMERAL_TYPE)
+	{
+	  prom = ENUM_UNDERLYING_TYPE (prom);
+	  if (!ENUM_IS_SCOPED (type)
+	      && ENUM_FIXED_UNDERLYING_TYPE_P (type))
+	    {
+	      /* ISO C++17, 7.6/4.  A prvalue of an unscoped enumeration type
+		 whose underlying type is fixed (10.2) can be converted to a
+		 prvalue of its underlying type. Moreover, if integral promotion
+		 can be applied to its underlying type, a prvalue of an unscoped
+		 enumeration type whose underlying type is fixed can also be 
+		 converted to a prvalue of the promoted underlying type.  */
+	      return type_promotes_to (prom);
+	    }
+	}
+
       int precision = MAX (TYPE_PRECISION (type),
 			   TYPE_PRECISION (integer_type_node));
       tree totype = c_common_type_for_size (precision, 0);
-      tree prom = type;
-      if (TREE_CODE (prom) == ENUMERAL_TYPE)
-	prom = ENUM_UNDERLYING_TYPE (prom);
       if (TYPE_UNSIGNED (prom)
 	  && ! int_fits_type_p (TYPE_MAX_VALUE (prom), totype))
 	prom = c_common_type_for_size (precision, 1);
Index: testsuite/g++.dg/cpp0x/enum35.C
===================================================================
--- testsuite/g++.dg/cpp0x/enum35.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/enum35.C	(working copy)
@@ -0,0 +1,14 @@
+// PR c++/82307
+// { dg-do run { target c++11 } }
+
+#include <cassert>
+
+enum : unsigned long long { VAL };
+
+bool foo (unsigned long long) { return true; }
+bool foo (int) { return false; }
+
+int main()
+{
+  assert (foo(VAL));
+}
Index: testsuite/g++.dg/cpp0x/enum36.C
===================================================================
--- testsuite/g++.dg/cpp0x/enum36.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/enum36.C	(working copy)
@@ -0,0 +1,14 @@
+// PR c++/82307
+// { dg-do run { target c++11 } }
+
+#include <cassert>
+
+enum : short { VAL };
+
+bool foo (int) { return true; }
+bool foo (unsigned long long) { return false; }
+
+int main()
+{
+  assert (foo (VAL));
+}
Nathan Sidwell Oct. 23, 2017, 4:58 p.m. UTC | #5 
On 10/23/2017 09:15 AM, Paolo Carlini wrote:
> Hi,
> 
> following up to a short off-line exchange with Nathan, I'm sending a 
> reworked patch which - among other things - avoids regressing on the 
> second testcase (cpp0x/enum36.C). Tested x86_64-linux.

ok, thanks!

nathan
diff mbox series

Patch

Index: gcc/cp/cvt.c
===================================================================
--- gcc/cp/cvt.c	(revision 253551)
+++ gcc/cp/cvt.c	(working copy)
@@ -1854,11 +1854,16 @@ 
       tree prom = type;
       if (TREE_CODE (prom) == ENUMERAL_TYPE)
 	prom = ENUM_UNDERLYING_TYPE (prom);
-      if (TYPE_UNSIGNED (prom)
-	  && ! int_fits_type_p (TYPE_MAX_VALUE (prom), totype))
-	prom = c_common_type_for_size (precision, 1);
-      else
-	prom = totype;
+      // If an unscoped enum has fixed underlying type,
+      // use that type (bug 82307)
+      if (!ENUM_FIXED_UNDERLYING_TYPE_P (type) || SCOPED_ENUM_P (type))
+	{
+	  if (TYPE_UNSIGNED (prom)
+	      && ! int_fits_type_p (TYPE_MAX_VALUE (prom), totype))
+	    prom = c_common_type_for_size (precision, 1);
+	  else
+	    prom = totype;
+	}
       if (SCOPED_ENUM_P (type))
 	{
 	  if (abi_version_crosses (6)
Index: gcc/testsuite/g++.dg/cpp0x/pr_82307.C
===================================================================
--- gcc/testsuite/g++.dg/cpp0x/pr_82307.C	(revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/pr_82307.C	(working copy)
@@ -0,0 +1,24 @@ 
+// PR c++/82307
+// { dg-do compile { target c++11 } }
+
+#include <cstdlib>
+#include <cstring>
+
+enum : unsigned long long { VAL };
+
+const char* foo( unsigned long long )
+{
+  return "unsigned long long";
+}
+
+const char* foo( int )
+{
+  return "int";
+}
+
+int main( void )
+{
+  if (strcmp(foo(VAL), "unsigned long long") != 0)
+    abort();
+}
+