[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

aaron.ballman closed this revision. aaron.ballman added a comment. In D140860#4050402 , @hazohelet wrote: > I added the release note. > >> Also, do you need someone to commit on your behalf? If so, what name and >> email address would you like used for p

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

hazohelet updated this revision to Diff 488863. hazohelet added a comment. I added the release note. > Also, do you need someone to commit on your behalf? If so, what name and > email address would you like used for patch attribution? I don't have commit access. Would you please land this patch

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

aaron.ballman accepted this revision. aaron.ballman added a comment. Code LGTM but please add a release note for the change so folks are aware. Also, do you need someone to commit on your behalf? If so, what name and email address would you like used for patch attribution? CHANGES SINCE LAST A

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. In D140860#4047632 , @hazohelet wrote: > In D140860#4047534 , @aaron.ballman > wrote: > >> In D140860#40

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

hazohelet added a comment. In D140860#4047534 , @aaron.ballman wrote: > In D140860#4045224 , @dblaikie > wrote: > >> In D140860#4044937 , >> @aaron.ballman wrote: >> >>

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

aaron.ballman added a comment. In D140860#4045224 , @dblaikie wrote: > In D140860#4044937 , @aaron.ballman > wrote: > >> In D140860#4031872 , @dblaikie >> wrote: >> >>>

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

dblaikie added a comment. In D140860#4044937 , @aaron.ballman wrote: > In D140860#4031872 , @dblaikie > wrote: > >> The risk now is that this might significantly regress/add new findings for >> this warning tha

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

aaron.ballman added a comment. In D140860#4031872 , @dblaikie wrote: > The risk now is that this might significantly regress/add new findings for > this warning that may not be sufficiently bug-finding to be worth immediate > cleanup, causing users to h

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

hazohelet added a comment. > FWIW a lot of build systems support setting CXXFLAGS/CFLAGS before invoking > the build system/build generator (cmake, for instance) and respects those - > so might be relatively easy to add a new warning flag to the build (& CXX/CC > to set the compiler to point to

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

dblaikie added a comment. In D140860#4033530 , @hazohelet wrote: > I have yet to do thorough checks using this patched clang to build > significant code bases. > It will likely take quite a bit of time as I am not used to editing build > tool files. FW

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

hazohelet added a comment. I have yet to do thorough checks using this patched clang to build significant code bases. It will likely take quite a bit of time as I am not used to editing build tool files. Instead, I used `grep` to find potentially newly-warned codes. The `grep` command is shown

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

dblaikie added a comment. The risk now is that this might significantly regress/add new findings for this warning that may not be sufficiently bug-finding to be worth immediate cleanup, causing users to have to choose between extensive lower-value cleanup and disabling the warning entirely. Ha

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

hazohelet added a comment. > So, so long as a string literal still does the suppression, it's probably > fine. I agree with you. I now also think that integer literals _should not_ do the warning suppression because programmers are sometimes unsure of the expansion result of predefined macros

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

hazohelet updated this revision to Diff 486793. hazohelet added a comment. add up the former 2 commits into 1 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140860/new/ https://reviews.llvm.org/D140860 Files: clang/lib/Sema/SemaExpr.cpp clang/test/Sema/logical-op-parentheses.c Inde

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

hazohelet updated this revision to Diff 486776. hazohelet added a comment. This update limits the warning suppression case to string literals only, and delete no longer necessary functions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140860/new/ https://reviews.llvm.org/D140860 File

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

dblaikie added a comment. In D140860#4028089 , @hazohelet wrote: > As you point out, enhancement may be more accurate than bug fix. > There are rare cases where enabling a warning for missing parentheses in > `constexpr` logical expressions can be helpfu

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

hazohelet added a comment. As you point out, enhancement may be more accurate than bug fix. There are rare cases where enabling a warning for missing parentheses in `constexpr` logical expressions can be helpful, I think. For example, consider the following code: constexpr A = ...; constexp

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

dblaikie added a comment. > This behavior can lead to the lack of warnings for complicated constexpr > expressions. Are those bugs, though? If the parentheses wouldn't change the outcome, due to the RHS being constant, maybe these undiagnosed cases aren't reflective of source code bugs/coding

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

hazohelet created this revision. hazohelet added a project: clang. Herald added a project: All. hazohelet requested review of this revision. When using the `&&` operator within a `||` operator, both Clang and GCC produce a warning for potentially confusing operator precedence. However, Clang avoi