[PATCH] ASTVector: Fix return value of various insert() methods

[Will Dietz]

Closest we have is a test to ensure ASTVector compiles correctly with
basic usage[1] due to complications in creating a fake ASTContext to
construct it with.  I'd be happy to write some functionality tests if
anyone has a good way to construct one for testing purposes, as I'm
not very familiar with the related code.

Or should I commit this as improving quality regardless?

~Will

[1] https://github.com/llvm-mirror/clang/blob/f475bf83a45435a211edb4e0ef6ac3481ce7b3fe/unittests/AST/ASTVectorTest.cpp

On Tue, Nov 19, 2013 at 2:06 AM, Richard Smith <richard at metafoo.co.uk> wrote:
> Change LGTM. It'd be nice to have test coverage for this that doesn't
> require running a sanitizer. Do we have any direct tests for ASTVector?
>
>
> On Mon, Nov 18, 2013 at 9:35 PM, Will Dietz <wdietz2 at illinois.edu> wrote:
>>
>> Ping! :)
>>
>> ~Will
>>
>> On Mon, Nov 4, 2013 at 4:32 PM, Will Dietz <wdietz2 at illinois.edu> wrote:
>> > Ping.
>> >
>> > It's easy to get clang to trigger this bug which results in an invalid
>> > iterator to be returned (which the current code happens to ignore, but
>> > that's just a lucky coincidence), as this regularly occurs during
>> > execution of the lit tests.
>> >
>> > On a related note, any suggestions on how to create a simple dummy
>> > ASTContext for testing? As noted in
>> > the commit that originally added ASTVectorTest.cpp (r186253) this
>> > blocks the creation of even basic
>> > functionality tests for this data structure.
>> >
>> > ~Will
>> >
>> > On Mon, Oct 28, 2013 at 5:11 PM, Will Dietz <wdietz2 at illinois.edu>
>> > wrote:
>> >> Error caught -fsanitize=pointer-overflow[1], curiously enough :).
>> >>
>> >> The pointer overflow occurred when insert() was invoked with From==To,
>> >> which is done in quite a few places.  While std::vector::insert
>> >> requires [From,To) to be valid, it looks like here From==To is
>> >> intended to be supported[2], making the bug in the container not in
>> >> its use.
>> >>
>> >> This patch fixes the overflow when From==To, as well as the return
>> >> value in this variant as well as the "fill" variant, changing them to
>> >> return an iterator pointing to the first of the inserted elements
>> >> (like SmallVector does).
>> >>
>> >> See attached.
>> >>
>> >> ~Will
>> >>
>> >> [1] Patches coming soon.
>> >> [2] See the implementation of append(), for example.
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>