[PATCH] D136886: [clang] ASTImporter: Fix importing of va_list types and declarations

2022-11-15 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. Herald added a reviewer: njames93. @mizvekov , I'm willing/trying to assist in addressing the buildbot failures that caused you to revert this change. We are eager to see this fix land stably upstream as the crash it addresses is breaking a number of our

[PATCH] D126534: [analyzer] Deadstore static analysis: Fix false positive on C++17 assignments

2022-08-29 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. This change affected analyzer findings for C language source as well. Mostly true positives, but there is one false positive we know about. I filed https://github.com/llvm/llvm-project/issues/57434 to track that new false positive. Repository: rG LLVM

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

2022-08-19 Thread Chris Hamilton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9e1a4ce0b55d: [clang-tidy] Fix for bugprone-sizeof-expression PR57167 (authored by chrish_ericsson_atx). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

2022-08-17 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 453442. chrish_ericsson_atx added a comment. Removed `// FIXME` comments, per @aaron.ballman's feedback. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131926/new/ https://reviews.llvm.org/D131926 Files:

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

2022-08-17 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp:236-238 - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate sum += sizeof(PMyStruct); - //

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

2022-08-17 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 453286. chrish_ericsson_atx added a comment. Corrected git-clang-format issue. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131926/new/ https://reviews.llvm.org/D131926 Files:

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

2022-08-16 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. Sounds fair. I had taken your acceptance of the change as a green light. :) TBH, the acceptance came much faster than I'd expected-- even though this is a trivial and low-risk change, I expected it to sit for at least several days. I'll plan to wait a

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

2022-08-16 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp:236-238 - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate sum += sizeof(PMyStruct); - //

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

2022-08-16 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 453022. chrish_ericsson_atx added a comment. Added missing newline and added // FIXME comments, per reviewer comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131926/new/ https://reviews.llvm.org/D131926 Files:

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

2022-08-16 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp:236-238 - // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)'; pointer to aggregate sum += sizeof(PMyStruct); - //

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

2022-08-15 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx created this revision. chrish_ericsson_atx added reviewers: mizvekov, baloghadamsoftware, njames93, cfe-commits. Herald added subscribers: carlosgalvezp, rnkovacs, xazax.hun. Herald added a project: All. chrish_ericsson_atx requested review of this revision. Herald added a

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays= for stricter handling of flexible arrays

2022-07-21 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. I've added a few suggestions for more rigorous testing, and raised a concern about a bug in this patch that is hiding expected -Warray-bounds warnings in certain cases. Comment at: clang/lib/CodeGen/CGExpr.cpp:888-889 if (const auto

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-07-15 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. In D112374#3653967 , @JDevlieghere wrote: > I don't. I think reverting your change was well within the guidelines > outlined by LLVM's patch reversion policy: >

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

2021-09-23 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. I believe this exposed another odd issue where a true positive (enabled by this commit) disappears when unrelated code is not present. Bug filed as: https://bugs.llvm.org/show_bug.cgi?id=51950. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

2021-09-22 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. I believe this commit exposed a new false-positive bug in [core.DivideZero]. I've filed the report here: https://bugs.llvm.org/show_bug.cgi?id=51940 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103314/new/

[PATCH] D104285: [analyzer] Retrieve value by direct index from list initialization of constant array declaration.

2021-06-29 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. In D104285#2847547 , @ASDenysPetrov wrote: > I don't think we should revert b30521c28a4d > because > it corrects symbol representation in CSA

[PATCH] D104285: [analyzer] Retrieve value by direct index from list initialization of constant array declaration.

2021-06-25 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. While this patch resolves the issue captured in https://bugs.llvm.org/show_bug.cgi?id=50604, it actually introduces a *new* bug. Perhaps this is what you were alluding to? Here's a reproducer which doesn't fail on main (with or without the problematic

[PATCH] D104285: [analyzer] Retrieve value by direct index from list initialization of constant array declaration.

