This is an archive of the discontinued LLVM Phabricator instance.

[llvm][ADT] Implement `BitVector::{pop_,}back`
ClosedPublic

Authored by jansvoboda11 on Jan 12 2022, 7:10 AM.

Details

Summary

LLVM Programmer’s Manual strongly discourages the use of std::vector<bool> and suggests llvm::BitVector as a possible replacement.

Currently, some users of std::vector<bool> cannot switch to llvm::BitVector because it doesn't implement the pop_back() and back() functions.

To enable easy transition of std::vector<bool> users, this patch implements llvm::BitVector::pop_back() and llvm::BitVector::back().

Diff Detail

Event Timeline

jansvoboda11 requested review of this revision.Jan 12 2022, 7:10 AM
jansvoboda11 created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJan 12 2022, 7:10 AM
dexonsmith accepted this revision.Jan 12 2022, 1:05 PM

Given that BitVector::back() already exists this makes sense.

LGTM, after you add some !empty() assertions for clarity if/when these are misused.

llvm/include/llvm/ADT/BitVector.h
456

I suggest asserting !empty() here for clarity of the assertion message.

480

I suggest asserting !empty() here for clarity of the assertion message.

llvm/include/llvm/ADT/SmallBitVector.h
475

I suggest asserting !empty() here for clarity of the assertion message.

487

I suggest asserting !empty() here for clarity of the assertion message.

This revision is now accepted and ready to land.Jan 12 2022, 1:05 PM
This revision was landed with ongoing or failed builds.Jan 21 2022, 5:51 AM
This revision was automatically updated to reflect the committed changes.
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJan 21 2022, 5:51 AM