Skip to content

[Sema] Synthesize default values for memberwise init #19743

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

Merged
merged 6 commits into from
Mar 26, 2019
Merged

[Sema] Synthesize default values for memberwise init #19743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6f7d20b
Synthesize default values for memberwise init
Azoy Oct 5, 2018
6d00a57
Disallow default values for patterns binding multiple variables
Azoy Mar 11, 2019
e8bc662
Cleanups from Slava's comments
Azoy Mar 13, 2019
bc7cb33
Fix generic types
Azoy Mar 14, 2019
2bd99ee
clean up subs
Azoy Mar 15, 2019
dcedba7
fix lazy vars
Azoy Mar 19, 2019
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
12 changes: 10 additions & 2 deletions include/swift/AST/Decl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4855,7 +4855,7 @@ class ParamDecl : public VarDecl {
SourceLoc SpecifierLoc;

struct StoredDefaultArgument {
Expr *DefaultArg = nullptr;
PointerUnion<Expr *, VarDecl *> DefaultArg;
Initializer *InitContext = nullptr;
StringRef StringRepresentation;
};
Expand Down Expand Up @@ -4912,12 +4912,20 @@ class ParamDecl : public VarDecl {

Expr *getDefaultValue() const {
if (auto stored = DefaultValueAndFlags.getPointer())
return stored->DefaultArg;
return stored->DefaultArg.dyn_cast<Expr *>();
return nullptr;
}

VarDecl *getStoredProperty() const {
if (auto stored = DefaultValueAndFlags.getPointer())
return stored->DefaultArg.dyn_cast<VarDecl *>();
return nullptr;
}

void setDefaultValue(Expr *E);

void setStoredProperty(VarDecl *var);

Initializer *getDefaultArgumentInitContext() const {
if (auto stored = DefaultValueAndFlags.getPointer())
return stored->InitContext;
Expand Down
4 changes: 4 additions & 0 deletions include/swift/AST/DefaultArgumentKind.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ enum class DefaultArgumentKind : uint8_t {
EmptyArray,
/// An empty dictionary literal.
EmptyDictionary,
/// A reference to the stored property. This is a special default argument
/// kind for the synthesized memberwise constructor to emit a call to the
// property's initializer.
StoredProperty,
};
enum { NumDefaultArgumentKindBits = 4 };

Expand Down
3 changes: 2 additions & 1 deletion include/swift/Serialization/ModuleFormat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
/// describe what change you made. The content of this comment isn't important;
/// it just ensures a conflict if two people change the module format.
/// Don't worry about adhering to the 80-column limit for this line.
const uint16_t SWIFTMODULE_VERSION_MINOR = 477; // SILUndef serialized with ownership kind
const uint16_t SWIFTMODULE_VERSION_MINOR = 478; // stored property default arg

using DeclIDField = BCFixed<31>;

Expand Down Expand Up @@ -379,6 +379,7 @@ enum class DefaultArgumentKind : uint8_t {
NilLiteral,
EmptyArray,
EmptyDictionary,
StoredProperty,
};
using DefaultArgumentField = BCFixed<4>;

Expand Down
1 change: 1 addition & 0 deletions lib/AST/ASTDumper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,7 @@ static StringRef getDefaultArgumentKindString(DefaultArgumentKind value) {
case DefaultArgumentKind::EmptyArray: return "[]";
case DefaultArgumentKind::EmptyDictionary: return "[:]";
case DefaultArgumentKind::Normal: return "normal";
case DefaultArgumentKind::StoredProperty: return "stored property";
}

llvm_unreachable("Unhandled DefaultArgumentKind in switch.");
Expand Down
27 changes: 25 additions & 2 deletions lib/AST/Decl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5173,6 +5173,17 @@ void ParamDecl::setDefaultValue(Expr *E) {
DefaultValueAndFlags.getPointer()->DefaultArg = E;
}

void ParamDecl::setStoredProperty(VarDecl *var) {
if (!DefaultValueAndFlags.getPointer()) {
if (!var) return;

DefaultValueAndFlags.setPointer(
getASTContext().Allocate<StoredDefaultArgument>());
}

DefaultValueAndFlags.getPointer()->DefaultArg = var;
}

void ParamDecl::setDefaultArgumentInitContext(Initializer *initContext) {
assert(DefaultValueAndFlags.getPointer());
DefaultValueAndFlags.getPointer()->InitContext = initContext;
Expand Down Expand Up @@ -5201,6 +5212,17 @@ ParamDecl::getDefaultValueStringRepresentation(
return extractInlinableText(getASTContext().SourceMgr, getDefaultValue(),
scratch);
}
case DefaultArgumentKind::StoredProperty: {
assert(DefaultValueAndFlags.getPointer() &&
"default value not provided yet");
auto existing = DefaultValueAndFlags.getPointer()->StringRepresentation;
if (!existing.empty())
return existing;
auto var = getStoredProperty();
return extractInlinableText(getASTContext().SourceMgr,
var->getParentInitializer(),
scratch);
}
case DefaultArgumentKind::Inherited:
// FIXME: This needs /some/ kind of textual representation, but this isn't
// a great one.
Expand All @@ -5219,7 +5241,8 @@ ParamDecl::getDefaultValueStringRepresentation(

void
ParamDecl::setDefaultValueStringRepresentation(StringRef stringRepresentation) {
assert(getDefaultArgumentKind() == DefaultArgumentKind::Normal);
assert(getDefaultArgumentKind() == DefaultArgumentKind::Normal ||
getDefaultArgumentKind() == DefaultArgumentKind::StoredProperty);
assert(!stringRepresentation.empty());

if (!DefaultValueAndFlags.getPointer()) {
Expand All @@ -5238,7 +5261,7 @@ void DefaultArgumentInitializer::changeFunction(
}

auto param = paramList->get(getIndex());
if (param->getDefaultValue())
if (param->getDefaultValue() || param->getStoredProperty())
param->setDefaultArgumentInitContext(this);
}

Expand Down
2 changes: 2 additions & 0 deletions lib/IDE/CodeCompletion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2034,6 +2034,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
for (auto param : *func->getParameters()) {
switch (param->getDefaultArgumentKind()) {
case DefaultArgumentKind::Normal:
case DefaultArgumentKind::StoredProperty:
case DefaultArgumentKind::Inherited: // FIXME: include this?
return true;
default:
Expand Down Expand Up @@ -2061,6 +2062,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
return false;

case DefaultArgumentKind::Normal:
case DefaultArgumentKind::StoredProperty:
case DefaultArgumentKind::Inherited:
case DefaultArgumentKind::NilLiteral:
case DefaultArgumentKind::EmptyArray:
Expand Down
1 change: 1 addition & 0 deletions lib/SILGen/SILGen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1082,6 +1082,7 @@ void SILGenModule::emitDefaultArgGenerator(SILDeclRef constant, Expr *arg,
case DefaultArgumentKind::NilLiteral:
case DefaultArgumentKind::EmptyArray:
case DefaultArgumentKind::EmptyDictionary:
case DefaultArgumentKind::StoredProperty:
return;
}

Expand Down
42 changes: 36 additions & 6 deletions lib/SILGen/SILGenApply.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "swift/AST/ASTContext.h"
#include "swift/AST/DiagnosticsSIL.h"
#include "swift/AST/ForeignErrorConvention.h"
#include "swift/AST/GenericEnvironment.h"
#include "swift/AST/GenericSignature.h"
#include "swift/AST/ParameterList.h"
#include "swift/AST/Module.h"
Expand Down Expand Up @@ -3340,12 +3341,41 @@ void DelayedArgument::emitDefaultArgument(SILGenFunction &SGF,
const DefaultArgumentStorage &info,
SmallVectorImpl<ManagedValue> &args,
size_t &argIndex) {
auto value = SGF.emitApplyOfDefaultArgGenerator(info.loc,
info.defaultArgsOwner,
info.destIndex,
info.resultType,
info.origResultType);

RValue value;
bool isStoredPropertyDefaultArg = false;

// Check if this is a synthesized memberwise constructor for stored property
// default values
if (auto ctor = dyn_cast<ConstructorDecl>(info.defaultArgsOwner.getDecl())) {
if (ctor->isMemberwiseInitializer() && ctor->isImplicit()) {
auto param = ctor->getParameters()->get(info.destIndex);

if (auto var = param->getStoredProperty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work across modules? Normally you can’t call a memberwise initializer since they’re internal, but if a module is built with -enable-testing and imported with @testable import you can. I might be wrong but I don’t see ParamDecl::getStoredProperty() serialized anywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to spin up a quick test to test the current behavior, and I don't believe that we currently deserialize any default value from a module, but rather just the arg kind and the string representation. I'd be curious to hear how we could go about setting the stored property arg after we deserialize (maybe just take param name and look for the corresponding property?). Right now this emits an apply to the default arg generator (which there is none, so I believe the linker will complain...).

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the right solution is to change the code they serializes a ParamDecl to serialize a cross reference to the corresponding VarDecl. I would try to avoid a name-based lookup here. @jrose-apple might have more suggestions

Copy link
Contributor

Choose a reason for hiding this comment

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

I think emitting an apply to the default arg generator is the correct behavior. The fact that the expression comes from a stored property shouldn't matter except when generating the function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Emitting a default arg generator in this case doesn't work because the module with the stored property default arg doesn't emit a default arg generator function. So when we go to deserialize a stored property default arg, if we emit a default arg generator apply then we're going to be linking against undefined symbols.

Copy link
Contributor

Choose a reason for hiding this comment

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

Emitting a default arg generator in this case doesn't work because the module with the stored property default arg doesn't emit a default arg generator function.

Hm. Why doesn't it? Isn't this a bug? (under -enable-testing, anyway)

// This is a stored property default arg. Do not emit a call to the
// default arg generator, but rather the variable initializer expression
isStoredPropertyDefaultArg = true;

auto pbd = var->getParentPatternBinding();
auto entry = pbd->getPatternEntryForVarDecl(var);
auto subs = info.defaultArgsOwner.getSubstitutions();

value = SGF.emitApplyOfStoredPropertyInitializer(info.loc,
entry, subs,
info.resultType,
info.origResultType,
SGFContext());
}
}
}

if (!isStoredPropertyDefaultArg) {
value = SGF.emitApplyOfDefaultArgGenerator(info.loc,
info.defaultArgsOwner,
info.destIndex,
info.resultType,
info.origResultType);
}

SmallVector<ManagedValue, 4> loweredArgs;
SmallVector<DelayedArgument, 4> delayedArgs;
Optional<ForeignErrorConvention> errorConvention = None;
Expand Down
1 change: 1 addition & 0 deletions lib/Sema/CSApply.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4896,6 +4896,7 @@ getCallerDefaultArg(ConstraintSystem &cs, DeclContext *dc,
case DefaultArgumentKind::None:
llvm_unreachable("No default argument here?");

case DefaultArgumentKind::StoredProperty:
case DefaultArgumentKind::Normal:
return {nullptr, param->getDefaultArgumentKind()};

Expand Down
71 changes: 63 additions & 8 deletions lib/Sema/CodeSynthesis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1643,6 +1643,52 @@ void synthesizeAccessorBody(AbstractFunctionDecl *fn, void *) {
llvm_unreachable("bad synthesized function kind");
}

static void maybeAddMemberwiseDefaultArg(ParamDecl *arg, VarDecl *var,
SmallVectorImpl<DefaultArgumentInitializer *> &defaultInits,
unsigned paramSize, ASTContext &ctx) {
// First and foremost, if this is a constant don't bother.
if (var->isLet())
return;

// We can only provide default values for patterns binding a single variable.
// i.e. var (a, b) = getSomeTuple() is not allowed.
if (!var->getParentPattern()->getSingleVar())
return;

// If we don't have an expression initializer or silgen can't assign a default
// initializer, then we can't generate a default value. An example of where
// silgen can assign a default is var x: Int? where the default is nil.
// If the variable is lazy, go ahead and give it a default value.
if (!var->getAttrs().hasAttribute<LazyAttr>() &&
!var->getParentPatternBinding()->isDefaultInitializable())
return;

// We can add a default value now.

// Give this some bogus context right now, we'll fix it after making
// the constructor.
auto *initDC = new (ctx) DefaultArgumentInitializer(
arg->getDeclContext(), paramSize);

defaultInits.push_back(initDC);

// If the variable has a type T? and no initial value, return a nil literal
// default arg. All lazy variables return a nil literal as well. *Note* that
// the type will always be a sugared T? because we don't default init an
// explicit Optional<T>.
if ((isa<OptionalType>(var->getValueInterfaceType().getPointer()) &&
!var->getParentInitializer()) ||
var->getAttrs().hasAttribute<LazyAttr>()) {
arg->setDefaultArgumentKind(DefaultArgumentKind::NilLiteral);
return;
}

// Set the default value to the variable. When we emit this in silgen
// we're going to call the variable's initializer expression.
arg->setStoredProperty(var);
arg->setDefaultArgumentKind(DefaultArgumentKind::StoredProperty);
}

/// Create an implicit struct or class constructor.
///
/// \param decl The struct or class for which a constructor will be created.
Expand All @@ -1655,12 +1701,13 @@ ConstructorDecl *swift::createImplicitConstructor(TypeChecker &tc,
ImplicitConstructorKind ICK) {
assert(!decl->hasClangNode());

ASTContext &context = tc.Context;
ASTContext &ctx = tc.Context;
SourceLoc Loc = decl->getLoc();
auto accessLevel = AccessLevel::Internal;

// Determine the parameter type of the implicit constructor.
SmallVector<ParamDecl*, 8> params;
SmallVector<DefaultArgumentInitializer *, 8> defaultInits;
if (ICK == ImplicitConstructorKind::Memberwise) {
assert(isa<StructDecl>(decl) && "Only struct have memberwise constructor");

Expand Down Expand Up @@ -1693,28 +1740,30 @@ ConstructorDecl *swift::createImplicitConstructor(TypeChecker &tc,
// If var is a lazy property, its value is provided for the underlying
// storage. We thus take an optional of the properties type. We only
// need to do this because the implicit constructor is added before all
// the properties are type checked. Perhaps init() synth should be moved
// the properties are type checked. Perhaps init() synth should be moved
// later.
if (var->getAttrs().hasAttribute<LazyAttr>())
varInterfaceType = OptionalType::get(varInterfaceType);

// Create the parameter.
auto *arg = new (context)
auto *arg = new (ctx)
ParamDecl(VarDecl::Specifier::Default, SourceLoc(), Loc,
var->getName(), Loc, var->getName(), decl);
arg->setInterfaceType(varInterfaceType);
arg->setImplicit();

maybeAddMemberwiseDefaultArg(arg, var, defaultInits, params.size(), ctx);

params.push_back(arg);
}
}

auto paramList = ParameterList::create(context, params);
auto paramList = ParameterList::create(ctx, params);

// Create the constructor.
DeclName name(context, DeclBaseName::createConstructor(), paramList);
DeclName name(ctx, DeclBaseName::createConstructor(), paramList);
auto *ctor =
new (context) ConstructorDecl(name, Loc,
new (ctx) ConstructorDecl(name, Loc,
OTK_None, /*FailabilityLoc=*/SourceLoc(),
/*Throws=*/false, /*ThrowsLoc=*/SourceLoc(),
paramList, /*GenericParams=*/nullptr, decl);
Expand All @@ -1723,15 +1772,21 @@ ConstructorDecl *swift::createImplicitConstructor(TypeChecker &tc,
ctor->setImplicit();
ctor->setAccess(accessLevel);

if (ICK == ImplicitConstructorKind::Memberwise)
if (ICK == ImplicitConstructorKind::Memberwise) {
ctor->setIsMemberwiseInitializer();

// Fix default argument init contexts now that we have a constructor.
for (auto initDC : defaultInits) {
initDC->changeFunction(ctor, paramList);
}
}

// If we are defining a default initializer for a class that has a superclass,
// it overrides the default initializer of its superclass. Add an implicit
// 'override' attribute.
if (auto classDecl = dyn_cast<ClassDecl>(decl)) {
if (classDecl->getSuperclass())
ctor->getAttrs().add(new (tc.Context) OverrideAttr(/*IsImplicit=*/true));
ctor->getAttrs().add(new (ctx) OverrideAttr(/*IsImplicit=*/true));
}

return ctor;
Expand Down
2 changes: 2 additions & 0 deletions lib/Serialization/Deserialization.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,8 @@ getActualDefaultArgKind(uint8_t raw) {
return swift::DefaultArgumentKind::EmptyArray;
case serialization::DefaultArgumentKind::EmptyDictionary:
return swift::DefaultArgumentKind::EmptyDictionary;
case serialization::DefaultArgumentKind::StoredProperty:
return swift::DefaultArgumentKind::StoredProperty;
}
return None;
}
Expand Down
11 changes: 7 additions & 4 deletions lib/Serialization/Serialization.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1146,6 +1146,7 @@ static uint8_t getRawStableDefaultArgumentKind(swift::DefaultArgumentKind kind)
CASE(NilLiteral)
CASE(EmptyArray)
CASE(EmptyDictionary)
CASE(StoredProperty)
#undef CASE
}

Expand Down Expand Up @@ -3266,11 +3267,13 @@ void Serializer::writeDecl(const Decl *D) {
auto contextID = addDeclContextRef(param->getDeclContext());
Type interfaceType = param->getInterfaceType();

// Only save the text for normal default arguments, not any of the special
// ones.
// Only save the text for normal and stored property default arguments, not
// any of the special ones.
StringRef defaultArgumentText;
SmallString<128> scratch;
if (param->getDefaultArgumentKind() == swift::DefaultArgumentKind::Normal)
swift::DefaultArgumentKind argKind = param->getDefaultArgumentKind();
if (argKind == swift::DefaultArgumentKind::Normal ||
argKind == swift::DefaultArgumentKind::StoredProperty)
defaultArgumentText =
param->getDefaultValueStringRepresentation(scratch);

Expand All @@ -3283,7 +3286,7 @@ void Serializer::writeDecl(const Decl *D) {
addTypeRef(interfaceType),
param->isVariadic(),
param->isAutoClosure(),
getRawStableDefaultArgumentKind(param->getDefaultArgumentKind()),
getRawStableDefaultArgumentKind(argKind),
defaultArgumentText);

if (interfaceType->hasError()) {
Expand Down
1 change: 1 addition & 0 deletions test/IDE/complete_at_top_level.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ func resyncParser7() {}
var topLevelVar2 = FooStruct#^TOP_LEVEL_VAR_INIT_2^#
// TOP_LEVEL_VAR_INIT_2: Begin completions
// TOP_LEVEL_VAR_INIT_2-NEXT: Decl[InstanceMethod]/CurrNominal: .instanceFunc({#(self): FooStruct#})[#(Int) -> Void#]{{; name=.+$}}
// TOP_LEVEL_VAR_INIT_2-NEXT: Decl[Constructor]/CurrNominal: ()[#FooStruct#]{{; name=.+$}}
// TOP_LEVEL_VAR_INIT_2-NEXT: Decl[Constructor]/CurrNominal: ({#instanceVar: Int#})[#FooStruct#]{{; name=.+$}}
// TOP_LEVEL_VAR_INIT_2-NEXT: Decl[Constructor]/CurrNominal: ()[#FooStruct#]{{; name=.+$}}
// TOP_LEVEL_VAR_INIT_2-NEXT: Keyword[self]/CurrNominal: .self[#FooStruct.Type#]; name=self
Expand Down
3 changes: 2 additions & 1 deletion test/IDE/complete_constructor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ struct ImplicitConstructors2 {

func testImplicitConstructors2() {
ImplicitConstructors2#^IMPLICIT_CONSTRUCTORS_2^#
// IMPLICIT_CONSTRUCTORS_2: Begin completions, 4 items
// IMPLICIT_CONSTRUCTORS_2: Begin completions, 5 items
// IMPLICIT_CONSTRUCTORS_2-DAG: Decl[Constructor]/CurrNominal: ()[#ImplicitConstructors2#]{{; name=.+$}}
// IMPLICIT_CONSTRUCTORS_2-DAG: Decl[Constructor]/CurrNominal: ({#instanceVar: Int#})[#ImplicitConstructors2#]{{; name=.+$}}
// IMPLICIT_CONSTRUCTORS_2-DAG: Decl[Constructor]/CurrNominal: ()[#ImplicitConstructors2#]{{; name=.+$}}
// IMPLICIT_CONSTRUCTORS_2-DAG: Keyword[self]/CurrNominal: .self[#ImplicitConstructors2.Type#]; name=self
Expand Down
Loading