Skip to content

[Function builder availability] Address review comments. #33046

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
Jul 22, 2020
Merged

[Function builder availability] Address review comments. #33046

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
2 changes: 1 addition & 1 deletion include/swift/AST/DiagnosticsSema.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5128,7 +5128,7 @@ NOTE(function_builder_infer_pick_specific, none,
"apply function builder %0 (inferred from %select{protocol|dynamic replacement of}1 %2)",
(Type, unsigned, DeclName))
WARNING(function_builder_missing_limited_availability, none,
"function builder %0 does not implement `buildLimitedAvailability`; "
"function builder %0 does not implement 'buildLimitedAvailability'; "
"this code may crash on earlier versions of the OS",
(Type))

Expand Down
13 changes: 7 additions & 6 deletions lib/Sema/BuilderTransform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ class BuilderClosureVisitor
return nullptr;

// If there is a #available in the condition, the 'then' will need to
// be wrapped in a call to buildAvailabilityErasure(_:), if available.
// be wrapped in a call to buildLimitedAvailability(_:), if available.
Expr *thenVarRefExpr = buildVarRef(
thenVar, ifStmt->getThenStmt()->getEndLoc());
if (findAvailabilityCondition(ifStmt->getCond()) &&
Expand Down Expand Up @@ -1202,10 +1202,14 @@ class BuilderClosureRewriter
ifStmt->setThenStmt(newThen);

// Look for a #available condition. If there is one, we need to check
// that the resulting type of the "then" does refer to any types that
// that the resulting type of the "then" doesn't refer to any types that
// are unavailable in the enclosing context.
//
// Note that this is for staging in support for
// Note that this is for staging in support for buildLimitedAvailability();
// the diagnostic is currently a warning, so that existing code that
// compiles today will continue to compile. Once function builder types
// have had the change to adopt buildLimitedAvailability(), we'll upgrade
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you mean "chance" instead of "change"? :-)

Suggested change
// have had the change to adopt buildLimitedAvailability(), we'll upgrade
// have had the chance to adopt buildLimitedAvailability(), we'll upgrade

Copy link
Member Author

Choose a reason for hiding this comment

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

ARGH! I did.

// this warning to an error.
if (auto availabilityCond = findAvailabilityCondition(ifStmt->getCond())) {
SourceLoc loc = availabilityCond->getStartLoc();
Type thenBodyType = solution.simplifyType(
Expand All @@ -1217,9 +1221,6 @@ class BuilderClosureRewriter

if (auto reason = TypeChecker::checkDeclarationAvailability(
nominal, loc, dc)) {
// Note that the problem is with the function builder, not the body.
// This is for staging only. We want to disable #available in
// function builders that don't support this operation.
ctx.Diags.diagnose(
loc, diag::function_builder_missing_limited_availability,
builderTransform.builderType);
Expand Down
2 changes: 1 addition & 1 deletion test/Constraints/function_builder_availability.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ tuplify(true) { cond in
if #available(OSX 10.51, *) {
globalFuncAvailableOn10_51()
tuplify(false) { cond2 in
if cond, #available(OSX 10.52, *) { // expected-warning{{function builder 'TupleBuilder' does not implement `buildLimitedAvailability`; this code may crash on earlier versions of the OS}}
if cond, #available(OSX 10.52, *) { // expected-warning{{function builder 'TupleBuilder' does not implement 'buildLimitedAvailability'; this code may crash on earlier versions of the OS}}
cond2
globalFuncAvailableOn10_52()
} else {
Expand Down