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

Szelethus added a comment. No worries, always happy to help! :) Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66014/new/ https://reviews.llvm.org/D66014 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

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

Szelethus added a comment. I seem to have been able to put this together by fetching the individual diffs and squashing them this time :) Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66014/new/ https://reviews.llvm.org/D66014

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL369760: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts (authored by Szelethus, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior

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

Szelethus added a comment. What I meant is that the diff has to be made against the master branch or I won't be able to apply it locally. Say your repo is structured like this: * dcba2 (HEAD -> my_fix) * dcba1 * abcd4 (master) * abcd3 * abcd2 * abcd1 Then the diff has to be made

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

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

Szelethus added a comment. You seem to have diffed against your latest local commit, rather than against trunk (or master, if you use the monorepo). Phabricator isn't smart enough to put two and two together, and only displays the uploaded diff (though one has to admit, its doing a damn good

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

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

Szelethus added a comment. Oh, there is no need for a new differential, you can update this one by clicking on 'update diff' in the right panel. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66014/new/ https://reviews.llvm.org/D66014

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

Szelethus added a comment. Yup, you can upload it and the green checkmark will stay. If it doesn't, I'll accept again. I think there was one case when I added like 800 extra LOCs to a patch and phabricator automatically marked it as "needs reviews", but I never came across this after that,

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

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

gamesh411 accepted this revision. gamesh411 added a comment. @chrish_ericsson_atx Thanks for fixing this! Your help is much appreciated :) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66014/new/ https://reviews.llvm.org/D66014

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

Szelethus accepted this revision. Szelethus added a comment. LGTM, thanks! Do you need someone to commit this on your behalf? Also, could you please make the comments capitalized, terminated, and fitting in 80 columns? Comment at:

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

NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Thanks! Comment at: clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:94-98 + // If cast is implicit LValueToRValue, no conversion is taking place, + // and

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

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

NoQ 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 range check is needed. Don't analyze further. + if (CE->getCastKind()

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

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

Szelethus added a comment. Oh, btw, thank you for working on this! Comment at: clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:121-122 // Check if any of the enum values possibly match. bool PossibleValueMatch = llvm::any_of( DeclValues,

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

NoQ 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 range check is needed. Don't analyze further. + if (CE->getCastKind()

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

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: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

NoQ 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 range check is needed. Don't analyze further. + if (CE->getCastKind()

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

Szelethus added reviewers: gamesh411, NoQ. Szelethus added subscribers: gamesh411, NoQ. Szelethus added a comment. + @gamesh411 as he took over this checker before I commited it on his behalf, +@NoQ because he is far more knowledgeable about this part of the analyzer. Comment