[PATCH] D120862: Sema: Allow scoped enums as source type for integral conversion.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D120862#3857372 , @shafik wrote:

> In D120862#3857340 , @pcc wrote:
>
>> Does that DR apply retroactively to C++17? I get the impression that 
>> "Status: C++20" means that the issue was only fixed in C++20, which would 
>> make this well-formed with `-std=c++17`.
>
> This was simply an oversite in the wording of the paper see discussion here 
> 
>  and this was never intended to work.
>
> We have some discretion about how far back to apply DRs and I believe as long 
> as applying them back does not cause major disruption we do CC @aaron.ballman

You're correct that "Status: C++20" means that the issue was fixed in C++20. 
And technically speaking, we only need to apply those changes in C++20 and 
later. However, the intent of the DR process is to fix mistakes in older 
language modes. ISO doesn't let us retroactively change a published standard, 
so we keep this side list of "oh you should also fix this stuff" with the 
intent that the fixes apply as far back as they're relevant. We try to follow 
that guidance whenever we can, but if a DR causes code to break and that turns 
out to be disruptive, we'll sometimes decide to not apply it farther back than 
the standard version it was fixed in. I don't think there's a problem applying 
that specific DR as far back as we can.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120862

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

[PATCH] D120862: Sema: Allow scoped enums as source type for integral conversion.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a subscriber: aaron.ballman.
shafik added a comment.

In D120862#3857340 , @pcc wrote:

> Does that DR apply retroactively to C++17? I get the impression that "Status: 
> C++20" means that the issue was only fixed in C++20, which would make this 
> well-formed with `-std=c++17`.

This was simply an oversite in the wording of the paper see discussion here 

 and this was never intended to work.

We have some discretion about how far back to apply DRs and I believe as long 
as applying them back does not cause major disruption we do CC @aaron.ballman


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120862

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

[PATCH] D120862: Sema: Allow scoped enums as source type for integral conversion.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Does that DR apply retroactively to C++17? I get the impression that "Status: 
C++20" means that the issue was only fixed in C++20, which would make this 
well-formed with `-std=c++17`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120862

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

[PATCH] D120862: Sema: Allow scoped enums as source type for integral conversion.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

I commented on the bug report: https://github.com/llvm/llvm-project/issues/54158

I wanted to also add a note here, I don't believe this is a valid change. The 
example should be ill-formed as was clarified by defect report 2374 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120862

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

[PATCH] D120862: Sema: Allow scoped enums as source type for integral conversion.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Ping^2.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120862

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

[PATCH] D120862: Sema: Allow scoped enums as source type for integral conversion.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120862

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

[PATCH] D120862: Sema: Allow scoped enums as source type for integral conversion.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added a reviewer: rsmith.
Herald added a project: All.
pcc requested review of this revision.
Herald added a project: clang.

Fixes pr54158.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120862

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/cxx17-enum-scoped.cpp


Index: clang/test/SemaCXX/cxx17-enum-scoped.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx17-enum-scoped.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -pedantic -std=c++17 -verify -triple 
x86_64-apple-darwin %s
+
+// expected-no-diagnostics
+
+namespace PR54158 {
+  enum class A : int;
+  enum class B : int;
+  B x{A{}};
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -420,7 +420,7 @@
   //value when converted back to the original type.
   case ICK_Integral_Conversion:
   IntegralConversion: {
-assert(FromType->isIntegralOrUnscopedEnumerationType());
+assert(FromType->isIntegralOrEnumerationType());
 assert(ToType->isIntegralOrUnscopedEnumerationType());
 const bool FromSigned = FromType->isSignedIntegerOrEnumerationType();
 const unsigned FromWidth = Ctx.getIntWidth(FromType);


Index: clang/test/SemaCXX/cxx17-enum-scoped.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx17-enum-scoped.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -pedantic -std=c++17 -verify -triple x86_64-apple-darwin %s
+
+// expected-no-diagnostics
+
+namespace PR54158 {
+  enum class A : int;
+  enum class B : int;
+  B x{A{}};
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -420,7 +420,7 @@
   //value when converted back to the original type.
   case ICK_Integral_Conversion:
   IntegralConversion: {
-assert(FromType->isIntegralOrUnscopedEnumerationType());
+assert(FromType->isIntegralOrEnumerationType());
 assert(ToType->isIntegralOrUnscopedEnumerationType());
 const bool FromSigned = FromType->isSignedIntegerOrEnumerationType();
 const unsigned FromWidth = Ctx.getIntWidth(FromType);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits