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

Apostrophe in name removal #22303

Closed
wants to merge 5 commits into from

Conversation

tonycoz
Copy link
Contributor

@tonycoz tonycoz commented Jun 19, 2024

Do the removal described in perldeprecation

=head3 Use of C<'> as a global name separator

Perl allows use of C<'> instead of C<::> to replace the parts of a
package or global variable name, for example C<A::B> and C<A'B> are
equivalent.

C<'> will no longer be recognized as a name separator in Perl 5.42.

@jkeenan
Copy link
Contributor

jkeenan commented Jun 22, 2024

I scanned the changes and ran the tests on FreeBSD; got PASS. LGTM but the C-level changes would benefit from other eyeballs.

}
#if PERL_VERSION_LT(5, 41, 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if the 0 is still valid? and elsewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We won't know what it should be until it's approved and merged.

@mauke
Copy link
Contributor

mauke commented Jul 27, 2024

  • op.c - Perl_newATTRSUB_x
    Looks like it still treats ' specially:
                   || memchr(name, ':', namlen) || memchr(name, '\'', namlen)
  • op.c - Perl_ck_method
    This code should go:
        /* replace ' with :: */
        while ((compatptr = (char *) memchr(SvPVX(sv), '\'',
                                            SvEND(sv) - SvPVX(sv) )))
        {
            *compatptr = ':';
            sv_insert(sv, compatptr - SvPVX_const(sv), 0, ":", 1);
        }
  • mg.c - Perl_magic_setsig
    Another suspicious use of ':
                if (!memchr(s, ':', len) && !memchr(s, '\'', len))

@tonycoz tonycoz force-pushed the apostrophe-in-name-removal branch from 1026a56 to 68bcecd Compare July 29, 2024 04:26
@tonycoz
Copy link
Contributor Author

tonycoz commented Jul 29, 2024

Thanks.

  • op.c - Perl_newATTRSUB_x
    Looks like it still treats ' specially:
    c || memchr(name, ':', namlen) || memchr(name, '\'', namlen)

    • op.c - Perl_ck_method
      This code should go:
          /* replace ' with :: */
          while ((compatptr = (char *) memchr(SvPVX(sv), '\'',
                                              SvEND(sv) - SvPVX(sv) )))
          {
              *compatptr = ':';
              sv_insert(sv, compatptr - SvPVX_const(sv), 0, ":", 1);
          }

I couldn't see a way to test these, but removed the left over code.

* `mg.c` - `Perl_magic_setsig`
  Another suspicious use of `'`:
  ```c
              if (!memchr(s, ':', len) && !memchr(s, '\'', len))
  ```

This one had observable behaviour which I added a test for.

@guest20
Copy link

guest20 commented Jul 29, 2024

I assume this PR calls for an update to perldata, under the heading Identifier parsing :

While you can mix double colons with singles quotes, the quotes must come after the colons: $::::'foo and $foo::'bar are legal, but $::'::foo and $foo'::bar are not.

There's also a big old regex in that section with rules for parsing identifiers.

@mauke
Copy link
Contributor

mauke commented Jul 29, 2024

pod/perlmod.pod has this:

The old package delimiter was a single quote, but double colon is now the preferred delimiter, in part because it's more readable to humans, and in part because it's more readable to emacs macros. It also makes C++ programmers feel like they know what's going on--as opposed to using the single quote as separator, which was there to make Ada programmers feel like they knew what was going on. Because the old-fashioned syntax is still supported for backwards compatibility, if you try to use a string like "This is $owner's house", you'll be accessing $owner::s; that is, the $s variable in package owner, which is probably not what you meant. Use braces to disambiguate, as in "This is ${owner}'s house".

Using ' as a package separator is deprecated and will be removed in Perl 5.40.

@jkeenan
Copy link
Contributor

jkeenan commented Aug 6, 2024

@tonycoz, could you respond to the 2 preceding comments in this ticket? Thanks.

@tonycoz
Copy link
Contributor Author

tonycoz commented Aug 7, 2024

I've added updates to perldata and perlmod, I expect they will be squashed into the top commit on merge.

pod/perldata.pod Outdated Show resolved Hide resolved
In general for tests I translate them to using :: if the test wasn't
specifically for ', and the test didn't duplicate a similar test
that did test ::.

This doesn't just change the parsing stage from accepting ' instead of
:: in names, but also removes the translation from ' to :: done in
several places, but that's really there to support the syntax.
@tonycoz
Copy link
Contributor Author

tonycoz commented Aug 12, 2024

applied manually, thanks everyone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants