lebedev.ri added a comment. The checking by itself looks sane-ish, but i don't have any reasonable knowledge in this area.
================ Comment at: lib/Sema/SemaChecking.cpp:925 + case Builtin::BI__builtin_signbitl: if (SemaBuiltinFPClassification(TheCall, 1)) return ExprError(); ---------------- The name of the function is unfortunate given that you call it for `__builtin_signbit(int)`. ================ Comment at: test/Sema/builtins.c:260 + (void)__builtin_signbit(1.0, 2.0, 3.0); // expected-error{{too many arguments to function call, expected 1, have 3}} + (void)__builtin_signbit(1); // expected-error {{floating point classification requires argument of floating point type (passed in 'int')}} + ---------------- What about ``` (void)__builtin_signbit(1.0); ``` ================ Comment at: test/Sema/builtins.c:260 + (void)__builtin_signbit(1.0, 2.0, 3.0); // expected-error{{too many arguments to function call, expected 1, have 3}} + (void)__builtin_signbit(1); // expected-error {{floating point classification requires argument of floating point type (passed in 'int')}} + ---------------- lebedev.ri wrote: > What about > ``` > (void)__builtin_signbit(1.0); > ``` Hm, is `__builtin_signbit()` taking an integer, or float? If integer, the comment about `floating point classification` is slightly misleading. ================ Comment at: test/Sema/builtins.c:264 + (void)__builtin_signbitf(1.0, 2.0, 3.0); // expected-error{{too many arguments to function call, expected 1, have 3}} + (void)__builtin_signbitf(1); + ---------------- `1` will be promoted to float? I'd add one more test with native float: ``` (void)__builtin_signbit(1.0); ``` ================ Comment at: test/Sema/builtins.c:268 + (void)__builtin_signbitl(1.0, 2.0, 3.0); // expected-error{{too many arguments to function call, expected 1, have 3}} + (void)__builtin_signbitl(1); +} ---------------- Same, would be good to see `__builtin_signbitf(1.0);`, `__builtin_signbitf(1.0L);`. Repository: rC Clang https://reviews.llvm.org/D47435 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits