Skip to content
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.

Sign up for GitHub

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

Explicit Codable & CodingKey Derived Conformance #8125

Closed
wants to merge 9 commits into from
Closed

Explicit Codable & CodingKey Derived Conformance #8125

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c41aabe
Explicitly derive Codable & CodingKey conformance
Mar 15, 2017
d7651eb
classInheritsCodable → superclassIsCodable
Mar 16, 2017
888fded
Integrate changes from feedback
Mar 20, 2017
b98b68e
Don't decode a let var with a default value
Mar 21, 2017
8ab7f87
Split derived conformance for Codable into Encodable and Decodable
Apr 3, 2017
f4d7d43
Make CodingKey.stringValue non-optional
Apr 3, 2017
369a89b
Fix stringValue nil return for empty CodingKey enums
Apr 3, 2017
a411077
Update CodingKey unit tests with non-optional stringValue
Apr 3, 2017
a1902cb
Sink core protocols and types into the Swift stdlib
Apr 24, 2017
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
11 changes: 8 additions & 3 deletions include/swift/AST/Decl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2883,6 +2883,9 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext {
private:
/// Predicate used to filter StoredPropertyRange.
struct ToStoredProperty {
ToStoredProperty(bool skipInaccessible = false) :
skipUserInaccessible(skipInaccessible) {}
bool skipUserInaccessible;
Optional<VarDecl *> operator()(Decl *decl) const;
};

Expand All @@ -2892,8 +2895,9 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext {
ToStoredProperty>;

/// Return a collection of the stored member variables of this type.
StoredPropertyRange getStoredProperties() const {
return StoredPropertyRange(getMembers(), ToStoredProperty());
StoredPropertyRange getStoredProperties(bool skipInaccessible = false) const {
return StoredPropertyRange(getMembers(),
ToStoredProperty(skipInaccessible));
}

// Implement isa/cast/dyncast/etc.
Expand Down Expand Up @@ -5985,7 +5989,8 @@ inline bool ValueDecl::isSettable(const DeclContext *UseDC,
inline Optional<VarDecl *>
NominalTypeDecl::ToStoredProperty::operator()(Decl *decl) const {
if (auto var = dyn_cast<VarDecl>(decl)) {
if (!var->isStatic() && var->hasStorage())
if (!var->isStatic() && var->hasStorage() &&
(!skipUserInaccessible || var->isUserAccessible()))
return var;
}

Expand Down
4 changes: 4 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1895,6 +1895,10 @@ ERROR(broken_equatable_eq_operator,none,
"Equatable protocol is broken: no infix operator declaration for '=='", ())
ERROR(no_equal_overload_for_int,none,
"no overload of '==' for Int", ())
ERROR(broken_coding_key_requirement,none,
"CodingKey protocol is broken: unexpected requirement", ())
ERROR(broken_codable_requirement,none,
"Codable protocol is broken: unexpected requirement", ())

// Dynamic Self
ERROR(dynamic_self_non_method,none,
Expand Down
21 changes: 21 additions & 0 deletions include/swift/AST/KnownIdentifiers.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,39 @@ IDENTIFIER(Any)
IDENTIFIER(atIndexedSubscript)
IDENTIFIER_(bridgeToObjectiveC)
IDENTIFIER_WITH_NAME(code_, "_code")
IDENTIFIER(Codable)
IDENTIFIER(CodingKeys)
IDENTIFIER(container)
IDENTIFIER(CoreGraphics)
IDENTIFIER(CoreMedia)
IDENTIFIER(CGFloat)
IDENTIFIER(CVarArg)
IDENTIFIER(Darwin)
IDENTIFIER(dealloc)
IDENTIFIER(decode)
IDENTIFIER(Decoder)
IDENTIFIER(decoder)
IDENTIFIER(deinit)
IDENTIFIER(Element)
IDENTIFIER(encode)
IDENTIFIER(Encoder)
IDENTIFIER(encoder)
IDENTIFIER(error)
IDENTIFIER(forKeyedSubscript)
IDENTIFIER(Foundation)
IDENTIFIER(forKey)
IDENTIFIER(from)
IDENTIFIER(fromRaw)
IDENTIFIER(hashValue)
IDENTIFIER(init)
IDENTIFIER(initialize)
IDENTIFIER(initStorage)
IDENTIFIER(initialValue)
IDENTIFIER(intValue)
IDENTIFIER(Key)
IDENTIFIER(KeyedDecodingContainer)
IDENTIFIER(KeyedEncodingContainer)
IDENTIFIER(keyedBy)
IDENTIFIER(makeIterator)
IDENTIFIER(Iterator)
IDENTIFIER(load)
Expand All @@ -69,10 +84,16 @@ IDENTIFIER(setObject)
IDENTIFIER(simd)
IDENTIFIER(some)
IDENTIFIER(storage)
IDENTIFIER(stringValue)
IDENTIFIER(subscript)
IDENTIFIER(super)
IDENTIFIER(superDecoder)
IDENTIFIER(superEncoder)
IDENTIFIER(SwiftObject)
IDENTIFIER(to)
IDENTIFIER(toRaw)
IDENTIFIER(Type)
IDENTIFIER(type)
IDENTIFIER(Value)
IDENTIFIER(value)
IDENTIFIER_WITH_NAME(value_, "_value")
Expand Down
2 changes: 2 additions & 0 deletions include/swift/AST/KnownProtocols.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ PROTOCOL_(ErrorCodeProtocol)
PROTOCOL(OptionSet)
PROTOCOL_(BridgedNSError)
PROTOCOL_(BridgedStoredNSError)
PROTOCOL(CodingKey)
PROTOCOL(Codable)

PROTOCOL_(ObjectiveCBridgeable)
PROTOCOL_(DestructorSafeContainer)
Expand Down
4 changes: 3 additions & 1 deletion lib/AST/ASTContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,9 @@ ProtocolDecl *ASTContext::getProtocol(KnownProtocolKind kind) const {
// are in the Foundation module.
if (kind == KnownProtocolKind::BridgedNSError ||
kind == KnownProtocolKind::BridgedStoredNSError ||
kind == KnownProtocolKind::ErrorCodeProtocol) {
kind == KnownProtocolKind::ErrorCodeProtocol ||
kind == KnownProtocolKind::CodingKey ||
kind == KnownProtocolKind::Codable) {
ModuleDecl *foundation =
const_cast<ASTContext *>(this)->getLoadedModule(Id_Foundation);
if (!foundation)
Expand Down
30 changes: 30 additions & 0 deletions lib/AST/Decl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2092,9 +2092,39 @@ bool NominalTypeDecl::derivesProtocolConformance(ProtocolDecl *protocol) const {
case KnownProtocolKind::BridgedNSError:
return isObjC() && enumDecl->hasOnlyCasesWithoutAssociatedValues();

// Enums without associated values and enums with a raw type of String
// or Int can explicitly derive CodingKey conformance.
case KnownProtocolKind::CodingKey: {
Type rawType = enumDecl->getRawType();
if (rawType) {
auto parentDC = enumDecl->getDeclContext();
ASTContext &C = parentDC->getASTContext();

auto nominal = rawType->getAnyNominal();
return nominal == C.getStringDecl() || nominal == C.getIntDecl();
} else {
// Empty enums are allowed to conform as well.
return enumDecl->getAllElements().empty() ||
enumDecl->hasOnlyCasesWithoutAssociatedValues();
}
}

default:
return false;
}
} else if (isa<StructDecl>(this) || isa<ClassDecl>(this)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This violates LLVM style:

http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return

All of the code paths before the else if return. The else is not necessary. In fact, you could even invert the if condition and do an early return false if you want to.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you fix this, can you also add a * to the auto above on knownProtocol. (Or even change it to a reference). I know that this is something that is not in the code that you have touched yourself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing the else after return. knownProtocol is an Optional<KnownProtocolKind>, so leaving as auto

// Structs and classes can explicitly derive Codable conformance.
if (*knownProtocol == KnownProtocolKind::Codable) {
// FIXME: This is not actually correct. We cannot promise to always
// provide a witness here for all structs and classes. Unfortunately,
// figuring out whether this is actually possible requires much more
// context -- a TypeChecker and the parent decl context at least -- and is
// tightly coupled to the logic within DerivedConformance.
// This unfortunately means that we expect a witness even if one will not
// be produced, which requires DerivedConformance::deriveCodable to output
// its own diagnostics.
return true;
}
}
return false;
}
Expand Down
2 changes: 2 additions & 0 deletions lib/IRGen/GenMeta.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5548,6 +5548,8 @@ SpecialProtocol irgen::getSpecialProtocolID(ProtocolDecl *P) {
case KnownProtocolKind::BridgedNSError:
case KnownProtocolKind::BridgedStoredNSError:
case KnownProtocolKind::ErrorCodeProtocol:
case KnownProtocolKind::CodingKey:
case KnownProtocolKind::Codable:
return SpecialProtocol::None;
}

Expand Down
2 changes: 2 additions & 0 deletions lib/Sema/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ add_swift_library(swiftSema STATIC
ConstraintGraph.cpp
ConstraintLocator.cpp
ConstraintSystem.cpp
DerivedConformanceCodable.cpp
DerivedConformanceCodingKey.cpp
DerivedConformanceEquatableHashable.cpp
DerivedConformanceError.cpp
DerivedConformanceRawRepresentable.cpp
Expand Down
Loading