Skip to content

[AST] Sink ParamDecl flag collection logic from Parse to AST #79503

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions include/swift/AST/ASTBridging.h
Original file line number Diff line number Diff line change
Expand Up @@ -1317,16 +1317,19 @@ BridgedPatternBindingDecl BridgedPatternBindingDecl_createParsed(
BridgedVarDeclIntroducer cIntorducer, BridgedArrayRef cBindingEntries);

SWIFT_NAME("BridgedParamDecl.createParsed(_:declContext:specifierLoc:argName:"
"argNameLoc:paramName:paramNameLoc:type:defaultValue:"
"argNameLoc:paramName:paramNameLoc:defaultValue:"
"defaultValueInitContext:)")
BridgedParamDecl BridgedParamDecl_createParsed(
BridgedASTContext cContext, BridgedDeclContext cDeclContext,
BridgedSourceLoc cSpecifierLoc, BridgedIdentifier cArgName,
BridgedSourceLoc cArgNameLoc, BridgedIdentifier cParamName,
BridgedSourceLoc cParamNameLoc, BridgedNullableTypeRepr type,
BridgedNullableExpr defaultValue,
BridgedSourceLoc cParamNameLoc, BridgedNullableExpr defaultValue,
BridgedNullableDefaultArgumentInitializer cDefaultArgumentInitContext);

SWIFT_NAME("BridgedParamDecl.setTypeRepr(self:_:)")
BRIDGED_INLINE void BridgedParamDecl_setTypeRepr(BridgedParamDecl cDecl,
BridgedTypeRepr cType);

/// The various spellings of ownership modifier that can be used in source.
enum ENUM_EXTENSIBILITY_ATTR(closed) BridgedParamSpecifier {
BridgedParamSpecifierDefault,
Expand Down
5 changes: 5 additions & 0 deletions include/swift/AST/ASTBridgingImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,11 @@ swift::ParamSpecifier unbridge(BridgedParamSpecifier specifier) {
}
}

void BridgedParamDecl_setTypeRepr(BridgedParamDecl cDecl,
BridgedTypeRepr cType) {
cDecl.unbridged()->setTypeRepr(cType.unbridged());
}

