[PATCH] D67897: Fix __is_signed builtin

2019-09-23 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver closed this revision. zoecarver added a comment. Resolved by rL372621 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67897/new/ https://reviews.llvm.org/D67897 ___

[PATCH] D67897: Fix __is_signed builtin

2019-09-23 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver marked an inline comment as done. zoecarver added inline comments. Comment at: clang/docs/LanguageExtensions.rst:1165 Note that this currently returns true for enumeration types if the underlying - type is signed, and returns false for floating-point types, in viola

[PATCH] D67897: Fix __is_signed builtin

2019-09-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. Thanks! Comment at: clang/docs/LanguageExtensions.rst:1165 Note that this currently returns true for enumeration types if the underlying - type is signed, and returns false for floating-point types, in violation of - the requirements for ``std::is_s

[PATCH] D67897: Fix __is_signed builtin

2019-09-22 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver updated this revision to Diff 221253. zoecarver added a comment. - fix behavior when passed an enumeration type Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67897/new/ https://reviews.llvm.org/D67897 Files: clang/docs/LanguageExtensio

[PATCH] D67897: Fix __is_signed builtin

2019-09-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. In D67897#1678420 , @zoecarver wrote: > > (Can I interest you in fixing the misbehaviour for enumeration types too?) > > Certainly. You mean that `__is_

[PATCH] D67897: Fix __is_signed builtin

2019-09-22 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver updated this revision to Diff 221247. zoecarver added a comment. - fix docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67897/new/ https://reviews.llvm.org/D67897 Files: clang/docs/LanguageExtensions.rst clang/lib/Sema/SemaExprCXX.

[PATCH] D67897: Fix __is_signed builtin

2019-09-22 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver added a comment. > (Can I interest you in fixing the misbehaviour for enumeration types too?) Certainly. You mean that `__is_signed` should return false for enumeration types? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67897/new/ htt

[PATCH] D67897: Fix __is_signed builtin

2019-09-22 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. In D67897#1678392 , @rsmith wrote: > In D67897#1678388 , @Quuxplusone > wrote: > > > But `std::is_signed_v` needs to yield `false`. > > > It should yield `true`; the spec says "If is_­a

[PATCH] D67897: Fix __is_signed builtin

2019-09-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D67897#1678388 , @Quuxplusone wrote: > But `std::is_signed_v` needs to yield `false`. It should yield `true`; the spec says "If is_­arithmetic_­v is true, the same result as T(-1) < T(0); otherwise, false". Repository: rG

[PATCH] D67897: Fix __is_signed builtin

2019-09-22 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver added a comment. In D67897#1678388 , @Quuxplusone wrote: > But `std::is_signed_v` needs to yield `false`. Isn't it cleaner to > leave the compiler builtin matching the library type-trait, so that the > library doesn't have to check for integral

[PATCH] D67897: Fix __is_signed builtin

2019-09-22 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. Looks good, but please also update http://clang.llvm.org/docs/LanguageExtensions.html#type-trait-primitives (Can I interest you in fixing the misbehaviour for enumeration types too?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D67897: Fix __is_signed builtin

2019-09-22 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. But `std::is_signed_v` needs to yield `false`. Isn't it cleaner to leave the compiler builtin matching the library type-trait, so that the library doesn't have to check for integral types separately? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D67897: Fix __is_signed builtin

2019-09-22 Thread Zoe Carver via Phabricator via cfe-commits
zoecarver created this revision. zoecarver added reviewers: EricWF, rsmith, erichkeane, craig.topper, efriedma. Herald added a project: clang. Herald added a subscriber: cfe-commits. This patch fixes the __is_signed builtin type trait to work with floating point types. Now, the builtin will retur