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

There is no way to distinguish a non-existing symbol from one bound to VMnull #735

Open
vrurg opened this issue Aug 25, 2021 · 18 comments
Open

Comments

@vrurg
Copy link
Contributor

vrurg commented Aug 25, 2021

The following code throws in Raku:

use nqp;
my $*INVISIBLE := nqp::null();
say $*INVISIBLE; # Dynamic variable $*INVISIBLE not found

because with nqp::getlexdyn op there is no way to distinguish wether VMnull is returned due to a missing symbol, or the symbol is bound to VMnull. The problem is common across all nqp::getlex* family ops. It means that in the current situation one would need to manually reproduce the internal behavior of a particular nqp::getlex* op to find out what exactly caused the negative outcome.

@vrurg
Copy link
Contributor Author

vrurg commented Aug 25, 2021

Apparently, the preferable fix would be to add nqp::existslex* (or nqp::lex*exists) family of ops. This would result in a little complication of symbol lookup code, produced by the compilers. But I consider it a worthwhile addition.

@niner
Copy link
Contributor

niner commented Aug 25, 2021 via email

@lizmat
Copy link
Contributor

lizmat commented Aug 25, 2021

Also, nqp::null is what is being used extensively for checking whether there's a key in a hash, as a non-existent key returns nqp::null for its value.

So to me, having something be nqp::null just means something is not there.

So this feels like a non-problem to me: nqp::null indicates the absence of a value, just like Nil does in Raku?

@duncand
Copy link

duncand commented Aug 25, 2021

For any collection type that is supposed to be able to have elements of any type at all, it seems a very poor design decision to have a special value to indicate no value, which is logically a contradiction in terms. It sounds like Raku is behaving here analagously to using the Perl code defined $myhash{foo} to see if the key foo exists when Raku should also have the analogy to the Perl operation exists $myhash{foo} so we can distinguish the non-existence of a key from an existing key whose corresponding value is undefined.

@lizmat
Copy link
Contributor

lizmat commented Aug 25, 2021

@duncand Raku has :exists, .EXISTS-KEY and nqp::existskey.

@duncand This is about the very low level implementation in Rakudo. Nothing to do with design. I suggest you familiarize yourself with Raku features before seemingly providing judgment.

@jnthn
Copy link
Contributor

jnthn commented Aug 25, 2021

Even NQP doesn't default undefined variables to nqp::null (they are NQPMu instead), so even standard NQP use won't run into this. One has to bind the nqp::null, and I think that's a case of DIHWIDT. Certainly I'm not inclined to add any "does it exist" variants into MoarVM for this.

@vrurg
Copy link
Contributor Author

vrurg commented Aug 25, 2021

I have tripped over this problem while working on rakudo/rakudo#4495. One of the problems with pseudos which always annoyed me is that .WHO<sym>:exists is not always true even if sym is returned by .WHO.keys. It was caused by inconsistent rules used internally and I think it's done right way now. But then I started writing a test to make sure that all symbols from .keys are visible on .WHO – and tremendously failed on $*DISPATCHER which is iterable as a symbol, but not accessible. It is iterable because pseudos walk over symbol tables manually and use nqp::iterate on each one. It is even accessible on MY because it is PRECISE_SCOPE and can use nqp::existskey directly on pseudo's $!store (which is bound to a lexpad).

But for chained pseudos like OUTERS, or DYNAMIC there is no quick, VM backed, way to make sure a symbol really exists. This will clearly turn into user confusion (with new issue tickets opened) and possible source of future bugs.

There is a workaround which would use manual iteration over contexts to find the requested symbol for EXISTS-KEY, AT-KEY, and BIND-KEY. But this will have performance penalty, apparently. And still would not help with questions like "why I see $*DISPATCHER but Raku throws when I try to read from it?".

So, generally speaking, I agree with @duncand in the point that using a special value to indicate absence of something doesn't look good to me in this particular case.

@lizmat
Copy link
Contributor

lizmat commented Aug 25, 2021

Isn't the DIHWIDT caused by the fact that $*DISPATCHER has been set to nqp::null, rather than a Mu or NQPMu? Wouldn't setting that to e.g. NQPMu fix this?

@jnthn
Copy link
Contributor

jnthn commented Aug 25, 2021

And still would not help with questions like "why I see $*DISPATCHER but Raku throws when I try to read from it?"

$*DISPATCHER is gone in new-disp

@vrurg
Copy link
Contributor Author

vrurg commented Aug 25, 2021

$*DISPATCHER might not be the only symbol of the kind.

Anyway, of course I can drop the idea of testing for roundtrip behavior of pseudos because it would be unreliable. But it would remain a mystery to me why is it a problem to add a set of ops which are basically identical to the existing ones except for the different return value and its meaning?

@lizmat
Copy link
Contributor

lizmat commented Aug 25, 2021

I think the idea of testing is good.

If nqp::isnull($*FOO) is true, then for all practical purposes, that dynamic variable does not exist. What is the problem with that?

@vrurg
Copy link
Contributor Author

vrurg commented Aug 25, 2021

