bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: bug#36370: 27.0.50; XFIXNAT called on negative numbers


From: Pip Cet
Subject: Re: bug#36370: 27.0.50; XFIXNAT called on negative numbers
Date: Sun, 30 Jun 2019 09:21:55 +0000

On Sat, Jun 29, 2019 at 5:31 PM Paul Eggert <address@hidden> wrote:
> >> This is not a valid use of 'assume'. It's documented that assume's argument
> >> should be free of side effects.
> >
> > But the compiler makes no such assumption
>
> Sure, but if the caller uses 'assume' contrary to its documentation, that's a
> problem with the caller's code, not with 'assume'. It's merely an 
> implementation
> detail as to which pothole the problematic code runs into.

I agree, for what it's worth, that the new documentation is clear on
what is and isn't allowed with assume(). It's just that it's a
significant API change from the previous state, and I dislike the API
change.

Again, this isn't about using `assume' contrary to its documentation,
it's about using it in a way that the compiler cannot prove obeys the
documented API, and thus handles suboptimally.

> > if GCC decided to add a
> > __builtin_assume() builtin, we could give it slightly different
> > semantics: that the expression passed to it evaluates to true, but
> > doesn't evaluate to false or fail to evaluate. Something like
> > __attribute__((does_return)) might do on a function.
>
> Yes, the expression should return true without side effects or looping. I 
> don't
> see this as an significant difference in semantics.

It's significant to GCC. It optimizes function calls that are known to
return differently from ordinary function calls.

> One should also not call
> Gnulib assume (R) with an expression that loops forever, as this defeats the
> intent of 'assume' which is to make code more efficient.

No disagreement there. However, that doesn't mean the compiler is free
to assume that assume()'s argument doesn't loop; it needs to prove
that, whereas with the new __builtin_assume() builtin, it wouldn't
have to go to that trouble.

> If there's any real
> confusion about this issue, we can add it to the 'assume' documentation as 
> well.

I think the new documentation is fine, in this regard.

> > Also, "should" doesn't mean "must", does it?
>
> It's not the "should" of an Internet RFC. It's more the "should" of "you 
> should
> do this, and if you don't you're on your own".

So if I add a debug print statement to an inline function that happens
to be called in an eassume, I'm on my own? That's a significant API
change.

> > I'd prefer rewording that
> > sentence as "R may or may not be evaluated: it should not normally
> > have side-effects".
>
> Better to say that it should not have side effects at all. There's no 
> "normally"
> about that. Side effects are trouble.

Except for debugging statements, other assert()s, abort()s, other
assume()s, divisions by zero that happen in the eassume() code rather
than after it...

Again, I think there are two issues here: you want to change the API
to be much more restrictive (my original patch intended to make it
more lenient), and you want to document the new API. Obviously, the
second point is fine.

If the new requirement is that R has no side effects, we don't need
the "R may or may not be evaluated". In fact, it might confuse others
in the same way I was confused.

I read the old documentation as:

As far as side effects go, each assume(R) may decide, independently
and unpredictably, whether side effects of R are visible or not: R may
or may not be evaluated. This unpredictable behavior can be confusing;
to avoid confusion, R RFC-SHOULD not have side effects.

the new documentation, I read as:

If R has any side effects, assume(R) is undefined. It can cause
compilation, linking, or run-time errors.

> > wouldn't it be even
> > nicer to give up (most of) the distinction between assert and assume
> > and just tell people to use assume?
>
> No, because 'assert (false)' has well-defined behavior, whereas behavior is
> undefined for programs that do 'assume (false)' . This is a fundamental
> difference between the two.

Thus the "most of", yes.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]