aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
On Tue, Oct 6, 2015 at 5:31 PM, Matthias Gehre <m.ge...@gmx.de> wrote:
> mgehre marked 2 inline comments as done.
>
> ================
> Comment at:
> test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp:37
> @@ +36,3 @@
> + q -= i;
> + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: do not use pointer arithmetic
> + q -= ENUM_LITERAL;
>
> ----------------
>
> aaron.ballman wrote:
>
> > I don't think this comment is "done" yet. I still don't know how this check
> > is intended to handle code like that. Does it currently diagnose? Does it
> > not diagnose? Should it diagnose?
>
>
> I had added your code, see line 55 of this file.
I am pretty sure I missed this by accidentally fiddling with a Phab knob,
because this is definitely in there this morning, but wasn't last night. Sorry
for the noise!
> It should diagnose, because an arithmetic operation (+) is used, which
> results in a (possibly) changed pointer.
> It does diagnose, as the CHECK-MESSAGE shows.
Great, thank you!
> ================
> Comment at:
> test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp:51
> @@ +50,3 @@
> +
> + i = p[1];
> + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use pointer arithmetic
>
> ----------------
>
> aaron.ballman wrote:
>
> > How well does this handle code like:
>
> >
>
> > void f(int i[], size_t s) {
>
> > i[s - 1] = 0;
>
> > }
>
> >
>
> >
>
> > Does it diagnose, and should it?
>
>
> Good point, I'll ask at https://github.com/isocpp/CppCoreGuidelines/issues/299
Thanks! The answer to this question should not hold up this patch, IMO.
LGTM!
~Aaron
http://reviews.llvm.org/D13311
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
