Tcl Source Code

The implementation of [namespace unknown] has a small bug - it works if command is invoked from the frame inside the namespace for which it was set, but it is completely evaded if command is called fully-qualified outside of NS.

Here a diff illustrating proper implementation:

  % namespace eval ::tcl::xxx { 
      proc unknown args {puts "XXX: [uplevel {namespace current}] $args"};
      namespace unknown ::tcl::xxx::unknown
    }
  ::tcl::xxx::unknown
  % namespace inscope ::tcl::xxx { test }
  XXX: ::tcl::xxx test
  % ::tcl::xxx::test
- invalid command name "::tcl::xxx::test"
+ XXX: :: ::tcl::xxx::test
  % namespace inscope ::tcl { xxx::test }
- invalid command name "xxx::test"
+ XXX: ::tcl xxx::test

The issue that specified handler doesn't work everywhere prevents many possible usecases like dynamic NS/ensemble commands or fallbacks, or for instance an implementation of NS-based auto-loading by setting of unknown handler for an NS (instead it forces to call slowly global "::unknown" for known/loaded namespaces with registered unknown handler, which normally should be invoked as first).

Because it is already fixed in my fork, it can be relative fast backported here, just must check against NRE (which is different on my side).

> Fix [merged] to core-8-branch and trunk.

This merge has now been undone, waiting for the TIP vote. The unkown handler for ::tcl::clock has been added as example to the TIP implementation.

TIP written now by Harald (thanks!)

Fix [1f0f766632e637cc|merged] to core-8-branch and trunk.

Not yet to 8.6 (can always be done later, if desired)

Now I know why it did not completely work initially after back-porting - I had a bit different variant of TclGetNamespaceForQualName (got amended a year later)...
Now it is fully back-ported and new tests added (see namespace-52.14), so it'd always work for a not simple command too (::A::B::C::cmd -> ::A::B with unknown, C::cmd is to search, even if ::A::B::C is not there, however also if ::A::B::C exists, but has no unknown handler.

@Jan
> I'm +1 merging this to 8.6 and up, considering it a bug-fix for a corner-case which was not considered before. I don't see any negative impact.

Although I also not believe it'd have any negative impact, I must admit I see one compat thing, so it should at least get a release note (as well as a documentation enhancement), explaining a precedence now - "deepest NS with unknown-handler always wins (regardless from where exactly it was executed)".
For instance in case of 2 nested NS with handlers (::A::B and ::A::B::C), for the call:
namespace eval ::A::B:: { C::cmd }
it'd invoke handler of ::A::B::C, where previously it'd rather invoke ::A::B (since the first was not implemented yet).

There is a small amend [98aab416a2bac6b0] for additional corner case (calling near-fully qualified command foo::bar::xxx from global NS).
For some reason there is a deviation, depending on invocation type (compiled/direct/NRE), how TEOV_NotFound may get the command in objv[0]: as ::foo::bar::xxx or as foo::bar::xxx (so with and without leading colons).
This amend considers that and add still one corner case in test.

To obtain real normalized command name inside the handler one would anyway use something like this (e. g. with auto_qualify like global ::unknown):

    # fully-qualified command name:
    lindex [auto_qualify $cmd [uplevel {namespace current}]] 0
    # fully-qualified command name (without auto_qualify):
    regsub -all {(::){2}} [uplevel {namespace current}]::$cmd {::}
    # relative command name:
    regsub {^(::){1,2}foo::bar(::){1,2}} [uplevel {namespace current}]::$cmd {}

(Suggestion, just as nice to have)
For the last (to obtain normalized relative name) one could extend [namespace tail] like:

    % namespace tail 
    namespace tail string ?relative?
    % namespace tail ::foo::bar::xxx::yyy ::foo::bar
    xxx::yyy

Let's see. 1) The change doesn't break any existing test-case. Good! 2) There's a new testcase demonstrating the corner-case. Good!

Implementation looks good to me (cleanly written, as usual for sebres!). It handles a situation apparently not handled before. That looks like a valid bug-fix to me, which doesn't need a TIP.

The bug-fix is written for 8.6, and can (trivially) be forward-merged.

I'm +1 merging this to 8.6 and up, considering it a bug-fix for a corner-case which was not considered before. I don't see any negative impact.

@Donal, do you agree too?

Do we need a TIP for 9.0?    If it is treated as a bug fix then no, but it must be clearly documented in the release notes.   If anyone objects to it being treated as a bug fix then yes, we need a TIP.  I would assume no to start with.

The fix is in [76660a84029aa1c1].

Regarding Donals comment, I already saw it in core mailing list...
As for "not a bug" and possible RFE, I also knew it'd follow sooner or later (nothing changes here).
But Jan asked about the ticket and the solution, so both are here.

I can only say: a feature that was implemented pretty useless (by design or not) can be for sure considered as a bug, isn't it?

Sure it is compat thing more or less, just if someone used it previously only inside the namespace (otherwise it was not usable), this fix would not affect such people at all.

Donal Fellow commented on the core list:

That’s apparently “not a bug” in that it’s how that feature was designed to work. I’ll merrily admit to being very doubtful that that’s an entirely sane result; it results in [namespace unknown] being one of those features that’s IMO virtually useless. In terms of actual known uses, the only in-core one is TclOO, which uses it to handle definition abbreviations (it was merely the easiest way to customize command resolution in definition scripts, and I recommend that people try to avoid abbreviating definition names).

There seem to be some more uses out in the wild, but I don’t know if they’re making effective use or are actually mostly just (accidentally?) avoiding being in the case which it handles.

Donal.

----

So, it is more an RFE to fix the design in 9.0 -> do we need a TIP ?

Harald