Skip to content

Remove visiblity and lookup deprecation #7241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MartinNowak wants to merge 5 commits into from
Closed

Remove visiblity and lookup deprecation #7241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e865e05
remove visiblity and lookup deprecation
MartinNowak Oct 24, 2017
ac66837
suggest non-visible symbols in search_correct
MartinNowak Oct 24, 2017
2feca68
Mark -transition=import as deprecated and trigger deprecation options
wilzbach Jan 18, 2018
4ec085c
Update tests
wilzbach Jan 18, 2018
245460b
Remove -transition=imports flag from its test
wilzbach Jan 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/import_deprecations.dd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 2 additions & 0 deletions docs/gen_man.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ 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;
Expand Down
11 changes: 7 additions & 4 deletions src/dmd/cli.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,18 +570,19 @@ dmd -cov -unittest myprog.d
{
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", true),
Transition(null, "checkimports", "check10378",
"give deprecation messages about 10378 anomalies"),
"give deprecation messages about 10378 anomalies", true),
Copy link
Contributor

@wilzbach wilzbach Jan 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As there's no Flag available here, I was also thinking about doing the following for better clarity:

enum Deprecated { no, yes}
....
"revert to single phase name lookup", Transition.Deprecated.yes),

Copy link
Member

@Geod24 Geod24 Jul 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started to work on something similar, and went with setting the paramName as null to mean deprecated

Transition("14488", "complex", "vcomplex",
"give deprecation messages about all usages of complex or imaginary types"),
Transition("16997", "intpromote", "fix16997",
Expand Down Expand Up @@ -658,6 +659,8 @@ 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;
Expand Down
75 changes: 4 additions & 71 deletions src/dmd/dscope.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,19 +528,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);
Expand All @@ -549,68 +536,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 (C++) 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)
Expand Down Expand Up @@ -651,6 +580,10 @@ struct Scope
return cast(void*)s;
}

Dsymbol scopesym = null;
// search for exact name ignoring visibility first
if (auto s = search(Loc(), ident, &scopesym, IgnoreErrors | IgnoreSymbolVisibility))
return s;
return cast(Dsymbol)speller(ident.toChars(), &scope_search_fp, idchars);
}

Expand Down
12 changes: 7 additions & 5 deletions src/dmd/dsymbol.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,9 @@ 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(), ident, IgnoreErrors | IgnoreSymbolVisibility))
return s;
return cast(Dsymbol)speller(ident.toChars(), &symbol_search_fp, idchars);
}

Expand Down Expand Up @@ -705,7 +708,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;
Expand Down Expand Up @@ -1300,10 +1304,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
sflags |= (flags & SearchLocalsOnly);
// only search locals, but not imports in mixin templates
sflags |= SearchLocalsOnly;
}

/* Don't find private members if ss is a module
Expand Down
6 changes: 4 additions & 2 deletions src/dmd/dsymbolsem.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1160,7 +1160,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;
Expand Down Expand Up @@ -3145,7 +3146,8 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
}

if (s)
funcdecl.error("does not override any function, did you mean to override `%s%s`?",
funcdecl.error("does not override any function, did you mean to override %s`%s%s`?",
s.ident == funcdecl.ident ? "non-visible ".ptr : "".ptr,
bc.sym.isCPPclass() ? "extern (C++) ".ptr : "".ptr, s.toPrettyChars());
else
funcdecl.error("does not override any function");
Expand Down
3 changes: 2 additions & 1 deletion src/dmd/dtemplate.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6409,7 +6409,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?",
Copy link
Contributor

@wilzbach wilzbach Jan 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs backticks

id.toChars(), s.ident == id ? "non-visible ".ptr : "".ptr, s.toChars());
else
error("template `%s` is not defined", id.toChars());
return false;
Expand Down
34 changes: 0 additions & 34 deletions src/dmd/expression.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -642,47 +642,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);
Expand Down
13 changes: 6 additions & 7 deletions src/dmd/expressionsem.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1382,7 +1382,8 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
if (const n = importHint(exp.ident.toChars()))
exp.error("`%s` is not defined, perhaps `import %s;` is needed?", exp.ident.toChars(), n);
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
Expand Down Expand Up @@ -9587,11 +9588,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)
{
Expand Down Expand Up @@ -9771,7 +9769,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();
Expand Down
2 changes: 0 additions & 2 deletions src/dmd/globals.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,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 vsafe; // use enhanced @safe checking
Expand Down
2 changes: 0 additions & 2 deletions src/dmd/globals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,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
Expand Down
3 changes: 2 additions & 1 deletion src/dmd/initsem.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ private extern(C++) final class InitializerSemanticVisitor : Visitor
{
s = sd.search_correct(id);
if (s)
error(i.loc, "`%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(i.loc, "`%s` is not a member of `%s`", id.toChars(), sd.toChars());
result = new ErrorInitializer();
Expand Down
23 changes: 20 additions & 3 deletions src/dmd/mars.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1736,7 +1736,16 @@ private bool parseCommandLine(const ref Strings arguments, const size_t argc, re
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, "The -transition=`~t.bugzillaNumber ~` has no effect anymore.");`;
else
buf ~= `params.`~t.paramName~` = true;`;

buf ~= "break;";
}
}
return buf;
}
Expand All @@ -1756,12 +1765,20 @@ private bool parseCommandLine(const ref Strings arguments, const size_t argc, re
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, "The -transition=`~t.name~` has no effect anymore.");`;
else
buf ~= `params.`~t.paramName~` = true;`;

buf ~= "break;";
}
return buf;
}
Expand Down
Loading