[committed] [PR99680] Check empty constraint before using CONSTRAINT_LEN.

[Segher Boessenkool]

On Mon, Mar 22, 2021 at 10:22:55AM +0000, Richard Sandiford wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> > On Sat, Mar 20, 2021 at 10:53:32AM -0400, Vladimir Makarov via Gcc-patches wrote:
> >> It seems CONSTRAINT_LEN treats constraint '\0' as one having length 1.
> >
> > Maybe we should fix that?  See attached patch.
> 
> I don't strongly object, but CONSTRAINT_LEN (0) doesn't feel to me like a
> question we should be asking.  IMO it would be confusing to have, say:
> 
>   p += CONSTRAINT_LEN (*p);

The CONSTRAINT_LEN macro has two arguments: the first character of the
constraint string, and a pointer to it.  That first character is just a
premature optimisation if you ask me (and it shouldn't be a macro at
all, it should use a function), but the important point is that the
pointer to the string is the important one.  So your example reads as
  p += CONSTRAINT_LEN (*p, p);

> “stick” at a null terminator even though the code is notionally
> an increment.

Then don't write code like that?

Errors should be handled *some* way, instead of nastiness like
      len = CONSTRAINT_LEN (c, constraint);
      do
        constraint++;
      while (--len && *constraint && *constraint != ',');
(from recog.c).  But since CONSTRAINT_LEN (0, "") should return
*something*, returning length 0 makes a lot of sense.  Many things do
not need to treat it different from how real constraints are handled,
but everything that wants can do so.

> '\0' is just a normal string null terminator and so I don't think we
> should be processing it as if it were a constraint character.

But we don't.  We process it as if it is the first character of the
remaining constraint string.  Which it is :-)

> How about having a gcc_unreachable on zero instead?

a) Then it should *definitely* not be a macro.
b) We can do better error handling than that.


Segher