[PATCH] D13311: [clang-tidy] Add check cppcoreguidelines-pro-bounds-pointer-arithmetic

[Aaron Ballman via cfe-commits]

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.Gehre at 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