Skip to content

Commit 2a0080c

Browse files
committed
[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`.
1 parent d69a1a5 commit 2a0080c

File tree

4 files changed

+40
-5
lines changed

4 files changed

+40
-5
lines changed

lib/Sema/CSFix.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,26 @@ bool RemoveExtraneousArguments::diagnose(Expr *root, bool asNote) const {
499499
return failure.diagnose(asNote);
500500
}
501501

502+
bool RemoveExtraneousArguments::isMinMaxNameShadowing(
503+
ConstraintSystem &cs, ConstraintLocatorBuilder locator) {
504+
auto *anchor = dyn_cast_or_null<CallExpr>(locator.getAnchor());
505+
if (!anchor)
506+
return false;
507+
508+
if (auto *UDE = dyn_cast<UnresolvedDotExpr>(anchor->getFn())) {
509+
if (auto *baseExpr = dyn_cast<DeclRefExpr>(UDE->getBase())) {
510+
auto *decl = baseExpr->getDecl();
511+
if (baseExpr->isImplicit() && decl &&
512+
decl->getFullName() == cs.getASTContext().Id_self) {
513+
auto memberName = UDE->getName();
514+
return memberName.isSimpleName("min") || memberName.isSimpleName("max");
515+
}
516+
}
517+
}
518+
519+
return false;
520+
}
521+
502522
RemoveExtraneousArguments *RemoveExtraneousArguments::create(
503523
ConstraintSystem &cs, FunctionType *contextualType,
504524
llvm::ArrayRef<IndexedParam> extraArgs, ConstraintLocator *locator) {

lib/Sema/CSFix.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,6 +1068,15 @@ class RemoveExtraneousArguments final
10681068

10691069
bool diagnose(Expr *root, bool asNote = false) const override;
10701070

1071+
/// FIXME(diagnostics): Once `resolveDeclRefExpr` is gone this
1072+
/// logic would be obsolete.
1073+
///
1074+
/// Determine whether presence of extraneous arguments indicates
1075+
/// potential name shadowing problem with local `min`/`max` shadowing
1076+
/// global definitions with different number of arguments.
1077+
static bool isMinMaxNameShadowing(ConstraintSystem &cs,
1078+
ConstraintLocatorBuilder locator);
1079+
10711080
static RemoveExtraneousArguments *
10721081
create(ConstraintSystem &cs, FunctionType *contextualType,
10731082
llvm::ArrayRef<IndexedParam> extraArgs, ConstraintLocator *locator);

lib/Sema/CSSimplify.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -998,6 +998,9 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments(
998998

999999
auto extraArguments = listener.getExtraneousArguments();
10001000
if (!extraArguments.empty()) {
1001+
if (RemoveExtraneousArguments::isMinMaxNameShadowing(cs, locator))
1002+
return cs.getTypeMatchFailure(locator);
1003+
10011004
// First let's see whether this is a situation where a single
10021005
// parameter is a tuple, but N distinct arguments were passed in.
10031006
if (AllowTupleSplatForSingleParameter::attempt(

test/NameBinding/name_lookup_min_max_conditional_conformance.swift

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ extension ContainsMinMax {
1717
}
1818

1919
func foo(_: Int, _: Int) {}
20-
// expected-note@-1 {{'foo' declared here}}
2120

2221
protocol ContainsFoo {}
2322
extension ContainsFoo {
@@ -34,9 +33,10 @@ extension NonConditional {
3433
_ = min(3, 4)
3534
// expected-error@-1{{use of 'min' refers to instance method}}
3635
// expected-note@-2{{use 'Swift.' to reference the global function}}
37-
_ = foo(5, 6)
38-
// expected-error@-1{{use of 'foo' refers to instance method}}
39-
// expected-note@-2{{use 'name_lookup_min_max_conditional_conformance.' to reference the global function}}
36+
37+
// FIXME(diagnostics): Better diagnostic in this case would be to suggest to add `name_lookup_min_max_conditional_conformance.`
38+
// to call because name `foo` is shadowed by instance method without arguments. Would be fixed by `resolveDeclRefExpr` refactoring.
39+
_ = foo(5, 6) // expected-error {{argument passed to call that takes no arguments}}
4040
}
4141
}
4242

@@ -52,7 +52,10 @@ extension Conditional {
5252
_ = min(3, 4)
5353
// 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}}
5454
// expected-note@-2{{use 'Swift.' to continue to reference the global function}}
55+
56+
// FIXME(diagnostics): Same as line 39, there should be only one error here about shadowing.
5557
_ = foo(5, 6)
56-
// expected-error@-1{{referencing instance method 'foo()' on 'Conditional' requires that 'T' conform to 'ContainsFoo'}}
58+
// expected-error@-1 {{referencing instance method 'foo()' on 'Conditional' requires that 'T' conform to 'ContainsFoo'}}
59+
// expected-error@-2 {{argument passed to call that takes no arguments}}
5760
}
5861
}

0 commit comments

Comments
 (0)