First of all, you can't do $*FOO if it's VMnull. The problem is:

for DYNAMIC::.keys -> $sym {
    ok DYNAMIC::{$sym}:exists, "$sym exists"; # not ok on $*DISPATCHER
}

And, apparently, DYNAMIC::{$sym} would explode because it's a X::NoSuchSymbol failure. Correspondingly, I can't do a clear test if all symbols returned by .keys are actually accessible via the pseudo.

@duncand
Copy link

duncand commented Aug 25, 2021

@duncand Raku has :exists, .EXISTS-KEY and nqp::existskey.

@duncand This is about the very low level implementation in Rakudo. Nothing to do with design. I suggest you familiarize yourself with Raku features before seemingly providing judgment.

@lizmat I'm talking about implementation design. While I'm not an expert or implementer in Raku I have been involved with it to a degree since 2005 and feel I understand enough to give some feedback. In this case I'm going on @vrurg own comments and the leading discussion as a basis for how things seem to work and I'm not second-guessing whether what they said is true. I'm taking there is no way to distinguish wether VMnull is returned due to a missing symbol, or the symbol is bound to VMnull at face value. I was also speaking somewhat abstractly, the "collection type" being the collection of symbols in this case.

However, I apologize that my exact choice of words very poor design decision looks more harsh than it should have been and I could have used different words. Thank you all for your continuing effort to make Raku good.

@lizmat
Copy link
Contributor

lizmat commented Aug 25, 2021

@duncand no worries. It felt that you were talking about HLL issue of being able if something exists or not. For which Raku does have a solution that doesn't depend on a special value (well, at least visible from Raku / NQP. I'm pretty sure Perl hashes actually use a special value to indicate a key has been removed internally, to avoid churn).

@vrurg: maybe DYNAMIC::.keys should not produce $*DISPATCHER if it is nqp::null ?

@vrurg
Copy link
Contributor Author

vrurg commented Aug 25, 2021

maybe DYNAMIC::.keys should not produce $*DISPATCHER if it is nqp::null ?

@lizmat I'm considering this. Not only $*DISPATCHER, but any other. After all, a purpose for the PR is to unify pseudo behaviors with compiler's approach to symbol visibility. Since the compiler considers it invisible – so be it. For anyone wishing to introspect symbol tables deeper than that, nqp::ctx* will always be at their disposal.

@vrurg
Copy link
Contributor Author

vrurg commented Aug 25, 2021

maybe DYNAMIC::.keys should not produce $*DISPATCHER if it is nqp::null ?

@lizmat I'm considering this.

No, it's not an option. Things are totally different about lexicals. These are known at compile time. Therefore the following is working:

use nqp;
my $nullish := nqp::null();
sub foo {
    say $nullish; # (Mu)
}
foo

But for pseudos it is crucial to know their symbols at run time. Therefore, replacing say $nullish with say OUTERS::<$nullish> results in a X::NoSuchSymbol failure.

The argument of nqp::null not being valid in Raku land is not compelling to me because it is neither formally nor technically prohibited. Therefore, binding to VMnull is possible, though perhaps not pretty. In some cases, it might be useful if done with care.

BTW, there is catch in the situation. Because PRECISE_SCOPE pseudos do work with only one lexpad and can use nqp::existskey on it, they do see all local symbols with EXISTS-KEY. But neither they can iterate over VMnull-ones nor they can read them. Apparently, I can replace existskey with atkey and check for null, but ...

Eventually, I'm currently have only the following options if the decision not to add exists* family of ops is final:

  1. Skip all VMnull-bounded symbols and keep pseudos behave differently to the compiler
  2. Skip none of the existing symbol and consider AT-KEY failing on some of them a feature
  3. Implement AT-KEY, and EXISTS-KEY in terms of manual iteration over contexts, effectively duplicating VM functionality, at the cost of lower performance

@niner
Copy link
Contributor

niner commented Aug 26, 2021 via email

@vrurg
Copy link
Contributor Author

vrurg commented Aug 26, 2021

There are lots of ways you can break Rakudo and get completely bogus results by using nqp.

I'm not talking about breaking, but rather about having a faster way for correct symbol introspection. It is currently possible to collect all available symbols by iterating over lexpads manually. But when it comes to finding out if a symbol exists nqp::getlex* does it faster, than NQP/Raku iterations would manage to. Except that nqp::getlex* may have it done incorrectly.

Why should we treat this one case specially?

Perhaps to have it done the right way? We all know that returning a value to signal about absence of an entry is no good if entries can carry that value. The reason I'm do not agree is because "strongly discouraged" doesn't mean "disallowed". I mean, as soon as my $var := nqp::null() is an error – I'd agree that exists is redundant. But not for now. Even though I clearly understand that this is an edge of edges case.

Anyway, I'm giving up and reconsidering my approach. However, I'm still puzzled as to why are you so reluctant to adding these ops? What's the reasoning behind this opposition? Because so far it all sounds just like "it is good as it is" which unconvincing. Would it affect optimizations? Would it bring extra complexity? Would it... what? My curiosity craves for food. :)

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

No branches or pull requests

5 participants