[PATCH] D39579: BuiltinOperatorOverloadBuilder: Don't consider types that are unavailable on the target (PR35174)

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/Sema/SemaOverload.cpp:7669
+    // End of integral types.
+    // FIXME: What about complex? What about half?/
   }
----------------
Spurious trailing slash.


================
Comment at: lib/Sema/SemaOverload.cpp:7731-7746
     // Validate some of our static helper constants in debug builds.
-    assert(getArithmeticType(FirstPromotedIntegralType) == S.Context.IntTy &&
+    assert(ArithmeticTypes[FirstPromotedIntegralType] == S.Context.IntTy &&
            "Invalid first promoted integral type");
-    assert(getArithmeticType(LastPromotedIntegralType - 1)
-             == S.Context.UnsignedInt128Ty &&
+    assert((ArithmeticTypes[LastPromotedIntegralType - 1] ==
+                S.Context.UnsignedInt128Ty ||
+            ArithmeticTypes[LastPromotedIntegralType - 1] ==
+                S.Context.UnsignedLongLongTy) &&
----------------
I don't think these asserts are particularly worthwhile now we're not hardcoding the indices.


================
Comment at: test/CodeGenCXX/microsoft-cannot-mangle-float128.cpp:3-11
+template <bool> struct B;
+template <class T> constexpr bool is_floating_point_v = false;
+
+struct StrictNumeric {
+  StrictNumeric(int);
+  template <typename Dst, B<is_floating_point_v<Dst>> = nullptr> operator Dst();
+};
----------------
This seems like a fragile way of checking for the bug; I'd prefer a test that tests `Sema` rather than code generation. For instance, how about a class with a conversion operator that converts to `T` with an `enable_if` disabling it for `float`, `double`, and `long double`, and a check that an instance of such a class can't be used in floating-point arithmetic?


https://reviews.llvm.org/D39579