Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Commit 996e6b9e17 breaks build with -Accflags="-DNO_LOCALE_NUMERIC" #19890

Closed
jkeenan opened this issue Jun 25, 2022 · 1 comment
Closed

Commit 996e6b9e17 breaks build with -Accflags="-DNO_LOCALE_NUMERIC" #19890

jkeenan opened this issue Jun 25, 2022 · 1 comment
Labels

Comments

@jkeenan
Copy link
Contributor

jkeenan commented Jun 25, 2022

When I compile with this ./Configure switch:

-Accflags="-DNO_LOCALE_NUMERIC"

... compilation fails on both OpenBSD and FreeBSD (using clang10 on both OSes) at this point:

$ sh ./Configure -des -Dusedevel -Accflags="-DNO_LOCALE_NUMERIC" && make miniperl | tee $P5P_DIR/make-output/`extract-sha`.openbsd.clang10.unthreaded.onlynolocalenumeric.makeminiperl.output.txt
...
cc -c -DPERL_CORE -DNO_LOCALE_NUMERIC -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -std=c99 -O2 -Wall -Werror=pointer-arith -Werror=vla -Wextra -Wno-long-long -Wno-declaration-after-statement -Wc++-compat -Wwrite-strings locale.c
locale.c:2293:13: error: use of undeclared identifier 'toggled'
        if (toggled) {
            ^
1 error generated.
*** Error 1 in /home/jkeenan/gitwork/perl (makefile:246 'locale.o': @`sh  cflags "optimize='-O2'" locale.o`  locale.c)

The above was observed on OpenBSD-6.9 at commit 7b92fe4. I have reproduced it on FreeBSD-12 as well. I originally spotted this problem in this smoke-test report: https://perl5.test-smoke.org/report/5019634. I customarily smoke-test on OpenBSD with -Accflags="-DNO_LOCALE_NUMERIC -DNO_LOCALE_COLLATE" because that is closer to the "system perl" configuration options used by @afresh1.

On an older Linux installation, with the same configuration I get:

$ uname -mrs
Linux 5.10.13-x86_64-linode141 x86_64
$ ./Configure -des -Dusedevel -Accflags="-DNO_LOCALE_NUMERIC" && make miniperl
...
cc -c -DPERL_CORE -std=gnu99 -DNO_LOCALE_NUMERIC -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2 -O2 -Wall -Werror=pointer-arith -Werror=vla -Wextra -Wno-long-long -Wno-declaration-after-statement -Wc++-compat -Wwrite-strings locale.c
locale.c: In function ‘Perl_setlocale’:
locale.c:2293:13: error: ‘toggled’ undeclared (first use in this function)
         if (toggled) {
             ^
locale.c:2293:13: note: each undeclared identifier is reported only once for each function it appears in
locale.c: At top level:
locale.c:1113:1: warning: ‘S_calculate_LC_ALL’ defined but not used [-Wunused-function]
 S_calculate_LC_ALL(pTHX_ const char ** individ_locales)
 ^
locale.c:1300:1: warning: ‘S_new_numeric’ defined but not used [-Wunused-function]
 S_new_numeric(pTHX_ const char *newnum)
 ^
locale.c:1264:1: warning: ‘S_set_numeric_radix’ defined but not used [-Wunused-function]
 S_set_numeric_radix(pTHX_ const bool use_locale)
 ^
make: *** [locale.o] Error 1
makefile:250: recipe for target 'locale.o' failed

The line in locale.c at which make failed was recently added:

commit 996e6b9e17bebd710ff658cdb123da073ac0ee97
Author:     Karl Williamson <khw@cpan.org>
AuthorDate: Sun Feb 21 06:01:20 2021 -0700
Commit:     Karl Williamson <khw@cpan.org>
CommitDate: Sun Jun 19 13:29:35 2022 -0600

    locale.c: Use low level macros at low level
    
    Implementing Perl_setlocale, we can safely use the internal macros that
    the public ones expand to call, without the overhead those public macros
    impose (which they do to be more immune from improper calls from outside
    code).

@khwilliamson, can you take a look?

khwilliamson added a commit that referenced this issue Jun 25, 2022
This fixes GH #19890.

This cleans up querying the current locale, which the blamed commit
caused a compilation error for on platforms without LC_NUMERIC.  That
commit was written before we required C99, and being able to move
declarations to not be at the beginning of a block made me realize that
things could be simplified by a bit of refactoring, which this commit
does.
khwilliamson added a commit that referenced this issue Jun 27, 2022
This fixes GH #19890.

This cleans up querying the current locale, broken by 996e6b9 on
platforms without LC_NUMERIC(*).

That commit was written before we required C99, and being able to move
declarations to not be at the beginning of a block made me realize that
things could be simplified by a bit of refactoring, which this commit
does.

(*): Lack of LC_NUMERIC can be simulated by using

  './Configure -Accflags="-DNO_LOCALE_NUMERIC'
@khwilliamson
Copy link
Contributor

Fixed by 6a7aca8

scottchiefbaker pushed a commit to scottchiefbaker/perl5 that referenced this issue Nov 3, 2022
This fixes GH Perl#19890.

This cleans up querying the current locale, broken by 996e6b9 on
platforms without LC_NUMERIC(*).

That commit was written before we required C99, and being able to move
declarations to not be at the beginning of a block made me realize that
things could be simplified by a bit of refactoring, which this commit
does.

(*): Lack of LC_NUMERIC can be simulated by using

  './Configure -Accflags="-DNO_LOCALE_NUMERIC'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants