ahatanak added inline comments.

================
Comment at: clang/lib/Sema/SemaInit.cpp:4506
         !S.Context.hasSameUnqualifiedType(E->getType(), DestType) &&
-        (E->getType()->isIntegralOrEnumerationType() ||
+        (E->getType()->isIntegralOrUnscopedEnumerationType() ||
          E->getType()->isFloatingType())) {
----------------
ahatanak wrote:
> aaron.ballman wrote:
> > ahatanak wrote:
> > > aaron.ballman wrote:
> > > > This doesn't match the comments immediately above here and I don't 
> > > > think is the correct fix.
> > > > 
> > > > We're handling this case: http://eel.is/c++draft/dcl.init.list#3.8
> > > > 
> > > > A scoped enumeration has a fixed underlying type 
> > > > (https://eel.is/c++draft/dcl.enum#5.sentence-5). The initializer list 
> > > > has a single element and that element can be implicitly converted to 
> > > > the underlying type (`int` in all of the test cases changed in this 
> > > > patch). And this is a direct initialization case, so I think we should 
> > > > be performing the conversion here rather than skipping to the next 
> > > > bullet.
> > > Can scoped enums be implicitly converted to integer types? Unscoped enums 
> > > can be converted to an integer type, but I don't see any mention of 
> > > scoped enums here: https://eel.is/c++draft/conv.integral
> > > 
> > > It seems that the original paper was trying to change the rules about 
> > > conversions from the underlying type to a scoped enum. It doesn't look 
> > > like it's allowing conversion from a scope enum to another scope enum.
> > > 
> > > https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0138r2.pdf
> > > Can scoped enums be implicitly converted to integer types? Unscoped enums 
> > > can be converted to an integer type, but I don't see any mention of 
> > > scoped enums here: https://eel.is/c++draft/conv.integral
> > 
> > Correct, they cannot be implicitly converted to an integer.
> > 
> > > It seems that the original paper was trying to change the rules about 
> > > conversions from the underlying type to a scoped enum. It doesn't look 
> > > like it's allowing conversion from a scope enum to another scope enum.
> > 
> > Agreed, however, I think where we want this to fail is below in the attempt 
> > at conversion. "v can be implicitly converted to U" is the part that should 
> > be failing here, and we're now skipping over the bit of code that's 
> > checking whether the implicit conversion is valid.
> Is the code below checking whether the implicit conversion is valid? It looks 
> like it's assuming the implicit conversion is valid and adding an implicit 
> conversion sequence based on that assumption. If the source is an integer, 
> unscoped enum, or floating type, the implicit conversion that is performed 
> later should succeed except when there is narrowing.
> 
> Or are you suggesting we should add a check to 
> `Sema::PerformImplicitConversion` that rejects conversions from scoped enums 
> to other types? It seems to me that it's better to detect the error earlier.
Alternatively, we can emit a diagnostic in the code below that specifically 
calls out conversion from scoped enums to integer types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126084

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

Reply via email to