Skip to content

[5.3] [Typechecker] Emit a specialised diagnostic for redeclaration errors when the declaration is synthesised #31938

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 5 commits into from
May 21, 2020
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
6 changes: 6 additions & 0 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -5217,6 +5217,12 @@ class VarDecl : public AbstractStorageDecl {
Optional<PropertyWrapperMutability>
getPropertyWrapperMutability() const;

/// Returns whether this property is the backing storage property or a storage
/// wrapper for wrapper instance's projectedValue. If this property is
/// neither, then it returns `None`.
Optional<PropertyWrapperSynthesizedPropertyKind>
getPropertyWrapperSynthesizedPropertyKind() const;

/// Retrieve the backing storage property for a property that has an
/// attached property wrapper.
///
Expand Down
8 changes: 8 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -755,12 +755,20 @@ ERROR(invalid_redecl,none,"invalid redeclaration of %0", (DeclName))
ERROR(invalid_redecl_init,none,
"invalid redeclaration of synthesized %select{|memberwise }1%0",
(DeclName, bool))
ERROR(invalid_redecl_implicit,none,
"invalid redeclaration of synthesized "
"%select{%0|implementation for protocol requirement}1 %2",
(DescriptiveDeclKind, bool, DeclName))
WARNING(invalid_redecl_swift5_warning,none,
"redeclaration of %0 is deprecated and will be an error in Swift 5",
(DeclName))

NOTE(invalid_redecl_prev,none,
"%0 previously declared here", (DeclName))
NOTE(invalid_redecl_implicit_wrapper,none,
"%0 synthesized for property wrapper "
"%select{projected value|backing storage}1",
(DeclName, bool))

ERROR(ambiguous_type_base,none,
"%0 is ambiguous for type lookup in this context", (DeclNameRef))
Expand Down
11 changes: 11 additions & 0 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5950,6 +5950,17 @@ VarDecl::getPropertyWrapperMutability() const {
None);
}

Optional<PropertyWrapperSynthesizedPropertyKind>
VarDecl::getPropertyWrapperSynthesizedPropertyKind() const {
if (getOriginalWrappedProperty(
PropertyWrapperSynthesizedPropertyKind::Backing))
return PropertyWrapperSynthesizedPropertyKind::Backing;
if (getOriginalWrappedProperty(
PropertyWrapperSynthesizedPropertyKind::StorageWrapper))
return PropertyWrapperSynthesizedPropertyKind::StorageWrapper;
return None;
}

