Remove visiblity and lookup deprecation (Take 2)

changelog/remove_visibility.dd

@@ -0,0 +1,5 @@
+The deprecation phase for access checks 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.

src/dmd/access.d

@@ -404,15 +404,11 @@ bool checkAccess(Loc loc, Scope* sc, Expression e, Declaration d)
         // Unittests are always accessible.
         return false;
     }
+
     if (!e)
-    {
-        if (d.prot().kind == Prot.Kind.private_ && d.getAccessModule() != sc._module || d.prot().kind == Prot.Kind.package_ && !hasPackageAccess(sc, d))
-        {
-            error(loc, "%s `%s` is not accessible from module `%s`", d.kind(), d.toPrettyChars(), sc._module.toChars());
-            return true;
-        }
-    }
-    else if (e.type.ty == Tclass)
+        return false;
+
+    if (e.type.ty == Tclass)
     {
         // Do access check
         ClassDeclaration cd = (cast(TypeClass)e.type).sym;
@@ -455,11 +451,7 @@ bool checkAccess(Loc loc, Scope* sc, Package p)
         if (sc.scopesym && sc.scopesym.isPackageAccessible(p, Prot(Prot.Kind.private_)))
             return false;
     }
-    auto name = p.toPrettyChars();
-    if (p.isPkgMod == PKG.module_ || p.isModule())
-        error(loc, "%s `%s` is not accessible here, perhaps add `static import %s;`", p.kind(), name, name);
-    else
-        error(loc, "%s `%s` is not accessible here", p.kind(), name);
+
     return true;
 }
 

src/dmd/cli.d

@@ -675,14 +675,15 @@ dmd -cov -unittest myprog.d
         string paramName; // internal transition parameter name
         string helpText; // detailed description of the feature
         bool documented = true; // whether this option should be shown in the documentation
+        bool deprecated_; /// whether the feature is still in use
     }
 
     /// Returns all available transitions
     static immutable transitions = [
         Feature("field", "vfield",
             "list all non-mutable fields which occupy an object instance"),
         Feature("checkimports", "check10378",
-            "give deprecation messages about 10378 anomalies"),
+            "give deprecation messages about 10378 anomalies", true, true),
         Feature("complex", "vcomplex",
             "give deprecation messages about all usages of complex or imaginary types"),
         Feature("tls", "vtls",
@@ -694,7 +695,7 @@ dmd -cov -unittest myprog.d
     /// Returns all available reverts
     static immutable reverts = [
         Feature("dip25", "noDIP25", "revert DIP25 changes https://github.com/dlang/DIPs/blob/master/DIPs/archive/DIP25.md"),
-        Feature("import", "bug10378", "revert to single phase name lookup"),
+        Feature("import", "bug10378", "revert to single phase name lookup", true, true),
     ];
 
     /// Returns all available previews
@@ -777,6 +778,8 @@ struct CLIUsage
             "list information on all " ~ description)] ~ features;
         foreach (t; allTransitions)
         {
+            if (t.deprecated_)
+                continue;
             if (!t.documented)
                 continue;
             buf ~= "  =";

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)
@@ -601,6 +530,10 @@ struct Scope
             return s;
         }
 
+        Dsymbol scopesym = null;
+        // search for exact name first
+        if (auto s = search(Loc.initial, ident, &scopesym, IgnoreErrors))
+            return s;
         return speller!scope_search_fp(ident.toChars());
     }
 

src/dmd/dsymbol.d

@@ -790,6 +790,9 @@ extern (C++) class Dsymbol : ASTNode
 
         if (global.gag)
             return null; // don't do it for speculative compiles; too time consuming
+        // search for exact name first
+        if (auto s = search(Loc.initial, ident, IgnoreErrors))
+            return s;
         return speller!symbol_search_fp(ident.toChars());
     }
 
@@ -1453,7 +1456,7 @@ public:
                 // If private import, don't search it
                 if ((flags & IgnorePrivateImports) && prots[i] == Prot.Kind.private_)
                     continue;
-                int sflags = flags & (IgnoreErrors | IgnoreAmbiguous | IgnoreSymbolVisibility); // remember these in recursive searches
+                int sflags = flags & (IgnoreErrors | IgnoreAmbiguous); // remember these in recursive searches
                 Dsymbol ss = (*importedScopes)[i];
                 //printf("\tscanning import '%s', prots = %d, isModule = %p, isImport = %p\n", ss.toChars(), prots[i], ss.isModule(), ss.isImport());
 
