Re: emacs-28 9c222b9: Port to C compilers that lack size-0 arrays

Re: emacs-28 9c222b9: Port to C compilers that lack size-0 arrays

[Eli Zaretskii]

> branch: emacs-28
> commit 9c222b9c1a7f91497a37567b4d7de3a511fff069
> Author: Paul Eggert <eggert@cs.ucla.edu>
> Commit: Paul Eggert <eggert@cs.ucla.edu>
> 
>     Port to C compilers that lack size-0 arrays
>     
>     The C standard does not allow size-zero arrays, so redo struct
>     Lisp_Subr to not use size-zero arrays when native compilation is
>     not being used.  Formerly, the code was using size-zero arrays (a
>     GNU C extension) to avoid using memory unnecessarily when
>     HAVE_NATIVE_COMP is not defined.  Replace this hack with the
>     more-traditional hack of putting the relevant members inside
>     ‘#ifdef HAVE_NATIVE_COMP’.
>     * src/alloc.c (cleanup_vector, mark_object):
>     * src/comp.c (make_subr):
>     * src/data.c (Fsubr_native_lambda_list, Fsubr_native_comp_unit):
>     * src/eval.c (init_eval_once, funcall_lambda):
>     * src/lisp.h (SUBR_NATIVE_COMPILEDP, SUBR_NATIVE_COMPILED_DYNP)
>     (SUBR_TYPE):
>     * src/lread.c (Fload):
>     Conditionally compile with ‘#ifdef HAVE_NATIVE_COMP’ instead of
>     with ‘if (NATIVE_COMP_FLAG)’.  Redo members like native_comp_u[0]
>     to be plain native_comp_u.  Put all uses of these members inside
>     ‘#ifdef HAVE_NATIVE_COMP’.
>     * src/lisp.h (struct Lisp_Subr): Members native_comp_u,
>     native_c_name, lambda_list, type are now all ifdeffed out if
>     HAVE_NATIVE_COMP is not defined, instead of being size-zero
>     arrays.  All uses changed.
>     * src/pdumper.c (dump_subr, dump_cold_native_subr)
>     (dump_do_dump_relocation):
>     * src/comp.h (NATIVE_COMP_FLAG): Remove; no longer needed.

Paul, why did you install this large and non-trivial changeset on the
release branch without any discussion?  Please don't do that.  We have
already started the pretest.

How serious is the problem this attempts to solve, and what bad things
will happen if we release Emacs 28.1 without those changes?

Re: emacs-28 9c222b9: Port to C compilers that lack size-0 arrays

[Paul Eggert]

On 12/3/21 00:16, Eli Zaretskii wrote:

> We have already started the pretest.

Yes, and the pretest is why I found the problems I fixed: I built and 
tested on less-common platforms that I normally don't bother with.

> How serious is the problem this attempts to solve, and what bad things
> will happen if we release Emacs 28.1 without those changes?

What I experienced was a ton of diagnostics with both the Oracle Solaris 
and the IBM AIX C compilers, which made it difficult for me to see the 
real problems on those platforms (of which there were some). Although I 
was able to finish the builds anyway, the old code was a clear violation 
of the C89/C99/C11 standards and a good indication that there will be 
problems building on other less-common platforms that I don't have 
access to.

Conforming to the C standard doesn't affect behavior here (it possibly 
doesn't even change the machine code, though I didn't check this) so I 
considered the change to be a safe prophylactic.

Re: emacs-28 9c222b9: Port to C compilers that lack size-0 arrays

[Eli Zaretskii]

> Date: Fri, 3 Dec 2021 08:46:23 -0800
> Cc: emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> What I experienced was a ton of diagnostics with both the Oracle Solaris 
> and the IBM AIX C compilers, which made it difficult for me to see the 
> real problems on those platforms (of which there were some).

Any reason why we should be concerned about these two platforms?  They
are proprietary compilers on proprietary OSes.

> Conforming to the C standard doesn't affect behavior here (it possibly 
> doesn't even change the machine code, though I didn't check this) so I 
> considered the change to be a safe prophylactic.

Famous last words.

Please always raise the issue on the list before installing such
non-trivial changes on the release branch.

Re: emacs-28 9c222b9: Port to C compilers that lack size-0 arrays

[Paul Eggert]

On 12/3/21 10:29, Eli Zaretskii wrote:
>> What I experienced was a ton of diagnostics with both the Oracle Solaris
>> and the IBM AIX C compilers, which made it difficult for me to see the
>> real problems on those platforms (of which there were some).
> Any reason why we should be concerned about these two platforms?

Emacs has run on Solaris and AIX for decades, these platforms are stable 
and are supported by Oracle and IBM respectively, and there's very 
little trouble keeping Emacs working on these platforms.

And it's not just these two platforms. My bigger motivation for this 
particular change was that the old code violates a constraint specified 
by the C standard. Even though the old code works on GCC, Clang and 
MSVC, there's a reasonable chance it won't work on less-commonly-used 
compilers and there's little point to taking that risk when the fix is 
so simple.

