Skip to content

Conversation

@khwilliamson
Copy link
Contributor

@khwilliamson khwilliamson commented Sep 4, 2025

With C99 variadic macros available, we can now call functions that take a printf style format in their short form; perlapi hadn't been updated to know that

It also did not know about the compatibility macros that have long existed for things like croak(). Those now appear in perlapi.

And finally, croak now always expands to croak_nocontext making the latter unnecessary to think about when writing new code. It also has the effect of preferring saving space over time when we are about to die.

  • This set of changes does not require a perldelta entry.

@Leont
Copy link
Contributor

Leont commented Sep 4, 2025

And finally, croak now always expands to croak_nocontext. Prefer saving space over time when are about to die.

Intuitively, I don't like this part of the change. Is that really going to save a meaningful amount of space?

I have no idea how others feel about it though.

@khwilliamson
Copy link
Contributor Author

Its not going to make a meaningful difference in size. And maybe I shouldn't have even mentioned that consequence. What it does do is to make croak_nocontext unnecessary. No new code need use it. It is one less detail that programmers have to know about. It is one less compatibility macro that we have to support. The change doesn't make our code more complicated to maintain. So I think it is worth it.

@khwilliamson
Copy link
Contributor Author

khwilliamson commented Sep 4, 2025

And I changed the pod to indicate that using croak_nocontext is not something to think about in new code.

And I changed the commit message so it argues that the former behavior of croak was misbegotten. It did not save a meaningful amount of time, and created cognitive load by having two different methods for the same task.

The flag was used to specify which functions are of a certain class, and
used to calculate a list to be sorted.  But that list is never going to
grow, so we can just specify it, already sorted.

The reason for doing this is mainly for a future commit which will want
the list in another program, and it's easier to not have to do the
calculations there.
embed.h has compatibility macros for things like croak().  These had
never been output in perlapi, so there was no signature given for these.

And now, PERL_WANT_VARARGS is available to get them to assume that a
thread context is available.  Mention that in each applicable entry.
We now create short name macros that expand to long name functions when
the function takes a printf-style format.  perlapi had not been changed
to reflect that.
Perl almost always opts for saving time over saving space.  Hence, we
have croak() that saves time at the expense of space, but needs thread
context available; and croak_no_context() that doesn't need that, but
takes extra time

But, when we are about to die, time isn't that important.  Even if we
are doing eval after eval in a tight loop, the potential time savings of
passing the thread context to Perl_croak is insignificant compared to
the tear-down that follows.  My claim then is that croak() never needed
a thread context parameter to save a bit of time just before death.  It
is an optimization that isn't worth it.  And having it do so required
the invention of croak_nocontext(), and the extra cognitive load
associated with two methods for the same task.

This commit changes plain croak() to not use a thread context parameter.
It and croak_nocontext() now behave identically.  That means that going
forward, people will likely choose croak() which requires less typing
and occupies fewer columns on the screen, and they won't have to
remember which form to use when.
@khwilliamson
Copy link
Contributor Author

Thanks to @Leont for his comment, which made me realize that I had lost sight of the real reason for the final commit here

@khwilliamson khwilliamson changed the title perlapi: short-form printf-style avail;; croak expands to nocontext perlapi: short-form printf-style avail, croak expands to nocontext Sep 7, 2025
@khwilliamson khwilliamson merged commit 03f24b8 into Perl:blead Sep 12, 2025
33 checks passed
@khwilliamson khwilliamson deleted the croak branch September 12, 2025 21:41
@Leont
Copy link
Contributor

Leont commented Sep 13, 2025

And I changed the pod to indicate that using croak_nocontext is not something to think about in new code.

I don't think anyone outside of core was ever thinking about it; and I don't think most core hackers think of it either. I strongly dislike this change because it's fundamentally inconsistent with the entire rest of the API; it was only created because C89 didn't support variadic macros

@khwilliamson
Copy link
Contributor Author

@Leont, I agree that at least the documentation should be changed. Nothing has changed for people outside core.

But I don't understand your objections. Everyone can continue to use the API exactly as before. But people writing for core don't need to have a thread context, just like outside core. If they already have one, great. If they don't, things will work that didn't before

@khwilliamson
Copy link
Contributor Author

khwilliamson commented Sep 15, 2025

#23706 changed the wording to

When called from outside the perl core, plain C<croak> and C<die> have always been the same as C<croak_nocontext> and C<die_no_context>, respectively. That is now true even when called from within core.

@Leont
Copy link
Contributor

Leont commented Sep 21, 2025

But I don't understand your objections.

And I don't understand why you feel this is desirable. This sort of thing is more subjective than functional and that makes it really difficult to argue about it.

Everyone can continue to use the API exactly as before. But people writing for core don't need to have a thread context, just like outside core.If they already have one, great. If they don't, things will work that didn't before

I don't see why that would be so desirable. I don't think this is a common thing to want; normally the interpreter is available anyway because you need it for other stuff as well.

@khwilliamson
Copy link
Contributor Author

I want it because it makes maintaining things a bit simpler. One never has to concern oneself with if thread context is available or not. It is true that it is usually available, but not always. I have actually run into this problem more than once where I was trying to croak from code that did not have aTHX available. The stumbling block wasn't very big, but nonetheless it did cause stumbles that were really unnecessary. My claim is that croak and die never needed to use aTHX. The speed up that gives wasn't useful for these two functions,, and complicated things. I can see how it could have happened, you have this batch of functions, and you just do the same thing for all of them. But it would have been better if these two had been treated differently

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.

3 participants