@@ -1466,13 +1469,8 @@ 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
-                    if (global.params.check10378)
-                        sflags |= (flags & SearchLocalsOnly);
-                    else
-                        sflags |= SearchLocalsOnly;
+
+                    sflags |= SearchLocalsOnly;
                 }
 
                 /* Don't find private members if ss is a module
@@ -1548,18 +1546,6 @@ public:
                         a = mergeOverloadSet(ident, a, s);
                     s = a;
                 }
-                // TODO: remove once private symbol visibility has been deprecated
-                if (!(flags & IgnoreErrors) && s.prot().kind == Prot.Kind.private_ &&
-                    !s.isOverloadable() && !s.parent.isTemplateMixin() && !s.parent.isNspace())
-                {
-                    AliasDeclaration ad = void;
-                    // accessing private selective and renamed imports is
-                    // deprecated by restricting the symbol visibility
-                    if (s.isImport() || (ad = s.isAliasDeclaration()) !is null && ad._import !is null)
-                    {}
-                    else
-                        error(loc, "%s `%s` is `private`", s.kind(), s.toPrettyChars());
-                }
                 //printf("\tfound in imports %s.%s\n", toChars(), s.toChars());
                 return s;
             }

src/dmd/expressionsem.d

@@ -427,47 +427,13 @@ 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:
 
     if (!s)
         return ue.e1.type.Type.getProperty(loc, ident, 0);
@@ -2448,9 +2414,13 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
             {
                 if (withsym)
                 {
-                    Declaration d = s.isDeclaration();
-                    if (d)
-                        checkAccess(exp.loc, sc, null, d);
+                    if (auto t = withsym.withstate.exp.isTypeExp())
+                    {
+                        e = new TypeExp(exp.loc, t.type);
+                        e = new DotIdExp(exp.loc, e, exp.ident);
+                        result = e.expressionSemantic(sc);
+                        return;
+                    }
                 }
 
                 /* If f is really a function template,
@@ -11028,17 +10998,18 @@ 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;
+            s = null;
+        }
+        if (s)
+        {
+            auto p = s.isPackage();
+            if (p && checkAccess(exp.loc, sc, p))
+            {
+                s = null;
+            }
         }
         if (s)
         {
-            if (auto p = s.isPackage())
-                checkAccess(exp.loc, sc, p);
-
             // if 's' is a tuple variable, the tuple is returned.
             s = s.toAlias();
 
@@ -11212,7 +11183,12 @@ 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());
+        {
+            if (s.isPackage())
+                exp.error("undefined identifier `%s` in %s `%s`, perhaps add `static import %s;`", exp.ident.toChars(), ie.sds.kind(), ie.sds.toPrettyChars(), s.toPrettyChars());
+            else
+                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());
+        }
         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

@@ -153,8 +153,8 @@ 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 check10378;        // check for issues transitioning to 10738 @@@DEPRECATED@@@ Remove in 2010-05 or later
+    bool bug10378;          // use pre- https://issues.dlang.org/show_bug.cgi?id=10378 search strategy  @@@DEPRECATED@@@ Remove in 2010-05 or later
     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/mars.d

@@ -1524,12 +1524,20 @@ bool parseCommandLine(const ref Strings arguments, const size_t argc, ref Param
                 import dmd.cli : Usage;
                 string buf = `case "all":`;
                 foreach (t; features)
+                {
+                    if (t.deprecated_)
+                        continue;
+
                     buf ~= `params.`~t.paramName~` = true;`;
-                buf ~= "break;";
+                }
+                buf ~= "break;\n";
 
                 foreach (t; features)
                 {
-                    buf ~= `case "`~t.name~`": params.`~t.paramName~` = true; return true;`;
+                    buf ~= `case "`~t.name~`":`;
+                    if (t.deprecated_)
+                        buf ~= "deprecation(Loc.initial, \"`-"~name~"="~t.name~"` no longer has any effect.\"); ";
+                    buf ~= `params.`~t.paramName~` = true; return true;`;
                 }
                 return buf;
             }

src/dmd/traits.d

@@ -736,7 +736,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)