Skip to content

[SE-0258] Ensure that we fully check the property type vs. wrapper's value type #25353

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
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
9 changes: 0 additions & 9 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5444,9 +5444,6 @@ PropertyWrapperTypeInfo VarDecl::getAttachedPropertyWrapperTypeInfo() const {

Type VarDecl::getAttachedPropertyWrapperType() const {
auto &ctx = getASTContext();
if (!ctx.getLazyResolver())
return nullptr;

auto mutableThis = const_cast<VarDecl *>(this);
return evaluateOrDefault(ctx.evaluator,
AttachedPropertyWrapperTypeRequest{mutableThis},
Expand All @@ -5455,9 +5452,6 @@ Type VarDecl::getAttachedPropertyWrapperType() const {

Type VarDecl::getPropertyWrapperBackingPropertyType() const {
ASTContext &ctx = getASTContext();
if (!ctx.getLazyResolver())
return nullptr;

auto mutableThis = const_cast<VarDecl *>(this);
return evaluateOrDefault(
ctx.evaluator, PropertyWrapperBackingPropertyTypeRequest{mutableThis},
Expand All @@ -5467,9 +5461,6 @@ Type VarDecl::getPropertyWrapperBackingPropertyType() const {
PropertyWrapperBackingPropertyInfo
VarDecl::getPropertyWrapperBackingPropertyInfo() const {
auto &ctx = getASTContext();
if (!ctx.getLazyResolver())
return PropertyWrapperBackingPropertyInfo();

auto mutableThis = const_cast<VarDecl *>(this);
return evaluateOrDefault(
ctx.evaluator,
Expand Down
1 change: 1 addition & 0 deletions lib/AST/TypeCheckRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,7 @@ void PropertyWrapperBackingPropertyTypeRequest::noteCycleStep(
DiagnosticEngine &diags) const {
std::get<0>(getStorage())->diagnose(diag::circular_reference_through);
}

bool PropertyWrapperBackingPropertyInfoRequest::isCached() const {
auto var = std::get<0>(getStorage());
return !var->getAttrs().isEmpty();
Expand Down
25 changes: 23 additions & 2 deletions lib/Sema/CodeSynthesis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1756,13 +1756,34 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,
auto dc = var->getDeclContext();
Type storageInterfaceType = wrapperType;

Type storageType =
var->getDeclContext()->mapTypeIntoContext(storageInterfaceType);
Type storageType = dc->mapTypeIntoContext(storageInterfaceType);
if (!storageType) {
storageType = ErrorType::get(ctx);
isInvalid = true;
}

// Make sure that the property type matches the value of the
// wrapper type.
if (!storageType->hasError()) {
Type expectedPropertyType =
storageType->getTypeOfMember(
dc->getParentModule(),
wrapperInfo.valueVar,
wrapperInfo.valueVar->getValueInterfaceType());
Type propertyType =
dc->mapTypeIntoContext(var->getValueInterfaceType());
if (!expectedPropertyType->hasError() &&
!propertyType->hasError() &&
!propertyType->isEqual(expectedPropertyType)) {
var->diagnose(diag::property_wrapper_incompatible_property,
propertyType, wrapperType);
if (auto nominalWrapper = wrapperType->getAnyNominal()) {
nominalWrapper->diagnose(diag::property_wrapper_declared_here,
nominalWrapper->getFullName());
}
}
}

// Create the backing storage property and note it in the cache.
VarDecl *backingVar = new (ctx) VarDecl(/*IsStatic=*/var->isStatic(),
VarDecl::Specifier::Var,
Expand Down
10 changes: 8 additions & 2 deletions lib/Sema/TypeCheckPropertyWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -371,12 +371,15 @@ AttachedPropertyWrapperTypeRequest::evaluate(Evaluator &evaluator,
if (!customAttr)
return Type();

ASTContext &ctx = var->getASTContext();
if (!ctx.getLazyResolver())
return nullptr;

auto resolution =
TypeResolution::forContextual(var->getDeclContext());
TypeResolutionOptions options(TypeResolverContext::PatternBindingDecl);
options |= TypeResolutionFlags::AllowUnboundGenerics;

ASTContext &ctx = var->getASTContext();
auto &tc = *static_cast<TypeChecker *>(ctx.getLazyResolver());
if (tc.validateType(customAttr->getTypeLoc(), resolution, options))
return ErrorType::get(ctx);
Expand Down Expand Up @@ -409,10 +412,13 @@ PropertyWrapperBackingPropertyTypeRequest::evaluate(
if (!binding)
return Type();

ASTContext &ctx = var->getASTContext();
if (!ctx.getLazyResolver())
return Type();

// If there's an initializer of some sort, checking it will determine the
// property wrapper type.
unsigned index = binding->getPatternEntryIndexForVarDecl(var);
ASTContext &ctx = var->getASTContext();
TypeChecker &tc = *static_cast<TypeChecker *>(ctx.getLazyResolver());
if (binding->isInitialized(index)) {
tc.validateDecl(var);
Expand Down
29 changes: 29 additions & 0 deletions test/Serialization/Inputs/def_property_wrappers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,32 @@ public struct SomeWrapper<T> {
public struct HasWrappers {
@SomeWrapper public var x: Int = 17
}

// SR-10844
@_propertyWrapper
class A<T: Equatable> {

private var _value: T

var value: T {
get { _value }
set { _value = newValue }
}

init(initialValue: T) {
_value = initialValue
}
}

@_propertyWrapper
class B: A<Double> {
override var value: Double {
get { super.value }
set { super.value = newValue }
}
}

class Holder {
// @A var b = 10.0 // ok
@B var b = 10.0 // crash in test target
}
12 changes: 10 additions & 2 deletions test/Serialization/property_wrappers.swift
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
// RUN: %empty-directory(%t)
// RUN: %target-swift-frontend -emit-module -o %t %S/Inputs/def_property_wrappers.swift
// RUN: %empty-directory(%t-scratch)
// RUN: %target-swift-frontend -emit-module -o %t-scratch/def_property_wrappers~partial.swiftmodule -primary-file %S/Inputs/def_property_wrappers.swift -module-name def_property_wrappers -enable-testing
// RUN: %target-swift-frontend -merge-modules -emit-module -parse-as-library -sil-merge-partial-modules -disable-diagnostic-passes -disable-sil-perf-optzns -enable-testing %t-scratch/def_property_wrappers~partial.swiftmodule -module-name def_property_wrappers -o %t/def_property_wrappers.swiftmodule
// RUN: %target-swift-frontend -typecheck -I%t -verify %s -verify-ignore-unknown

import def_property_wrappers
@testable import def_property_wrappers

// SR-10844
func testSR10844() {
let holder = Holder()
holder.b = 100
}

func useWrappers(hd: HasWrappers) {
// Access the original properties
Expand Down
10 changes: 10 additions & 0 deletions test/decl/var/property_wrappers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -758,3 +758,13 @@ struct UsesWrapperRequiringP {
// expected-error@-2{{expected declaration}}
// expected-error@-3{{type annotation missing in pattern}}
}

// SR-10899 / rdar://problem/51588022
@_propertyWrapper
struct SR_10899_Wrapper { // expected-note{{property wrapper type 'SR_10899_Wrapper' declared here}}
var value: String { "hi" }
}

struct SR_10899_Usage {
@SR_10899_Wrapper var thing: Bool // expected-error{{property type 'Bool' does not match that of the 'value' property of its wrapper type 'SR_10899_Wrapper'}}
}