Merge pull request #10951 from JuliaLang/jb/usingclashes

RFC: warn and don't import when two `usings` provide the same name

src/julia.h

@@ -1079,6 +1079,7 @@ DLLEXPORT void jl_module_use(jl_module_t *to, jl_module_t *from, jl_sym_t *s);
 DLLEXPORT void jl_module_import(jl_module_t *to, jl_module_t *from, jl_sym_t *s);
 DLLEXPORT void jl_module_importall(jl_module_t *to, jl_module_t *from);
 DLLEXPORT void jl_module_export(jl_module_t *from, jl_sym_t *s);
+DLLEXPORT int jl_is_imported(jl_module_t *m, jl_sym_t *s);
 DLLEXPORT jl_module_t *jl_new_main_module(void);
 DLLEXPORT void jl_add_standard_imports(jl_module_t *m);
 STATIC_INLINE jl_function_t *jl_get_function(jl_module_t *m, const char *name)

src/module.c

@@ -152,21 +152,35 @@ static jl_binding_t *jl_get_binding_(jl_module_t *m, jl_sym_t *var, modstack_t *
     }
     jl_binding_t *b = (jl_binding_t*)ptrhash_get(&m->bindings, var);
     if (b == HT_NOTFOUND || b->owner == NULL) {
+        jl_module_t *owner = NULL;
         for(int i=(int)m->usings.len-1; i >= 0; --i) {
             jl_module_t *imp = (jl_module_t*)m->usings.items[i];
-            b = (jl_binding_t*)ptrhash_get(&imp->bindings, var);
-            if (b != HT_NOTFOUND && b->exportp) {
-                b = jl_get_binding_(imp, var, &top);
-                if (b == NULL || b->owner == NULL)
+            jl_binding_t *tempb = (jl_binding_t*)ptrhash_get(&imp->bindings, var);
+            if (tempb != HT_NOTFOUND && tempb->exportp) {
+                tempb = jl_get_binding_(imp, var, &top);
+                if (tempb == NULL || tempb->owner == NULL)
                     // couldn't resolve; try next using (see issue #6105)
                     continue;
-                // do a full import to prevent the result of this lookup
-                // from changing, for example if this var is assigned to
-                // later.
-                module_import_(m, b->owner, var, 0);
-                return b;
+                if (owner != NULL && tempb->owner != b->owner &&
+                    !(tempb->constp && tempb->value && b->constp && b->value == tempb->value)) {
+                    jl_printf(JL_STDERR,
+                              "Warning: both %s and %s export %s; uses of it in module %s must be qualified\n",
+                              owner->name->name, imp->name->name, var->name, m->name->name);
+                    // mark this binding resolved, to avoid repeating the warning
+                    (void)jl_get_binding_wr(m, var);
+                    return NULL;
+                }
+                owner = imp;
+                b = tempb;
             }
         }
+        if (owner != NULL) {
+            // do a full import to prevent the result of this lookup
+            // from changing, for example if this var is assigned to
+            // later.
+            module_import_(m, b->owner, var, 0);
+            return b;
+        }
         return NULL;
     }
     if (b->owner != m)
@@ -187,6 +201,14 @@ static int eq_bindings(jl_binding_t *a, jl_binding_t *b)
     return 0;
 }
 
+// does module m explicitly import s?
+int jl_is_imported(jl_module_t *m, jl_sym_t *s)
+{
+    jl_binding_t **bp = (jl_binding_t**)ptrhash_bp(&m->bindings, s);
+    jl_binding_t *bto = *bp;
+    return (bto != HT_NOTFOUND && bto->imported);
+}
+
 // NOTE: we use explici since explicit is a C++ keyword
 static void module_import_(jl_module_t *to, jl_module_t *from, jl_sym_t *s,
                            int explici)

src/toplevel.c

@@ -314,11 +314,12 @@ static jl_module_t *eval_import_path_(jl_array_t *args, int retrying)
         if (jl_binding_resolved_p(m, var)) {
             jl_binding_t *mb = jl_get_binding(m, var);
             jl_module_t *m0 = m;
+            int isimp = jl_is_imported(m, var);
             assert(mb != NULL);
-            if (mb->owner == m0 || mb->imported) {
+            if (mb->owner == m0 || isimp) {
                 m = (jl_module_t*)mb->value;
                 if ((mb->owner == m0 && m != NULL && !jl_is_module(m)) ||
-                    (mb->imported && (m == NULL || !jl_is_module(m))))
+                    (isimp && (m == NULL || !jl_is_module(m))))
                     jl_errorf("invalid module path (%s does not name a module)", var->name);
                 // If the binding has been resolved but is (1) undefined, and (2) owned
                 // by the module we're importing into, then allow the import into the