Skip to content

[clang] Implement CWG2627 Bit-fields and narrowing conversions #78112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ Resolutions to C++ Defect Reports
Otherwise, if there is no initializer list constructor, the copy will be elided as if it was ``T(e)``.
(`CWG2311: Missed case for guaranteed copy elision <https://cplusplus.github.io/CWG/issues/2311.html>`)

- Casts from a bit-field to an integral type is now not considered narrowing if the
width of the bit-field means that all potential values are in the range
of the target type, even if the type of the bit-field is larger.
(`CWG2627: Bit-fields and narrowing conversions <https://cplusplus.github.io/CWG/issues/2627.html>`_)

C Language Changes
------------------

Expand Down
114 changes: 73 additions & 41 deletions clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -489,61 +489,93 @@ NarrowingKind StandardConversionSequence::getNarrowingKind(

// -- from an integer type or unscoped enumeration type to an integer type
// that cannot represent all the values of the original type, except where
// the source is a constant expression and the actual value after
// (CWG2627) -- the source is a bit-field whose width w is less than that
// of its type (or, for an enumeration type, its underlying type) and the
// target type can represent all the values of a hypothetical extended
// integer type with width w and with the same signedness as the original
// type or
// -- the source is a constant expression and the actual value after
// conversion will fit into the target type and will produce the original
// value when converted back to the original type.
case ICK_Integral_Conversion:
IntegralConversion: {
assert(FromType->isIntegralOrUnscopedEnumerationType());
assert(ToType->isIntegralOrUnscopedEnumerationType());
const bool FromSigned = FromType->isSignedIntegerOrEnumerationType();
const unsigned FromWidth = Ctx.getIntWidth(FromType);
unsigned FromWidth = Ctx.getIntWidth(FromType);
const bool ToSigned = ToType->isSignedIntegerOrEnumerationType();
const unsigned ToWidth = Ctx.getIntWidth(ToType);

if (FromWidth > ToWidth ||
(FromWidth == ToWidth && FromSigned != ToSigned) ||
(FromSigned && !ToSigned)) {
// Not all values of FromType can be represented in ToType.
const Expr *Initializer = IgnoreNarrowingConversion(Ctx, Converted);
constexpr auto CanRepresentAll = [](bool FromSigned, unsigned FromWidth,
bool ToSigned, unsigned ToWidth) {
return (FromWidth < ToWidth + (FromSigned == ToSigned)) &&
(FromSigned <= ToSigned);
};

// If it's value-dependent, we can't tell whether it's narrowing.
if (Initializer->isValueDependent())
return NK_Dependent_Narrowing;
if (CanRepresentAll(FromSigned, FromWidth, ToSigned, ToWidth))
return NK_Not_Narrowing;

std::optional<llvm::APSInt> OptInitializerValue;
if (!(OptInitializerValue = Initializer->getIntegerConstantExpr(Ctx))) {
// Such conversions on variables are always narrowing.
return NK_Variable_Narrowing;
}
llvm::APSInt &InitializerValue = *OptInitializerValue;
bool Narrowing = false;
if (FromWidth < ToWidth) {
// Negative -> unsigned is narrowing. Otherwise, more bits is never
// narrowing.
if (InitializerValue.isSigned() && InitializerValue.isNegative())
Narrowing = true;
} else {
// Add a bit to the InitializerValue so we don't have to worry about
// signed vs. unsigned comparisons.
InitializerValue = InitializerValue.extend(
InitializerValue.getBitWidth() + 1);
// Convert the initializer to and from the target width and signed-ness.
llvm::APSInt ConvertedValue = InitializerValue;
ConvertedValue = ConvertedValue.trunc(ToWidth);
ConvertedValue.setIsSigned(ToSigned);
ConvertedValue = ConvertedValue.extend(InitializerValue.getBitWidth());
ConvertedValue.setIsSigned(InitializerValue.isSigned());
// If the result is different, this was a narrowing conversion.
if (ConvertedValue != InitializerValue)
Narrowing = true;
}
if (Narrowing) {
ConstantType = Initializer->getType();
ConstantValue = APValue(InitializerValue);
return NK_Constant_Narrowing;
// Not all values of FromType can be represented in ToType.
const Expr *Initializer = IgnoreNarrowingConversion(Ctx, Converted);

bool DependentBitField = false;
if (const FieldDecl *BitField = Initializer->getSourceBitField()) {
if (BitField->getBitWidth()->isValueDependent())
DependentBitField = true;
else if (unsigned BitFieldWidth = BitField->getBitWidthValue(Ctx);
BitFieldWidth < FromWidth) {
if (CanRepresentAll(FromSigned, BitFieldWidth, ToSigned, ToWidth))
return NK_Not_Narrowing;

// The initializer will be truncated to the bit-field width
FromWidth = BitFieldWidth;
}
}

// If it's value-dependent, we can't tell whether it's narrowing.
if (Initializer->isValueDependent())
return NK_Dependent_Narrowing;

std::optional<llvm::APSInt> OptInitializerValue =
Initializer->getIntegerConstantExpr(Ctx);
if (!OptInitializerValue) {
// If the bit-field width was dependent, it might end up being small
// enough to fit in the target type (unless the target type is unsigned
// and the source type is signed, in which case it will never fit)
if (DependentBitField && (FromSigned <= ToSigned))
return NK_Dependent_Narrowing;

// Otherwise, such a conversion is always narrowing
return NK_Variable_Narrowing;
}
llvm::APSInt &InitializerValue = *OptInitializerValue;
bool Narrowing = false;
if (FromWidth < ToWidth) {
// Negative -> unsigned is narrowing. Otherwise, more bits is never
// narrowing.
if (InitializerValue.isSigned() && InitializerValue.isNegative())
Narrowing = true;
} else {
// Add a bit to the InitializerValue so we don't have to worry about
// signed vs. unsigned comparisons.
InitializerValue =
InitializerValue.extend(InitializerValue.getBitWidth() + 1);
// Convert the initializer to and from the target width and signed-ness.
llvm::APSInt ConvertedValue = InitializerValue;
ConvertedValue = ConvertedValue.trunc(ToWidth);
ConvertedValue.setIsSigned(ToSigned);
ConvertedValue = ConvertedValue.extend(InitializerValue.getBitWidth());
ConvertedValue.setIsSigned(InitializerValue.isSigned());
// If the result is different, this was a narrowing conversion.
if (ConvertedValue != InitializerValue)
Narrowing = true;
}
if (Narrowing) {
ConstantType = Initializer->getType();
ConstantValue = APValue(InitializerValue);
return NK_Constant_Narrowing;
}

return NK_Not_Narrowing;
}
case ICK_Complex_Real:
Expand Down
123 changes: 116 additions & 7 deletions clang/test/CXX/drs/cwg26xx.cpp
Original file line number Diff line number Diff line change
@@ -1,11 +1,39 @@
// RUN: %clang_cc1 -std=c++98 -triple x86_64-unknown-unknown -pedantic-errors %s -verify=expected
// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-unknown -pedantic-errors %s -verify=expected,since-cxx11,cxx11
// RUN: %clang_cc1 -std=c++14 -triple x86_64-unknown-unknown -pedantic-errors %s -verify=expected,since-cxx11
// RUN: %clang_cc1 -std=c++17 -triple x86_64-unknown-unknown -pedantic-errors %s -verify=expected,since-cxx11
// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown -pedantic-errors %s -verify=expected,since-cxx11,since-cxx20
// RUN: %clang_cc1 -std=c++23 -triple x86_64-unknown-unknown -pedantic-errors %s -verify=expected,since-cxx11,since-cxx20,since-cxx23
// RUN: %clang_cc1 -std=c++2c -triple x86_64-unknown-unknown -pedantic-errors %s -verify=expected,since-cxx11,since-cxx20,since-cxx23
// RUN: %clang_cc1 -std=c++98 -pedantic-errors %s -verify=expected,cxx98
// RUN: %clang_cc1 -std=c++11 -pedantic-errors %s -verify=expected,since-cxx11,cxx11
// RUN: %clang_cc1 -std=c++14 -pedantic-errors %s -verify=expected,since-cxx11
// RUN: %clang_cc1 -std=c++17 -pedantic-errors %s -verify=expected,since-cxx11
// RUN: %clang_cc1 -std=c++20 -pedantic-errors %s -verify=expected,since-cxx11,since-cxx20
// RUN: %clang_cc1 -std=c++23 -pedantic-errors %s -verify=expected,since-cxx11,since-cxx20,since-cxx23
// RUN: %clang_cc1 -std=c++2c -pedantic-errors %s -verify=expected,since-cxx11,since-cxx20,since-cxx23

#if __cplusplus == 199711L
#define static_assert(...) __extension__ _Static_assert(__VA_ARGS__)
// cxx98-error@-1 {{variadic macros are a C99 feature}}
#endif

namespace std {
#if __cplusplus >= 202002L
struct strong_ordering {
int n;
constexpr operator int() const { return n; }
static const strong_ordering less, equal, greater;
};
constexpr strong_ordering strong_ordering::less{-1},
strong_ordering::equal{0}, strong_ordering::greater{1};
#endif

typedef short int16_t;
typedef unsigned short uint16_t;
typedef int int32_t;
typedef unsigned uint32_t;
typedef long long int64_t;
// cxx98-error@-1 {{'long long' is a C++11 extension}}
typedef unsigned long long uint64_t;
// cxx98-error@-1 {{'long long' is a C++11 extension}}
static_assert(sizeof(int16_t) == 2 && sizeof(int32_t) == 4 && sizeof(int64_t) == 8, "Some tests rely on these sizes");

template<typename T> T declval();
}

namespace cwg2621 { // cwg2621: sup 2877
#if __cplusplus >= 202002L
Expand All @@ -23,6 +51,87 @@ using enum E;
#endif
}

namespace cwg2627 { // cwg2627: 20
#if __cplusplus >= 202002L
struct C {
long long i : 8;
friend auto operator<=>(C, C) = default;
};

void f() {
C x{1}, y{2};
static_cast<void>(x <=> y);
static_cast<void>(x.i <=> y.i);
}

template<typename T>
struct CDependent {
T i : 8;
friend auto operator<=>(CDependent, CDependent) = default;
};

template<typename T>
concept three_way_comparable = requires(T t) { { t <=> t }; };
template<typename T>
concept bf_three_way_comparable = requires(T t) { { t.i <=> t.i }; };
static_assert(three_way_comparable<CDependent<long long>>);
static_assert(bf_three_way_comparable<CDependent<long long>>);
#endif

#if __cplusplus >= 201103L
template<typename T, int N>
struct D {
T i : N;
};

template<typename T, int N>
D<T, N> d();

std::int32_t d1{ d<std::int64_t, 31>().i };
std::int32_t d2{ d<std::int64_t, 32>().i };
std::int32_t d3{ d<std::int64_t, 33>().i };
// since-cxx11-error@-1 {{non-constant-expression cannot be narrowed from type 'long long' to 'std::int32_t' (aka 'int') in initializer list}}
// since-cxx11-note@-2 {{insert an explicit cast to silence this issue}}

std::int16_t d6{ d<int, 16>().i };
std::int16_t d7{ d<unsigned, 15>().i };
std::int16_t d8{ d<unsigned, 16>().i };
// since-cxx11-error@-1 {{non-constant-expression cannot be narrowed from type 'unsigned int' to 'std::int16_t' (aka 'short') in initializer list}}
// since-cxx11-note@-2 {{insert an explicit cast to silence this issue}}
std::uint16_t d9{ d<unsigned, 16>().i };
std::uint16_t da{ d<int, 1>().i };
// since-cxx11-error@-1 {{non-constant-expression cannot be narrowed from type 'int' to 'std::uint16_t' (aka 'unsigned short') in initializer list}}
// since-cxx11-note@-2 {{insert an explicit cast to silence this issue}}

bool db{ d<unsigned, 1>().i };
bool dc{ d<int, 1>().i };
// since-cxx11-error@-1 {{non-constant-expression cannot be narrowed from type 'int' to 'bool' in initializer list}}
// since-cxx11-note@-2 {{insert an explicit cast to silence this issue}}

template<typename Target, typename Source>
constexpr decltype(Target{ std::declval<Source>().i }, false) is_narrowing(int) { return false; }
template<typename Target, typename Source>
constexpr bool is_narrowing(long) { return true; }

static_assert(!is_narrowing<std::int16_t, D<int, 16>>(0), "");
static_assert(!is_narrowing<std::int16_t, D<unsigned, 15>>(0), "");
static_assert(is_narrowing<std::int16_t, D<unsigned, 16>>(0), "");
static_assert(!is_narrowing<std::uint16_t, D<unsigned, 16>>(0), "");
static_assert(is_narrowing<std::uint16_t, D<int, 1>>(0), "");
static_assert(!is_narrowing<bool, D<unsigned, 1>>(0), "");
static_assert(is_narrowing<bool, D<int, 1>>(0), "");

template<int N>
struct E {
signed int x : N;
decltype(std::int16_t{ x }) dependent_narrowing;
decltype(unsigned{ x }) always_narrowing;
// since-cxx11-error@-1 {{non-constant-expression cannot be narrowed from type 'int' to 'unsigned int' in initializer list}}
// since-cxx11-note@-2 {{insert an explicit cast to silence this issue}}
};
#endif
} // namespace cwg2627

namespace cwg2628 { // cwg2628: no
// this was reverted for the 16.x release
// due to regressions, see the issue for more details:
Expand Down
7 changes: 7 additions & 0 deletions clang/test/Sema/constexpr.c
Original file line number Diff line number Diff line change
Expand Up @@ -360,3 +360,10 @@ void infsNaNs() {

constexpr struct S9 s9 = { }; // expected-error {{variable has incomplete type 'const struct S9'}} \
// expected-note {{forward declaration of 'struct S9'}}

struct S10 {
signed long long i : 8;
};
constexpr struct S10 c = { 255 };
// FIXME-expected-error@-1 {{constexpr initializer evaluates to 255 which is not exactly representable in 'long long' bit-field with width 8}}
// See: GH#101299
36 changes: 36 additions & 0 deletions clang/test/SemaCXX/bitint-narrowing.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
// RUN: %clang_cc1 -triple x86_64 -fsyntax-only -verify -std=c++11 %s
// RUN: %clang_cc1 -triple i386 -fsyntax-only -verify -std=c++11 %s

struct {
_BitInt(35) i : 33;
} x;
struct {
_BitInt(35) i : 34;
} y;
_BitInt(33) xx{ x.i };
_BitInt(33) yy{ y.i };
// expected-error@-1 {{non-constant-expression cannot be narrowed from type '_BitInt(35)' to '_BitInt(33)' in initializer list}}
// FIXME-expected-note@-2 {{insert an explicit cast to silence this issue}}

_BitInt(2) S2 = 0;
unsigned _BitInt(2) U2 = 0;
_BitInt(3) S3 = 0;
unsigned _BitInt(3) U3 = 0;

_BitInt(2) bi0{ S2 };
_BitInt(2) bi1{ U2 }; // expected-error {{non-constant-expression cannot be narrowed from type 'unsigned _BitInt(2)' to '_BitInt(2)' in initializer list}}
_BitInt(2) bi2{ S3 }; // expected-error {{non-constant-expression cannot be narrowed from type '_BitInt(3)' to '_BitInt(2)' in initializer list}}
_BitInt(2) bi3{ U3 }; // expected-error {{non-constant-expression cannot be narrowed from type 'unsigned _BitInt(3)' to '_BitInt(2)' in initializer list}}
unsigned _BitInt(2) bi4{ S2 }; // expected-error {{non-constant-expression cannot be narrowed from type '_BitInt(2)' to 'unsigned _BitInt(2)' in initializer list}}
unsigned _BitInt(2) bi5{ U2 };
unsigned _BitInt(2) bi6{ S3 }; // expected-error {{non-constant-expression cannot be narrowed from type '_BitInt(3)' to 'unsigned _BitInt(2)' in initializer list}}
unsigned _BitInt(2) bi7{ U3 }; // expected-error {{non-constant-expression cannot be narrowed from type 'unsigned _BitInt(3)' to 'unsigned _BitInt(2)' in initializer list}}
_BitInt(3) bi8{ S2 };
_BitInt(3) bi9{ U2 };
_BitInt(3) bia{ S3 };
_BitInt(3) bib{ U3 }; // expected-error {{non-constant-expression cannot be narrowed from type 'unsigned _BitInt(3)' to '_BitInt(3)' in initializer list}}
unsigned _BitInt(3) bic{ S2 }; // expected-error {{non-constant-expression cannot be narrowed from type '_BitInt(2)' to 'unsigned _BitInt(3)' in initializer list}}
unsigned _BitInt(3) bid{ U2 };
unsigned _BitInt(3) bie{ S3 }; // expected-error {{non-constant-expression cannot be narrowed from type '_BitInt(3)' to 'unsigned _BitInt(3)' in initializer list}}
unsigned _BitInt(3) bif{ U3 };
2 changes: 1 addition & 1 deletion clang/www/cxx_dr_status.html
Original file line number Diff line number Diff line change
Expand Up @@ -15577,7 +15577,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/2627.html">2627</a></td>
<td>C++23</td>
<td>Bit-fields and narrowing conversions</td>
<td class="unknown" align="center">Unknown</td>
<td class="unreleased" align="center">Clang 20</td>
</tr>
<tr id="2628">
<td><a href="https://cplusplus.github.io/CWG/issues/2628.html">2628</a></td>
Expand Down