Generics "sandwiched" between two modules don't mixin their scope symbols properly

[zah]

This problem is a bit tricky to understand, so please read carefully.

Let's imagine we have a library featuring generic functions that mixes in some symbols from the caller scope:

generic_library.nim

proc libraryFunc*[T](x: T) =
  mixin mixedIn, indirectlyMixedIn
  echo mixedIn()
  echo indirectlyMixedIn()

This library is used from another module that provides the needed symbols. mixedIn is defined locally, while indirectlyMixedIn is imported from yet another module:

module_using_generic_library.nim

import
  generic_library, helper_module

proc mixedIn: int = 100

proc makeUseOfLibrary*[T](x: T) =
  libraryFunc(x)

when isMainModule:
  makeUseOfLibrary "test"

helper_module.nim

proc indirectlyMixedIn*: int =
  200

So far, so good. If we compile module_using_generic_library, it will produce the expected output.

But let's introduce another module that imports and uses module_using_generic_library:

main.nim

import
  module_using_generic_library

makeUseOfLibrary "test"

If we try to compile it, we'll get the following error:

module_using_generic_library.nim(4, 6) Hint: 'mixedIn' is declared but not used [XDeclaredButNotUsed]
main.nim(4, 18) template/generic instantiation of `makeUseOfLibrary` from here
module_using_generic_library.nim(7, 14) template/generic instantiation of `libraryFunc` from here
generic_library.nim(3, 8) Error: undeclared identifier: 'mixedIn'

Why did this happen?

Please notice that the makeUseOfLibrary proc in the module_using_generic_library module is also generic. It is instantiated in main, but ultimately it consumes another generic defined in generic_library. Its scope is "sandwiched" between the main scope and the inner-most generic_library scope. At the moment mixin fails to recognise this and attempts to look for symbols in the outer-most scope (the one of main). This is highly surprising behavior for the user, because if you examine the lexical scopes, libraryFunc seems instantiated properly in the sandwiched module.

We've hit this problem multiple times during the development of nimcrypto, chronicles and nim-serialization. I've been asked to explain it on multiple occasions by almost everyone on our team, so it's an ongoing source of confusion and wasted time even for Nim experts. For this reason, I'm assigning high priority to the issue.

[Araq]

I'm not sure I'm buying that there is a bug here. When you write

import
  generic_library, helper_module

proc mixedIn: int = 100

proc makeUseOfLibrary*[T](x: T) =
  libraryFunc(x)

you're clearly aware that mixedIn is used for libraryFunc, so the "obvious" solution is to either export it here or to bind/mixin it for the libraryFunc inside makeUseOfLibrary. This doesn't "break the abstraction" or anything like that because there isn't any, makeUseOfLibrary does know about libraryFunc's requirements.

Notice how the casual reader can easily believe that proc mixedIn: int = 100 is dead code since it's not used anywhere and it lacks the export marker.

Or to put it differently: The mixin statement within a generic is in reality a requirement that should be captured by the parameter list and so it needs to be "repeated" / passed around in makeUseOfLibrary.

[Araq]

Thinking more about this issue ... your solution would imply that in a module that has a generic proc somewhere everything that is declared before the generic proc could be subject to some inferred mixin declaration and so the compiler would never even have a chance of telling you "deadProc is never used"! That's a worse design than the existing solution, all things considered.

Never mind, there is a way to do this and keeping the "deadProc is never used" analysis.

[zah]

The real-world manifestations of this issue are much more WTF-ivoking than this simplified test case. I can point to more examples, but the two linked issues here are a good start.

To clarify the expected behavior, I think the compiler should determine the lexical scope at the position where libraryFunc is instantiated and symbols should be mixed in from that scope. The author of the sandwiched module is still free to extend the symbol search to another module by repeating the mixin statement. As I mentioned, the current behavior is surprising enough in practice that I've been summoned to help with related compiler errors by almost everyone on our team.

[Araq]

As I mentioned, the current behavior is surprising enough in practice that I've been summoned to help with related compiler errors by almost everyone on our team.

Yes, I read it and I do believe you. But it's a tradeoff here, just imagine the compiler could never reliably warn about unused symbols in e.g. asyncdispatch just because it contains a generic proc. And even if the compiler itself can figure it out (as I've said, I think I can make it work), it's totally suprising behaviour for every casual reader of the code.

[Araq]

How about this solution:

# declare module-wise that these are mixins and also export them
mixin mixedIn*, indirectlyMixedIn*


proc libraryFunc*[T](x: T) =
  echo indirectlyMixedIn()

It would be a language change, but #7385 also indicates something like that is required.

Of course, in today's Nim this can be written as:

proc mixedIn*() = discard "overload one"
proc mixedIn*(x: typeof(nil)) = discard "overload two to make it a mixin"

[zah]

I think top level mixins are necessary in general - both because it's typical to use the same mixins in multiple procs and also because at the moment there is no way to refer to mixed in symbols in the right-hand side of a type section:

type
  Foo[T] = object
    hashValue: type(hash(default(T))

I've encountered the need to use something like this on few occasions.

But the other part of the proposal makes me feel uneasy. My suggestion tries to honour the standard behavior of lexical scopes. The forwarding approach seems less sound due to the following reasons:

1. The mixed in symbols I'm exporting are unnecessary pollution.

2. The feature is still tricky to use - I must remember to export the right symbols, even when the additional mixin definitions are not necessary when compiling usages of the sandwiched generic inside the sandwiched module. I think focusing on this issue reveals the problem. How come it's reasonable for the same code to compile in one occasion, but to fail in another? In practice, I think even with this extra feature, I'll see the developers being tripped by the problem frequently.

[Araq]

The mixed in symbols I'm exporting are unnecessary pollution.

Well mixin itself is a bandaid, ideally you would write a concept for the generic code that covers the required procs and then symbol lookup within generic code can be precise.

The feature is still tricky to use.

Well generics with their two different interacting scopes are simply harder to write that non-generic code. However, your point is valid.

proc mixedIn: int = 100

proc makeUseOfLibrary*[T](x: T) =
  libraryFunc(x)

This is what it comes down to and maybe it shouldn't compile at all, regardless of the context? What is really done here is a form of implicit parameter passing libraryFunc(x, mixedIn) and we need to be able to write it down more explicitly IMO.

[zah]

As far as I understand, your concern about the proposed behavior is that it will interfere with the ability of the user or the compiler to reason about unused symbols.

I think the analysis for the compiler is certainly possible. Every time a symbol gets mixed in at a call-site, this counts as an usage. Symbols with zero usages are still detected normally.

The problem for the user is perhaps a bit overstated. Why would anyone bother to manually hunt for unused symbols if the compiler provides accurate hints for them? If I am confused and I want to know where a particular symbol is used, I can still invoke the "find references" operation in my IDE and nimsuggest can output the call-sites where the symbol was mixed in as well.