This is an archive of the discontinued LLVM Phabricator instance.

[libc++][spaceship] Additional tests for `operator<=>` `map` and `multimap`
ClosedPublic

Authored by H-G-Hristov on May 23 2023, 5:52 AM.

Details

Summary
  • Added additional tests
  • Improved existing tests
  • Moved misplaced test files to the correct location

Diff Detail

Event Timeline

H-G-Hristov created this revision.May 23 2023, 5:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 23 2023, 5:52 AM
Herald added a subscriber: yaxunl. · View Herald Transcript
H-G-Hristov edited the summary of this revision. (Show Details)May 23 2023, 5:53 AM
H-G-Hristov retitled this revision from [libc++][spaceship] Additional tests for `operator<=>` - `map` and `multimap` to [libc++][spaceship] Additional tests for `operator<=>` `map` and `multimap`.

Try to fix CI

Try to fix CI: Ignore expected-error message with {{}} syntax due to differing error message locally vs remotely (on CI).

H-G-Hristov edited the summary of this revision. (Show Details)May 23 2023, 8:40 AM
H-G-Hristov edited the summary of this revision. (Show Details)
H-G-Hristov published this revision for review.May 23 2023, 12:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 23 2023, 12:24 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
H-G-Hristov edited the summary of this revision. (Show Details)May 23 2023, 12:25 PM
H-G-Hristov edited the summary of this revision. (Show Details)

Try to fix CI: Ignore expected-error message with {{}} syntax due to differing error message locally vs remotely (on CI).

You probably want to update your local compiler then. We only support Clang 15, 16 and trunk.

libcxx/test/std/containers/associative/map/map.nonmember/compare.three_way.verify.cpp
22

This include doesn't seem to be used,

29

IMO it would be better to have two separate // expected-errors to make sure they are actually generated by the two lines below.

Minor tweaks

H-G-Hristov added inline comments.May 23 2023, 2:14 PM
libcxx/test/std/containers/associative/map/map.nonmember/compare.three_way.verify.cpp
22

It is used for the last test.

29

Thanks! That makes sense. I'll update my tests.

Try to fix CI: Ignore expected-error message with {{}} syntax due to differing error message locally vs remotely (on CI).

You probably want to update your local compiler then. We only support Clang 15, 16 and trunk.

My local compiler is Apple Clang 14.0.3 which is based on Clang 15, which is the latest from Apple.

Try to fix CI: Ignore expected-error message with {{}} syntax due to differing error message locally vs remotely (on CI).

You probably want to update your local compiler then. We only support Clang 15, 16 and trunk.

My local compiler is Apple Clang 14.0.3 which is based on Clang 15, which is the latest from Apple.

What is the difference in error messages then? Can you check the actual message with a regex maybe? (// expected-error-re {{... {{regex}} ...}})

Try to fix CI: Ignore expected-error message with {{}} syntax due to differing error message locally vs remotely (on CI).

You probably want to update your local compiler then. We only support Clang 15, 16 and trunk.

My local compiler is Apple Clang 14.0.3 which is based on Clang 15, which is the latest from Apple.

What is the difference in error messages then? Can you check the actual message with a regex maybe? (// expected-error-re {{... {{regex}} ...}})

I just updated this test with the `//expected-error-re {{... {{regex}} }} syntax now. Apple Clang outputs "static_assert" vs "static assertion" on the CI. I've seen a patch that you want to remove this syntax. Then I guess may need to revert this change and figure out how to setup a non-Apple compiler.

I just updated this test with the `//expected-error-re {{... {{regex}} }} syntax now. Apple Clang outputs "static_assert" vs "static assertion" on the CI. I've seen a patch that you want to remove this syntax. Then I guess may need to revert this change and figure out how to setup a non-Apple compiler.

I thought AppleClang also outputs static assertion. If that's not the case yet, I guess I have to wait until we update it.

Addressed review comments

H-G-Hristov marked 2 inline comments as done.May 25 2023, 2:41 AM

@philnik Thank you for the review.

libcxx/test/std/containers/associative/map/map.nonmember/compare.three_way.verify.cpp
22

It is used for the last test.

Moved the test with the allocator to the top.

Rebased + fixed merge conflicts

Rebased to fix CI

@Mordante FYI this patch adds the additional tests to map you requested.

philnik accepted this revision.May 30 2023, 3:21 PM
This revision is now accepted and ready to land.May 30 2023, 3:21 PM