[clang] [clang][ExprConst] Add diagnostics for invalid binary arithmetic (PR #118475)

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 4 10:36:17 PST 2024


Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/118475 at github.com>


================
@@ -14548,8 +14548,21 @@ bool IntExprEvaluator::VisitBinaryOperator(const BinaryOperator *E) {
         return Error(E);
       const Expr *LHSExpr = LHSValue.Base.dyn_cast<const Expr *>();
       const Expr *RHSExpr = RHSValue.Base.dyn_cast<const Expr *>();
-      if (!LHSExpr || !RHSExpr)
-        return Error(E);
+      if (!LHSExpr || !RHSExpr) {
+        std::string LHS = LHSValue.toString(Info.Ctx, E->getLHS()->getType());
+        std::string RHS = RHSValue.toString(Info.Ctx, E->getRHS()->getType());
+        Info.FFDiag(E, diag::note_constexpr_pointer_arith_unspecified)
+            << LHS << RHS;
+        return false;
+      }
+
+      if (ArePotentiallyOverlappingStringLiterals(Info, LHSValue, RHSValue)) {
----------------
zygoloid wrote:

Yeah, that's a bit unfortunate. But it's how we order diagnostics in general: the "context" notes for an error (eg, template instantiation, macro expansion) appear between the error diagnostic and its proper notes. It'd be nice to address that in general somehow, but I think the *order* is correct, because the context notes explain how we got to the immediately preceding diagnostic. (Another weird thing: the "In file included from" context appears before the diagnostic whereas other context appears afterwards.)

I've been wondering if we should split the "note" diagnostic category into "notes" that provide additional information about the diagnostic, and "context" diagnostics that provide additional information about the location of the previous diagnostic. We could then de-emphasize the context diagnostics in some way -- perhaps using a less bright color for them, and perhaps omitting the snippet -- to make it easier to scan from a diagnostic to its notes.

But obviously none of that is in scope for this PR :)

https://github.com/llvm/llvm-project/pull/118475


More information about the cfe-commits mailing list