This is an archive of the discontinued LLVM Phabricator instance.

Improve the representation of <compare>'s zero-only type.
ClosedPublic

Authored by rsmith on Jul 31 2020, 3:16 PM.

Details

Summary
  • Use an empty struct instead of a member pointer to represent this type, so that we don't actually pass a zero member pointer at runtime.
  • Mark the constructor as consteval to ensure that no code is emitted for it whenever possible.
  • Add a honeypot constructor to reject all non-int arguments, so that the only argument that can arrive at the real constructor is the literal 0.

This results in better generated code, and rejecting invalid comparisons
against nullptr, 0L, and so on, while also rejecting invalid comparisons
against (1-1) and similar that would be allowed if we required an
integer constant expression with value 0.

Diff Detail

Event Timeline

rsmith created this revision.Jul 31 2020, 3:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2020, 3:16 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
rsmith requested review of this revision.Jul 31 2020, 3:16 PM
rsmith edited reviewers, added: ldionne; removed: mclow.lists.Jul 31 2020, 3:19 PM
ldionne added inline comments.Aug 3 2020, 8:40 AM
libcxx/test/std/language.support/cmp/cmp.categories.pre/zero_type.fail.cpp
1

Can you name this file zero_type.verify.cpp instead?

19

You could take both op and v here to avoid v coming from "nowhere".

ldionne requested changes to this revision.Aug 3 2020, 8:40 AM
This revision now requires changes to proceed.Aug 3 2020, 8:40 AM
rsmith updated this revision to Diff 290191.Sep 7 2020, 12:49 AM
  • Address review comments.
  • Add some positive tests.
rsmith marked 2 inline comments as done.Sep 7 2020, 12:49 AM
ldionne accepted this revision.Sep 8 2020, 4:49 AM
This revision is now accepted and ready to land.Sep 8 2020, 4:49 AM

@rsmith Would you like me to commit this?

This revision was landed with ongoing or failed builds.Sep 29 2020, 3:44 PM
This revision was automatically updated to reflect the committed changes.

@rsmith Would you like me to commit this?

Sorry for the delay :) Committed.