2021-06-24 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. In D104285#2836190 , @ASDenysPetrov wrote: >> I think the presence of the initializer list in the test case is not >> necessary to trigger the spurious warnings > > Could you please provide some test cases that you

[PATCH] D104285: [analyzer] Retrieve value by direct index from list initialization of constant array declaration.

2021-06-20 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. I've looked this over and tested it locally, and I'm pretty sure it's a good patch. If it were solely up to me, I'd accept this patch as-is. I don't think I should assume I have enough experience in this area though... @NoQ , could you take a look over

[PATCH] D104285: [analyzer] Retrieve value by direct index from list initialization of constant array declaration.

2021-06-18 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added inline comments. Comment at: clang/include/clang/AST/Expr.h:4961 + /// - `nullptr` for invalid index (`i < 0` or `i >= array_size`). + const Expr *getExprForConstArrayByRawIndex(int64_t Idx) const; + I think in most (all?) other

[PATCH] D104285: [analyzer] Retrieve value by direct index from list initialization of constant array declaration.

2021-06-17 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. In D104285#2823305 , @chrish_ericsson_atx wrote: > I'm not sure about whether or not this patch would only work for constant > arrays with initializer lists. If it does only work for such arrays, then I > wonder

[PATCH] D104424: [Sema] Fix for PR50741

2021-06-17 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. Delivered as rGfc6ec9b98cf9 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104424/new/ https://reviews.llvm.org/D104424

[PATCH] D104424: [Sema] Fix for PR50741

2021-06-17 Thread Chris Hamilton via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfc6ec9b98cf9: [Sema] Fix for PR50741 (authored by chrish_ericsson_atx). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D104424: [Sema] Fix for PR50741

2021-06-17 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked 2 inline comments as done. chrish_ericsson_atx added a comment. Thanks for the hints, @erichkeane ! Especially thanks for pointing out the root cause early on. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104424/new/

[PATCH] D104424: [Sema] Fix for PR50741

2021-06-17 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 352711. chrish_ericsson_atx edited the summary of this revision. chrish_ericsson_atx added a comment. Addressed review feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104424/new/

[PATCH] D104285: [analyzer] Retrieve value by direct index from list initialization of constant array declaration.

2021-06-16 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. I'm not sure about whether or not this patch would only work for constant arrays with initializer lists. If it does only work for such arrays, then I wonder whether the fix is broad enough -- I haven't tested (yet), but I think the presence of the

[PATCH] D104424: [Sema] Fix for PR50741

2021-06-16 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 352569. chrish_ericsson_atx added a comment. Updated summary. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104424/new/ https://reviews.llvm.org/D104424 Files: clang/lib/Sema/SemaChecking.cpp

[PATCH] D104424: [Sema] Fix for PR50741

2021-06-16 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx created this revision. chrish_ericsson_atx added reviewers: erichkeane, aaron.ballman, abhinavgaba. chrish_ericsson_atx requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixed crash when doing pointer math on a void

[PATCH] D88174: [Sema] Address-space sensitive check for unbounded arrays (v2)

2021-06-11 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. I'm aware that this commit triggers one failure in ASan/Windows -- I have posted a patch for review to address that here: https://reviews.llvm.org/D104141 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88174: [Sema] Address-space sensitive check for unbounded arrays (v2)

2021-06-11 Thread Chris Hamilton via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGce44fe199bbf: [Sema] Address-space sensitive check for unbounded arrays (v2) (authored by chrish_ericsson_atx). Repository: rG LLVM Github

[PATCH] D88174: [Sema] Address-space sensitive check for unbounded arrays (v2)

2021-06-11 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 351491. chrish_ericsson_atx added a comment. Corrected APSInt.toString() usage to comply with intervening change 61cdaf66fe22be2b5 Repository: rG LLVM Github Monorepo

[PATCH] D88174: [Sema] Address-space sensitive check for unbounded arrays (v2)

