Re: WITH_THR and WITH_THX

On Wed, Jun 16, 2010 at 07:13:41PM +0100, Dave Mitchell wrote:
> On Wed, Jun 16, 2010 at 05:43:35PM +0100, Nicholas Clark wrote:
> > There are currently exactly two uses of WITH_THR/WITH_THX in core.
> > 
> > #define WITH_THX(s) STMT_START { dTHX; s; } STMT_END
> > #define WITH_THR(s) WITH_THX(s)
> [snip]
> 
> > Is there a reason why they're actually needed? I can't work it out for sure,
> > and I didn't really want to remove something on the basis of "works on my
> > machine" without understanding it.
> 
> At a guess, when I created the DEBUG_CX maco, I just cargo-culted it from
> the existing DEBUG_SCOPE macro. I certainly don't recall any specific
> reason to need WITH_THR/WITH_THX, so I'd say go ahead and remove them.

Aha. This is layering*, as applied to code**, as the DEBUG_SCOPE macro was
added by Dave Mitchell (2003 Ltd)(no connection with the current firm, honest)

http://perl5.git.perl.org/perl.git/commit/b4ab917c3d812d8e

At that time there were 5 existing uses of WITH_THR. However, all were removed
in 2006 by Dave Mitchell (2006 Ltd):

http://perl5.git.perl.org/perl.git/commitdiff/11206fddaf7ef068

I believe that all 5 (plus a 6th) were added by Malcom Beattie in 1997:

http://perl5.git.perl.org/perl.git/commit/0f15f207c55ce70f

The current definition of WITH_THR as WITH_THX comes from 2000

http://perl5.git.perl.org/perl.git/commit/411caa507cab4ba3

    dTHR is a nop in 5.6.0 onwards.  Ergo, it can go.

However, I think that that diff is wrong. Instead of

 #define WITH_THX(s) STMT_START { dTHX; s; } STMT_END
-#define WITH_THR(s) STMT_START { dTHR; s; } STMT_END
+#define WITH_THR(s) WITH_THX(s)

It should have been

+#define WITH_THR(s) STMT_START { s; } STMT_END

as dTHR had effectively become a no-op earlier in 2000 with

http://perl5.git.perl.org/perl.git/commit/ba869debd80c55cf

    support fetching current interpreter from TLS under useithreads


and, I think I've done enough digging when I report that even at that commit's
parent, neutering WITH_THR like this:

diff --git a/perl.h b/perl.h
index 7d42b0f..0c8aec8 100644
--- a/perl.h
+++ b/perl.h
@@ -297,7 +297,7 @@ register struct op *Perl_op asm(stringify(OP_IN_REGISTER));
 #endif
 
 #define WITH_THX(s) STMT_START { dTHX; s; } STMT_END
-#define WITH_THR(s) STMT_START { dTHR; s; } STMT_END
+#define WITH_THR(s) STMT_START { s; } STMT_END
 
 /*
  * SOFT_CAST can be used for args to prototyped functions to retain some

doesn't break the compile. Suggesting that it was already cargo-cult, or had
become redundant.

The second use of WITH_THR in mg.c was removed in 1998 by Nick Ing-Simmons in
http://perl5.git.perl.org/perl.git/commit/c485e6072d15e92c

   Add dTHR so that it compiles miniperl in threaded mode

which makes me wonder if the other WITH_THR had become redundant due to later
additions of dTHR in their functions.


Conclusion - it can go. It should go. In fact, I think that both WITH_THX and
WITH_THR are redundant and counter-productive. If you want dTHX, type dTHX.

Nicholas Clark

*  Wikipedia (presently, apparently) fails to define this term in the page on
   money laundering, despite referencing "layering" once near the end of the
   page. It's the step of moving the money between accounts, multiple times,
   to cover its history.
** We were joking about it at work, in terms of bugs. Transfer the bug multiple
   times between developers, then close it. Thus laundering the bug.