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

Add module syntax #17966

Closed
wants to merge 7 commits into from
Closed

Add module syntax #17966

wants to merge 7 commits into from

Conversation

leonerd
Copy link
Contributor

@leonerd leonerd commented Jul 13, 2020

WORK-IN-PROGRESS. Do not merge yet. I will edit this description as I go.

Adds new syntax:

module Foo;

equivalent to, and intended as a neat shortcut for

package Foo;
use strict;
use warnings;
use feature qw(bitwise current_sub declared_refs evalbytes fc
    isa postderef_qq refaliasing say state unicode_eval unicode_strings);

(Notably, all features currently defined apart from indirect and switch which we aim to remove, and signatures because it is still experimental and its presence is contentious because it would lead to a lot of code breakage due to its collision with pre-5.20 prototypes)

The fact that this new keyword is not feature-guarded is intentional. At present I do not intend to apply this change to a 5.x version of Perl; only a 7. In fact it could be the defining feature of a Perl 7, the way to enable the new default syntax. Its merits and downsides remain to be debated by Perl-Core. It was however designed and generally agreed at the "P5H Core Hackathon" event in Amsterdam in October, 2019; so I am hoping its addition should be relatively non-contentious, details about exact enabled features aside.

@leonerd leonerd added the do not merge Don't merge this PR, at least for now label Jul 13, 2020
@leonerd leonerd marked this pull request as draft July 13, 2020 21:57
@leonerd leonerd removed the do not merge Don't merge this PR, at least for now label Jul 13, 2020
@hvds
Copy link
Contributor

hvds commented Jul 14, 2020

Notably, all features currently defined apart from indirect and switch which we aim to remove, and signatures because it is still experimental and its presence is contentious because it would lead to a lot of code breakage due to its collision with pre-5.20 prototypes

I think the value of this new keyword would be much reduced if it doesn't bring signatures with it, so I think it's worth reconsidering that decision.

Can you give an example of how signatures would cause "collision with pre-5.20 prototypes" within the scope of this new keyword? I'd have expected that it would cause no more collision than choosing to turn on signatures for this (presumably new) code would, so you'd simply write this code with new-style prototypes if you need them.

I'm more worried about the 'bitwise' feature, but that's probably just because I've written too much code using the existing semantics and it'll take me time to revise my expectations - it'll be a pain and a waste of time the first couple of times I have to debug that, but eventually I'll get used to the new semantics.

@atoomic atoomic added the do not merge Don't merge this PR, at least for now label Jul 15, 2020
@toddr toddr added Feature A New Feature. do not merge Don't merge this PR, at least for now and removed do not merge Don't merge this PR, at least for now labels Jul 30, 2020
@jkeenan
Copy link
Contributor

jkeenan commented Jan 26, 2021

@leonerd, this p.r. has had a hasConflicts label on it since Aug 6. Just now, I tried to rebase it on blead and got merge conflicts in these files:

CONFLICT (content): Merge conflict in perly.y
Auto-merging perly.tab
CONFLICT (content): Merge conflict in perly.tab
Auto-merging perly.h
CONFLICT (content): Merge conflict in perly.h
Auto-merging perly.act
CONFLICT (content): Merge conflict in perly.act
Auto-merging lib/B/Deparse-core.t
Auto-merging keywords.h
CONFLICT (content): Merge conflict in keywords.h
Auto-merging keywords.c
CONFLICT (content): Merge conflict in keywords.c

Handling merge conflicts in gutsy files like these is above my pay grade. Do you expect to be pursuing this work further in the near- to medium-term? If not, I recommend closing the p.r. (though perhaps retaining the branch in the repo).

Thank you very much.
Jim Keenan

@leonerd leonerd closed this May 8, 2021
@leonerd leonerd deleted the leonerd/module branch May 8, 2021 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Don't merge this PR, at least for now Feature A New Feature. hasConflicts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants