Skip to content

Sema: Three fixes for the new @escaping attribute (3.0) #4388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 19, 2016
Merged

Sema: Three fixes for the new @escaping attribute (3.0) #4388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 105 additions & 46 deletions lib/Sema/TypeCheckType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1402,6 +1402,38 @@ static bool diagnoseAvailability(Type ty, IdentTypeRepr *IdType, SourceLoc Loc,
return false;
}

/// Whether the given DC is a noescape-by-default context, i.e. not a property
/// setter
static bool isDefaultNoEscapeContext(const DeclContext *DC) {
auto funcDecl = dyn_cast<FuncDecl>(DC);
return !funcDecl || !funcDecl->isSetter();
Copy link
Contributor

@jrose-apple jrose-apple Aug 19, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

As noted on the other PR, what about subscript setter indices?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean if a subscript is defined with a function type as its index? Wow. What is the intended behavior here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I hope no one does it, but if they do do it I would expect to be like function parameters: non-escaping by default and permitted to be marked escaping.

}

// Hack to apply context-specific @escaping to an AST function type.
static Type adjustFunctionExtInfo(DeclContext *DC,
Copy link
Member

Choose a reason for hiding this comment

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

More specific name here, that is, adjust it to do what?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to applyNonEscapingFromContext().

Type ty,
TypeResolutionOptions options) {
// Remember whether this is a function parameter.
bool isFunctionParam =
options.contains(TR_FunctionInput) ||
options.contains(TR_ImmediateFunctionInput);

bool defaultNoEscape = isFunctionParam && isDefaultNoEscapeContext(DC);

// Desugar here
auto *funcTy = ty->castTo<FunctionType>();
auto extInfo = funcTy->getExtInfo();
if (defaultNoEscape && !extInfo.isNoEscape()) {
extInfo = extInfo.withNoEscape();

// We lost the sugar to flip the isNoEscape bit
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you mark this as a FIXME, to introduce something like AttributedType later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

return FunctionType::get(funcTy->getInput(), funcTy->getResult(), extInfo);
}

// Note: original sugared type
return ty;
}

