Ovid/assignment context

[Ovid]

RFC for adding "assignment context" attributes to safely replace wantarray heuristics.

[iabyn]

I still don't understand this proposal at at all. I don't see what problems it solves. The example shows the function used in boolean context, and this is understood to be a Bad Thing, but without explaining why it's actually bad.

I think the whole idea of 'assignment context' is murky and will have endless edge cases. Does unshift @a, foo() count as an assignment? Etc.

[tobyink]

Honestly, I think a better solution, even though it will only kick in at run-time (while Ovid's could theoretically be checked at compile time) is allowing destructors to throw real exceptions which don't get downgraded to warnings.

How does this help Ovid's use case? Say munginator would normally return a string. Instead, it could return an object blessed into this class:

package Local::Munginator::Result {
  use overload
    q[""]    => 'to_string',
    q[bool]  => 'to_bool',
    fallback => 1;
  sub new ( $class, $string ) {
    bless [ 0, $string ], $class;
  }
  sub to_string ( $self, @args ) {
    # @args is a dummy because overloaded methods will always be
    # called with three parameters!
    $self->[0]++;
    return $self->[1];
  }
  sub to_bool ( $self, @args ) {
    return builtin::true;
  }
  sub DESTROY ( $self ) {
    die "Munginator result was ignored!" unless $self->[0];
  }
}

This allows munginator to not just check that its result hasn't been immediately discarded, but enforce that the caller has performed certain required operations on it. (In this case, force that it's been stringified at least once.)

[tonycoz]

I still don't understand this proposal at at all. I don't see what problems it solves. The example shows the function used in boolean context, and this is understood to be a Bad Thing, but without explaining why it's actually bad.

One obvious case is DBI::connect():

if (DBI->connect(...)) {
    # well, I connected, now what?
}

For me Imager is related too:

# scale warns in void context to try to detect calls in void context,
# which people did.  That doesn't help here
if ($img->scale(...)) {
    # discarded the scaled image, oops
}

I think the whole idea of 'assignment context' is murky and will have endless edge cases. Does unshift @a, foo() count as an assignment? Etc.

There seems to be two different concepts discussed here (which @Grinnz discusses above), the boolean check covers one side of it, forcing scalar/list/void context on return seems to me to be only indirectly connected.

Using :scalar to disallow boolean context seems incorrect to me - boolean context is scalar context.

I'd expect anything that actually saves the returned value to be accepted, including use as a function argument, based on the discussion above.

Also, while the above discusses boolean context as a problem - use in numeric or string context could be similar problem if the function returns a non-overloaded reference.

[MartijnLievaart]

Op 12-01-2023 om 04:58 schreef Tony Cook:

I still don't understand this proposal at at all. I don't see what
problems it solves. The example shows the function used in boolean
context, and this is understood to be a Bad Thing, but without
explaining why it's actually bad.

One obvious case is DBI::connect():

|if (DBI->connect(...)) { # well, I connected, now what? } |

For me Imager is related too:

|# scale warns in void context to try to detect calls in void context,

which people did. That doesn't help here if ($img->scale(...)) {

discarded the scaled image, oops } |

I think the whole idea of 'assignment context' is murky and will
have endless edge cases. Does unshift @A <https://github.com/A>,
foo() count as an assignment? Etc.

There seems to be two different concepts discussed here (which @Grinnz https://github.com/Grinnz discusses above), the boolean check covers one side of it, forcing scalar/list/void context on return seems to me to be only indirectly connected.

Using |:scalar| to disallow boolean context seems incorrect to me - boolean context is scalar context.

I'd expect anything that actually saves the returned value to be accepted, including use as a function argument, based on the discussion above.

So what is actually wanted is something like :nodiscard?

HTH,

M4

[tonycoz]

So what is actually wanted is something like :nodiscard?

That covers completely ignoring the result (void context), but not boolean/numeric/string context that might discard the useful part of a reference.

Your mention did get me to look at the C++ nodiscard again, which reminded me that can optionally take a message, so you can explain why void/boolean/numeric/string context is bad.

sub DBI::connect :noboolean("discarded returned database handle") {
  ...
}

Maybe ":nodowngrade" to cover all the cases, though downgrade is overloaded by strings, at least for me.