2021-06-11 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx reopened this revision. chrish_ericsson_atx added a comment. This revision is now accepted and ready to land. Reverted commit due to buildbot failures -- will update shortly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88174: [Sema] Address-space sensitive check for unbounded arrays (v2)

2021-06-11 Thread Chris Hamilton via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe42a347b7440: [Sema] Address-space sensitive check for unbounded arrays (v2) (authored by chrish_ericsson_atx). Changed prior to commit:

[PATCH] D88174: [Sema] Address-space sensitive check for unbounded arrays (v2)

2021-06-08 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 350722. chrish_ericsson_atx added a comment. Refreshed previously-reverted changeset and re-tested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88174/new/ https://reviews.llvm.org/D88174 Files:

[PATCH] D89055: [analyzer] Wrong type cast occures during pointer dereferencing after type punning

2021-06-07 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added inline comments. Herald added a subscriber: manas. Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:763-765 + if (const auto *ER = dyn_cast(R)) { +R = StateMgr.getStoreManager().castRegion(ER, CastTy); +return

[PATCH] D101108: [PR49761] Fix variadic arg handling in matcher

2021-04-23 Thread Chris Hamilton via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcae3b70cebc1: [PR49761] Fix variadic arg handling in matcher (authored by chrish_ericsson_atx). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D101108: [PR49761] Fix variadic arg handling in matcher

2021-04-23 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 340094. chrish_ericsson_atx added a comment. Addressed feedback from aaron.ballman. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101108/new/ https://reviews.llvm.org/D101108 Files:

[PATCH] D101108: [PR49761] Fix variadic arg handling in matcher

2021-04-23 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4760-4765 + unsigned numArgs = Node.getNumArgs(); + if (FProto && FProto->isVariadic() && FProto->getNumParams() < numArgs) { +numArgs = FProto->getNumParams(); + } +

[PATCH] D101108: [PR49761] Fix variadic arg handling in matcher

2021-04-22 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx created this revision. chrish_ericsson_atx requested review of this revision. Herald added projects: clang, clang-tools-extra. Herald added a subscriber: cfe-commits. Mishandling of variadic arguments in a function call caused a crash (runtime assert fail) in

[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

2020-11-02 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. Is anything planned or in progress that would address this issue? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86743/new/ https://reviews.llvm.org/D86743 ___

[PATCH] D88174: [Sema] Address-space sensitive check for unbounded arrays (v2)

2020-09-29 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx reopened this revision. chrish_ericsson_atx added a comment. This revision is now accepted and ready to land. Reverted due to another obscure test failure. Working to diagnose. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88174: [Sema] Address-space sensitive check for unbounded arrays (v2)

2020-09-29 Thread Chris Hamilton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd9ee935679e7: [Sema] Address-space sensitive check for unbounded arrays (v2) (authored by chrish_ericsson_atx). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88174: [Sema] Address-space sensitive check for unbounded arrays (v2)

2020-09-29 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. I'm going to land this as-is, based on Bevin's LGTM and my own confidence that the 10 lines I added are correct (enough). @aaron.ballman , please reopen this review if you have additional comments or concerns. Thanks. Repository: rG LLVM Github

[PATCH] D88174: [Sema] Address-space sensitive check for unbounded arrays (v2)

2020-09-28 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. Ping. :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88174/new/ https://reviews.llvm.org/D88174 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D88174: [Sema] Address-space sensitive check for unbounded arrays (v2)

2020-09-23 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:14054 + if (IndexNegated) { +index.setIsUnsigned(false); index = -index; This call to index.setIsUnsigned(false) is the only unreviewed part of this change in this

[PATCH] D88174: [Sema] Address-space sensitive check for unbounded arrays (v2)

2020-09-23 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx created this revision. chrish_ericsson_atx added reviewers: aaron.ballman, rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. chrish_ericsson_atx requested review of this revision. Check applied to unbounded (incomplete) arrays and pointers to spot

[PATCH] D87942: [Analyzer] GNU named variadic macros in Plister

2020-09-21 Thread Chris Hamilton via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2697d138a65a: [Analyzer] GNU named variadic macros in Plister (authored by chrish_ericsson_atx). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D87942: [Analyzer] GNU named variadic macros in Plister

2020-09-21 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked 2 inline comments as done. chrish_ericsson_atx added a comment. Thanks for the quick feedback, Kristof! I changed the macro to be just a decrement operator, and triggered DBZ with that. Added the checks as you suggested. Repository: rG LLVM Github Monorepo

[PATCH] D87942: [Analyzer] GNU named variadic macros in Plister

2020-09-21 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 293154. chrish_ericsson_atx added a comment. Addressed feedback from @Szelethus Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87942/new/ https://reviews.llvm.org/D87942 Files:

[PATCH] D87942: [Analyzer] GNU named variadic macros in Plister

2020-09-19 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added inline comments. Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:536 + do { \ + } while (0) + Seems the do {} while (0) idiom is a problem, because the plist output gets formatted differently on

[PATCH] D87942: [Analyzer] GNU named variadic macros in Plister

2020-09-18 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx created this revision. chrish_ericsson_atx added reviewers: NoQ, Szelethus, dcoughlin. Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun. Herald added a project:

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-14 Thread Chris Hamilton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGda55e9ba1273: [Sema] Address-space sensitive index check for unbounded arrays (authored by chrish_ericsson_atx). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-11 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked 11 inline comments as done. chrish_ericsson_atx added a comment. Addressed all feedback from Aaron, except for two comments about reachability that I don't understand. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-11 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 291184. chrish_ericsson_atx added a comment. Addressed feedback from Aaron Ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86796/new/ https://reviews.llvm.org/D86796 Files:

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-10 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. Thank you for the comments, @aaron.ballman . I'll update with the changes you requested shortly. I did have some requests for clarification of you, though. Thanks! Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8841-8844

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-09 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. Ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86796/new/ https://reviews.llvm.org/D86796 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-04 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. Thanks for the excellent feedback, @ebevhan . @aaron.ballman , @krememek , or @rsmith , could one of you take a look at this change and if it's acceptable, please approve it? I have not requested commit privileges yet, either, so I will need your

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 289819. chrish_ericsson_atx added a comment. NC push did not resolve failed test. Rebased in hopes that whatever has broken the build has been resolved in the intervening commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 289815. chrish_ericsson_atx added a comment. NC. Pushing null change in hopes of re-triggering testing. Unit test that failed is low-level LLVM test, which doesn't even exercise the code I've changed here, so I'm assuming it's a spurious

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked an inline comment as done. chrish_ericsson_atx added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13966 if (index.isUnsigned() || !index.isNegative()) { -// It is possible that the type of the base expression after -//

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 289785. chrish_ericsson_atx added a comment. Refactored as ebevhan suggested to simplify patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86796/new/ https://reviews.llvm.org/D86796 Files:

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked 2 inline comments as done. chrish_ericsson_atx added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13966 if (index.isUnsigned() || !index.isNegative()) { -// It is possible that the type of the base expression after -//

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 289753. chrish_ericsson_atx marked 2 inline comments as done. chrish_ericsson_atx added a comment. Addressed reviewer feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86796/new/

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-03 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked 3 inline comments as done. chrish_ericsson_atx added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13981 +bool overflow; +llvm::APInt product(index); +product += 1; ebevhan wrote: > What if index

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-02 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked 2 inline comments as done. chrish_ericsson_atx added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13989 + MaxElems <<= AddrBits; + MaxElems /= ElemBytes; + ebevhan wrote: > The size calculations here could

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-02 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 289503. chrish_ericsson_atx added a comment. Updating D86796 : [Sema] Address-space sensitive index check for unbounded arrays Refactored math as suggested by Bevin Hansson. Repository: rG LLVM Github

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-09-01 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked an inline comment as done. chrish_ericsson_atx added a comment. I will tinker with the math to simplify as you suggest. Working with APInt and APSInt seems to promulgate sensitive and brittle code, which makes trying alternative expressions more tedious than I'd like

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-08-31 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 288995. chrish_ericsson_atx added a comment. Corrected formatting (per git-clang-format) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86796/new/ https://reviews.llvm.org/D86796 Files:

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-08-28 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 288649. chrish_ericsson_atx added a comment. Removed Change-Id from commit log message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86796/new/ https://reviews.llvm.org/D86796 Files:

[PATCH] D86796: [Sema] Address-space sensitive index check for unbounded arrays

2020-08-28 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx created this revision. Herald added subscribers: cfe-commits, arphaman. Herald added a project: clang. chrish_ericsson_atx requested review of this revision. Check applied to unbounded (incomplete) arrays and pointers to spot cases where the computed address is beyond the

[PATCH] D74746: [clang][doxygen] Fix -Wdocumentation warning for tag typedefs

2020-06-02 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. I reported this issue back in November, as https://bugs.llvm.org/show_bug.cgi?id=44143. This change fixes the bug -- I'll mark it as such. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74746/new/

[PATCH] D70203: [AST] Attach comment in `/** doc */ typedef struct A {} B` to B as well as A.

2019-11-26 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. In D70203#1760299 , @bjope wrote: > Hi @sammccall . > Just a heads up. Looks like this might have caused: > https://bugs.llvm.org/show_bug.cgi?id=44143 . Yes indeed. I locally reverted commit a433e714

[PATCH] D62953: [Syntax] Do not glue multiple empty PP expansions to a single mapping

2019-11-16 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added inline comments. Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:447 +tryConsumeSpelledUntil(File, EndOffset + 1, SpelledIndex).hasValue(); +(void)HitMapping; +assert(!HitMapping && "recursive macro expansion?"); What is

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-25 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. @Szelethus, firstly, thank you for landing this change. I really appreciate it! Secondly, apologies for misspelling your name earlier. And lastly, thank you for your patient explanation of how to get the diffs updated correctly in a Phabricator review.

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-21 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx added a comment. Kristoff, if you wouldn't mind, since you offered earlier, please go ahead and commit this change as-is, since it was accepted. I ran into some non-technical issues with my follow-up changes and I'm going to be unavailable for several weeks. To mitigate

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-15 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx updated this revision to Diff 215433. chrish_ericsson_atx added a comment. Follow-up on reviewer feedback. Changed from blacklisting LValueToRValue to whitelisting IntegralCast. This was a good call -- additional testing with different cast kinds showed that the assertion

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-15 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked 5 inline comments as done. chrish_ericsson_atx added a comment. In D66014#1627858 , @Szelethus wrote: > LGTM, thanks! Do you need someone to commit this on your behalf? Also, could > you please make the comments capitalized,

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-13 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked an inline comment as done. chrish_ericsson_atx added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:94-98 + // If cast is implicit LValueToRValue, no conversion is taking place, + // and therefore no

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-12 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked an inline comment as done. chrish_ericsson_atx added a comment. In D66014#1625733 , @Szelethus wrote: > Oh, btw, thank you for working on this! Glad to help! Comment at:

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-12 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx marked 3 inline comments as done. chrish_ericsson_atx added inline comments. Comment at: clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:94-98 + // If cast is implicit LValueToRValue, no conversion is taking place, + // and therefore no

[PATCH] D66014: Avoid unnecessary enum range check on LValueToRValue casts

2019-08-09 Thread Chris Hamilton via Phabricator via cfe-commits
chrish_ericsson_atx created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. EnumCastOutOfRangeChecker should not perform enum range checks on LValueToRValue casts, since this type of cast does not actually change the underlying type. Performing the