[clang] Add bit-precise overloads for builtin operators (PR #84755)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 11 09:26:25 PDT 2024


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 <aaron at 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 <aaron at 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 <aaron at 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 <aaron at 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.");
   }
 



More information about the cfe-commits mailing list