This is an archive of the discontinued LLVM Phabricator instance.

Add assert in `constexpr string_view::operator[]`
ClosedPublic

Authored by palmer on Oct 5 2020, 5:17 PM.

Details

Summary

Bounds checking.

Diff Detail

Event Timeline

palmer requested review of this revision.Oct 5 2020, 5:17 PM
palmer created this revision.
miscco accepted this revision.Oct 6 2020, 1:44 AM
miscco added a subscriber: miscco.

I wanted to cry at the use of a comma operator, but all the other member functions do the same...

We might want to directly use __size rather than size() here to avoid the function call in DEBUG mode

This revision is now accepted and ready to land.Oct 6 2020, 1:44 AM
miscco added a comment.Oct 6 2020, 1:44 AM

Note that I am not a maintainer and you should hold on until one accepts it too

ldionne accepted this revision.Oct 6 2020, 10:01 AM

What Author Name <email@domain> do you want this committed as?

Thanks, everyone. Please commit it as "Chris Palmer <palmer@google.com>".

I suspect size() is always inlined, but I can change it to __size if you like. Other code, such as at, uses size().

Herald added a project: Restricted Project. · View Herald TranscriptOct 6 2020, 1:58 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Late LGTM.

I wanted to cry at the use of a comma operator, but all the other member functions do the same...

We discussed this over in https://reviews.llvm.org/D70343. IIUC this is required because libc++'s string_view is available in all dialects, and C++11 only allows you to have a single return statement in a constexpr function.