ilya marked 2 inline comments as done.
ilya added inline comments.

================
Comment at: clang/lib/Sema/SemaChecking.cpp:13384
       case Stmt::MemberExprClass: {
         expr = cast<MemberExpr>(expr)->getBase();
         break;
----------------
ebevhan wrote:
> ilya wrote:
> > rsmith wrote:
> > > ilya wrote:
> > > > rsmith wrote:
> > > > > Hmm, don't we need to do different things for dot and arrow in this 
> > > > > case?
> > > > There are several test cases for an out of bounds access on an array 
> > > > member using dot and arrow operators in array-bounds.cpp. Do you have a 
> > > > specific test case for which you think the code is broken?
> > > > There are several test cases for an out of bounds access on an array 
> > > > member using dot and arrow operators in array-bounds.cpp. Do you have a 
> > > > specific test case for which you think the code is broken?
> > > 
> > > Sure. There's a false negative for this:
> > > 
> > > ```
> > > struct A { int n; };
> > > A *a[4];
> > > int *n = &a[4]->n;
> > > ```
> > > 
> > > ... because we incorrectly visit the left-hand side of the `->` with 
> > > `AllowOnePastEnd == 1`. The left-hand side of `->` is subject to 
> > > lvalue-to-rvalue conversion, so can't be one-past-the-end regardless of 
> > > the context in which the `->` appears.
> > > ... because we incorrectly visit the left-hand side of the -> with 
> > > AllowOnePastEnd == 1. The left-hand side of -> is subject to 
> > > lvalue-to-rvalue conversion, so can't be one-past-the-end regardless of 
> > > the context in which the -> appears.
> > 
> > Thanks for clarifying. So basically we don't want to allow one-past-end for 
> > MemberExpr?
> > Thanks for clarifying. So basically we don't want to allow one-past-end for 
> > MemberExpr?
> 
> I think the point is rather that when the MemberExpr isArrow, we want to 
> think of it as performing an implicit dereference first, and thus we should 
> do `AllowOnePastEnd--` before recursing if that is the case.
> I think the point is rather that when the MemberExpr isArrow, we want to 
> think of it as performing an implicit dereference first, and thus we should 
> do AllowOnePastEnd-- before recursing if that is the case.

@ebevhan and I have discussed this offline, and we believe that there's no 
reason to allow an //address-of// of a member of a one past-the-end value, 
using either '.' or '->' operators, regardless of the fact that referencing a 
member of a one past-the-end value using '.' is merely a pointer arithmetic and 
doesn't imply dereferencing, as with operator '->'.

We couldn't find any reference in the [[ 
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4659.pdf| C++ 2017 
N4659 ]] regarding member access of a one past-the-end value. Only mentions of 
one past-the-end in the context of iterators (27.2.1 p. 7) and pointer 
arithmetic (8.3.1 p. 3).

[[ http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf | ISO C99 ]] 
describes in 6.5.3.2 p. 3 that if the operand to the '&' operator is the result 
of an unary '*' operator (or array subscript, implictly), the two operators 
"cancel" each other, but it says nothing about the case when there's a member 
expression "in-between", (the member expression section, 6.5.2.3, says nothing 
about that either).

@rsmith, what do you think?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71714/new/

https://reviews.llvm.org/D71714



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to