This is an archive of the discontinued LLVM Phabricator instance.

Update __is_unsigned builtin to match the Standard.
ClosedPublic

Authored by zoecarver on Mar 5 2021, 10:31 PM.

Details

Summary

Updates __is_unsigned to have the same behavior as the standard specifies. This is in line with 511dbd8, which applied the same change to __is_signed.

Refs D67897.

Diff Detail

Event Timeline

zoecarver requested review of this revision.Mar 5 2021, 10:31 PM
zoecarver created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMar 5 2021, 10:32 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
zoecarver edited the summary of this revision. (Show Details)Mar 5 2021, 10:32 PM
zoecarver added inline comments.
clang/docs/LanguageExtensions.rst
1199

Just re-flowing the line here, which I neglected to do in the other commit.

🎉

clang/lib/Sema/SemaExprCXX.cpp
4863

FWIW, I'd lose the parenthetical comment, and I'd write the conditions as respectively

return T->isUnsignedIntegerType() && !T->isEnumeralType();

return T->isFloatingType() || (T->isSignedIntegerType() && !T->isEnumeralType());

both because "T not being an enum type" is expected to be usually true, and because I think (!x && y) runs the risk of being misread as !(x && y), whereas (x && !y) is easy on the brain.

clang/test/SemaCXX/type-traits.cpp
1473

I think you should check F(__is_unsigned(SignedEnum)) here, too, just for the record.
And F(__is_signed(UnsignedEnum)) in the other place.

zoecarver added inline comments.Mar 7 2021, 1:55 PM
clang/lib/Sema/SemaExprCXX.cpp
4863

Fair enough. I originally was thinking, if we have to perform this check roughly 50% of this time, either way, it makes sense to do the much faster check first. But, if we assume that we almost never receive an enum type, then what you're saying makes sense. I'll fix it.

zoecarver updated this revision to Diff 328905.Mar 7 2021, 1:59 PM
  • Address Arthur's comments.

LGTM now, mod the (important!) typo in the test.

clang/test/SemaCXX/type-traits.cpp
1444

s/is_unsigned/is_signed/

rsmith added inline comments.Mar 8 2021, 2:54 PM
clang/docs/LanguageExtensions.rst
1206–1207

Please add a trailing colon to this line to match the other lines with additional text after the parentheses.

clang/lib/Sema/SemaExprCXX.cpp
4859–4860

Please reflow this line so it fits in 80 columns.

rsmith added a comment.Mar 8 2021, 2:55 PM

LGTM too, once the remaining pending comments are addressed.

Sorry for the delay in updating. All review comments have been addressed. Thanks!

zoecarver updated this revision to Diff 329761.Mar 10 2021, 1:32 PM
  • Fix review comments: add colon, reflow line, and fix typo.
rsmith accepted this revision.Mar 10 2021, 2:27 PM
This revision is now accepted and ready to land.Mar 10 2021, 2:27 PM
This revision was landed with ongoing or failed builds.Mar 10 2021, 3:00 PM
This revision was automatically updated to reflect the committed changes.