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

PPC 0032: Optional Strictures and Warnings #65

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Ovid
Copy link

@Ovid Ovid commented Jan 3, 2025

I've created a PPC that combines the ideas from both the stringification module and the GitHub thread about optional strictures.

The PPC proposes a mechanism for adding optional strictures and warnings, using the stringification functionality as the example of an optional stricture.

@Ovid Ovid changed the title First draft of PPC 0032: Optional Strictures and Warnings PPC 0032: Optional Strictures and Warnings Jan 3, 2025
Here's a trivial example:

```perl
use strict 'all', 'strings';
Copy link

Choose a reason for hiding this comment

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

Note there is no "all" flag for strict currently as there is for warnings, so for this example to work, such a flag would also need to be added. But I would suggest a different name (default, standard?), as this proposal is in fact making it not enable all available strictures.

Copy link
Author

Choose a reason for hiding this comment

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

D'oh! That's what I get for throwing this together too quickly. I've just pushed a new version.


```perl
use warnings 'optional'; # Enable all optional warnings
use warnings 'optional::specific'; # Enable specific optional warning
Copy link

Choose a reason for hiding this comment

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

I'm not sure whether treating "optional" as a category is the best approach for this. Warnings have a classification separate from their category (see perldiag) which dictates among other things in what cases they are enabled. My thought was to add a classification for optional warnings which could then be added to any category that makes sense. The category approach would make them easier to enable all at once, but I'm not sure that's necessary. Difficult to reason about some of this until we know what optional warnings we will want to add.

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure, either. That's why it's listed as open question number 3, with a link to the original discussion.

@leonerd
Copy link
Contributor

leonerd commented Jan 10, 2025

There's something of an overlapping of ideas here. This doc is both proposing a general idea for non-default strictness flags or warnings and additionally proposing details about one specific flag for it. I'm not sure if it would make more sense to discuss those two parts separately?

@Grinnz
Copy link

Grinnz commented Jan 10, 2025

Maybe, but the specific features we wish to accomplish with it really assists in the discussion about the general design

@FractalBoy
Copy link

Hi, hope it's okay to comment. Just wanted to put in my two cents that this stringification stricture makes more sense to me as a warning. Would it be possible at the very least to make it a warning as well as a stricture? The CPAN module gives that option but I don't see it mentioned here.

For example, I may want to turn it on and then put in some debugging print statements. Having it crash in that scenario if I forget to do no strict 'stringification' for that block would be a little annoying.

Also would this also handle other stringy operations such as eq and eval (the CPAN module mentions that substr and regexes are not yet handled)? Or using the ref as a class name to access class methods? Just to name a few that the CPAN module doesn't mention.

@Grinnz
Copy link

Grinnz commented Jan 11, 2025

Or using the ref as a class name to access class methods?

This is already an error for unblessed references, so explicit stringification would be needed via some other method that would already be handled:

> perl -E'[]->foo'
Can't call method "foo" on unblessed reference at -e line 1.

@FractalBoy
Copy link

FractalBoy commented Jan 11, 2025

Oh I meant something like this:

use File::Spec;
my $mod = 'File::Spec';
$mod->catfile(qw(a b));

I guess for a stringified ref it would require explicit stringification, which would already trip the stricture. So nevermind on that one.

my $mod = [];
$mod = "$mod";
$mod->catfile(qw(a b));

Edit: re-reading, that is exactly what you said, my apologies.

@akarelas
Copy link

I've been using $hash{$ref} all my life and have found it very useful. How come I'm the only one? Is it bad practice somehow?

@Grinnz
Copy link

Grinnz commented Jan 13, 2025

It's not necessarily bad practice, but it's usually just a "dirty" substitute for using refaddr, and in either case I believe it's not thread-safe as the ref addresses will change on clone.

@akarelas
Copy link

Wouldn't having to type refaddr inside every hash index (as in $hash{refaddr $ref}) be tiresome and more error-prone? (what if I forget for example to type refaddr once?)

@haarg
Copy link
Contributor

haarg commented Jan 15, 2025

Sometimes correct code is longer than incorrect code. If there were warnings or errors on stringification of refs, forgetting to use refaddr would be immediately noticeable.

@akarelas
Copy link

Indeed. But is it really dirty to use a ref as a hash key? Could you please explain why or give an example?

@akarelas
Copy link

akarelas commented Jan 15, 2025

Ok, i guess I can restrict this practice in certain modules only, which will contain no strict 'strings'; when it eventually gets turned on by default.

@guest20
Copy link

guest20 commented Jan 15, 2025

stringification feels more like a fatal warning than a stricture. It has the vibe of a fatal "hey, you stringified an undef" more than an undeclared variable / sub.

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

Successfully merging this pull request may close these issues.

7 participants