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@debian:~$ g++ -D_GLIBCXX_DEBUG -g bad-stl.cpp -o bad-stl                
                                                                                
         
danielm@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/



_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to