[Diagnostics] Add a workaround for inability to diagnose name shadowing

Let's cover at least the most common cases - min/max. Fixing the
problem requires refactoring of `resolveDeclRefExpr`.
This commit is contained in:
Pavel Yaskevich
2019-10-16 10:14:55 -07:00
parent d69a1a5eee
commit 2a0080c6e0
4 changed files with 40 additions and 5 deletions

View File

@@ -499,6 +499,26 @@ bool RemoveExtraneousArguments::diagnose(Expr *root, bool asNote) const {
return failure.diagnose(asNote);
}
bool RemoveExtraneousArguments::isMinMaxNameShadowing(
ConstraintSystem &cs, ConstraintLocatorBuilder locator) {
auto *anchor = dyn_cast_or_null<CallExpr>(locator.getAnchor());
if (!anchor)
return false;
if (auto *UDE = dyn_cast<UnresolvedDotExpr>(anchor->getFn())) {
if (auto *baseExpr = dyn_cast<DeclRefExpr>(UDE->getBase())) {
auto *decl = baseExpr->getDecl();
if (baseExpr->isImplicit() && decl &&
decl->getFullName() == cs.getASTContext().Id_self) {
auto memberName = UDE->getName();
return memberName.isSimpleName("min") || memberName.isSimpleName("max");
}
}
}
return false;
}
RemoveExtraneousArguments *RemoveExtraneousArguments::create(
ConstraintSystem &cs, FunctionType *contextualType,
llvm::ArrayRef<IndexedParam> extraArgs, ConstraintLocator *locator) {

View File

@@ -1068,6 +1068,15 @@ public:
bool diagnose(Expr *root, bool asNote = false) const override;
/// FIXME(diagnostics): Once `resolveDeclRefExpr` is gone this
/// logic would be obsolete.
///
/// Determine whether presence of extraneous arguments indicates
/// potential name shadowing problem with local `min`/`max` shadowing
/// global definitions with different number of arguments.
static bool isMinMaxNameShadowing(ConstraintSystem &cs,
ConstraintLocatorBuilder locator);
static RemoveExtraneousArguments *
create(ConstraintSystem &cs, FunctionType *contextualType,
llvm::ArrayRef<IndexedParam> extraArgs, ConstraintLocator *locator);

View File

@@ -998,6 +998,9 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments(
auto extraArguments = listener.getExtraneousArguments();
if (!extraArguments.empty()) {
if (RemoveExtraneousArguments::isMinMaxNameShadowing(cs, locator))
return cs.getTypeMatchFailure(locator);
// First let's see whether this is a situation where a single
// parameter is a tuple, but N distinct arguments were passed in.
if (AllowTupleSplatForSingleParameter::attempt(

View File

@@ -17,7 +17,6 @@ extension ContainsMinMax {
}
func foo(_: Int, _: Int) {}
// expected-note@-1 {{'foo' declared here}}
protocol ContainsFoo {}
extension ContainsFoo {
@@ -34,9 +33,10 @@ extension NonConditional {
_ = min(3, 4)
// expected-error@-1{{use of 'min' refers to instance method}}
// expected-note@-2{{use 'Swift.' to reference the global function}}
_ = foo(5, 6)
// expected-error@-1{{use of 'foo' refers to instance method}}
// expected-note@-2{{use 'name_lookup_min_max_conditional_conformance.' to reference the global function}}
// FIXME(diagnostics): Better diagnostic in this case would be to suggest to add `name_lookup_min_max_conditional_conformance.`
// to call because name `foo` is shadowed by instance method without arguments. Would be fixed by `resolveDeclRefExpr` refactoring.
_ = foo(5, 6) // expected-error {{argument passed to call that takes no arguments}}
}
}
@@ -52,7 +52,10 @@ extension Conditional {
_ = min(3, 4)
// expected-warning@-1{{use of 'min' as reference to global function in module 'Swift' will change in future versions of Swift to reference instance method in generic struct 'Conditional' which comes via a conditional conformance}}
// expected-note@-2{{use 'Swift.' to continue to reference the global function}}
// FIXME(diagnostics): Same as line 39, there should be only one error here about shadowing.
_ = foo(5, 6)
// expected-error@-1{{referencing instance method 'foo()' on 'Conditional' requires that 'T' conform to 'ContainsFoo'}}
// expected-error@-1 {{referencing instance method 'foo()' on 'Conditional' requires that 'T' conform to 'ContainsFoo'}}
// expected-error@-2 {{argument passed to call that takes no arguments}}
}
}