[PATCH] D21256: Improved Visual Studio 2015 visualization of SmallVectorImpl

[David Blaikie via cfe-commits]

On Mon, Jun 13, 2016 at 8:55 AM, Michael Spertus <mike at spertus.com> wrote:

> Hi David,
> While I understand the initial reasoning. I have found that this is like a
> hundred times better for working on Clang in practice and can't imagine
> working without it. The point is that many Clang data structures contain
> SmallVectors and having to do zero expansion clicks instead of multiple
> each time you take a step through the code is really helpful. If you want
> me to back it out and rereview we can, but I'd encourage you to try it out
> first.
>

Oh, I don't use MSVC at all, so it's totally up to you, I'd just be curious
if the visualizers for SmallVector were different for those of std::vector.
Not that the authors of the inbuilt visualizers in MSVC have a monopoly on
correct/good design here.

Might be worth roping STL (Stephan) into the thread to discuss MSVC
visualizers of the STL - and/or filing a bug, if we think there are better
ways to visualize containers than those provided by MSVC.


>
> To ask more about the aside, I'm sorry if I violated community norms. Let
> me tell you my reasoning, and you can clarify how I should handle in the
> future: Aaron approved me to do post-commit reviews on natvis changes,
> which I have done frequently. For this change, I wasn't putting it into
> phabricator because I thought pre-commit approval is required but more as a
> heads up. Should I change that to be if I don't feel comfortable submitting
> without phabricator, then do the full review process?
>
> Thanks,
>
> Mike
>
> On Mon, Jun 13, 2016 at 10:16 AM, David Blaikie <dblaikie at gmail.com>
> wrote:
>
>> As for the original change proposed: My guiding principle would be "do
>> whatever std::vector does". (& that's what I did when implementing GDB
>> pretty printers for SmallVector/SmallString/ArrayRef, etc... )
>>
>> An aside: We generally don't do time limited reviews like this. Either
>> something needs review because you're not sure about it, or it doesn't. It
>> sounds like the feedback you were looking for probably would've been fine a
>> post-commit review feedback just as easily & perhaps might've been a better
>> option. (while in this case it was fine - it's sort of a community
>> habit/standards thing - we don't want to create the idea that lack of
>> feedback is consent/approval in the review process)
>>
>> On Sun, Jun 12, 2016 at 7:01 PM, Mike Spertus via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> mspertus closed this revision.
>>> mspertus added a comment.
>>>
>>> revision 272525
>>>
>>>
>>> http://reviews.llvm.org/D21256
>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160613/d8bbb083/attachment.html>