[PATCH] clang-tidy checker that enforce proper parentheses in macros

[Daniel Marjamäki]

Here are updated results I made today with latest patch:

https://drive.google.com/file/d/0BykPmWrCOxt2S2lkZWpOSEJKUHM/view


================
Comment at: clang-tidy/misc/MacroParenthesesCheck.cpp:77
@@ +76,3 @@
+          !Tok.is(tok::minus) && (TI + 1) != TE &&
+          (TI + 1)->is(tok::numeric_constant) && (TI + 2) == TE) {
+        return;
----------------
alexfh wrote:
> danielmarjamaki wrote:
> > alexfh wrote:
> > > `TI + 2 == TE` implies `TI + 1 != TE`. I'd also put it next to `TI == MI->tokens_begin()`.
> > Yes that is implied. But are you sure it's not UB? I wanted to avoid undefined behaviour when creating an out-of-bounds pointer (calculating TI+2 without checking TI+1). is the buffer always padded with extra elements after TE?
> Why does the buffer need to be padded with something when you compare iterators without dereferencing them? I didn't find any violation of the preconditions for random access iterators (http://en.cppreference.com/w/cpp/concept/RandomAccessIterator).
TI is a pointer.

I refer to 6.5.6.8 in the C standard (about pointer addition):

If both the pointer operand and the result point to elements of the same array object, or one past the last
element of the array object, the evaluation shall not produce an overflow; otherwise, the behavior is undefined.


================
Comment at: clang-tidy/misc/MacroParenthesesCheck.cpp:77
@@ +76,3 @@
+          !Tok.is(tok::minus) && (TI + 1) != TE &&
+          (TI + 1)->is(tok::numeric_constant) && (TI + 2) == TE) {
+        return;
----------------
danielmarjamaki wrote:
> alexfh wrote:
> > danielmarjamaki wrote:
> > > alexfh wrote:
> > > > `TI + 2 == TE` implies `TI + 1 != TE`. I'd also put it next to `TI == MI->tokens_begin()`.
> > > Yes that is implied. But are you sure it's not UB? I wanted to avoid undefined behaviour when creating an out-of-bounds pointer (calculating TI+2 without checking TI+1). is the buffer always padded with extra elements after TE?
> > Why does the buffer need to be padded with something when you compare iterators without dereferencing them? I didn't find any violation of the preconditions for random access iterators (http://en.cppreference.com/w/cpp/concept/RandomAccessIterator).
> TI is a pointer.
> 
> I refer to 6.5.6.8 in the C standard (about pointer addition):
> 
> If both the pointer operand and the result point to elements of the same array object, or one past the last
> element of the array object, the evaluation shall not produce an overflow; otherwise, the behavior is undefined.
> 
hmm.. maybe I was confused. I thought it was a pointer addition.. however random access iterators should not overflow neither as far as I know..

================
Comment at: clang-tidy/misc/MacroParenthesesCheck.cpp:77
@@ +76,3 @@
+          !Tok.is(tok::minus) && (TI + 1) != TE &&
+          (TI + 1)->is(tok::numeric_constant) && (TI + 2) == TE) {
+        return;
----------------
danielmarjamaki wrote:
> danielmarjamaki wrote:
> > alexfh wrote:
> > > danielmarjamaki wrote:
> > > > alexfh wrote:
> > > > > `TI + 2 == TE` implies `TI + 1 != TE`. I'd also put it next to `TI == MI->tokens_begin()`.
> > > > Yes that is implied. But are you sure it's not UB? I wanted to avoid undefined behaviour when creating an out-of-bounds pointer (calculating TI+2 without checking TI+1). is the buffer always padded with extra elements after TE?
> > > Why does the buffer need to be padded with something when you compare iterators without dereferencing them? I didn't find any violation of the preconditions for random access iterators (http://en.cppreference.com/w/cpp/concept/RandomAccessIterator).
> > TI is a pointer.
> > 
> > I refer to 6.5.6.8 in the C standard (about pointer addition):
> > 
> > If both the pointer operand and the result point to elements of the same array object, or one past the last
> > element of the array object, the evaluation shall not produce an overflow; otherwise, the behavior is undefined.
> > 
> hmm.. maybe I was confused. I thought it was a pointer addition.. however random access iterators should not overflow neither as far as I know..
I can't find that TI+2 is UB right now in the specification.

However for information this sample code:


```
#include <vector>

int main() {
  std::vector<int> x;
  x.push_back(123);
  std::vector<int>::iterator it = x.begin();
  if (it+2 == x.end()) {}
  return 0;
}
```

When I compile and run it with the STL assertions turned on I get a fault..

```
danielm at debian:~$ g++ -D_GLIBCXX_DEBUG -g bad-stl.cpp -o bad-stl                                                                                                         
danielm at debian:~$ ./bad-stl                                                                                                                    
/usr/include/c++/4.7/debug/safe_iterator.h:359:error: attempt to advance a                                                                                                       
    dereferenceable (start-of-sequence) iterator 2 steps, which falls     
    outside its valid range.

```

http://reviews.llvm.org/D9528

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/