VarDecl *VarDecl::getPropertyWrapperBackingProperty() const {
return getPropertyWrapperBackingPropertyInfo().backingVar;
}
Expand Down
70 changes: 69 additions & 1 deletion lib/Sema/TypeCheckDeclPrimary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -711,7 +711,75 @@ CheckRedeclarationRequest::evaluate(Evaluator &eval, ValueDecl *current) const {
current->diagnose(diag::invalid_redecl_init,
current->getName(),
otherInit->isMemberwiseInitializer());
} else if (!current->isImplicit() && !other->isImplicit()) {
} else if (current->isImplicit() || other->isImplicit()) {
// If both declarations are implicit, we do not diagnose anything
// as it would lead to misleading diagnostics and it's likely that
// there's nothing actionable about it due to its implicit nature.
// One special case for this is property wrappers.
//
// Otherwise, if 'current' is implicit, then we diagnose 'other'
// since 'other' is a redeclaration of 'current'. Similarly, if
// 'other' is implicit, we diagnose 'current'.
const Decl *declToDiagnose = nullptr;
if (current->isImplicit() && other->isImplicit()) {
// If 'current' is a property wrapper backing storage property
// or projected value property, then diagnose the wrapped
// property.
if (auto VD = dyn_cast<VarDecl>(current)) {
if (auto originalWrappedProperty =
VD->getOriginalWrappedProperty()) {
declToDiagnose = originalWrappedProperty;
}
}
} else {
declToDiagnose = current->isImplicit() ? other : current;
}

if (declToDiagnose) {
// Figure out if the the declaration we've redeclared is a
// synthesized witness for a protocol requirement.
bool isProtocolRequirement = false;
if (auto VD = dyn_cast<ValueDecl>(current->isImplicit() ? current
: other)) {
isProtocolRequirement = llvm::any_of(
VD->getSatisfiedProtocolRequirements(), [&](ValueDecl *req) {
return req->getName() == VD->getName();
});
}
declToDiagnose->diagnose(diag::invalid_redecl_implicit,
current->getDescriptiveKind(),
isProtocolRequirement, other->getName());
}

// Emit a specialized note if the one of the declarations is
// the backing storage property ('_foo') or projected value
// property ('$foo') for a wrapped property. The backing or
// projected var has the same source location as the wrapped
// property we diagnosed above, so we don't need to extract
// the original property.
const VarDecl *varToDiagnose = nullptr;
auto kind = PropertyWrapperSynthesizedPropertyKind::Backing;
if (auto currentVD = dyn_cast<VarDecl>(current)) {
if (auto currentKind =
currentVD->getPropertyWrapperSynthesizedPropertyKind()) {
varToDiagnose = currentVD;
kind = *currentKind;
}
}
if (auto otherVD = dyn_cast<VarDecl>(other)) {
if (auto otherKind =
otherVD->getPropertyWrapperSynthesizedPropertyKind()) {
varToDiagnose = otherVD;
kind = *otherKind;
}
}

if (varToDiagnose) {
varToDiagnose->diagnose(
diag::invalid_redecl_implicit_wrapper, varToDiagnose->getName(),
kind == PropertyWrapperSynthesizedPropertyKind::Backing);
}
} else {
ctx.Diags.diagnoseWithNotes(
current->diagnose(diag::invalid_redecl,
current->getName()), [&]() {
Expand Down
3 changes: 1 addition & 2 deletions test/Sema/enum_conformance_synthesis.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func customHashable() {
enum InvalidCustomHashable {
case A, B

var hashValue: String { return "" }
var hashValue: String { return "" } // expected-error {{invalid redeclaration of synthesized implementation for protocol requirement 'hashValue'}}
}
func ==(x: InvalidCustomHashable, y: InvalidCustomHashable) -> String {
return ""
Expand Down Expand Up @@ -369,7 +369,6 @@ func canEatHotChip(_ birthyear:Birthyear) -> Bool {
return birthyear > .nineties(3)
}
// FIXME: Remove -verify-ignore-unknown.
// <unknown>:0: error: unexpected error produced: invalid redeclaration of 'hashValue'
// <unknown>:0: error: unexpected note produced: candidate has non-matching type '(Foo, Foo) -> Bool'
// <unknown>:0: error: unexpected note produced: candidate has non-matching type '<T> (Generic<T>, Generic<T>) -> Bool'
// <unknown>:0: error: unexpected note produced: candidate has non-matching type '(InvalidCustomHashable, InvalidCustomHashable) -> Bool'
Expand Down
19 changes: 18 additions & 1 deletion test/decl/var/property_wrappers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -849,11 +849,28 @@ struct WrapperWithProjectedValue<T> {
var projectedValue: T { return wrappedValue }
}

class TestInvalidRedeclaration {
class TestInvalidRedeclaration1 {

@WrapperWithProjectedValue var i = 17
// expected-note@-1 {{'i' previously declared here}}
// expected-note@-2 {{'$i' synthesized for property wrapper projected value}}
// expected-note@-3 {{'_i' synthesized for property wrapper backing storage}}

@WrapperWithProjectedValue var i = 39
// expected-error@-1 {{invalid redeclaration of 'i'}}
// expected-error@-2 {{invalid redeclaration of synthesized property '$i'}}
// expected-error@-3 {{invalid redeclaration of synthesized property '_i'}}
}

// SR-12839
struct TestInvalidRedeclaration2 {
var _foo1 = 123 // expected-error {{invalid redeclaration of synthesized property '_foo1'}}
@WrapperWithInitialValue var foo1 = 123 // expected-note {{'_foo1' synthesized for property wrapper backing storage}}
}

struct TestInvalidRedeclaration3 {
@WrapperWithInitialValue var foo1 = 123 // expected-note {{'_foo1' synthesized for property wrapper backing storage}}
var _foo1 = 123 // expected-error {{invalid redeclaration of synthesized property '_foo1'}}
}

// ---------------------------------------------------------------------------
Expand Down