jtmott-intel marked 6 inline comments as done. jtmott-intel added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7939 "to a non-const integer (%0 invalid)">; +def err_overflow_builtin_extint_size : Error< + "_ExtInt argument larger than 64-bits to overflow builtin requires runtime " ---------------- rjmccall wrote: > jtmott-intel wrote: > > rjmccall wrote: > > > erichkeane wrote: > > > > Mentioning target-specific support here seems incorrect. @rjmccall I > > > > cannot think of a better wording, can you? > > > "overflow builtins do not support _ExtInt operands of more than %0 bits > > > on this target"? I don't think it's unreasonable to mention the > > > target-specificness of it. Hard-coding the number 64 in the diagnostic > > > seems excessively targeted, though. > > I discovered there's a good existing message I could use that already takes > > the bitwidth as a parameter. I decided not to add a new one. > > Thoughts/preferences? Here's how the message would come out. > > > > test.c:5:43: error: signed _ExtInt of bit sizes greater than 128 not > > supported > > _Bool status = __builtin_mul_overflow(x, y, &result); > > ^ > > 1 error generated. > > > It'd be much clearer to say something about the fact that it's just this > builtin that's not supported. Updated message to something halfway between John and Erich's suggestions. Current result: test.c:5:43: error: __builtin_mul_overflow does not support signed _ExtInt operands of more than 128 bits _Bool status = __builtin_mul_overflow(x, y, &result); ^ 1 error generated. ================ 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}} + } ---------------- erichkeane wrote: > 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. Updated message to something halfway between your and John's suggestions. Current result: test.c:5:43: error: __builtin_mul_overflow does not support signed _ExtInt operands of more than 128 bits _Bool status = __builtin_mul_overflow(x, y, &result); ^ 1 error generated. 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