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())) {
----------------
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.


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