averello/swift-mirror
1
0
Fork 0
mirror of https://github.com/apple/swift.git synced 2025-12-14 20:36:38 +01:00
Code Issues Packages Projects Releases Wiki Activity

[ConstraintSystem] Use new fix/diagnostic for name shadowing

Browse Source 
Stop filtering outer overload choices while trying to pre-check
expression, instead have it always fetch those and use new
fix to only attempt them in diagnostic mode (unless it's min/max
situation with conditional conformances).
This commit is contained in:
Pavel Yaskevich
2020-01-06 08:57:01 -08:00
parent c9c20afe27
commit 78fda9ed98
6 changed files with 77 additions and 86 deletions
Download Patch File Download Diff File Expand all files Collapse all files

55
lib/Sema/CSSimplify.cpp
View File

@@ -3273,6 +3273,30 @@ bool ConstraintSystem::repairFailures(
locator))
break;
{
auto *calleeLocator = getCalleeLocator(loc);
if (hasFixFor(calleeLocator, FixKind::AddQualifierToAccessTopLevelName)) {
if (auto overload = findSelectedOverloadFor(calleeLocator)) {
if (auto choice = overload->choice.getDeclOrNull()) {
// If this is an argument of a symetric function/operator let's
// not fix any position rather than first because we'd just end
// up with ambiguity instead of reporting an actual problem with
// mismatched type since each argument can have district bindings.
if (auto *AFD = dyn_cast<AbstractFunctionDecl>(choice)) {
auto *paramList = AFD->getParameters();
auto firstParamType = paramList->get(0)->getInterfaceType();
if (elt.castTo<LocatorPathElt::ApplyArgToParam>().getParamIdx() >
0 &&
llvm::all_of(*paramList, [&](const ParamDecl *param) -> bool {
return param->getInterfaceType()->isEqual(firstParamType);
}))
return true;
}
}
}
}
}
conversionsOrFixes.push_back(
AllowArgumentMismatch::create(*this, lhs, rhs, loc));
break;
@@ -6201,8 +6225,27 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
if (candidates.size() == 1)
candidates.front()->setFavored();
generateConstraints(candidates, memberTy, outerAlternatives,
useDC, locator);
// We *might* include any non-members that we found in outer contexts in
// some special cases, for backwards compatibility: first, we have to be
// looking for one of the special names ('min' or 'max'), and second, all
// of the inner (viable) results need to come from conditional
// conformances. The second condition is how the problem here was
// encountered: a type ('Range') was made to conditionally conform to a
// new protocol ('Sequence'), which introduced some extra methods
// ('min' and 'max') that shadowed global functions that people regularly
// called within extensions to that type (usually adding 'clamp').
bool treatAsViable =
(member.isSimpleName("min") || member.isSimpleName("max")) &&
allFromConditionalConformances(DC, baseTy, result.ViableCandidates);
generateConstraints(
candidates, memberTy, outerAlternatives, useDC, locator, None,
/*requiresFix=*/!treatAsViable,
[&](unsigned, const OverloadChoice &) {
return treatAsViable ? nullptr
: AddQualifierToAccessTopLevelName::create(
*this, locator);
});
}
}
@@ -8852,6 +8895,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
case FixKind::AllowInvalidUseOfTrailingClosure:
case FixKind::AllowNonClassTypeToConvertToAnyObject:
case FixKind::SpecifyClosureReturnType:
case FixKind::AddQualifierToAccessTopLevelName:
llvm_unreachable("handled elsewhere");
}
@@ -9315,7 +9359,12 @@ ConstraintSystem::simplifyConstraint(const Constraint &constraint) {
case ConstraintKind::BindOverload:
if (auto *fix = constraint.getFix()) {
if (recordFix(fix))
// TODO(diagnostics): Impact should be associated with a fix unless
// it's a contextual problem, then only solver can decide what the impact
// would be in each particular situation.
auto impact =
fix->getKind() == FixKind::AddQualifierToAccessTopLevelName ? 10 : 1;
if (recordFix(fix, impact))
return SolutionKind::Error;
}

77
lib/Sema/TypeCheckConstraints.cpp
View File

@@ -452,20 +452,6 @@ static bool findNonMembers(ArrayRef<LookupResultEntry> lookupResults,
return AllDeclRefs;
}
/// Whether we should be looking at the outer results for a function called \c
/// name.
///
/// This is very restrictive because it's a source compatibility issue (see the
/// if (AllConditionalConformances) { (void)findNonMembers(...); } below).
static bool shouldConsiderOuterResultsFor(DeclNameRef name) {
const StringRef specialNames[] = {"min", "max"};
for (auto specialName : specialNames)
if (name.isSimpleName(specialName))
return true;
return false;
}
/// Bind an UnresolvedDeclRefExpr by performing name lookup and
/// returning the resultant expression. Context is the DeclContext used
/// for the lookup.
@@ -479,8 +465,11 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
NameLookupOptions lookupOptions = defaultUnqualifiedLookupOptions;
if (isa<AbstractFunctionDecl>(DC))
lookupOptions |= NameLookupFlags::KnownPrivate;
if (shouldConsiderOuterResultsFor(Name))
lookupOptions |= NameLookupFlags::IncludeOuterResults;
// TODO: Include all of the possible members to give a solver a
// chance to diagnose name shadowing which requires explicit
// name/module qualifier to access top-level name.
lookupOptions |= NameLookupFlags::IncludeOuterResults;
auto Lookup = TypeChecker::lookupUnqualified(DC, Name, Loc, lookupOptions);
@@ -625,14 +614,6 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
// better matching candidates.
if (localDeclAfterUse) {
auto innerDecl = localDeclAfterUse;
// Perform a thorough lookup if outer results was not included before.
if (!lookupOptions.contains(NameLookupFlags::IncludeOuterResults)) {
auto option = lookupOptions;
option |= NameLookupFlags::IncludeOuterResults;
Lookup = lookupUnqualified(DC, Name, Loc, option);
}
while (localDeclAfterUse) {
if (Lookup.outerResults().empty()) {
Context.Diags.diagnose(Loc, diag::use_local_before_declaration, Name);
@@ -649,13 +630,6 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
findNonMembers(Lookup.innerResults(), UDRE->getRefKind(),
/*breakOnMember=*/true, ResultValues, isValid);
}
// Drop outer results if they are not supposed to be included.
if (!lookupOptions.contains(NameLookupFlags::IncludeOuterResults)) {
Lookup.filter([&](LookupResultEntry Result, bool isOuter) {
return !isOuter;
});
}
}
// If we have an unambiguous reference to a type decl, form a TypeExpr.
@@ -715,7 +689,6 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
ResultValues.clear();
bool AllMemberRefs = true;
bool AllConditionalConformances = true;
ValueDecl *Base = nullptr;
DeclContext *BaseDC = nullptr;
for (auto Result : Lookup) {
@@ -732,26 +705,6 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
Base = ThisBase;
BaseDC = Result.getDeclContext();
// Check if this result is derived through a conditional conformance,
// meaning it comes from a protocol (or extension) where there's a
// conditional conformance for the type with the method in question
// (NB. that type may not be the type associated with DC, for tested types
// with static methods).
if (auto Proto = Value->getDeclContext()->getSelfProtocolDecl()) {
auto contextSelfType =
BaseDC->getInnermostTypeContext()->getDeclaredInterfaceType();
auto conformance = conformsToProtocol(
contextSelfType, Proto, DC,
ConformanceCheckFlags::InExpression |
ConformanceCheckFlags::SkipConditionalRequirements);
if (conformance.isInvalid() ||
conformance.getConditionalRequirements().empty()) {
AllConditionalConformances = false;
}
}
continue;
}
@@ -774,22 +727,12 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
/*Implicit=*/true);
}
// We *might* include any non-members that we found in outer contexts in
// some special cases, for backwards compatibility: first, we have to be
// looking for one of the special names
// ('shouldConsiderOuterResultsFor(Name)'), and second, all of the inner
// results need to come from conditional conformances. The second condition
// is how the problem here was encountered: a type ('Range') was made to
// conditionally conform to a new protocol ('Sequence'), which introduced
// some extra methods ('min' and 'max') that shadowed global functions that
// people regularly called within extensions to that type (usually adding
// 'clamp').
llvm::SmallVector<ValueDecl *, 4> outerAlternatives;
if (AllConditionalConformances) {
(void)findNonMembers(Lookup.outerResults(), UDRE->getRefKind(),
/*breakOnMember=*/false, outerAlternatives,
/*isValid=*/[&](ValueDecl *) { return true; });
}
(void)findNonMembers(Lookup.outerResults(), UDRE->getRefKind(),
/*breakOnMember=*/false, outerAlternatives,
/*isValid=*/[](ValueDecl *choice) -> bool {
return !choice->isInvalid();
});
// Otherwise, form an UnresolvedDotExpr and sema will resolve it based on
// type information.

