xbolva00 marked 2 inline comments as done.
xbolva00 added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8431
"result of comparison against %select{a string literal|@encode}0 is "
- "unspecified (use strncmp instead)">,
- InGroup<StringCompare>;
+ "unspecified">, InGroup<StringCompare>;
----------------
aaron.ballman wrote:
> I think this is a regression in the diagnostic because we dropped the
> recommendation for how to fix the issue. Why do we need to drop that? I'd be
> fine if it was made less specific, like: `(use an explicit string comparison
> function instead)` if the concern is over `strncmp` specifically.
Yes, my concern was about strncmp. Your suggestion is fine.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:10368
- if (Expr::isSameComparisonOperand(LHS, RHS)) {
- unsigned Result;
- switch (Opc) {
- case BO_EQ: case BO_LE: case BO_GE:
- Result = AlwaysTrue;
- break;
- case BO_NE: case BO_LT: case BO_GT:
- Result = AlwaysFalse;
- break;
- case BO_Cmp:
- Result = AlwaysEqual;
- break;
- default:
- Result = AlwaysConstant;
- break;
- }
- S.DiagRuntimeBehavior(Loc, nullptr,
- S.PDiag(diag::warn_comparison_always)
- << 0 /*self-comparison*/
- << Result);
- } else if (checkForArray(LHSStripped) && checkForArray(RHSStripped)) {
- // What is it always going to evaluate to?
- unsigned Result;
- switch(Opc) {
- case BO_EQ: // e.g. array1 == array2
- Result = AlwaysFalse;
- break;
- case BO_NE: // e.g. array1 != array2
- Result = AlwaysTrue;
- break;
- default: // e.g. array1 <= array2
- // The best we can say is 'a constant'
- Result = AlwaysConstant;
- break;
+ if (!LHS->getBeginLoc().isMacroID() && !RHS->getBeginLoc().isMacroID()) {
+ if (Expr::isSameComparisonOperand(LHS, RHS)) {
----------------
aaron.ballman wrote:
> Why do we care about the macros here? It seems just as reasonable to warn on:
> ```
> #define MACRO1 "one"
> #define MACRO2 "one
>
> if (MACRO1 == MACRO2) // Why not warn here too?
> ```
>
This “macro loc” check is for array comparison diagnostic. I preserved original
logic of this function and enabled -Wstring-compare for macro locations.
We warn here (I will add a test).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70624/new/
https://reviews.llvm.org/D70624
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
