From ed084c05766c3437411171ea172c51fcf696b2d0 Mon Sep 17 00:00:00 2001 From: Richard Wei Date: Sun, 24 May 2020 05:02:46 -0700 Subject: [PATCH] [AutoDiff] Fix false 'Differentiable' synthesis warning on class property wrappers. In `Differentiable` derived conformances, determine whether a property-wrapped property is mutable by checking whether a setter exists, instead of calling `VarDecl::getPropertyWrapperMutability()`. Resolves rdar://63577692 (SR-12873) where class-typed property wrappers with a mutable `wrappedValue` are rejected by synthesis (not included in the `TangentVector`). Also improve the property-wrapper-spe cific immutability warning message. --- include/swift/AST/DiagnosticsSema.def | 6 +++--- lib/Sema/DerivedConformanceDifferentiable.cpp | 14 +++++--------- .../DerivedConformances/class_differentiable.swift | 13 ++++++++++++- .../struct_differentiable.swift | 13 ++++++++++++- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 02ae15baf0630..f96cb3b00edad 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -2764,10 +2764,10 @@ WARNING(differentiable_nondiff_type_implicit_noderivative_fixit,none, /*nominalCanDeriveAdditiveArithmetic*/ bool)) WARNING(differentiable_immutable_wrapper_implicit_noderivative_fixit,none, "synthesis of the 'Differentiable.move(along:)' requirement for %1 " - "requires all stored properties not marked with `@noDerivative` to be " - "mutable; add an explicit '@noDerivative' attribute" + "requires 'wrappedValue' in property wrapper %0 to be mutable; " + "add an explicit '@noDerivative' attribute" "%select{|, or conform %1 to 'AdditiveArithmetic'}2", - (/*wrapperType*/ StringRef, /*nominalName*/ Identifier, + (/*wrapperType*/ Identifier, /*nominalName*/ Identifier, /*nominalCanDeriveAdditiveArithmetic*/ bool)) WARNING(differentiable_let_property_implicit_noderivative_fixit,none, "synthesis of the 'Differentiable.move(along:)' requirement for %0 " diff --git a/lib/Sema/DerivedConformanceDifferentiable.cpp b/lib/Sema/DerivedConformanceDifferentiable.cpp index d838e1eec1ceb..6254ea6cb95f6 100644 --- a/lib/Sema/DerivedConformanceDifferentiable.cpp +++ b/lib/Sema/DerivedConformanceDifferentiable.cpp @@ -44,9 +44,7 @@ getStoredPropertiesForDifferentiation(NominalTypeDecl *nominal, DeclContext *DC, if (auto *originalProperty = vd->getOriginalWrappedProperty()) { // Skip immutable wrapped properties. `mutating func move(along:)` cannot // be synthesized to update these properties. - auto mutability = originalProperty->getPropertyWrapperMutability(); - assert(mutability.hasValue() && "Expected wrapped property mutability"); - if (mutability->Setter != PropertyWrapperMutability::Value::Mutating) + if (!originalProperty->getAccessor(AccessorKind::Set)) continue; // Use the original wrapped property. vd = originalProperty; @@ -508,11 +506,9 @@ static void checkAndDiagnoseImplicitNoDerivative(ASTContext &Context, // Diagnose wrapped properties whose property wrappers do not define // `wrappedValue.set`. `mutating func move(along:)` cannot be synthesized // to update these properties. - auto *wrapperDecl = - vd->getInterfaceType()->getNominalOrBoundGenericNominal(); - auto mutability = originalProperty->getPropertyWrapperMutability(); - assert(mutability.hasValue() && "Expected wrapped property mutability"); - if (mutability->Setter != PropertyWrapperMutability::Value::Mutating) { + if (!originalProperty->getAccessor(AccessorKind::Set)) { + auto *wrapperDecl = + vd->getInterfaceType()->getNominalOrBoundGenericNominal(); auto loc = originalProperty->getAttributeInsertionLoc(/*forModifier*/ false); Context.Diags @@ -520,7 +516,7 @@ static void checkAndDiagnoseImplicitNoDerivative(ASTContext &Context, loc, diag:: differentiable_immutable_wrapper_implicit_noderivative_fixit, - wrapperDecl->getNameStr(), nominal->getName(), + wrapperDecl->getName(), nominal->getName(), nominalCanDeriveAdditiveArithmetic) .fixItInsert(loc, "@noDerivative "); // Add an implicit `@noDerivative` attribute. diff --git a/test/AutoDiff/Sema/DerivedConformances/class_differentiable.swift b/test/AutoDiff/Sema/DerivedConformances/class_differentiable.swift index f363617f6fd3d..05e95b05738d6 100644 --- a/test/AutoDiff/Sema/DerivedConformances/class_differentiable.swift +++ b/test/AutoDiff/Sema/DerivedConformances/class_differentiable.swift @@ -541,18 +541,29 @@ struct Wrapper { var wrappedValue: Value } +@propertyWrapper +class ClassWrapper { + var wrappedValue: Value + init(wrappedValue: Value) { self.wrappedValue = wrappedValue } +} + struct Generic {} extension Generic: Differentiable where T: Differentiable {} class WrappedProperties: Differentiable { - // expected-warning @+1 {{synthesis of the 'Differentiable.move(along:)' requirement for 'WrappedProperties' requires all stored properties not marked with `@noDerivative` to be mutable; add an explicit '@noDerivative' attribute}} + // expected-warning @+1 {{synthesis of the 'Differentiable.move(along:)' requirement for 'WrappedProperties' requires 'wrappedValue' in property wrapper 'ImmutableWrapper' to be mutable; add an explicit '@noDerivative' attribute}} @ImmutableWrapper var immutableInt: Generic = Generic() // expected-warning @+1 {{stored property 'mutableInt' has no derivative because 'Generic' does not conform to 'Differentiable'; add an explicit '@noDerivative' attribute}} @Wrapper var mutableInt: Generic = Generic() @Wrapper var float: Generic = Generic() + @ClassWrapper var float2: Generic = Generic() @noDerivative @ImmutableWrapper var nondiff: Generic = Generic() + + static func testTangentMemberwiseInitializer() { + _ = TangentVector(float: .init(), float2: .init()) + } } // Test derived conformances in disallowed contexts. diff --git a/test/AutoDiff/Sema/DerivedConformances/struct_differentiable.swift b/test/AutoDiff/Sema/DerivedConformances/struct_differentiable.swift index a6078c129bf8b..c9ca868598ce3 100644 --- a/test/AutoDiff/Sema/DerivedConformances/struct_differentiable.swift +++ b/test/AutoDiff/Sema/DerivedConformances/struct_differentiable.swift @@ -354,18 +354,29 @@ struct Wrapper { var wrappedValue: Value } +@propertyWrapper +class ClassWrapper { + var wrappedValue: Value + init(wrappedValue: Value) { self.wrappedValue = wrappedValue } +} + struct Generic {} extension Generic: Differentiable where T: Differentiable {} struct WrappedProperties: Differentiable { - // expected-warning @+1 {{synthesis of the 'Differentiable.move(along:)' requirement for 'WrappedProperties' requires all stored properties not marked with `@noDerivative` to be mutable; add an explicit '@noDerivative' attribute}} + // expected-warning @+1 {{synthesis of the 'Differentiable.move(along:)' requirement for 'WrappedProperties' requires 'wrappedValue' in property wrapper 'ImmutableWrapper' to be mutable; add an explicit '@noDerivative' attribute}} @ImmutableWrapper var immutableInt: Generic // expected-warning @+1 {{stored property 'mutableInt' has no derivative because 'Generic' does not conform to 'Differentiable'; add an explicit '@noDerivative' attribute}} @Wrapper var mutableInt: Generic @Wrapper var float: Generic + @ClassWrapper var float2: Generic @noDerivative @ImmutableWrapper var nondiff: Generic + + static func testTangentMemberwiseInitializer() { + _ = TangentVector(float: .init(), float2: .init()) + } } // Verify that cross-file derived conformances are disallowed.