Remove visiblity and lookup deprecation - Take 2

changelog/import_deprecations.dd

@@ -0,0 +1,5 @@
+The deprecation phase for visiblity and lookup changes is finished
+
+The `-transition=import` and `-transition=checkimports` switches no longer have an
+effect and are now deprecated. Symbols that are not visible in a particular
+scope will no longer be found by the compiler.

docs/gen_man.d

@@ -127,6 +127,9 @@ void main()
 Language changes listed by \fB-transition=id\fR:`);
     foreach (transition; Usage.transitions)
     {
+        if (transition.deprecated_)
+            continue;
+
         string additionalOptions;
         if (transition.bugzillaNumber)
             additionalOptions = "," ~ transition.bugzillaNumber;

src/dmd/cli.d

@@ -607,22 +607,25 @@ dmd -cov -unittest myprog.d
     /// Representation of a CLI transition
     struct Transition
     {
+        enum Deprecated { no, yes}
+
         string bugzillaNumber; /// bugzilla issue number (if existent)
         string name; /// name of the transition
-        string paramName; // internal transition parameter name
-        string helpText; // detailed description of the transition
+        string paramName; /// internal transition parameter name
+        string helpText; /// detailed description of the transition
+        bool deprecated_; /// whether the flag is still in use
     }
 
     /// Returns all available transitions
     static immutable transitions = [
         Transition("3449", "field", "vfield",
             "list all non-mutable fields which occupy an object instance"),
         Transition("10378", "import", "bug10378",
-            "revert to single phase name lookup"),
+            "revert to single phase name lookup", Transition.Deprecated.yes),              // @@@DEPRECATED_2019-12@@@.
         Transition("14246", "dtorfields", "dtorFields",
             "destruct fields of partially constructed objects"),
         Transition(null, "checkimports", "check10378",
-            "give deprecation messages about 10378 anomalies"),
+            "give deprecation messages about 10378 anomalies", Transition.Deprecated.yes), // @@@DEPRECATED_2019-12@@@.
         Transition("14488", "complex", "vcomplex",
             "give deprecation messages about all usages of complex or imaginary types"),
         Transition("16997", "intpromote", "fix16997",
@@ -705,6 +708,9 @@ CPU architectures supported by -mcpu=id:
                 "list information on all language changes")] ~ Usage.transitions;
             foreach (t; allTransitions)
             {
+                if (t.deprecated_)
+                    continue;
+
                 buf ~= "  =";
                 buf ~= t.name;
                 auto lineLength = 3 + t.name.length;

src/dmd/dscope.d

@@ -479,19 +479,6 @@ struct Scope
         if (this.flags & SCOPE.ignoresymbolvisibility)
             flags |= IgnoreSymbolVisibility;
 
-        Dsymbol sold = void;
-        if (global.params.bug10378 || global.params.check10378)
-        {
-            sold = searchScopes(flags | IgnoreSymbolVisibility);
-            if (!global.params.check10378)
-                return sold;
-
-            if (ident == Id.dollar) // https://issues.dlang.org/show_bug.cgi?id=15825
-                return sold;
-
-            // Search both ways
-        }
-
         // First look in local scopes
         Dsymbol s = searchScopes(flags | SearchLocalsOnly);
         version (LOGSEARCH) if (s) printMsg("-Scope.search() found local", s);
@@ -500,68 +487,10 @@ struct Scope
             // Second look in imported modules
             s = searchScopes(flags | SearchImportsOnly);
             version (LOGSEARCH) if (s) printMsg("-Scope.search() found import", s);
-
-            /** Still find private symbols, so that symbols that weren't access
-             * checked by the compiler remain usable.  Once the deprecation is over,
-             * this should be moved to search_correct instead.
-             */
-            if (!s && !(flags & IgnoreSymbolVisibility))
-            {
-                s = searchScopes(flags | SearchLocalsOnly | IgnoreSymbolVisibility);
-                if (!s)
-                    s = searchScopes(flags | SearchImportsOnly | IgnoreSymbolVisibility);
-
-                if (s && !(flags & IgnoreErrors))
-                    .deprecation(loc, "`%s` is not visible from module `%s`", s.toPrettyChars(), _module.toChars());
-                version (LOGSEARCH) if (s) printMsg("-Scope.search() found imported private symbol", s);
-            }
-        }
-        if (global.params.check10378)
-        {
-            alias snew = s;
-            if (sold !is snew)
-                deprecation10378(loc, sold, snew);
-            if (global.params.bug10378)
-                s = sold;
         }
         return s;
     }
 
-    /* A helper function to show deprecation message for new name lookup rule.
-     */
-    extern (D) static void deprecation10378(Loc loc, Dsymbol sold, Dsymbol snew)
-    {
-        // https://issues.dlang.org/show_bug.cgi?id=15857
-        //
-        // The overloadset found via the new lookup rules is either
-        // equal or a subset of the overloadset found via the old
-        // lookup rules, so it suffices to compare the dimension to
-        // check for equality.
-        OverloadSet osold, osnew;
-        if (sold && (osold = sold.isOverloadSet()) !is null &&
-            snew && (osnew = snew.isOverloadSet()) !is null &&
-            osold.a.dim == osnew.a.dim)
-            return;
-
-        OutBuffer buf;
-        buf.writestring("local import search method found ");
-        if (osold)
-            buf.printf("%s `%s` (%d overloads)", sold.kind(), sold.toPrettyChars(), cast(int) osold.a.dim);
-        else if (sold)
-            buf.printf("%s `%s`", sold.kind(), sold.toPrettyChars());
-        else
-            buf.writestring("nothing");
-        buf.writestring(" instead of ");
-        if (osnew)
-            buf.printf("%s `%s` (%d overloads)", snew.kind(), snew.toPrettyChars(), cast(int) osnew.a.dim);
-        else if (snew)
-            buf.printf("%s `%s`", snew.kind(), snew.toPrettyChars());
-        else
-            buf.writestring("nothing");
-
-        deprecation(loc, buf.peekString());
-    }
-
     extern (C++) Dsymbol search_correct(Identifier ident)
     {
         if (global.gag)
@@ -602,6 +531,11 @@ struct Scope
             return cast(void*)s;
         }
 
+        Dsymbol scopesym = null;
+        // search for exact name ignoring visibility first
+        if (auto s = search(Loc.initial, ident, &scopesym, IgnoreErrors | IgnoreSymbolVisibility))
+            return s;
+
         return cast(Dsymbol)speller(ident.toChars(), &scope_search_fp, idchars);
     }
 

src/dmd/dsymbol.d

@@ -699,6 +699,11 @@ extern (C++) class Dsymbol : RootObject
 
         if (global.gag)
             return null; // don't do it for speculative compiles; too time consuming
+
+        // search for exact name ignoring visibility first
+        if (auto s = search(Loc.initial, ident, IgnoreErrors | IgnoreSymbolVisibility))
+            return s;
+
         return cast(Dsymbol)speller(ident.toChars(), &symbol_search_fp, idchars);
     }
 
@@ -744,7 +749,8 @@ extern (C++) class Dsymbol : RootObject
                 {
                     sm = s.search_correct(ti.name);
                     if (sm)
-                        .error(loc, "template identifier `%s` is not a member of %s `%s`, did you mean %s `%s`?", ti.name.toChars(), s.kind(), s.toPrettyChars(), sm.kind(), sm.toChars());
+                        .error(loc, "template identifier `%s` is not a member of %s `%s`, did you mean %s%s `%s`?",
+                               ti.name.toChars(), s.kind(), s.toPrettyChars(), sm.ident == ti.name ? "non-visible ".ptr : "".ptr, sm.kind(), sm.toChars());
                     else
                         .error(loc, "template identifier `%s` is not a member of %s `%s`", ti.name.toChars(), s.kind(), s.toPrettyChars());
                     return null;
@@ -1348,10 +1354,9 @@ public:
                 {
                     if (flags & SearchImportsOnly)
                         continue;
-                    // compatibility with -transition=import
-                    // https://issues.dlang.org/show_bug.cgi?id=15925
-                    // SearchLocalsOnly should always get set for new lookup rules
-                    sflags |= (flags & SearchLocalsOnly);
+
+                    // only search locals, but not imports in mixin templates
+                    sflags |= SearchLocalsOnly;
                 }
 
                 /* Don't find private members if ss is a module

src/dmd/dsymbolsem.d

@@ -1468,7 +1468,8 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
                 {
                     Dsymbol s = imp.mod.search_correct(imp.names[i]);
                     if (s)
-                        imp.mod.error(imp.loc, "import `%s` not found, did you mean %s `%s`?", imp.names[i].toChars(), s.kind(), s.toPrettyChars());
+                        imp.mod.error(imp.loc, "import `%s` not found, did you mean %s%s `%s`?",
+                            imp.names[i].toChars(), s.ident == imp.names[i] ? "non-visible ".ptr : "".ptr, s.kind(), s.toPrettyChars());
                     else
                         imp.mod.error(imp.loc, "import `%s` not found", imp.names[i].toChars());
                     ad.type = Type.terror;
@@ -3558,13 +3559,17 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
                         functionToBufferFull(cast(TypeFunction)(fd.type), &buf1,
                             new Identifier(fd.toPrettyChars()), &hgs, null);
 
-                        error(funcdecl.loc, "function `%s` does not override any function, did you mean to override `%s`?",
-                            funcdeclToChars, buf1.peekString());
+                        error(funcdecl.loc, "function `%s` does not override any function, did you mean to override %s%s `%s`?",
+                            funcdeclToChars,
+                            s.ident == funcdecl.ident ? "non-visible ".ptr : "".ptr,
+                            s.kind, buf1.peekString());
                     }
                     else
                     {
-                        error(funcdecl.loc, "function `%s` does not override any function, did you mean to override %s `%s`?",
-                            funcdeclToChars, s.kind, s.toPrettyChars());
+                        error(funcdecl.loc, "function `%s` does not override any function, did you mean to override %s%s `%s`?",
+                            funcdeclToChars,
+                            s.ident == funcdecl.ident ? "non-visible ".ptr : "".ptr,
+                            s.kind, s.toPrettyChars());
                         errorSupplemental(funcdecl.loc, "Functions are the only declarations that may be overriden");
                     }
                 }

src/dmd/dtemplate.d

@@ -6422,7 +6422,8 @@ extern (C++) class TemplateInstance : ScopeDsymbol
             {
                 s = sc.search_correct(id);
                 if (s)
-                    error("template `%s` is not defined, did you mean %s?", id.toChars(), s.toChars());
+                    error("template `%s` is not defined, did you mean %s`%s`?",
+                        id.toChars(), s.ident == id ? "non-visible ".ptr : "".ptr, s.toChars());
                 else
                     error("template `%s` is not defined", id.toChars());
                 return false;

src/dmd/expressionsem.d

@@ -413,45 +413,12 @@ private Expression searchUFCS(Scope* sc, UnaExp ue, Identifier ident)
     if (sc.flags & SCOPE.ignoresymbolvisibility)
         flags |= IgnoreSymbolVisibility;
 
-    Dsymbol sold = void;
-    if (global.params.bug10378 || global.params.check10378)
-    {
-        sold = searchScopes(flags | IgnoreSymbolVisibility);
-        if (!global.params.check10378)
-        {
-            s = sold;
-            goto Lsearchdone;
-        }
-    }
-
     // First look in local scopes
     s = searchScopes(flags | SearchLocalsOnly);
     if (!s)
     {
         // Second look in imported modules
         s = searchScopes(flags | SearchImportsOnly);
-
-        /** Still find private symbols, so that symbols that weren't access
-         * checked by the compiler remain usable.  Once the deprecation is over,
-         * this should be moved to search_correct instead.
-         */
-        if (!s && !(flags & IgnoreSymbolVisibility))
-        {
-            s = searchScopes(flags | SearchLocalsOnly | IgnoreSymbolVisibility);
-            if (!s)
-                s = searchScopes(flags | SearchImportsOnly | IgnoreSymbolVisibility);
-
-            if (s)
-                .deprecation(loc, "`%s` is not visible from module `%s`", s.toPrettyChars(), sc._module.toChars());
-        }
-    }
-    if (global.params.check10378)
-    {
-        alias snew = s;
-        if (sold !is snew)
-            Scope.deprecation10378(loc, sold, snew);
-        if (global.params.bug10378)
-            s = sold;
     }
 Lsearchdone:
 
@@ -2527,7 +2494,8 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
         if (const n = importHint(exp.ident.toString()))
             exp.error("`%s` is not defined, perhaps `import %.*s;` is needed?", exp.ident.toChars(), cast(int)n.length, n.ptr);
         else if (auto s2 = sc.search_correct(exp.ident))
-            exp.error("undefined identifier `%s`, did you mean %s `%s`?", exp.ident.toChars(), s2.kind(), s2.toChars());
+            exp.error("undefined identifier `%s`, did you mean %s%s `%s`?",
+                exp.ident.toChars(), s2.ident == exp.ident ? "non-visible ".ptr : "".ptr, s2.kind(), s2.toChars());
         else if (const p = Scope.search_correct_C(exp.ident))
             exp.error("undefined identifier `%s`, did you mean `%s`?", exp.ident.toChars(), p);
         else
@@ -10816,11 +10784,8 @@ Expression semanticY(DotIdExp exp, Scope* sc, int flag)
          */
         if (s && !(sc.flags & SCOPE.ignoresymbolvisibility) && !symbolIsVisible(sc._module, s))
         {
-            if (s.isDeclaration())
-                error(exp.loc, "`%s` is not visible from module `%s`", s.toPrettyChars(), sc._module.toChars());
-            else
-                deprecation(exp.loc, "`%s` is not visible from module `%s`", s.toPrettyChars(), sc._module.toChars());
-            // s = null;
+            error(exp.loc, "`%s` is not visible from module `%s`", s.toPrettyChars(), sc._module.toChars());
+            s = null;
         }
         if (s)
         {
@@ -11000,7 +10965,8 @@ Expression semanticY(DotIdExp exp, Scope* sc, int flag)
             return null;
         s = ie.sds.search_correct(exp.ident);
         if (s)
-            exp.error("undefined identifier `%s` in %s `%s`, did you mean %s `%s`?", exp.ident.toChars(), ie.sds.kind(), ie.sds.toPrettyChars(), s.kind(), s.toChars());
+            exp.error("undefined identifier `%s` in %s `%s`, did you mean %s%s `%s`?",
+                exp.ident.toChars(), ie.sds.kind(), ie.sds.toPrettyChars(), s.ident == exp.ident ? "non-visible ".ptr : "".ptr, s.kind(), s.toChars());
         else
             exp.error("undefined identifier `%s` in %s `%s`", exp.ident.toChars(), ie.sds.kind(), ie.sds.toPrettyChars());
         return new ErrorExp();

src/dmd/globals.d

@@ -143,8 +143,6 @@ struct Param
     bool betterC;           // be a "better C" compiler; no dependency on D runtime
     bool addMain;           // add a default main() function
     bool allInst;           // generate code for all template instantiations
-    bool check10378;        // check for issues transitioning to 10738
-    bool bug10378;          // use pre- https://issues.dlang.org/show_bug.cgi?id=10378 search strategy
     bool fix16997;          // fix integral promotions for unary + - ~ operators
                             // https://issues.dlang.org/show_bug.cgi?id=16997
     bool fixAliasThis;      // if the current scope has an alias this, check it before searching upper scopes

src/dmd/globals.h

@@ -118,8 +118,6 @@ struct Param
     bool betterC;       // be a "better C" compiler; no dependency on D runtime
     bool addMain;       // add a default main() function
     bool allInst;       // generate code for all template instantiations
-    bool check10378;    // check for issues transitioning to 10738
-    bool bug10378;      // use pre-bugzilla 10378 search strategy
     bool fix16997;      // fix integral promotions for unary + - ~ operators
                         // https://issues.dlang.org/show_bug.cgi?id=16997
     bool vsafe;         // use enhanced @safe checking

src/dmd/initsem.d

@@ -139,7 +139,8 @@ extern(C++) Initializer initializerSemantic(Initializer init, Scope* sc, Type t,
                         s = sd.search_correct(id);
                         Loc initLoc = i.value[j].loc;
                         if (s)
-                            error(initLoc, "`%s` is not a member of `%s`, did you mean %s `%s`?", id.toChars(), sd.toChars(), s.kind(), s.toChars());
+                            error(i.loc, "`%s` is not a member of `%s`, did you mean %s%s `%s`?",
+                                id.toChars(), sd.toChars(), s.ident == id ? "non-visible ".ptr : "".ptr, s.kind(), s.toChars());
                         else
                             error(initLoc, "`%s` is not a member of `%s`", id.toChars(), sd.toChars());
                         return new ErrorInitializer();

src/dmd/mars.d

@@ -1742,7 +1742,17 @@ bool parseCommandLine(const ref Strings arguments, const size_t argc, ref Param
                         foreach (t; Usage.transitions)
                         {
                             if (t.bugzillaNumber !is null)
-                                buf ~= `case `~t.bugzillaNumber~`: params.`~t.paramName~` = true;break;`;
+                            {
+                                buf ~= `case `~t.bugzillaNumber~`:`;
+
+                                if (t.deprecated_)
+                                    buf ~= `Loc loc;
+                                        deprecation(loc, "-transition=`~t.bugzillaNumber ~` has been deprecated and no longer has any effect.");`;
+                                else
+                                    buf ~= `params.`~t.paramName~` = true;`;
+
+                                buf ~= "break;";
+                            }
                         }
                         return buf;
                     }
@@ -1762,12 +1772,21 @@ bool parseCommandLine(const ref Strings arguments, const size_t argc, ref Param
                         import dmd.cli : Usage;
                         string buf = `case "all":`;
                         foreach (t; Usage.transitions)
-                            buf ~= `params.`~t.paramName~` = true;`;
+                            if (!t.deprecated_)
+                                buf ~= `params.`~t.paramName~` = true;`;
                         buf ~= "break;";
 
                         foreach (t; Usage.transitions)
                         {
-                            buf ~= `case "`~t.name~`": params.`~t.paramName~` = true;break;`;
+                            buf ~= `case "`~t.name~`":`;
+
+                            if (t.deprecated_)
+                                buf ~= `Loc loc;
+                                    deprecation(loc, "-transition=`~t.name~` has been deprecated and no longer has any effect.");`;
+                            else
+                                buf ~= `params.`~t.paramName~` = true;`;
+
+                            buf ~= "break;";
                         }
                         return buf;
                     }

src/dmd/traits.d

@@ -751,7 +751,7 @@ Expression semanticTraits(TraitsExp e, Scope* sc)
             return dimError(1);
 
         Scope* sc2 = sc.push();
-        sc2.flags = sc.flags | SCOPE.noaccesscheck;
+        sc2.flags = sc.flags | SCOPE.noaccesscheck | SCOPE.ignoresymbolvisibility;
         bool ok = TemplateInstance.semanticTiargs(e.loc, sc2, e.args, 1);
         sc2.pop();
         if (!ok)