[PATCH] D126084: [Sema] Reject implicit conversions between different scoped enum types in list initialization
aaron.ballman added a reviewer: erichkeane.
aaron.ballman added a subscriber: erichkeane.
aaron.ballman 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:
> ahatanak wrote:
> > aaron.ballman wrote:
> > > ahatanak wrote:
> > > > 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.
> > > > Is the code below checking whether the implicit conversion is valid?
> > >
> > > It's forming the conversion sequence as-if it must be valid, but that
> > > causes us to get the right diagnostics. We do the same for narrowing
> > > float conversions:
> > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaInit.cpp#L4521
> > > and I would expect us to then need changes so we get to here:
> > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaInit.cpp#L8478
> > But a conversion from a scoped enum to another scoped enum or its
> > underlying type isn't a narrowing conversion unless the conversion from the
> > underlying type is narrowing. I guess the current code is forming the
> > conversion sequence as if it is valid when the source type is a floating
> > type just to call `DiagnoseNarrowingInInitList`. @rsmith, any comments?
> >
> > If we want to detect the invalid conversion while performing conversion,
> > shouldn't the call to `PerformImplicitConversion`, which is called before
> > reaching the call to `DiagnoseNarrowingInInitList`, fail? Why should it
>
[PATCH] D126084: [Sema] Reject implicit conversions between different scoped enum types in list initialization
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:
> > > 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.
> > > Is the code below checking whether the implicit conversion is valid?
> >
> > It's forming the conversion sequence as-if it must be valid, but that
> > causes us to get the right diagnostics. We do the same for narrowing float
> > conversions:
> > https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaInit.cpp#L4521
> > and I would expect us to then need changes so we get to here:
> > https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaInit.cpp#L8478
> But a conversion from a scoped enum to another scoped enum or its underlying
> type isn't a narrowing conversion unless the conversion from the underlying
> type is narrowing. I guess the current code is forming the conversion
> sequence as if it is valid when the source type is a floating type just to
> call `DiagnoseNarrowingInInitList`. @rsmith, any comments?
>
> If we want to detect the invalid conversion while performing conversion,
> shouldn't the call to `PerformImplicitConversion`, which is called before
> reaching the call to `DiagnoseNarrowingInInitList`, fail? Why should it
> succeed?
>
> https://github.com/llvm/llvm-project/blob/7689c7fc9e08cc430daca3714bcffdd00fd538bd/clang/lib/Sema/SemaInit.cpp#L8467
>
> But I think the invalid conversion should be detected at the very beginning
> of the function before conversion is attempted where it checks whether the
> initialization sequence is invalid
[PATCH] D126084: [Sema] Reject implicit conversions between different scoped enum types in list initialization
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:
> > 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.
> > Is the code below checking whether the implicit conversion is valid?
>
> It's forming the conversion sequence as-if it must be valid, but that causes
> us to get the right diagnostics. We do the same for narrowing float
> conversions:
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaInit.cpp#L4521
> and I would expect us to then need changes so we get to here:
> https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaInit.cpp#L8478
But a conversion from a scoped enum to another scoped enum or its underlying
type isn't a narrowing conversion unless the conversion from the underlying
type is narrowing. I guess the current code is forming the conversion sequence
as if it is valid when the source type is a floating type just to call
`DiagnoseNarrowingInInitList`. @rsmith, any comments?
If we want to detect the invalid conversion while performing conversion,
shouldn't the call to `PerformImplicitConversion`, which is called before
reaching the call to `DiagnoseNarrowingInInitList`, fail? Why should it
succeed?
https://github.com/llvm/llvm-project/blob/7689c7fc9e08cc430daca3714bcffdd00fd538bd/clang/lib/Sema/SemaInit.cpp#L8467
But I think the invalid conversion should be detected at the very beginning of
the function before conversion is attempted where it checks whether the
initialization sequence is invalid
(https://github.com/llvm/llvm-project/blob/7689c7fc9e08cc430daca3714bcffdd00fd538bd/clang/lib/Sema/SemaInit.cpp#L8020).
That can be done by calling `Sequence.SetFailed`
[PATCH] D126084: [Sema] Reject implicit conversions between different scoped enum types in list initialization
aaron.ballman 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:
> 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.
> Is the code below checking whether the implicit conversion is valid?
It's forming the conversion sequence as-if it must be valid, but that causes us
to get the right diagnostics. We do the same for narrowing float conversions:
https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaInit.cpp#L4521
and I would expect us to then need changes so we get to here:
https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaInit.cpp#L8478
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
[PATCH] D126084: [Sema] Reject implicit conversions between different scoped enum types in list initialization
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
[PATCH] D126084: [Sema] Reject implicit conversions between different scoped enum types in list initialization
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
[PATCH] D126084: [Sema] Reject implicit conversions between different scoped enum types in list initialization
aaron.ballman 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:
> > 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.
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
[PATCH] D126084: [Sema] Reject implicit conversions between different scoped enum types in list initialization
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:
> 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
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
[PATCH] D126084: [Sema] Reject implicit conversions between different scoped enum types in list initialization
aaron.ballman added a reviewer: clang-language-wg.
aaron.ballman added a comment.
Please add more details to the summary and remove the rdar link (nobody outside
of Apple can access that anyway). Also, this should have a release note for the
bug fix.
Comment at: clang/lib/Sema/SemaInit.cpp:4506
!S.Context.hasSameUnqualifiedType(E->getType(), DestType) &&
-(E->getType()->isIntegralOrEnumerationType() ||
+(E->getType()->isIntegralOrUnscopedEnumerationType() ||
E->getType()->isFloatingType())) {
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.
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
[PATCH] D126084: [Sema] Reject implicit conversions between different scoped enum types in list initialization
ahatanak added a comment. The assertion `assert(FromType->isIntegralOrUnscopedEnumerationType())` in `StandardConversionSequence::getNarrowingKind` fails when the invalid initialization is performed. 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
[PATCH] D126084: [Sema] Reject implicit conversions between different scoped enum types in list initialization
ahatanak created this revision.
ahatanak added reviewers: rsmith, aaron.ballman.
ahatanak added a project: clang.
Herald added a project: All.
ahatanak requested review of this revision.
rdar://93660425
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D126084
Files:
clang/lib/Sema/SemaInit.cpp
clang/test/SemaCXX/enum-scoped.cpp
Index: clang/test/SemaCXX/enum-scoped.cpp
===
--- clang/test/SemaCXX/enum-scoped.cpp
+++ clang/test/SemaCXX/enum-scoped.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -pedantic -std=c++11 -verify -triple
x86_64-apple-darwin %s
+// RUN: %clang_cc1 -fsyntax-only -pedantic -std=c++17 -verify -triple
x86_64-apple-darwin %s
enum class E1 {
Val1 = 1L
@@ -31,10 +32,22 @@
int x2 = Val2;
int a1[Val2];
-int a2[E1::Val1]; // expected-error{{size of array has non-integer type}}
+int a2[E1::Val1];
+
+#if __cplusplus >= 201703L
+// expected-error@-3 {{type 'E1' is not implicitly convertible to 'unsigned
long'}}
+#else
+// expected-error@-5 {{size of array has non-integer type}}
+#endif
int* p1 = new int[Val2];
-int* p2 = new int[E1::Val1]; // expected-error{{array size expression must
have integral or unscoped enumeration type, not 'E1'}}
+int* p2 = new int[E1::Val1];
+
+#if __cplusplus >= 201703L
+// expected-error@-3 {{converting 'E1' to incompatible type 'unsigned long'}}
+#else
+// expected-error@-5 {{array size expression must have integral or unscoped
enumeration type, not 'E1'}}
+#endif
enum class E4 {
e1 = -2147483648, // ok
@@ -317,3 +330,11 @@
enum C { R, G, B };
enum B { F = (enum C) -1, T}; // this should compile cleanly, it used to
assert.
};
+
+namespace test12 {
+// Check that clang rejects this code without crashing in c++17.
+enum class A;
+enum class B;
+A a;
+B b{a}; // expected-error {{cannot initialize}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -4503,7 +4503,7 @@
Kind.getKind() == InitializationKind::IK_DirectList &&
ET && ET->getDecl()->isFixed() &&
!S.Context.hasSameUnqualifiedType(E->getType(), DestType) &&
-(E->getType()->isIntegralOrEnumerationType() ||
+(E->getType()->isIntegralOrUnscopedEnumerationType() ||
E->getType()->isFloatingType())) {
// There are two ways that T(v) can work when T is an enumeration type.
// If there is either an implicit conversion sequence from v to T or
Index: clang/test/SemaCXX/enum-scoped.cpp
===
--- clang/test/SemaCXX/enum-scoped.cpp
+++ clang/test/SemaCXX/enum-scoped.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -pedantic -std=c++11 -verify -triple x86_64-apple-darwin %s
+// RUN: %clang_cc1 -fsyntax-only -pedantic -std=c++17 -verify -triple x86_64-apple-darwin %s
enum class E1 {
Val1 = 1L
@@ -31,10 +32,22 @@
int x2 = Val2;
int a1[Val2];
-int a2[E1::Val1]; // expected-error{{size of array has non-integer type}}
+int a2[E1::Val1];
+
+#if __cplusplus >= 201703L
+// expected-error@-3 {{type 'E1' is not implicitly convertible to 'unsigned long'}}
+#else
+// expected-error@-5 {{size of array has non-integer type}}
+#endif
int* p1 = new int[Val2];
-int* p2 = new int[E1::Val1]; // expected-error{{array size expression must have integral or unscoped enumeration type, not 'E1'}}
+int* p2 = new int[E1::Val1];
+
+#if __cplusplus >= 201703L
+// expected-error@-3 {{converting 'E1' to incompatible type 'unsigned long'}}
+#else
+// expected-error@-5 {{array size expression must have integral or unscoped enumeration type, not 'E1'}}
+#endif
enum class E4 {
e1 = -2147483648, // ok
@@ -317,3 +330,11 @@
enum C { R, G, B };
enum B { F = (enum C) -1, T}; // this should compile cleanly, it used to assert.
};
+
+namespace test12 {
+// Check that clang rejects this code without crashing in c++17.
+enum class A;
+enum class B;
+A a;
+B b{a}; // expected-error {{cannot initialize}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -4503,7 +4503,7 @@
Kind.getKind() == InitializationKind::IK_DirectList &&
ET && ET->getDecl()->isFixed() &&
!S.Context.hasSameUnqualifiedType(E->getType(), DestType) &&
-(E->getType()->isIntegralOrEnumerationType() ||
+(E->getType()->isIntegralOrUnscopedEnumerationType() ||
E->getType()->isFloatingType())) {
// There are two ways that T(v) can work when T is an enumeration type.
// If there is either an implicit conversion sequence from v to T or
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