/// \brief Returns a valid type or ErrorType in case of an error.
Type TypeChecker::resolveIdentifierType(
DeclContext *DC,
Expand Down Expand Up @@ -1432,6 +1464,11 @@ Type TypeChecker::resolveIdentifierType(
return ErrorType::get(Context);
}

// Hack to apply context-specific @escaping to a typealias with an underlying
// function type.
if (result->is<FunctionType>())
result = adjustFunctionExtInfo(DC, result, options);

// We allow a type to conform to a protocol that is less available than
// the type itself. This enables a type to retroactively model or directly
// conform to a protocol only available on newer OSes and yet still be used on
Expand Down Expand Up @@ -1637,29 +1674,20 @@ Type TypeChecker::resolveType(TypeRepr *TyR, DeclContext *DC,
return result;
}

/// Whether the given DC is a noescape-by-default context, i.e. not a property
/// setter
static bool isDefaultNoEscapeContext(const DeclContext *DC) {
auto funcDecl = dyn_cast<FuncDecl>(DC);
return !funcDecl || !funcDecl->isSetter();
}

Type TypeResolver::resolveType(TypeRepr *repr, TypeResolutionOptions options) {
assert(repr && "Cannot validate null TypeReprs!");

// If we know the type representation is invalid, just return an
// error type.
if (repr->isInvalid()) return ErrorType::get(TC.Context);

// Remember whether this is a function parameter.
bool isFunctionParam =
options.contains(TR_FunctionInput) ||
options.contains(TR_ImmediateFunctionInput);

// Strip the "is function input" bits unless this is a type that knows about
// them.
if (!isa<InOutTypeRepr>(repr) && !isa<TupleTypeRepr>(repr) &&
!isa<AttributedTypeRepr>(repr)) {
if (!isa<InOutTypeRepr>(repr) &&
!isa<TupleTypeRepr>(repr) &&
!isa<AttributedTypeRepr>(repr) &&
!isa<FunctionTypeRepr>(repr) &&
!isa<IdentTypeRepr>(repr)) {
options -= TR_ImmediateFunctionInput;
options -= TR_FunctionInput;
}
Expand All @@ -1686,11 +1714,10 @@ Type TypeResolver::resolveType(TypeRepr *repr, TypeResolutionOptions options) {
case TypeReprKind::Function:
if (!(options & TR_SILType)) {
// Default non-escaping for closure parameters
auto info = AnyFunctionType::ExtInfo().withNoEscape(
isFunctionParam &&
isDefaultNoEscapeContext(DC));
return resolveASTFunctionType(cast<FunctionTypeRepr>(repr), options,
info);
auto result = resolveASTFunctionType(cast<FunctionTypeRepr>(repr), options);
if (result && result->is<FunctionType>())
return adjustFunctionExtInfo(DC, result, options);
return result;
}
return resolveSILFunctionType(cast<FunctionTypeRepr>(repr), options);

Expand Down Expand Up @@ -1747,8 +1774,6 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
bool isFunctionParam =
options.contains(TR_FunctionInput) ||
options.contains(TR_ImmediateFunctionInput);
options -= TR_ImmediateFunctionInput;
options -= TR_FunctionInput;

// The type we're working with, in case we want to build it differently
// based on the attributes we see.
Expand All @@ -1772,7 +1797,11 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
if (base) {
Optional<MetatypeRepresentation> storedRepr;
// The instance type is not a SIL type.
auto instanceOptions = options - TR_SILType;
auto instanceOptions = options;
instanceOptions -= TR_SILType;
instanceOptions -= TR_ImmediateFunctionInput;
instanceOptions -= TR_FunctionInput;

auto instanceTy = resolveType(base, instanceOptions);
if (!instanceTy || instanceTy->is<ErrorType>())
return instanceTy;
Expand Down Expand Up @@ -1901,10 +1930,6 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,

ty = resolveSILFunctionType(fnRepr, options, extInfo, calleeConvention);
if (!ty || ty->is<ErrorType>()) return ty;

for (auto i : FunctionAttrs)
attrs.clearAttribute(i);
attrs.convention = None;
} else if (hasFunctionAttr && fnRepr) {

FunctionType::Representation rep = FunctionType::Representation::Swift;
Expand Down Expand Up @@ -1948,29 +1973,54 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
.fixItReplace(resultRange, "Never");
}

bool defaultNoEscape = false;
if (isFunctionParam && !attrs.has(TAK_escaping)) {
defaultNoEscape = isDefaultNoEscapeContext(DC);
}

if (isFunctionParam && attrs.has(TAK_noescape) &&
isDefaultNoEscapeContext(DC)) {
if (attrs.has(TAK_noescape)) {
// FIXME: diagnostic to tell user this is redundant and drop it
Copy link
Member

Choose a reason for hiding this comment

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

FIXME is no longer relevant, as we actually detect this fact in the parser.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

}

// Resolve the function type directly with these attributes.
FunctionType::ExtInfo extInfo(rep,
attrs.has(TAK_autoclosure),
defaultNoEscape | attrs.has(TAK_noescape),
attrs.has(TAK_noescape),
fnRepr->throws());

ty = resolveASTFunctionType(fnRepr, options, extInfo);
if (!ty || ty->is<ErrorType>()) return ty;
}

for (auto i : FunctionAttrs)
attrs.clearAttribute(i);
attrs.convention = None;
} else if (hasFunctionAttr) {
auto instanceOptions = options;
instanceOptions -= TR_ImmediateFunctionInput;
instanceOptions -= TR_FunctionInput;

// If we didn't build the type differently above, we might have
// a typealias pointing at a function type with the @escaping
// attribute. Resolve the type as if it were in non-parameter
// context, and then set isNoEscape if @escaping is not present.
if (!ty) ty = resolveType(repr, instanceOptions);
if (!ty || ty->is<ErrorType>()) return ty;

// Handle @escaping
if (hasFunctionAttr && ty->is<FunctionType>()) {
if (attrs.has(TAK_escaping)) {
// The attribute is meaningless except on parameter types.
if (!isFunctionParam) {
auto &SM = TC.Context.SourceMgr;
auto loc = attrs.getLoc(TAK_escaping);
auto attrRange = SourceRange(
loc.getAdvancedLoc(-1),
Lexer::getLocForEndOfToken(SM, loc));

TC.diagnose(loc, diag::escaping_function_type)
.fixItRemove(attrRange);
}

attrs.clearAttribute(TAK_escaping);
} else {
// No attribute; set the isNoEscape bit if we're in parameter context.
ty = adjustFunctionExtInfo(DC, ty, options);
}
}

if (hasFunctionAttr && !fnRepr) {
// @autoclosure usually auto-implies @noescape, don't complain about both
// of them.
if (attrs.has(TAK_autoclosure))
Expand All @@ -1983,11 +2033,11 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
attrs.clearAttribute(i);
}
}
}

// If we didn't build the type differently above, build it normally now.
if (!ty) ty = resolveType(repr, options);
if (!ty || ty->is<ErrorType>()) return ty;
} else if (hasFunctionAttr && fnRepr) {
for (auto i : FunctionAttrs)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this part doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was there before, I think it's just clearing the attributes so that we don't diagnose about them later.

attrs.clearAttribute(i);
attrs.convention = None;
}

// In SIL, handle @opened (n), which creates an existential archetype.
if (attrs.has(TAK_opened)) {
Expand Down Expand Up @@ -2036,6 +2086,9 @@ Type TypeResolver::resolveAttributedType(TypeAttributes &attrs,
Type TypeResolver::resolveASTFunctionType(FunctionTypeRepr *repr,
TypeResolutionOptions options,
FunctionType::ExtInfo extInfo) {
options -= TR_ImmediateFunctionInput;
options -= TR_FunctionInput;

Type inputTy = resolveType(repr->getArgsTypeRepr(),
options | TR_ImmediateFunctionInput);
if (!inputTy || inputTy->is<ErrorType>()) return inputTy;
Expand Down Expand Up @@ -2100,6 +2153,9 @@ Type TypeResolver::resolveSILFunctionType(FunctionTypeRepr *repr,
TypeResolutionOptions options,
SILFunctionType::ExtInfo extInfo,
ParameterConvention callee) {
options -= TR_ImmediateFunctionInput;
options -= TR_FunctionInput;

bool hasError = false;

SmallVector<SILParameterInfo, 4> params;
Expand Down Expand Up @@ -2428,9 +2484,12 @@ Type TypeResolver::resolveTupleType(TupleTypeRepr *repr,

// If this is the top level of a function input list, peel off the
// ImmediateFunctionInput marker and install a FunctionInput one instead.
auto elementOptions = withoutContext(options);
if (options & TR_ImmediateFunctionInput)
elementOptions |= TR_FunctionInput;
auto elementOptions = options;
if (!repr->isParenType()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain this part?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

elementOptions = withoutContext(elementOptions);
if (options & TR_ImmediateFunctionInput)
elementOptions |= TR_FunctionInput;
}

for (auto tyR : repr->getElements()) {
NamedTypeRepr *namedTyR = dyn_cast<NamedTypeRepr>(tyR);
Expand Down
10 changes: 5 additions & 5 deletions lib/Serialization/Serialization.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3097,9 +3097,9 @@ void Serializer::writeType(Type ty) {
}

case TypeKind::Optional: {
auto sliceTy = cast<OptionalType>(ty.getPointer());
auto optionalTy = cast<OptionalType>(ty.getPointer());

Type base = sliceTy->getBaseType();
Type base = optionalTy->getBaseType();

unsigned abbrCode = DeclTypeAbbrCodes[OptionalTypeLayout::Code];
OptionalTypeLayout::emitRecord(Out, ScratchRecord, abbrCode,
Expand All @@ -3108,13 +3108,13 @@ void Serializer::writeType(Type ty) {
}

case TypeKind::ImplicitlyUnwrappedOptional: {
auto sliceTy = cast<ImplicitlyUnwrappedOptionalType>(ty.getPointer());
auto optionalTy = cast<ImplicitlyUnwrappedOptionalType>(ty.getPointer());

Type base = sliceTy->getBaseType();
Type base = optionalTy->getBaseType();

unsigned abbrCode = DeclTypeAbbrCodes[ImplicitlyUnwrappedOptionalTypeLayout::Code];
ImplicitlyUnwrappedOptionalTypeLayout::emitRecord(Out, ScratchRecord, abbrCode,
addTypeRef(base));
addTypeRef(base));
break;
}

Expand Down
4 changes: 2 additions & 2 deletions stdlib/private/StdlibCollectionUnittest/CheckCollectionType.swift.gyb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,8 +511,8 @@ internal func _product<C1 : Collection, C2 : Collection>(
wrapValueIntoEquatable: @escaping (
MinimalEquatableValue) -> CollectionWithEquatableElement.Iterator.Element,

extractValueFromEquatable:
((CollectionWithEquatableElement.Iterator.Element) -> MinimalEquatableValue),
extractValueFromEquatable: @escaping (
CollectionWithEquatableElement.Iterator.Element) -> MinimalEquatableValue,

resiliencyChecks: CollectionMisuseResiliencyChecks = .all,
outOfBoundsIndexOffset: Int = 1,
Expand Down
20 changes: 10 additions & 10 deletions stdlib/private/StdlibCollectionUnittest/CheckMutableCollectionType.swift.gyb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@ extension TestSuite {

makeCollectionOfEquatable: @escaping ([CollectionWithEquatableElement.Iterator.Element]) -> CollectionWithEquatableElement,
wrapValueIntoEquatable: @escaping (MinimalEquatableValue) -> CollectionWithEquatableElement.Iterator.Element,
extractValueFromEquatable: ((CollectionWithEquatableElement.Iterator.Element) -> MinimalEquatableValue),
extractValueFromEquatable: @escaping ((CollectionWithEquatableElement.Iterator.Element) -> MinimalEquatableValue),

makeCollectionOfComparable: @escaping ([CollectionWithComparableElement.Iterator.Element]) -> CollectionWithComparableElement,
wrapValueIntoComparable: @escaping (MinimalComparableValue) -> CollectionWithComparableElement.Iterator.Element,
extractValueFromComparable: ((CollectionWithComparableElement.Iterator.Element) -> MinimalComparableValue),
extractValueFromComparable: @escaping ((CollectionWithComparableElement.Iterator.Element) -> MinimalComparableValue),

resiliencyChecks: CollectionMisuseResiliencyChecks = .all,
outOfBoundsIndexOffset: Int = 1,
Expand Down Expand Up @@ -502,8 +502,8 @@ self.test("\(testNamePrefix).sorted/DispatchesThrough_withUnsafeMutableBufferPoi

func checkSort_${'Predicate' if predicate else 'WhereElementIsComparable'}(
sequence: [Int],
equalImpl: ((Int, Int) -> Bool),
lessImpl: ((Int, Int) -> Bool),
equalImpl: @escaping ((Int, Int) -> Bool),
lessImpl: @escaping ((Int, Int) -> Bool),
verifyOrder: Bool
) {
% if predicate:
Expand Down Expand Up @@ -683,11 +683,11 @@ self.test("\(testNamePrefix).partition/InvalidOrderings") {

makeCollectionOfEquatable: @escaping ([CollectionWithEquatableElement.Iterator.Element]) -> CollectionWithEquatableElement,
wrapValueIntoEquatable: @escaping (MinimalEquatableValue) -> CollectionWithEquatableElement.Iterator.Element,
extractValueFromEquatable: ((CollectionWithEquatableElement.Iterator.Element) -> MinimalEquatableValue),
extractValueFromEquatable: @escaping ((CollectionWithEquatableElement.Iterator.Element) -> MinimalEquatableValue),

makeCollectionOfComparable: @escaping ([CollectionWithComparableElement.Iterator.Element]) -> CollectionWithComparableElement,
wrapValueIntoComparable: @escaping (MinimalComparableValue) -> CollectionWithComparableElement.Iterator.Element,
extractValueFromComparable: ((CollectionWithComparableElement.Iterator.Element) -> MinimalComparableValue),
extractValueFromComparable: @escaping ((CollectionWithComparableElement.Iterator.Element) -> MinimalComparableValue),

resiliencyChecks: CollectionMisuseResiliencyChecks = .all,
outOfBoundsIndexOffset: Int = 1,
Expand Down Expand Up @@ -836,11 +836,11 @@ self.test("\(testNamePrefix).partition/DispatchesThrough_withUnsafeMutableBuffer

makeCollectionOfEquatable: @escaping ([CollectionWithEquatableElement.Iterator.Element]) -> CollectionWithEquatableElement,
wrapValueIntoEquatable: @escaping (MinimalEquatableValue) -> CollectionWithEquatableElement.Iterator.Element,
extractValueFromEquatable: ((CollectionWithEquatableElement.Iterator.Element) -> MinimalEquatableValue),
extractValueFromEquatable: @escaping ((CollectionWithEquatableElement.Iterator.Element) -> MinimalEquatableValue),

makeCollectionOfComparable: @escaping ([CollectionWithComparableElement.Iterator.Element]) -> CollectionWithComparableElement,
wrapValueIntoComparable: @escaping (MinimalComparableValue) -> CollectionWithComparableElement.Iterator.Element,
extractValueFromComparable: ((CollectionWithComparableElement.Iterator.Element) -> MinimalComparableValue),
extractValueFromComparable: @escaping ((CollectionWithComparableElement.Iterator.Element) -> MinimalComparableValue),

resiliencyChecks: CollectionMisuseResiliencyChecks = .all,
outOfBoundsIndexOffset: Int = 1,
Expand Down Expand Up @@ -919,8 +919,8 @@ self.test("\(testNamePrefix).partition/DispatchesThrough_withUnsafeMutableBuffer

func checkSortInPlace_${'Predicate' if predicate else 'WhereElementIsComparable'}(
sequence: [Int],
equalImpl: ((Int, Int) -> Bool),
lessImpl: ((Int, Int) -> Bool),
equalImpl: @escaping ((Int, Int) -> Bool),
lessImpl: @escaping ((Int, Int) -> Bool),
verifyOrder: Bool
) {
% if predicate:
Expand Down
6 changes: 3 additions & 3 deletions stdlib/private/StdlibCollectionUnittest/CheckRangeReplaceableCollectionType.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ extension TestSuite {

makeCollectionOfEquatable: @escaping ([CollectionWithEquatableElement.Iterator.Element]) -> CollectionWithEquatableElement,
wrapValueIntoEquatable: @escaping (MinimalEquatableValue) -> CollectionWithEquatableElement.Iterator.Element,
extractValueFromEquatable: ((CollectionWithEquatableElement.Iterator.Element) -> MinimalEquatableValue),
extractValueFromEquatable: @escaping ((CollectionWithEquatableElement.Iterator.Element) -> MinimalEquatableValue),

resiliencyChecks: CollectionMisuseResiliencyChecks = .all,
outOfBoundsIndexOffset: Int = 1,
Expand Down Expand Up @@ -1168,7 +1168,7 @@ self.test("\(testNamePrefix).OperatorPlus") {

makeCollectionOfEquatable: @escaping ([CollectionWithEquatableElement.Iterator.Element]) -> CollectionWithEquatableElement,
wrapValueIntoEquatable: @escaping (MinimalEquatableValue) -> CollectionWithEquatableElement.Iterator.Element,
extractValueFromEquatable: ((CollectionWithEquatableElement.Iterator.Element) -> MinimalEquatableValue),
extractValueFromEquatable: @escaping ((CollectionWithEquatableElement.Iterator.Element) -> MinimalEquatableValue),

resiliencyChecks: CollectionMisuseResiliencyChecks = .all,
outOfBoundsIndexOffset: Int = 1
Expand Down Expand Up @@ -1297,7 +1297,7 @@ self.test("\(testNamePrefix).removeLast(n: Int)/whereIndexIsBidirectional/remove

makeCollectionOfEquatable: @escaping ([CollectionWithEquatableElement.Iterator.Element]) -> CollectionWithEquatableElement,
wrapValueIntoEquatable: @escaping (MinimalEquatableValue) -> CollectionWithEquatableElement.Iterator.Element,
extractValueFromEquatable: ((CollectionWithEquatableElement.Iterator.Element) -> MinimalEquatableValue),
extractValueFromEquatable: @escaping ((CollectionWithEquatableElement.Iterator.Element) -> MinimalEquatableValue),

resiliencyChecks: CollectionMisuseResiliencyChecks = .all,
outOfBoundsIndexOffset: Int = 1
Expand Down
Loading