[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2021-02-08 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Thanks @thakis, per @rsmith 's suggestion in the review, I'll revert this. Apologies for the inconvenience. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71714/new/ https://reviews.llvm.org/D71714 _

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2021-02-05 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. @vabridgers: ping ^ 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.

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2021-02-03 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. Hi, this causes a warning in harfbuzz on code that looks like so: UChar decomposed[4]; ... int len = unorm2_getRawDecomposition(...decomposed, ARRAY_LENGTH (decomposed)...) if (len == 1) { U16_GET_UNSAFE (decomposed, 0, *a); where `U16_GET_UNSAFE` looks like

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2021-02-03 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Thanks @rsmith, will do. Best! 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

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2021-02-03 Thread Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe48f444751cf: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array… (authored by ilya, committed by einvbri ). Repository: rG LLVM Github Monorepo

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2021-01-28 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaChecking.cpp:13384 case Stmt::MemberExprClass: { expr = cast(expr)->getBase(); break; ilya wrote:

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2021-01-28 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a subscriber: lattner. vabridgers added a comment. Gentle and polite ping :) Could someone have a look at this? @rsmith , or @lattner ? Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71714/new/ https://reviews.llvm.org/D71

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2021-01-20 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Ping. Could someone pick up review of this patch again, please? Ilya left off with a question to @rsmith , and at that point all activity fell off. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71714/new/ https:

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2021-01-17 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers updated this revision to Diff 317252. vabridgers added a comment. Rebase, commandeer this patch from Ilya. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71714/new/ https://reviews.llvm.org/D71714 Files: clang/include/clang/Sema/Sema.h

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2021-01-14 Thread Vince Bridgers via Phabricator via cfe-commits
vabridgers added a comment. Ping @rsmith . Are you ok if I rebase and push this change? Or do you have specific items that require attention? I've spoken to @ilya , he's ok if I clean this up and push with credit to him. Thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2020-03-13 Thread Ilya Mirsky via Phabricator via cfe-commits
ilya added a comment. One last kind ping 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.l

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2020-02-17 Thread Ilya Mirsky via Phabricator via cfe-commits
ilya added a comment. Ping 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-bi

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2020-01-21 Thread Ilya Mirsky via Phabricator via cfe-commits
ilya marked 2 inline comments as done. ilya added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13384 case Stmt::MemberExprClass: { expr = cast(expr)->getBase(); break; ebevhan wrote: > ilya wrote: > > rsmith wrote: > > > i

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2020-01-16 Thread Bevin Hansson via Phabricator via cfe-commits
ebevhan added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13384 case Stmt::MemberExprClass: { expr = cast(expr)->getBase(); break; ilya wrote: > rsmith wrote: > > ilya wrote: > > > rsmith wrote: > > > > Hmm, don't we need

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2020-01-14 Thread Ilya Mirsky via Phabricator via cfe-commits
ilya marked 4 inline comments as done. ilya added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13384 case Stmt::MemberExprClass: { expr = cast(expr)->getBase(); break; rsmith wrote: > ilya wrote: > > rsmith wrote: > > > Hm

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2020-01-14 Thread Ilya Mirsky via Phabricator via cfe-commits
ilya updated this revision to Diff 238106. ilya added a comment. Address rsmith's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71714/new/ https://reviews.llvm.org/D71714 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaChec

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2020-01-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D71714#1795313 , @ilya wrote: > While I agree with the general notion about the flaws with the current > implementation, I feel that the more major refactoring proposed in this > review is out of the scope of this minor fix.

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2020-01-06 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13384 case Stmt::MemberExprClass: { expr = cast(expr)->getBase(); break; ilya wrote: > rsmith wrote: > > Hmm, don't we need to do different things for dot and arrow

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2020-01-06 Thread Ilya Mirsky via Phabricator via cfe-commits
ilya added a comment. Kind ping 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/c

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2019-12-23 Thread Ilya Mirsky via Phabricator via cfe-commits
ilya marked an inline comment as done. ilya added a comment. Implemented @riccibruno's and @rsmith's comments. While I agree with the general notion about the flaws with the current implementation, I feel that the more major refactoring proposed in this review is out of the scope of this minor f

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2019-12-23 Thread Ilya Mirsky via Phabricator via cfe-commits
ilya updated this revision to Diff 235184. ilya added a comment. 1. Refactor `CheckArrayAccess()` to take `AllowPastTheEnd` parameter to avoid a false positive array out-of-bounds warning for `&(cond ? arr1[N] : arr2[N])`. 2. Use `IgnoreParenCasts()` instead of `IgnoreParenImpCasts()` in `CheckA

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2019-12-23 Thread Ilya Mirsky via Phabricator via cfe-commits
ilya marked an inline comment as done. ilya added a comment. In D71714#1791464 , @riccibruno wrote: > These are not the only AST nodes representing cast expressions (there is also > `CXXFunctionalCastExpr`, ...). What about replacing the > `IgnoreParenIm

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2019-12-20 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. > and it seems to involve a lot of AST traversal I was thinking we'd just call into SemaChecking in appropriate places. I guess there's a little AST traversal to figure whether an expression forms an array address. Your idea seems simpler. > remove elements from the

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2019-12-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In D71714#1792085 , @efriedma wrote: > To be rigorous, we should perform "pointer" checking for every operation that > performs pointer arithmetic. Then we should perform "lvalue" checking (which > doesn't allow pointers one past

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2019-12-19 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. To be rigorous, we should perform "pointer" checking for every operation that performs pointer arithmetic. Then we should perform "lvalue" checking (which doesn't allow pointers one past the end) in the following places: 1. When we take the address of an lvalue. 2. Wh

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2019-12-19 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In addition to @riccibruno's comment, I found a couple of other suspicious things nearby (unrelated to the fix in this patch). No need to address them in this patch unless you feel motivated :) Comment at: clang/lib/Sema/SemaChecking.cpp:13384 c

[PATCH] D71714: [Sema] Fix -Warray-bounds false negative when casting an out-of-bounds array item

2019-12-19 Thread Bruno Ricci via Phabricator via cfe-commits
riccibruno added a comment. These are not the only AST nodes representing cast expressions (there is also `CXXFunctionalCastExpr`, ...). What about replacing the `IgnoreParenImpCasts()` above by `IgnoreParenCasts()` ? Incidentally doing this uncovers another test (`Parser/cxx-ambig-decl-expr.cp