<div dir="ltr"><div dir="ltr">So there's lots of fragments to this thread and a lot of emails & I think it might be simpler if I put thoughts in one place rather than replying to each subthread.<br><br>To Chris's comment(s)<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">1) In a design like today where we have two names for “inlined vector” and “not inlined vector”, then I think it is important for “InlinedVector<T>” to have at least 1 element.  Defaulting to out of line when using the inline vector name betrays intention: it would be better to generate a compile time error or warning if the element size is too long, because the compiler engineer should use the out of line name.<br><div><br class="gmail-Apple-interchange-newline">2) In a design where we have "one name", then it makes more sense to have the default argument type be the “do what I mean” size, which defaults to zero if T is too large.</div><div><br></div><div>I’m pretty skeptical of #2 as an approach, because inline and out of line design points are pretty different when sizeof(T) is 4 or 8, and when sizeof(thevector) matters, e.g. in 2D cases.</div></blockquote><div><br>To me, this is more like std::string, which is (2) - and more, to me, about the contract of the type - std::string doesn't guarantee iterator validity over swap/move, where std::vector does. If std::vector didn't have this guarantee, it'd be possible to have small vector optimizations in std::vector the same way std::string does - without the user-facing customization, mind you.<br><br>So I think (2) has some legs - but the problem for me is that if we name the type llvm::Vector, which already gets a bit close to std::vector (unqualified "Vector" seems like fairly subtle code to me - but I'm leaning towards being OK with that aspect if other peopel are) and it has different iterator invalidation/move semantics than std::vector, I think that could be problematic. I don't know what to call it, though - I agree to some extent with (1) that calling such a thing SmallVector is a bit confusing - but I don't think it's that confusing, to my mind at least - it doesn't change the contract, and if the user isn't specifying the small buffer size, I think we could reasonably say "it's whatever size is good, and that might be zero" - and importantly if you change the size of the structs inside the SmallVector, it can change the number of elements (including down to, or up from, zero) - compared to having it break when you change the size of a struct, or have it have a weird single element that isn't what the user probably wanted, I think that (the ability to implicitly go to zero) would be a good thing.<br><br><div class="gmail_attr">To J. Y. Knight's comments:</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>Why does this need a long tail? We have fancy ast refactoring tooling, and a single repository with all the code visible, after all. We can use thar to discover all of the missing noexcepts, and add them, all at once. And then use a clang tidy to help it remain true.</div></blockquote><br>At least to the best of my knowledge, I don't think we have a really nice clang-tidy integration to help check these things on an ongoing basis - catching them only in code review (seems we have some kind of clang-tidy integration in Phabricator) I think is inadequate. If there's some way to tie in clang-tidy to the build system so it could be enabled at the same level as clang warnings, I'd be up for that general idea, for sure! (I think there are maybe still some issues as Mehdi mentioned, with using that as the route forward for this particular issue, though - in terms of annotating all the necessary ctors with noexcept... but I'm not set against it, if it gets more consideration, I'd be up for giving it a closer look - I like the idea of writing conformantly well performing code using the standard library, for sure)<br></div><div> </div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Dec 2, 2020 at 12:39 PM Duncan P. N. Exon Smith via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><br><div><br><blockquote type="cite"><div>On 2020 Dec  1, at 21:04, Chris Lattner <<a href="mailto:clattner@nondot.org" target="_blank">clattner@nondot.org</a>> wrote:</div><div><div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><div><br></div><div>1) are you, or anyone else, interested in driving an llvm::Vector proposal + coding standard change to get us going in the right direction?  I don’t think we need a mass migration of the code base, just get the policy aligned right plus the new type name to exist.</div></div></div></blockquote><div><br></div><div>I'll try to get a minimal patch together with docs and send an RFC later this week.</div><div><br></div><div>(</div><div>I think the initial patch could be as simple as:</div><div>```</div><div>template <class T> using Vector = SmallVector<T, 0>;</div><div>```</div><div>but maybe we'd want to rename `SmallVectorImpl` to `VectorImpl`, or maybe there are other ideas, but off topic for this thread...</div><div>)</div></div></div></blockquote><div><br>Yeah, I'm of mixed feelings here - as mentioned above, naming is hard. SmallVector<.., 0> certainly has the iterator validity semantics to match std::vector - but I almost feel like /that/ semantic feature should be the exception here, and the more common name should cover the "use an inline buffer if it's useful" situation (including the possibility of that inline buffer being zero). But for the problem with then having a name really similar to std::vector but with differing semantics.<br><br>Maybe that's what it comes down to - the naming collission/semantic difference with std::vector is problematic, so SmallVector<0> is the LLVM std::vector (small sizeof footprint and iterator validity on move). Renaming SmallVector to somehow let it be more general if the "Small" is too confusing if the small buffer size could be implicitly zero (which I think would be valuable, even if we had llvm::Vector where 0 is guaranteed (small sizeof/+iterator validity on move) - because some/most uses don't need that guarantee but would still be better off not hardcoding "at least one" or "zero" size because fo the size of their elements today and would be better off with a floating guarantee that can pick implicitly based on the size of the struct on any given day/machine/etc).<br><br>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div><div><br></div><div><blockquote type="cite"><div>On 2020 Dec  2, at 09:51, James Y Knight <<a href="mailto:jyknight@google.com" target="_blank">jyknight@google.com</a>> wrote:</div><br><div><span style="float:none;display:inline">I strongly disagree here. Not wanting to bother to add 'noexcept' to user-defined move-constructors is a poor justification for switching to a different vector type. Perhaps there are </span><i>other</i><span style="float:none;display:inline"> reasons which might justify avoiding std::vector, but not that...</span></div></blockquote><div><div><span style="float:none;display:inline"><br></span></div></div></div><div>I'll be sure to mention this alternative in the RFC for llvm::Vector; we can talk about it there; maybe you'll convince the rest of us to add the `noexcept`s everywhere and then the justification for llvm::Vector would indeed be weaker (not completely absent though)...</div></div></div>_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>
</div>