[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

aaron.ballman added a comment. In D142609#4568095 , @xgupta wrote: > In D142609#4566418 , @aaron.ballman > wrote: > >> Concerns have been raised in >> https://github.com/llvm/llvm-project/issues/64356 that this

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta added a comment. In D142609#4566418 , @aaron.ballman wrote: > Concerns have been raised in > https://github.com/llvm/llvm-project/issues/64356 that this is an undesirable > change in diagnostic behavior. The diagnostic is supposed to fire when

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

aaron.ballman added subscribers: porglezomp, cjdb. aaron.ballman added a comment. Concerns have been raised in https://github.com/llvm/llvm-project/issues/64356 that this is an undesirable change in diagnostic behavior. The diagnostic is supposed to fire when misusing a logical operand that most

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

aaron.ballman added a comment. In D142609#4513478 , @xgupta wrote: > Thanks, @nickdesaulniers for reviewing and @nathanchance for testing the > change. > > @aaron.ballman, I also agree with @xbolva00 seems warning in kernel code is > valid but I also ag

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

nathanchance added a comment. For the record, I have sent https://lore.kernel.org/20230718-nsecs_to_jiffies_timeout-constant-logical-operand-v1-0-36ed8fc8f...@kernel.org/ to address those warnings for the kernel. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta added a comment. Thanks, @nickdesaulniers for reviewing and @nathanchance for testing the change. @aaron.ballman, I also agree with @xbolva00 seems warning in kernel code is valid but I also agree with your comment about macro, may be better to track the macro-related issue with another

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

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 rGdfdfd306cfaf: [Clang] Fix -Wconstant-logical-operand when LHS is a constant (authored by shivam-amd). Changed prior to co

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

nathanchance added a comment. In D142609#4511579 , @nickdesaulniers wrote: > In D142609#4507696 , @nathanchance > wrote: > >> but I see a new one along a similar line as those: >> >> https://elixir.bootlin.com/l

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

nickdesaulniers added a comment. In D142609#4507696 , @nathanchance wrote: > but I see a new one along a similar line as those: > > https://elixir.bootlin.com/linux/v6.5-rc2/source/drivers/gpu/drm/v3d/v3d_drv.h#L343 I only see 4 instances of `NSEC_PER_S

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta added a comment. In D142609#4510995 , @aaron.ballman wrote: > In D142609#4510596 , @xbolva00 > wrote: > >> In my opinion it makes sense to adjust that kernel code based on this >> warning in the current

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

aaron.ballman added a comment. In D142609#4510596 , @xbolva00 wrote: > In my opinion it makes sense to adjust that kernel code based on this warning > in the current inplementation state. > > @aaron.ballman ? I think that use of macros for any of the co

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xbolva00 added a comment. I my opinion it makes sense to adjust that kernel code based on this warning in the current inplementation state. @aaron.ballman ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta added a comment. In D142609#4507696 , @nathanchance wrote: > I took the most recent version for a spin against the Linux kernel. The > couple of issues that a previous revision of the warning flagged > (https://github.com/ClangBuiltLinux/linux/i

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

nathanchance added a comment. I took the most recent version for a spin against the Linux kernel. The couple of issues that a previous revision of the warning flagged (https://github.com/ClangBuiltLinux/linux/issues/1806, https://github.com/ClangBuiltLinux/linux/issues/1807) are no longer visib

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta updated this revision to Diff 540922. xgupta added a comment. Address xbolva00's comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaExpr.cpp clang/test/C/drs/dr4xx.c

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xbolva00 added a comment. We should not warn for macros. if (ENABLE_XYZ && x) {} } This pattern is used in real world codebases. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609 ___ cfe-commits mail

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:13635 + if (ECDHS && ECDHS->getInitVal() != 0 && ECDHS->getInitVal() != 1) +EnumConstantInBoolContext = true; +} xgupta wrote: > nickdesaulniers wrote: > > Hmm...I wonder if we

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta updated this revision to Diff 540914. xgupta marked 5 inline comments as done. xgupta added a comment. Address comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaExpr.c

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

