From 6096a6b52d60e1ee681f5a0e2752db3c662d60cd Mon Sep 17 00:00:00 2001 From: Rintaro Ishizaki Date: Wed, 17 Apr 2019 11:07:37 -0700 Subject: [PATCH 01/38] [CodeCompletion] Enable type completion at beginning of attribute for 'VarDecl' or if we don't know the kind of the decl. Property delegate allows arbitrary type name (with `@propertyDelegate` attr). --- lib/Parse/ParseDecl.cpp | 9 +++++++++ lib/Parse/ParsePattern.cpp | 13 +++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index e37e3587fd33f..93047a85b4332 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -2943,6 +2943,15 @@ Parser::parseDecl(ParseDeclOptions Flags, // Obvious nonsense. default: + if (FoundCCTokenInAttr) { + if (!CodeCompletion) { + delayParseFromBeginningToHere(BeginParserPosition, Flags); + } else { + CodeCompletion->completeDeclAttrBeginning(nullptr, isInSILMode(), + false); + } + } + diagnose(Tok, diag::expected_decl); if (CurDeclContext) { diff --git a/lib/Parse/ParsePattern.cpp b/lib/Parse/ParsePattern.cpp index f048145b10f3d..54ebb661eb81c 100644 --- a/lib/Parse/ParsePattern.cpp +++ b/lib/Parse/ParsePattern.cpp @@ -221,10 +221,15 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc, unsigned defaultArgIndex = defaultArgs ? defaultArgs->NextIndex++ : 0; // Attributes. - if (paramContext != ParameterContextKind::EnumElement) { - auto AttrStatus = parseDeclAttributeList(param.Attrs); - if (AttrStatus.hasCodeCompletion()) - status.setHasCodeCompletion(); + bool FoundCCToken = false; + if (paramContext != ParameterContextKind::EnumElement) + parseDeclAttributeList(param.Attrs, FoundCCToken); + if (FoundCCToken) { + if (CodeCompletion) { + CodeCompletion->completeDeclAttrBeginning(nullptr, isInSILMode(), true); + } else { + status |= makeParserCodeCompletionStatus(); + } } // ('inout' | 'let' | 'var' | '__shared' | '__owned')? From dec254c5e4ee98c62bb838e23d221202ae25454c Mon Sep 17 00:00:00 2001 From: Rintaro Ishizaki Date: Thu, 18 Apr 2019 16:00:40 -0700 Subject: [PATCH 02/38] [CodeCompletion] Implement completion at custom attribute argument --- lib/Parse/ParseDecl.cpp | 9 --------- lib/Parse/ParsePattern.cpp | 13 ++++--------- test/IDE/complete_decl_attribute.swift | 1 + 3 files changed, 5 insertions(+), 18 deletions(-) diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index 93047a85b4332..e37e3587fd33f 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -2943,15 +2943,6 @@ Parser::parseDecl(ParseDeclOptions Flags, // Obvious nonsense. default: - if (FoundCCTokenInAttr) { - if (!CodeCompletion) { - delayParseFromBeginningToHere(BeginParserPosition, Flags); - } else { - CodeCompletion->completeDeclAttrBeginning(nullptr, isInSILMode(), - false); - } - } - diagnose(Tok, diag::expected_decl); if (CurDeclContext) { diff --git a/lib/Parse/ParsePattern.cpp b/lib/Parse/ParsePattern.cpp index 54ebb661eb81c..f048145b10f3d 100644 --- a/lib/Parse/ParsePattern.cpp +++ b/lib/Parse/ParsePattern.cpp @@ -221,15 +221,10 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc, unsigned defaultArgIndex = defaultArgs ? defaultArgs->NextIndex++ : 0; // Attributes. - bool FoundCCToken = false; - if (paramContext != ParameterContextKind::EnumElement) - parseDeclAttributeList(param.Attrs, FoundCCToken); - if (FoundCCToken) { - if (CodeCompletion) { - CodeCompletion->completeDeclAttrBeginning(nullptr, isInSILMode(), true); - } else { - status |= makeParserCodeCompletionStatus(); - } + if (paramContext != ParameterContextKind::EnumElement) { + auto AttrStatus = parseDeclAttributeList(param.Attrs); + if (AttrStatus.hasCodeCompletion()) + status.setHasCodeCompletion(); } // ('inout' | 'let' | 'var' | '__shared' | '__owned')? diff --git a/test/IDE/complete_decl_attribute.swift b/test/IDE/complete_decl_attribute.swift index d3bdd5d7db4ac..2ec81d4978417 100644 --- a/test/IDE/complete_decl_attribute.swift +++ b/test/IDE/complete_decl_attribute.swift @@ -194,6 +194,7 @@ struct _S { // ON_MEMBER_LAST-DAG: Keyword/None: GKInspectable[#Declaration Attribute#]; name=GKInspectable // ON_MEMBER_LAST-DAG: Keyword/None: IBSegueAction[#Declaration Attribute#]; name=IBSegueAction // ON_MEMBER_LAST-DAG: Keyword/None: _propertyWrapper[#Declaration Attribute#]; name=_propertyWrapper +// ON_MEMBER_LAST-DAG: Keyword/None: _functionBuilder[#Declaration Attribute#]; name=_functionBuilder // ON_MEMBER_LAST-NOT: Keyword // ON_MEMBER_LAST: Decl[Struct]/CurrModule: MyStruct[#MyStruct#]; name=MyStruct // ON_MEMBER_LAST-NOT: Decl[PrecedenceGroup] From 08dc287fc26b3d078637a4906f435420bdf3797e Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 16 Apr 2019 01:34:05 -0400 Subject: [PATCH 03/38] Add @functionBuilder and check its applications to parameters. --- include/swift/AST/Attr.def | 3 + include/swift/AST/Decl.h | 8 +++ include/swift/AST/DiagnosticsSema.def | 19 ++++++ include/swift/AST/TypeCheckRequests.h | 27 +++++++++ include/swift/AST/TypeCheckerTypeIDZone.def | 1 + lib/AST/Decl.cpp | 24 ++++++++ lib/AST/TypeCheckRequests.cpp | 20 +++++++ lib/Sema/TypeCheckAttr.cpp | 64 +++++++++++++++++++++ lib/Sema/TypeCheckDeclOverride.cpp | 2 +- lib/Sema/TypeCheckRequestFunctions.cpp | 22 +++++++ test/IDE/complete_decl_attribute.swift | 7 ++- test/decl/var/function_builders.swift | 38 ++++++++++++ 12 files changed, 233 insertions(+), 2 deletions(-) create mode 100644 test/decl/var/function_builders.swift diff --git a/include/swift/AST/Attr.def b/include/swift/AST/Attr.def index 84cbc5459f134..d5efd885d3c1f 100644 --- a/include/swift/AST/Attr.def +++ b/include/swift/AST/Attr.def @@ -406,6 +406,9 @@ SIMPLE_DECL_ATTR(_propertyWrapper, PropertyWrapper, SIMPLE_DECL_ATTR(_disfavoredOverload, DisfavoredOverload, OnAbstractFunction | OnVar | OnSubscript | UserInaccessible, 87) +SIMPLE_DECL_ATTR(functionBuilder, FunctionBuilder, + OnNominalType, + 88) SIMPLE_DECL_ATTR(IBSegueAction, IBSegueAction, OnFunc, diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 5a437cf65c8a2..daae1bc737828 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -5309,6 +5309,14 @@ class ParamDecl : public VarDecl { assert(isVariadic()); return getVarargBaseTy(getInterfaceType()); } + + /// Retrieve the attribute marking this as a function builder parameter, + /// if there is one. + CustomAttr *getAttachedFunctionBuilder() const; + + /// Retrieve the @functionBuilder type attached to this parameter, + /// if there is one. + NominalTypeDecl *getFunctionBuilderType() const; SourceRange getSourceRange() const; diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index adf79ee540ac3..618007e852e57 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4441,6 +4441,25 @@ ERROR(property_wrapper_type_not_usable_from_inline,none, "must be '@usableFromInline' or public", (bool, bool)) +//------------------------------------------------------------------------------ +// MARK: function builder diagnostics +//------------------------------------------------------------------------------ +ERROR(function_builder_reserved_name, none, + "function builder type name must start with an uppercase letter", ()) +ERROR(function_builder_attribute_not_on_parameter, none, + "function builder attribute %0 can only be applied to a parameter", + (DeclName)) +ERROR(function_builder_parameter_not_of_function_type, none, + "function builder attribute %0 can only be applied to a parameter of " + "function type", + (DeclName)) +ERROR(function_builder_multiple, none, + "only one function builder attribute can be attached to a parameter", ()) +NOTE(previous_function_builder_here, none, + "previous function builder specified here", ()) +ERROR(function_builder_arguments, none, + "function builder attributes cannot have arguments", ()) + #ifndef DIAG_NO_UNDEF # if defined(DIAG) # undef DIAG diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index 38ad44b64d721..16ac52e1f75fe 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -30,6 +30,7 @@ namespace swift { class GenericParamList; +class ParamDecl; struct PropertyWrapperBackingPropertyInfo; class RequirementRepr; class SpecializeAttr; @@ -521,6 +522,32 @@ class PropertyWrapperBackingPropertyInfoRequest : void noteCycleStep(DiagnosticEngine &diags) const; }; +/// Request the custom attribute which attaches a function builder to the +/// given parameter. +class AttachedFunctionBuilderRequest : + public SimpleRequest { +public: + using SimpleRequest::SimpleRequest; + +private: + friend SimpleRequest; + + // Evaluation. + llvm::Expected + evaluate(Evaluator &evaluator, ParamDecl *) const; + +public: + // Caching + bool isCached() const; + + // Cycle handling + void diagnoseCycle(DiagnosticEngine &diags) const; + void noteCycleStep(DiagnosticEngine &diags) const; +}; + // Allow AnyValue to compare two Type values, even though Type doesn't // support ==. template<> diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def index 4bb3228be1a69..6090b34cc08ec 100644 --- a/include/swift/AST/TypeCheckerTypeIDZone.def +++ b/include/swift/AST/TypeCheckerTypeIDZone.def @@ -29,3 +29,4 @@ SWIFT_TYPEID(AttachedPropertyWrapperRequest) SWIFT_TYPEID(AttachedPropertyWrapperTypeRequest) SWIFT_TYPEID(PropertyWrapperBackingPropertyTypeRequest) SWIFT_TYPEID(PropertyWrapperBackingPropertyInfoRequest) +SWIFT_TYPEID(AttachedFunctionBuilderRequest) diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 8240f8f2670c1..0b6a07757635b 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -5649,6 +5649,30 @@ void ParamDecl::setStoredProperty(VarDecl *var) { DefaultValueAndFlags.getPointer()->DefaultArg = var; } +NominalTypeDecl *ParamDecl::getFunctionBuilderType() const { + auto attr = getAttachedFunctionBuilder(); + if (!attr) return nullptr; + + auto mutableAttr = const_cast(attr); + auto &ctx = getASTContext(); + auto dc = getDeclContext(); + return evaluateOrDefault(ctx.evaluator, + CustomAttrNominalRequest{mutableAttr, dc}, + nullptr); +} + +CustomAttr *ParamDecl::getAttachedFunctionBuilder() const { + // Fast path: most parameters do not have any attributes at all, + // much less custom ones. + if (!getAttrs().hasAttribute()) return nullptr; + + auto &ctx = getASTContext(); + auto mutableThis = const_cast(this); + return evaluateOrDefault(ctx.evaluator, + AttachedFunctionBuilderRequest{mutableThis}, + nullptr); +} + void ParamDecl::setDefaultArgumentInitContext(Initializer *initContext) { assert(DefaultValueAndFlags.getPointer()); DefaultValueAndFlags.getPointer()->InitContext = initContext; diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp index 0c343f0922513..d053ec858a2cd 100644 --- a/lib/AST/TypeCheckRequests.cpp +++ b/lib/AST/TypeCheckRequests.cpp @@ -627,3 +627,23 @@ void swift::simple_display(llvm::raw_ostream &out, const Type &type) { else out << "null"; } + +//----------------------------------------------------------------------------// +// Finding the attached @functionBuilder for a parameter. +//----------------------------------------------------------------------------// + +bool AttachedFunctionBuilderRequest::isCached() const { + // Only needs to be cached if there are any custom attributes. + auto var = std::get<0>(getStorage()); + return var->getAttrs().hasAttribute(); +} + +void AttachedFunctionBuilderRequest::diagnoseCycle( + DiagnosticEngine &diags) const { + std::get<0>(getStorage())->diagnose(diag::circular_reference); +} + +void AttachedFunctionBuilderRequest::noteCycleStep( + DiagnosticEngine &diags) const { + std::get<0>(getStorage())->diagnose(diag::circular_reference_through); +} diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 56b493551c595..2c8f4ef4a9263 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -132,6 +132,7 @@ class AttributeEarlyChecker : public AttributeVisitor { IGNORED_ATTR(Custom) IGNORED_ATTR(PropertyWrapper) IGNORED_ATTR(DisfavoredOverload) + IGNORED_ATTR(FunctionBuilder) #undef IGNORED_ATTR void visitAlignmentAttr(AlignmentAttr *attr) { @@ -857,6 +858,7 @@ class AttributeChecker : public AttributeVisitor { void visitNonOverrideAttr(NonOverrideAttr *attr); void visitCustomAttr(CustomAttr *attr); void visitPropertyWrapperAttr(PropertyWrapperAttr *attr); + void visitFunctionBuilderAttr(FunctionBuilderAttr *attr); }; } // end anonymous namespace @@ -2573,6 +2575,48 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) { return; } + // If the nominal type is a function builder type, verify that D is a + // parameter of function type. + if (nominal->getAttrs().hasAttribute()) { + auto param = dyn_cast(D); + if (!param) { + TC.diagnose(attr->getLocation(), + diag::function_builder_attribute_not_on_parameter, + nominal->getFullName()); + attr->setInvalid(); + return; + } + + // If we can't resolve an interface type here, we should be in invalid + // code. + Type type = param->getInterfaceType(); + if (type && !type->is()) { + TC.diagnose(attr->getLocation(), + diag::function_builder_parameter_not_of_function_type, + nominal->getFullName()); + attr->setInvalid(); + return; + } + + // Diagnose and ignore arguments. + if (attr->getArg()) { + TC.diagnose(attr->getLocation(), diag::function_builder_arguments) + .highlight(attr->getArg()->getSourceRange()); + } + + // Complain if this isn't the primary function-builder attribute. + auto attached = param->getAttachedFunctionBuilder(); + if (attached != attr) { + TC.diagnose(attr->getLocation(), diag::function_builder_multiple); + TC.diagnose(attached->getLocation(), + diag::previous_function_builder_here); + attr->setInvalid(); + return; + } + + return; + } + TC.diagnose(attr->getLocation(), diag::nominal_type_not_attribute, nominal->getDescriptiveKind(), nominal->getFullName()); nominal->diagnose(diag::decl_declared_here, nominal->getFullName()); @@ -2588,6 +2632,26 @@ void AttributeChecker::visitPropertyWrapperAttr(PropertyWrapperAttr *attr) { (void)nominal->getPropertyWrapperTypeInfo(); } +void AttributeChecker::visitFunctionBuilderAttr(FunctionBuilderAttr *attr) { + auto nominal = dyn_cast(D); + if (!nominal) + return; + + // Make sure the name isn't reserved. + if (isReservedAttributeName(nominal->getName().str())) { + nominal->diagnose(diag::function_builder_reserved_name); + } + + // TODO: check that the type at least provides a `sequence` factory? + // Any other validation? +} + +void TypeChecker::checkParameterAttributes(ParameterList *params) { + for (auto param: *params) { + checkDeclAttributes(param); + } +} + void TypeChecker::checkDeclAttributes(Decl *D) { AttributeChecker Checker(*this, D); diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp index 9c010bb64cc66..fa71ba86be680 100644 --- a/lib/Sema/TypeCheckDeclOverride.cpp +++ b/lib/Sema/TypeCheckDeclOverride.cpp @@ -1329,7 +1329,7 @@ namespace { UNINTERESTING_ATTR(Custom) UNINTERESTING_ATTR(PropertyWrapper) UNINTERESTING_ATTR(DisfavoredOverload) - + UNINTERESTING_ATTR(FunctionBuilder) #undef UNINTERESTING_ATTR void visitAvailableAttr(AvailableAttr *attr) { diff --git a/lib/Sema/TypeCheckRequestFunctions.cpp b/lib/Sema/TypeCheckRequestFunctions.cpp index d74e026474000..3a2b07c454c71 100644 --- a/lib/Sema/TypeCheckRequestFunctions.cpp +++ b/lib/Sema/TypeCheckRequestFunctions.cpp @@ -12,9 +12,11 @@ #include "TypeChecker.h" #include "TypeCheckType.h" #include "swift/AST/PropertyWrappers.h" +#include "swift/AST/Attr.h" #include "swift/AST/TypeCheckRequests.h" #include "swift/AST/Decl.h" #include "swift/AST/ExistentialLayout.h" +#include "swift/AST/NameLookupRequests.h" #include "swift/AST/TypeLoc.h" #include "swift/AST/Types.h" #include "swift/Subsystems.h" @@ -146,6 +148,26 @@ EnumRawTypeRequest::evaluate(Evaluator &evaluator, EnumDecl *enumDecl, return Type(); } +llvm::Expected +AttachedFunctionBuilderRequest::evaluate(Evaluator &evaluator, + ParamDecl *param) const { + ASTContext &ctx = param->getASTContext(); + auto dc = param->getDeclContext(); + for (auto attr : param->getAttrs().getAttributes()) { + auto mutableAttr = const_cast(attr); + // Figure out which nominal declaration this custom attribute refers to. + auto nominal = evaluateOrDefault(ctx.evaluator, + CustomAttrNominalRequest{mutableAttr, dc}, + nullptr); + + // Return the first custom attribute that is a function builder type. + if (nominal->getAttrs().hasAttribute()) + return mutableAttr; + } + + return nullptr; +} + // Define request evaluation functions for each of the type checker requests. static AbstractRequestFunction *typeCheckerRequestFunctions[] = { #define SWIFT_TYPEID(Name) \ diff --git a/test/IDE/complete_decl_attribute.swift b/test/IDE/complete_decl_attribute.swift index 2ec81d4978417..8415b875ff557 100644 --- a/test/IDE/complete_decl_attribute.swift +++ b/test/IDE/complete_decl_attribute.swift @@ -75,6 +75,7 @@ class C {} // KEYWORD3-NEXT: Keyword/None: NSApplicationMain[#Class Attribute#]; name=NSApplicationMain{{$}} // KEYWORD3-NEXT: Keyword/None: usableFromInline[#Class Attribute#]; name=usableFromInline // KEYWORD3-NEXT: Keyword/None: _propertyWrapper[#Class Attribute#]; name=_propertyWrapper +// KEYWORD3-NEXT: Keyword/None: functionBuilder[#Class Attribute#]; name=functionBuilder // KEYWORD3-NEXT: End completions @#^KEYWORD3_2^#IB @@ -90,6 +91,7 @@ enum E {} // KEYWORD4-NEXT: Keyword/None: dynamicMemberLookup[#Enum Attribute#]; name=dynamicMemberLookup // KEYWORD4-NEXT: Keyword/None: usableFromInline[#Enum Attribute#]; name=usableFromInline // KEYWORD4-NEXT: Keyword/None: _propertyWrapper[#Enum Attribute#]; name=_propertyWrapper +// KEYWORD4-NEXT: Keyword/None: functionBuilder[#Enum Attribute#]; name=functionBuilder // KEYWORD4-NEXT: End completions @@ -101,6 +103,7 @@ struct S{} // KEYWORD5-NEXT: Keyword/None: dynamicMemberLookup[#Struct Attribute#]; name=dynamicMemberLookup // KEYWORD5-NEXT: Keyword/None: usableFromInline[#Struct Attribute#]; name=usableFromInline // KEYWORD5-NEXT: Keyword/None: _propertyWrapper[#Struct Attribute#]; name=_propertyWrapper +// KEYWORD5-NEXT: Keyword/None: functionBuilder[#Struct Attribute#]; name=functionBuilder // KEYWORD5-NEXT: End completions @#^ON_GLOBALVAR^# @@ -194,7 +197,7 @@ struct _S { // ON_MEMBER_LAST-DAG: Keyword/None: GKInspectable[#Declaration Attribute#]; name=GKInspectable // ON_MEMBER_LAST-DAG: Keyword/None: IBSegueAction[#Declaration Attribute#]; name=IBSegueAction // ON_MEMBER_LAST-DAG: Keyword/None: _propertyWrapper[#Declaration Attribute#]; name=_propertyWrapper -// ON_MEMBER_LAST-DAG: Keyword/None: _functionBuilder[#Declaration Attribute#]; name=_functionBuilder +// ON_MEMBER_LAST-DAG: Keyword/None: functionBuilder[#Declaration Attribute#]; name=functionBuilder // ON_MEMBER_LAST-NOT: Keyword // ON_MEMBER_LAST: Decl[Struct]/CurrModule: MyStruct[#MyStruct#]; name=MyStruct // ON_MEMBER_LAST-NOT: Decl[PrecedenceGroup] @@ -227,5 +230,7 @@ struct _S { // KEYWORD_LAST-NEXT: Keyword/None: GKInspectable[#Declaration Attribute#]; name=GKInspectable{{$}} // KEYWORD_LAST-NEXT: Keyword/None: _propertyWrapper[#Declaration Attribute#]; name=_propertyWrapper // KEYWORD_LAST-NEXT: Keyword/None: IBSegueAction[#Declaration Attribute#]; name=IBSegueAction{{$}} +// KEYWORD_LAST-NEXT: Keyword/None: functionBuilder[#Declaration Attribute#]; name=functionBuilder{{$}} +// KEYWORD_LAST-NOT: Keyword // KEYWORD_LAST: Decl[Struct]/CurrModule: MyStruct[#MyStruct#]; name=MyStruct // KEYWORD_LAST: End completions diff --git a/test/decl/var/function_builders.swift b/test/decl/var/function_builders.swift new file mode 100644 index 0000000000000..1760b051d61be --- /dev/null +++ b/test/decl/var/function_builders.swift @@ -0,0 +1,38 @@ +// RUN: %target-typecheck-verify-swift + +@functionBuilder // expected-error {{'@functionBuilder' attribute cannot be applied to this declaration}} +var globalBuilder: Int + +@functionBuilder // expected-error {{'@functionBuilder' attribute cannot be applied to this declaration}} +func globalBuilderFunction() -> Int { return 0 } + +@functionBuilder +struct Maker {} + +@functionBuilder +class Inventor {} + +@Maker // expected-error {{function builder attribute 'Maker' can only be applied to a parameter}} +var global: Int + +@Maker // expected-error {{function builder attribute 'Maker' can only be applied to a parameter}} +func globalFunction() {} + +@Maker // expected-error {{function builder attribute 'Maker' can only be applied to a parameter}} +func globalFunctionWithFunctionParam(fn: () -> ()) {} + +func makerParam(@Maker + fn: () -> ()) {} + +// FIXME: these diagnostics are reversed? +func makerParamRedundant(@Maker // expected-error {{only one function builder attribute can be attached to a parameter}} + @Maker // expected-note {{previous function builder specified here}} + fn: () -> ()) {} + +func makerParamConflict(@Maker // expected-error {{only one function builder attribute can be attached to a parameter}} + @Inventor // expected-note {{previous function builder specified here}} + fn: () -> ()) {} + +func makerParamExtra(@Maker(5) // expected-error {{function builder attributes cannot have arguments}} + fn: () -> ()) {} + From c5b3af0d1e9bc118bed9b6306bdc934eae3f7491 Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 16 Apr 2019 17:23:10 -0400 Subject: [PATCH 04/38] Ignore unresolvable custom attributes in the function-builder checker. --- lib/Sema/TypeCheckRequestFunctions.cpp | 4 ++++ test/decl/var/function_builders.swift | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/lib/Sema/TypeCheckRequestFunctions.cpp b/lib/Sema/TypeCheckRequestFunctions.cpp index 3a2b07c454c71..ba7da17390b80 100644 --- a/lib/Sema/TypeCheckRequestFunctions.cpp +++ b/lib/Sema/TypeCheckRequestFunctions.cpp @@ -160,6 +160,10 @@ AttachedFunctionBuilderRequest::evaluate(Evaluator &evaluator, CustomAttrNominalRequest{mutableAttr, dc}, nullptr); + // Ignore unresolvable custom attributes. + if (!nominal) + continue; + // Return the first custom attribute that is a function builder type. if (nominal->getAttrs().hasAttribute()) return mutableAttr; diff --git a/test/decl/var/function_builders.swift b/test/decl/var/function_builders.swift index 1760b051d61be..6d1301d4a60aa 100644 --- a/test/decl/var/function_builders.swift +++ b/test/decl/var/function_builders.swift @@ -33,6 +33,14 @@ func makerParamConflict(@Maker // expected-error {{only one function builder att @Inventor // expected-note {{previous function builder specified here}} fn: () -> ()) {} +func makerParamMissing1(@Missing // expected-error {{unknown attribute 'Missing'}} + @Maker + fn: () -> ()) {} + +func makerParamMissing2(@Maker + @Missing // expected-error {{unknown attribute 'Missing'}} + fn: () -> ()) {} + func makerParamExtra(@Maker(5) // expected-error {{function builder attributes cannot have arguments}} fn: () -> ()) {} From d2fbd1e61ad9c9bef4d99848116d817f183e4a68 Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 16 Apr 2019 19:20:03 -0400 Subject: [PATCH 05/38] Add an unused request for getting the type of a custom attribute. Includes code anticipating property delegates in part because it's been largely stolen from Doug's PR for that. --- include/swift/AST/ASTTypeIDs.h | 2 + include/swift/AST/TypeCheckRequests.h | 44 ++++++++++++++++++++- include/swift/AST/TypeCheckerTypeIDZone.def | 1 + lib/AST/TypeCheckRequests.cpp | 30 +++++++++++++- lib/Sema/TypeCheckRequestFunctions.cpp | 40 +++++++++++++++++++ 5 files changed, 114 insertions(+), 3 deletions(-) diff --git a/include/swift/AST/ASTTypeIDs.h b/include/swift/AST/ASTTypeIDs.h index 7b1833a44f265..1a7b06b6ee775 100644 --- a/include/swift/AST/ASTTypeIDs.h +++ b/include/swift/AST/ASTTypeIDs.h @@ -26,6 +26,8 @@ struct PropertyWrapperBackingPropertyInfo; struct PropertyWrapperTypeInfo; class Type; class VarDecl; +class TypeAliasDecl; +class Type; #define SWIFT_AST_TYPEID_ZONE 1 diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index 16ac52e1f75fe..d0cc17bd1b20c 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -548,12 +548,54 @@ class AttachedFunctionBuilderRequest : void noteCycleStep(DiagnosticEngine &diags) const; }; +/// Kinds of types for CustomAttr. +enum class CustomAttrTypeKind { + /// The type is required to not be expressed in terms of + /// any contextual type parameters. + NonGeneric, + + /// Property delegates have some funky rules, like allowing + /// unbound generic types. + PropertyDelegate, +}; + +void simple_display(llvm::raw_ostream &out, CustomAttrTypeKind value); + +/// Request the type spelled out in a custom attribute. +/// +/// Different parameters cannot be used for the same attribute. +class CustomAttrTypeRequest : + public SimpleRequest { +public: + using SimpleRequest::SimpleRequest; + +private: + friend SimpleRequest; + + llvm::Expected + evaluate(Evaluator &evaluator, CustomAttr *attr, DeclContext *dc, + CustomAttrTypeKind typeKind) const; + +public: + // Caching + bool isCached() const { return true; } + + // Cycle handling + void diagnoseCycle(DiagnosticEngine &diags) const; + void noteCycleStep(DiagnosticEngine &diags) const; +}; + // Allow AnyValue to compare two Type values, even though Type doesn't // support ==. template<> bool AnyValue::Holder::equals(const HolderBase &other) const; -void simple_display(llvm::raw_ostream &out, const Type &type); +void simple_display(llvm::raw_ostream &out, Type value); /// The zone number for the type checker. #define SWIFT_TYPE_CHECKER_REQUESTS_TYPEID_ZONE 10 diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def index 6090b34cc08ec..426ec9518733d 100644 --- a/include/swift/AST/TypeCheckerTypeIDZone.def +++ b/include/swift/AST/TypeCheckerTypeIDZone.def @@ -30,3 +30,4 @@ SWIFT_TYPEID(AttachedPropertyWrapperTypeRequest) SWIFT_TYPEID(PropertyWrapperBackingPropertyTypeRequest) SWIFT_TYPEID(PropertyWrapperBackingPropertyInfoRequest) SWIFT_TYPEID(AttachedFunctionBuilderRequest) +SWIFT_TYPEID(CustomAttrTypeRequest) diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp index d053ec858a2cd..500e27d36ac33 100644 --- a/lib/AST/TypeCheckRequests.cpp +++ b/lib/AST/TypeCheckRequests.cpp @@ -613,7 +613,6 @@ void swift::simple_display( out << " }"; } - template<> bool swift::AnyValue::Holder::equals(const HolderBase &other) const { assert(typeID == other.typeID && "Caller should match type IDs"); @@ -621,13 +620,40 @@ bool swift::AnyValue::Holder::equals(const HolderBase &other) const { static_cast &>(other).value.getPointer(); } -void swift::simple_display(llvm::raw_ostream &out, const Type &type) { +void swift::simple_display(llvm::raw_ostream &out, Type type) { if (type) type.print(out); else out << "null"; } +//----------------------------------------------------------------------------// +// CustomAttrTypeRequest. +//----------------------------------------------------------------------------// + +void CustomAttrTypeRequest::diagnoseCycle(DiagnosticEngine &diags) const { + auto attr = std::get<0>(getStorage()); + diags.diagnose(attr->getLocation(), diag::circular_reference); +} + +void CustomAttrTypeRequest::noteCycleStep(DiagnosticEngine &diags) const { + auto attr = std::get<0>(getStorage()); + diags.diagnose(attr->getLocation(), diag::circular_reference_through); +} + +void swift::simple_display(llvm::raw_ostream &out, CustomAttrTypeKind value) { + switch (value) { + case CustomAttrTypeKind::NonGeneric: + out << "non-generic"; + return; + + case CustomAttrTypeKind::PropertyDelegate: + out << "property-delegate"; + return; + } + llvm_unreachable("bad kind"); +} + //----------------------------------------------------------------------------// // Finding the attached @functionBuilder for a parameter. //----------------------------------------------------------------------------// diff --git a/lib/Sema/TypeCheckRequestFunctions.cpp b/lib/Sema/TypeCheckRequestFunctions.cpp index ba7da17390b80..5c08f6a55bd92 100644 --- a/lib/Sema/TypeCheckRequestFunctions.cpp +++ b/lib/Sema/TypeCheckRequestFunctions.cpp @@ -172,6 +172,46 @@ AttachedFunctionBuilderRequest::evaluate(Evaluator &evaluator, return nullptr; } +llvm::Expected +CustomAttrTypeRequest::evaluate(Evaluator &evaluator, + CustomAttr *attr, DeclContext *dc, + CustomAttrTypeKind typeKind) const { + auto resolution = TypeResolution::forContextual(dc); + TypeResolutionOptions options(TypeResolverContext::PatternBindingDecl); + + // Property delegates allow their type to be an unbound generic. + if (typeKind == CustomAttrTypeKind::PropertyDelegate) + options |= TypeResolutionFlags::AllowUnboundGenerics; + + ASTContext &ctx = dc->getASTContext(); + auto &tc = *static_cast(ctx.getLazyResolver()); + if (tc.validateType(attr->getTypeLoc(), resolution, options)) + return ErrorType::get(ctx); + + // We always require the type to resolve to a nominal type. + Type type = attr->getTypeLoc().getType(); + if (!type->getAnyNominal()) { + assert(ctx.Diags.hadAnyError()); + return ErrorType::get(ctx); + } + + switch (typeKind) { + case CustomAttrTypeKind::NonGeneric: + if (type->hasArchetype()) { + ctx.Diags.diagnose(attr->getLocation(), + diag::function_builder_type_contextual, type); + return ErrorType::get(ctx); + } + break; + + case CustomAttrTypeKind::PropertyDelegate: + // No further logic required here. + break; + } + + return type; +} + // Define request evaluation functions for each of the type checker requests. static AbstractRequestFunction *typeCheckerRequestFunctions[] = { #define SWIFT_TYPEID(Name) \ From 137b90cf5a6cc7e37e6778b044085fff7b2b249b Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 16 Apr 2019 19:22:06 -0400 Subject: [PATCH 06/38] Make getFunctionBuilderType() return a Type. --- include/swift/AST/Decl.h | 2 +- include/swift/AST/DiagnosticsSema.def | 3 +++ lib/AST/Decl.cpp | 9 +++++---- lib/Sema/TypeCheckAttr.cpp | 4 ++++ test/decl/var/function_builders.swift | 25 +++++++++++++++++++++++++ 5 files changed, 38 insertions(+), 5 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index daae1bc737828..e1a1d980b04f0 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -5316,7 +5316,7 @@ class ParamDecl : public VarDecl { /// Retrieve the @functionBuilder type attached to this parameter, /// if there is one. - NominalTypeDecl *getFunctionBuilderType() const; + Type getFunctionBuilderType() const; SourceRange getSourceRange() const; diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 618007e852e57..5a855443d37d2 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4459,6 +4459,9 @@ NOTE(previous_function_builder_here, none, "previous function builder specified here", ()) ERROR(function_builder_arguments, none, "function builder attributes cannot have arguments", ()) +ERROR(function_builder_type_contextual, none, + "function builder type %0 cannot depend on the current generic context", + (Type)) #ifndef DIAG_NO_UNDEF # if defined(DIAG) diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 0b6a07757635b..6b7af13f34283 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -5649,16 +5649,17 @@ void ParamDecl::setStoredProperty(VarDecl *var) { DefaultValueAndFlags.getPointer()->DefaultArg = var; } -NominalTypeDecl *ParamDecl::getFunctionBuilderType() const { +Type ParamDecl::getFunctionBuilderType() const { auto attr = getAttachedFunctionBuilder(); - if (!attr) return nullptr; + if (!attr) return Type(); auto mutableAttr = const_cast(attr); auto &ctx = getASTContext(); auto dc = getDeclContext(); return evaluateOrDefault(ctx.evaluator, - CustomAttrNominalRequest{mutableAttr, dc}, - nullptr); + CustomAttrTypeRequest{mutableAttr, dc, + CustomAttrTypeKind::NonGeneric}, + Type()); } CustomAttr *ParamDecl::getAttachedFunctionBuilder() const { diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 2c8f4ef4a9263..9909418776635 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -2612,6 +2612,10 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) { diag::previous_function_builder_here); attr->setInvalid(); return; + } else { + // Force any diagnostics associated with computing the function-builder + // type. + (void) param->getFunctionBuilderType(); } return; diff --git a/test/decl/var/function_builders.swift b/test/decl/var/function_builders.swift index 6d1301d4a60aa..1ca54303ba575 100644 --- a/test/decl/var/function_builders.swift +++ b/test/decl/var/function_builders.swift @@ -44,3 +44,28 @@ func makerParamMissing2(@Maker func makerParamExtra(@Maker(5) // expected-error {{function builder attributes cannot have arguments}} fn: () -> ()) {} +@functionBuilder +struct GenericMaker {} // expected-note {{generic type 'GenericMaker' declared here}} + +struct GenericContainer { // expected-note {{generic type 'GenericContainer' declared here}} + @functionBuilder + struct Maker {} +} + +func makeParamUnbound(@GenericMaker // expected-error {{reference to generic type 'GenericMaker' requires arguments}} + fn: () -> ()) {} + +func makeParamBound(@GenericMaker + fn: () -> ()) {} + +func makeParamNestedUnbound(@GenericContainer.Maker // expected-error {{reference to generic type 'GenericContainer' requires arguments}} + fn: () -> ()) {} + +func makeParamNestedBound(@GenericContainer.Maker + fn: () -> ()) {} + + +struct WithinGeneric { + func makeParamBoundInContext(@GenericMaker // expected-error {{function builder type 'GenericMaker' cannot depend on the current generic context}} + fn: () -> ()) {} +} From a7b16ae1d94b648cd2d90702299f4427434219ee Mon Sep 17 00:00:00 2001 From: John McCall Date: Wed, 17 Apr 2019 15:42:59 -0400 Subject: [PATCH 07/38] Underscore _functionBuilder. --- include/swift/AST/Attr.def | 2 +- test/IDE/complete_decl_attribute.swift | 10 +++++----- test/decl/var/function_builders.swift | 12 ++++++------ 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/include/swift/AST/Attr.def b/include/swift/AST/Attr.def index d5efd885d3c1f..684bd5281f324 100644 --- a/include/swift/AST/Attr.def +++ b/include/swift/AST/Attr.def @@ -406,7 +406,7 @@ SIMPLE_DECL_ATTR(_propertyWrapper, PropertyWrapper, SIMPLE_DECL_ATTR(_disfavoredOverload, DisfavoredOverload, OnAbstractFunction | OnVar | OnSubscript | UserInaccessible, 87) -SIMPLE_DECL_ATTR(functionBuilder, FunctionBuilder, +SIMPLE_DECL_ATTR(_functionBuilder, FunctionBuilder, OnNominalType, 88) diff --git a/test/IDE/complete_decl_attribute.swift b/test/IDE/complete_decl_attribute.swift index 8415b875ff557..9e0f6842e9d37 100644 --- a/test/IDE/complete_decl_attribute.swift +++ b/test/IDE/complete_decl_attribute.swift @@ -75,7 +75,7 @@ class C {} // KEYWORD3-NEXT: Keyword/None: NSApplicationMain[#Class Attribute#]; name=NSApplicationMain{{$}} // KEYWORD3-NEXT: Keyword/None: usableFromInline[#Class Attribute#]; name=usableFromInline // KEYWORD3-NEXT: Keyword/None: _propertyWrapper[#Class Attribute#]; name=_propertyWrapper -// KEYWORD3-NEXT: Keyword/None: functionBuilder[#Class Attribute#]; name=functionBuilder +// KEYWORD3-NEXT: Keyword/None: _functionBuilder[#Class Attribute#]; name=_functionBuilder // KEYWORD3-NEXT: End completions @#^KEYWORD3_2^#IB @@ -91,7 +91,7 @@ enum E {} // KEYWORD4-NEXT: Keyword/None: dynamicMemberLookup[#Enum Attribute#]; name=dynamicMemberLookup // KEYWORD4-NEXT: Keyword/None: usableFromInline[#Enum Attribute#]; name=usableFromInline // KEYWORD4-NEXT: Keyword/None: _propertyWrapper[#Enum Attribute#]; name=_propertyWrapper -// KEYWORD4-NEXT: Keyword/None: functionBuilder[#Enum Attribute#]; name=functionBuilder +// KEYWORD4-NEXT: Keyword/None: _functionBuilder[#Enum Attribute#]; name=_functionBuilder // KEYWORD4-NEXT: End completions @@ -103,7 +103,7 @@ struct S{} // KEYWORD5-NEXT: Keyword/None: dynamicMemberLookup[#Struct Attribute#]; name=dynamicMemberLookup // KEYWORD5-NEXT: Keyword/None: usableFromInline[#Struct Attribute#]; name=usableFromInline // KEYWORD5-NEXT: Keyword/None: _propertyWrapper[#Struct Attribute#]; name=_propertyWrapper -// KEYWORD5-NEXT: Keyword/None: functionBuilder[#Struct Attribute#]; name=functionBuilder +// KEYWORD5-NEXT: Keyword/None: _functionBuilder[#Struct Attribute#]; name=_functionBuilder // KEYWORD5-NEXT: End completions @#^ON_GLOBALVAR^# @@ -197,7 +197,7 @@ struct _S { // ON_MEMBER_LAST-DAG: Keyword/None: GKInspectable[#Declaration Attribute#]; name=GKInspectable // ON_MEMBER_LAST-DAG: Keyword/None: IBSegueAction[#Declaration Attribute#]; name=IBSegueAction // ON_MEMBER_LAST-DAG: Keyword/None: _propertyWrapper[#Declaration Attribute#]; name=_propertyWrapper -// ON_MEMBER_LAST-DAG: Keyword/None: functionBuilder[#Declaration Attribute#]; name=functionBuilder +// ON_MEMBER_LAST-DAG: Keyword/None: _functionBuilder[#Declaration Attribute#]; name=_functionBuilder // ON_MEMBER_LAST-NOT: Keyword // ON_MEMBER_LAST: Decl[Struct]/CurrModule: MyStruct[#MyStruct#]; name=MyStruct // ON_MEMBER_LAST-NOT: Decl[PrecedenceGroup] @@ -230,7 +230,7 @@ struct _S { // KEYWORD_LAST-NEXT: Keyword/None: GKInspectable[#Declaration Attribute#]; name=GKInspectable{{$}} // KEYWORD_LAST-NEXT: Keyword/None: _propertyWrapper[#Declaration Attribute#]; name=_propertyWrapper // KEYWORD_LAST-NEXT: Keyword/None: IBSegueAction[#Declaration Attribute#]; name=IBSegueAction{{$}} -// KEYWORD_LAST-NEXT: Keyword/None: functionBuilder[#Declaration Attribute#]; name=functionBuilder{{$}} +// KEYWORD_LAST-NEXT: Keyword/None: _functionBuilder[#Declaration Attribute#]; name=_functionBuilder{{$}} // KEYWORD_LAST-NOT: Keyword // KEYWORD_LAST: Decl[Struct]/CurrModule: MyStruct[#MyStruct#]; name=MyStruct // KEYWORD_LAST: End completions diff --git a/test/decl/var/function_builders.swift b/test/decl/var/function_builders.swift index 1ca54303ba575..823a94a94fd4e 100644 --- a/test/decl/var/function_builders.swift +++ b/test/decl/var/function_builders.swift @@ -1,15 +1,15 @@ // RUN: %target-typecheck-verify-swift -@functionBuilder // expected-error {{'@functionBuilder' attribute cannot be applied to this declaration}} +@_functionBuilder // expected-error {{'@_functionBuilder' attribute cannot be applied to this declaration}} var globalBuilder: Int -@functionBuilder // expected-error {{'@functionBuilder' attribute cannot be applied to this declaration}} +@_functionBuilder // expected-error {{'@_functionBuilder' attribute cannot be applied to this declaration}} func globalBuilderFunction() -> Int { return 0 } -@functionBuilder +@_functionBuilder struct Maker {} -@functionBuilder +@_functionBuilder class Inventor {} @Maker // expected-error {{function builder attribute 'Maker' can only be applied to a parameter}} @@ -44,11 +44,11 @@ func makerParamMissing2(@Maker func makerParamExtra(@Maker(5) // expected-error {{function builder attributes cannot have arguments}} fn: () -> ()) {} -@functionBuilder +@_functionBuilder struct GenericMaker {} // expected-note {{generic type 'GenericMaker' declared here}} struct GenericContainer { // expected-note {{generic type 'GenericContainer' declared here}} - @functionBuilder + @_functionBuilder struct Maker {} } From 1ea00869cdddfe681a28908cc38b07efeadecf55 Mon Sep 17 00:00:00 2001 From: John McCall Date: Wed, 17 Apr 2019 17:39:28 -0400 Subject: [PATCH 08/38] Rework the requests for getting a parameter's function-builder type. Turn the generic CustomAttrTypeRequest into a helper function and introduce a FunctionBuilderTypeRequest that starts from a ParamDecl. This has better caching characteristics and also means we only need to do a single cache lookup in order to resolve the type in the normal path. It also means we don't need as much parameterization in the cache. In addition, check that the parameter has function type in the request, not just when late-checking the attribute, and add a check that it isn't an autoclosure. --- include/swift/AST/DiagnosticsSema.def | 4 + include/swift/AST/TypeCheckRequests.h | 24 +----- include/swift/AST/TypeCheckerTypeIDZone.def | 2 +- lib/AST/Decl.cpp | 11 ++- lib/AST/TypeCheckRequests.cpp | 38 ++++++---- lib/Sema/TypeCheckAttr.cpp | 11 --- lib/Sema/TypeCheckRequestFunctions.cpp | 73 +++++++++++-------- lib/Sema/TypeCheckType.cpp | 24 ++++++ lib/Sema/TypeCheckType.h | 15 ++++ test/decl/var/function_builders.swift | 3 + .../Inputs/function_builder_definition.swift | 26 +++++++ .../function_builder_multifile.swift | 9 +++ 12 files changed, 157 insertions(+), 83 deletions(-) create mode 100644 test/multifile/Inputs/function_builder_definition.swift create mode 100644 test/multifile/function_builder_multifile.swift diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 5a855443d37d2..851b467da715f 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4453,6 +4453,10 @@ ERROR(function_builder_parameter_not_of_function_type, none, "function builder attribute %0 can only be applied to a parameter of " "function type", (DeclName)) +ERROR(function_builder_parameter_autoclosure, none, + "function builder attribute %0 cannot be applied to an autoclosure " + "parameter", + (DeclName)) ERROR(function_builder_multiple, none, "only one function builder attribute can be attached to a parameter", ()) NOTE(previous_function_builder_here, none, diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index d0cc17bd1b20c..e34dea56ef10c 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -548,29 +548,14 @@ class AttachedFunctionBuilderRequest : void noteCycleStep(DiagnosticEngine &diags) const; }; -/// Kinds of types for CustomAttr. -enum class CustomAttrTypeKind { - /// The type is required to not be expressed in terms of - /// any contextual type parameters. - NonGeneric, - - /// Property delegates have some funky rules, like allowing - /// unbound generic types. - PropertyDelegate, -}; - -void simple_display(llvm::raw_ostream &out, CustomAttrTypeKind value); - /// Request the type spelled out in a custom attribute. /// /// Different parameters cannot be used for the same attribute. -class CustomAttrTypeRequest : - public SimpleRequest { + ParamDecl *> { public: using SimpleRequest::SimpleRequest; @@ -578,8 +563,7 @@ class CustomAttrTypeRequest : friend SimpleRequest; llvm::Expected - evaluate(Evaluator &evaluator, CustomAttr *attr, DeclContext *dc, - CustomAttrTypeKind typeKind) const; + evaluate(Evaluator &evaluator, ParamDecl *param) const; public: // Caching diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def index 426ec9518733d..be1f65852f9f2 100644 --- a/include/swift/AST/TypeCheckerTypeIDZone.def +++ b/include/swift/AST/TypeCheckerTypeIDZone.def @@ -30,4 +30,4 @@ SWIFT_TYPEID(AttachedPropertyWrapperTypeRequest) SWIFT_TYPEID(PropertyWrapperBackingPropertyTypeRequest) SWIFT_TYPEID(PropertyWrapperBackingPropertyInfoRequest) SWIFT_TYPEID(AttachedFunctionBuilderRequest) -SWIFT_TYPEID(CustomAttrTypeRequest) +SWIFT_TYPEID(FunctionBuilderTypeRequest) diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 6b7af13f34283..401461cbad2fc 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -5650,15 +5650,14 @@ void ParamDecl::setStoredProperty(VarDecl *var) { } Type ParamDecl::getFunctionBuilderType() const { - auto attr = getAttachedFunctionBuilder(); - if (!attr) return Type(); + // Fast path: most parameters do not have any attributes at all, + // much less custom ones. + if (!getAttrs().hasAttribute()) return Type(); - auto mutableAttr = const_cast(attr); auto &ctx = getASTContext(); - auto dc = getDeclContext(); + auto mutableThis = const_cast(this); return evaluateOrDefault(ctx.evaluator, - CustomAttrTypeRequest{mutableAttr, dc, - CustomAttrTypeKind::NonGeneric}, + FunctionBuilderTypeRequest{mutableThis}, Type()); } diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp index 500e27d36ac33..a8529e36d4291 100644 --- a/lib/AST/TypeCheckRequests.cpp +++ b/lib/AST/TypeCheckRequests.cpp @@ -59,6 +59,20 @@ void swift::simple_display(llvm::raw_ostream &out, } } +template<> +bool swift::AnyValue::Holder::equals(const HolderBase &other) const { + assert(typeID == other.typeID && "Caller should match type IDs"); + return value.getPointer() == + static_cast &>(other).value.getPointer(); +} + +void swift::simple_display(llvm::raw_ostream &out, Type type) { + if (type) + type.print(out); + else + out << "null"; +} + //----------------------------------------------------------------------------// // Inherited type computation. //----------------------------------------------------------------------------// @@ -613,20 +627,6 @@ void swift::simple_display( out << " }"; } -template<> -bool swift::AnyValue::Holder::equals(const HolderBase &other) const { - assert(typeID == other.typeID && "Caller should match type IDs"); - return value.getPointer() == - static_cast &>(other).value.getPointer(); -} - -void swift::simple_display(llvm::raw_ostream &out, Type type) { - if (type) - type.print(out); - else - out << "null"; -} - //----------------------------------------------------------------------------// // CustomAttrTypeRequest. //----------------------------------------------------------------------------// @@ -655,7 +655,7 @@ void swift::simple_display(llvm::raw_ostream &out, CustomAttrTypeKind value) { } //----------------------------------------------------------------------------// -// Finding the attached @functionBuilder for a parameter. +// FunctionBuilder-related requests. //----------------------------------------------------------------------------// bool AttachedFunctionBuilderRequest::isCached() const { @@ -673,3 +673,11 @@ void AttachedFunctionBuilderRequest::noteCycleStep( DiagnosticEngine &diags) const { std::get<0>(getStorage())->diagnose(diag::circular_reference_through); } + +void FunctionBuilderTypeRequest::diagnoseCycle(DiagnosticEngine &diags) const { + std::get<0>(getStorage())->diagnose(diag::circular_reference); +} + +void FunctionBuilderTypeRequest::noteCycleStep(DiagnosticEngine &diags) const { + std::get<0>(getStorage())->diagnose(diag::circular_reference_through); +} diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 9909418776635..dcbb81c7b9e50 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -2587,17 +2587,6 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) { return; } - // If we can't resolve an interface type here, we should be in invalid - // code. - Type type = param->getInterfaceType(); - if (type && !type->is()) { - TC.diagnose(attr->getLocation(), - diag::function_builder_parameter_not_of_function_type, - nominal->getFullName()); - attr->setInvalid(); - return; - } - // Diagnose and ignore arguments. if (attr->getArg()) { TC.diagnose(attr->getLocation(), diag::function_builder_arguments) diff --git a/lib/Sema/TypeCheckRequestFunctions.cpp b/lib/Sema/TypeCheckRequestFunctions.cpp index 5c08f6a55bd92..d51b52cc823d2 100644 --- a/lib/Sema/TypeCheckRequestFunctions.cpp +++ b/lib/Sema/TypeCheckRequestFunctions.cpp @@ -173,40 +173,53 @@ AttachedFunctionBuilderRequest::evaluate(Evaluator &evaluator, } llvm::Expected -CustomAttrTypeRequest::evaluate(Evaluator &evaluator, - CustomAttr *attr, DeclContext *dc, - CustomAttrTypeKind typeKind) const { - auto resolution = TypeResolution::forContextual(dc); - TypeResolutionOptions options(TypeResolverContext::PatternBindingDecl); - - // Property delegates allow their type to be an unbound generic. - if (typeKind == CustomAttrTypeKind::PropertyDelegate) - options |= TypeResolutionFlags::AllowUnboundGenerics; - - ASTContext &ctx = dc->getASTContext(); - auto &tc = *static_cast(ctx.getLazyResolver()); - if (tc.validateType(attr->getTypeLoc(), resolution, options)) - return ErrorType::get(ctx); - - // We always require the type to resolve to a nominal type. - Type type = attr->getTypeLoc().getType(); - if (!type->getAnyNominal()) { +FunctionBuilderTypeRequest::evaluate(Evaluator &evaluator, + ParamDecl *param) const { + // Look for a function-builder custom attribute. + auto attr = param->getAttachedFunctionBuilder(); + if (!attr) return Type(); + + // Resolve a type for the attribute. + auto mutableAttr = const_cast(attr); + auto dc = param->getDeclContext(); + auto &ctx = dc->getASTContext(); + Type type = resolveCustomAttrType(mutableAttr, dc, + CustomAttrTypeKind::NonGeneric); + if (!type) return Type(); + + // The type must not be contextually-dependent. + if (type->hasArchetype()) { + ctx.Diags.diagnose(attr->getLocation(), + diag::function_builder_type_contextual, type); + return Type(); + } + + auto nominal = type->getAnyNominal(); + if (!nominal) { assert(ctx.Diags.hadAnyError()); - return ErrorType::get(ctx); + return Type(); } - switch (typeKind) { - case CustomAttrTypeKind::NonGeneric: - if (type->hasArchetype()) { - ctx.Diags.diagnose(attr->getLocation(), - diag::function_builder_type_contextual, type); - return ErrorType::get(ctx); - } - break; + // The parameter had better already have an interface type. + Type paramType = param->getInterfaceType(); + assert(paramType); + auto paramFnType = paramType->getAs(); + + // Require the parameter to be an interface type. + if (!paramFnType) { + ctx.Diags.diagnose(attr->getLocation(), + diag::function_builder_parameter_not_of_function_type, + nominal->getFullName()); + mutableAttr->setInvalid(); + return Type(); + } - case CustomAttrTypeKind::PropertyDelegate: - // No further logic required here. - break; + if (param->isAutoClosure()) { + ctx.Diags.diagnose(attr->getLocation(), + diag::function_builder_parameter_autoclosure, + nominal->getFullName()); + mutableAttr->setInvalid(); + return Type(); } return type; diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index c6a058f74e1dd..91462c5807001 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -3561,3 +3561,27 @@ void TypeChecker::checkUnsupportedProtocolType(GenericParamList *genericParams) UnsupportedProtocolVisitor visitor(*this, /*checkStatements=*/false); visitor.visitRequirements(genericParams->getRequirements()); } + +Type swift::resolveCustomAttrType(CustomAttr *attr, DeclContext *dc, + CustomAttrTypeKind typeKind) { + auto resolution = TypeResolution::forContextual(dc); + TypeResolutionOptions options(TypeResolverContext::PatternBindingDecl); + + // Property delegates allow their type to be an unbound generic. + if (typeKind == CustomAttrTypeKind::PropertyDelegate) + options |= TypeResolutionFlags::AllowUnboundGenerics; + + ASTContext &ctx = dc->getASTContext(); + auto &tc = *static_cast(ctx.getLazyResolver()); + if (tc.validateType(attr->getTypeLoc(), resolution, options)) + return Type(); + + // We always require the type to resolve to a nominal type. + Type type = attr->getTypeLoc().getType(); + if (!type->getAnyNominal()) { + assert(ctx.Diags.hadAnyError()); + return Type(); + } + + return type; +} diff --git a/lib/Sema/TypeCheckType.h b/lib/Sema/TypeCheckType.h index 7d7b066f904e9..397904ccdb2ed 100644 --- a/lib/Sema/TypeCheckType.h +++ b/lib/Sema/TypeCheckType.h @@ -377,6 +377,21 @@ class TypeResolution { bool areSameType(Type type1, Type type2) const; }; +/// Kinds of types for CustomAttr. +enum class CustomAttrTypeKind { + /// The type is required to not be expressed in terms of + /// any contextual type parameters. + NonGeneric, + + /// Property delegates have some funky rules, like allowing + /// unbound generic types. + PropertyDelegate, +}; + +/// Attempt to resolve a concrete type for a custom attribute. +Type resolveCustomAttrType(CustomAttr *attr, DeclContext *dc, + CustomAttrTypeKind typeKind); + } // end namespace swift #endif /* SWIFT_SEMA_TYPE_CHECK_TYPE_H */ diff --git a/test/decl/var/function_builders.swift b/test/decl/var/function_builders.swift index 823a94a94fd4e..4273f52a21090 100644 --- a/test/decl/var/function_builders.swift +++ b/test/decl/var/function_builders.swift @@ -44,6 +44,9 @@ func makerParamMissing2(@Maker func makerParamExtra(@Maker(5) // expected-error {{function builder attributes cannot have arguments}} fn: () -> ()) {} +func makerParamAutoclosure(@Maker // expected-error {{function builder attribute 'Maker' cannot be applied to an autoclosure parameter}} + fn: @autoclosure () -> ()) {} + @_functionBuilder struct GenericMaker {} // expected-note {{generic type 'GenericMaker' declared here}} diff --git a/test/multifile/Inputs/function_builder_definition.swift b/test/multifile/Inputs/function_builder_definition.swift new file mode 100644 index 0000000000000..0ea2a999db246 --- /dev/null +++ b/test/multifile/Inputs/function_builder_definition.swift @@ -0,0 +1,26 @@ +@_functionBuilder +struct TupleBuilder { + static func buildBlock(_ t1: T1, _ t2: T2) -> (T1, T2) { + return (t1, t2) + } + + static func buildBlock(_ t1: T1, _ t2: T2, _ t3: T3) + -> (T1, T2, T3) { + return (t1, t2, t3) + } + + static func buildBlock(_ t1: T1, _ t2: T2, _ t3: T3, _ t4: T4) + -> (T1, T2, T3, T4) { + return (t1, t2, t3, t4) + } + + static func buildBlock( + _ t1: T1, _ t2: T2, _ t3: T3, _ t4: T4, _ t5: T5 + ) -> (T1, T2, T3, T4, T5) { + return (t1, t2, t3, t4, t5) + } +} + +func tuplify(@TupleBuilder body: () -> T) -> T { + return body() +} diff --git a/test/multifile/function_builder_multifile.swift b/test/multifile/function_builder_multifile.swift new file mode 100644 index 0000000000000..de6850df25737 --- /dev/null +++ b/test/multifile/function_builder_multifile.swift @@ -0,0 +1,9 @@ +// RUN: %target-swift-frontend -typecheck %S/Inputs/function_builder_definition.swift -primary-file %s + +func test0() -> (Int, Float, String) { + return tuplify { + 17 + 3.14159 + "Hello" + } +} From 15e8b4b544e5cf908c45c194bab1fd6ac65ded13 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 16 Apr 2019 15:55:17 -0700 Subject: [PATCH 09/38] Factor the computation of default arguments into ParameterListInfo. Provide a place where we can capture more information about the parameters from a declaration being called. --- include/swift/AST/Types.h | 28 +++++++++++++++++++------ lib/AST/Type.cpp | 36 +++++++++++++++++++------------- lib/IDE/CodeCompletion.cpp | 7 +++---- lib/Sema/CSApply.cpp | 5 ++--- lib/Sema/CSDiag.cpp | 35 ++++++++++++++++--------------- lib/Sema/CSRanking.cpp | 13 ++++++------ lib/Sema/CSSimplify.cpp | 29 +++++++++++-------------- lib/Sema/CalleeCandidateInfo.cpp | 6 +++--- lib/Sema/ConstraintSystem.h | 8 +++---- 9 files changed, 93 insertions(+), 74 deletions(-) diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h index c438d9e0f779e..b5b7d387b396b 100644 --- a/include/swift/AST/Types.h +++ b/include/swift/AST/Types.h @@ -3118,12 +3118,28 @@ BEGIN_CAN_TYPE_WRAPPER(FunctionType, AnyFunctionType) } END_CAN_TYPE_WRAPPER(FunctionType, AnyFunctionType) -/// Map the given parameter list onto a bitvector describing whether -/// the argument type at each index has a default argument associated with -/// it. -SmallBitVector -computeDefaultMap(ArrayRef params, - const ValueDecl *paramOwner, bool skipCurriedSelf); +/// Provides information about the parameter list of a given declaration, including whether each parameter +/// has a default argument. +struct ParameterListInfo { + SmallBitVector defaultArguments; + +public: + ParameterListInfo() { } + + ParameterListInfo(ArrayRef params, + const ValueDecl *paramOwner, bool skipCurriedSelf); + + /// Whether the parameter at the given index has a default argument. + bool hasDefaultArgument(unsigned paramIdx) const; + + /// Retrieve the number of non-defaulted parameters. + unsigned numNonDefaultedParameters() const { + return defaultArguments.count(); + } + + /// Retrieve the number of parameters for which we have information. + unsigned size() const { return defaultArguments.size(); } +}; /// Turn a param list into a symbolic and printable representation that does not /// include the types, something like (: , b:, c:) diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index 051ccf27ff1e1..e4dd8157e2706 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -742,16 +742,18 @@ Type TypeBase::replaceCovariantResultType(Type newResultType, return FunctionType::get(inputType, resultType, fnType->getExtInfo()); } -SmallBitVector -swift::computeDefaultMap(ArrayRef params, - const ValueDecl *paramOwner, bool skipCurriedSelf) { - SmallBitVector resultVector(params.size()); +ParameterListInfo::ParameterListInfo( + ArrayRef params, + const ValueDecl *paramOwner, + bool skipCurriedSelf) { + defaultArguments.resize(params.size()); + // No parameter owner means no parameter list means no default arguments // - hand back the zeroed bitvector. // // FIXME: We ought to not request default argument info in this case. if (!paramOwner) - return resultVector; + return; // Find the corresponding parameter list. const ParameterList *paramList = nullptr; @@ -772,27 +774,33 @@ swift::computeDefaultMap(ArrayRef params, // No parameter list means no default arguments - hand back the zeroed // bitvector. if (!paramList) - return resultVector; + return; switch (params.size()) { case 0: - return resultVector; + return; default: // Arguments and parameters are not guaranteed to always line-up // perfectly, e.g. failure diagnostics tries to match argument type // to different "candidate" parameters. if (params.size() != paramList->size()) - return resultVector; + return; - for (auto i : range(0, params.size())) { - if (paramList->get(i)->isDefaultArgument()) { - resultVector.set(i); - } - } break; } - return resultVector; + + // Note which parameters have default arguments. + for (auto i : range(0, params.size())) { + if (paramList->get(i)->isDefaultArgument()) { + defaultArguments.set(i); + } + } +} + +bool ParameterListInfo::hasDefaultArgument(unsigned paramIdx) const { + return paramIdx < defaultArguments.size() ? defaultArguments[paramIdx] + : false; } /// Turn a param list into a symbolic and printable representation that does not diff --git a/lib/IDE/CodeCompletion.cpp b/lib/IDE/CodeCompletion.cpp index 2cb99e1e8a4e3..3c5a83042b47d 100644 --- a/lib/IDE/CodeCompletion.cpp +++ b/lib/IDE/CodeCompletion.cpp @@ -1470,11 +1470,10 @@ protocolForLiteralKind(CodeCompletionLiteralKind kind) { /// that is of type () -> (). static bool hasTrivialTrailingClosure(const FuncDecl *FD, AnyFunctionType *funcType) { - SmallBitVector defaultMap = - computeDefaultMap(funcType->getParams(), FD, - /*level*/ FD->isInstanceMember() ? 1 : 0); + ParameterListInfo paramInfo(funcType->getParams(), FD, + /*level*/ FD->isInstanceMember() ? 1 : 0); - if (defaultMap.size() - defaultMap.count() == 1) { + if (paramInfo.size() - paramInfo.numNonDefaultedParameters() == 1) { auto param = funcType->getParams().back(); if (!param.isAutoClosure()) { if (auto Fn = param.getOldType()->getAs()) { diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 414780321db73..f4f25dc46b2fe 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -5515,8 +5515,7 @@ Expr *ExprRewriter::coerceCallArguments( bool skipCurriedSelf = apply ? hasCurriedSelf(cs, callee, apply) : false; // Determine the parameter bindings. - SmallBitVector defaultMap - = computeDefaultMap(params, callee.getDecl(), skipCurriedSelf); + ParameterListInfo paramInfo(params, callee.getDecl(), skipCurriedSelf); SmallVector args; AnyFunctionType::decomposeInput(cs.getType(arg), args); @@ -5531,7 +5530,7 @@ Expr *ExprRewriter::coerceCallArguments( MatchCallArgumentListener listener; SmallVector parameterBindings; bool failed = constraints::matchCallArguments(args, params, - defaultMap, + paramInfo, hasTrailingClosure, /*allowFixes=*/false, listener, parameterBindings); diff --git a/lib/Sema/CSDiag.cpp b/lib/Sema/CSDiag.cpp index 1a9712bbf158b..a325a723ab677 100644 --- a/lib/Sema/CSDiag.cpp +++ b/lib/Sema/CSDiag.cpp @@ -2732,10 +2732,12 @@ typeCheckArgumentChildIndependently(Expr *argExpr, Type argType, // If we have a candidate function around, compute the position of its // default arguments. - SmallBitVector defaultMap(params.size()); + ParameterListInfo paramInfo; if (!candidates.empty()) { - defaultMap = computeDefaultMap(params, candidates[0].getDecl(), - candidates[0].skipCurriedSelf); + paramInfo = ParameterListInfo(params, candidates[0].getDecl(), + candidates[0].skipCurriedSelf); + } else { + paramInfo = ParameterListInfo(params, nullptr, /*skipCurriedSelf=*/false); } // Form a set of call arguments, using a dummy type (Void), because the @@ -2754,7 +2756,7 @@ typeCheckArgumentChildIndependently(Expr *argExpr, Type argType, } listener; SmallVector paramBindings; - if (!matchCallArguments(args, params, defaultMap, + if (!matchCallArguments(args, params, paramInfo, callArgHasTrailingClosure(argExpr), /*allowFixes=*/true, listener, paramBindings)) { @@ -3350,7 +3352,7 @@ class ArgumentMatcher : public MatchCallArgumentListener { Expr *FnExpr; Expr *ArgExpr; ArrayRef &Parameters; - const SmallBitVector &DefaultMap; + const ParameterListInfo &ParamInfo; SmallVectorImpl &Arguments; CalleeCandidateInfo CandidateInfo; @@ -3366,11 +3368,11 @@ class ArgumentMatcher : public MatchCallArgumentListener { public: ArgumentMatcher(Expr *fnExpr, Expr *argExpr, ArrayRef ¶ms, - const SmallBitVector &defaultMap, + const ParameterListInfo ¶mInfo, SmallVectorImpl &args, CalleeCandidateInfo &CCI, bool isSubscript) : TC(CCI.CS.TC), FnExpr(fnExpr), ArgExpr(argExpr), Parameters(params), - DefaultMap(defaultMap), Arguments(args), CandidateInfo(CCI), + ParamInfo(paramInfo), Arguments(args), CandidateInfo(CCI), IsSubscript(isSubscript) {} void extraArgument(unsigned extraArgIdx) override { @@ -3572,7 +3574,7 @@ class ArgumentMatcher : public MatchCallArgumentListener { // Use matchCallArguments to determine how close the argument list is (in // shape) to the specified candidates parameters. This ignores the // concrete types of the arguments, looking only at the argument labels. - matchCallArguments(Arguments, Parameters, DefaultMap, + matchCallArguments(Arguments, Parameters, ParamInfo, CandidateInfo.hasTrailingClosure, /*allowFixes:*/ true, *this, Bindings); @@ -3599,9 +3601,8 @@ diagnoseSingleCandidateFailures(CalleeCandidateInfo &CCI, Expr *fnExpr, return false; auto params = candidate.getParameters(); - - SmallBitVector defaultMap = - computeDefaultMap(params, candidate.getDecl(), candidate.skipCurriedSelf); + ParameterListInfo paramInfo(params, candidate.getDecl(), + candidate.skipCurriedSelf); auto args = decomposeArgType(CCI.CS.getType(argExpr), argLabels); // Check the case where a raw-representable type is constructed from an @@ -3653,7 +3654,7 @@ diagnoseSingleCandidateFailures(CalleeCandidateInfo &CCI, Expr *fnExpr, // If we have a single candidate that failed to match the argument list, // attempt to use matchCallArguments to diagnose the problem. - return ArgumentMatcher(fnExpr, argExpr, params, defaultMap, args, CCI, + return ArgumentMatcher(fnExpr, argExpr, params, paramInfo, args, CCI, isa(fnExpr)) .diagnose(); } @@ -4247,13 +4248,13 @@ bool FailureDiagnosis::diagnoseArgumentGenericRequirements( return false; auto params = candidate.getParameters(); - SmallBitVector defaultMap = - computeDefaultMap(params, candidate.getDecl(), candidate.skipCurriedSelf); + ParameterListInfo paramInfo(params, candidate.getDecl(), + candidate.skipCurriedSelf); auto args = decomposeArgType(CS.getType(argExpr), argLabels); SmallVector bindings; MatchCallArgumentListener listener; - if (matchCallArguments(args, params, defaultMap, + if (matchCallArguments(args, params, paramInfo, candidates.hasTrailingClosure, /*allowFixes=*/false, listener, bindings)) return false; @@ -4720,8 +4721,8 @@ static bool isViableOverloadSet(const CalleeCandidateInfo &CCI, return true; }; - auto defaultMap = computeDefaultMap(params, funcDecl, cand.skipCurriedSelf); - InputMatcher IM(params, defaultMap); + ParameterListInfo paramInfo(params, funcDecl, cand.skipCurriedSelf); + InputMatcher IM(params, paramInfo); auto result = IM.match(numArgs, pairMatcher); if (result == InputMatcher::IM_Succeeded) return true; diff --git a/lib/Sema/CSRanking.cpp b/lib/Sema/CSRanking.cpp index 69a753507baa5..3f5269e2acf61 100644 --- a/lib/Sema/CSRanking.cpp +++ b/lib/Sema/CSRanking.cpp @@ -692,7 +692,7 @@ static bool isDeclAsSpecializedAs(TypeChecker &tc, DeclContext *dc, return true; }; - auto defaultMap = computeDefaultMap( + ParameterListInfo paramInfo( params2, decl2, decl2->getDeclContext()->isTypeContext()); auto params2ForMatching = params2; if (compareTrailingClosureParamsSeparately) { @@ -700,7 +700,7 @@ static bool isDeclAsSpecializedAs(TypeChecker &tc, DeclContext *dc, params2ForMatching = params2.drop_back(); } - InputMatcher IM(params2ForMatching, defaultMap); + InputMatcher IM(params2ForMatching, paramInfo); if (IM.match(numParams1, pairMatcher) != InputMatcher::IM_Succeeded) return false; @@ -1468,8 +1468,8 @@ SolutionDiff::SolutionDiff(ArrayRef solutions) { } InputMatcher::InputMatcher(const ArrayRef params, - const SmallBitVector &defaultValueMap) - : NumSkippedParameters(0), DefaultValueMap(defaultValueMap), + const ParameterListInfo ¶mInfo) + : NumSkippedParameters(0), ParamInfo(paramInfo), Params(params) {} InputMatcher::Result @@ -1482,7 +1482,7 @@ InputMatcher::match(int numInputs, // If we've claimed all of the inputs, the rest of the parameters should // be either default or variadic. if (inputIdx == numInputs) { - if (!DefaultValueMap[i] && !Params[i].isVariadic()) + if (!ParamInfo.hasDefaultArgument(i) && !Params[i].isVariadic()) return IM_HasUnmatchedParam; ++NumSkippedParameters; continue; @@ -1500,7 +1500,8 @@ InputMatcher::match(int numInputs, // params: (a: Int, b: Int = 0, c: Int) // // and we shouldn't claim any input and just skip such parameter. - if ((numInputs - inputIdx) < (numParams - i) && DefaultValueMap[i]) { + if ((numInputs - inputIdx) < (numParams - i) && + ParamInfo.hasDefaultArgument(i)) { ++NumSkippedParameters; continue; } diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 4cf607e581072..3ba25a56efb7d 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -168,13 +168,12 @@ bool constraints::areConservativelyCompatibleArgumentLabels( } auto params = levelTy->getParams(); - SmallBitVector defaultMap = - computeDefaultMap(params, decl, hasCurriedSelf); + ParameterListInfo paramInfo(params, decl, hasCurriedSelf); MatchCallArgumentListener listener; SmallVector unusedParamBindings; - return !matchCallArguments(argInfos, params, defaultMap, + return !matchCallArguments(argInfos, params, paramInfo, hasTrailingClosure, /*allow fixes*/ false, listener, unusedParamBindings); @@ -211,12 +210,12 @@ static ConstraintSystem::TypeMatchOptions getDefaultDecompositionOptions( bool constraints:: matchCallArguments(ArrayRef args, ArrayRef params, - const SmallBitVector &defaultMap, + const ParameterListInfo ¶mInfo, bool hasTrailingClosure, bool allowFixes, MatchCallArgumentListener &listener, SmallVectorImpl ¶meterBindings) { - assert(params.size() == defaultMap.size() && "Default map does not match"); + assert(params.size() == paramInfo.size() && "Default map does not match"); // Keep track of the parameter we're matching and what argument indices // got bound to each parameter. @@ -234,10 +233,6 @@ matchCallArguments(ArrayRef args, // requiring further checking at the end. bool potentiallyOutOfOrder = false; - auto hasDefault = [&defaultMap, &numParams](unsigned idx) -> bool { - return idx < numParams ? defaultMap.test(idx) : false; - }; - // Local function that claims the argument at \c argNumber, returning the // index of the claimed argument. This is primarily a helper for // \c claimNextNamed. @@ -327,7 +322,8 @@ matchCallArguments(ArrayRef args, // func foo(_ a: Int, _ b: Int = 0, c: Int = 0, _ d: Int) {} // foo(1, c: 2, 3) // -> `3` will be claimed as '_ b:'. // ``` - if (argLabel.empty() && (hasDefault(i) || !forVariadic)) + if (argLabel.empty() && + (paramInfo.hasDefaultArgument(i) || !forVariadic)) continue; potentiallyOutOfOrder = true; @@ -531,7 +527,7 @@ matchCallArguments(ArrayRef args, // now if label doesn't match because it's incorrect // or argument belongs to some other parameter, so // we just leave this parameter unfulfilled. - if (defaultMap.test(i)) + if (paramInfo.hasDefaultArgument(i)) continue; // Looks like there was no parameter claimed at the same @@ -568,7 +564,7 @@ matchCallArguments(ArrayRef args, continue; // Parameters with defaults can be unfulfilled. - if (hasDefault(paramIdx)) + if (paramInfo.hasDefaultArgument(paramIdx)) continue; listener.missingArgument(paramIdx); @@ -601,7 +597,7 @@ matchCallArguments(ArrayRef args, // If we are moving the the position with a different label // and there is no default value for it, can't diagnose the // problem as a simple re-ordering. - if (!defaultMap.test(actualIndex)) + if (!paramInfo.hasDefaultArgument(actualIndex)) return false; } @@ -609,7 +605,7 @@ matchCallArguments(ArrayRef args, if (oldLabel == params[i].getLabel()) break; - if (!defaultMap.test(i)) + if (!paramInfo.hasDefaultArgument(i)) return false; } @@ -914,8 +910,7 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments( std::tie(callee, hasCurriedSelf, argLabels, hasTrailingClosure) = getCalleeDeclAndArgs(cs, locator, argLabelsScratch); - SmallBitVector defaultMap = - computeDefaultMap(params, callee, hasCurriedSelf); + ParameterListInfo paramInfo(params, callee, hasCurriedSelf); // Apply labels to arguments. SmallVector argsWithLabels; @@ -926,7 +921,7 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments( SmallVector parameterBindings; ArgumentFailureTracker listener(cs, parameterBindings, locator); if (constraints::matchCallArguments(argsWithLabels, params, - defaultMap, + paramInfo, hasTrailingClosure, cs.shouldAttemptFixes(), listener, parameterBindings)) diff --git a/lib/Sema/CalleeCandidateInfo.cpp b/lib/Sema/CalleeCandidateInfo.cpp index b6f80e88ef5a6..1bd316722dc4e 100644 --- a/lib/Sema/CalleeCandidateInfo.cpp +++ b/lib/Sema/CalleeCandidateInfo.cpp @@ -320,8 +320,8 @@ CalleeCandidateInfo::ClosenessResultTy CalleeCandidateInfo::evaluateCloseness( return {CC_GeneralMismatch, {}}; auto candArgs = candidate.getParameters(); - SmallBitVector candDefaultMap = - computeDefaultMap(candArgs, candidate.getDecl(), candidate.skipCurriedSelf); + ParameterListInfo candParamInfo(candArgs, candidate.getDecl(), + candidate.skipCurriedSelf); struct OurListener : public MatchCallArgumentListener { CandidateCloseness result = CC_ExactMatch; @@ -362,7 +362,7 @@ CalleeCandidateInfo::ClosenessResultTy CalleeCandidateInfo::evaluateCloseness( // types of the arguments, looking only at the argument labels etc. SmallVector paramBindings; if (matchCallArguments(actualArgs, candArgs, - candDefaultMap, + candParamInfo, hasTrailingClosure, /*allowFixes:*/ true, listener, paramBindings)) diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index 9009d7d51bb17..5b7a097d740fe 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -3652,7 +3652,7 @@ class MatchCallArgumentListener { /// /// \param args The arguments. /// \param params The parameters. -/// \param defaultMap A map indicating if the parameter at that index has a default value. +/// \param paramInfo Declaration-level information about the parameters. /// \param hasTrailingClosure Whether the last argument is a trailing closure. /// \param allowFixes Whether to allow fixes when matching arguments. /// @@ -3664,7 +3664,7 @@ class MatchCallArgumentListener { /// \returns true if the call arguments could not be matched to the parameters. bool matchCallArguments(ArrayRef args, ArrayRef params, - const SmallBitVector &defaultMap, + const ParameterListInfo ¶mInfo, bool hasTrailingClosure, bool allowFixes, MatchCallArgumentListener &listener, @@ -4043,7 +4043,7 @@ class OverloadSetCounter : public ASTWalker { /// in the custom function when necessary. class InputMatcher { size_t NumSkippedParameters; - const SmallBitVector DefaultValueMap; + const ParameterListInfo &ParamInfo; const ArrayRef Params; public: @@ -4059,7 +4059,7 @@ class InputMatcher { }; InputMatcher(const ArrayRef params, - const SmallBitVector &defaultValueMap); + const ParameterListInfo ¶mInfo); /// Matching a given array of inputs. /// From 42aac2fae4ac5538f758736c232ee87cb127d3ec Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 16 Apr 2019 18:13:15 -0700 Subject: [PATCH 10/38] [Type checker] Transform multi-statement closures via function builders. When calling a function whose parameter specifies a function builder with a multi-statement closure argument, transform the closure into a single expression via the function builder. Should the result type checker, replace the closure body with the single expression. --- include/swift/AST/KnownIdentifiers.def | 1 + include/swift/AST/Types.h | 4 + lib/AST/Type.cpp | 16 +- lib/Sema/CSApply.cpp | 15 ++ lib/Sema/CSSimplify.cpp | 10 ++ lib/Sema/CSSolver.cpp | 18 +++ lib/Sema/ConstraintSystem.cpp | 191 ++++++++++++++++++++++++ lib/Sema/ConstraintSystem.h | 14 ++ lib/Sema/TypeChecker.h | 7 + test/Constraints/function_builder.swift | 36 +++++ 10 files changed, 310 insertions(+), 2 deletions(-) create mode 100644 test/Constraints/function_builder.swift diff --git a/include/swift/AST/KnownIdentifiers.def b/include/swift/AST/KnownIdentifiers.def index 6fe0b809917b2..7d51fd00cdd4d 100644 --- a/include/swift/AST/KnownIdentifiers.def +++ b/include/swift/AST/KnownIdentifiers.def @@ -31,6 +31,7 @@ IDENTIFIER(Any) IDENTIFIER(ArrayLiteralElement) IDENTIFIER(atIndexedSubscript) IDENTIFIER_(bridgeToObjectiveC) +IDENTIFIER(buildBlock) IDENTIFIER(Change) IDENTIFIER_WITH_NAME(code_, "_code") IDENTIFIER(CodingKeys) diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h index b5b7d387b396b..d34a096548e2a 100644 --- a/include/swift/AST/Types.h +++ b/include/swift/AST/Types.h @@ -3122,6 +3122,7 @@ END_CAN_TYPE_WRAPPER(FunctionType, AnyFunctionType) /// has a default argument. struct ParameterListInfo { SmallBitVector defaultArguments; + std::vector functionBuilderTypes; public: ParameterListInfo() { } @@ -3139,6 +3140,9 @@ struct ParameterListInfo { /// Retrieve the number of parameters for which we have information. unsigned size() const { return defaultArguments.size(); } + + /// Retrieve the function builder type for the given parameter. + Type getFunctionBuilderType(unsigned paramIdx) const; }; /// Turn a param list into a symbolic and printable representation that does not diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index e4dd8157e2706..309f29d1bf9e7 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -747,6 +747,7 @@ ParameterListInfo::ParameterListInfo( const ValueDecl *paramOwner, bool skipCurriedSelf) { defaultArguments.resize(params.size()); + functionBuilderTypes.resize(params.size()); // No parameter owner means no parameter list means no default arguments // - hand back the zeroed bitvector. @@ -790,11 +791,16 @@ ParameterListInfo::ParameterListInfo( break; } - // Note which parameters have default arguments. + // Note which parameters have default arguments and/or function builders. for (auto i : range(0, params.size())) { - if (paramList->get(i)->isDefaultArgument()) { + auto param = paramList->get(i); + if (param->isDefaultArgument()) { defaultArguments.set(i); } + + if (Type functionBuilderType = param->getFunctionBuilderType()) { + functionBuilderTypes[i] = functionBuilderType; + } } } @@ -803,6 +809,12 @@ bool ParameterListInfo::hasDefaultArgument(unsigned paramIdx) const { : false; } +Type ParameterListInfo::getFunctionBuilderType(unsigned paramIdx) const { + return paramIdx < functionBuilderTypes.size() + ? functionBuilderTypes[paramIdx] + : Type(); +} + /// Turn a param list into a symbolic and printable representation that does not /// include the types, something like (_:, b:, c:) std::string swift::getParamListAsString(ArrayRef params) { diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index f4f25dc46b2fe..1b8383bdca6c2 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -7541,6 +7541,21 @@ namespace { // metadata types/conformances during IRGen. tc.requestRequiredNominalTypeLayoutForParameters(params); + // If this closure had a function builder applied, rewrite it to a + // closure with a single expression body containing the builder + // invocations. + auto builder = + Rewriter.solution.builderTransformedClosures.find(closure); + if (builder != Rewriter.solution.builderTransformedClosures.end()) { + auto singleExpr = builder->second.second; + auto returnStmt = new (tc.Context) ReturnStmt( + singleExpr->getStartLoc(), singleExpr, /*implicit=*/true); + auto braceStmt = BraceStmt::create( + tc.Context, returnStmt->getStartLoc(), ASTNode(returnStmt), + returnStmt->getEndLoc(), /*implicit=*/true); + closure->setBody(braceStmt, /*isSingleExpression=*/true); + } + // If this is a single-expression closure, convert the expression // in the body to the result type of the closure. if (closure->hasSingleExpressionBody()) { diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 3ba25a56efb7d..fec2fbbb3a6ad 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -979,6 +979,16 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments( } } + // If the parameter has a function builder type and the argument is a + // closure, apply the function builder transformation. + if (Type functionBuilderType + = paramInfo.getFunctionBuilderType(paramIdx)) { + Expr *arg = getArgumentExpr(locator.getAnchor(), argIdx); + if (auto closure = dyn_cast_or_null(arg)) { + cs.applyFunctionBuilder(closure, functionBuilderType, locator); + } + } + // If argument comes for declaration it should loose // `@autoclosure` flag, because in context it's used // as a function type represented by autoclosure. diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 27a1fcc1a71c8..2580d6623676a 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -172,6 +172,13 @@ Solution ConstraintSystem::finalize() { for (auto &e : CheckedConformances) solution.Conformances.push_back({e.first, e.second}); + for (const auto &transformed : builderTransformedClosures) { + solution.builderTransformedClosures.insert( + std::make_pair(std::get<0>(transformed), + std::make_pair(std::get<1>(transformed), + std::get<2>(transformed)))); + } + return solution; } @@ -236,6 +243,13 @@ void ConstraintSystem::applySolution(const Solution &solution) { for (auto &conformance : solution.Conformances) CheckedConformances.push_back(conformance); + for (const auto &transformed : solution.builderTransformedClosures) { + builderTransformedClosures.push_back( + std::make_tuple(transformed.first, + transformed.second.first, + transformed.second.second)); + } + // Register any fixes produced along this path. Fixes.append(solution.Fixes.begin(), solution.Fixes.end()); @@ -425,6 +439,7 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numMissingMembers = cs.MissingMembers.size(); numDisabledConstraints = cs.solverState->getNumDisabledConstraints(); numFavoredConstraints = cs.solverState->getNumFavoredConstraints(); + numBuilderTransformedClosures = cs.builderTransformedClosures.size(); PreviousScore = cs.CurrentScore; @@ -479,6 +494,9 @@ ConstraintSystem::SolverScope::~SolverScope() { // Remove any missing members found along the current path. truncate(cs.MissingMembers, numMissingMembers); + /// Remove any builder transformed closures. + truncate(cs.builderTransformedClosures, numBuilderTransformedClosures); + // Reset the previous score. cs.CurrentScore = PreviousScore; diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index e940e973dc6bc..aedc64e2cffee 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2764,3 +2764,194 @@ bool constraints::isKnownKeyPathDecl(ASTContext &ctx, ValueDecl *decl) { decl == ctx.getReferenceWritableKeyPathDecl() || decl == ctx.getPartialKeyPathDecl() || decl == ctx.getAnyKeyPathDecl(); } + +namespace { + /// Visitor to classify the contents of the given closure. + class BuilderClosureVisitor + : public StmtVisitor { + ASTContext &ctx; + Type builderType; + + public: + BuilderClosureVisitor(ASTContext &ctx, Type builderType) + : ctx(ctx), builderType(builderType) { } + + ReturnStmt *returnStmt = nullptr; + Stmt *controlFlowStmt = nullptr; + Decl *decl = nullptr; + +#define CONTROL_FLOW_STMT(StmtClass) \ + Expr *visit##StmtClass##Stmt(StmtClass##Stmt *stmt) { \ + if (!controlFlowStmt) \ + controlFlowStmt = stmt; \ + \ + return nullptr; \ + } + + Expr *visitBraceStmt(BraceStmt *braceStmt) { + SmallVector expressions; + for (const auto &node : braceStmt->getElements()) { + if (auto stmt = node.dyn_cast()) { + auto expr = visit(stmt); + if (expr) + expressions.push_back(expr); + continue; + } + + if (auto decl = node.dyn_cast()) { + if (!this->decl) + this->decl = decl; + + continue; + } + + auto expr = node.get(); + expressions.push_back(expr); + } + + if (!builderType) + return nullptr; + + // Call Builder.buildBlock(... args ...) + auto typeExpr = TypeExpr::createImplicit(builderType, ctx); + auto memberRef = new (ctx) UnresolvedDotExpr( + typeExpr, SourceLoc(), ctx.Id_buildBlock, DeclNameLoc(), + /*implicit=*/true); + return CallExpr::createImplicit(ctx, memberRef, expressions, { }); + } + + Expr *visitReturnStmt(ReturnStmt *returnStmt) { + if (!this->returnStmt) + this->returnStmt = returnStmt; + return nullptr; + } + + CONTROL_FLOW_STMT(Do) + CONTROL_FLOW_STMT(Yield) + CONTROL_FLOW_STMT(Defer) + CONTROL_FLOW_STMT(If) + CONTROL_FLOW_STMT(Guard) + CONTROL_FLOW_STMT(While) + CONTROL_FLOW_STMT(DoCatch) + CONTROL_FLOW_STMT(RepeatWhile) + CONTROL_FLOW_STMT(ForEach) + CONTROL_FLOW_STMT(Switch) + CONTROL_FLOW_STMT(Case) + CONTROL_FLOW_STMT(Catch) + CONTROL_FLOW_STMT(Break) + CONTROL_FLOW_STMT(Continue) + CONTROL_FLOW_STMT(Fallthrough) + CONTROL_FLOW_STMT(Fail) + CONTROL_FLOW_STMT(Throw) + CONTROL_FLOW_STMT(PoundAssert) + +#undef CONTROL_FLOW_STMT + }; +} + +BuilderClosureKind swift::classifyBuilderClosure(ASTContext &ctx, + ClosureExpr *closure, + bool diagnose) { + if (closure->hasSingleExpressionBody()) + return BuilderClosureKind::SingleExpression; + + BuilderClosureVisitor visitor(ctx, Type()); + (void)visitor.visit(closure->getBody()); + + // The presence of an explicit return makes this not a builder. + if (visitor.returnStmt) { + if (diagnose) { + ctx.Diags.diagnose(visitor.returnStmt->getReturnLoc(), + diag::closure_builder_return_stmt); + } + + return BuilderClosureKind::ExplicitReturn; + } + + // The presence of control flow makes this not work with closure builders. + if (visitor.controlFlowStmt) { + if (diagnose) { + ctx.Diags.diagnose(visitor.controlFlowStmt->getStartLoc(), + diag::closure_builder_control_flow); + } + + return BuilderClosureKind::UnsupportedConstruct; + } + + // The presence of any declaration makes this not work with closure builders. + if (visitor.decl) { + if (diagnose) { + visitor.decl->diagnose(diag::closure_builder_decl); + } + + return BuilderClosureKind::UnsupportedConstruct; + } + + return BuilderClosureKind::Supported; +} + + +void ConstraintSystem::applyFunctionBuilder(ClosureExpr *closure, + Type builderType, + ConstraintLocatorBuilder locator) { + auto builder = builderType->getAnyNominal(); + assert(builder && "Bad function builder type"); + assert(builder->getAttrs().hasAttribute()); + switch (classifyBuilderClosure( + getASTContext(), closure, /*diagnose=*/false)) { + case BuilderClosureKind::ExplicitReturn: + case BuilderClosureKind::UnsupportedConstruct: + return; + + case BuilderClosureKind::SingleExpression: + // FIXME: We might be able to do this, but it's not clear. + return; + + case BuilderClosureKind::Supported: + break; + } + + auto &appliedBuilders = TC.appliedFunctionBuilders[closure]; + + // Check whether we have already applied this function builder to + // this closure. + Expr *singleExpr = nullptr; + for (const auto &applied : appliedBuilders) { + if (applied.first->isEqual(builderType)) { + singleExpr = applied.second; + break; + } + } + + // If we didn't find anything yet, go build the expression. + if (!singleExpr) { + BuilderClosureVisitor visitor(getASTContext(), builderType); + singleExpr = visitor.visit(closure->getBody()); + + // FIXME: Failure mode. + (void)TC.preCheckExpression(singleExpr, DC); + + // Record this expression for later. + TC.appliedFunctionBuilders[closure].push_back( + {builderType->getCanonicalType(), singleExpr}); + } + + + singleExpr = generateConstraints(singleExpr); + if (!singleExpr) + return; + + Type transformedType = getType(singleExpr); + assert(transformedType && "Missing type"); + + // Record the transformation. + builderTransformedClosures.push_back( + std::make_tuple(closure, builderType, singleExpr)); + + // Bind the result type of the closure to the type of the transformed + // expression. + Type closureType = getType(closure); + auto fnType = closureType->castTo(); + addConstraint(ConstraintKind::Equal, fnType->getResult(), transformedType, + locator); +} diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index 5b7a097d740fe..bd5888f951846 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -619,6 +619,10 @@ class Solution { llvm::SmallVector, 8> Conformances; + /// The set of closures that have been transformed by a function builder. + llvm::MapVector> + builderTransformedClosures; + /// Simplify the given type by substituting all occurrences of /// type variables for their fixed types. Type simplifyType(Type type) const; @@ -1101,6 +1105,10 @@ class ConstraintSystem { SmallVector, 8> CheckedConformances; + /// The set of closures that have been transformed by a function builder. + SmallVector, 4> + builderTransformedClosures; + public: /// The locators of \c Defaultable constraints whose defaults were used. SmallVector DefaultedConstraints; @@ -1568,6 +1576,8 @@ class ConstraintSystem { unsigned numFavoredConstraints; + unsigned numBuilderTransformedClosures; + /// The previous score. Score PreviousScore; @@ -2966,6 +2976,10 @@ class ConstraintSystem { /// Simplify the given disjunction choice. void simplifyDisjunctionChoice(Constraint *choice); + /// Apply the given function builder to the closure expression. + void applyFunctionBuilder(ClosureExpr *closure, Type builderType, + ConstraintLocatorBuilder locator); + private: /// The kind of bindings that are permitted. enum class AllowedBindingKind : uint8_t { diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index aa88e6aeaa90a..6cfdc0fbd9642 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -669,6 +669,13 @@ class TypeChecker final : public LazyResolver { /// when executing scripts. bool InImmediateMode = false; +public: + /// Cached mapping from closure expressions that have had function builders applied + /// to the (builder type, resulting single expression) pairs for that closure expression. + llvm::DenseMap, 2>> + appliedFunctionBuilders; + +private: /// A helper to construct and typecheck call to super.init(). /// /// \returns NULL if the constructed expression does not typecheck. diff --git a/test/Constraints/function_builder.swift b/test/Constraints/function_builder.swift new file mode 100644 index 0000000000000..05f791033d3d8 --- /dev/null +++ b/test/Constraints/function_builder.swift @@ -0,0 +1,36 @@ +// RUN: %target-run-simple-swift | %FileCheck %s +// REQUIRES: executable_test + +@_functionBuilder +struct TupleBuilder { + static func buildBlock(_ t1: T1, _ t2: T2) -> (T1, T2) { + return (t1, t2) + } + + static func buildBlock(_ t1: T1, _ t2: T2, _ t3: T3) + -> (T1, T2, T3) { + return (t1, t2, t3) + } + + static func buildBlock(_ t1: T1, _ t2: T2, _ t3: T3, _ t4: T4) + -> (T1, T2, T3, T4) { + return (t1, t2, t3, t4) + } + + static func buildBlock( + _ t1: T1, _ t2: T2, _ t3: T3, _ t4: T4, _ t5: T5 + ) -> (T1, T2, T3, T4, T5) { + return (t1, t2, t3, t4, t5) + } +} + +func tuplify(@TupleBuilder body: () -> T) { + print(body()) +} + +// CHECK: (17, 3.14159, "Hello") +tuplify { + 17 + 3.14159 + "Hello" +} From 6947fd82e69aad7ade135aba63d6738c62fbbc36 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Wed, 17 Apr 2019 14:26:18 -0700 Subject: [PATCH 11/38] Ensure that we use the right closure context for constraint generation. --- lib/Sema/CSGen.cpp | 13 +++++++------ lib/Sema/ConstraintSystem.cpp | 4 ++-- lib/Sema/ConstraintSystem.h | 2 +- test/Constraints/function_builder.swift | 5 +++-- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index d13a8ac84c56a..12a51c0501b1f 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -1119,7 +1119,9 @@ namespace { } public: - ConstraintGenerator(ConstraintSystem &CS) : CS(CS), CurDC(CS.DC) { } + ConstraintGenerator(ConstraintSystem &CS, DeclContext *DC) + : CS(CS), CurDC(DC ? DC : CS.DC) { } + virtual ~ConstraintGenerator() { // We really ought to have this assertion: // assert(DCStack.empty() && CurDC == CS.DC); @@ -3045,8 +3047,7 @@ namespace { Type visitTapExpr(TapExpr *expr) { DeclContext *varDC = expr->getVar()->getDeclContext(); - assert(varDC == CS.DC || (varDC && isa(varDC) && - cast(varDC)->hasSingleExpressionBody()) && + assert(varDC == CS.DC || (varDC && isa(varDC)) && "TapExpr var should be in the same DeclContext we're checking it in!"); auto locator = CS.getConstraintLocator(expr); @@ -3606,7 +3607,7 @@ namespace { } // end anonymous namespace -Expr *ConstraintSystem::generateConstraints(Expr *expr) { +Expr *ConstraintSystem::generateConstraints(Expr *expr, DeclContext *dc) { // Remove implicit conversions from the expression. expr = expr->walk(SanitizeExpr(*this)); @@ -3614,7 +3615,7 @@ Expr *ConstraintSystem::generateConstraints(Expr *expr) { expr->walk(ArgumentLabelWalker(*this, expr)); // Walk the expression, generating constraints. - ConstraintGenerator cg(*this); + ConstraintGenerator cg(*this, dc); ConstraintWalker cw(cg); Expr* result = expr->walk(cw); @@ -3627,7 +3628,7 @@ Expr *ConstraintSystem::generateConstraints(Expr *expr) { Type ConstraintSystem::generateConstraints(Pattern *pattern, ConstraintLocatorBuilder locator) { - ConstraintGenerator cg(*this); + ConstraintGenerator cg(*this, nullptr); return cg.getTypeForPattern(pattern, locator); } diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index aedc64e2cffee..5ef7f0c063d61 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2929,7 +2929,7 @@ void ConstraintSystem::applyFunctionBuilder(ClosureExpr *closure, singleExpr = visitor.visit(closure->getBody()); // FIXME: Failure mode. - (void)TC.preCheckExpression(singleExpr, DC); + (void)TC.preCheckExpression(singleExpr, closure); // Record this expression for later. TC.appliedFunctionBuilders[closure].push_back( @@ -2937,7 +2937,7 @@ void ConstraintSystem::applyFunctionBuilder(ClosureExpr *closure, } - singleExpr = generateConstraints(singleExpr); + singleExpr = generateConstraints(singleExpr, closure); if (!singleExpr) return; diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index bd5888f951846..a72bb20457354 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -2489,7 +2489,7 @@ class ConstraintSystem { /// Generate constraints for the given (unchecked) expression. /// /// \returns a possibly-sanitized expression, or null if an error occurred. - Expr *generateConstraints(Expr *E); + Expr *generateConstraints(Expr *E, DeclContext *dc = nullptr); /// Generate constraints for binding the given pattern to the /// value of the given expression. diff --git a/test/Constraints/function_builder.swift b/test/Constraints/function_builder.swift index 05f791033d3d8..bd5052fb549a9 100644 --- a/test/Constraints/function_builder.swift +++ b/test/Constraints/function_builder.swift @@ -28,9 +28,10 @@ func tuplify(@TupleBuilder body: () -> T) { print(body()) } -// CHECK: (17, 3.14159, "Hello") +// CHECK: (17, 3.14159, "Hello, DSL") +let name = "dsl" tuplify { 17 3.14159 - "Hello" + "Hello, \(name.map { $0.uppercased() }.joined())" } From 1bd52383fc82f5f6d7243e1d9a4787492b2165cb Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 16 Apr 2019 22:39:29 -0700 Subject: [PATCH 12/38] [DSL] Allow function builders to opt in to "do" support via buildDo(). --- include/swift/AST/KnownIdentifiers.def | 1 + lib/Sema/ConstraintSystem.cpp | 77 ++++++++++++++++++++----- lib/Sema/ConstraintSystem.h | 21 +++++++ test/Constraints/function_builder.swift | 8 ++- 4 files changed, 90 insertions(+), 17 deletions(-) diff --git a/include/swift/AST/KnownIdentifiers.def b/include/swift/AST/KnownIdentifiers.def index 7d51fd00cdd4d..2c89af3af8145 100644 --- a/include/swift/AST/KnownIdentifiers.def +++ b/include/swift/AST/KnownIdentifiers.def @@ -32,6 +32,7 @@ IDENTIFIER(ArrayLiteralElement) IDENTIFIER(atIndexedSubscript) IDENTIFIER_(bridgeToObjectiveC) IDENTIFIER(buildBlock) +IDENTIFIER(buildDo) IDENTIFIER(Change) IDENTIFIER_WITH_NAME(code_, "_code") IDENTIFIER(CodingKeys) diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 5ef7f0c063d61..f6a3cfbc0e6f7 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2768,18 +2768,56 @@ bool constraints::isKnownKeyPathDecl(ASTContext &ctx, ValueDecl *decl) { namespace { /// Visitor to classify the contents of the given closure. class BuilderClosureVisitor - : public StmtVisitor { + : public StmtVisitor { ASTContext &ctx; + bool wantExpr; Type builderType; + NominalTypeDecl *builder = nullptr; + llvm::SmallDenseMap supportedOps; public: - BuilderClosureVisitor(ASTContext &ctx, Type builderType) - : ctx(ctx), builderType(builderType) { } - ReturnStmt *returnStmt = nullptr; Stmt *controlFlowStmt = nullptr; Decl *decl = nullptr; + private: + /// Produce a builder call to the given named function with the given arguments. + CallExpr *buildCallIfWanted(Identifier fnName, ArrayRef args) { + if (!wantExpr) + return nullptr; + + auto typeExpr = TypeExpr::createImplicit(builderType, ctx); + auto memberRef = new (ctx) UnresolvedDotExpr( + typeExpr, SourceLoc(), fnName, DeclNameLoc(), /*implicit=*/true); + return CallExpr::createImplicit(ctx, memberRef, args, { }); + } + + /// Check whether the builder supports the given operation. + bool builderSupports(Identifier fnName) { + auto known = supportedOps.find(fnName); + if (known != supportedOps.end()) { + return known->second; + } + + bool found = false; + for (auto decl : builder->lookupDirect(fnName)) { + if (auto func = dyn_cast(decl)) { + if (func->isStatic()) { + found = true; + break; + } + } + } + + return supportedOps[fnName] = found; + } + + public: + BuilderClosureVisitor(ASTContext &ctx, bool wantExpr, Type builderType) + : ctx(ctx), wantExpr(wantExpr), builderType(builderType) { + builder = builderType->getAnyNominal(); + } + #define CONTROL_FLOW_STMT(StmtClass) \ Expr *visit##StmtClass##Stmt(StmtClass##Stmt *stmt) { \ if (!controlFlowStmt) \ @@ -2809,15 +2847,8 @@ namespace { expressions.push_back(expr); } - if (!builderType) - return nullptr; - // Call Builder.buildBlock(... args ...) - auto typeExpr = TypeExpr::createImplicit(builderType, ctx); - auto memberRef = new (ctx) UnresolvedDotExpr( - typeExpr, SourceLoc(), ctx.Id_buildBlock, DeclNameLoc(), - /*implicit=*/true); - return CallExpr::createImplicit(ctx, memberRef, expressions, { }); + return buildCallIfWanted(ctx.Id_buildBlock, expressions); } Expr *visitReturnStmt(ReturnStmt *returnStmt) { @@ -2826,7 +2857,19 @@ namespace { return nullptr; } - CONTROL_FLOW_STMT(Do) + Expr *visitDoStmt(DoStmt *doStmt) { + if (!builderSupports(ctx.Id_buildDo)) { + this->controlFlowStmt = doStmt; + return nullptr; + } + + auto arg = visit(doStmt->getBody()); + if (!arg) + return nullptr; + + return buildCallIfWanted(ctx.Id_buildDo, arg); + } + CONTROL_FLOW_STMT(Yield) CONTROL_FLOW_STMT(Defer) CONTROL_FLOW_STMT(If) @@ -2851,11 +2894,12 @@ namespace { BuilderClosureKind swift::classifyBuilderClosure(ASTContext &ctx, ClosureExpr *closure, + Type builderType, bool diagnose) { if (closure->hasSingleExpressionBody()) return BuilderClosureKind::SingleExpression; - BuilderClosureVisitor visitor(ctx, Type()); + BuilderClosureVisitor visitor(ctx, /*wantExpr=*/false, builderType); (void)visitor.visit(closure->getBody()); // The presence of an explicit return makes this not a builder. @@ -2898,7 +2942,7 @@ void ConstraintSystem::applyFunctionBuilder(ClosureExpr *closure, assert(builder && "Bad function builder type"); assert(builder->getAttrs().hasAttribute()); switch (classifyBuilderClosure( - getASTContext(), closure, /*diagnose=*/false)) { + getASTContext(), closure, builderType, /*diagnose=*/false)) { case BuilderClosureKind::ExplicitReturn: case BuilderClosureKind::UnsupportedConstruct: return; @@ -2925,7 +2969,8 @@ void ConstraintSystem::applyFunctionBuilder(ClosureExpr *closure, // If we didn't find anything yet, go build the expression. if (!singleExpr) { - BuilderClosureVisitor visitor(getASTContext(), builderType); + BuilderClosureVisitor visitor( + getASTContext(), /*wantExpr=*/true, builderType); singleExpr = visitor.visit(closure->getBody()); // FIXME: Failure mode. diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index a72bb20457354..a8b49ae6df612 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -4127,6 +4127,27 @@ bool exprNeedsParensOutsideFollowingOperator( /// Determine whether this is a SIMD operator. bool isSIMDOperator(ValueDecl *value); +/// Describes how a particular closure can be used with a closure builder (or not). +enum class BuilderClosureKind { + /// The closure appears to be supported for use with a closure builder. + Supported, + /// The closure contains a single expression. + SingleExpression, + /// The closure contains an explicit return. + ExplicitReturn, + /// The closure contains an unsupported construct. + UnsupportedConstruct, +}; + +/// Classify the given closure based on its suitability as a builder closure. +/// +/// \param diagnoseNonBuilder whether we should diagnose any constructs that occur in the closure +/// that won't work with a closure builder. +BuilderClosureKind classifyBuilderClosure(ASTContext &ctx, + ClosureExpr *closure, + Type builderType, + bool diagnoseNonBuilder); + } // end namespace swift #endif // LLVM_SWIFT_SEMA_CONSTRAINT_SYSTEM_H diff --git a/test/Constraints/function_builder.swift b/test/Constraints/function_builder.swift index bd5052fb549a9..bb77dfe668fae 100644 --- a/test/Constraints/function_builder.swift +++ b/test/Constraints/function_builder.swift @@ -22,16 +22,22 @@ struct TupleBuilder { ) -> (T1, T2, T3, T4, T5) { return (t1, t2, t3, t4, t5) } + + static func buildDo(_ value: T) -> T { return value } } func tuplify(@TupleBuilder body: () -> T) { print(body()) } -// CHECK: (17, 3.14159, "Hello, DSL") +// CHECK: (17, 3.14159, "Hello, DSL", (["nested", "do"], 6)) let name = "dsl" tuplify { 17 3.14159 "Hello, \(name.map { $0.uppercased() }.joined())" + do { + ["nested", "do"] + 1 + 2 + 3 + } } From 0e595484480997f3b39ec935faceca98bfa55211 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 16 Apr 2019 23:34:59 -0700 Subject: [PATCH 13/38] [DSL] Allow function builders to opt in to "if" statements. If a function builder contains a buildIf function, then "if" statements will be supported by passing an optional of the "then" branch. "if" statements with an "else" statement are unsupported at present. --- include/swift/AST/KnownIdentifiers.def | 1 + lib/AST/Expr.cpp | 2 ++ lib/Sema/ConstraintSystem.cpp | 46 ++++++++++++++++++++++++- test/Constraints/function_builder.swift | 22 +++++++++--- 4 files changed, 66 insertions(+), 5 deletions(-) diff --git a/include/swift/AST/KnownIdentifiers.def b/include/swift/AST/KnownIdentifiers.def index 2c89af3af8145..58f5b5bb2f727 100644 --- a/include/swift/AST/KnownIdentifiers.def +++ b/include/swift/AST/KnownIdentifiers.def @@ -32,6 +32,7 @@ IDENTIFIER(ArrayLiteralElement) IDENTIFIER(atIndexedSubscript) IDENTIFIER_(bridgeToObjectiveC) IDENTIFIER(buildBlock) +IDENTIFIER(buildIf) IDENTIFIER(buildDo) IDENTIFIER(Change) IDENTIFIER_WITH_NAME(code_, "_code") diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index c5b07de2e2e73..4ed365fcdc980 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -117,6 +117,8 @@ namespace { return getStartLocImpl(E); } template static SourceRange getSourceRange(const T *E) { + if (E->getStartLoc().isInvalid() != E->getEndLoc().isInvalid()) + return SourceRange(); return { E->getStartLoc(), E->getEndLoc() }; } }; diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index f6a3cfbc0e6f7..e060997647bde 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2872,7 +2872,51 @@ namespace { CONTROL_FLOW_STMT(Yield) CONTROL_FLOW_STMT(Defer) - CONTROL_FLOW_STMT(If) + + static Expr *getTrivialBooleanCondition(StmtCondition condition) { + if (condition.size() != 1) + return nullptr; + + return condition.front().getBooleanOrNull(); + } + + Expr *visitIfStmt(IfStmt *ifStmt) { + if (!builderSupports(ctx.Id_buildIf) || + ifStmt->getElseStmt() || + !getTrivialBooleanCondition(ifStmt->getCond())) { + this->controlFlowStmt = ifStmt; + return nullptr; + } + + auto thenArg = visit(ifStmt->getThenStmt()); + if (!thenArg) + return nullptr; + + if (!wantExpr) + return nullptr; + + auto optionalDecl = ctx.getOptionalDecl(); + auto optionalType = optionalDecl->getDeclaredType(); + + auto optionalTypeExpr = TypeExpr::createImplicit(optionalType, ctx); + auto someRef = new (ctx) UnresolvedDotExpr( + optionalTypeExpr, SourceLoc(), ctx.getIdentifier("some"), + DeclNameLoc(), /*implicit=*/true); + auto someCall = CallExpr::createImplicit(ctx, someRef, thenArg, { }); + + optionalTypeExpr = TypeExpr::createImplicit(optionalType, ctx); + auto noneRef = new (ctx) UnresolvedDotExpr( + optionalTypeExpr, ifStmt->getEndLoc(), ctx.getIdentifier("none"), + DeclNameLoc(ifStmt->getEndLoc()), /*implicit=*/true); + + auto ifExpr = new (ctx) IfExpr( + getTrivialBooleanCondition(ifStmt->getCond()), + SourceLoc(), someCall, + SourceLoc(), noneRef); + ifExpr->setImplicit(); + return buildCallIfWanted(ctx.Id_buildIf, ifExpr); + } + CONTROL_FLOW_STMT(Guard) CONTROL_FLOW_STMT(While) CONTROL_FLOW_STMT(DoCatch) diff --git a/test/Constraints/function_builder.swift b/test/Constraints/function_builder.swift index bb77dfe668fae..ef0dc431228b0 100644 --- a/test/Constraints/function_builder.swift +++ b/test/Constraints/function_builder.swift @@ -24,15 +24,16 @@ struct TupleBuilder { } static func buildDo(_ value: T) -> T { return value } + static func buildIf(_ value: T?) -> T? { return value } } -func tuplify(@TupleBuilder body: () -> T) { - print(body()) +func tuplify(_ cond: Bool, @TupleBuilder body: (Bool) -> T) { + print(body(cond)) } -// CHECK: (17, 3.14159, "Hello, DSL", (["nested", "do"], 6)) +// CHECK: (17, 3.14159, "Hello, DSL", (["nested", "do"], 6), Optional((2.71828, ["if", "stmt"]))) let name = "dsl" -tuplify { +tuplify(true) { 17 3.14159 "Hello, \(name.map { $0.uppercased() }.joined())" @@ -40,4 +41,17 @@ tuplify { ["nested", "do"] 1 + 2 + 3 } + if $0 { + 2.71828 + ["if", "stmt"] + } +} + +// CHECK: ("Empty optional", nil) +tuplify(false) { + "Empty optional" + if $0 { + 2.71828 + ["if", "stmt"] + } } From efc75ed8295ec098be46e20a72b1acdbe4de87d7 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 18 Apr 2019 12:09:35 -0700 Subject: [PATCH 14/38] [Function builders] Diagnose unhandled closure constructs within the type checker --- include/swift/AST/DiagnosticsSema.def | 12 ++ lib/Sema/CSFix.cpp | 33 +++++ lib/Sema/CSFix.h | 32 ++++ lib/Sema/CSSimplify.cpp | 8 +- lib/Sema/ConstraintSystem.cpp | 138 +++++++----------- lib/Sema/ConstraintSystem.h | 25 +--- lib/Sema/TypeChecker.h | 9 +- test/Constraints/function_builder_diags.swift | 126 ++++++++++++++++ 8 files changed, 265 insertions(+), 118 deletions(-) create mode 100644 test/Constraints/function_builder_diags.swift diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 851b467da715f..5b8b8b8d426af 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4444,6 +4444,18 @@ ERROR(property_wrapper_type_not_usable_from_inline,none, //------------------------------------------------------------------------------ // MARK: function builder diagnostics //------------------------------------------------------------------------------ +ERROR(function_builder_decl, none, + "closure containing a declaration cannot be used with function " + "builder %0", (DeclName)) +NOTE(note_function_builder_decl, none, + "closure containing a declaration cannot be used with function " + "builder %0", (DeclName)) +ERROR(function_builder_control_flow, none, + "closure containing control flow statement cannot be used with function " + "builder %0", (DeclName)) +NOTE(note_function_builder_control_flow, none, + "closure containing control flow statement cannot be used with function " + "builder %0", (DeclName)) ERROR(function_builder_reserved_name, none, "function builder type name must start with an uppercase letter", ()) ERROR(function_builder_attribute_not_on_parameter, none, diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp index 2cf356fa98103..ed47afef63a47 100644 --- a/lib/Sema/CSFix.cpp +++ b/lib/Sema/CSFix.cpp @@ -540,3 +540,36 @@ CollectionElementContextualMismatch::create(ConstraintSystem &cs, Type srcType, return new (cs.getAllocator()) CollectionElementContextualMismatch(cs, srcType, dstType, locator); } + +SkipUnhandledConstructInFunctionBuilder * +SkipUnhandledConstructInFunctionBuilder::create(ConstraintSystem &cs, + UnhandledNode unhandled, + NominalTypeDecl *builder, + ConstraintLocator *locator) { + return new (cs.getAllocator()) + SkipUnhandledConstructInFunctionBuilder(cs, unhandled, builder, locator); +} + +bool SkipUnhandledConstructInFunctionBuilder::diagnose(Expr *root, + bool asNote) const { + auto &diags = getConstraintSystem().getASTContext().Diags; + if (auto stmt = unhandled.dyn_cast()) { + diags.diagnose(stmt->getStartLoc(), + asNote ? diag::note_function_builder_control_flow + : diag::function_builder_control_flow, + builder->getFullName()); + } else { + auto decl = unhandled.get(); + decl->diagnose(asNote ? diag::note_function_builder_decl + : diag::function_builder_decl, + builder->getFullName()); + } + + if (!asNote) { + builder->diagnose(diag::kind_declname_declared_here, + builder->getDescriptiveKind(), + builder->getFullName()); + } + + return true; +} diff --git a/lib/Sema/CSFix.h b/lib/Sema/CSFix.h index 327de47621a80..68aba52f34c23 100644 --- a/lib/Sema/CSFix.h +++ b/lib/Sema/CSFix.h @@ -153,6 +153,10 @@ enum class FixKind : uint8_t { /// Remove `return` or default last expression of single expression /// function to `Void` to conform to expected result type. RemoveReturn, + + /// Skip any unhandled constructs that occur within a closure argument that matches up with a + /// parameter that has a function builder. + SkipUnhandledConstructInFunctionBuilder, }; class ConstraintFix { @@ -917,6 +921,34 @@ class CollectionElementContextualMismatch final : public ContextualMismatch { ConstraintLocator *locator); }; +class SkipUnhandledConstructInFunctionBuilder final : public ConstraintFix { +public: + using UnhandledNode = llvm::PointerUnion; + +private: + UnhandledNode unhandled; + NominalTypeDecl *builder; + + SkipUnhandledConstructInFunctionBuilder(ConstraintSystem &cs, + UnhandledNode unhandled, + NominalTypeDecl *builder, + ConstraintLocator *locator) + : ConstraintFix(cs, FixKind::SkipUnhandledConstructInFunctionBuilder, + locator), + unhandled(unhandled), builder(builder) { } + +public: + std::string getName() const override { + return "skip unhandled constructs when applying a function builder"; + } + + bool diagnose(Expr *root, bool asNote = false) const override; + + static SkipUnhandledConstructInFunctionBuilder * + create(ConstraintSystem &cs, UnhandledNode unhandledNode, + NominalTypeDecl *builder, ConstraintLocator *locator); +}; + } // end namespace constraints } // end namespace swift diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index fec2fbbb3a6ad..bc219382b8928 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -985,7 +985,10 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments( = paramInfo.getFunctionBuilderType(paramIdx)) { Expr *arg = getArgumentExpr(locator.getAnchor(), argIdx); if (auto closure = dyn_cast_or_null(arg)) { - cs.applyFunctionBuilder(closure, functionBuilderType, locator); + auto result = + cs.applyFunctionBuilder(closure, functionBuilderType, loc); + if (result.isFailure()) + return result; } } @@ -6604,7 +6607,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint( case FixKind::SkipSameTypeRequirement: case FixKind::SkipSuperclassRequirement: case FixKind::ContextualMismatch: - case FixKind::AddMissingArguments: { + case FixKind::AddMissingArguments: + case FixKind::SkipUnhandledConstructInFunctionBuilder: { return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved; } diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index e060997647bde..5666aca62ebb3 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -18,6 +18,7 @@ #include "ConstraintSystem.h" #include "ConstraintGraph.h" #include "CSDiagnostics.h" +#include "CSFix.h" #include "TypeCheckType.h" #include "swift/AST/GenericEnvironment.h" #include "swift/AST/ParameterList.h" @@ -2777,8 +2778,7 @@ namespace { public: ReturnStmt *returnStmt = nullptr; - Stmt *controlFlowStmt = nullptr; - Decl *decl = nullptr; + SkipUnhandledConstructInFunctionBuilder::UnhandledNode unhandledNode; private: /// Produce a builder call to the given named function with the given arguments. @@ -2820,8 +2820,8 @@ namespace { #define CONTROL_FLOW_STMT(StmtClass) \ Expr *visit##StmtClass##Stmt(StmtClass##Stmt *stmt) { \ - if (!controlFlowStmt) \ - controlFlowStmt = stmt; \ + if (!unhandledNode) \ + unhandledNode = stmt; \ \ return nullptr; \ } @@ -2837,8 +2837,8 @@ namespace { } if (auto decl = node.dyn_cast()) { - if (!this->decl) - this->decl = decl; + if (!unhandledNode) + unhandledNode = decl; continue; } @@ -2859,7 +2859,8 @@ namespace { Expr *visitDoStmt(DoStmt *doStmt) { if (!builderSupports(ctx.Id_buildDo)) { - this->controlFlowStmt = doStmt; + if (!unhandledNode) + unhandledNode = doStmt; return nullptr; } @@ -2884,7 +2885,8 @@ namespace { if (!builderSupports(ctx.Id_buildIf) || ifStmt->getElseStmt() || !getTrivialBooleanCondition(ifStmt->getCond())) { - this->controlFlowStmt = ifStmt; + if (!unhandledNode) + unhandledNode = ifStmt; return nullptr; } @@ -2936,99 +2938,60 @@ namespace { }; } -BuilderClosureKind swift::classifyBuilderClosure(ASTContext &ctx, - ClosureExpr *closure, - Type builderType, - bool diagnose) { - if (closure->hasSingleExpressionBody()) - return BuilderClosureKind::SingleExpression; - - BuilderClosureVisitor visitor(ctx, /*wantExpr=*/false, builderType); - (void)visitor.visit(closure->getBody()); - - // The presence of an explicit return makes this not a builder. - if (visitor.returnStmt) { - if (diagnose) { - ctx.Diags.diagnose(visitor.returnStmt->getReturnLoc(), - diag::closure_builder_return_stmt); - } - - return BuilderClosureKind::ExplicitReturn; - } - - // The presence of control flow makes this not work with closure builders. - if (visitor.controlFlowStmt) { - if (diagnose) { - ctx.Diags.diagnose(visitor.controlFlowStmt->getStartLoc(), - diag::closure_builder_control_flow); - } - - return BuilderClosureKind::UnsupportedConstruct; - } - - // The presence of any declaration makes this not work with closure builders. - if (visitor.decl) { - if (diagnose) { - visitor.decl->diagnose(diag::closure_builder_decl); - } - - return BuilderClosureKind::UnsupportedConstruct; - } - - return BuilderClosureKind::Supported; -} - - -void ConstraintSystem::applyFunctionBuilder(ClosureExpr *closure, - Type builderType, - ConstraintLocatorBuilder locator) { +ConstraintSystem::TypeMatchResult ConstraintSystem::applyFunctionBuilder( + ClosureExpr *closure, Type builderType, ConstraintLocatorBuilder locator) { auto builder = builderType->getAnyNominal(); assert(builder && "Bad function builder type"); assert(builder->getAttrs().hasAttribute()); - switch (classifyBuilderClosure( - getASTContext(), closure, builderType, /*diagnose=*/false)) { - case BuilderClosureKind::ExplicitReturn: - case BuilderClosureKind::UnsupportedConstruct: - return; - - case BuilderClosureKind::SingleExpression: - // FIXME: We might be able to do this, but it's not clear. - return; - case BuilderClosureKind::Supported: - break; - } + // Check the form of this closure to see if we can apply the function-builder + // translation at all. + { + // FIXME: Right now, single-expression closures suppress the function + // builder translation. + if (closure->hasSingleExpressionBody()) + return getTypeMatchSuccess(); - auto &appliedBuilders = TC.appliedFunctionBuilders[closure]; + // Check whether we can apply this function builder. + BuilderClosureVisitor visitor( + getASTContext(), /*wantExpr=*/false, builderType); + (void)visitor.visit(closure->getBody()); - // Check whether we have already applied this function builder to - // this closure. - Expr *singleExpr = nullptr; - for (const auto &applied : appliedBuilders) { - if (applied.first->isEqual(builderType)) { - singleExpr = applied.second; - break; + // The presence of an explicit return suppresses the function builder + // translation. + if (visitor.returnStmt) { + return getTypeMatchSuccess(); } - } - - // If we didn't find anything yet, go build the expression. - if (!singleExpr) { - BuilderClosureVisitor visitor( - getASTContext(), /*wantExpr=*/true, builderType); - singleExpr = visitor.visit(closure->getBody()); - // FIXME: Failure mode. - (void)TC.preCheckExpression(singleExpr, closure); + // If we saw a control-flow statement or declaration that the builder + // cannot handle, we don't have a well-formed function builder application. + if (visitor.unhandledNode) { + // If we aren't supposed to attempt fixes, fail. + if (!shouldAttemptFixes()) { + return getTypeMatchFailure(locator); + } - // Record this expression for later. - TC.appliedFunctionBuilders[closure].push_back( - {builderType->getCanonicalType(), singleExpr}); + // Record the first unhandled construct as a fix. + if (recordFix( + SkipUnhandledConstructInFunctionBuilder::create( + *this, visitor.unhandledNode, builder, + getConstraintLocator(locator)))) { + return getTypeMatchFailure(locator); + } + } } + BuilderClosureVisitor visitor( + getASTContext(), /*wantExpr=*/true, builderType); + Expr *singleExpr = visitor.visit(closure->getBody()); + + if (TC.precheckedClosures.insert(closure).second && + TC.preCheckExpression(singleExpr, closure)) + return getTypeMatchFailure(locator); singleExpr = generateConstraints(singleExpr, closure); if (!singleExpr) - return; + return getTypeMatchFailure(locator); Type transformedType = getType(singleExpr); assert(transformedType && "Missing type"); @@ -3043,4 +3006,5 @@ void ConstraintSystem::applyFunctionBuilder(ClosureExpr *closure, auto fnType = closureType->castTo(); addConstraint(ConstraintKind::Equal, fnType->getResult(), transformedType, locator); + return getTypeMatchSuccess(); } diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index a8b49ae6df612..8a94cb5e96c3d 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -2977,8 +2977,8 @@ class ConstraintSystem { void simplifyDisjunctionChoice(Constraint *choice); /// Apply the given function builder to the closure expression. - void applyFunctionBuilder(ClosureExpr *closure, Type builderType, - ConstraintLocatorBuilder locator); + TypeMatchResult applyFunctionBuilder(ClosureExpr *closure, Type builderType, + ConstraintLocatorBuilder locator); private: /// The kind of bindings that are permitted. @@ -4127,27 +4127,6 @@ bool exprNeedsParensOutsideFollowingOperator( /// Determine whether this is a SIMD operator. bool isSIMDOperator(ValueDecl *value); -/// Describes how a particular closure can be used with a closure builder (or not). -enum class BuilderClosureKind { - /// The closure appears to be supported for use with a closure builder. - Supported, - /// The closure contains a single expression. - SingleExpression, - /// The closure contains an explicit return. - ExplicitReturn, - /// The closure contains an unsupported construct. - UnsupportedConstruct, -}; - -/// Classify the given closure based on its suitability as a builder closure. -/// -/// \param diagnoseNonBuilder whether we should diagnose any constructs that occur in the closure -/// that won't work with a closure builder. -BuilderClosureKind classifyBuilderClosure(ASTContext &ctx, - ClosureExpr *closure, - Type builderType, - bool diagnoseNonBuilder); - } // end namespace swift #endif // LLVM_SWIFT_SEMA_CONSTRAINT_SYSTEM_H diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index 6cfdc0fbd9642..8b83a53e9b9f8 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -669,13 +669,9 @@ class TypeChecker final : public LazyResolver { /// when executing scripts. bool InImmediateMode = false; -public: - /// Cached mapping from closure expressions that have had function builders applied - /// to the (builder type, resulting single expression) pairs for that closure expression. - llvm::DenseMap, 2>> - appliedFunctionBuilders; + /// Closure expressions that have already been prechecked. + llvm::SmallPtrSet precheckedClosures; -private: /// A helper to construct and typecheck call to super.init(). /// /// \returns NULL if the constructed expression does not typecheck. @@ -683,6 +679,7 @@ class TypeChecker final : public LazyResolver { TypeChecker(ASTContext &Ctx); friend class ASTContext; + friend class constraints::ConstraintSystem; public: /// Create a new type checker instance for the given ASTContext, if it diff --git a/test/Constraints/function_builder_diags.swift b/test/Constraints/function_builder_diags.swift new file mode 100644 index 0000000000000..298679ac963cd --- /dev/null +++ b/test/Constraints/function_builder_diags.swift @@ -0,0 +1,126 @@ +// RUN: %target-typecheck-verify-swift + +@_functionBuilder +struct TupleBuilder { // expected-note 2{{struct 'TupleBuilder' declared here}} + static func buildBlock() -> () { } + + static func buildBlock(_ t1: T1) -> T1 { + return t1 + } + + static func buildBlock(_ t1: T1, _ t2: T2) -> (T1, T2) { + return (t1, t2) + } + + static func buildBlock(_ t1: T1, _ t2: T2, _ t3: T3) + -> (T1, T2, T3) { + return (t1, t2, t3) + } + + static func buildBlock(_ t1: T1, _ t2: T2, _ t3: T3, _ t4: T4) + -> (T1, T2, T3, T4) { + return (t1, t2, t3, t4) + } + + static func buildBlock( + _ t1: T1, _ t2: T2, _ t3: T3, _ t4: T4, _ t5: T5 + ) -> (T1, T2, T3, T4, T5) { + return (t1, t2, t3, t4, t5) + } + + static func buildDo(_ value: T) -> T { return value } + static func buildIf(_ value: T?) -> T? { return value } +} + +@_functionBuilder +struct TupleBuilderWithoutIf { // expected-note {{struct 'TupleBuilderWithoutIf' declared here}} + static func buildBlock() -> () { } + + static func buildBlock(_ t1: T1) -> T1 { + return t1 + } + + static func buildBlock(_ t1: T1, _ t2: T2) -> (T1, T2) { + return (t1, t2) + } + + static func buildBlock(_ t1: T1, _ t2: T2, _ t3: T3) + -> (T1, T2, T3) { + return (t1, t2, t3) + } + + static func buildBlock(_ t1: T1, _ t2: T2, _ t3: T3, _ t4: T4) + -> (T1, T2, T3, T4) { + return (t1, t2, t3, t4) + } + + static func buildBlock( + _ t1: T1, _ t2: T2, _ t3: T3, _ t4: T4, _ t5: T5 + ) -> (T1, T2, T3, T4, T5) { + return (t1, t2, t3, t4, t5) + } + + static func buildDo(_ value: T) -> T { return value } +} + +func tuplify(_ cond: Bool, @TupleBuilder body: (Bool) -> T) { + print(body(cond)) +} + +func tuplifyWithoutIf(_ cond: Bool, @TupleBuilderWithoutIf body: (Bool) -> T) { + print(body(cond)) +} + +func testDiags() { + // For loop + tuplify(true) { _ in + 17 + for c in name { // expected-error{{closure containing control flow statement cannot be used with function builder 'TupleBuilder'}} + } + } + + // Declarations + tuplify(true) { _ in + 17 + let x = 17 // expected-error{{closure containing a declaration cannot be used with function builder 'TupleBuilder'}} + x + 25 + } + + // Statements unsupported by the particular builder. + tuplifyWithoutIf(true) { + if $0 { // expected-error{{closure containing control flow statement cannot be used with function builder 'TupleBuilderWithoutIf'}} + "hello" + } + } +} + +struct A { } +struct B { } + +func overloadedTuplify(_ cond: Bool, @TupleBuilder body: (Bool) -> T) -> A { + return A() +} + +func overloadedTuplify(_ cond: Bool, @TupleBuilderWithoutIf body: (Bool) -> T) -> B { + return B() +} + +func testOverloading(name: String) { + let a1 = overloadedTuplify(true) { b in + if b { + "Hello, \(name)" + } + } + + let _: A = a1 + +#if false + // FIXME: Using the overloads causes a crash, which is likely related + // to us generating the constraints from the child expressions multiple + // times. + let b1 = overloadedTuplify(true) { b in + b ? "Hello, \(name)" : "Goodbye" + 42 + } +#endif +} From 32b6f555d07641f3bc95c4ff9a61d0d823e09f6e Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 18 Apr 2019 14:22:07 -0700 Subject: [PATCH 15/38] [Function builders] Create a FailureDiagnostic for unhandled constructs. --- lib/Sema/CSDiagnostics.cpp | 34 ++++++++++++++++++++++++++++++++++ lib/Sema/CSDiagnostics.h | 23 +++++++++++++++++++++++ lib/Sema/CSFix.cpp | 23 +++-------------------- 3 files changed, 60 insertions(+), 20 deletions(-) diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index 38a8d971ed71d..6f5bb03ce57be 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -2908,3 +2908,37 @@ MissingContextualConformanceFailure::getDiagnosticFor( } return None; } + +bool SkipUnhandledConstructInFunctionBuilderFailure::diagnoseAsError() { + if (auto stmt = unhandled.dyn_cast()) { + emitDiagnostic(stmt->getStartLoc(), + diag::function_builder_control_flow, + builder->getFullName()); + } else { + auto decl = unhandled.get(); + emitDiagnostic(decl, + diag::function_builder_decl, + builder->getFullName()); + } + + emitDiagnostic(builder, + diag::kind_declname_declared_here, + builder->getDescriptiveKind(), + builder->getFullName()); + return true; +} + +bool SkipUnhandledConstructInFunctionBuilderFailure::diagnoseAsNote() { + if (auto stmt = unhandled.dyn_cast()) { + emitDiagnostic(stmt->getStartLoc(), + diag::note_function_builder_control_flow, + builder->getFullName()); + } else { + auto decl = unhandled.get(); + emitDiagnostic(decl, + diag::note_function_builder_decl, + builder->getFullName()); + } + + return true; +} diff --git a/lib/Sema/CSDiagnostics.h b/lib/Sema/CSDiagnostics.h index 547f08ab7694e..df86602c4a3ed 100644 --- a/lib/Sema/CSDiagnostics.h +++ b/lib/Sema/CSDiagnostics.h @@ -1223,6 +1223,29 @@ class MissingContextualConformanceFailure final : public ContextualFailure { getDiagnosticFor(ContextualTypePurpose purpose); }; +class SkipUnhandledConstructInFunctionBuilderFailure final + : public FailureDiagnostic { +public: + using UnhandledNode = llvm::PointerUnion; + + UnhandledNode unhandled; + NominalTypeDecl *builder; + +public: + SkipUnhandledConstructInFunctionBuilderFailure(Expr *root, + ConstraintSystem &cs, + UnhandledNode unhandled, + NominalTypeDecl *builder, + ConstraintLocator *locator) + : FailureDiagnostic(root, cs, locator), + unhandled(unhandled), + builder(builder) { } + +public: + bool diagnoseAsError() override; + bool diagnoseAsNote() override; +}; + } // end namespace constraints } // end namespace swift diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp index ed47afef63a47..c94322cdc5741 100644 --- a/lib/Sema/CSFix.cpp +++ b/lib/Sema/CSFix.cpp @@ -552,24 +552,7 @@ SkipUnhandledConstructInFunctionBuilder::create(ConstraintSystem &cs, bool SkipUnhandledConstructInFunctionBuilder::diagnose(Expr *root, bool asNote) const { - auto &diags = getConstraintSystem().getASTContext().Diags; - if (auto stmt = unhandled.dyn_cast()) { - diags.diagnose(stmt->getStartLoc(), - asNote ? diag::note_function_builder_control_flow - : diag::function_builder_control_flow, - builder->getFullName()); - } else { - auto decl = unhandled.get(); - decl->diagnose(asNote ? diag::note_function_builder_decl - : diag::function_builder_decl, - builder->getFullName()); - } - - if (!asNote) { - builder->diagnose(diag::kind_declname_declared_here, - builder->getDescriptiveKind(), - builder->getFullName()); - } - - return true; + SkipUnhandledConstructInFunctionBuilderFailure failure( + root, getConstraintSystem(), unhandled, builder, getLocator()); + return failure.diagnose(asNote); } From b7279fb7f369b7892a12678e53530af50b29b5ad Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 18 Apr 2019 15:38:59 -0700 Subject: [PATCH 16/38] [Type checker] Collapse some duplicate code. Thanks, Pavel! --- lib/Sema/CSDiagnostics.cpp | 24 ++++++++++-------------- lib/Sema/CSDiagnostics.h | 3 ++- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index 6f5bb03ce57be..524c465fba6a9 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -2909,18 +2909,24 @@ MissingContextualConformanceFailure::getDiagnosticFor( return None; } -bool SkipUnhandledConstructInFunctionBuilderFailure::diagnoseAsError() { +void SkipUnhandledConstructInFunctionBuilderFailure::diagnosePrimary( + bool asNote) { if (auto stmt = unhandled.dyn_cast()) { emitDiagnostic(stmt->getStartLoc(), - diag::function_builder_control_flow, + asNote? diag::note_function_builder_control_flow + : diag::function_builder_control_flow, builder->getFullName()); } else { auto decl = unhandled.get(); emitDiagnostic(decl, - diag::function_builder_decl, + asNote ? diag::note_function_builder_decl + : diag::function_builder_decl, builder->getFullName()); } +} +bool SkipUnhandledConstructInFunctionBuilderFailure::diagnoseAsError() { + diagnosePrimary(/*asNote=*/false); emitDiagnostic(builder, diag::kind_declname_declared_here, builder->getDescriptiveKind(), @@ -2929,16 +2935,6 @@ bool SkipUnhandledConstructInFunctionBuilderFailure::diagnoseAsError() { } bool SkipUnhandledConstructInFunctionBuilderFailure::diagnoseAsNote() { - if (auto stmt = unhandled.dyn_cast()) { - emitDiagnostic(stmt->getStartLoc(), - diag::note_function_builder_control_flow, - builder->getFullName()); - } else { - auto decl = unhandled.get(); - emitDiagnostic(decl, - diag::note_function_builder_decl, - builder->getFullName()); - } - + diagnosePrimary(/*asNote=*/true); return true; } diff --git a/lib/Sema/CSDiagnostics.h b/lib/Sema/CSDiagnostics.h index df86602c4a3ed..877788c60dd2d 100644 --- a/lib/Sema/CSDiagnostics.h +++ b/lib/Sema/CSDiagnostics.h @@ -1231,6 +1231,8 @@ class SkipUnhandledConstructInFunctionBuilderFailure final UnhandledNode unhandled; NominalTypeDecl *builder; + void diagnosePrimary(bool asNote); + public: SkipUnhandledConstructInFunctionBuilderFailure(Expr *root, ConstraintSystem &cs, @@ -1241,7 +1243,6 @@ class SkipUnhandledConstructInFunctionBuilderFailure final unhandled(unhandled), builder(builder) { } -public: bool diagnoseAsError() override; bool diagnoseAsNote() override; }; From d88e0f1bff8e836eb7980c52e9f4cb8cb9cf7841 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 23 Apr 2019 16:26:24 -0700 Subject: [PATCH 17/38] [Function builders] Allow uses of generic function builders. Use the opened type from the callee declaration to open up references to generic function builders that contain type parameters. This allows general use of generic function builders. --- include/swift/AST/DiagnosticsSema.def | 3 - lib/Sema/CSGen.cpp | 4 +- lib/Sema/CSSimplify.cpp | 31 ++++++---- lib/Sema/ConstraintSystem.cpp | 35 ++++++++--- lib/Sema/ConstraintSystem.h | 1 + lib/Sema/TypeCheckRequestFunctions.cpp | 9 +-- test/Constraints/function_builder.swift | 81 +++++++++++++++++++++++++ test/decl/var/function_builders.swift | 13 +++- 8 files changed, 143 insertions(+), 34 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 5b8b8b8d426af..34a5e896351b6 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4475,9 +4475,6 @@ NOTE(previous_function_builder_here, none, "previous function builder specified here", ()) ERROR(function_builder_arguments, none, "function builder attributes cannot have arguments", ()) -ERROR(function_builder_type_contextual, none, - "function builder type %0 cannot depend on the current generic context", - (Type)) #ifndef DIAG_NO_UNDEF # if defined(DIAG) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 12a51c0501b1f..3af43286a184d 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -1395,7 +1395,9 @@ namespace { Type type; // If this is an implicit TypeExpr, don't validate its contents. auto &typeLoc = E->getTypeLoc(); - if (typeLoc.wasValidated()) { + if (E->isImplicit() && CS.hasType(E)) { + type = CS.getType(E); + } else if (typeLoc.wasValidated()) { type = typeLoc.getType(); } else if (typeLoc.hasLocation()) { type = resolveTypeReferenceInExpression(typeLoc); diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index bc219382b8928..6fc5f1c17c490 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -696,7 +696,8 @@ matchCallArguments(ArrayRef args, /// Find the callee declaration and uncurry level for a given call /// locator. -static std::tuple, bool> +static std::tuple, bool, + ConstraintLocator *> getCalleeDeclAndArgs(ConstraintSystem &cs, ConstraintLocatorBuilder callLocator, SmallVectorImpl &argLabelsScratch) { @@ -708,14 +709,14 @@ getCalleeDeclAndArgs(ConstraintSystem &cs, auto callExpr = callLocator.getLocatorParts(path); if (!callExpr) return std::make_tuple(nullptr, /*hasCurriedSelf=*/false, argLabels, - hasTrailingClosure); + hasTrailingClosure, nullptr); // Our remaining path can only be 'ApplyArgument'. if (!path.empty() && !(path.size() <= 2 && path.back().getKind() == ConstraintLocator::ApplyArgument)) return std::make_tuple(nullptr, /*hasCurriedSelf=*/false, argLabels, - hasTrailingClosure); + hasTrailingClosure, nullptr); // Dig out the callee. ConstraintLocator *targetLocator; @@ -740,12 +741,12 @@ getCalleeDeclAndArgs(ConstraintSystem &cs, path[0].getKind() != ConstraintLocator::KeyPathComponent || path[1].getKind() != ConstraintLocator::ApplyArgument) return std::make_tuple(nullptr, /*hasCurriedSelf=*/false, argLabels, - hasTrailingClosure); + hasTrailingClosure, nullptr); auto componentIndex = path[0].getValue(); if (componentIndex >= keyPath->getComponents().size()) return std::make_tuple(nullptr, /*hasCurriedSelf=*/false, argLabels, - hasTrailingClosure); + hasTrailingClosure, nullptr); auto &component = keyPath->getComponents()[componentIndex]; switch (component.getKind()) { @@ -765,7 +766,7 @@ getCalleeDeclAndArgs(ConstraintSystem &cs, case KeyPathExpr::Component::Kind::Identity: case KeyPathExpr::Component::Kind::TupleElement: return std::make_tuple(nullptr, /*hasCurriedSelf=*/false, argLabels, - hasTrailingClosure); + hasTrailingClosure, nullptr); } } else { @@ -777,13 +778,14 @@ getCalleeDeclAndArgs(ConstraintSystem &cs, hasTrailingClosure = objectLiteral->hasTrailingClosure(); } return std::make_tuple(nullptr, /*hasCurriedSelf=*/false, argLabels, - hasTrailingClosure); + hasTrailingClosure, nullptr); } // Find the overload choice corresponding to the callee locator. // FIXME: This linearly walks the list of resolved overloads, which is // potentially very expensive. Optional choice; + ConstraintLocator *calleeLocator = nullptr; for (auto resolved = cs.getResolvedOverloadSets(); resolved; resolved = resolved->Previous) { // FIXME: Workaround null locators. @@ -810,6 +812,7 @@ getCalleeDeclAndArgs(ConstraintSystem &cs, resolvedLocator = simplifyLocator(cs, resolvedLocator, range); if (resolvedLocator == targetLocator) { + calleeLocator = resolved->Locator; choice = resolved->Choice; break; } @@ -818,7 +821,7 @@ getCalleeDeclAndArgs(ConstraintSystem &cs, // If we didn't find any matching overloads, we're done. if (!choice) return std::make_tuple(nullptr, /*hasCurriedSelf=*/false, argLabels, - hasTrailingClosure); + hasTrailingClosure, nullptr); // If there's a declaration, return it. if (choice->isDecl()) { @@ -842,11 +845,12 @@ getCalleeDeclAndArgs(ConstraintSystem &cs, } } - return std::make_tuple(decl, hasCurriedSelf, argLabels, hasTrailingClosure); + return std::make_tuple(decl, hasCurriedSelf, argLabels, hasTrailingClosure, + calleeLocator); } return std::make_tuple(nullptr, /*hasCurriedSelf=*/false, argLabels, - hasTrailingClosure); + hasTrailingClosure, calleeLocator); } class ArgumentFailureTracker : public MatchCallArgumentListener { @@ -907,7 +911,9 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments( ArrayRef argLabels; SmallVector argLabelsScratch; bool hasTrailingClosure = false; - std::tie(callee, hasCurriedSelf, argLabels, hasTrailingClosure) = + ConstraintLocator *calleeLocator; + std::tie(callee, hasCurriedSelf, argLabels, hasTrailingClosure, + calleeLocator) = getCalleeDeclAndArgs(cs, locator, argLabelsScratch); ParameterListInfo paramInfo(params, callee, hasCurriedSelf); @@ -986,7 +992,8 @@ ConstraintSystem::TypeMatchResult constraints::matchCallArguments( Expr *arg = getArgumentExpr(locator.getAnchor(), argIdx); if (auto closure = dyn_cast_or_null(arg)) { auto result = - cs.applyFunctionBuilder(closure, functionBuilderType, loc); + cs.applyFunctionBuilder(closure, functionBuilderType, + calleeLocator, loc); if (result.isFailure()) return result; } diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 5666aca62ebb3..d390dae2999bf 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2770,6 +2770,7 @@ namespace { /// Visitor to classify the contents of the given closure. class BuilderClosureVisitor : public StmtVisitor { + ConstraintSystem &cs; ASTContext &ctx; bool wantExpr; Type builderType; @@ -2786,7 +2787,9 @@ namespace { if (!wantExpr) return nullptr; - auto typeExpr = TypeExpr::createImplicit(builderType, ctx); + auto typeExpr = new (ctx) TypeExpr(TypeLoc()); + cs.setType(typeExpr, builderType); + typeExpr->setImplicit(); auto memberRef = new (ctx) UnresolvedDotExpr( typeExpr, SourceLoc(), fnName, DeclNameLoc(), /*implicit=*/true); return CallExpr::createImplicit(ctx, memberRef, args, { }); @@ -2813,8 +2816,9 @@ namespace { } public: - BuilderClosureVisitor(ASTContext &ctx, bool wantExpr, Type builderType) - : ctx(ctx), wantExpr(wantExpr), builderType(builderType) { + BuilderClosureVisitor(ConstraintSystem &cs, bool wantExpr, Type builderType) + : cs(cs), ctx(cs.getASTContext()), wantExpr(wantExpr), + builderType(builderType) { builder = builderType->getAnyNominal(); } @@ -2939,7 +2943,8 @@ namespace { } ConstraintSystem::TypeMatchResult ConstraintSystem::applyFunctionBuilder( - ClosureExpr *closure, Type builderType, ConstraintLocatorBuilder locator) { + ClosureExpr *closure, Type builderType, ConstraintLocator *calleeLocator, + ConstraintLocatorBuilder locator) { auto builder = builderType->getAnyNominal(); assert(builder && "Bad function builder type"); assert(builder->getAttrs().hasAttribute()); @@ -2953,8 +2958,7 @@ ConstraintSystem::TypeMatchResult ConstraintSystem::applyFunctionBuilder( return getTypeMatchSuccess(); // Check whether we can apply this function builder. - BuilderClosureVisitor visitor( - getASTContext(), /*wantExpr=*/false, builderType); + BuilderClosureVisitor visitor(*this, /*wantExpr=*/false, builderType); (void)visitor.visit(closure->getBody()); // The presence of an explicit return suppresses the function builder @@ -2981,8 +2985,23 @@ ConstraintSystem::TypeMatchResult ConstraintSystem::applyFunctionBuilder( } } - BuilderClosureVisitor visitor( - getASTContext(), /*wantExpr=*/true, builderType); + // If the builder type has a type parameter, substitute in the type + // variables. + if (builderType->hasTypeParameter()) { + // Find the opened type for this callee and substitute in the type + // parametes. + for (const auto &opened : OpenedTypes) { + if (opened.first == calleeLocator) { + OpenedTypeMap replacements(opened.second.begin(), + opened.second.end()); + builderType = openType(builderType, replacements); + break; + } + } + assert(!builderType->hasTypeParameter()); + } + + BuilderClosureVisitor visitor(*this, /*wantExpr=*/true, builderType); Expr *singleExpr = visitor.visit(closure->getBody()); if (TC.precheckedClosures.insert(closure).second && diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index 8a94cb5e96c3d..792dbf4e58353 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -2978,6 +2978,7 @@ class ConstraintSystem { /// Apply the given function builder to the closure expression. TypeMatchResult applyFunctionBuilder(ClosureExpr *closure, Type builderType, + ConstraintLocator *calleeLocator, ConstraintLocatorBuilder locator); private: diff --git a/lib/Sema/TypeCheckRequestFunctions.cpp b/lib/Sema/TypeCheckRequestFunctions.cpp index d51b52cc823d2..50ada97030bf3 100644 --- a/lib/Sema/TypeCheckRequestFunctions.cpp +++ b/lib/Sema/TypeCheckRequestFunctions.cpp @@ -187,13 +187,6 @@ FunctionBuilderTypeRequest::evaluate(Evaluator &evaluator, CustomAttrTypeKind::NonGeneric); if (!type) return Type(); - // The type must not be contextually-dependent. - if (type->hasArchetype()) { - ctx.Diags.diagnose(attr->getLocation(), - diag::function_builder_type_contextual, type); - return Type(); - } - auto nominal = type->getAnyNominal(); if (!nominal) { assert(ctx.Diags.hadAnyError()); @@ -222,7 +215,7 @@ FunctionBuilderTypeRequest::evaluate(Evaluator &evaluator, return Type(); } - return type; + return type->mapTypeOutOfContext(); } // Define request evaluation functions for each of the type checker requests. diff --git a/test/Constraints/function_builder.swift b/test/Constraints/function_builder.swift index ef0dc431228b0..4a67d61aba8bd 100644 --- a/test/Constraints/function_builder.swift +++ b/test/Constraints/function_builder.swift @@ -55,3 +55,84 @@ tuplify(false) { ["if", "stmt"] } } + +struct Tagged { + let tag: Tag + let entity: Entity +} + +protocol Taggable { +} + +extension Taggable { + func tag(_ tag: Tag) -> Tagged { + return Tagged(tag: tag, entity: self) + } +} + +extension Int: Taggable { } +extension String: Taggable { } +extension Double: Taggable { } + +@_functionBuilder +struct TaggedBuilder { + static func buildBlock() -> () { } + + static func buildBlock(_ t1: Tagged) -> Tagged { + return t1 + } + + static func buildBlock(_ t1: Tagged, _ t2: Tagged) -> (Tagged, Tagged) { + return (t1, t2) + } + + static func buildBlock(_ t1: Tagged, _ t2: Tagged, _ t3: Tagged) + -> (Tagged, Tagged, Tagged) { + return (t1, t2, t3) + } + + static func buildBlock(_ t1: Tagged, _ t2: Tagged, _ t3: Tagged, _ t4: Tagged) + -> (Tagged, Tagged, Tagged, Tagged) { + return (t1, t2, t3, t4) + } + + static func buildBlock( + _ t1: Tagged, _ t2: Tagged, _ t3: Tagged, _ t4: Tagged, _ t5: Tagged + ) -> (Tagged, Tagged, Tagged, Tagged, Tagged) { + return (t1, t2, t3, t4, t5) + } + + static func buildIf(_ value: Tagged?) -> Tagged? { return value } +} + +enum Color { + case red, green, blue +} + +func acceptColorTagged(@TaggedBuilder body: () -> Result) { + print(body()) +} + +struct TagAccepter { + static func acceptTagged(@TaggedBuilder body: () -> Result) { + print(body()) + } +} + +func testAcceptColorTagged(i: Int, s: String, d: Double) { + // CHECK: Tagged< + acceptColorTagged { + i.tag(.red) + s.tag(.green) + d.tag(.blue) + } + + // CHECK: Tagged< + TagAccepter.acceptTagged { + i.tag(.red) + s.tag(.green) + d.tag(.blue) + } +} + +testAcceptColorTagged(i: 17, s: "Hello", d: 3.14159) diff --git a/test/decl/var/function_builders.swift b/test/decl/var/function_builders.swift index 4273f52a21090..ead48c384b3c0 100644 --- a/test/decl/var/function_builders.swift +++ b/test/decl/var/function_builders.swift @@ -68,7 +68,16 @@ func makeParamNestedBound(@GenericContainer.Maker fn: () -> ()) {} +protocol P { } + +@_functionBuilder +struct ConstrainedGenericMaker {} + + struct WithinGeneric { - func makeParamBoundInContext(@GenericMaker // expected-error {{function builder type 'GenericMaker' cannot depend on the current generic context}} - fn: () -> ()) {} + func makeParamBoundInContext(@GenericMaker fn: () -> ()) {} + + // expected-error@+1{{type 'U' does not conform to protocol 'P'}} + func makeParamBoundInContextBad(@ConstrainedGenericMaker + fn: () -> ()) {} } From 891a2a345533041306d57d18a0aa9d951b875b9c Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Tue, 23 Apr 2019 16:34:20 -0700 Subject: [PATCH 18/38] [Function builders] Add AST walker support for custom attributes on parameters --- lib/AST/ASTWalker.cpp | 7 +++-- test/Index/function_builders.swift | 45 ++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 test/Index/function_builders.swift diff --git a/lib/AST/ASTWalker.cpp b/lib/AST/ASTWalker.cpp index 3766251ce27b2..9578f9d3786d8 100644 --- a/lib/AST/ASTWalker.cpp +++ b/lib/AST/ASTWalker.cpp @@ -1160,13 +1160,16 @@ class Traversal : public ASTVisitorisImplicit() && !P->isTypeLocImplicit() && doIt(P->getTypeLoc())) return true; - + if (auto *E = P->getDefaultValue()) { auto res = doIt(E); if (!res) return true; diff --git a/test/Index/function_builders.swift b/test/Index/function_builders.swift new file mode 100644 index 0000000000000..f914ad5b77ebf --- /dev/null +++ b/test/Index/function_builders.swift @@ -0,0 +1,45 @@ +// RUN: %target-swift-ide-test -print-indexed-symbols -source-filename %s | %FileCheck -check-prefix=CHECK %s + +struct Tagged { + let tag: Tag + let entity: Entity +} + +protocol Taggable { +} + +extension Taggable { + func tag(_ tag: Tag) -> Tagged { + return Tagged(tag: tag, entity: self) + } +} + +extension Int: Taggable { } +extension String: Taggable { } +extension Double: Taggable { } + +@_functionBuilder +struct TaggedBuilder { + static func buildBlock() -> () { } + + static func buildBlock(_ t1: Tagged) -> Tagged { + return t1 + } + + static func buildBlock(_ t1: Tagged, _ t2: Tagged) -> (Tagged, Tagged) { + return (t1, t2) + } + + static func buildIf(_ value: Tagged?) -> Tagged? { return value } +} + +enum Color { + case red, green, blue +} + +func acceptColorTagged(@TaggedBuilder body: () -> Result) { + print(body()) +} + +// CHECK: 40:33 | struct/Swift | TaggedBuilder | s:14swift_ide_test13TaggedBuilderV | Ref,RelCont | rel: 1 +// CHECK: 40:47 | enum/Swift | Color | s:14swift_ide_test5ColorO | Ref,RelCont | rel: 1 From 621d88fa312956e715116a8d6adee59cc02dbf0e Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 26 Apr 2019 11:47:55 -0700 Subject: [PATCH 19/38] [AST Printing] Print custom attributes on parameters. Fixes rdar://problem/50232955. --- lib/AST/ASTPrinter.cpp | 1 + .../Inputs/function_builders_client.swift | 17 +++++++++ .../function_builders.swift | 35 +++++++++++++++++++ 3 files changed, 53 insertions(+) create mode 100644 test/ParseableInterface/Inputs/function_builders_client.swift create mode 100644 test/ParseableInterface/function_builders.swift diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index 744f607d2a90d..264ad9e5400dd 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -2598,6 +2598,7 @@ void PrintAST::printOneParameter(const ParamDecl *param, auto TheTypeLoc = param->getTypeLoc(); + printAttributes(param); printArgName(); if (!TheTypeLoc.getTypeRepr() && param->hasInterfaceType()) diff --git a/test/ParseableInterface/Inputs/function_builders_client.swift b/test/ParseableInterface/Inputs/function_builders_client.swift new file mode 100644 index 0000000000000..8ce9da388385b --- /dev/null +++ b/test/ParseableInterface/Inputs/function_builders_client.swift @@ -0,0 +1,17 @@ +import FunctionBuilders + +let name = "dsl" +tuplify(true) { + 17 + 3.14159 + "Hello, \(name.map { $0.uppercased() }.joined())" + do { + ["nested", "do"] + 1 + 2 + 3 + } + if $0 { + 2.71828 + ["if", "stmt"] + } +} + diff --git a/test/ParseableInterface/function_builders.swift b/test/ParseableInterface/function_builders.swift new file mode 100644 index 0000000000000..28ba559237cc2 --- /dev/null +++ b/test/ParseableInterface/function_builders.swift @@ -0,0 +1,35 @@ +// RUN: %empty-directory(%t) +// RUN: %target-swift-frontend -typecheck -module-name FunctionBuilders -emit-parseable-module-interface-path %t/FunctionBuilders.swiftinterface %s +// RUN: %FileCheck %s < %t/FunctionBuilders.swiftinterface +// RUN: %target-swift-frontend -I %t -typecheck -verify %S/Inputs/function_builders_client.swift + +@_functionBuilder +public struct TupleBuilder { + public static func buildBlock(_ t1: T1, _ t2: T2) -> (T1, T2) { + return (t1, t2) + } + + public static func buildBlock(_ t1: T1, _ t2: T2, _ t3: T3) + -> (T1, T2, T3) { + return (t1, t2, t3) + } + + public static func buildBlock(_ t1: T1, _ t2: T2, _ t3: T3, _ t4: T4) + -> (T1, T2, T3, T4) { + return (t1, t2, t3, t4) + } + + public static func buildBlock( + _ t1: T1, _ t2: T2, _ t3: T3, _ t4: T4, _ t5: T5 + ) -> (T1, T2, T3, T4, T5) { + return (t1, t2, t3, t4, t5) + } + + public static func buildDo(_ value: T) -> T { return value } + public static func buildIf(_ value: T?) -> T? { return value } +} + +// CHECK-LABEL: public func tuplify(_ cond: Bool, @FunctionBuilders.TupleBuilder body: (Bool) -> T) +public func tuplify(_ cond: Bool, @TupleBuilder body: (Bool) -> T) { + print(body(cond)) +} From aa33a1e7929af3c827b4bf4e9135ccf221fb300f Mon Sep 17 00:00:00 2001 From: Rintaro Ishizaki Date: Thu, 25 Apr 2019 14:39:38 -0700 Subject: [PATCH 20/38] [CodeCompletion] Enable type name completion for param decl attribute rdar://problem/50074177 --- include/swift/Parse/CodeCompletionCallbacks.h | 2 +- lib/AST/TypeCheckRequests.cpp | 7 ------- lib/IDE/CodeCompletion.cpp | 10 +++------- lib/Parse/ParseDecl.cpp | 5 ++++- lib/Parse/ParsePattern.cpp | 5 ++++- 5 files changed, 12 insertions(+), 17 deletions(-) diff --git a/include/swift/Parse/CodeCompletionCallbacks.h b/include/swift/Parse/CodeCompletionCallbacks.h index d897055c43b6b..bca5ad49e786f 100644 --- a/include/swift/Parse/CodeCompletionCallbacks.h +++ b/include/swift/Parse/CodeCompletionCallbacks.h @@ -116,7 +116,7 @@ class CodeCompletionCallbacks { }; /// Set target decl for attribute if the CC token is in attribute of the decl. - virtual void setAttrTargetDecl(Decl *D) {} + virtual void setAttrTargetDeclKind(Optional DK) {} /// Complete the whole expression. This is a fallback that should /// produce results when more specific completion methods failed. diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp index a8529e36d4291..547d969891575 100644 --- a/lib/AST/TypeCheckRequests.cpp +++ b/lib/AST/TypeCheckRequests.cpp @@ -59,13 +59,6 @@ void swift::simple_display(llvm::raw_ostream &out, } } -template<> -bool swift::AnyValue::Holder::equals(const HolderBase &other) const { - assert(typeID == other.typeID && "Caller should match type IDs"); - return value.getPointer() == - static_cast &>(other).value.getPointer(); -} - void swift::simple_display(llvm::raw_ostream &out, Type type) { if (type) type.print(out); diff --git a/lib/IDE/CodeCompletion.cpp b/lib/IDE/CodeCompletion.cpp index 3c5a83042b47d..c3ebaf06c74e0 100644 --- a/lib/IDE/CodeCompletion.cpp +++ b/lib/IDE/CodeCompletion.cpp @@ -1350,12 +1350,7 @@ class CodeCompletionCallbacksImpl : public CodeCompletionCallbacks { Consumer(Consumer) { } - void setAttrTargetDecl(Decl *D) override { - if (D == nullptr) { - AttTargetDK = None; - return; - } - auto DK = D->getKind(); + void setAttrTargetDeclKind(Optional DK) override { if (DK == DeclKind::PatternBinding) DK = DeclKind::Var; AttTargetDK = DK; @@ -5387,7 +5382,8 @@ void CodeCompletionCallbacksImpl::doneParsing() { Lookup.getAttributeDeclCompletions(IsInSil, AttTargetDK); // Provide any type name for property delegate. - if (!AttTargetDK || *AttTargetDK == DeclKind::Var) + if (!AttTargetDK || *AttTargetDK == DeclKind::Var || + *AttTargetDK == DeclKind::Param) Lookup.getTypeCompletionsInDeclContext( P.Context.SourceMgr.getCodeCompletionLoc()); break; diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp index e37e3587fd33f..fce243986266a 100644 --- a/lib/Parse/ParseDecl.cpp +++ b/lib/Parse/ParseDecl.cpp @@ -2998,7 +2998,10 @@ Parser::parseDecl(ParseDeclOptions Flags, if (AttrStatus.hasCodeCompletion()) { if (CodeCompletion) { - CodeCompletion->setAttrTargetDecl(DeclResult.getPtrOrNull()); + Optional DK; + if (DeclResult.isNonNull()) + DK = DeclResult.get()->getKind(); + CodeCompletion->setAttrTargetDeclKind(DK); } else { delayParseFromBeginningToHere(BeginParserPosition, Flags); return makeParserError(); diff --git a/lib/Parse/ParsePattern.cpp b/lib/Parse/ParsePattern.cpp index f048145b10f3d..6c659bb5c2357 100644 --- a/lib/Parse/ParsePattern.cpp +++ b/lib/Parse/ParsePattern.cpp @@ -223,8 +223,11 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc, // Attributes. if (paramContext != ParameterContextKind::EnumElement) { auto AttrStatus = parseDeclAttributeList(param.Attrs); - if (AttrStatus.hasCodeCompletion()) + if (AttrStatus.hasCodeCompletion()) { + if (CodeCompletion) + CodeCompletion->setAttrTargetDeclKind(DeclKind::Param); status.setHasCodeCompletion(); + } } // ('inout' | 'let' | 'var' | '__shared' | '__owned')? From 75323a311903c3a3e39187a468d3feee54007398 Mon Sep 17 00:00:00 2001 From: Rintaro Ishizaki Date: Wed, 24 Apr 2019 16:17:27 -0700 Subject: [PATCH 21/38] [SourceKit] Function builder: Add cursor-info test cases rdar://problem/50074156 --- .../CursorInfo/function_builder.swift | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 test/SourceKit/CursorInfo/function_builder.swift diff --git a/test/SourceKit/CursorInfo/function_builder.swift b/test/SourceKit/CursorInfo/function_builder.swift new file mode 100644 index 0000000000000..6e9f31d60f982 --- /dev/null +++ b/test/SourceKit/CursorInfo/function_builder.swift @@ -0,0 +1,115 @@ +struct Tagged { + let tag: Tag + let entity: Entity +} + +protocol Taggable { +} + +extension Taggable { + func tag(_ tag: Tag) -> Tagged { + return Tagged(tag: tag, entity: self) + } +} + +extension Int: Taggable { } +extension String: Taggable { } + +@_functionBuilder +struct TaggedBuilder { + static func buildBlock() -> () { } + + static func buildBlock(_ t1: Tagged) -> Tagged { + return t1 + } + + static func buildBlock(_ t1: Tagged, _ t2: Tagged) -> (Tagged, Tagged) { + return (t1, t2) + } +} + +enum Color { + case red, green, blue +} + +func acceptColorTagged(@TaggedBuilder body: (Color) -> Result) { + print(body(.blue)) +} + +func testAcceptColorTagged(i: Int, s: String) { + acceptColorTagged { color in + i.tag(color) + s.tag(.green) + } +} + +// Custom attribute name. +// RUN: %sourcekitd-test -req=cursor -pos=35:33 %s -- %s -module-name BuilderTest | %FileCheck %s --check-prefix=ATTR_NAME +// ATTR_NAME: source.lang.swift.ref.struct (19:8-19:21) +// ATTR_NAME-NEXT: TaggedBuilder +// ATTR_NAME-NEXT: s:11BuilderTest06TaggedA0V +// ATTR_NAME-NEXT: TaggedBuilder.Type +// ATTR_NAME-NEXT: $s11BuilderTest06TaggedA0VyxGmD +// ATTR_NAME-NEXT: @_functionBuilder struct TaggedBuilder<Tag> +// ATTR_NAME-NEXT: @_functionBuilder struct TaggedBuilder<Tag> + +// Generic argument in attribute name. +// RUN: %sourcekitd-test -req=cursor -pos=35:47 %s -- %s -module-name BuilderTest | %FileCheck %s --check-prefix=ATTR_GENERICARG +// ATTR_GENERICARG: source.lang.swift.ref.enum (31:6-31:11) +// ATTR_GENERICARG-NEXT: Color +// ATTR_GENERICARG-NEXT: s:11BuilderTest5ColorO +// ATTR_GENERICARG-NEXT: Color.Type +// ATTR_GENERICARG-NEXT: $s11BuilderTest5ColorOmD +// ATTR_GENERICARG-NEXT: enum Color +// ATTR_GENERICARG-NEXT: enum Color + +// Call for function with builder. +// RUN: %sourcekitd-test -req=cursor -pos=40:3 %s -- %s -module-name BuilderTest | %FileCheck %s --check-prefix=CALL_BUILDERFUNC +// CALL_BUILDERFUNC: source.lang.swift.ref.function.free (35:6-35:78) +// CALL_BUILDERFUNC-NEXT: acceptColorTagged(body:) +// CALL_BUILDERFUNC-NEXT: s:11BuilderTest17acceptColorTagged4bodyyxAA0D0OXE_tlF +// CALL_BUILDERFUNC-NEXT: (body: (Color) -> Result) -> () +// CALL_BUILDERFUNC-NEXT: $s4bodyyx11BuilderTest5ColorOXE_tcluD +// CALL_BUILDERFUNC-NEXT: func acceptColorTagged<Result>(@TaggedBuilder<Color> body: (Color) -> Result) +// CALL_BUILDERFUNC-NEXT: func acceptColorTagged<Result>(@TaggedBuilder<Color> body: (Color) -> Result) + +// Closure parameter - decl-site. +// RUN: %sourcekitd-test -req=cursor -pos=40:23 %s -- %s -module-name BuilderTest | %FileCheck %s --check-prefix=CLOSUREPARAM_DECL +// CLOSUREPARAM_DECL: source.lang.swift.decl.var.parameter (40:23-40:28) +// CLOSUREPARAM_DECL-NEXT: color +// CLOSUREPARAM_DECL-NEXT: s:11BuilderTest21testAcceptColorTagged1i1sySi_SStFAA0F0VyAA0E0OSiG_AFyAHSSGtAHXEfU_5colorL_AHvp +// CLOSUREPARAM_DECL-NEXT: Color +// CLOSUREPARAM_DECL-NEXT: $s11BuilderTest5ColorOD +// CLOSUREPARAM_DECL-NEXT: let color: Color +// CLOSUREPARAM_DECL-NEXT: let color: Color + +// Closure parameter - use-site. +// RUN: %sourcekitd-test -req=cursor -pos=41:11 %s -- %s -module-name BuilderTest | %FileCheck %s --check-prefix=CLOSUREPARAM_USER +// CLOSUREPARAM_USER: source.lang.swift.ref.var.local (40:23-40:28) +// CLOSUREPARAM_USER-NEXT: color +// CLOSUREPARAM_USER-NEXT: s:11BuilderTest21testAcceptColorTagged1i1sySi_SStFAA0F0VyAA0E0OSiG_AFyAHSSGtAHXEfU_5colorL_AHvp +// CLOSUREPARAM_USER-NEXT: Color +// CLOSUREPARAM_USER-NEXT: $s11BuilderTest5ColorOD +// CLOSUREPARAM_USER-NEXT: let color: Color +// CLOSUREPARAM_USER-NEXT: let color: Color + +// Captured variable. +// RUN: %sourcekitd-test -req=cursor -pos=41:5 %s -- %s -module-name BuilderTest | %FileCheck %s --check-prefix=CAPTURED_VALUE +// CAPTURED_VALUE: source.lang.swift.ref.var.local (39:28-39:29) +// CAPTURED_VALUE-NEXT: i +// CAPTURED_VALUE-NEXT: s:11BuilderTest21testAcceptColorTagged1i1sySi_SStFACL_Sivp +// CAPTURED_VALUE-NEXT: Int +// CAPTURED_VALUE-NEXT: $sSiD +// CAPTURED_VALUE-NEXT: let i: Int +// CAPTURED_VALUE-NEXT: let i: Int + +// Method call to captured variable. +// RUN: %sourcekitd-test -req=cursor -pos=41:7 %s -- %s -module-name BuilderTest | %FileCheck %s --check-prefix=CAPTURED_VALUE_METHOD +// CAPTURED_VALUE_METHOD: source.lang.swift.ref.function.method.instance (10:8-10:28) +// CAPTURED_VALUE_METHOD: tag(_:) +// CAPTURED_VALUE_METHOD: s:11BuilderTest8TaggablePAAE3tagyAA6TaggedVyqd__xGqd__lF +// CAPTURED_VALUE_METHOD: (Self) -> (Tag) -> Tagged +// CAPTURED_VALUE_METHOD: $sy11BuilderTest6TaggedVyqd__xGqd__cluD +// CAPTURED_VALUE_METHOD: $sSiD +// CAPTURED_VALUE_METHOD: func tag<Tag>(_ tag: Tag) -> Tagged<Tag, Int> +// CAPTURED_VALUE_METHOD: func tag<Tag>(_ tag: Tag) -> Tagged<Tag, Int> From 2bbd78d494ca462ccff0bd95bb1937f25325033e Mon Sep 17 00:00:00 2001 From: Rintaro Ishizaki Date: Thu, 25 Apr 2019 16:16:39 -0700 Subject: [PATCH 22/38] [IDE] Function builder: add syntax coloring test for custom attribute rdar://problem/50074156 --- test/IDE/coloring.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/IDE/coloring.swift b/test/IDE/coloring.swift index 6e6c1becf22b7..c633e78adb94c 100644 --- a/test/IDE/coloring.swift +++ b/test/IDE/coloring.swift @@ -439,3 +439,9 @@ class PropertyDelgate { }) var something } + +// CHECK: func acceptBuilder( +func acceptBuilder( + // CHECK: @SomeBuilder label param: () -> T + @SomeBuilder label param: () -> T +) {} From afca64511643277a9766bdd70d84bc7346f3b19b Mon Sep 17 00:00:00 2001 From: Rintaro Ishizaki Date: Thu, 25 Apr 2019 16:50:05 -0700 Subject: [PATCH 23/38] [CodeCompletion] Add basic test cases inside builder closure rdar://problem/50074177 --- test/IDE/complete_function_builder.swift | 95 ++++++++++++++++++++++++ 1 file changed, 95 insertions(+) create mode 100644 test/IDE/complete_function_builder.swift diff --git a/test/IDE/complete_function_builder.swift b/test/IDE/complete_function_builder.swift new file mode 100644 index 0000000000000..f9b45e70fcf8a --- /dev/null +++ b/test/IDE/complete_function_builder.swift @@ -0,0 +1,95 @@ +// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=IN_CLOSURE_TOP | %FileCheck %s -check-prefix=IN_CLOSURE_TOP +// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=IN_CLOSURE_NONTOP | %FileCheck %s -check-prefix=IN_CLOSURE_TOP +// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=IN_CLOSURE_COLOR_CONTEXT | %FileCheck %s -check-prefix=IN_CLOSURE_COLOR_CONTEXT +// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=IN_CLOSURE_COLOR_CONTEXT_DOT | %FileCheck %s -check-prefix=IN_CLOSURE_COLOR_CONTEXT_DOT + +struct Tagged { + let tag: Tag + let entity: Entity + + static func fo +} + +protocol Taggable { +} + +extension Taggable { + func tag(_ tag: Tag) -> Tagged { + return Tagged(tag: tag, entity: self) + } +} + +extension Int: Taggable { } +extension String: Taggable { } + +@_functionBuilder +struct TaggedBuilder { + static func buildBlock() -> () { } + + static func buildBlock(_ t1: Tagged) -> Tagged { + return t1 + } + + static func buildBlock(_ t1: Tagged, _ t2: Tagged) -> (Tagged, Tagged) { + return (t1, t2) + } + static func buildBlock(_ t1: Tagged, _ t2: Tagged, _ t2: Tagged) -> (Tagged, Tagged, Tagged) { + return (t1, t2, t3) + } +} + +enum Color { + case red, green, blue +} + +func acceptColorTagged(@TaggedBuilder body: (Color) -> Result) { + print(body(.blue)) +} + +var globalIntVal: Int = 1 +let globalStringVal: String = "" + +func testAcceptColorTagged(paramIntVal: Int, paramStringVal: String) { + + let taggedValue = paramIntVal.tag(Color.red) + + acceptColorTagged { color in + #^IN_CLOSURE_TOP^# +// IN_CLOSURE_TOP_CONTEXT: Begin completions +// IN_CLOSURE_TOP-DAG: Decl[LocalVar]/Local: taggedValue[#Tagged#]; name=taggedValue +// IN_CLOSURE_TOP-DAG: Decl[GlobalVar]/CurrModule: globalIntVal[#Int#]; name=globalIntVal +// IN_CLOSURE_TOP-DAG: Decl[GlobalVar]/CurrModule: globalStringVal[#String#]; name=globalStringVal +// IN_CLOSURE_TOP-DAG: Decl[LocalVar]/Local: color; name=color +// IN_CLOSURE_TOP-DAG: Decl[LocalVar]/Local: paramIntVal[#Int#]; name=paramIntVal +// IN_CLOSURE_TOP-DAG: Decl[LocalVar]/Local: paramStringVal[#String#]; name=paramStringVal +// IN_CLOSURE_TOP: End completions + } + + acceptColorTagged { color in + paramIntVal.tag(.red) + #^IN_CLOSURE_NONTOP^# +// Same as IN_CLOSURE_TOP. + } + + acceptColorTagged { color in + paramIntVal.tag(#^IN_CLOSURE_COLOR_CONTEXT^#) +// IN_CLOSURE_COLOR_CONTEXT: Begin completions +// IN_CLOSURE_COLOR_CONTEXT-DAG: Decl[InstanceMethod]/CurrNominal: ['(']{#(tag): Color#}[')'][#Tagged#]; name=tag: Color +// IN_CLOSURE_COLOR_CONTEXT-DAG: Decl[LocalVar]/Local/TypeRelation[Identical]: color[#Color#]; name=color +// IN_CLOSURE_COLOR_CONTEXT-DAG: Decl[LocalVar]/Local: taggedValue[#Tagged#]; name=taggedValue +// IN_CLOSURE_COLOR_CONTEXT-DAG: Decl[LocalVar]/Local: paramIntVal[#Int#]; name=paramIntVal +// IN_CLOSURE_COLOR_CONTEXT-DAG: Decl[LocalVar]/Local: paramStringVal[#String#]; name=paramStringVal +// IN_CLOSURE_COLOR_CONTEXT-DAG: Decl[Enum]/CurrModule/TypeRelation[Identical]: Color[#Color#]; name=Color +// IN_CLOSURE_COLOR_CONTEXT: End completions + } + + acceptColorTagged { color in + paramIntVal.tag(.#^IN_CLOSURE_COLOR_CONTEXT_DOT^#) +// IN_CLOSURE_COLOR_CONTEXT_DOT: Begin completions, 3 items +// IN_CLOSURE_COLOR_CONTEXT_DOT-DAG: Decl[EnumElement]/ExprSpecific: red[#Color#]; name=red +// IN_CLOSURE_COLOR_CONTEXT_DOT-DAG: Decl[EnumElement]/ExprSpecific: green[#Color#]; name=green +// IN_CLOSURE_COLOR_CONTEXT_DOT-DAG: Decl[EnumElement]/ExprSpecific: blue[#Color#]; name=blue +// IN_CLOSURE_COLOR_CONTEXT_DOT: End completions + } +} + From 72f765669be0cb7e9ef5741bcd0a64720afd4026 Mon Sep 17 00:00:00 2001 From: Rintaro Ishizaki Date: Thu, 25 Apr 2019 16:58:43 -0700 Subject: [PATCH 24/38] [CodeCompletion] Function builder: add test case for attribute at params Make sure we don't suggest any builtin attributes, but only custom type names. rdar://problem/50074177 --- test/IDE/complete_decl_attribute.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/IDE/complete_decl_attribute.swift b/test/IDE/complete_decl_attribute.swift index 9e0f6842e9d37..a54245094a788 100644 --- a/test/IDE/complete_decl_attribute.swift +++ b/test/IDE/complete_decl_attribute.swift @@ -9,6 +9,7 @@ // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=ON_INIT | %FileCheck %s -check-prefix=ON_INIT // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=ON_PROPERTY | %FileCheck %s -check-prefix=ON_PROPERTY // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=ON_METHOD | %FileCheck %s -check-prefix=ON_METHOD +// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=ON_PARAM | %FileCheck %s -check-prefix=ON_PARAM // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=ON_MEMBER_LAST | %FileCheck %s -check-prefix=ON_MEMBER_LAST // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=KEYWORD_LAST | %FileCheck %s -check-prefix=KEYWORD_LAST @@ -172,6 +173,13 @@ struct _S { // ON_METHOD-DAG: Keyword/None: IBSegueAction[#Func Attribute#]; name=IBSegueAction // ON_METHOD: End completions + func bar(@#^ON_PARAM^#) +// ON_PARAM: Begin completions +// ON_PARAM-NOT: Keyword +// ON_PARAM: Decl[Struct]/CurrModule: MyStruct[#MyStruct#]; name=MyStruct +// ON_PARAM-NOT: Keyword +// ON_PARAM: End completions + @#^ON_MEMBER_LAST^# // ON_MEMBER_LAST: Begin completions // ON_MEMBER_LAST-DAG: Keyword/None: available[#Declaration Attribute#]; name=available From e550ecfa4187e8586ed936eb85dfd375384bfadb Mon Sep 17 00:00:00 2001 From: Rintaro Ishizaki Date: Fri, 26 Apr 2019 15:56:22 -0700 Subject: [PATCH 25/38] [placeholder-expansion] Function builder: add basic expansion test rdar://problem/50074177 --- test/SourceKit/CodeExpand/code-expand.swift | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/SourceKit/CodeExpand/code-expand.swift b/test/SourceKit/CodeExpand/code-expand.swift index afe0d856012d8..4b87375a0e369 100644 --- a/test/SourceKit/CodeExpand/code-expand.swift +++ b/test/SourceKit/CodeExpand/code-expand.swift @@ -37,6 +37,16 @@ dispatch_after(<#T##when: dispatch_time_t##dispatch_time_t#>, <#T##queue: dispat // CHECK-NEXT: <#code#> // CHECK-NEXT: } +@_functionBuilder +struct MyBuilder {} +func acceptBuilder(@MyBuilder body: () -> Result) {} +do { + acceptBuilder(body: <#T##() -> Result#>) + // CHECK: acceptBuilder { + // CHECK-NEXT: <#code#> + // CHECK-NEXT: } +} + foo(x: <#T##Self.SegueIdentifier -> Void#>) // CHECK: foo { (<#Self.SegueIdentifier#>) in From ff9b1ed9409a301c25f851bd5ae1bf157c76d626 Mon Sep 17 00:00:00 2001 From: Rintaro Ishizaki Date: Fri, 26 Apr 2019 17:27:14 -0700 Subject: [PATCH 26/38] [CodeCompletion] Function builder: test case with context types in builder --- test/IDE/complete_function_builder.swift | 55 ++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/test/IDE/complete_function_builder.swift b/test/IDE/complete_function_builder.swift index f9b45e70fcf8a..eb5867e3d4573 100644 --- a/test/IDE/complete_function_builder.swift +++ b/test/IDE/complete_function_builder.swift @@ -3,6 +3,12 @@ // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=IN_CLOSURE_COLOR_CONTEXT | %FileCheck %s -check-prefix=IN_CLOSURE_COLOR_CONTEXT // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=IN_CLOSURE_COLOR_CONTEXT_DOT | %FileCheck %s -check-prefix=IN_CLOSURE_COLOR_CONTEXT_DOT +// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=CONTEXTUAL_TYPE_1 | %FileCheck %s -check-prefix=CONTEXTUAL_TYPE_INVALID +// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=CONTEXTUAL_TYPE_2 | %FileCheck %s -check-prefix=CONTEXTUAL_TYPE_INVALID +// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=CONTEXTUAL_TYPE_3 | %FileCheck %s -check-prefix=CONTEXTUAL_TYPE_VALID +// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=CONTEXTUAL_TYPE_4 | %FileCheck %s -check-prefix=CONTEXTUAL_TYPE_VALID +// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=CONTEXTUAL_TYPE_5 | %FileCheck %s -check-prefix=CONTEXTUAL_TYPE_INVALID + struct Tagged { let tag: Tag let entity: Entity @@ -93,3 +99,52 @@ func testAcceptColorTagged(paramIntVal: Int, paramStringVal: String) { } } +enum MyEnum { + case east, west + case north, south +} +@_functionBuilder +struct EnumToVoidBuilder { + static func buildBlock() {} + static func buildBlock(_ :MyEnum) {} + static func buildBlock(_ :MyEnum, _: MyEnum) {} + static func buildBlock(_ :MyEnum, _: MyEnum, _: MyEnum) {} +} +func acceptBuilder(@EnumToVoidBuilder body: () -> Void) {} + +// CONTEXTUAL_TYPE_INVALID-NOT: Begin completions + +// CONTEXTUAL_TYPE_VALID: Begin completions, 4 items +// CONTEXTUAL_TYPE_VALID-DAG: Decl[EnumElement]/ExprSpecific: east[#MyEnum#]; name=east +// CONTEXTUAL_TYPE_VALID-DAG: Decl[EnumElement]/ExprSpecific: west[#MyEnum#]; name=west +// CONTEXTUAL_TYPE_VALID-DAG: Decl[EnumElement]/ExprSpecific: north[#MyEnum#]; name=north +// CONTEXTUAL_TYPE_VALID-DAG: Decl[EnumElement]/ExprSpecific: south[#MyEnum#]; name=south +// CONTEXTUAL_TYPE_VALID: End completions + +func testContextualType() { + acceptBuilder { +// FIXME: This should suggest enum values. + .#^CONTEXTUAL_TYPE_1^# + } + acceptBuilder { +// FIXME: This should suggest enum values. + .#^CONTEXTUAL_TYPE_2^#; + .north; + } + acceptBuilder { + .north; + .#^CONTEXTUAL_TYPE_3^#; + } + acceptBuilder { + .north; + .east; + .#^CONTEXTUAL_TYPE_4^# + } + acceptBuilder { + .north; + .east; + .south; +// NOTE: Invalid because 'EnumToVoidBuilder' doesn't have 4 params overload. + .#^CONTEXTUAL_TYPE_5^# + } +} From 8d802b3667d666c823f96f5a2801ef1735dbd512 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Mon, 29 Apr 2019 16:39:07 -0700 Subject: [PATCH 27/38] [Function builders] Prescan closure bodies for a 'return' statement. Since we short-circuit in the function builder application when we hit something we cannot translate, relying on that visitor to detect 'return' statements (which disable the application) is bogus. Use a separate, earlier visitor to find 'return' statements consistently. Fixes rdar://problem/50266341. --- lib/Sema/ConstraintSystem.cpp | 55 +++++++++++++++++++++---- test/Constraints/function_builder.swift | 13 +++++- 2 files changed, 57 insertions(+), 11 deletions(-) diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index d390dae2999bf..c2ce13c178252 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2778,7 +2778,6 @@ namespace { llvm::SmallDenseMap supportedOps; public: - ReturnStmt *returnStmt = nullptr; SkipUnhandledConstructInFunctionBuilder::UnhandledNode unhandledNode; private: @@ -2856,9 +2855,7 @@ namespace { } Expr *visitReturnStmt(ReturnStmt *returnStmt) { - if (!this->returnStmt) - this->returnStmt = returnStmt; - return nullptr; + llvm_unreachable("Should not try visit bodies with return statements"); } Expr *visitDoStmt(DoStmt *doStmt) { @@ -2942,6 +2939,45 @@ namespace { }; } +/// Determine whether the given statement contains a 'return' statement anywhere. +static bool hasReturnStmt(Stmt *stmt) { + class ReturnStmtFinder : public ASTWalker { + public: + bool hasReturnStmt = false; + + std::pair walkToExprPre(Expr *expr) override { + return { false, expr }; + } + + std::pair walkToStmtPre(Stmt *stmt) override { + // Did we find a 'return' statement? + if (isa(stmt)) { + hasReturnStmt = true; + } + + return { !hasReturnStmt, stmt }; + } + + Stmt *walkToStmtPost(Stmt *stmt) override { + return hasReturnStmt ? nullptr : stmt; + } + + std::pair walkToPatternPre(Pattern *pattern) override { + return { false, pattern }; + } + + bool walkToDeclPre(Decl *D) override { return false; } + + bool walkToTypeLocPre(TypeLoc &TL) override { return false; } + + bool walkToTypeReprPre(TypeRepr *T) override { return false; } + }; + + ReturnStmtFinder finder{}; + stmt->walk(finder); + return finder.hasReturnStmt; +} + ConstraintSystem::TypeMatchResult ConstraintSystem::applyFunctionBuilder( ClosureExpr *closure, Type builderType, ConstraintLocator *calleeLocator, ConstraintLocatorBuilder locator) { @@ -2957,16 +2993,17 @@ ConstraintSystem::TypeMatchResult ConstraintSystem::applyFunctionBuilder( if (closure->hasSingleExpressionBody()) return getTypeMatchSuccess(); - // Check whether we can apply this function builder. - BuilderClosureVisitor visitor(*this, /*wantExpr=*/false, builderType); - (void)visitor.visit(closure->getBody()); - // The presence of an explicit return suppresses the function builder // translation. - if (visitor.returnStmt) { + if (hasReturnStmt(closure->getBody())) { return getTypeMatchSuccess(); } + // Check whether we can apply this function builder. + BuilderClosureVisitor visitor(*this, /*wantExpr=*/false, builderType); + (void)visitor.visit(closure->getBody()); + + // If we saw a control-flow statement or declaration that the builder // cannot handle, we don't have a well-formed function builder application. if (visitor.unhandledNode) { diff --git a/test/Constraints/function_builder.swift b/test/Constraints/function_builder.swift index 4a67d61aba8bd..b1f860948a705 100644 --- a/test/Constraints/function_builder.swift +++ b/test/Constraints/function_builder.swift @@ -119,7 +119,7 @@ struct TagAccepter { } } -func testAcceptColorTagged(i: Int, s: String, d: Double) { +func testAcceptColorTagged(b: Bool, i: Int, s: String, d: Double) { // CHECK: Tagged< acceptColorTagged { i.tag(.red) @@ -133,6 +133,15 @@ func testAcceptColorTagged(i: Int, s: String, d: Double) { s.tag(.green) d.tag(.blue) } + + // CHECK: Tagged< + TagAccepter.acceptTagged { () -> Tagged in + if b { + return i.tag(Color.green) + } else { + return i.tag(Color.blue) + } + } } -testAcceptColorTagged(i: 17, s: "Hello", d: 3.14159) +testAcceptColorTagged(b: true, i: 17, s: "Hello", d: 3.14159) From 5cbfd58580fa5fa494e76a7769ab4ad210f86b86 Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 30 Apr 2019 15:52:19 -0400 Subject: [PATCH 28/38] Support if-else chains on function builders. A substantial amount of this patch goes towards trying to get at least minimal diagnostics working, since of course I messed up the rule a few times when implementing this. rdar://50149837 --- include/swift/AST/KnownIdentifiers.def | 5 +- lib/Sema/ConstraintSystem.cpp | 263 +++++++++++++++++++++--- test/Constraints/function_builder.swift | 93 +++++++++ 3 files changed, 333 insertions(+), 28 deletions(-) diff --git a/include/swift/AST/KnownIdentifiers.def b/include/swift/AST/KnownIdentifiers.def index 58f5b5bb2f727..15e32329cd776 100644 --- a/include/swift/AST/KnownIdentifiers.def +++ b/include/swift/AST/KnownIdentifiers.def @@ -32,8 +32,9 @@ IDENTIFIER(ArrayLiteralElement) IDENTIFIER(atIndexedSubscript) IDENTIFIER_(bridgeToObjectiveC) IDENTIFIER(buildBlock) -IDENTIFIER(buildIf) IDENTIFIER(buildDo) +IDENTIFIER(buildEither) +IDENTIFIER(buildIf) IDENTIFIER(Change) IDENTIFIER_WITH_NAME(code_, "_code") IDENTIFIER(CodingKeys) @@ -62,6 +63,7 @@ IDENTIFIER(Encoder) IDENTIFIER(encoder) IDENTIFIER(error) IDENTIFIER(errorDomain) +IDENTIFIER(first) IDENTIFIER(forKeyedSubscript) IDENTIFIER(Foundation) IDENTIFIER(for) @@ -96,6 +98,7 @@ IDENTIFIER(parameter) IDENTIFIER(Protocol) IDENTIFIER(rawValue) IDENTIFIER(RawValue) +IDENTIFIER(second) IDENTIFIER(Selector) IDENTIFIER(self) IDENTIFIER(Self) diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index c2ce13c178252..a2ee652f96ab2 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2782,20 +2782,30 @@ namespace { private: /// Produce a builder call to the given named function with the given arguments. - CallExpr *buildCallIfWanted(Identifier fnName, ArrayRef args) { + CallExpr *buildCallIfWanted(SourceLoc loc, + Identifier fnName, ArrayRef args, + ArrayRef argLabels = {}) { if (!wantExpr) return nullptr; - auto typeExpr = new (ctx) TypeExpr(TypeLoc()); + // FIXME: Setting a TypeLoc on this expression is necessary in order + // to get diagnostics if something about this builder call fails, + // e.g. if there isn't a matching overload for `buildBlock`. + // But we can only do this if there isn't a type variable in the type. + TypeLoc typeLoc(nullptr, + builderType->hasTypeVariable() ? Type() : builderType); + + auto typeExpr = new (ctx) TypeExpr(typeLoc); cs.setType(typeExpr, builderType); typeExpr->setImplicit(); auto memberRef = new (ctx) UnresolvedDotExpr( - typeExpr, SourceLoc(), fnName, DeclNameLoc(), /*implicit=*/true); - return CallExpr::createImplicit(ctx, memberRef, args, { }); + typeExpr, loc, fnName, DeclNameLoc(), /*implicit=*/true); + return CallExpr::createImplicit(ctx, memberRef, args, argLabels); } /// Check whether the builder supports the given operation. - bool builderSupports(Identifier fnName) { + bool builderSupports(Identifier fnName, + ArrayRef argLabels = {}) { auto known = supportedOps.find(fnName); if (known != supportedOps.end()) { return known->second; @@ -2804,10 +2814,21 @@ namespace { bool found = false; for (auto decl : builder->lookupDirect(fnName)) { if (auto func = dyn_cast(decl)) { - if (func->isStatic()) { - found = true; - break; + // Function must be static. + if (!func->isStatic()) + continue; + + // Function must have the right argument labels, if provided. + if (!argLabels.empty()) { + auto funcLabels = func->getFullName().getArgumentNames(); + if (argLabels.size() > funcLabels.size() || + funcLabels.slice(0, argLabels.size()) != argLabels) + continue; } + + // Okay, it's a good-enough match. + found = true; + break; } } @@ -2851,7 +2872,8 @@ namespace { } // Call Builder.buildBlock(... args ...) - return buildCallIfWanted(ctx.Id_buildBlock, expressions); + return buildCallIfWanted(braceStmt->getStartLoc(), + ctx.Id_buildBlock, expressions); } Expr *visitReturnStmt(ReturnStmt *returnStmt) { @@ -2869,7 +2891,7 @@ namespace { if (!arg) return nullptr; - return buildCallIfWanted(ctx.Id_buildDo, arg); + return buildCallIfWanted(doStmt->getStartLoc(), ctx.Id_buildDo, arg); } CONTROL_FLOW_STMT(Yield) @@ -2882,22 +2904,211 @@ namespace { return condition.front().getBooleanOrNull(); } + static bool isBuildableIfChainRecursive(IfStmt *ifStmt, + unsigned &numPayloads, + bool &isOptional) { + // The conditional must be trivial. + if (!getTrivialBooleanCondition(ifStmt->getCond())) + return false; + + // The 'then' clause contributes a payload. + numPayloads++; + + // If there's an 'else' clause, it contributes payloads: + if (auto elseStmt = ifStmt->getElseStmt()) { + // If it's 'else if', it contributes payloads recursively. + if (auto elseIfStmt = dyn_cast(elseStmt)) { + return isBuildableIfChainRecursive(elseIfStmt, numPayloads, + isOptional); + // Otherwise it's just the one. + } else { + numPayloads++; + } + + // If not, the chain result is at least optional. + } else { + isOptional = true; + } + + return true; + } + + bool isBuildableIfChain(IfStmt *ifStmt, unsigned &numPayloads, + bool &isOptional) { + if (!isBuildableIfChainRecursive(ifStmt, numPayloads, isOptional)) + return false; + + // If there's a missing 'else', we need 'buildIf' to exist. + if (isOptional && !builderSupports(ctx.Id_buildIf)) + return false; + + // If there are multiple clauses, we need 'buildEither(first:)' and + // 'buildEither(second:)' to both exist. + if (numPayloads > 1) { + if (!builderSupports(ctx.Id_buildEither, {ctx.Id_first}) || + !builderSupports(ctx.Id_buildEither, {ctx.Id_second})) + return false; + } + + return true; + } + Expr *visitIfStmt(IfStmt *ifStmt) { - if (!builderSupports(ctx.Id_buildIf) || - ifStmt->getElseStmt() || - !getTrivialBooleanCondition(ifStmt->getCond())) { + // Check whether the chain is buildable and whether it terminates + // without an `else`. + bool isOptional = false; + unsigned numPayloads = 0; + if (!isBuildableIfChain(ifStmt, numPayloads, isOptional)) { if (!unhandledNode) unhandledNode = ifStmt; return nullptr; } - auto thenArg = visit(ifStmt->getThenStmt()); - if (!thenArg) + // Attempt to build the chain, propagating short-circuits, which + // might arise either do to error or not wanting an expression. + auto chainExpr = + buildIfChainRecursive(ifStmt, 0, numPayloads, isOptional); + if (!chainExpr) return nullptr; + assert(wantExpr); - if (!wantExpr) + // The operand should have optional type if we had optional results, + // so we just need to call `buildIf` now, since we're at the top level. + if (isOptional) { + chainExpr = buildCallIfWanted(ifStmt->getStartLoc(), + ctx.Id_buildIf, chainExpr); + } + + return chainExpr; + } + + /// Recursively build an if-chain: build an expression which will have + /// a value of the chain result type before any call to `buildIf`. + /// The expression will perform any necessary calls to `buildEither`, + /// and the result will have optional type if `isOptional` is true. + Expr *buildIfChainRecursive(IfStmt *ifStmt, unsigned payloadIndex, + unsigned numPayloads, bool isOptional) { + assert(payloadIndex < numPayloads); + // Make sure we recursively visit both sides even if we're not + // building expressions. + + // Build the then clause. This will have the corresponding payload + // type (i.e. not wrapped in any way). + Expr *thenArg = visit(ifStmt->getThenStmt()); + + // Build the else clause, if present. If this is from an else-if, + // this will be fully wrapped; otherwise it will have the corresponding + // payload type (at index `payloadIndex + 1`). + assert(ifStmt->getElseStmt() || isOptional); + bool isElseIf = false; + Optional elseChain; + if (auto elseStmt = ifStmt->getElseStmt()) { + if (auto elseIfStmt = dyn_cast(elseStmt)) { + isElseIf = true; + elseChain = buildIfChainRecursive(elseIfStmt, payloadIndex + 1, + numPayloads, isOptional); + } else { + elseChain = visit(elseStmt); + } + } + + // Short-circuit if appropriate. + if (!wantExpr || !thenArg || (elseChain && !*elseChain)) return nullptr; + // Okay, build the conditional expression. + + // Prepare the `then` operand by wrapping it to produce a chain result. + SourceLoc thenLoc = ifStmt->getThenStmt()->getStartLoc(); + Expr *thenExpr = buildWrappedChainPayload(thenArg, payloadIndex, + numPayloads, isOptional); + + // Prepare the `else operand: + Expr *elseExpr; + SourceLoc elseLoc; + + // - If there's no `else` clause, use `Optional.none`. + if (!elseChain) { + assert(isOptional); + elseLoc = ifStmt->getEndLoc(); + elseExpr = buildNoneExpr(elseLoc); + + // - If there's an `else if`, the chain expression from that + // should already be producing a chain result. + } else if (isElseIf) { + elseExpr = *elseChain; + elseLoc = ifStmt->getElseLoc(); + + // - Otherwise, wrap it to produce a chain result. + } else { + elseLoc = ifStmt->getElseLoc(); + elseExpr = buildWrappedChainPayload(*elseChain, + payloadIndex + 1, numPayloads, + isOptional); + } + + Expr *condition = getTrivialBooleanCondition(ifStmt->getCond()); + assert(condition && "checked by isBuildableIfChain"); + + auto ifExpr = new (ctx) IfExpr(condition, thenLoc, thenExpr, + elseLoc, elseExpr); + ifExpr->setImplicit(); + return ifExpr; + } + + /// Wrap a payload value in an expression which will produce a chain + /// result (without `buildIf`). + Expr *buildWrappedChainPayload(Expr *operand, unsigned payloadIndex, + unsigned numPayloads, bool isOptional) { + assert(payloadIndex < numPayloads); + + // Inject into the appropriate chain position. + // + // We produce a (left-biased) balanced binary tree of Eithers in order + // to prevent requiring a linear number of injections in the worst case. + // That is, if we have 13 clauses, we want to produce: + // + // /------------------Either------------\ + // /-------Either-------\ /--Either--\ + // /--Either--\ /--Either--\ /--Either--\ \ + // /-E-\ /-E-\ /-E-\ /-E-\ /-E-\ /-E-\ \ + // 0000 0001 0010 0011 0100 0101 0110 0111 1000 1001 1010 1011 1100 + // + // Note that a prefix of length D of the payload index acts as a path + // through the tree to the node at depth D. On the rightmost path + // through the tree (when this prefix is equal to the corresponding + // prefix of the maximum payload index), the bits of the index mark + // where Eithers are required. + // + // Since we naturally want to build from the innermost Either out, and + // therefore work with progressively shorter prefixes, we can do it all + // with right-shifts. + for (auto path = payloadIndex, maxPath = numPayloads - 1; + maxPath != 0; path >>= 1, maxPath >>= 1) { + // Skip making Eithers on the rightmost path where they aren't required. + // This isn't just an optimization: adding spurious Eithers could + // leave us with unresolvable type variables if `buildEither` has + // a signature like: + // static func buildEither(first value: T) -> Either + // which relies on unification to work. + if (path == maxPath && !(maxPath & 1)) continue; + + bool isSecond = (path & 1); + operand = buildCallIfWanted(operand->getStartLoc(), + ctx.Id_buildEither, operand, + {isSecond ? ctx.Id_second : ctx.Id_first}); + } + + // Inject into Optional if required. We'll be adding the call to + // `buildIf` after all the recursive calls are complete. + if (isOptional) { + operand = buildSomeExpr(operand); + } + + return operand; + } + + Expr *buildSomeExpr(Expr *arg) { auto optionalDecl = ctx.getOptionalDecl(); auto optionalType = optionalDecl->getDeclaredType(); @@ -2905,19 +3116,17 @@ namespace { auto someRef = new (ctx) UnresolvedDotExpr( optionalTypeExpr, SourceLoc(), ctx.getIdentifier("some"), DeclNameLoc(), /*implicit=*/true); - auto someCall = CallExpr::createImplicit(ctx, someRef, thenArg, { }); + return CallExpr::createImplicit(ctx, someRef, arg, { }); + } - optionalTypeExpr = TypeExpr::createImplicit(optionalType, ctx); - auto noneRef = new (ctx) UnresolvedDotExpr( - optionalTypeExpr, ifStmt->getEndLoc(), ctx.getIdentifier("none"), - DeclNameLoc(ifStmt->getEndLoc()), /*implicit=*/true); + Expr *buildNoneExpr(SourceLoc endLoc) { + auto optionalDecl = ctx.getOptionalDecl(); + auto optionalType = optionalDecl->getDeclaredType(); - auto ifExpr = new (ctx) IfExpr( - getTrivialBooleanCondition(ifStmt->getCond()), - SourceLoc(), someCall, - SourceLoc(), noneRef); - ifExpr->setImplicit(); - return buildCallIfWanted(ctx.Id_buildIf, ifExpr); + auto optionalTypeExpr = TypeExpr::createImplicit(optionalType, ctx); + return new (ctx) UnresolvedDotExpr( + optionalTypeExpr, endLoc, ctx.getIdentifier("none"), + DeclNameLoc(endLoc), /*implicit=*/true); } CONTROL_FLOW_STMT(Guard) diff --git a/test/Constraints/function_builder.swift b/test/Constraints/function_builder.swift index b1f860948a705..79d4859f9cea6 100644 --- a/test/Constraints/function_builder.swift +++ b/test/Constraints/function_builder.swift @@ -1,6 +1,11 @@ // RUN: %target-run-simple-swift | %FileCheck %s // REQUIRES: executable_test +enum Either { + case first(T) + case second(U) +} + @_functionBuilder struct TupleBuilder { static func buildBlock(_ t1: T1, _ t2: T2) -> (T1, T2) { @@ -25,6 +30,13 @@ struct TupleBuilder { static func buildDo(_ value: T) -> T { return value } static func buildIf(_ value: T?) -> T? { return value } + + static func buildEither(first value: T) -> Either { + return .first(value) + } + static func buildEither(second value: U) -> Either { + return .second(value) + } } func tuplify(_ cond: Bool, @TupleBuilder body: (Bool) -> T) { @@ -56,6 +68,87 @@ tuplify(false) { } } +// CHECK: ("chain0", main.Either<(Swift.String, Swift.Double), (Swift.Double, Swift.String)>.second(2.8, "capable")) +tuplify(false) { + "chain0" + if $0 { + "marginal" + 2.9 + } else { + 2.8 + "capable" + } +} + +// CHECK: ("chain1", nil) +tuplify(false) { + "chain1" + if $0 { + "marginal" + 2.9 + } else if $0 { + 2.8 + "capable" + } +} + +// CHECK: ("chain2", Optional(main.Either<(Swift.String, Swift.Double), (Swift.Double, Swift.String)>.first("marginal", 2.9))) +tuplify(true) { + "chain2" + if $0 { + "marginal" + 2.9 + } else if $0 { + 2.8 + "capable" + } +} + +// CHECK: ("chain3", main.Either, main.Either<(Swift.Double, Swift.Double), (Swift.String, Swift.String)>>.first(main.Either<(Swift.String, Swift.Double), (Swift.Double, Swift.String)>.first("marginal", 2.9))) +tuplify(true) { + "chain3" + if $0 { + "marginal" + 2.9 + } else if $0 { + 2.8 + "capable" + } else if $0 { + 2.8 + 1.0 + } else { + "wild" + "broken" + } +} + +// CHECK: ("chain4", main.Either, main.Either<(Swift.String, Swift.Int), (Swift.String, Swift.Int)>>, main.Either, (Swift.String, Swift.Int)>>.first +tuplify(true) { + "chain4" + if $0 { + "0" + 0 + } else if $0 { + "1" + 1 + } else if $0 { + "2" + 2 + } else if $0 { + "3" + 3 + } else if $0 { + "4" + 4 + } else if $0 { + "5" + 5 + } else { + "6" + 6 + } +} + struct Tagged { let tag: Tag let entity: Entity From 078005925f62fed61f6a3db386559a4ecaf7e780 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 19 Apr 2019 16:30:51 -0700 Subject: [PATCH 29/38] [Constraint solver] Capture/roll back type mappings generated while solving. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we perform constraint generation while solving, capture the type mappings we generate as part of the solver scope and solutions, rolling them back and replaying them as necessary. Otherwise, we’ll end up with uses of stale type variables, expression/parameter/type-loc types set twice, etc. Fixes rdar://problem/50390449 and rdar://problem/50150314. --- lib/Sema/CSApply.cpp | 5 ++ lib/Sema/CSSolver.cpp | 21 ++++++ lib/Sema/ConstraintSystem.cpp | 6 ++ lib/Sema/ConstraintSystem.h | 66 ++++++++++++++----- test/Constraints/function_builder_diags.swift | 7 +- 5 files changed, 83 insertions(+), 22 deletions(-) diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 1b8383bdca6c2..c4e1ff50e4b3b 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -7690,6 +7690,11 @@ Expr *ConstraintSystem::applySolution(Solution &solution, Expr *expr, Type convertType, bool discardedExpr, bool skipClosures) { + // Add the node types back. + for (auto &nodeType : solution.addedNodeTypes) { + setType(nodeType.first, nodeType.second); + } + // If any fixes needed to be applied to arrive at this solution, resolve // them to specific expressions. if (!solution.Fixes.empty()) { diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 2580d6623676a..2112d61f2149b 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -169,10 +169,19 @@ Solution ConstraintSystem::finalize() { solution.DefaultedConstraints.insert(DefaultedConstraints.begin(), DefaultedConstraints.end()); + for (auto &nodeType : addedNodeTypes) { + solution.addedNodeTypes.push_back(nodeType); + } + for (auto &e : CheckedConformances) solution.Conformances.push_back({e.first, e.second}); for (const auto &transformed : builderTransformedClosures) { + auto known = + solution.builderTransformedClosures.find(std::get<0>(transformed)); + if (known != solution.builderTransformedClosures.end()) { + assert(known->second.second == std::get<2>(transformed)); + } solution.builderTransformedClosures.insert( std::make_pair(std::get<0>(transformed), std::make_pair(std::get<1>(transformed), @@ -239,6 +248,11 @@ void ConstraintSystem::applySolution(const Solution &solution) { DefaultedConstraints.append(solution.DefaultedConstraints.begin(), solution.DefaultedConstraints.end()); + // Add the node types back. + for (auto &nodeType : solution.addedNodeTypes) { + setType(nodeType.first, nodeType.second); + } + // Register the conformances checked along the way to arrive to solution. for (auto &conformance : solution.Conformances) CheckedConformances.push_back(conformance); @@ -435,6 +449,7 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs) numOpenedTypes = cs.OpenedTypes.size(); numOpenedExistentialTypes = cs.OpenedExistentialTypes.size(); numDefaultedConstraints = cs.DefaultedConstraints.size(); + numAddedNodeTypes = cs.addedNodeTypes.size(); numCheckedConformances = cs.CheckedConformances.size(); numMissingMembers = cs.MissingMembers.size(); numDisabledConstraints = cs.solverState->getNumDisabledConstraints(); @@ -488,6 +503,12 @@ ConstraintSystem::SolverScope::~SolverScope() { // Remove any defaulted type variables. truncate(cs.DefaultedConstraints, numDefaultedConstraints); + // Remove any node types we registered. + for (unsigned i : range(numAddedNodeTypes, cs.addedNodeTypes.size())) { + cs.eraseType(cs.addedNodeTypes[i].first); + } + truncate(cs.addedNodeTypes, numAddedNodeTypes); + // Remove any conformances checked along the current path. truncate(cs.CheckedConformances, numCheckedConformances); diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index a2ee652f96ab2..fc03f85e75783 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -3262,6 +3262,12 @@ ConstraintSystem::TypeMatchResult ConstraintSystem::applyFunctionBuilder( assert(transformedType && "Missing type"); // Record the transformation. + assert(std::find_if(builderTransformedClosures.begin(), + builderTransformedClosures.end(), + [&](const std::tuple &elt) { + return std::get<0>(elt) == closure; + }) == builderTransformedClosures.end() && + "already transformed this closure along this path!?!"); builderTransformedClosures.push_back( std::make_tuple(closure, builderType, singleExpr)); diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index 792dbf4e58353..23e803ab8a2fb 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -544,6 +544,9 @@ struct Score { }; +/// An AST node that can gain type information while solving. +using TypedNode = llvm::PointerUnion3; + /// Display a score. llvm::raw_ostream &operator<<(llvm::raw_ostream &out, const Score &score); @@ -616,6 +619,9 @@ class Solution { /// The locators of \c Defaultable constraints whose defaults were used. llvm::SmallPtrSet DefaultedConstraints; + /// The node -> type mappings introduced by this solution. + llvm::SmallVector, 8> addedNodeTypes; + llvm::SmallVector, 8> Conformances; @@ -1102,6 +1108,9 @@ class ConstraintSystem { SmallVector, 4> OpenedExistentialTypes; + /// The node -> type mappings introduced by generating constraints. + llvm::SmallVector, 8> addedNodeTypes; + SmallVector, 8> CheckedConformances; @@ -1568,6 +1577,8 @@ class ConstraintSystem { /// The length of \c DefaultedConstraints. unsigned numDefaultedConstraints; + unsigned numAddedNodeTypes; + unsigned numCheckedConformances; unsigned numMissingMembers; @@ -1718,33 +1729,56 @@ class ConstraintSystem { this->FavoredTypes[E] = T; } + /// Set the type in our type map for the given node. + /// + /// The side tables are used through the expression type checker to avoid mutating nodes until + /// we know we have successfully type-checked them. + void setType(TypedNode node, Type type) { + assert(!node.isNull() && "Cannot set type information on null node"); + assert(type && "Expected non-null type"); + + // Record the type. + if (auto expr = node.dyn_cast()) { + ExprTypes[expr] = type.getPointer(); + } else if (auto typeLoc = node.dyn_cast()) { + TypeLocTypes[typeLoc] = type.getPointer(); + } else { + auto param = node.get(); + ParamTypes[param] = type.getPointer(); + } + + // Record the fact that we ascribed a type to this node. + if (solverState && solverState->depth > 0) { + addedNodeTypes.push_back({node, type}); + } + } + /// Set the type in our type map for a given expression. The side /// map is used throughout the expression type checker in order to /// avoid mutating expressions until we know we have successfully /// type-checked them. void setType(Expr *E, Type T) { - assert(E != nullptr && "Expected non-null expression!"); - assert(T && "Expected non-null type!"); - - // FIXME: We sometimes set the type and then later set it to a - // value that is slightly different, e.g. not an lvalue. - // assert((ExprTypes.find(E) == ExprTypes.end() || - // ExprTypes.find(E)->second->isEqual(T) || - // ExprTypes.find(E)->second->hasTypeVariable()) && - // "Expected type to be invariant!"); - - ExprTypes[E] = T.getPointer(); + setType(TypedNode(E), T); } void setType(TypeLoc &L, Type T) { - assert(T && "Expected non-null type!"); - TypeLocTypes[&L] = T.getPointer(); + setType(TypedNode(&L), T); } void setType(ParamDecl *P, Type T) { - assert(P && "Expected non-null parameter!"); - assert(T && "Expected non-null type!"); - ParamTypes[P] = T.getPointer(); + setType(TypedNode(P), T); + } + + /// Erase the type for the given node. + void eraseType(TypedNode node) { + if (auto expr = node.dyn_cast()) { + ExprTypes.erase(expr); + } else if (auto typeLoc = node.dyn_cast()) { + TypeLocTypes.erase(typeLoc); + } else { + auto param = node.get(); + ParamTypes.erase(param); + } } /// Check to see if we have a type for an expression. diff --git a/test/Constraints/function_builder_diags.swift b/test/Constraints/function_builder_diags.swift index 298679ac963cd..715dff972bf53 100644 --- a/test/Constraints/function_builder_diags.swift +++ b/test/Constraints/function_builder_diags.swift @@ -114,13 +114,8 @@ func testOverloading(name: String) { let _: A = a1 -#if false - // FIXME: Using the overloads causes a crash, which is likely related - // to us generating the constraints from the child expressions multiple - // times. - let b1 = overloadedTuplify(true) { b in + _ = overloadedTuplify(true) { b in b ? "Hello, \(name)" : "Goodbye" 42 } -#endif } From 57cd6106e908ff42e734b66a54e92836e1bb0712 Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 7 May 2019 16:38:18 -0400 Subject: [PATCH 30/38] Allow function-builder attributes on funcs and computed vars. rdar://50150690 --- include/swift/AST/Decl.h | 16 +++---- include/swift/AST/DiagnosticsSema.def | 13 ++++-- include/swift/AST/TypeCheckRequests.h | 17 ++++---- lib/AST/Decl.cpp | 16 +++---- lib/Sema/ConstraintSystem.cpp | 52 +++++++++++++++++++---- lib/Sema/TypeCheckAttr.cpp | 32 ++++++++++---- lib/Sema/TypeCheckRequestFunctions.cpp | 56 +++++++++++++------------ lib/Sema/TypeCheckStmt.cpp | 18 ++++++++ lib/Sema/TypeChecker.h | 1 + test/Constraints/function_builder.swift | 19 +++++++++ test/decl/var/function_builders.swift | 13 +++--- 11 files changed, 178 insertions(+), 75 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index e1a1d980b04f0..4ad87a3634b43 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -2705,6 +2705,14 @@ class ValueDecl : public Decl { /// `this` must be of a decl type that supports opaque return types, and /// must not have previously had an opaque result type set. void setOpaqueResultTypeDecl(OpaqueTypeDecl *D); + + /// Retrieve the attribute associating this declaration with a + /// function builder, if there is one. + CustomAttr *getAttachedFunctionBuilder() const; + + /// Retrieve the @functionBuilder type attached to this declaration, + /// if there is one. + Type getFunctionBuilderType() const; }; /// This is a common base class for declarations which declare a type. @@ -5310,14 +5318,6 @@ class ParamDecl : public VarDecl { return getVarargBaseTy(getInterfaceType()); } - /// Retrieve the attribute marking this as a function builder parameter, - /// if there is one. - CustomAttr *getAttachedFunctionBuilder() const; - - /// Retrieve the @functionBuilder type attached to this parameter, - /// if there is one. - Type getFunctionBuilderType() const; - SourceRange getSourceRange() const; // Implement isa/cast/dyncast/etc. diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 34a5e896351b6..1233fcee476b4 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4458,9 +4458,13 @@ NOTE(note_function_builder_control_flow, none, "builder %0", (DeclName)) ERROR(function_builder_reserved_name, none, "function builder type name must start with an uppercase letter", ()) -ERROR(function_builder_attribute_not_on_parameter, none, - "function builder attribute %0 can only be applied to a parameter", - (DeclName)) +ERROR(function_builder_attribute_not_allowed_here, none, + "function builder attribute %0 can only be applied to a parameter, " + "function, or storage with a getter", (DeclName)) +ERROR(function_builder_attribute_on_storage_without_getter, none, + "function builder attribute %0 can only be applied to a " + "%select{subscript|property|constant|variable}1 if it defines a getter", + (DeclName, unsigned)) ERROR(function_builder_parameter_not_of_function_type, none, "function builder attribute %0 can only be applied to a parameter of " "function type", @@ -4470,7 +4474,8 @@ ERROR(function_builder_parameter_autoclosure, none, "parameter", (DeclName)) ERROR(function_builder_multiple, none, - "only one function builder attribute can be attached to a parameter", ()) + "only one function builder attribute can be attached to a " + "%select{declaration|parameter}0", (bool)) NOTE(previous_function_builder_here, none, "previous function builder specified here", ()) ERROR(function_builder_arguments, none, diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index e34dea56ef10c..07d6148cd6947 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -30,11 +30,11 @@ namespace swift { class GenericParamList; -class ParamDecl; struct PropertyWrapperBackingPropertyInfo; class RequirementRepr; class SpecializeAttr; struct TypeLoc; +class ValueDecl; /// Display a nominal type or extension thereof. void simple_display( @@ -523,12 +523,12 @@ class PropertyWrapperBackingPropertyInfoRequest : }; /// Request the custom attribute which attaches a function builder to the -/// given parameter. +/// given declaration. class AttachedFunctionBuilderRequest : public SimpleRequest { + ValueDecl *> { public: using SimpleRequest::SimpleRequest; @@ -537,7 +537,7 @@ class AttachedFunctionBuilderRequest : // Evaluation. llvm::Expected - evaluate(Evaluator &evaluator, ParamDecl *) const; + evaluate(Evaluator &evaluator, ValueDecl *decl) const; public: // Caching @@ -548,14 +548,13 @@ class AttachedFunctionBuilderRequest : void noteCycleStep(DiagnosticEngine &diags) const; }; -/// Request the type spelled out in a custom attribute. -/// -/// Different parameters cannot be used for the same attribute. +/// Request the function builder type attached to the given declaration, +/// if any. class FunctionBuilderTypeRequest : public SimpleRequest { + ValueDecl *> { public: using SimpleRequest::SimpleRequest; @@ -563,7 +562,7 @@ class FunctionBuilderTypeRequest : friend SimpleRequest; llvm::Expected - evaluate(Evaluator &evaluator, ParamDecl *param) const; + evaluate(Evaluator &evaluator, ValueDecl *decl) const; public: // Caching diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 401461cbad2fc..0d59e6aa997b8 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -5649,25 +5649,25 @@ void ParamDecl::setStoredProperty(VarDecl *var) { DefaultValueAndFlags.getPointer()->DefaultArg = var; } -Type ParamDecl::getFunctionBuilderType() const { - // Fast path: most parameters do not have any attributes at all, - // much less custom ones. +Type ValueDecl::getFunctionBuilderType() const { + // Fast path: most declarations (especially parameters, which is where + // this is hottest) do not have any custom attributes at all. if (!getAttrs().hasAttribute()) return Type(); auto &ctx = getASTContext(); - auto mutableThis = const_cast(this); + auto mutableThis = const_cast(this); return evaluateOrDefault(ctx.evaluator, FunctionBuilderTypeRequest{mutableThis}, Type()); } -CustomAttr *ParamDecl::getAttachedFunctionBuilder() const { - // Fast path: most parameters do not have any attributes at all, - // much less custom ones. +CustomAttr *ValueDecl::getAttachedFunctionBuilder() const { + // Fast path: most declarations (especially parameters, which is where + // this is hottest) do not have any custom attributes at all. if (!getAttrs().hasAttribute()) return nullptr; auto &ctx = getASTContext(); - auto mutableThis = const_cast(this); + auto mutableThis = const_cast(this); return evaluateOrDefault(ctx.evaluator, AttachedFunctionBuilderRequest{mutableThis}, nullptr); diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index fc03f85e75783..3a7993a0b6b11 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2770,7 +2770,7 @@ namespace { /// Visitor to classify the contents of the given closure. class BuilderClosureVisitor : public StmtVisitor { - ConstraintSystem &cs; + ConstraintSystem *cs; ASTContext &ctx; bool wantExpr; Type builderType; @@ -2796,10 +2796,10 @@ namespace { builderType->hasTypeVariable() ? Type() : builderType); auto typeExpr = new (ctx) TypeExpr(typeLoc); - cs.setType(typeExpr, builderType); + if (cs) cs->setType(typeExpr, builderType); typeExpr->setImplicit(); auto memberRef = new (ctx) UnresolvedDotExpr( - typeExpr, loc, fnName, DeclNameLoc(), /*implicit=*/true); + typeExpr, loc, fnName, DeclNameLoc(loc), /*implicit=*/true); return CallExpr::createImplicit(ctx, memberRef, args, argLabels); } @@ -2836,9 +2836,12 @@ namespace { } public: - BuilderClosureVisitor(ConstraintSystem &cs, bool wantExpr, Type builderType) - : cs(cs), ctx(cs.getASTContext()), wantExpr(wantExpr), - builderType(builderType) { + BuilderClosureVisitor(ASTContext &ctx, ConstraintSystem *cs, + bool wantExpr, Type builderType) + : cs(cs), ctx(ctx), wantExpr(wantExpr), builderType(builderType) { + assert((cs || !builderType->hasTypeVariable()) && + "cannot handle builder type with type variables without " + "constraint system"); builder = builderType->getAnyNominal(); } @@ -3187,6 +3190,37 @@ static bool hasReturnStmt(Stmt *stmt) { return finder.hasReturnStmt; } +bool TypeChecker::typeCheckFunctionBuilderFuncBody(FuncDecl *FD, + Type builderType) { + // Try to build a single result expression. + BuilderClosureVisitor visitor(Context, nullptr, + /*wantExpr=*/true, builderType); + Expr *returnExpr = visitor.visit(FD->getBody()); + if (!returnExpr) + return true; + + // Make sure we have a usable result type for the body. + Type returnType = AnyFunctionRef(FD).getBodyResultType(); + if (!returnType || returnType->hasError()) + return true; + + // Type-check the single result expression. + Type returnExprType = typeCheckExpression(returnExpr, FD, + TypeLoc::withoutLoc(returnType), + CTP_ReturnStmt); + if (!returnExprType) + return true; + assert(returnExprType->isEqual(returnType)); + + auto returnStmt = new (Context) ReturnStmt(SourceLoc(), returnExpr); + auto origBody = FD->getBody(); + auto fakeBody = BraceStmt::create(Context, origBody->getLBraceLoc(), + { returnStmt }, + origBody->getRBraceLoc()); + FD->setBody(fakeBody); + return false; +} + ConstraintSystem::TypeMatchResult ConstraintSystem::applyFunctionBuilder( ClosureExpr *closure, Type builderType, ConstraintLocator *calleeLocator, ConstraintLocatorBuilder locator) { @@ -3209,7 +3243,8 @@ ConstraintSystem::TypeMatchResult ConstraintSystem::applyFunctionBuilder( } // Check whether we can apply this function builder. - BuilderClosureVisitor visitor(*this, /*wantExpr=*/false, builderType); + BuilderClosureVisitor visitor(getASTContext(), this, + /*wantExpr=*/false, builderType); (void)visitor.visit(closure->getBody()); @@ -3247,7 +3282,8 @@ ConstraintSystem::TypeMatchResult ConstraintSystem::applyFunctionBuilder( assert(!builderType->hasTypeParameter()); } - BuilderClosureVisitor visitor(*this, /*wantExpr=*/true, builderType); + BuilderClosureVisitor visitor(getASTContext(), this, + /*wantExpr=*/true, builderType); Expr *singleExpr = visitor.visit(closure->getBody()); if (TC.precheckedClosures.insert(closure).second && diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index dcbb81c7b9e50..2682f77415505 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -2576,12 +2576,29 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) { } // If the nominal type is a function builder type, verify that D is a - // parameter of function type. + // function, storage with an explicit getter, or parameter of function type. if (nominal->getAttrs().hasAttribute()) { - auto param = dyn_cast(D); - if (!param) { + ValueDecl *decl; + if (auto param = dyn_cast(D)) { + decl = param; + } else if (auto func = dyn_cast(D)) { + decl = func; + } else if (auto storage = dyn_cast(D)) { + decl = storage; + auto getter = storage->getGetter(); + if (!getter || getter->isImplicit() || !getter->hasBody()) { + TC.diagnose(attr->getLocation(), + diag::function_builder_attribute_on_storage_without_getter, + nominal->getFullName(), + isa(storage) ? 0 + : storage->getDeclContext()->isTypeContext() ? 1 + : cast(storage)->isLet() ? 2 : 3); + attr->setInvalid(); + return; + } + } else { TC.diagnose(attr->getLocation(), - diag::function_builder_attribute_not_on_parameter, + diag::function_builder_attribute_not_allowed_here, nominal->getFullName()); attr->setInvalid(); return; @@ -2594,9 +2611,10 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) { } // Complain if this isn't the primary function-builder attribute. - auto attached = param->getAttachedFunctionBuilder(); + auto attached = decl->getAttachedFunctionBuilder(); if (attached != attr) { - TC.diagnose(attr->getLocation(), diag::function_builder_multiple); + TC.diagnose(attr->getLocation(), diag::function_builder_multiple, + isa(decl)); TC.diagnose(attached->getLocation(), diag::previous_function_builder_here); attr->setInvalid(); @@ -2604,7 +2622,7 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) { } else { // Force any diagnostics associated with computing the function-builder // type. - (void) param->getFunctionBuilderType(); + (void) decl->getFunctionBuilderType(); } return; diff --git a/lib/Sema/TypeCheckRequestFunctions.cpp b/lib/Sema/TypeCheckRequestFunctions.cpp index 50ada97030bf3..3b22da813bbe1 100644 --- a/lib/Sema/TypeCheckRequestFunctions.cpp +++ b/lib/Sema/TypeCheckRequestFunctions.cpp @@ -150,10 +150,10 @@ EnumRawTypeRequest::evaluate(Evaluator &evaluator, EnumDecl *enumDecl, llvm::Expected AttachedFunctionBuilderRequest::evaluate(Evaluator &evaluator, - ParamDecl *param) const { - ASTContext &ctx = param->getASTContext(); - auto dc = param->getDeclContext(); - for (auto attr : param->getAttrs().getAttributes()) { + ValueDecl *decl) const { + ASTContext &ctx = decl->getASTContext(); + auto dc = decl->getDeclContext(); + for (auto attr : decl->getAttrs().getAttributes()) { auto mutableAttr = const_cast(attr); // Figure out which nominal declaration this custom attribute refers to. auto nominal = evaluateOrDefault(ctx.evaluator, @@ -174,14 +174,14 @@ AttachedFunctionBuilderRequest::evaluate(Evaluator &evaluator, llvm::Expected FunctionBuilderTypeRequest::evaluate(Evaluator &evaluator, - ParamDecl *param) const { + ValueDecl *decl) const { // Look for a function-builder custom attribute. - auto attr = param->getAttachedFunctionBuilder(); + auto attr = decl->getAttachedFunctionBuilder(); if (!attr) return Type(); // Resolve a type for the attribute. auto mutableAttr = const_cast(attr); - auto dc = param->getDeclContext(); + auto dc = decl->getDeclContext(); auto &ctx = dc->getASTContext(); Type type = resolveCustomAttrType(mutableAttr, dc, CustomAttrTypeKind::NonGeneric); @@ -193,26 +193,30 @@ FunctionBuilderTypeRequest::evaluate(Evaluator &evaluator, return Type(); } - // The parameter had better already have an interface type. - Type paramType = param->getInterfaceType(); - assert(paramType); - auto paramFnType = paramType->getAs(); - - // Require the parameter to be an interface type. - if (!paramFnType) { - ctx.Diags.diagnose(attr->getLocation(), - diag::function_builder_parameter_not_of_function_type, - nominal->getFullName()); - mutableAttr->setInvalid(); - return Type(); - } + // Do some additional checking on parameters. + if (auto param = dyn_cast(decl)) { + // The parameter had better already have an interface type. + Type paramType = param->getInterfaceType(); + assert(paramType); + auto paramFnType = paramType->getAs(); + + // Require the parameter to be an interface type. + if (!paramFnType) { + ctx.Diags.diagnose(attr->getLocation(), + diag::function_builder_parameter_not_of_function_type, + nominal->getFullName()); + mutableAttr->setInvalid(); + return Type(); + } - if (param->isAutoClosure()) { - ctx.Diags.diagnose(attr->getLocation(), - diag::function_builder_parameter_autoclosure, - nominal->getFullName()); - mutableAttr->setInvalid(); - return Type(); + // Forbid the parameter to be an autoclosure. + if (param->isAutoClosure()) { + ctx.Diags.diagnose(attr->getLocation(), + diag::function_builder_parameter_autoclosure, + nominal->getFullName()); + mutableAttr->setInvalid(); + return Type(); + } } return type->mapTypeOutOfContext(); diff --git a/lib/Sema/TypeCheckStmt.cpp b/lib/Sema/TypeCheckStmt.cpp index 2fbd312bf834f..1aef894aa877d 100644 --- a/lib/Sema/TypeCheckStmt.cpp +++ b/lib/Sema/TypeCheckStmt.cpp @@ -1937,6 +1937,20 @@ bool TypeChecker::typeCheckAbstractFunctionBody(AbstractFunctionDecl *AFD) { return false; } +static Type getFunctionBuilderType(FuncDecl *FD) { + Type builderType = FD->getFunctionBuilderType(); + + // For getters, fall back on looking on the attribute on the storage. + if (!builderType) { + auto accessor = dyn_cast(FD); + if (accessor && accessor->getAccessorKind() == AccessorKind::Get) { + builderType = accessor->getStorage()->getFunctionBuilderType(); + } + } + + return builderType; +} + // Type check a function body (defined with the func keyword) that is either a // named function or an anonymous func expression. bool TypeChecker::typeCheckFunctionBodyUntil(FuncDecl *FD, @@ -1949,6 +1963,10 @@ bool TypeChecker::typeCheckFunctionBodyUntil(FuncDecl *FD, BraceStmt *BS = FD->getBody(); assert(BS && "Should have a body"); + if (Type builderType = getFunctionBuilderType(FD)) { + return typeCheckFunctionBuilderFuncBody(FD, builderType); + } + if (FD->hasSingleExpressionBody()) { auto resultTypeLoc = FD->getBodyResultTypeLoc(); auto E = FD->getSingleExpressionBody(); diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index 8b83a53e9b9f8..b054d2fb76e0b 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -1039,6 +1039,7 @@ class TypeChecker final : public LazyResolver { bool typeCheckDestructorBodyUntil(DestructorDecl *DD, SourceLoc EndTypeCheckLoc); + bool typeCheckFunctionBuilderFuncBody(FuncDecl *FD, Type builderType); bool typeCheckClosureBody(ClosureExpr *closure); bool typeCheckTapBody(TapExpr *expr, DeclContext *DC); diff --git a/test/Constraints/function_builder.swift b/test/Constraints/function_builder.swift index 79d4859f9cea6..0c3952128fcbc 100644 --- a/test/Constraints/function_builder.swift +++ b/test/Constraints/function_builder.swift @@ -149,6 +149,25 @@ tuplify(true) { } } +// CHECK: ("getterBuilder", 0, 4, 12) +@TupleBuilder +var globalBuilder: (String, Int, Int, Int) { + "getterBuilder" + 0 + 4 + 12 +} +print(globalBuilder) + +// CHECK: ("funcBuilder", 13, 45.0) +@TupleBuilder +func funcBuilder(d: Double) -> (String, Int, Double) { + "funcBuilder" + 13 + d +} +print(funcBuilder(d: 45)) + struct Tagged { let tag: Tag let entity: Entity diff --git a/test/decl/var/function_builders.swift b/test/decl/var/function_builders.swift index ead48c384b3c0..cedaf98d3ae10 100644 --- a/test/decl/var/function_builders.swift +++ b/test/decl/var/function_builders.swift @@ -12,14 +12,17 @@ struct Maker {} @_functionBuilder class Inventor {} -@Maker // expected-error {{function builder attribute 'Maker' can only be applied to a parameter}} +@Maker // expected-error {{function builder attribute 'Maker' can only be applied to a variable if it defines a getter}} var global: Int -@Maker // expected-error {{function builder attribute 'Maker' can only be applied to a parameter}} -func globalFunction() {} +@Make +var globalWithGetter: Int {} // expected-error {{ype 'Maker' has no member 'buildBlock'}} -@Maker // expected-error {{function builder attribute 'Maker' can only be applied to a parameter}} -func globalFunctionWithFunctionParam(fn: () -> ()) {} +@Maker +func globalFunction() {} // expected-error {{ype 'Maker' has no member 'buildBlock'}} + +@Maker +func globalFunctionWithFunctionParam(fn: () -> ()) {} // expected-error {{ype 'Maker' has no member 'buildBlock'}} func makerParam(@Maker fn: () -> ()) {} From 6afd405fbd6c28c474ef3eab93c09078c5743963 Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 7 May 2019 20:02:13 -0400 Subject: [PATCH 31/38] Fix treatment of single-line getters with function builders. rdar://50561122 --- lib/Sema/ConstraintSystem.cpp | 11 +++++++++-- test/decl/var/function_builders.swift | 16 ++++++++++++++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 3a7993a0b6b11..e7cfc0e914433 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2879,8 +2879,15 @@ namespace { ctx.Id_buildBlock, expressions); } - Expr *visitReturnStmt(ReturnStmt *returnStmt) { - llvm_unreachable("Should not try visit bodies with return statements"); + Expr *visitReturnStmt(ReturnStmt *stmt) { + // Allow implicit returns due to 'return' elision. + if (!stmt->isImplicit() || !stmt->hasResult()) { + if (!unhandledNode) + unhandledNode = stmt; + return nullptr; + } + + return stmt->getResult(); } Expr *visitDoStmt(DoStmt *doStmt) { diff --git a/test/decl/var/function_builders.swift b/test/decl/var/function_builders.swift index cedaf98d3ae10..2ae02c1769947 100644 --- a/test/decl/var/function_builders.swift +++ b/test/decl/var/function_builders.swift @@ -15,8 +15,20 @@ class Inventor {} @Maker // expected-error {{function builder attribute 'Maker' can only be applied to a variable if it defines a getter}} var global: Int -@Make -var globalWithGetter: Int {} // expected-error {{ype 'Maker' has no member 'buildBlock'}} +// FIXME: should this be allowed? +@Maker +var globalWithEmptyImplicitGetter: Int {} +// expected-error@-1 {{computed property must have accessors specified}} +// expected-error@-3 {{function builder attribute 'Maker' can only be applied to a variable if it defines a getter}} + +@Maker +var globalWithEmptyExplicitGetter: Int { get {} } // expected-error {{ype 'Maker' has no member 'buildBlock'}} + +@Maker +var globalWithSingleGetter: Int { 0 } // expected-error {{ype 'Maker' has no member 'buildBlock'}} + +@Maker +var globalWithMultiGetter: Int { 0; 0 } // expected-error {{ype 'Maker' has no member 'buildBlock'}} @Maker func globalFunction() {} // expected-error {{ype 'Maker' has no member 'buildBlock'}} From 3c3d130bec0db6b429977c9668e581a55ce56c89 Mon Sep 17 00:00:00 2001 From: John McCall Date: Wed, 8 May 2019 15:37:48 -0400 Subject: [PATCH 32/38] Address review feedback on the function-builders-on-funcs/vars patch. --- include/swift/AST/DiagnosticsSema.def | 2 +- test/Constraints/function_builder.swift | 31 +++++++++++++++++++++++++ test/decl/var/function_builders.swift | 3 +++ 3 files changed, 35 insertions(+), 1 deletion(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 1233fcee476b4..e7b603cf1fad6 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4460,7 +4460,7 @@ ERROR(function_builder_reserved_name, none, "function builder type name must start with an uppercase letter", ()) ERROR(function_builder_attribute_not_allowed_here, none, "function builder attribute %0 can only be applied to a parameter, " - "function, or storage with a getter", (DeclName)) + "function, or computed property", (DeclName)) ERROR(function_builder_attribute_on_storage_without_getter, none, "function builder attribute %0 can only be applied to a " "%select{subscript|property|constant|variable}1 if it defines a getter", diff --git a/test/Constraints/function_builder.swift b/test/Constraints/function_builder.swift index 0c3952128fcbc..9d3a2a2376512 100644 --- a/test/Constraints/function_builder.swift +++ b/test/Constraints/function_builder.swift @@ -168,6 +168,37 @@ func funcBuilder(d: Double) -> (String, Int, Double) { } print(funcBuilder(d: 45)) +struct MemberBuilders { + @TupleBuilder + func methodBuilder(_ i: Int) -> (String, Int) { + "methodBuilder" + i + } + + @TupleBuilder + static func staticMethodBuilder(_ i: Int) -> (String, Int) { + "staticMethodBuilder" + i + 14 + } + + @TupleBuilder + var propertyBuilder: (String, Int) { + "propertyBuilder" + 12 + } +} + +// CHECK: ("staticMethodBuilder", 27) +print(MemberBuilders.staticMethodBuilder(13)) + +let mbuilders = MemberBuilders() + +// CHECK: ("methodBuilder", 13) +print(mbuilders.methodBuilder(13)) + +// CHECK: ("propertyBuilder", 12) +print(mbuilders.propertyBuilder) + struct Tagged { let tag: Tag let entity: Entity diff --git a/test/decl/var/function_builders.swift b/test/decl/var/function_builders.swift index 2ae02c1769947..ec56a0d27677f 100644 --- a/test/decl/var/function_builders.swift +++ b/test/decl/var/function_builders.swift @@ -12,6 +12,9 @@ struct Maker {} @_functionBuilder class Inventor {} +@Maker // expected-error {{function builder attribute 'Maker' can only be applied to a parameter, function, or computed property}} +typealias typename = Inventor + @Maker // expected-error {{function builder attribute 'Maker' can only be applied to a variable if it defines a getter}} var global: Int From 810cfc4d7862e5b1be6a427a3a5fec051b4bde23 Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Thu, 16 May 2019 21:37:34 -0700 Subject: [PATCH 33/38] [Type checker] Don't re-insert node types when merging solutions. Merging partial solutions can end up assigning the same type to a particular typed node (expression, parameter, etc.), which can lead to unbalanced set/clear when exploring the solution space (and later on, crashes). Don't re-insert such information. This is the same approach taken for type variable bindings, but it's all pretty unfortunate: partial solutions should only record information relative to their part of the constraint system, which would save time and memory during solving. Howver, that's too big a change for right now. Fixes rdar://problem/50853028. --- lib/Sema/CSSolver.cpp | 3 +- lib/Sema/ConstraintSystem.h | 40 ++++++++++--------- test/Constraints/function_builder_diags.swift | 7 ++++ 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 2112d61f2149b..4280f25f078ef 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -250,7 +250,8 @@ void ConstraintSystem::applySolution(const Solution &solution) { // Add the node types back. for (auto &nodeType : solution.addedNodeTypes) { - setType(nodeType.first, nodeType.second); + if (!hasType(nodeType.first)) + setType(nodeType.first, nodeType.second); } // Register the conformances checked along the way to arrive to solution. diff --git a/lib/Sema/ConstraintSystem.h b/lib/Sema/ConstraintSystem.h index 23e803ab8a2fb..76f4c360851b8 100644 --- a/lib/Sema/ConstraintSystem.h +++ b/lib/Sema/ConstraintSystem.h @@ -545,7 +545,9 @@ struct Score { }; /// An AST node that can gain type information while solving. -using TypedNode = llvm::PointerUnion3; +using TypedNode = + llvm::PointerUnion3; /// Display a score. llvm::raw_ostream &operator<<(llvm::raw_ostream &out, const Score &score); @@ -1738,12 +1740,12 @@ class ConstraintSystem { assert(type && "Expected non-null type"); // Record the type. - if (auto expr = node.dyn_cast()) { + if (auto expr = node.dyn_cast()) { ExprTypes[expr] = type.getPointer(); - } else if (auto typeLoc = node.dyn_cast()) { + } else if (auto typeLoc = node.dyn_cast()) { TypeLocTypes[typeLoc] = type.getPointer(); } else { - auto param = node.get(); + auto param = node.get(); ParamTypes[param] = type.getPointer(); } @@ -1757,26 +1759,18 @@ class ConstraintSystem { /// map is used throughout the expression type checker in order to /// avoid mutating expressions until we know we have successfully /// type-checked them. - void setType(Expr *E, Type T) { - setType(TypedNode(E), T); - } - void setType(TypeLoc &L, Type T) { setType(TypedNode(&L), T); } - void setType(ParamDecl *P, Type T) { - setType(TypedNode(P), T); - } - /// Erase the type for the given node. void eraseType(TypedNode node) { - if (auto expr = node.dyn_cast()) { + if (auto expr = node.dyn_cast()) { ExprTypes.erase(expr); - } else if (auto typeLoc = node.dyn_cast()) { + } else if (auto typeLoc = node.dyn_cast()) { TypeLocTypes.erase(typeLoc); } else { - auto param = node.get(); + auto param = node.get(); ParamTypes.erase(param); } } @@ -1788,12 +1782,20 @@ class ConstraintSystem { } bool hasType(const TypeLoc &L) const { - return TypeLocTypes.find(&L) != TypeLocTypes.end(); + return hasType(TypedNode(&L)); } - bool hasType(const ParamDecl *P) const { - assert(P != nullptr && "Expected non-null parameter!"); - return ParamTypes.find(P) != ParamTypes.end(); + /// Check to see if we have a type for a node. + bool hasType(TypedNode node) const { + assert(!node.isNull() && "Expected non-null node"); + if (auto expr = node.dyn_cast()) { + return ExprTypes.find(expr) != ExprTypes.end(); + } else if (auto typeLoc = node.dyn_cast()) { + return TypeLocTypes.find(typeLoc) != TypeLocTypes.end(); + } else { + auto param = node.get(); + return ParamTypes.find(param) != ParamTypes.end(); + } } /// Get the type for an expression. diff --git a/test/Constraints/function_builder_diags.swift b/test/Constraints/function_builder_diags.swift index 715dff972bf53..a629f54b687e5 100644 --- a/test/Constraints/function_builder_diags.swift +++ b/test/Constraints/function_builder_diags.swift @@ -117,5 +117,12 @@ func testOverloading(name: String) { _ = overloadedTuplify(true) { b in b ? "Hello, \(name)" : "Goodbye" 42 + overloadedTuplify(false) { + $0 ? "Hello, \(name)" : "Goodbye" + 42 + if b { + "Hello, \(name)" + } + } } } From 5c5127033fda5aa033ec3d39a449f833a6d19b0c Mon Sep 17 00:00:00 2001 From: Doug Gregor Date: Fri, 17 May 2019 14:06:28 -0700 Subject: [PATCH 34/38] [DSL] Clean up assignment of types to synthesized TypeExprs. The implicit TypeExprs for function builders were getting inconsistently set types, which would sometimes be the metatype and sometimes not be the metatype, leading to a crash in the new test code. --- lib/Sema/CSGen.cpp | 8 +++----- lib/Sema/ConstraintSystem.cpp | 6 +++++- test/Constraints/function_builder_diags.swift | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 3af43286a184d..d0a6f79ff39ec 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -1395,12 +1395,12 @@ namespace { Type type; // If this is an implicit TypeExpr, don't validate its contents. auto &typeLoc = E->getTypeLoc(); - if (E->isImplicit() && CS.hasType(E)) { - type = CS.getType(E); - } else if (typeLoc.wasValidated()) { + if (typeLoc.wasValidated()) { type = typeLoc.getType(); } else if (typeLoc.hasLocation()) { type = resolveTypeReferenceInExpression(typeLoc); + } else if (E->isImplicit() && CS.hasType(&typeLoc)) { + type = CS.getType(typeLoc); } if (!type || type->hasError()) return Type(); @@ -3515,8 +3515,6 @@ namespace { if (closureTy && closureTy->hasError()) return nullptr; - CS.setType(closure, closureTy); - // Visit the body. It's type needs to be convertible to the function's // return type. auto resultTy = closureTy->castTo()->getResult(); diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index e7cfc0e914433..2e7ba81f1a534 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2796,7 +2796,11 @@ namespace { builderType->hasTypeVariable() ? Type() : builderType); auto typeExpr = new (ctx) TypeExpr(typeLoc); - if (cs) cs->setType(typeExpr, builderType); + if (cs) { + cs->setType(typeExpr, MetatypeType::get(builderType)); + cs->setType(&typeExpr->getTypeLoc(), builderType); + } + typeExpr->setImplicit(); auto memberRef = new (ctx) UnresolvedDotExpr( typeExpr, loc, fnName, DeclNameLoc(loc), /*implicit=*/true); diff --git a/test/Constraints/function_builder_diags.swift b/test/Constraints/function_builder_diags.swift index a629f54b687e5..923849bf65c85 100644 --- a/test/Constraints/function_builder_diags.swift +++ b/test/Constraints/function_builder_diags.swift @@ -120,7 +120,7 @@ func testOverloading(name: String) { overloadedTuplify(false) { $0 ? "Hello, \(name)" : "Goodbye" 42 - if b { + if $0 { "Hello, \(name)" } } From 13fb516e2b50208b9378362d1cf30b0bb1b426d4 Mon Sep 17 00:00:00 2001 From: John McCall Date: Wed, 5 Jun 2019 11:58:37 -0700 Subject: [PATCH 35/38] Remove restriction that @functionBuilders have to be uppercased; we're not doing this anymore. --- include/swift/AST/DiagnosticsSema.def | 2 -- lib/Sema/TypeCheckAttr.cpp | 9 --------- 2 files changed, 11 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index e7b603cf1fad6..fec247889f80c 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -4456,8 +4456,6 @@ ERROR(function_builder_control_flow, none, NOTE(note_function_builder_control_flow, none, "closure containing control flow statement cannot be used with function " "builder %0", (DeclName)) -ERROR(function_builder_reserved_name, none, - "function builder type name must start with an uppercase letter", ()) ERROR(function_builder_attribute_not_allowed_here, none, "function builder attribute %0 can only be applied to a parameter, " "function, or computed property", (DeclName)) diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 2682f77415505..d55142c87f8eb 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -2644,15 +2644,6 @@ void AttributeChecker::visitPropertyWrapperAttr(PropertyWrapperAttr *attr) { } void AttributeChecker::visitFunctionBuilderAttr(FunctionBuilderAttr *attr) { - auto nominal = dyn_cast(D); - if (!nominal) - return; - - // Make sure the name isn't reserved. - if (isReservedAttributeName(nominal->getName().str())) { - nominal->diagnose(diag::function_builder_reserved_name); - } - // TODO: check that the type at least provides a `sequence` factory? // Any other validation? } From 8d2cc9f1e9f808ce6e0606ad1abd0bd03db2a367 Mon Sep 17 00:00:00 2001 From: John McCall Date: Thu, 6 Jun 2019 14:48:15 -0700 Subject: [PATCH 36/38] Merge fix: IBSegueAction apparently comes last in this list. --- test/IDE/complete_decl_attribute.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/IDE/complete_decl_attribute.swift b/test/IDE/complete_decl_attribute.swift index a54245094a788..32dec634f0a7c 100644 --- a/test/IDE/complete_decl_attribute.swift +++ b/test/IDE/complete_decl_attribute.swift @@ -237,8 +237,8 @@ struct _S { // KEYWORD_LAST-NEXT: Keyword/None: discardableResult[#Declaration Attribute#]; name=discardableResult // KEYWORD_LAST-NEXT: Keyword/None: GKInspectable[#Declaration Attribute#]; name=GKInspectable{{$}} // KEYWORD_LAST-NEXT: Keyword/None: _propertyWrapper[#Declaration Attribute#]; name=_propertyWrapper -// KEYWORD_LAST-NEXT: Keyword/None: IBSegueAction[#Declaration Attribute#]; name=IBSegueAction{{$}} // KEYWORD_LAST-NEXT: Keyword/None: _functionBuilder[#Declaration Attribute#]; name=_functionBuilder{{$}} +// KEYWORD_LAST-NEXT: Keyword/None: IBSegueAction[#Declaration Attribute#]; name=IBSegueAction{{$}} // KEYWORD_LAST-NOT: Keyword // KEYWORD_LAST: Decl[Struct]/CurrModule: MyStruct[#MyStruct#]; name=MyStruct // KEYWORD_LAST: End completions From 26756088ed1cdf42b344401e397efbd97ca5d0e5 Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 11 Jun 2019 18:58:11 -0700 Subject: [PATCH 37/38] Move the core builder-transform logic into its own file. --- lib/Sema/BuilderTransform.cpp | 597 ++++++++++++++++++++++++++++++++++ lib/Sema/CMakeLists.txt | 1 + lib/Sema/ConstraintSystem.cpp | 561 -------------------------------- 3 files changed, 598 insertions(+), 561 deletions(-) create mode 100644 lib/Sema/BuilderTransform.cpp diff --git a/lib/Sema/BuilderTransform.cpp b/lib/Sema/BuilderTransform.cpp new file mode 100644 index 0000000000000..5655dd6105771 --- /dev/null +++ b/lib/Sema/BuilderTransform.cpp @@ -0,0 +1,597 @@ +//===--- BuilderTransform.cpp - Function-builder transformation -----------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// +// +// This file implements routines associated with the function-builder +// transformation. +// +//===----------------------------------------------------------------------===// + +#include "ConstraintSystem.h" +#include "TypeChecker.h" +#include "swift/AST/ASTVisitor.h" +#include "swift/AST/ASTWalker.h" +#include "swift/AST/NameLookup.h" +#include "swift/AST/NameLookupRequests.h" +#include "swift/AST/ParameterList.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/SmallVector.h" +#include +#include +#include +#include +#include + +using namespace swift; +using namespace constraints; + +namespace { + +/// Visitor to classify the contents of the given closure. +class BuilderClosureVisitor + : public StmtVisitor { + ConstraintSystem *cs; + ASTContext &ctx; + bool wantExpr; + Type builderType; + NominalTypeDecl *builder = nullptr; + llvm::SmallDenseMap supportedOps; + +public: + SkipUnhandledConstructInFunctionBuilder::UnhandledNode unhandledNode; + +private: + /// Produce a builder call to the given named function with the given arguments. + CallExpr *buildCallIfWanted(SourceLoc loc, + Identifier fnName, ArrayRef args, + ArrayRef argLabels = {}) { + if (!wantExpr) + return nullptr; + + // FIXME: Setting a TypeLoc on this expression is necessary in order + // to get diagnostics if something about this builder call fails, + // e.g. if there isn't a matching overload for `buildBlock`. + // But we can only do this if there isn't a type variable in the type. + TypeLoc typeLoc(nullptr, + builderType->hasTypeVariable() ? Type() : builderType); + + auto typeExpr = new (ctx) TypeExpr(typeLoc); + if (cs) { + cs->setType(typeExpr, MetatypeType::get(builderType)); + cs->setType(&typeExpr->getTypeLoc(), builderType); + } + + typeExpr->setImplicit(); + auto memberRef = new (ctx) UnresolvedDotExpr( + typeExpr, loc, fnName, DeclNameLoc(loc), /*implicit=*/true); + return CallExpr::createImplicit(ctx, memberRef, args, argLabels); + } + + /// Check whether the builder supports the given operation. + bool builderSupports(Identifier fnName, + ArrayRef argLabels = {}) { + auto known = supportedOps.find(fnName); + if (known != supportedOps.end()) { + return known->second; + } + + bool found = false; + for (auto decl : builder->lookupDirect(fnName)) { + if (auto func = dyn_cast(decl)) { + // Function must be static. + if (!func->isStatic()) + continue; + + // Function must have the right argument labels, if provided. + if (!argLabels.empty()) { + auto funcLabels = func->getFullName().getArgumentNames(); + if (argLabels.size() > funcLabels.size() || + funcLabels.slice(0, argLabels.size()) != argLabels) + continue; + } + + // Okay, it's a good-enough match. + found = true; + break; + } + } + + return supportedOps[fnName] = found; + } + +public: + BuilderClosureVisitor(ASTContext &ctx, ConstraintSystem *cs, + bool wantExpr, Type builderType) + : cs(cs), ctx(ctx), wantExpr(wantExpr), builderType(builderType) { + assert((cs || !builderType->hasTypeVariable()) && + "cannot handle builder type with type variables without " + "constraint system"); + builder = builderType->getAnyNominal(); + } + +#define CONTROL_FLOW_STMT(StmtClass) \ + Expr *visit##StmtClass##Stmt(StmtClass##Stmt *stmt) { \ + if (!unhandledNode) \ + unhandledNode = stmt; \ + \ + return nullptr; \ + } + + Expr *visitBraceStmt(BraceStmt *braceStmt) { + SmallVector expressions; + for (const auto &node : braceStmt->getElements()) { + if (auto stmt = node.dyn_cast()) { + auto expr = visit(stmt); + if (expr) + expressions.push_back(expr); + continue; + } + + if (auto decl = node.dyn_cast()) { + if (!unhandledNode) + unhandledNode = decl; + + continue; + } + + auto expr = node.get(); + expressions.push_back(expr); + } + + // Call Builder.buildBlock(... args ...) + return buildCallIfWanted(braceStmt->getStartLoc(), + ctx.Id_buildBlock, expressions); + } + + Expr *visitReturnStmt(ReturnStmt *stmt) { + // Allow implicit returns due to 'return' elision. + if (!stmt->isImplicit() || !stmt->hasResult()) { + if (!unhandledNode) + unhandledNode = stmt; + return nullptr; + } + + return stmt->getResult(); + } + + Expr *visitDoStmt(DoStmt *doStmt) { + if (!builderSupports(ctx.Id_buildDo)) { + if (!unhandledNode) + unhandledNode = doStmt; + return nullptr; + } + + auto arg = visit(doStmt->getBody()); + if (!arg) + return nullptr; + + return buildCallIfWanted(doStmt->getStartLoc(), ctx.Id_buildDo, arg); + } + + CONTROL_FLOW_STMT(Yield) + CONTROL_FLOW_STMT(Defer) + + static Expr *getTrivialBooleanCondition(StmtCondition condition) { + if (condition.size() != 1) + return nullptr; + + return condition.front().getBooleanOrNull(); + } + + static bool isBuildableIfChainRecursive(IfStmt *ifStmt, + unsigned &numPayloads, + bool &isOptional) { + // The conditional must be trivial. + if (!getTrivialBooleanCondition(ifStmt->getCond())) + return false; + + // The 'then' clause contributes a payload. + numPayloads++; + + // If there's an 'else' clause, it contributes payloads: + if (auto elseStmt = ifStmt->getElseStmt()) { + // If it's 'else if', it contributes payloads recursively. + if (auto elseIfStmt = dyn_cast(elseStmt)) { + return isBuildableIfChainRecursive(elseIfStmt, numPayloads, + isOptional); + // Otherwise it's just the one. + } else { + numPayloads++; + } + + // If not, the chain result is at least optional. + } else { + isOptional = true; + } + + return true; + } + + bool isBuildableIfChain(IfStmt *ifStmt, unsigned &numPayloads, + bool &isOptional) { + if (!isBuildableIfChainRecursive(ifStmt, numPayloads, isOptional)) + return false; + + // If there's a missing 'else', we need 'buildIf' to exist. + if (isOptional && !builderSupports(ctx.Id_buildIf)) + return false; + + // If there are multiple clauses, we need 'buildEither(first:)' and + // 'buildEither(second:)' to both exist. + if (numPayloads > 1) { + if (!builderSupports(ctx.Id_buildEither, {ctx.Id_first}) || + !builderSupports(ctx.Id_buildEither, {ctx.Id_second})) + return false; + } + + return true; + } + + Expr *visitIfStmt(IfStmt *ifStmt) { + // Check whether the chain is buildable and whether it terminates + // without an `else`. + bool isOptional = false; + unsigned numPayloads = 0; + if (!isBuildableIfChain(ifStmt, numPayloads, isOptional)) { + if (!unhandledNode) + unhandledNode = ifStmt; + return nullptr; + } + + // Attempt to build the chain, propagating short-circuits, which + // might arise either do to error or not wanting an expression. + auto chainExpr = + buildIfChainRecursive(ifStmt, 0, numPayloads, isOptional); + if (!chainExpr) + return nullptr; + assert(wantExpr); + + // The operand should have optional type if we had optional results, + // so we just need to call `buildIf` now, since we're at the top level. + if (isOptional) { + chainExpr = buildCallIfWanted(ifStmt->getStartLoc(), + ctx.Id_buildIf, chainExpr); + } + + return chainExpr; + } + + /// Recursively build an if-chain: build an expression which will have + /// a value of the chain result type before any call to `buildIf`. + /// The expression will perform any necessary calls to `buildEither`, + /// and the result will have optional type if `isOptional` is true. + Expr *buildIfChainRecursive(IfStmt *ifStmt, unsigned payloadIndex, + unsigned numPayloads, bool isOptional) { + assert(payloadIndex < numPayloads); + // Make sure we recursively visit both sides even if we're not + // building expressions. + + // Build the then clause. This will have the corresponding payload + // type (i.e. not wrapped in any way). + Expr *thenArg = visit(ifStmt->getThenStmt()); + + // Build the else clause, if present. If this is from an else-if, + // this will be fully wrapped; otherwise it will have the corresponding + // payload type (at index `payloadIndex + 1`). + assert(ifStmt->getElseStmt() || isOptional); + bool isElseIf = false; + Optional elseChain; + if (auto elseStmt = ifStmt->getElseStmt()) { + if (auto elseIfStmt = dyn_cast(elseStmt)) { + isElseIf = true; + elseChain = buildIfChainRecursive(elseIfStmt, payloadIndex + 1, + numPayloads, isOptional); + } else { + elseChain = visit(elseStmt); + } + } + + // Short-circuit if appropriate. + if (!wantExpr || !thenArg || (elseChain && !*elseChain)) + return nullptr; + + // Okay, build the conditional expression. + + // Prepare the `then` operand by wrapping it to produce a chain result. + SourceLoc thenLoc = ifStmt->getThenStmt()->getStartLoc(); + Expr *thenExpr = buildWrappedChainPayload(thenArg, payloadIndex, + numPayloads, isOptional); + + // Prepare the `else operand: + Expr *elseExpr; + SourceLoc elseLoc; + + // - If there's no `else` clause, use `Optional.none`. + if (!elseChain) { + assert(isOptional); + elseLoc = ifStmt->getEndLoc(); + elseExpr = buildNoneExpr(elseLoc); + + // - If there's an `else if`, the chain expression from that + // should already be producing a chain result. + } else if (isElseIf) { + elseExpr = *elseChain; + elseLoc = ifStmt->getElseLoc(); + + // - Otherwise, wrap it to produce a chain result. + } else { + elseLoc = ifStmt->getElseLoc(); + elseExpr = buildWrappedChainPayload(*elseChain, + payloadIndex + 1, numPayloads, + isOptional); + } + + Expr *condition = getTrivialBooleanCondition(ifStmt->getCond()); + assert(condition && "checked by isBuildableIfChain"); + + auto ifExpr = new (ctx) IfExpr(condition, thenLoc, thenExpr, + elseLoc, elseExpr); + ifExpr->setImplicit(); + return ifExpr; + } + + /// Wrap a payload value in an expression which will produce a chain + /// result (without `buildIf`). + Expr *buildWrappedChainPayload(Expr *operand, unsigned payloadIndex, + unsigned numPayloads, bool isOptional) { + assert(payloadIndex < numPayloads); + + // Inject into the appropriate chain position. + // + // We produce a (left-biased) balanced binary tree of Eithers in order + // to prevent requiring a linear number of injections in the worst case. + // That is, if we have 13 clauses, we want to produce: + // + // /------------------Either------------\ + // /-------Either-------\ /--Either--\ + // /--Either--\ /--Either--\ /--Either--\ \ + // /-E-\ /-E-\ /-E-\ /-E-\ /-E-\ /-E-\ \ + // 0000 0001 0010 0011 0100 0101 0110 0111 1000 1001 1010 1011 1100 + // + // Note that a prefix of length D of the payload index acts as a path + // through the tree to the node at depth D. On the rightmost path + // through the tree (when this prefix is equal to the corresponding + // prefix of the maximum payload index), the bits of the index mark + // where Eithers are required. + // + // Since we naturally want to build from the innermost Either out, and + // therefore work with progressively shorter prefixes, we can do it all + // with right-shifts. + for (auto path = payloadIndex, maxPath = numPayloads - 1; + maxPath != 0; path >>= 1, maxPath >>= 1) { + // Skip making Eithers on the rightmost path where they aren't required. + // This isn't just an optimization: adding spurious Eithers could + // leave us with unresolvable type variables if `buildEither` has + // a signature like: + // static func buildEither(first value: T) -> Either + // which relies on unification to work. + if (path == maxPath && !(maxPath & 1)) continue; + + bool isSecond = (path & 1); + operand = buildCallIfWanted(operand->getStartLoc(), + ctx.Id_buildEither, operand, + {isSecond ? ctx.Id_second : ctx.Id_first}); + } + + // Inject into Optional if required. We'll be adding the call to + // `buildIf` after all the recursive calls are complete. + if (isOptional) { + operand = buildSomeExpr(operand); + } + + return operand; + } + + Expr *buildSomeExpr(Expr *arg) { + auto optionalDecl = ctx.getOptionalDecl(); + auto optionalType = optionalDecl->getDeclaredType(); + + auto optionalTypeExpr = TypeExpr::createImplicit(optionalType, ctx); + auto someRef = new (ctx) UnresolvedDotExpr( + optionalTypeExpr, SourceLoc(), ctx.getIdentifier("some"), + DeclNameLoc(), /*implicit=*/true); + return CallExpr::createImplicit(ctx, someRef, arg, { }); + } + + Expr *buildNoneExpr(SourceLoc endLoc) { + auto optionalDecl = ctx.getOptionalDecl(); + auto optionalType = optionalDecl->getDeclaredType(); + + auto optionalTypeExpr = TypeExpr::createImplicit(optionalType, ctx); + return new (ctx) UnresolvedDotExpr( + optionalTypeExpr, endLoc, ctx.getIdentifier("none"), + DeclNameLoc(endLoc), /*implicit=*/true); + } + + CONTROL_FLOW_STMT(Guard) + CONTROL_FLOW_STMT(While) + CONTROL_FLOW_STMT(DoCatch) + CONTROL_FLOW_STMT(RepeatWhile) + CONTROL_FLOW_STMT(ForEach) + CONTROL_FLOW_STMT(Switch) + CONTROL_FLOW_STMT(Case) + CONTROL_FLOW_STMT(Catch) + CONTROL_FLOW_STMT(Break) + CONTROL_FLOW_STMT(Continue) + CONTROL_FLOW_STMT(Fallthrough) + CONTROL_FLOW_STMT(Fail) + CONTROL_FLOW_STMT(Throw) + CONTROL_FLOW_STMT(PoundAssert) + +#undef CONTROL_FLOW_STMT +}; + +} // end anonymous namespace + +/// Determine whether the given statement contains a 'return' statement anywhere. +static bool hasReturnStmt(Stmt *stmt) { + class ReturnStmtFinder : public ASTWalker { + public: + bool hasReturnStmt = false; + + std::pair walkToExprPre(Expr *expr) override { + return { false, expr }; + } + + std::pair walkToStmtPre(Stmt *stmt) override { + // Did we find a 'return' statement? + if (isa(stmt)) { + hasReturnStmt = true; + } + + return { !hasReturnStmt, stmt }; + } + + Stmt *walkToStmtPost(Stmt *stmt) override { + return hasReturnStmt ? nullptr : stmt; + } + + std::pair walkToPatternPre(Pattern *pattern) override { + return { false, pattern }; + } + + bool walkToDeclPre(Decl *D) override { return false; } + + bool walkToTypeLocPre(TypeLoc &TL) override { return false; } + + bool walkToTypeReprPre(TypeRepr *T) override { return false; } + }; + + ReturnStmtFinder finder{}; + stmt->walk(finder); + return finder.hasReturnStmt; +} + +bool TypeChecker::typeCheckFunctionBuilderFuncBody(FuncDecl *FD, + Type builderType) { + // Try to build a single result expression. + BuilderClosureVisitor visitor(Context, nullptr, + /*wantExpr=*/true, builderType); + Expr *returnExpr = visitor.visit(FD->getBody()); + if (!returnExpr) + return true; + + // Make sure we have a usable result type for the body. + Type returnType = AnyFunctionRef(FD).getBodyResultType(); + if (!returnType || returnType->hasError()) + return true; + + // Type-check the single result expression. + Type returnExprType = typeCheckExpression(returnExpr, FD, + TypeLoc::withoutLoc(returnType), + CTP_ReturnStmt); + if (!returnExprType) + return true; + assert(returnExprType->isEqual(returnType)); + + auto returnStmt = new (Context) ReturnStmt(SourceLoc(), returnExpr); + auto origBody = FD->getBody(); + auto fakeBody = BraceStmt::create(Context, origBody->getLBraceLoc(), + { returnStmt }, + origBody->getRBraceLoc()); + FD->setBody(fakeBody); + return false; +} + +ConstraintSystem::TypeMatchResult ConstraintSystem::applyFunctionBuilder( + ClosureExpr *closure, Type builderType, ConstraintLocator *calleeLocator, + ConstraintLocatorBuilder locator) { + auto builder = builderType->getAnyNominal(); + assert(builder && "Bad function builder type"); + assert(builder->getAttrs().hasAttribute()); + + // Check the form of this closure to see if we can apply the function-builder + // translation at all. + { + // FIXME: Right now, single-expression closures suppress the function + // builder translation. + if (closure->hasSingleExpressionBody()) + return getTypeMatchSuccess(); + + // The presence of an explicit return suppresses the function builder + // translation. + if (hasReturnStmt(closure->getBody())) { + return getTypeMatchSuccess(); + } + + // Check whether we can apply this function builder. + BuilderClosureVisitor visitor(getASTContext(), this, + /*wantExpr=*/false, builderType); + (void)visitor.visit(closure->getBody()); + + + // If we saw a control-flow statement or declaration that the builder + // cannot handle, we don't have a well-formed function builder application. + if (visitor.unhandledNode) { + // If we aren't supposed to attempt fixes, fail. + if (!shouldAttemptFixes()) { + return getTypeMatchFailure(locator); + } + + // Record the first unhandled construct as a fix. + if (recordFix( + SkipUnhandledConstructInFunctionBuilder::create( + *this, visitor.unhandledNode, builder, + getConstraintLocator(locator)))) { + return getTypeMatchFailure(locator); + } + } + } + + // If the builder type has a type parameter, substitute in the type + // variables. + if (builderType->hasTypeParameter()) { + // Find the opened type for this callee and substitute in the type + // parametes. + for (const auto &opened : OpenedTypes) { + if (opened.first == calleeLocator) { + OpenedTypeMap replacements(opened.second.begin(), + opened.second.end()); + builderType = openType(builderType, replacements); + break; + } + } + assert(!builderType->hasTypeParameter()); + } + + BuilderClosureVisitor visitor(getASTContext(), this, + /*wantExpr=*/true, builderType); + Expr *singleExpr = visitor.visit(closure->getBody()); + + if (TC.precheckedClosures.insert(closure).second && + TC.preCheckExpression(singleExpr, closure)) + return getTypeMatchFailure(locator); + + singleExpr = generateConstraints(singleExpr, closure); + if (!singleExpr) + return getTypeMatchFailure(locator); + + Type transformedType = getType(singleExpr); + assert(transformedType && "Missing type"); + + // Record the transformation. + assert(std::find_if(builderTransformedClosures.begin(), + builderTransformedClosures.end(), + [&](const std::tuple &elt) { + return std::get<0>(elt) == closure; + }) == builderTransformedClosures.end() && + "already transformed this closure along this path!?!"); + builderTransformedClosures.push_back( + std::make_tuple(closure, builderType, singleExpr)); + + // Bind the result type of the closure to the type of the transformed + // expression. + Type closureType = getType(closure); + auto fnType = closureType->castTo(); + addConstraint(ConstraintKind::Equal, fnType->getResult(), transformedType, + locator); + return getTypeMatchSuccess(); +} diff --git a/lib/Sema/CMakeLists.txt b/lib/Sema/CMakeLists.txt index dfc3898ae7649..6dc488ba4eef1 100644 --- a/lib/Sema/CMakeLists.txt +++ b/lib/Sema/CMakeLists.txt @@ -4,6 +4,7 @@ if (SWIFT_FORCE_OPTIMIZED_TYPECHECKER) endif() add_swift_host_library(swiftSema STATIC + BuilderTransform.cpp CSApply.cpp CSBindings.cpp CSDiag.cpp diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index 2e7ba81f1a534..3426374370bea 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -2765,564 +2765,3 @@ bool constraints::isKnownKeyPathDecl(ASTContext &ctx, ValueDecl *decl) { decl == ctx.getReferenceWritableKeyPathDecl() || decl == ctx.getPartialKeyPathDecl() || decl == ctx.getAnyKeyPathDecl(); } - -namespace { - /// Visitor to classify the contents of the given closure. - class BuilderClosureVisitor - : public StmtVisitor { - ConstraintSystem *cs; - ASTContext &ctx; - bool wantExpr; - Type builderType; - NominalTypeDecl *builder = nullptr; - llvm::SmallDenseMap supportedOps; - - public: - SkipUnhandledConstructInFunctionBuilder::UnhandledNode unhandledNode; - - private: - /// Produce a builder call to the given named function with the given arguments. - CallExpr *buildCallIfWanted(SourceLoc loc, - Identifier fnName, ArrayRef args, - ArrayRef argLabels = {}) { - if (!wantExpr) - return nullptr; - - // FIXME: Setting a TypeLoc on this expression is necessary in order - // to get diagnostics if something about this builder call fails, - // e.g. if there isn't a matching overload for `buildBlock`. - // But we can only do this if there isn't a type variable in the type. - TypeLoc typeLoc(nullptr, - builderType->hasTypeVariable() ? Type() : builderType); - - auto typeExpr = new (ctx) TypeExpr(typeLoc); - if (cs) { - cs->setType(typeExpr, MetatypeType::get(builderType)); - cs->setType(&typeExpr->getTypeLoc(), builderType); - } - - typeExpr->setImplicit(); - auto memberRef = new (ctx) UnresolvedDotExpr( - typeExpr, loc, fnName, DeclNameLoc(loc), /*implicit=*/true); - return CallExpr::createImplicit(ctx, memberRef, args, argLabels); - } - - /// Check whether the builder supports the given operation. - bool builderSupports(Identifier fnName, - ArrayRef argLabels = {}) { - auto known = supportedOps.find(fnName); - if (known != supportedOps.end()) { - return known->second; - } - - bool found = false; - for (auto decl : builder->lookupDirect(fnName)) { - if (auto func = dyn_cast(decl)) { - // Function must be static. - if (!func->isStatic()) - continue; - - // Function must have the right argument labels, if provided. - if (!argLabels.empty()) { - auto funcLabels = func->getFullName().getArgumentNames(); - if (argLabels.size() > funcLabels.size() || - funcLabels.slice(0, argLabels.size()) != argLabels) - continue; - } - - // Okay, it's a good-enough match. - found = true; - break; - } - } - - return supportedOps[fnName] = found; - } - - public: - BuilderClosureVisitor(ASTContext &ctx, ConstraintSystem *cs, - bool wantExpr, Type builderType) - : cs(cs), ctx(ctx), wantExpr(wantExpr), builderType(builderType) { - assert((cs || !builderType->hasTypeVariable()) && - "cannot handle builder type with type variables without " - "constraint system"); - builder = builderType->getAnyNominal(); - } - -#define CONTROL_FLOW_STMT(StmtClass) \ - Expr *visit##StmtClass##Stmt(StmtClass##Stmt *stmt) { \ - if (!unhandledNode) \ - unhandledNode = stmt; \ - \ - return nullptr; \ - } - - Expr *visitBraceStmt(BraceStmt *braceStmt) { - SmallVector expressions; - for (const auto &node : braceStmt->getElements()) { - if (auto stmt = node.dyn_cast()) { - auto expr = visit(stmt); - if (expr) - expressions.push_back(expr); - continue; - } - - if (auto decl = node.dyn_cast()) { - if (!unhandledNode) - unhandledNode = decl; - - continue; - } - - auto expr = node.get(); - expressions.push_back(expr); - } - - // Call Builder.buildBlock(... args ...) - return buildCallIfWanted(braceStmt->getStartLoc(), - ctx.Id_buildBlock, expressions); - } - - Expr *visitReturnStmt(ReturnStmt *stmt) { - // Allow implicit returns due to 'return' elision. - if (!stmt->isImplicit() || !stmt->hasResult()) { - if (!unhandledNode) - unhandledNode = stmt; - return nullptr; - } - - return stmt->getResult(); - } - - Expr *visitDoStmt(DoStmt *doStmt) { - if (!builderSupports(ctx.Id_buildDo)) { - if (!unhandledNode) - unhandledNode = doStmt; - return nullptr; - } - - auto arg = visit(doStmt->getBody()); - if (!arg) - return nullptr; - - return buildCallIfWanted(doStmt->getStartLoc(), ctx.Id_buildDo, arg); - } - - CONTROL_FLOW_STMT(Yield) - CONTROL_FLOW_STMT(Defer) - - static Expr *getTrivialBooleanCondition(StmtCondition condition) { - if (condition.size() != 1) - return nullptr; - - return condition.front().getBooleanOrNull(); - } - - static bool isBuildableIfChainRecursive(IfStmt *ifStmt, - unsigned &numPayloads, - bool &isOptional) { - // The conditional must be trivial. - if (!getTrivialBooleanCondition(ifStmt->getCond())) - return false; - - // The 'then' clause contributes a payload. - numPayloads++; - - // If there's an 'else' clause, it contributes payloads: - if (auto elseStmt = ifStmt->getElseStmt()) { - // If it's 'else if', it contributes payloads recursively. - if (auto elseIfStmt = dyn_cast(elseStmt)) { - return isBuildableIfChainRecursive(elseIfStmt, numPayloads, - isOptional); - // Otherwise it's just the one. - } else { - numPayloads++; - } - - // If not, the chain result is at least optional. - } else { - isOptional = true; - } - - return true; - } - - bool isBuildableIfChain(IfStmt *ifStmt, unsigned &numPayloads, - bool &isOptional) { - if (!isBuildableIfChainRecursive(ifStmt, numPayloads, isOptional)) - return false; - - // If there's a missing 'else', we need 'buildIf' to exist. - if (isOptional && !builderSupports(ctx.Id_buildIf)) - return false; - - // If there are multiple clauses, we need 'buildEither(first:)' and - // 'buildEither(second:)' to both exist. - if (numPayloads > 1) { - if (!builderSupports(ctx.Id_buildEither, {ctx.Id_first}) || - !builderSupports(ctx.Id_buildEither, {ctx.Id_second})) - return false; - } - - return true; - } - - Expr *visitIfStmt(IfStmt *ifStmt) { - // Check whether the chain is buildable and whether it terminates - // without an `else`. - bool isOptional = false; - unsigned numPayloads = 0; - if (!isBuildableIfChain(ifStmt, numPayloads, isOptional)) { - if (!unhandledNode) - unhandledNode = ifStmt; - return nullptr; - } - - // Attempt to build the chain, propagating short-circuits, which - // might arise either do to error or not wanting an expression. - auto chainExpr = - buildIfChainRecursive(ifStmt, 0, numPayloads, isOptional); - if (!chainExpr) - return nullptr; - assert(wantExpr); - - // The operand should have optional type if we had optional results, - // so we just need to call `buildIf` now, since we're at the top level. - if (isOptional) { - chainExpr = buildCallIfWanted(ifStmt->getStartLoc(), - ctx.Id_buildIf, chainExpr); - } - - return chainExpr; - } - - /// Recursively build an if-chain: build an expression which will have - /// a value of the chain result type before any call to `buildIf`. - /// The expression will perform any necessary calls to `buildEither`, - /// and the result will have optional type if `isOptional` is true. - Expr *buildIfChainRecursive(IfStmt *ifStmt, unsigned payloadIndex, - unsigned numPayloads, bool isOptional) { - assert(payloadIndex < numPayloads); - // Make sure we recursively visit both sides even if we're not - // building expressions. - - // Build the then clause. This will have the corresponding payload - // type (i.e. not wrapped in any way). - Expr *thenArg = visit(ifStmt->getThenStmt()); - - // Build the else clause, if present. If this is from an else-if, - // this will be fully wrapped; otherwise it will have the corresponding - // payload type (at index `payloadIndex + 1`). - assert(ifStmt->getElseStmt() || isOptional); - bool isElseIf = false; - Optional elseChain; - if (auto elseStmt = ifStmt->getElseStmt()) { - if (auto elseIfStmt = dyn_cast(elseStmt)) { - isElseIf = true; - elseChain = buildIfChainRecursive(elseIfStmt, payloadIndex + 1, - numPayloads, isOptional); - } else { - elseChain = visit(elseStmt); - } - } - - // Short-circuit if appropriate. - if (!wantExpr || !thenArg || (elseChain && !*elseChain)) - return nullptr; - - // Okay, build the conditional expression. - - // Prepare the `then` operand by wrapping it to produce a chain result. - SourceLoc thenLoc = ifStmt->getThenStmt()->getStartLoc(); - Expr *thenExpr = buildWrappedChainPayload(thenArg, payloadIndex, - numPayloads, isOptional); - - // Prepare the `else operand: - Expr *elseExpr; - SourceLoc elseLoc; - - // - If there's no `else` clause, use `Optional.none`. - if (!elseChain) { - assert(isOptional); - elseLoc = ifStmt->getEndLoc(); - elseExpr = buildNoneExpr(elseLoc); - - // - If there's an `else if`, the chain expression from that - // should already be producing a chain result. - } else if (isElseIf) { - elseExpr = *elseChain; - elseLoc = ifStmt->getElseLoc(); - - // - Otherwise, wrap it to produce a chain result. - } else { - elseLoc = ifStmt->getElseLoc(); - elseExpr = buildWrappedChainPayload(*elseChain, - payloadIndex + 1, numPayloads, - isOptional); - } - - Expr *condition = getTrivialBooleanCondition(ifStmt->getCond()); - assert(condition && "checked by isBuildableIfChain"); - - auto ifExpr = new (ctx) IfExpr(condition, thenLoc, thenExpr, - elseLoc, elseExpr); - ifExpr->setImplicit(); - return ifExpr; - } - - /// Wrap a payload value in an expression which will produce a chain - /// result (without `buildIf`). - Expr *buildWrappedChainPayload(Expr *operand, unsigned payloadIndex, - unsigned numPayloads, bool isOptional) { - assert(payloadIndex < numPayloads); - - // Inject into the appropriate chain position. - // - // We produce a (left-biased) balanced binary tree of Eithers in order - // to prevent requiring a linear number of injections in the worst case. - // That is, if we have 13 clauses, we want to produce: - // - // /------------------Either------------\ - // /-------Either-------\ /--Either--\ - // /--Either--\ /--Either--\ /--Either--\ \ - // /-E-\ /-E-\ /-E-\ /-E-\ /-E-\ /-E-\ \ - // 0000 0001 0010 0011 0100 0101 0110 0111 1000 1001 1010 1011 1100 - // - // Note that a prefix of length D of the payload index acts as a path - // through the tree to the node at depth D. On the rightmost path - // through the tree (when this prefix is equal to the corresponding - // prefix of the maximum payload index), the bits of the index mark - // where Eithers are required. - // - // Since we naturally want to build from the innermost Either out, and - // therefore work with progressively shorter prefixes, we can do it all - // with right-shifts. - for (auto path = payloadIndex, maxPath = numPayloads - 1; - maxPath != 0; path >>= 1, maxPath >>= 1) { - // Skip making Eithers on the rightmost path where they aren't required. - // This isn't just an optimization: adding spurious Eithers could - // leave us with unresolvable type variables if `buildEither` has - // a signature like: - // static func buildEither(first value: T) -> Either - // which relies on unification to work. - if (path == maxPath && !(maxPath & 1)) continue; - - bool isSecond = (path & 1); - operand = buildCallIfWanted(operand->getStartLoc(), - ctx.Id_buildEither, operand, - {isSecond ? ctx.Id_second : ctx.Id_first}); - } - - // Inject into Optional if required. We'll be adding the call to - // `buildIf` after all the recursive calls are complete. - if (isOptional) { - operand = buildSomeExpr(operand); - } - - return operand; - } - - Expr *buildSomeExpr(Expr *arg) { - auto optionalDecl = ctx.getOptionalDecl(); - auto optionalType = optionalDecl->getDeclaredType(); - - auto optionalTypeExpr = TypeExpr::createImplicit(optionalType, ctx); - auto someRef = new (ctx) UnresolvedDotExpr( - optionalTypeExpr, SourceLoc(), ctx.getIdentifier("some"), - DeclNameLoc(), /*implicit=*/true); - return CallExpr::createImplicit(ctx, someRef, arg, { }); - } - - Expr *buildNoneExpr(SourceLoc endLoc) { - auto optionalDecl = ctx.getOptionalDecl(); - auto optionalType = optionalDecl->getDeclaredType(); - - auto optionalTypeExpr = TypeExpr::createImplicit(optionalType, ctx); - return new (ctx) UnresolvedDotExpr( - optionalTypeExpr, endLoc, ctx.getIdentifier("none"), - DeclNameLoc(endLoc), /*implicit=*/true); - } - - CONTROL_FLOW_STMT(Guard) - CONTROL_FLOW_STMT(While) - CONTROL_FLOW_STMT(DoCatch) - CONTROL_FLOW_STMT(RepeatWhile) - CONTROL_FLOW_STMT(ForEach) - CONTROL_FLOW_STMT(Switch) - CONTROL_FLOW_STMT(Case) - CONTROL_FLOW_STMT(Catch) - CONTROL_FLOW_STMT(Break) - CONTROL_FLOW_STMT(Continue) - CONTROL_FLOW_STMT(Fallthrough) - CONTROL_FLOW_STMT(Fail) - CONTROL_FLOW_STMT(Throw) - CONTROL_FLOW_STMT(PoundAssert) - -#undef CONTROL_FLOW_STMT - }; -} - -/// Determine whether the given statement contains a 'return' statement anywhere. -static bool hasReturnStmt(Stmt *stmt) { - class ReturnStmtFinder : public ASTWalker { - public: - bool hasReturnStmt = false; - - std::pair walkToExprPre(Expr *expr) override { - return { false, expr }; - } - - std::pair walkToStmtPre(Stmt *stmt) override { - // Did we find a 'return' statement? - if (isa(stmt)) { - hasReturnStmt = true; - } - - return { !hasReturnStmt, stmt }; - } - - Stmt *walkToStmtPost(Stmt *stmt) override { - return hasReturnStmt ? nullptr : stmt; - } - - std::pair walkToPatternPre(Pattern *pattern) override { - return { false, pattern }; - } - - bool walkToDeclPre(Decl *D) override { return false; } - - bool walkToTypeLocPre(TypeLoc &TL) override { return false; } - - bool walkToTypeReprPre(TypeRepr *T) override { return false; } - }; - - ReturnStmtFinder finder{}; - stmt->walk(finder); - return finder.hasReturnStmt; -} - -bool TypeChecker::typeCheckFunctionBuilderFuncBody(FuncDecl *FD, - Type builderType) { - // Try to build a single result expression. - BuilderClosureVisitor visitor(Context, nullptr, - /*wantExpr=*/true, builderType); - Expr *returnExpr = visitor.visit(FD->getBody()); - if (!returnExpr) - return true; - - // Make sure we have a usable result type for the body. - Type returnType = AnyFunctionRef(FD).getBodyResultType(); - if (!returnType || returnType->hasError()) - return true; - - // Type-check the single result expression. - Type returnExprType = typeCheckExpression(returnExpr, FD, - TypeLoc::withoutLoc(returnType), - CTP_ReturnStmt); - if (!returnExprType) - return true; - assert(returnExprType->isEqual(returnType)); - - auto returnStmt = new (Context) ReturnStmt(SourceLoc(), returnExpr); - auto origBody = FD->getBody(); - auto fakeBody = BraceStmt::create(Context, origBody->getLBraceLoc(), - { returnStmt }, - origBody->getRBraceLoc()); - FD->setBody(fakeBody); - return false; -} - -ConstraintSystem::TypeMatchResult ConstraintSystem::applyFunctionBuilder( - ClosureExpr *closure, Type builderType, ConstraintLocator *calleeLocator, - ConstraintLocatorBuilder locator) { - auto builder = builderType->getAnyNominal(); - assert(builder && "Bad function builder type"); - assert(builder->getAttrs().hasAttribute()); - - // Check the form of this closure to see if we can apply the function-builder - // translation at all. - { - // FIXME: Right now, single-expression closures suppress the function - // builder translation. - if (closure->hasSingleExpressionBody()) - return getTypeMatchSuccess(); - - // The presence of an explicit return suppresses the function builder - // translation. - if (hasReturnStmt(closure->getBody())) { - return getTypeMatchSuccess(); - } - - // Check whether we can apply this function builder. - BuilderClosureVisitor visitor(getASTContext(), this, - /*wantExpr=*/false, builderType); - (void)visitor.visit(closure->getBody()); - - - // If we saw a control-flow statement or declaration that the builder - // cannot handle, we don't have a well-formed function builder application. - if (visitor.unhandledNode) { - // If we aren't supposed to attempt fixes, fail. - if (!shouldAttemptFixes()) { - return getTypeMatchFailure(locator); - } - - // Record the first unhandled construct as a fix. - if (recordFix( - SkipUnhandledConstructInFunctionBuilder::create( - *this, visitor.unhandledNode, builder, - getConstraintLocator(locator)))) { - return getTypeMatchFailure(locator); - } - } - } - - // If the builder type has a type parameter, substitute in the type - // variables. - if (builderType->hasTypeParameter()) { - // Find the opened type for this callee and substitute in the type - // parametes. - for (const auto &opened : OpenedTypes) { - if (opened.first == calleeLocator) { - OpenedTypeMap replacements(opened.second.begin(), - opened.second.end()); - builderType = openType(builderType, replacements); - break; - } - } - assert(!builderType->hasTypeParameter()); - } - - BuilderClosureVisitor visitor(getASTContext(), this, - /*wantExpr=*/true, builderType); - Expr *singleExpr = visitor.visit(closure->getBody()); - - if (TC.precheckedClosures.insert(closure).second && - TC.preCheckExpression(singleExpr, closure)) - return getTypeMatchFailure(locator); - - singleExpr = generateConstraints(singleExpr, closure); - if (!singleExpr) - return getTypeMatchFailure(locator); - - Type transformedType = getType(singleExpr); - assert(transformedType && "Missing type"); - - // Record the transformation. - assert(std::find_if(builderTransformedClosures.begin(), - builderTransformedClosures.end(), - [&](const std::tuple &elt) { - return std::get<0>(elt) == closure; - }) == builderTransformedClosures.end() && - "already transformed this closure along this path!?!"); - builderTransformedClosures.push_back( - std::make_tuple(closure, builderType, singleExpr)); - - // Bind the result type of the closure to the type of the transformed - // expression. - Type closureType = getType(closure); - auto fnType = closureType->castTo(); - addConstraint(ConstraintKind::Equal, fnType->getResult(), transformedType, - locator); - return getTypeMatchSuccess(); -} From 9c8be4c1f6d4b1ddea298dc804241f0868b23439 Mon Sep 17 00:00:00 2001 From: John McCall Date: Wed, 12 Jun 2019 11:34:27 -0700 Subject: [PATCH 38/38] Fix cherry-pick to 5.1. --- include/swift/AST/TypeCheckRequests.h | 6 +++++- lib/AST/TypeCheckRequests.cpp | 27 --------------------------- lib/Sema/TypeCheckAttr.cpp | 6 ------ 3 files changed, 5 insertions(+), 34 deletions(-) diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index 07d6148cd6947..82176997f4c00 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -576,7 +576,11 @@ class FunctionBuilderTypeRequest : // Allow AnyValue to compare two Type values, even though Type doesn't // support ==. template<> -bool AnyValue::Holder::equals(const HolderBase &other) const; +inline bool AnyValue::Holder::equals(const HolderBase &other) const { + assert(typeID == other.typeID && "Caller should match type IDs"); + return value.getPointer() == + static_cast &>(other).value.getPointer(); +} void simple_display(llvm::raw_ostream &out, Type value); diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp index 547d969891575..daf96becb1c19 100644 --- a/lib/AST/TypeCheckRequests.cpp +++ b/lib/AST/TypeCheckRequests.cpp @@ -620,33 +620,6 @@ void swift::simple_display( out << " }"; } -//----------------------------------------------------------------------------// -// CustomAttrTypeRequest. -//----------------------------------------------------------------------------// - -void CustomAttrTypeRequest::diagnoseCycle(DiagnosticEngine &diags) const { - auto attr = std::get<0>(getStorage()); - diags.diagnose(attr->getLocation(), diag::circular_reference); -} - -void CustomAttrTypeRequest::noteCycleStep(DiagnosticEngine &diags) const { - auto attr = std::get<0>(getStorage()); - diags.diagnose(attr->getLocation(), diag::circular_reference_through); -} - -void swift::simple_display(llvm::raw_ostream &out, CustomAttrTypeKind value) { - switch (value) { - case CustomAttrTypeKind::NonGeneric: - out << "non-generic"; - return; - - case CustomAttrTypeKind::PropertyDelegate: - out << "property-delegate"; - return; - } - llvm_unreachable("bad kind"); -} - //----------------------------------------------------------------------------// // FunctionBuilder-related requests. //----------------------------------------------------------------------------// diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index d55142c87f8eb..3fda19ba0ce0c 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -2518,12 +2518,6 @@ void AttributeChecker::visitNonOverrideAttr(NonOverrideAttr *attr) { } -void TypeChecker::checkParameterAttributes(ParameterList *params) { - for (auto param: *params) { - checkDeclAttributes(param); - } -} - void AttributeChecker::visitCustomAttr(CustomAttr *attr) { auto dc = D->getInnermostDeclContext();