disallow existing type names in universal quantifiers

[vtjnash]

It seems this is tripping up newcomers, without having any benefit for users, so I propose that using an existing type name as a universal quantifier should raise an error.

Thus, given a declatation:

abstract AbstractX

The following should be errors, since they don't mean what they look like they mean:

type X{AbstractX}
end

function y{AbstractX}(x::AbstractX}
end

[pao]

This could hurt composability. Say you defined a type in one file which I happened to use as a typevar in another, I'd error out on loading if I load your module first. This might make sense once there's a story for namespaces (#57).

[StefanKarpinski]

Typevars quantified like this are a lot like local variables or function parameters. I think most languages have deemed it a bad idea for those to not be allowed to shadow global names. I think we should wait on this one and see how it goes.

[vtjnash]

Maybe just a warning then? Overriding globals can be annoying too, since names like max can be used both as a variable and a function. But trying to call a variable instead of the function tends to lead to a error (although it can be confusing without context) and not a silent reinterpretation. But I understand that local scope is essential in many other cases.

In the standard library, it seemed most qualifiers are a single letters, which seems like it would reduce the likelihood of a conflict. However, I guess pao's counterexample would go something like:

suppose I want to declare a function:

function some_function{TAbstractX <: AbstractX}(s::TAbstractX, t::TAbstractX}
end

but someone else then goes and declares a type:

abstract TAbstractX <: AbstractX

but again the first almost looks like someone thought they should repeat the declaration of TAbstractX.

Wouldn't composability also fail whenever two modules try to define the same type / function / global / constant anyways, without namespaces. Is this significantly different? (other than the type qualifier not being in the global scope itself)

Please don't takes this as argumentative, I'm just wondering aloud about ways that might make errors more helpful. Perhaps with time, the larger body of example code will give users a better starting point and make this irrelevant.

[StefanKarpinski]

No, it's a good discussion to have. I just think the right answer won't be apparent until we don't just have one big, flat namespace. Right now, having it be an error or even a warning would really screw up composability. On the other hand, disallowing a typevar from shadowing a top-level variable in the same namespace seems pretty sane to me. As long as stuff is in the same file, I feel like keeping names unique is reasonable; once you start crossing file boundaries, it's a problem.

[JeffBezanson]

This patch provides the warning if you're curious. It triggers a few times running the tests, since T and n are used by tests as global variables.

diff --git a/src/builtins.c b/src/builtins.c
index bdfe6fa..9cc59f7 100644
--- a/src/builtins.c
+++ b/src/builtins.c
@@ -1038,6 +1038,9 @@ JL_CALLABLE(jl_f_typevar)
         JL_NARGS(typevar, 1, 1);
     }
     JL_TYPECHK(typevar, symbol, args[0]);
+    if (jl_boundp(jl_current_module, (jl_sym_t*)args[0])) {
+        ios_printf(ios_stderr, "Warning: type parameter name %s shadows an identifier name\n", ((jl_sym_t*)args[0])->name);
+    }
     jl_value_t *lb = (jl_value_t*)jl_bottom_type;
     jl_value_t *ub = (jl_value_t*)jl_any_type;
     if (nargs > 1) {

I don't think we need this though.

[StefanKarpinski]

Resolved: we apply this patch and issue a warning.

[ViralBShah]

As per IRC discussion, this should issue a warning.