Re: RFA: Improve message for wrong number of alternatives

[Joern Rennecke]

On Sun, 16 May 2021 at 22:01, Martin Sebor <mse...@gmail.com> wrote:
 > I think it's very helpful to provide this sort of detail.  Just as
> a matter of readability, the new error message
>
>    "wrong number of alternatives in operand %d, %d, expected %d"
>
> would be improved by avoiding the two consecutive %d's,

We could also do that by phrasing it:

"wrong number of alternatives in operand %d, seen: %d, expected: %d"

so that the change is just about adding extra information.

> e.g., by
> rephrasing it like so:
>
>    "%d alternatives provided to operand %d where %d are expected"

This has an additional change in that we no longer jump to the conclusion
that the operand where we notice the discrepancy is the point that's wrong.
I suppose that conclusion is more often right than wrong (assuming more than
two operands on average for patterns that have alternatives and at least two
operands), but when it's wrong, it's particularly confusing and/or jarring,
so it's an improvement to just stick to the known facts.
But if we go that way, I suppose we should spell also out where the
expectation comes from: we have a loop over the operands, and we look at
operand 0 first.  We could do that by using the diagnostic:

              error_at (d->loc,
                        "alternative number mismatch: operand %d has
%d, operand %d had %d",
                        start, d->operand[start].n_alternatives, 0, n);


I notice in passing here that printf is actually awkward for repharasings
and hence also for translations, because we can't interchange the order of
the data in the message string.

But for multi-alternative patterns, we also have the awkwardness of
repeating the abstract of the error message and the recap of the number
of alternatives of operand 0.

So I propose the attached patch now.

Bootstrapped on x86_64-pc-linux-gnu.

2021-05-17  Joern Rennecke  <joern.renne...@embecosm.com>

        Make "wrong number of alternatives" message more specific, and
        remove assumption on where the problem is.

diff --git a/gcc/genoutput.c b/gcc/genoutput.c
index 8e911cce2f5..6313b722cf7 100644
--- a/gcc/genoutput.c
+++ b/gcc/genoutput.c
@@ -757,6 +757,7 @@ validate_insn_alternatives (class data *d)
        int which_alternative = 0;
        int alternative_count_unsure = 0;
        bool seen_write = false;
+       bool alt_mismatch = false;
 
        for (p = d->operand[start].constraint; (c = *p); p += len)
          {
@@ -813,8 +814,19 @@ validate_insn_alternatives (class data *d)
            if (n == 0)
              n = d->operand[start].n_alternatives;
            else if (n != d->operand[start].n_alternatives)
-             error_at (d->loc, "wrong number of alternatives in operand %d",
-                       start);
+             {
+               if (!alt_mismatch)
+                 {
+                   alt_mismatch = true;
+                   error_at (d->loc,
+                             "alternative number mismatch: "
+                             "operand %d had %d, operand %d has %d",
+                             0, n, start, d->operand[start].n_alternatives);
+                 }
+               else
+                 error_at (d->loc, "operand %d has %d alternatives",
+                   start, d->operand[start].n_alternatives);
+             }
          }
       }