Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Lars Ingebrigtsen]

lekktu@gmail.com (Juanma Barranquero) writes:

>     lisp/*.el: Force non-nil result to t, to match docstring
>
>     * lisp/emacs-lock.el (emacs-lock-live-process-p):
>     * lisp/shadowfile.el (shadow-file-match):

(etc)

Aren't these mostly doc string bugs, though?  That is, the doc string
says "t" instead of "non-nil"; it's a common bug.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Juanma Barranquero]

[-- Attachment #1: Type: text/plain, Size: 519 bytes --]

On Thu, Oct 17, 2019 at 3:08 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Aren't these mostly doc string bugs, though?  That is, the doc string
> says "t" instead of "non-nil"; it's a common bug.

Depends. When the function is a predicate, t/nil makes more sense
(generally speaking, there are exceptions) that non-nil/nil.

In this and the other patches, I've opted for the fix (either doc fix, or
code fix) that I thought it was more logical, or matched better the
function's intent, or less prone to cause a bug.

[-- Attachment #2: Type: text/html, Size: 670 bytes --]

Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Lars Ingebrigtsen]

Juanma Barranquero <lekktu@gmail.com> writes:

> On Thu, Oct 17, 2019 at 3:08 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
>> Aren't these mostly doc string bugs, though?  That is, the doc string
>> says "t" instead of "non-nil"; it's a common bug.
>
> Depends. When the function is a predicate, t/nil makes more sense
> (generally speaking, there are exceptions) that non-nil/nil.

I thought it was kinda opposite: When it's a predicate, we only care
about nil/non-nil.

We only care about the distinction between non-nil and t when we have a
function that can return many different values, and t has a special
meaning among those values.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Juanma Barranquero]

[-- Attachment #1: Type: text/plain, Size: 439 bytes --]

On Thu, Oct 17, 2019 at 4:19 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:

> I thought it was kinda opposite: When it's a predicate, we only care
> about nil/non-nil.

Most predicates are defined as t/nil, like numberp, booleanp, etc. Usually,
the code that uses them only worries about the result being nil/non-nil,
but there are some exceptions.

Anyway, if you feel strongly about anything on my recent patches, feel free
to change it.

[-- Attachment #2: Type: text/html, Size: 575 bytes --]

Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Ergus]

On Thu, Oct 17, 2019 at 04:26:34AM +0200, Juanma Barranquero wrote:
>On Thu, Oct 17, 2019 at 4:19 AM Lars Ingebrigtsen <larsi@gnus.org> wrote:
>
>> I thought it was kinda opposite: When it's a predicate, we only care
>> about nil/non-nil.
>
>Most predicates are defined as t/nil, like numberp, booleanp, etc. Usually,
>the code that uses them only worries about the result being nil/non-nil,
>but there are some exceptions.
>
>Anyway, if you feel strongly about anything on my recent patches, feel free
>to change it.

AFAIK: In lisp true means anything non-nil for most of the
purposes. This change I am not sure how convenient is it. It just ads
some extra complexity to read with no benefit.

Maybe it made more sense to fix the docstrings to say non-nil instead of
forcing the code to return t?

Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Juanma Barranquero]

[-- Attachment #1: Type: text/plain, Size: 659 bytes --]

On Thu, Oct 17, 2019 at 4:50 AM Ergus <spacibba@aol.com> wrote:

> AFAIK: In lisp true means anything non-nil for most of the
> purposes.

Yes. And still, most predicates are defined as returning t/nil.

> This change I am not sure how convenient is it. It just ads
> some extra complexity to read with no benefit.

There are lots of places in the sources that already use (and ... t) to
normalize the result to t. It's not a lot of complexity.

> Maybe it made more sense to fix the docstrings to say non-nil instead of
> forcing the code to return t?

That means that you're sure that there's no code out there depending on the
(documented) result being t.

[-- Attachment #2: Type: text/html, Size: 894 bytes --]

Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Lars Ingebrigtsen]

Juanma Barranquero <lekktu@gmail.com> writes:

> That means that you're sure that there's no code out there depending on the
> (documented) result being t.

If that code existed, it would have failed, wouldn't it?  So it seems
unlikely.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Juanma Barranquero]

[-- Attachment #1: Type: text/plain, Size: 658 bytes --]

> If that code existed, it would have failed, wouldn't it?  So it seems
> unlikely.

Either that, or the users just did the (and ... t) trick to normalize the
result themselves.

Or it failed, but very rarely. In some cases that I've fixed, the t result
can be produced through many branches of an `or', for example, and most of
them already returned t, but not all.

Or look at speedbar-this-file-in-vc: it had a flaw, because it
promised non-nil but could in some cases return the result of
`run-hook-with-args' (which is documented as having an unspecified return
value). So certainly such bugs can exist and just be triggered in very
unusual situations.

[-- Attachment #2: Type: text/html, Size: 744 bytes --]

Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 17 Oct 2019 04:19:10 +0200
> Cc: Emacs developers <emacs-devel@gnu.org>
> 
> > Depends. When the function is a predicate, t/nil makes more sense
> > (generally speaking, there are exceptions) that non-nil/nil.
> 
> I thought it was kinda opposite: When it's a predicate, we only care
> about nil/non-nil.

No, predicate returns a boolean result.  Recall the recent discussion
of using -p in function and variable names.

RE: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Drew Adams]

> > > Depends. When the function is a predicate, t/nil makes more sense
> > > (generally speaking, there are exceptions) that non-nil/nil.
> >
> > I thought it was kinda opposite: When it's a predicate, we only care
> > about nil/non-nil.
> 
> No, predicate returns a boolean result.  Recall the recent discussion
> of using -p in function and variable names.

That begs the question of what "boolean result"
means.  Is the number 42 or the string "Bernie"
or "false" a Boolean value (true), or not?

In Lisp, in general (e.g., other than Scheme),
Boolean values include so-called "generalized
booleans", which means nil (for false) and
any non-nil value (for true).

That some particular function (predicate) might
want to always return t as the non-nil value to
indicate true is up to that function, and if
the fact that it does so is important to its
behavior then it should be documented that it
does so.

If there's no good reason for a predicate to
return something other than t to represent
true, then it should typically just use t.

But in general there should be no requirement
for a predicate to return t for true.  Any
non-nil value means true, a priori.

I agree with this general and longstanding
Lisp convention.  So +1 for not, in general
depending on t to be the only true value.

So IIUC, I agree with Lars on this general
question.

There's nothing wrong with a predicate always
returning only t for true, of course.  That
doesn't affect how other predicates handle
generalized Boolean values.  t is non-nil,
after all, even if non-nil is not necessarily
t.

(Not saying anything about whether this or
that particular predicate is better "fixed"
by ensuring that it uses only t for true or
changing its doc to say that t is the only
true value.  Not saying anything about the
"fixes" Juanma made.)

Node (elisp) `nil and t' says it very well:

 In contexts where a truth value is expected,
 any non-'nil' value is considered to be TRUE.

 However, 't' is the preferred way to represent
 the truth value TRUE.  When you need to choose
 a value that represents TRUE, and there is no
 other basis for choosing, use 't'.  The symbol
 't' always has the value 't'.

I agree with every bit of that text.

If there is no good reason not to use t, use t.
But something TESTING a value as a truth value
should, in general, consider any non-nil value
to be true.

("In general", because sometimes the value to
test can be either a truth value or something
else.  In such a case, different non-nil values
can mean different things.)

And there is often a good reason for a given
predicate to return some other non-nil value
than t for true.  Why is this true so often?
Because it lets the predicate be, secondarily,
more than a predicate, returning secondary
information along with truth value t. 

Common Lisp is also a good reference in this
regard.  "In Common Lisp, as in most Lisp
dialects..."

https://www.cs.cmu.edu/Groups/AI/html/cltl/clm/node9.html

http://www.lispworks.com/documentation/HyperSpec/Body/26_glo_g.htm

Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Juanma Barranquero]

[-- Attachment #1: Type: text/plain, Size: 1284 bytes --]

The issue here is not transforming every "predicate" -p function into one
returning t/nil. That's not what my patches did. And in my original answer
I already said that most predicates return t/nil, but not *all*. I would
contend that "predicates" not returning t/nil aren't really predicates, but
that's a discussion for another day.

Most of my latest patches make the function follow its documented interface
(the docstring), which shouldn't be so controversial. In fact, if some
function relied on the documented t result, if was risking getting
something different, often in obscure cases. So I'd say this patch will
help uncover bugs. (I'd be surprised that a function, for example,
documented as "return t", returning a marker in *some* cases, is being used
purposely so by the client code.)

In other cases, they fix problems; either because the docstring says that
it returns t, but in fact they return a useful non-nil value, or because
the functions say non-nil/nil, but are in fact functions that, but it's
very nature, will always return only t/nil. That shouldn't be
controversial, either.

I mean, if some specific change in my patches is a mistake, let's discuss
it or fix it. But mostly, what I've done is fixing latent bugs, either in
the documentation or the code.

[-- Attachment #2: Type: text/html, Size: 1402 bytes --]

Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Stefan Monnier]

Just to be clear: this is a typical bikeshed subject.

Some people think functions that return booleans should return nil/t and
other think they should return nil/non-nil.  It's a question
of opinion.  There are advantages on both sides.

So far, AFAIK most of the code and docstrings use nil/non-nil, AFAICT
(which is why we're all familiar with the notion of "non-nil" as
a matter of fact) and as a maintainer I've made an effort to try and
standardize on this, e.g. by fixing docstrings which overspecified the
return value to `t`.

This is similar to the issue of documenting side-effecting function's
return value versus discouraging the use of their return value.

The most important thing is to try and choose one and stick to it, both
for the benefit of consistency and to avoid back-and-forth
cosmetic changes.


        Stefan


Juanma Barranquero <lekktu@gmail.com> writes:

> The issue here is not transforming every "predicate" -p function into one
> returning t/nil. That's not what my patches did. And in my original answer
> I already said that most predicates return t/nil, but not *all*. I would
> contend that "predicates" not returning t/nil aren't really predicates, but
> that's a discussion for another day.
>
> Most of my latest patches make the function follow its documented interface
> (the docstring), which shouldn't be so controversial. In fact, if some
> function relied on the documented t result, if was risking getting
> something different, often in obscure cases. So I'd say this patch will
> help uncover bugs. (I'd be surprised that a function, for example,
> documented as "return t", returning a marker in *some* cases, is being used
> purposely so by the client code.)
>
> In other cases, they fix problems; either because the docstring says that
> it returns t, but in fact they return a useful non-nil value, or because
> the functions say non-nil/nil, but are in fact functions that, but it's
> very nature, will always return only t/nil. That shouldn't be
> controversial, either.
>
> I mean, if some specific change in my patches is a mistake, let's discuss
> it or fix it. But mostly, what I've done is fixing latent bugs, either in
> the documentation or the code.

Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Juanma Barranquero]

[-- Attachment #1: Type: text/plain, Size: 959 bytes --]

On Thu, Oct 17, 2019 at 8:02 PM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:
>
> Just to be clear: this is a typical bikeshed subject.

Definitely.

> So far, AFAIK most of the code and docstrings use nil/non-nil, AFAICT
> (which is why we're all familiar with the notion of "non-nil" as
> a matter of fact) and as a maintainer I've made an effort to try and
> standardize on this, e.g. by fixing docstrings which overspecified the
> return value to `t`.

There's a cascading effect. Some functions with multiple non-nil return
values rely on other functions's declared boolean result to construct their
own. So I think functions that can only return a boolean are not
"overspecified" by saying so. YMMV.

> The most important thing is to try and choose one and stick to it, both
> for the benefit of consistency and to avoid back-and-forth
> cosmetic changes.

FWIW, I don't consider making a function return what its docstring says "a
cosmetic change".

[-- Attachment #2: Type: text/html, Size: 1225 bytes --]

Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Stefan Monnier]

> There's a cascading effect.

Indeed, that's another reason why consistency is beneficial.

> So I think functions that can only return a boolean are not
> "overspecified" by saying so.

That's because you think they should return t.  For those whose think
they should return non-nil, it's overspecified (even if they do only
ever return t in practice).

> FWIW, I don't consider making a function return what its docstring says "a
> cosmetic change".

I think a single bikeshed subject is sufficient, so I won't take this
opportunity to debate the definition of "cosmetic".


        Stefan

Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Thu, 17 Oct 2019 14:02:49 -0400
> Cc: Eli Zaretskii <eliz@gnu.org>, Lars Ingebrigtsen <larsi@gnus.org>,
>  Drew Adams <drew.adams@oracle.com>, Emacs developers <emacs-devel@gnu.org>
> 
> Just to be clear: this is a typical bikeshed subject.

I don't think I agree.

I'd like to thank Juanma for quietly working on increasing the
correlation between the code and the documentation, in this and other
cases.  I think it's an important task, albeit a mundane one.

Re: master 2bb0703 1/2: lisp/*.el: Force non-nil result to t, to match docstring

[Stefan Monnier]

> I'd like to thank Juanma for quietly working on increasing the
> correlation between the code and the documentation, in this and other
> cases.  I think it's an important task, albeit a mundane one.

Agreed.  The bikeshed part is the question of whether the doc or the
code should be changed.


        Stefan