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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits