Skip to content

[5.9][PreCheck] Filter out macro declarations from result set #68288

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.

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

Merged
merged 1 commit into from
Sep 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
42 changes: 12 additions & 30 deletions lib/Sema/CSGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1601,40 +1601,22 @@ namespace {
TVO_CanBindToLValue | TVO_CanBindToNoEscape);
ArrayRef<ValueDecl*> decls = expr->getDecls();
SmallVector<OverloadChoice, 4> 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<MacroDecl>(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.
Expand Down
39 changes: 30 additions & 9 deletions lib/Sema/PreCheckExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<TypeDecl>(Lookup[0].getValueDecl())) {
auto *D = cast<TypeDecl>(Lookup[0].getValueDecl());
auto buildTypeExpr = [&](TypeDecl *D) -> Expr * {
// FIXME: This is odd.
if (isa<ModuleDecl>(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();
Expand All @@ -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<TypeDecl>(Lookup[0].getValueDecl())) {
return buildTypeExpr(cast<TypeDecl>(Lookup[0].getValueDecl()));
}

if (AllDeclRefs) {
Expand Down Expand Up @@ -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<MacroDecl>(D); })) {
ResultValues.erase(
llvm::remove_if(ResultValues,
[](const ValueDecl *D) { return isa<MacroDecl>(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<TypeDecl>(ResultValues.front())) {
return buildTypeExpr(cast<TypeDecl>(ResultValues.front()));
}
}

return buildRefExpr(ResultValues, DC, UDRE->getNameLoc(),
UDRE->isImplicit(), UDRE->getFunctionRefKind());
}
Expand Down
38 changes: 38 additions & 0 deletions test/Constraints/observable_macro_shadowing.swift
Original file line number Diff line number Diff line change
@@ -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<Element> : ObservableType {
}

extension ObservableType {
public static func empty() -> Observable<Element> { fatalError() }
}

//--- main.swift
import Test
import Observation

extension Observable {
func test() -> Observable<Bool> {
return Observable<Bool>.empty()
}
}
16 changes: 6 additions & 10 deletions test/Macros/macro_and_typealias.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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<Value> {
public struct Printer<Value> { // expected-note {{generic type 'Printer' declared here}}
init(_: (Value) -> Void) {}
}

public struct MultiPrinter<T, U> {
// expected-note@-1 {{'T' declared as parameter to type 'MultiPrinter'}}
// expected-note@-2 {{'U' declared as parameter to type 'MultiPrinter'}}
public struct MultiPrinter<T, U> { // expected-note {{generic type 'MultiPrinter' declared here}}
}

typealias Print = Printer
Expand All @@ -34,22 +32,20 @@ struct Test {
}

let _ = Print<Object, Int> {
// 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<Object> { // Ok
compute(root: $0, \.prop)
}

let _ = ConcretePrint<Object> { // expected-error {{cannot specialize non-generic type 'ConcretePrint' (aka 'Printer<Any>')}}
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<Int>()
// 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<R, V>(root: R, _: KeyPath<R, V>) {}
Expand Down