Skip to content

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

Merged
Stephan T. Lavavej (StephanTLavavej) merged 3 commits into
microsoft:masterfrom
miscco:span_llvm
May 19, 2020
Merged

Stephan T. Lavavej (StephanTLavavej) merged 3 commits into
microsoft:masterfrom
miscco:span_llvm

Conversation

@miscco

Copy link
Copy Markdown
Contributor

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

Consequently we should now be able to use those tests.

Copy link
Copy Markdown
Member

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
Copy Markdown
Contributor Author

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

Casey Carter (CaseyCarter) commented May 15, 2020 •

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor Author

Thanks a lot for the help

@StephanTLavavej

Copy link
Copy Markdown
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.

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