[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-20 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment. In D135920#3872705 , @shafik wrote: > Apologies for the late reply but after the last changes to the diagnostic > messages are much better. Thanks! Sorry I jumped the gun and submitted before your reply... Repository: rG LLVM G

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-20 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment. Apologies for the late reply but after the last changes to the diagnostic messages are much better. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135920/new/ https://reviews.llvm.org/D135920 ___

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-20 Thread Bill Wendling via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGb76219b59020: [clang][Sema] Use correct array size for diagnostic (authored by void). Repository: rG LLVM Github Monore

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-19 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 469044. void added a comment. Reformatting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135920/new/ https://reviews.llvm.org/D135920 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSe

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-19 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment. @shafik PTAL. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135920/new/ https://reviews.llvm.org/D135920 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-19 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 468997. void added a comment. Add release note. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135920/new/ https://reviews.llvm.org/D135920 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/Diagnost

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-19 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment. In D135920#3868170 , @aaron.ballman wrote: > Can you please add a release note to mention the diagnostic wording was > patched up? Done. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM, I think the new diagnostic wording is an improvement. Can you please add a release note to mention the diagnostic wording was patched up? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-18 Thread serge via Phabricator via cfe-commits
serge-sans-paille accepted this revision. serge-sans-paille added a comment. I really like the new message, thanks for taking account the diverging reviews! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135920/new/ https://reviews.llvm.org/D135920

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-18 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 468728. void added a comment. Update the error message to remove the reference to bytes, which can be confusing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135920/new/ https://reviews.llvm.org/D135920 Files:

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-18 Thread Bill Wendling via Phabricator via cfe-commits
void added inline comments. Comment at: clang/test/SemaCXX/array-bounds.cpp:240 -((char*)foo)[sizeof(foo)] = '\0'; // expected-warning {{array index 32768 is past the end of the array (which contains 32768 elements)}} +((char*)foo)[sizeof(foo)] = '\0'; // expected-wa

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-18 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/test/SemaCXX/array-bounds.cpp:240 -((char*)foo)[sizeof(foo)] = '\0'; // expected-warning {{array index 32768 is past the end of the array (which contains 32768 elements)}} +((char*)foo)[sizeof(foo)] = '\0'; // ex

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-18 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment. In D135920#3863648 , @shafik wrote: > The current approach of mixing bytes and indices in the same diagnostic is > too confusing. "Current" approach? > I think we see some acceptable messages for out of bounds access by looking >

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-17 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik requested changes to this revision. shafik added a comment. This revision now requires changes to proceed. The current approach of mixing bytes and indices in the same diagnostic is too confusing. I think we see some acceptable messages for out of bounds access by looking at three differe

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-17 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 468356. void added a comment. Fix rogue test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135920/new/ https://reviews.llvm.org/D135920 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-17 Thread Bill Wendling via Phabricator via cfe-commits
void added a comment. The messages that's a bit annoying are the ones reporting "0 bytes past the end of the array": warning: array index 2 is 0 bytes past the end of the array (that has type 'int[2]') I'm not sure what to do with these (except to leave them alone). Repository: rG LLVM G

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-17 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 468355. void added a comment. Update more testcases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135920/new/ https://reviews.llvm.org/D135920 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/li

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-17 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 468353. void added a comment. Improve the error message to include how many bytes the index goes over warning: the pointer incremented by 14 refers 1 byte past the end of the array (that has type 'const char[13]') Repository: rG LLVM Github Monorepo CHAN

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-17 Thread Bill Wendling via Phabricator via cfe-commits
void added inline comments. Comment at: clang/test/SemaCXX/array-bounds.cpp:240 -((char*)foo)[sizeof(foo)] = '\0'; // expected-warning {{array index 32768 is past the end of the array (which contains 32768 elements)}} +((char*)foo)[sizeof(foo)] = '\0'; // expected-wa

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-17 Thread serge via Phabricator via cfe-commits
serge-sans-paille added inline comments. Comment at: clang/test/SemaCXX/array-bounds.cpp:240 -((char*)foo)[sizeof(foo)] = '\0'; // expected-warning {{array index 32768 is past the end of the array (which contains 32768 elements)}} +((char*)foo)[sizeof(foo)] = '\0'; /

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-17 Thread Kees Cook via Phabricator via cfe-commits
kees added inline comments. Comment at: clang/test/SemaCXX/array-bounds.cpp:240 -((char*)foo)[sizeof(foo)] = '\0'; // expected-warning {{array index 32768 is past the end of the array (which contains 32768 elements)}} +((char*)foo)[sizeof(foo)] = '\0'; // expected-wa

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-17 Thread serge via Phabricator via cfe-commits
serge-sans-paille added inline comments. Comment at: clang/test/SemaCXX/array-bounds.cpp:240 -((char*)foo)[sizeof(foo)] = '\0'; // expected-warning {{array index 32768 is past the end of the array (which contains 32768 elements)}} +((char*)foo)[sizeof(foo)] = '\0'; /

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-14 Thread Bill Wendling via Phabricator via cfe-commits
void updated this revision to Diff 467941. void added a comment. Update test to use a smaller index. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135920/new/ https://reviews.llvm.org/D135920 Files: clang/lib/Sema/SemaChecking.cpp clang/test/S

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-13 Thread Kees Cook via Phabricator via cfe-commits
kees added inline comments. Comment at: clang/test/Sema/array-bounds-ptr-arith.c:15 { -return (void *)es->s_uuid + 80; // expected-warning {{refers past the end of the array}} +return (void *)es->s_uuid + 80; // expected-warning {{refers past the end of the arr

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-13 Thread Bill Wendling via Phabricator via cfe-commits
void added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:16068 DiagRuntimeBehavior(BaseExpr->getBeginLoc(), BaseExpr, PDiag(DiagID) << toString(index, 10, true) - << toString(size, 10, true) ---

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-13 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment. Thank you for this fix, I have some comments. Comment at: clang/lib/Sema/SemaChecking.cpp:16068 DiagRuntimeBehavior(BaseExpr->getBeginLoc(), BaseExpr, PDiag(DiagID) << toString(index, 10, true) -

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

2022-10-13 Thread Bill Wendling via Phabricator via cfe-commits
void created this revision. void added reviewers: kees, serge-sans-paille, rsmith, efriedma. Herald added a project: All. void requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The diagnostic was confusing and reporting that an array contains