This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] span: Fix incorrect static asserts
ClosedPublic

Authored by miscco on Dec 30 2019, 5:06 AM.

Details

Summary

The static asserts in span<T, N>::front() and span<T, N>::back() are incorrect as they may be triggered from valid code
due to evaluation of a never taken branch.

span<int, 0> foo;
if (!foo.empty()) {
    auto x = foo.front();
}

The problem is that the branch is always evaluated by the compiler creating invalid compile errors for span<T, 0>.

Diff Detail

Event Timeline

miscco created this revision.Dec 30 2019, 5:06 AM
Herald added a project: Restricted Project. · View Herald Transcript
miscco edited the summary of this revision. (Show Details)Dec 30 2019, 5:33 AM
miscco added a reviewer: ldionne.
ldionne requested changes to this revision.Feb 11 2020, 3:05 AM

It's sad to get rid of the opportunistic static_assert, but I agree your example should compile. Can you add tests with your example?

This revision now requires changes to proceed.Feb 11 2020, 3:05 AM
miscco updated this revision to Diff 243797.Feb 11 2020, 3:47 AM

Add tests

@ldionne I aggree with your sentiment and I only discovered this when I implemented span for MSVC STL and added the same static assert to operator[]

I believe that this should be something for if consteval

I added some tests

miscco updated this revision to Diff 243798.Feb 11 2020, 3:49 AM

Remove superfluous newlines

Harbormaster completed remote builds in B46203: Diff 243798.
miscco updated this revision to Diff 243807.Feb 11 2020, 4:23 AM

Make STL happy, as they apply [[nodiscard]] everywhere

ldionne accepted this revision.Feb 14 2020, 5:32 AM

Thanks!

This revision is now accepted and ready to land.Feb 14 2020, 5:32 AM
This revision was automatically updated to reflect the committed changes.