Add builtin::is_tainted

[JRaspass]

I know we intend to re-revert #19541 at some point this cycle but I still think taint is useful and does have users, so with that in mind I've added builtin::taint.

It seemed like the least controversial of the taint related options to add but I would like to add taint and untaint in the future too, we just need to bikeshed whether we think they ought to handle nested structures or variadic arguments, etc.

If and when #19541 comes back in we will need to make the new tests conditional on taint support. We also should probably decide what the op should do when taint mode is disabled. Either throw or return false, ATM it'll do whatever SvTAINTED does on perls with no taint support, I think return false always.

There might be a simpler way to write the C but I couldn't come up with much, I considered adding a PUSHb macro to push a bool onto the stack but I wasn't sure if there would be enough interest in that, it could simplify other ops like pp_is_bool and pp_is_weak

[leonerd]

I think this all looks good.

As you say, at some point we should bikeshed the idea of whether to also add taint and untaint. It's a bit unfortunate that we're adding these to builtin now while also contemplating adding a configuration option to remove tainting altogether; though even with that in place many people will still have taint-enabled perls for a long time yet; so we might as well add these to builtin all the same.

[JRaspass]

I've simplified the code as per tonycoz's suggestion and applied the same change to other bool builtins (is_bool and is_weak).

So am I right in thinking the reason we don't need/want the target logic is that it's primarily an optimisation to avoid needing to create a new SV in the event that there's already an SV on the left, and since we're returning one of two global bools now we're not building a new SV anyways.

[Leont]

I don't think that test will pass when tainting is disabled.

[tonycoz]

So am I right in thinking the reason we don't need/want the target logic is that it's primarily an optimisation to avoid needing to create a new SV in the event that there's already an SV on the left, and since we're returning one of two global bools now we're not building a new SV anyways.

That's right.

I'll admit I wasn't thinking of the targlex optimization when I made that comment, but I still think it's better to optimize for the more common use in an expression or conditional for these functions rather than the rare direct assignment to a non-LVAL_INTRO lexical.

[JRaspass]

I don't think that test will pass when tainting is disabled.

True, I pushed this change which seems to work for me in my local testing:

-    ok(tainted($0), "\$0 is tainted");
+    is(tainted($0), !!${^TAINT}, "\$0 is tainted (if tainting is supported)");

[xenu]

Adding this function would feel like an endorsement of a very controversial feature. I'm not in favour of this.

[JRaspass]

So what's the plan wrt tainting. I was under the impression that we were going to turn the compiler define into a configure define in the near future (making no taint support the same level of official as no thread support). And then in the future maybe default to it off. I don't see how (or why) we could remove it without needlessly breaking people's code (including my employer's large perl codebase).

Tainting is defence in depth for people who want it, I can understand the appeal of disabling it for more performance, and even making it default to off but to remove it seems unlikely, but that's just my $.02.

Also I don't see how making a faster version of what's already in core via Scalar::Util makes it any more of an endorsement or hurts anything.

[xenu]

So what's the plan wrt tainting.

I don't think there's a long term plan. The only thing that was agreed on was adding a Configure flag to disable it.

My personal opinion is that it should be completely removed, but it's just me.

we could remove it without needlessly breaking people's code (including my employer's large perl codebase).

I'm curious in what way your code relies on the taint mode. What exactly (apart from tests which explicitly check for taintness) would break if taint mode disappeared and -T was a no-op?

Also I don't see how making a faster version of what's already in core via Scalar::Util makes it any more of an endorsement or hurts anything.

If we're extending some feature, in this case by adding a new builtin for it, it sends a message that this feature is fully supported and here to stay.

[Leont]

I can understand the appeal of disabling it for more performance, and even making it default to off but to remove it seems unlikely, but that's just my $.02.

Yeah, that's also my analysis.

[tonycoz]

There's been no discussion of completely removing taint, so I think it's reasonable to apply this.

If we were consistent on naming our boolean test functions I'd suggest renaming to is_tainted, but we haven't been consistent (is_weak, is_bool, blessed).

If you can rebase it and resolve the conflicts I'll apply it.

If we decide at some point to remove tainting this can be modified to always return false, or be removed.

[haarg]

blessed doesn't return a boolean, so it doesn't use an is_ prefix. This should probably be is_tainted.

[JRaspass]

blessed doesn't return a boolean, so it doesn't use an is_ prefix. This should probably be is_tainted.

Fair, I just copied Scalar::Util, I'll rename.

[JRaspass]

Rebased, renamed, and squashed.