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 known bugs in class feature to perlclass.pod #22247

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

johannessen
Copy link
Contributor

When the experimental class feature was added in v5.38, it had some known problems that could cause segmentation faults, including #20956, #20947, and #20890 (duplicate: #21221).

As these issues persist to this day, I think they should be mentioned somewhere in Perl’s documentation. This change adds them to perl5380delta, and includes a brief erratum in the new v5.40 perldelta.

@Leont
Copy link
Contributor

Leont commented May 25, 2024

I'm not sure perldelta is really the right place for this.

@rwp0
Copy link
Contributor

rwp0 commented May 26, 2024

Good to know about those specific cases.

Since there's no perlbugs, I'd go to perldelta by default to check the status for v5.38 updates, as a user.

@@ -2175,6 +2175,30 @@ space is actually allocated. C<SSCHECK()> is now an alias for C<SSGROW()>.

=back

=head1 Known Problems
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know why but "Known Issues" sounds more familiar 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general, I’d tend to agree, but in perldelta, “known problems” is the usual terminology:

https://github.com/Perl/perl5/blob/blead/Porting/perldelta_template.pod#known-problems

Copy link
Contributor

Choose a reason for hiding this comment

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

=head1 KNOWN BUGS would be the traditional heading. level-1 headings are all-caps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I think =heads1 BUGS is the traditional heading in Perl reference documents other than perldelta, see e. g. perlre or perlebcdic. But I’m happy with “known bugs” if you prefer it.

pod/perldelta.pod Outdated Show resolved Hide resolved
pod/perldelta.pod Outdated Show resolved Hide resolved
@johannessen
Copy link
Contributor Author

The description of the ”Known Problems” section in perldelta_template.pod is as follows:

XXX Descriptions of platform agnostic bugs we know we can't fix go here. Any tests that had to be TODOed for the release would be noted here. Unfixed platform specific bugs also go here.

There is certainly room to split some hairs about what exactly this language means. But I think it’s not meant to be exclusive.

The way I see it, the intention is that any unfixed platform-agnostic bugs which simply happen to not have TODO tests yet should indeed be listed as “known problems,” too.

@Leont
Copy link
Contributor

Leont commented May 26, 2024

We don't usually update perldelta like that (except minor edits like typos and broken links). Perhaps perlexperimental is a more logical place.

@johannessen
Copy link
Contributor Author

I see your point, but a “KNOWN PROBLEMS” section in perlexperiment might imply to readers that all known issues for experiments are listed. I’m not in a position to produce such a comprehensive list.

I suppose adding a “BUGS” section to perlclass might work, plus a pointer in the new 5.40 perldelta for “updated documentation.” Would that be more appropriate?

@Leont
Copy link
Contributor

Leont commented May 26, 2024

@leonerd probably some ideas about where the best place would be

@leonerd
Copy link
Contributor

leonerd commented May 30, 2024

I imagine a "KNOWN BUGS" section in perlclass.pod would be handy.

Copy link
Contributor

@leonerd leonerd left a comment

Choose a reason for hiding this comment

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

I imagine putting a KNOWN BUGS section into perlclass.pod would be better than perldelta. perldelta is where people look for changes between versions. These bugs aren't new here, they've always been around. Listing them as known alongside the place where the feature itself is documented would be the best place.

@johannessen johannessen changed the title Add known problems with class feature to perldelta Add known bugs in class feature to perlclass.pod May 30, 2024
@johannessen
Copy link
Contributor Author

Thank you all for your guidance! PR updated.

I’m not sure how much detail I should go into when describing the issues in perlclass.pod. I’ve opted for just a brief summary plus a GH link. Is that appropriate here?

Copy link
Contributor

@khwilliamson khwilliamson left a comment

Choose a reason for hiding this comment

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

I don't know anything about the accuracy of the wording, but stylistically, it LGTM. A brief mention and a GH link are appropriate, I think.

pod/perlclass.pod Show resolved Hide resolved
@khwilliamson khwilliamson dismissed leonerd’s stale review May 30, 2024 22:24

requested changes were made


In Perl v5.38, inheriting from a class would not always attempt to load the
parent class (fixed in Perl v5.40).
[L<GH #21332|https://github.com/Perl/perl5/issues/21332>]
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is fixed, why are we listing it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It’s been frustrating trying to write code that uses the ongoing class experiment because bugs like this prevent some code from working properly on certain versions of perl. So this information can help to make the evaluation of the experiment a bit easier.

Another reason would be completeness: We forgot to mention several of these issues in the v5.38 release entirely. Now that the list of known bugs is added, it might not hurt to mention the fixed one as well.

There also seems to be precedent for this in the one of the documents I mentioned earlier, perlebcdic. Seeing that is, in fact, what gave me the idea.

@haarg haarg merged commit 4565b04 into Perl:blead Jun 4, 2024
29 checks passed
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.

None yet

6 participants