void BridgedParamDecl_setSpecifier(BridgedParamDecl cDecl,
BridgedParamSpecifier cSpecifier) {
cDecl.unbridged()->setSpecifier(unbridge(cSpecifier));
Expand Down
4 changes: 3 additions & 1 deletion include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -7031,7 +7031,9 @@ class ParamDecl : public VarDecl {

/// Retrieve the TypeRepr corresponding to the parsed type of the parameter, if it exists.
TypeRepr *getTypeRepr() const { return TyReprAndFlags.getPointer(); }
void setTypeRepr(TypeRepr *repr) { TyReprAndFlags.setPointer(repr); }

/// Set the parsed TypeRepr on the parameter.
void setTypeRepr(TypeRepr *repr);

bool isDestructured() const {
auto flags = ArgumentNameAndFlags.getInt();
Expand Down
49 changes: 2 additions & 47 deletions lib/AST/Bridging/DeclBridging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,58 +193,13 @@ BridgedParamDecl BridgedParamDecl_createParsed(
BridgedASTContext cContext, BridgedDeclContext cDeclContext,
BridgedSourceLoc cSpecifierLoc, BridgedIdentifier cArgName,
BridgedSourceLoc cArgNameLoc, BridgedIdentifier cParamName,
BridgedSourceLoc cParamNameLoc, BridgedNullableTypeRepr opaqueType,
BridgedNullableExpr cDefaultArgument,
BridgedSourceLoc cParamNameLoc, BridgedNullableExpr cDefaultArgument,
BridgedNullableDefaultArgumentInitializer cDefaultArgumentInitContext) {
auto *paramDecl = ParamDecl::createParsed(
return ParamDecl::createParsed(
cContext.unbridged(), cSpecifierLoc.unbridged(), cArgNameLoc.unbridged(),
cArgName.unbridged(), cParamNameLoc.unbridged(), cParamName.unbridged(),
cDefaultArgument.unbridged(), cDefaultArgumentInitContext.unbridged(),
cDeclContext.unbridged());

if (auto type = opaqueType.unbridged()) {
paramDecl->setTypeRepr(type);

// FIXME: Copied from 'Parser::parsePattern()'. This should be in Sema.
// Dig through the type to find any attributes or modifiers that are
// associated with the type but should also be reflected on the
// declaration.
auto unwrappedType = type;
while (true) {
if (auto *ATR = dyn_cast<AttributedTypeRepr>(unwrappedType)) {
auto attrs = ATR->getAttrs();
// At this point we actually don't know if that's valid to mark
// this parameter declaration as `autoclosure` because type has
// not been resolved yet - it should either be a function type
// or typealias with underlying function type.
bool autoclosure = llvm::any_of(attrs, [](TypeOrCustomAttr attr) {
if (auto typeAttr = attr.dyn_cast<TypeAttribute *>())
return isa<AutoclosureTypeAttr>(typeAttr);
return false;
});
paramDecl->setAutoClosure(autoclosure);

unwrappedType = ATR->getTypeRepr();
continue;
}

if (auto *STR = dyn_cast<SpecifierTypeRepr>(unwrappedType)) {
if (isa<IsolatedTypeRepr>(STR))
paramDecl->setIsolated(true);
else if (isa<CompileTimeConstTypeRepr>(STR))
paramDecl->setCompileTimeConst(true);
else if (isa<SendingTypeRepr>(STR))
paramDecl->setSending(true);

unwrappedType = STR->getBase();
continue;
}

break;
}
}

return paramDecl;
}

void BridgedConstructorDecl_setParsedBody(BridgedConstructorDecl decl,
Expand Down
41 changes: 41 additions & 0 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8712,6 +8712,47 @@ ParamDecl *ParamDecl::createParsed(
return decl;
}

void ParamDecl::setTypeRepr(TypeRepr *repr) {
ASSERT(!getTypeRepr() && "TypeRepr already set");

TyReprAndFlags.setPointer(repr);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we assert that the type repr hasn't been set before?


// Dig through the type to find any attributes or modifiers that are
// associated with the type but should also be reflected on the
// declaration.
{
auto unwrappedType = repr;
while (true) {
if (auto *ATR = dyn_cast<AttributedTypeRepr>(unwrappedType)) {
// At this point we actually don't know if that's valid to mark
// this parameter declaration as `autoclosure` because type has
// not been resolved yet - it should either be a function type
// or typealias with underlying function type.
if (ATR->has(TypeAttrKind::Autoclosure))
setAutoClosure(true);
if (ATR->has(TypeAttrKind::Addressable))
setAddressable(true);

unwrappedType = ATR->getTypeRepr();
continue;
}

if (auto *STR = dyn_cast<SpecifierTypeRepr>(unwrappedType)) {
if (isa<IsolatedTypeRepr>(STR))
setIsolated(true);
else if (isa<CompileTimeConstTypeRepr>(STR))
setCompileTimeConst(true);
else if (isa<SendingTypeRepr>(STR))
setSending(true);
unwrappedType = STR->getBase();
continue;
}

break;
}
}
}

void ParamDecl::setDefaultArgumentKind(DefaultArgumentKind K) {
assert(getDefaultArgumentKind() == DefaultArgumentKind::None &&
"Overwrite of default argument kind");
Expand Down
1 change: 0 additions & 1 deletion lib/ASTGen/Sources/ASTGen/Exprs.swift
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,6 @@ extension ASTGenVisitor {
argNameLoc: nil,
paramName: ctx.getDollarIdentifier(idx),
paramNameLoc: loc,
type: nil,
defaultValue: nil,
defaultValueInitContext: nil
)
Expand Down
8 changes: 3 additions & 5 deletions lib/ASTGen/Sources/ASTGen/ParameterClause.swift
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,12 @@ extension ASTGenVisitor {
argNameLoc: argNameLoc,
paramName: paramName,
paramNameLoc: paramNameLoc,
type: type.asNullable,
defaultValue: initExpr.asNullable,
defaultValueInitContext: initContext.asNullable
)
if let type {
param.setTypeRepr(type)
}
param.asDecl.attachParsedAttrs(attrs)
return param
}
Expand All @@ -194,11 +196,9 @@ extension ASTGenVisitor {
argNameLoc: nil,
paramName: name.identifier,
paramNameLoc: name.sourceLoc,
type: nil,
defaultValue: nil,
defaultValueInitContext: nil
)
param.setSpecifier(.default)
return param
}
}
Expand Down Expand Up @@ -259,7 +259,6 @@ extension ASTGenVisitor {
argNameLoc: nil,
paramName: name,
paramNameLoc: nameLoc,
type: nil,
defaultValue: nil,
defaultValueInitContext: nil
)
Expand All @@ -276,7 +275,6 @@ extension ASTGenVisitor {
params.reserveCapacity(node.parameters.count)
for (index, node) in node.parameters.enumerated() {
let param = self.generate(closureParameter: node, at: index)
param.setSpecifier(.default)
Copy link
Member Author

Choose a reason for hiding this comment

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

This line was a mistake because this set the closure parameter specifier to .default even if the TypeRepr has been set. So closures like { (arg: inout T) in .. } wasn't correctly generated.

params.append(param)
}

Expand Down
38 changes: 0 additions & 38 deletions lib/Parse/ParsePattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -607,40 +607,6 @@ mapParsedParameters(Parser &parser,

param->setTypeRepr(type);

// Dig through the type to find any attributes or modifiers that are
// associated with the type but should also be reflected on the
// declaration.
{
auto unwrappedType = type;
while (true) {
if (auto *ATR = dyn_cast<AttributedTypeRepr>(unwrappedType)) {
// At this point we actually don't know if that's valid to mark
// this parameter declaration as `autoclosure` because type has
// not been resolved yet - it should either be a function type
// or typealias with underlying function type.
if (ATR->has(TypeAttrKind::Autoclosure))
param->setAutoClosure(true);
if (ATR->has(TypeAttrKind::Addressable))
param->setAddressable(true);

unwrappedType = ATR->getTypeRepr();
continue;
}

if (auto *STR = dyn_cast<SpecifierTypeRepr>(unwrappedType)) {
if (isa<IsolatedTypeRepr>(STR))
param->setIsolated(true);
else if (isa<CompileTimeConstTypeRepr>(STR))
param->setCompileTimeConst(true);
else if (isa<SendingTypeRepr>(STR))
param->setSending(true);
unwrappedType = STR->getBase();
continue;
}

break;
}
}
} else if (paramInfo.SpecifierLoc.isValid()) {
llvm::SmallString<16> specifier;
{
Expand All @@ -653,10 +619,6 @@ mapParsedParameters(Parser &parser,
specifier);
paramInfo.SpecifierLoc = SourceLoc();
paramInfo.SpecifierKind = ParamDecl::Specifier::Default;

param->setSpecifier(ParamSpecifier::Default);
} else {
param->setSpecifier(ParamSpecifier::Default);
}

return param;
Expand Down
6 changes: 6 additions & 0 deletions lib/Sema/TypeCheckDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2234,6 +2234,12 @@ ParamSpecifierRequest::evaluate(Evaluator &evaluator,
}

auto typeRepr = param->getTypeRepr();

if (!typeRepr && !param->isImplicit()) {
// Untyped closure parameter.
return ParamSpecifier::Default;
}

assert(typeRepr != nullptr && "Should call setSpecifier() on "
"synthesized parameter declarations");

Expand Down
8 changes: 7 additions & 1 deletion test/ASTGen/decls.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,13 @@ func test2(y: Int = 0, oi: Int? = nil) -> Int {
}

func test3(_ b: inout Bool) {
// b = true
b = true
}

func testInOutClosureParam() -> (inout (Int, String)) -> Void {
return { (arg: inout (Int, String)) in
arg.1 = "rewritten"
}
}

func test4(_ i: _const Int) {
Expand Down