erichkeane added inline comments.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:330
+
+ // Disallow ExtIntType args larger than 128 bits to mul function until we
+ // improve backend support.
----------------
Disallow _signed_ ...
================
Comment at: clang/lib/Sema/SemaChecking.cpp:334
+ for (unsigned I = 0; I < 3; ++I) {
+ ExprResult Arg = TheCall->getArg(I);
+ // Third argument will be a pointer
----------------
Since you're always doing "Arg.get" below, this should probably just store the
Expr*.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:335
+ ExprResult Arg = TheCall->getArg(I);
+ // Third argument will be a pointer
+ QualType Ty =
----------------
Comments must end in a period.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:337
+ QualType Ty =
+ I < 2 ? Arg.get()->getType()
+ : Arg.get()->getType()->getAs<PointerType>()->getPointeeType();
----------------
Try:
Type *Ty = ArgExpr->getType()->getPointeeOrArrayElementType();
instead of the ternary.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:340
+ if (Ty->isExtIntType() && Ty->isSignedIntegerType() &&
+ S.getASTContext().getIntWidth(Ty) > 128) {
+ S.Diag(Arg.get()->getBeginLoc(), diag::err_ext_int_max_size)
----------------
remove the curley brackets, and make this:
return S.Diag(....
================
Comment at: clang/test/Sema/builtins-overflow.c:39
+ _ExtInt(129) result;
+ _Bool status = __builtin_mul_overflow(x, y, &result); // expected-error
{{signed _ExtInt of bit sizes greater than 128 not supported}}
+ }
----------------
As @rjmccall said, I'd suggest the new message anyway that mentions the
builtin, otherwise this would be confusing for me. Something like:
signed argument to __builtin_mul_overflow must have a bitwidth of less than or
equal to 128.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81420/new/
https://reviews.llvm.org/D81420
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
