[PATCH] D77621: ADT: SmallVector size/capacity use word-size integers when elements are small
Nikita Popov via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 24 14:39:56 PDT 2020
nikic added inline comments.
================
Comment at: llvm/include/llvm/ADT/SmallVector.h:84
+template <class Size_T> const size_t SmallVectorBase<Size_T>::SizeTypeMax;
+
----------------
dblaikie wrote:
> nikic wrote:
> > Is this needed? I don't think it makes a lot of sense to allow odr-use of `SizeTypeMax`. As it's a protected member, it's only used in the SmallVector implementation, where we control how it is used.
> It's used as a parameter to std::min, so it's already odr used & I'd rather not leave it as a trap to walk around even if we addressed that issue.
>
> I assume if it were a constexpr local in a protected inline function it wouldn't hinder optimizations in any real way?
> It's used as a parameter to std::min, so it's already odr used & I'd rather not leave it as a trap to walk around even if we addressed that issue.
Oh, right you are! In that case this seems fine :)
> I assume if it were a constexpr local in a protected inline function it wouldn't hinder optimizations in any real way?
The change from constexpr function to constexpr static didn't change anything performance-wise, so either way works for me.
Another option is:
```
enum : size_t { SizeTypeMax = std::numeric_limits<Size_T>::max() };
```
Kind of sad that in C++14, using an enum is still the only "no nonsense" way to declare a constant.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77621/new/
https://reviews.llvm.org/D77621
More information about the cfe-commits
mailing list