nickdesaulniers added inline comments. Comment at: clang/test/SemaCXX/expressions.cpp:146-148 #define Y2 2 bool r2 = X || Y2; // expected-warning {{use of logical '||' with constant operand}} \ // expected-note {{use '|' for a bitwise operation}}

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

nickdesaulniers added a comment. Hmm...looking at some of the commits to the related code, it might be very intentional that we don't warn symmetrically: 938533db602b32ab435078e723b656ac6e779a1b e54ff6cc3e479523b71e4c7eb4bd13707d84de0f Comment at: clang/test/SemaCXX/expressio

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

nickdesaulniers added a comment. I did kernel builds of x86_64 and aarch64 defconfigs. This found new instance: https://github.com/ClangBuiltLinux/linux/issues/1806 which looks like something we can fix in the kernel sources. Our CI will probably find more instances once this lands, but I'm happ

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta added a comment. gentle ping for review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta marked 5 inline comments as done. xgupta added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:13635 + if (ECDHS && ECDHS->getInitVal() != 0 && ECDHS->getInitVal() != 1) +EnumConstantInBoolContext = true; +} nickdesaulniers wrote:

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta updated this revision to Diff 495375. xgupta added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaExpr.cpp

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

nickdesaulniers added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:13583 +// bitwise one. +void Sema::diagnoseLogicalInsteadOfBitwise(ExprResult &LHS, ExprResult &RHS, SourceLocation Loc, Instead of `LHS` an

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta updated this revision to Diff 495122. xgupta added a comment. remove old code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaExpr.cpp

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta updated this revision to Diff 495116. xgupta added a comment. use function to avoid code duplication Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609 Files: clang/include/clang/Sema/Sema.h clang

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xbolva00 requested changes to this revision. xbolva00 added a comment. This revision now requires changes to proceed. Huge code duplication. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609 ___

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:13611-13653 if (RHS.get()->EvaluateAsInt(EVResult, Context)) { llvm::APSInt Result = EVResult.Val.getInt(); if ((getLangOpts().Bool && !RHS.get()->getType()->isBooleanType() && !R

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta updated this revision to Diff 494631. xgupta added a comment. remov blank Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609 Files: clang/lib/Sema/SemaExpr.cpp clang/test/C/drs/dr4xx.c clang/tes

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta updated this revision to Diff 494629. xgupta added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609 Files: clang/lib/Sema/SemaExpr.cpp Index: clang/lib/Sema/SemaExpr.cpp

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta updated this revision to Diff 492995. xgupta added a comment. update test cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609 Files: clang/lib/Sema/SemaExpr.cpp clang/test/C/drs/dr4xx.c cla

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

nickdesaulniers added a comment. Thanks for the patch! In D142609#4082259 , @xgupta wrote: > WIP && TODO- Fixed test > > Failed Tests (7): > > Clang :: C/drs/dr4xx.c > Clang :: Modules/explicit-build-extra-files.cpp > Clang :: Parser/cxx2a-concept-

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta added a comment. WIP && TODO- Fixed test Failed Tests (7): Clang :: C/drs/dr4xx.c Clang :: Modules/explicit-build-extra-files.cpp Clang :: Parser/cxx2a-concept-declaration.cpp Clang :: SemaCXX/expressions.cpp Clang :: SemaCXX/warn-unsequenced.cpp Clang :: SemaObjC/unguarded-av

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta updated this revision to Diff 492374. xgupta added a comment. add test cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142609/new/ https://reviews.llvm.org/D142609 Files: clang/lib/Sema/SemaExpr.cpp clang/test/Sema/exprs.c clang/t

[PATCH] D142609: [Clang] Fix -Wconstant-logical-operand when LHS is a constant

xgupta created this revision. xgupta added reviewers: nickdesaulniers, aaron.ballman. Herald added a project: All. xgupta requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This fix PR37919 The below code produces -Wconstant-logical-operand f