erichkeane added a subscriber: tahonermann. erichkeane added inline comments.
================ Comment at: clang/lib/Sema/SemaExpr.cpp:15539 +/// Emit a diagnostic together with a fixit hint that wraps the inner comparison +/// expression in parentheses. +static void EmitDiagnosticForCompOpInCompOp(Sema &Self, SourceLocation OpLoc, ---------------- I'd repalce the comment with something more like: ```Emit a diagnostic for a case where a comparison operation is a direct sub-expression of another comparison operation. Additionally, emit a fixit hint to suggest the inner comparison expression be wrapped in parentheses.``` ================ Comment at: clang/lib/Sema/SemaExpr.cpp:15548 + + SuggestParentheses(Self, Bop->getOperatorLoc(), + Self.PDiag(diag::note_precedence_silence) ---------------- I find myself wondering if we could provide a better 'suggested fix' here. Aaron is better with the diagnostics than I am, but I would think that someone doing: `a < b < c` PROBABLY doesn't mean that, they probably mean: `a < b && b < c`. Also, is the mistake the 'same' when they do something like `a > b != c` ? It would seem to me that mixing operators might make it something either more intentional/meaningful. ADDITIONALLY, in the case where they are booleans, these end up being overly noisy. The pattern of the == c (where 'c' is bool, or convertible to bool) is probably intentional. I think the logic here needs to be more complicated than just "Comparison within Comparison", however I don't have a fully formed idea of when to diagnose. @tahonermann : Do you perhaps have a good idea? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142800/new/ https://reviews.llvm.org/D142800 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
