From 179fd9645a4e24c6f56353e493f67fcd41f223ee Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 31 Aug 2023 17:30:14 -0700 Subject: [PATCH] [PreCheck] Filter out macro declarations from result set Move logic from `ConstraintGenerator::visitOverloadedDeclRefExpr` to pre-check to avoid including macro declarations referenced without `#`. This means that pre-checking would synthesize `TypeExpr` in situations when there is a type reference that is shadowed by a stdlib macro. Resolves: https://github.com/apple/swift/issues/67815 Resolves: rdar://114796811 (cherry picked from commit cf257aa64a41b855b0318fc20a3f23b80c2492cf) --- lib/Sema/CSGen.cpp | 42 ++++++------------- lib/Sema/PreCheckExpr.cpp | 39 +++++++++++++---- .../observable_macro_shadowing.swift | 38 +++++++++++++++++ test/Macros/macro_and_typealias.swift | 16 +++---- 4 files changed, 86 insertions(+), 49 deletions(-) create mode 100644 test/Constraints/observable_macro_shadowing.swift diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index c103109419a88..f50b7d8fee110 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -1601,40 +1601,22 @@ namespace { TVO_CanBindToLValue | TVO_CanBindToNoEscape); ArrayRef decls = expr->getDecls(); SmallVector choices; - bool anyMacros = false; - auto addChoices = [&](bool skipMacros) { - for (unsigned i = 0, n = decls.size(); i != n; ++i) { - // If the result is invalid, skip it. - // FIXME: Note this as invalid, in case we don't find a solution, - // so we don't let errors cascade further. - if (decls[i]->isInvalid()) - continue; - - // If the result is a macro, skip it if we're supposed to. - if (skipMacros && isa(decls[i])) { - anyMacros = true; - continue; - } - OverloadChoice choice = - OverloadChoice(Type(), decls[i], expr->getFunctionRefKind()); - choices.push_back(choice); - } - }; + for (unsigned i = 0, n = decls.size(); i != n; ++i) { + // If the result is invalid, skip it. + // FIXME: Note this as invalid, in case we don't find a solution, + // so we don't let errors cascade further. + if (decls[i]->isInvalid()) + continue; - addChoices(/*skipMacros=*/true); + OverloadChoice choice = + OverloadChoice(Type(), decls[i], expr->getFunctionRefKind()); + choices.push_back(choice); + } if (choices.empty()) { - // If there are no valid overloads, but we ignored some macros, add - // the macros. This improves recovery when the user forgot the leading - // '#'. - if (anyMacros) { - addChoices(/*skipMacros=*/false); - assert(!choices.empty()); - } else { - // There are no suitable overloads. Just fail. - return nullptr; - } + // There are no suitable overloads. Just fail. + return nullptr; } // Record this overload set. diff --git a/lib/Sema/PreCheckExpr.cpp b/lib/Sema/PreCheckExpr.cpp index f74a70952598a..17d3167b317c5 100644 --- a/lib/Sema/PreCheckExpr.cpp +++ b/lib/Sema/PreCheckExpr.cpp @@ -650,16 +650,12 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, // FIXME: Need to refactor the way we build an AST node from a lookup result! - // If we have an unambiguous reference to a type decl, form a TypeExpr. - if (Lookup.size() == 1 && UDRE->getRefKind() == DeclRefKind::Ordinary && - isa(Lookup[0].getValueDecl())) { - auto *D = cast(Lookup[0].getValueDecl()); + auto buildTypeExpr = [&](TypeDecl *D) -> Expr * { // FIXME: This is odd. if (isa(D)) { - return new (Context) DeclRefExpr(D, UDRE->getNameLoc(), - /*Implicit=*/false, - AccessSemantics::Ordinary, - D->getInterfaceType()); + return new (Context) DeclRefExpr( + D, UDRE->getNameLoc(), + /*Implicit=*/false, AccessSemantics::Ordinary, D->getInterfaceType()); } auto *LookupDC = Lookup[0].getDeclContext(); @@ -668,12 +664,19 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, UDRE->getNameLoc(), D, LookupDC, // It might happen that LookupDC is null if this is checking // synthesized code, in that case, don't map the type into context, - // but return as is -- the synthesis should ensure the type is correct. + // but return as is -- the synthesis should ensure the type is + // correct. LookupDC ? LookupDC->mapTypeIntoContext(D->getInterfaceType()) : D->getInterfaceType()); } else { return TypeExpr::createForDecl(UDRE->getNameLoc(), D, LookupDC); } + }; + + // If we have an unambiguous reference to a type decl, form a TypeExpr. + if (Lookup.size() == 1 && UDRE->getRefKind() == DeclRefKind::Ordinary && + isa(Lookup[0].getValueDecl())) { + return buildTypeExpr(cast(Lookup[0].getValueDecl())); } if (AllDeclRefs) { @@ -710,6 +713,24 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, }); } + // Filter out macro declarations without `#` if there are valid + // non-macro results. + if (llvm::any_of(ResultValues, + [](const ValueDecl *D) { return !isa(D); })) { + ResultValues.erase( + llvm::remove_if(ResultValues, + [](const ValueDecl *D) { return isa(D); }), + ResultValues.end()); + + // If there is only one type reference in results, let's handle + // this in a special way. + if (ResultValues.size() == 1 && + UDRE->getRefKind() == DeclRefKind::Ordinary && + isa(ResultValues.front())) { + return buildTypeExpr(cast(ResultValues.front())); + } + } + return buildRefExpr(ResultValues, DC, UDRE->getNameLoc(), UDRE->isImplicit(), UDRE->getFunctionRefKind()); } diff --git a/test/Constraints/observable_macro_shadowing.swift b/test/Constraints/observable_macro_shadowing.swift new file mode 100644 index 0000000000000..8c59cb275371b --- /dev/null +++ b/test/Constraints/observable_macro_shadowing.swift @@ -0,0 +1,38 @@ +// RUN: %empty-directory(%t/src) +// RUN: %empty-directory(%t/sdk) +// RUN: split-file %s %t/src + +// RUN: %target-swift-frontend -emit-module %t/src/Test.swift \ +// RUN: -module-name Test -swift-version 5 -enable-library-evolution \ +// RUN: -emit-module-path %t/Test.swiftmodule + +// RUN: %target-swift-frontend -typecheck %t/src/main.swift \ +// RUN: -module-name main -I %t -verify + +// REQUIRES: swift_swift_parser +// REQUIRES: observation + +//--- Test.swift + +public protocol ObservableConvertibleType { + associatedtype Element +} + +public protocol ObservableType : ObservableConvertibleType {} + +public class Observable : ObservableType { +} + +extension ObservableType { + public static func empty() -> Observable { fatalError() } +} + +//--- main.swift +import Test +import Observation + +extension Observable { + func test() -> Observable { + return Observable.empty() + } +} diff --git a/test/Macros/macro_and_typealias.swift b/test/Macros/macro_and_typealias.swift index d2c28cd0d8fdb..fffd8b0fe17a2 100644 --- a/test/Macros/macro_and_typealias.swift +++ b/test/Macros/macro_and_typealias.swift @@ -9,13 +9,11 @@ @freestanding(expression) public macro ConcretePrint(_ value: Any) = #externalMacro(module: "MacroDefinition", type: "PrintMacro") @freestanding(expression) public macro MultiPrint(_ value: Any) = #externalMacro(module: "MacroDefinition", type: "PrintMacro") -public struct Printer { +public struct Printer { // expected-note {{generic type 'Printer' declared here}} init(_: (Value) -> Void) {} } -public struct MultiPrinter { - // expected-note@-1 {{'T' declared as parameter to type 'MultiPrinter'}} - // expected-note@-2 {{'U' declared as parameter to type 'MultiPrinter'}} +public struct MultiPrinter { // expected-note {{generic type 'MultiPrinter' declared here}} } typealias Print = Printer @@ -34,7 +32,7 @@ struct Test { } let _ = Print { - // expected-error@-1 {{generic type 'Print' specialized with too many type parameters (got 2, but expected 1)}} + // expected-error@-1 {{generic type 'Printer' specialized with too many type parameters (got 2, but expected 1)}} } let _ = OtherPrint { // Ok @@ -42,14 +40,12 @@ struct Test { } let _ = ConcretePrint { // expected-error {{cannot specialize non-generic type 'ConcretePrint' (aka 'Printer')}} - compute(root: $0, \.prop) // expected-error {{value of type 'Any' has no member 'prop'}} - // expected-note@-1 {{cast 'Any' to 'AnyObject' or use 'as!' to force downcast to a more specific type to access members}} + // expected-error@-1 {{cannot infer type of closure parameter '$0' without a type annotation}} + compute(root: $0, \.prop) } let _ = MultiPrint() - // expected-error@-1 {{generic type 'MultiPrint' specialized with too few type parameters (got 1, but expected 2)}} - // expected-error@-2 {{generic parameter 'T' could not be inferred}} - // expected-error@-3 {{generic parameter 'U' could not be inferred}} + // expected-error@-1 {{generic type 'MultiPrinter' specialized with too few type parameters (got 1, but expected 2)}} } func compute(root: R, _: KeyPath) {}