https://github.com/AaronBallman updated https://github.com/llvm/llvm-project/pull/84755
>From 1f57de0e2e11a4bd834871e7d033ec53c43dc42e Mon Sep 17 00:00:00 2001 From: Aaron Ballman <aa...@aaronballman.com> Date: Mon, 11 Mar 2024 08:41:37 -0400 Subject: [PATCH 1/4] Add bit-precise overloads for builtin operators We previously were not adding them to the candidate set and so use of a bit-precise integer as a class member could lead to ambiguous overload sets. Fixes #82998 --- clang/docs/ReleaseNotes.rst | 3 ++ clang/lib/Sema/SemaOverload.cpp | 23 ++++++++++++++ clang/test/SemaCXX/overload-bitint.cpp | 42 ++++++++++++++++++++++++++ 3 files changed, 68 insertions(+) create mode 100644 clang/test/SemaCXX/overload-bitint.cpp diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 0ee2801766a9cc..ab95d74d4356f5 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -254,6 +254,9 @@ Bug Fixes in This Version operator. Fixes (#GH83267). +- Clang now correctly generates overloads for bit-precise integer types for + builtin operators in C++. Fixes #GH82998. + Bug Fixes to Compiler Builtins ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index b0c693f078efe2..675f6f9ebcda5f 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -8516,6 +8516,9 @@ class BuiltinCandidateTypeSet { /// candidates. TypeSet MatrixTypes; + /// The set of _BitInt types that will be used in the built-in candidates. + TypeSet BitIntTypes; + /// A flag indicating non-record types are viable candidates bool HasNonRecordTypes; @@ -8564,6 +8567,7 @@ class BuiltinCandidateTypeSet { } llvm::iterator_range<iterator> vector_types() { return VectorTypes; } llvm::iterator_range<iterator> matrix_types() { return MatrixTypes; } + llvm::iterator_range<iterator> bitint_types() { return BitIntTypes; } bool containsMatrixType(QualType Ty) const { return MatrixTypes.count(Ty); } bool hasNonRecordTypes() { return HasNonRecordTypes; } @@ -8735,6 +8739,9 @@ BuiltinCandidateTypeSet::AddTypesConvertedFrom(QualType Ty, } else if (Ty->isEnumeralType()) { HasArithmeticOrEnumeralTypes = true; EnumerationTypes.insert(Ty); + } else if (Ty->isBitIntType()) { + HasArithmeticOrEnumeralTypes = true; + BitIntTypes.insert(Ty); } else if (Ty->isVectorType()) { // We treat vector types as arithmetic types in many contexts as an // extension. @@ -8955,6 +8962,22 @@ class BuiltinOperatorOverloadBuilder { (S.Context.getAuxTargetInfo() && S.Context.getAuxTargetInfo()->hasInt128Type())) ArithmeticTypes.push_back(S.Context.UnsignedInt128Ty); + + /// _BitInt overloads are a bit special. We don't want to add candidates + /// for the entire family of _BitInt types, so instead we only add + /// candidates for the unique, unqualified _BitInt types present in the + /// candidate type set. The candidate set already handled ensuring the type + /// is unqualified and canonical, but because we're adding from N different + /// sets, we need to do some extra work to unique things. Copy the + /// candidates into a unique set, then copy from that set into the list of + /// arithmetic types. + llvm::SmallSetVector<CanQualType, 2> BitIntCandidates; + llvm::for_each(CandidateTypes, [&BitIntCandidates]( + BuiltinCandidateTypeSet &Candidate) { + for (QualType BitTy : Candidate.bitint_types()) + BitIntCandidates.insert(CanQualType::CreateUnsafe(BitTy)); + }); + llvm::copy(BitIntCandidates, std::back_inserter(ArithmeticTypes)); LastPromotedIntegralType = ArithmeticTypes.size(); LastPromotedArithmeticType = ArithmeticTypes.size(); // End of promoted types. diff --git a/clang/test/SemaCXX/overload-bitint.cpp b/clang/test/SemaCXX/overload-bitint.cpp new file mode 100644 index 00000000000000..b834a3b01fede6 --- /dev/null +++ b/clang/test/SemaCXX/overload-bitint.cpp @@ -0,0 +1,42 @@ +// RUN: %clang_cc1 -std=c++20 %s -verify +// expected-no-diagnostics + +#include "Inputs/std-compare.h" + +struct S { + _BitInt(12) a; + + constexpr operator _BitInt(12)() const { return a; } +}; + +// None of these used to compile because we weren't adding _BitInt types to the +// overload set for builtin operators. See GH82998. +static_assert(S{10} < 11); +static_assert(S{10} <= 11); +static_assert(S{12} > 11); +static_assert(S{12} >= 11); +static_assert(S{10} == 10); +static_assert((S{10} <=> 10) == 0); +static_assert(S{10} != 11); +static_assert(S{10} + 0 == 10); +static_assert(S{10} - 0 == 10); +static_assert(S{10} * 1 == 10); +static_assert(S{10} / 1 == 10); +static_assert(S{10} % 1 == 0); +static_assert(S{10} << 0 == 10); +static_assert(S{10} >> 0 == 10); +static_assert((S{10} | 0) == 10); +static_assert((S{10} & 10) == 10); +static_assert((S{10} ^ 0) == 10); +static_assert(-S{10} == -10); +static_assert(+S{10} == +10); +static_assert(~S{10} == ~10); + +struct A { + _BitInt(12) a; + + bool operator==(const A&) const = default; + bool operator!=(const A&) const = default; + std::strong_ordering operator<=>(const A&) const = default; +}; + >From 4dfc234d073e8a4b1c4b3f49f817af0da8171c25 Mon Sep 17 00:00:00 2001 From: Aaron Ballman <aa...@aaronballman.com> Date: Mon, 11 Mar 2024 11:41:54 -0400 Subject: [PATCH 2/4] Small optimization to use move instead of copy --- clang/lib/Sema/SemaOverload.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 675f6f9ebcda5f..52e49007dfbb85 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -8977,7 +8977,7 @@ class BuiltinOperatorOverloadBuilder { for (QualType BitTy : Candidate.bitint_types()) BitIntCandidates.insert(CanQualType::CreateUnsafe(BitTy)); }); - llvm::copy(BitIntCandidates, std::back_inserter(ArithmeticTypes)); + llvm::move(BitIntCandidates, std::back_inserter(ArithmeticTypes)); LastPromotedIntegralType = ArithmeticTypes.size(); LastPromotedArithmeticType = ArithmeticTypes.size(); // End of promoted types. >From 1500a15f69301689e75ebebdec0fcfa843b58db0 Mon Sep 17 00:00:00 2001 From: Aaron Ballman <aa...@aaronballman.com> Date: Mon, 11 Mar 2024 11:49:01 -0400 Subject: [PATCH 3/4] Improve comment based on review feedback; NFC --- clang/lib/Sema/SemaOverload.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 52e49007dfbb85..9c524cfa0450be 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -8963,14 +8963,12 @@ class BuiltinOperatorOverloadBuilder { S.Context.getAuxTargetInfo()->hasInt128Type())) ArithmeticTypes.push_back(S.Context.UnsignedInt128Ty); - /// _BitInt overloads are a bit special. We don't want to add candidates - /// for the entire family of _BitInt types, so instead we only add - /// candidates for the unique, unqualified _BitInt types present in the - /// candidate type set. The candidate set already handled ensuring the type - /// is unqualified and canonical, but because we're adding from N different - /// sets, we need to do some extra work to unique things. Copy the - /// candidates into a unique set, then copy from that set into the list of - /// arithmetic types. + /// We add candidates for the unique, unqualified _BitInt types present in + /// the candidate type set. The candidate set already handled ensuring the + /// type is unqualified and canonical, but because we're adding from N + /// different sets, we need to do some extra work to unique things. Insert + /// the candidates into a unique set, then move from that set into the list + /// of arithmetic types. llvm::SmallSetVector<CanQualType, 2> BitIntCandidates; llvm::for_each(CandidateTypes, [&BitIntCandidates]( BuiltinCandidateTypeSet &Candidate) { >From f64cd87aa1ec6870543048afd1cf7957a426f462 Mon Sep 17 00:00:00 2001 From: Aaron Ballman <aa...@aaronballman.com> Date: Mon, 11 Mar 2024 12:22:06 -0400 Subject: [PATCH 4/4] Account for bit-precise types in an assert We're subtracting out the bit-precise types when determining whether we're under the cap because we don't know for sure how many bit-precise types will wind up in the overload set. This assertion exists to let us know when we've hit a point where we need to allocate rather than use the small vector inline storage, so to avoid needing those allocations, I've bumped the cap up by two elements on the assumption that builtin overload set will often not have more than two candidate types. --- clang/lib/Sema/SemaOverload.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp index 9c524cfa0450be..f6bd85bdc64692 100644 --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -8920,7 +8920,7 @@ class BuiltinOperatorOverloadBuilder { SmallVectorImpl<BuiltinCandidateTypeSet> &CandidateTypes; OverloadCandidateSet &CandidateSet; - static constexpr int ArithmeticTypesCap = 24; + static constexpr int ArithmeticTypesCap = 26; SmallVector<CanQualType, ArithmeticTypesCap> ArithmeticTypes; // Define some indices used to iterate over the arithmetic types in @@ -8996,7 +8996,11 @@ class BuiltinOperatorOverloadBuilder { // End of integral types. // FIXME: What about complex? What about half? - assert(ArithmeticTypes.size() <= ArithmeticTypesCap && + // We don't know for sure how many bit-precise candidates were involved, so + // we subtract those from the total when testing whether we're under the + // cap or not. + assert(ArithmeticTypes.size() - BitIntCandidates.size() <= + ArithmeticTypesCap && "Enough inline storage for all arithmetic types."); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits