Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.98k stars 560 forks source link

BBC: 0351a629e7 prevents List-Keywords from building #20743

Closed jkeenan closed 1 year ago

jkeenan commented 1 year ago

On IRC #p5p yesterday it was mentioned that CPAN distribution List-Keywords was no longer building against blead. Sample failure report. Bisection of failure reports with the following command:

perl Porting/bisect.pl \
--module=List::Keywords \
--start=fecef29147d516d89233c43e3a9c9800e6c7f9f4 \
--end=1e45e087ebebb524df3c2247e485d7d6aebcd6f0

... pointed to 0351a629e71de127cbfd1b142e9eaa6069deabf5

0351a629e71de127cbfd1b142e9eaa6069deabf5 is the first bad commit
commit 0351a629e71de127cbfd1b142e9eaa6069deabf5
Author: Tomasz Konojacki <me@xenu.pl>
Date:   Sat Jun 18 07:26:58 2022 +0200
Commit:     Tomasz Konojacki <me@xenu.pl>
CommitDate: Sat Jun 18 08:51:14 2022 +0200

    hide private functions with __attribute__((visibility("hidden")))

    This allows us to enforce API boundaries and potentially enables
    compiler optimisations.

    We've been always hiding non-public symbols on Windows. This commit
    brings that to the other platforms.

@xenu, @LeoNerd, can you take a look?

demerphq commented 1 year ago

This is pretty simple:

git grep optimize_optree embed.fnc
embed.fnc:pd    |void   |optimize_optree|NN OP *o

The optimize_optree function is not marked as API. If we want to change that add an A to the flags line and do a regen.

leonerd commented 1 year ago

Yup. I don't think either of these facts surprises me. The only question is should we? Those two functions weren't initially marked as API, but should they be?

demerphq commented 1 year ago

I think we need a flag that says "we grudgingly put this into the API, but if we change it and your code breaks you get keep both pieces". :-)