2
test/APINotes/versioned-objc.swift
View File

@@ -166,7 +166,7 @@ extension PrintingInterference {
func testDroppingRenamedPrints() {
// CHECK-DIAGS-4: [[@LINE+1]]:{{[0-9]+}}: warning: use of 'print' treated as a reference to instance method
print(self)
// CHECK-DIAGS-5: [[@LINE-1]]:{{[0-9]+}}: error: missing argument for parameter 'extra' in call
// CHECK-DIAGS-5: [[@LINE-1]]:{{[0-9]+}}: error: use of 'print' refers to instance method rather than global function 'print(_:separator:terminator:)' in module 'Swift'
// CHECK-DIAGS-4-NOT: [[@LINE+1]]:{{[0-9]+}}:
print(self, extra: self)

10
test/Constraints/members.swift
View File

@@ -355,7 +355,7 @@ do {
// rdar://problem/25341015
extension Sequence {
func r25341015_1() -> Int {
return max(1, 2) // expected-error {{use of 'max' refers to instance method 'max(by:)' rather than global function 'max' in module 'Swift'}} expected-note {{use 'Swift.' to reference the global function in module 'Swift'}}
return max(1, 2) // expected-error {{use of 'max' refers to instance method rather than global function 'max' in module 'Swift'}} expected-note {{use 'Swift.' to reference the global function in module 'Swift'}}
}
}
@@ -381,7 +381,7 @@ func r25341015() {
class Bar {
func baz() {}
func qux() {
baz(1, 2) // expected-error {{argument passed to call that takes no arguments}}
baz(1, 2) // expected-error {{use of 'baz' refers to instance method rather than local function 'baz'}}
}
}
}
@@ -405,17 +405,17 @@ func bar_32854314() -> Int {
extension Array where Element == Int {
func foo() {
let _ = min(foo_32854314(), bar_32854314()) // expected-note {{use 'Swift.' to reference the global function in module 'Swift'}} {{13-13=Swift.}}
// expected-error@-1 {{use of 'min' nearly matches global function 'min' in module 'Swift' rather than instance method 'min(by:)'}}
// expected-error@-1 {{use of 'min' refers to instance method rather than global function 'min' in module 'Swift'}}
}
func foo(_ x: Int, _ y: Double) {
let _ = min(x, y) // expected-note {{use 'Swift.' to reference the global function in module 'Swift'}} {{13-13=Swift.}}
// expected-error@-1 {{use of 'min' nearly matches global function 'min' in module 'Swift' rather than instance method 'min(by:)'}}
// expected-error@-1 {{use of 'min' refers to instance method rather than global function 'min' in module 'Swift'}}
}
func bar() {
let _ = min(1.0, 2) // expected-note {{use 'Swift.' to reference the global function in module 'Swift'}} {{13-13=Swift.}}
// expected-error@-1 {{use of 'min' nearly matches global function 'min' in module 'Swift' rather than instance method 'min(by:)'}}
// expected-error@-1 {{use of 'min' refers to instance method rather than global function 'min' in module 'Swift'}}
}
}

14
test/NameBinding/name_lookup_min_max_conditional_conformance.swift
View File

@@ -16,7 +16,7 @@ extension ContainsMinMax {
func min() {}
}
func foo(_: Int, _: Int) {}
func foo(_: Int, _: Int) {} // expected-note 2 {{'foo' declared here}}
protocol ContainsFoo {}
extension ContainsFoo {
@@ -34,15 +34,14 @@ extension NonConditional {
// expected-error@-1{{use of 'min' refers to instance method}}
// expected-note@-2{{use 'Swift.' 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}}
_ = foo(5, 6) // expected-error {{use of 'foo' refers to instance method rather than global function 'foo' in module 'name_lookup_min_max_conditional_conformance'}}
// expected-note@-1 {{use 'name_lookup_min_max_conditional_conformance.' to reference the global function in module 'name_lookup_min_max_conditional_conformance'}} {{13-13=name_lookup_min_max_conditional_conformance.}}
}
}
struct Conditional<T> {}
extension Conditional: ContainsMinMax where T: ContainsMinMax {}
extension Conditional: ContainsFoo where T: ContainsFoo {} // expected-note {{requirement from conditional conformance of 'Conditional<T>' to 'ContainsFoo'}}
extension Conditional: ContainsFoo where T: ContainsFoo {}
extension Conditional {
func f() {
@@ -53,9 +52,8 @@ extension Conditional {
// 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@-2 {{argument passed to call that takes no arguments}}
// expected-error@-1 {{use of 'foo' refers to instance method rather than global function 'foo' in module 'name_lookup_min_max_conditional_conformance'}}
// expected-note@-2 {{use 'name_lookup_min_max_conditional_conformance.' to reference the global function in module 'name_lookup_min_max_conditional_conformance'}} {{13-13=name_lookup_min_max_conditional_conformance.}}
}
}

5
test/Sema/circular_decl_checking.swift
View File

@@ -23,9 +23,10 @@ class HasGenericFunc {
}
}
class HasProp {
class HasProp { // expected-note {{'HasProp' declared here}}
var HasProp: HasProp {
return HasProp() // expected-error {{cannot call value of non-function type 'HasProp'}}{{19-21=}}
return HasProp() // expected-error {{use of 'HasProp' refers to instance method rather than class 'HasProp' in module 'circular_decl_checking'}}
// expected-note@-1 {{use 'circular_decl_checking.' to reference the class in module 'circular_decl_checking'}} {{12-12=circular_decl_checking.}}
}
var SomethingElse: SomethingElse? { // expected-error {{use of undeclared type 'SomethingElse'}}
return nil