Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tests] Enable span tests now that libc++ has been updated #839

Merged
merged 3 commits into from
May 19, 2020

Conversation

miscco
Copy link
Contributor

@miscco miscco commented May 15, 2020

It finally happened and the updates to std::span landed in libc++

Consequently we should now be able to use those tests.

@miscco miscco requested a review from a team as a code owner May 15, 2020 08:23
@StephanTLavavej StephanTLavavej added the test Related to test code label May 15, 2020
Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks like other updates to the expected/skip lists are necessary, though.

@miscco
Copy link
Contributor Author

miscco commented May 15, 2020

So looking at this a bit I guess we should:

  • Fix the offending const int* const test because it indeed doesnt make too much sense.
  • Skip the utf8 check
  • Investigate the std::function test

@CaseyCarter
Copy link
Member

CaseyCarter commented May 15, 2020

So looking at this a bit I guess we should:

  • Fix the offending const int* const test because it indeed doesn't make too much sense.

Yeah, it's easiest to restructure this test to avoid the warning.

  • Skip the utf8 check

Unskip - this is an XFAIL that now succeeds since someone finally looked at the PR I submitted last May.

  • Investigate the std::function test

This is testing UB by passing an insufficiently-complete type to is_assignable.

You forgot "disable the bogus portion of the shared_ptr test that won't work on libraries that have implemented LWG-2996" ;)

I pushed changes to skip/XFAIL these tests as appropriate, and submitted https://reviews.llvm.org/D80030 upstream to fix them. (Feel free to look and comment, but please don't approve or it will break their workflow and no one with approver powers will ever notice they need to look at it.)

@miscco
Copy link
Contributor Author

miscco commented May 16, 2020

Thanks a lot for the help

@StephanTLavavej StephanTLavavej self-assigned this May 18, 2020
@StephanTLavavej StephanTLavavej merged commit ff94756 into microsoft:master May 19, 2020
@StephanTLavavej
Copy link
Member

Thanks for updating libc++'s test coverage upstream and here - we really, really appreciate it! 😺 😸 😺

This will lower the "Libcxx Skips" line in the Status Chart too.

@miscco miscco deleted the span_llvm branch June 3, 2020 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Related to test code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants