[clang][Interp] Implement IntegralAP subtraction

[tbaederr]

The tests currently fail because they need one of the other open IntegralAP PRs.

Will update this once they are pushed.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

The tests currently fail because they need one of the other open IntegralAP PRs.

Will update this once they are pushed.

---

Full diff: https://github.com/llvm/llvm-project/pull/71648.diff

2 Files Affected:

• (modified) clang/lib/AST/Interp/IntegralAP.h (+14-20)
• (modified) clang/test/AST/Interp/intap.cpp (+15)

diff --git a/clang/lib/AST/Interp/IntegralAP.h b/clang/lib/AST/Interp/IntegralAP.h
index 6d301bad784af47..9a9a886ede47216 100644
--- a/clang/lib/AST/Interp/IntegralAP.h
+++ b/clang/lib/AST/Interp/IntegralAP.h
@@ -183,12 +183,11 @@ template <bool Signed> class IntegralAP final {
   }
 
   static bool add(IntegralAP A, IntegralAP B, unsigned OpBits, IntegralAP *R) {
-    return CheckAddUB(A, B, OpBits, R);
+    return CheckAddSubUB<std::plus>(A, B, OpBits, R);
   }
 
   static bool sub(IntegralAP A, IntegralAP B, unsigned OpBits, IntegralAP *R) {
-    /// FIXME: Gotta check if the result fits into OpBits bits.
-    return CheckSubUB(A, B, R);
+    return CheckAddSubUB<std::minus>(A, B, OpBits, R);
   }
 
   static bool mul(IntegralAP A, IntegralAP B, unsigned OpBits, IntegralAP *R) {
@@ -256,28 +255,23 @@ template <bool Signed> class IntegralAP final {
   }
 
 private:
-  static bool CheckAddUB(const IntegralAP &A, const IntegralAP &B,
-                         unsigned BitWidth, IntegralAP *R) {
-    if (!A.isSigned()) {
-      R->V = A.V + B.V;
+  template <template <typename T> class Op>
+  static bool CheckAddSubUB(const IntegralAP &A, const IntegralAP &B,
+                            unsigned BitWidth, IntegralAP *R) {
+    if constexpr (!Signed) {
+      auto UOp = Op<APInt>();
+      R->V = UOp(A.V, B.V);
       return false;
     }
 
-    const APSInt &LHS = APSInt(A.V, A.isSigned());
-    const APSInt &RHS = APSInt(B.V, B.isSigned());
-
-    APSInt Value(LHS.extend(BitWidth) + RHS.extend(BitWidth), false);
+    auto SOp = Op<APSInt>();
+    const APSInt &LHS = A.toAPSInt();
+    const APSInt &RHS = B.toAPSInt();
+    APSInt Value(SOp(LHS.extend(BitWidth), RHS.extend(BitWidth)), false);
     APSInt Result = Value.trunc(LHS.getBitWidth());
-    if (Result.extend(BitWidth) != Value)
-      return true;
-
     R->V = Result;
-    return false;
-  }
-  static bool CheckSubUB(const IntegralAP &A, const IntegralAP &B,
-                         IntegralAP *R) {
-    R->V = A.V - B.V;
-    return false; // Success!
+
+    return Result.extend(BitWidth) != Value;
   }
 };
 
diff --git a/clang/test/AST/Interp/intap.cpp b/clang/test/AST/Interp/intap.cpp
index 45961e6fc74b7a7..a6f1fc4e38dfca6 100644
--- a/clang/test/AST/Interp/intap.cpp
+++ b/clang/test/AST/Interp/intap.cpp
@@ -11,7 +11,12 @@ constexpr _BitInt(2) B = A + 1;
 constexpr _BitInt(2) C = B + 1; // expected-warning {{from 2 to -2}} \
                                 // ref-warning {{from 2 to -2}}
 static_assert(C == -2, "");
+static_assert(C - B == A, ""); // expected-error {{not an integral constant expression}} \
+                               // expected-note {{value -3 is outside the range of representable values}} \
+                               // ref-error {{not an integral constant expression}} \
+                               // ref-note {{value -3 is outside the range of representable values}}
 
+static_assert(B - 1 == 0, "");
 
 constexpr MaxBitInt A_ = 0;
 constexpr MaxBitInt B_ = A_ + 1;
@@ -121,6 +126,16 @@ namespace i128 {
                                            // expected-warning {{implicit conversion of out of range value}} \
                                            // expected-error {{must be initialized by a constant expression}} \
                                            // expected-note {{is outside the range of representable values of type}}
+
+  constexpr uint128_t Zero = 0;
+  static_assert((Zero -1) == -1, "");
+  constexpr int128_t Five = 5;
+  static_assert(Five - Zero == Five, "");
+
+  constexpr int128_t Sub1 = INT128_MIN - 1; // expected-error {{must be initialized by a constant expression}} \
+                                            // expected-note {{-170141183460469231731687303715884105729 is outside the range}} \
+                                            // ref-error {{must be initialized by a constant expression}} \
+                                            // ref-note {{-170141183460469231731687303715884105729 is outside the range}}
 }
 
 namespace AddSubOffset {

[AaronBallman]

Feel free to ignore the suggestions if you think they're not useful, but I tend to prefer not using a local variable for something used as a functor unless it's going to be reused.

Generally LG but I'd prefer to hold off on approval until CI is green with the changes.

[tbaederr]

Tests should work now

... and they do!

[tbaederr]

Ping

[AaronBallman]

LGTM!