Re: emacs-28 9c222b9: Port to C compilers that lack size-0 arrays

[Eli Zaretskii]

> Date: Fri, 3 Dec 2021 11:42:08 -0800
> Cc: emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> On 12/3/21 10:29, Eli Zaretskii wrote:
> >> What I experienced was a ton of diagnostics with both the Oracle Solaris
> >> and the IBM AIX C compilers, which made it difficult for me to see the
> >> real problems on those platforms (of which there were some).
> > Any reason why we should be concerned about these two platforms?
> 
> Emacs has run on Solaris and AIX for decades, these platforms are stable 
> and are supported by Oracle and IBM respectively, and there's very 
> little trouble keeping Emacs working on these platforms.

Emacs still did compile on those platforms before your changes.  So we
are not talking about dropping support for them or breaking the build
on them, we are talking about avoiding warnings.

> And it's not just these two platforms. My bigger motivation for this 
> particular change was that the old code violates a constraint specified 
> by the C standard. Even though the old code works on GCC, Clang and 
> MSVC, there's a reasonable chance it won't work on less-commonly-used 
> compilers and there's little point to taking that risk when the fix is 
> so simple.

That's okay, but such cleanup changes should be done on master, not on
the release branch during a pretest.  The time for cleanups on the
release branch has come and gone.  (And we don't support MSVC builds
for a very long time now.)

So I'm inclined to revert that changeset on the release branch.

Re: emacs-28 9c222b9: Port to C compilers that lack size-0 arrays

[Paul Eggert]

On 12/3/21 12:01, Eli Zaretskii wrote:

> Emacs still did compile on those platforms before your changes.

Actually the Emacs compile failed on both platforms, with many 
diagnostics, before my changes today. You're correct that the 
size-zero-array part of those changes was not needed to unbreak the two 
builds. Still, other builders may run into other less-commonly-used 
platforms that do not support this GNU C extension.


>> there's a reasonable chance it won't work on less-commonly-used
>> compilers and there's little point to taking that risk when the fix is
>> so simple.
> 
> That's okay, but such cleanup changes should be done on master, not on
> the release branch during a pretest.  The time for cleanups on the
> release branch has come and gone.  (And we don't support MSVC builds
> for a very long time now.)

Oh, I didn't know that Emacs builds with Microsoft compilers were no 
longer supported. Live and learn.... I suppose at some point any 
remaining references to MSVC could be removed from the Emacs source, if 
only to prevent further confusion in this area. Of course that'd be a 
low-priority cleanup.

Anyway, the size-zero-array patch wasn't intended to be a mere cleanup 
change; it was a fix to avoid a portability regression in Emacs 28 with 
respect to this Standard C constraint. But if you'd rather move the 
patch to the master branch, feel free; or let me know and I'll do it.

Re: emacs-28 9c222b9: Port to C compilers that lack size-0 arrays

[Eli Zaretskii]

> Date: Fri, 3 Dec 2021 15:32:34 -0800
> Cc: emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> Oh, I didn't know that Emacs builds with Microsoft compilers were no 
> longer supported. Live and learn....

We only support GCC and GNU Binutils for building Emacs on MS-Windows.
(And the MS-DOS build never supported anything else.)

> I suppose at some point any remaining references to MSVC could be
> removed from the Emacs source, if only to prevent further confusion
> in this area.

Yes.

> Anyway, the size-zero-array patch wasn't intended to be a mere cleanup 
> change; it was a fix to avoid a portability regression in Emacs 28 with 
> respect to this Standard C constraint. But if you'd rather move the 
> patch to the master branch, feel free; or let me know and I'll do it.

Let's see if it actually causes some trouble on some system.  If it
does, I will ask you to revert it on the release branch.

But in the future, please ask first.

Re: emacs-28 9c222b9: Port to C compilers that lack size-0 arrays

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> I suppose at some point any remaining references to MSVC could be
>> removed from the Emacs source, if only to prevent further confusion
>> in this area.
>
> Yes.

It would be good to start with changing the entry on MSVC in
efaq-w32.texi to say that it is unsupported, I think.

Re: emacs-28 9c222b9: Port to C compilers that lack size-0 arrays

[Po Lu]

Stefan Kangas <stefankangas@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> I suppose at some point any remaining references to MSVC could be
>>> removed from the Emacs source, if only to prevent further confusion
>>> in this area.
>>
>> Yes.
>
> It would be good to start with changing the entry on MSVC in
> efaq-w32.texi to say that it is unsupported, I think.

Doesn't that talk about developing software with MSVC, as opposed to
building Emacs with MSVC?

Re: emacs-28 9c222b9: Port to C compilers that lack size-0 arrays

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> Emacs still did compile on those platforms before your changes.  So we
> are not talking about dropping support for them or breaking the build
> on them, we are talking about avoiding warnings.

FWIW, I can't reproduce the glyph-related crash I found with Developer
Studio (the Sun compiler) anymore after this change.

It is exactly 9c222b9 that fixed the problem.

> So I'm inclined to revert that changeset on the release branch.

I don't think that would be the best idea, see above.

Thanks.