Skip to content

Revert "Remove The Last Vestiges of isInvalid from Parse" #27835

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
Oct 22, 2019
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
2 changes: 2 additions & 0 deletions include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,8 @@ ERROR(expected_parameter_colon,PointsToFirstBadToken,
"expected ':' following argument label and parameter name", ())
ERROR(expected_assignment_instead_of_comparison_operator,none,
"expected '=' instead of '==' to assign default value for parameter", ())
ERROR(missing_parameter_type,PointsToFirstBadToken,
"parameter requires an explicit type", ())
ERROR(multiple_parameter_ellipsis,none,
"only a single variadic parameter '...' is permitted", ())
ERROR(parameter_vararg_default,none,
Expand Down
14 changes: 10 additions & 4 deletions lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4933,6 +4933,12 @@ void Parser::ParsedAccessors::record(Parser &P, AbstractStorageDecl *storage,
storage->setAccessors(LBLoc, Accessors, RBLoc);
}

static void flagInvalidAccessor(AccessorDecl *func) {
if (func) {
func->setInvalid();
}
}

static void diagnoseConflictingAccessors(Parser &P, AccessorDecl *first,
AccessorDecl *&second) {
if (!second) return;
Expand All @@ -4943,7 +4949,7 @@ static void diagnoseConflictingAccessors(Parser &P, AccessorDecl *first,
P.diagnose(first->getLoc(), diag::previous_accessor,
getAccessorNameForDiagnostic(first, /*article*/ false),
/*already*/ false);
second->setInvalid();
flagInvalidAccessor(second);
}

template <class... DiagArgs>
Expand All @@ -4953,11 +4959,11 @@ static void diagnoseAndIgnoreObservers(Parser &P,
typename std::enable_if<true, DiagArgs>::type... args) {
if (auto &accessor = accessors.WillSet) {
P.diagnose(accessor->getLoc(), diagnostic, /*willSet*/ 0, args...);
accessor->setInvalid();
flagInvalidAccessor(accessor);
}
if (auto &accessor = accessors.DidSet) {
P.diagnose(accessor->getLoc(), diagnostic, /*didSet*/ 1, args...);
accessor->setInvalid();
flagInvalidAccessor(accessor);
}
}

Expand All @@ -4969,7 +4975,7 @@ void Parser::ParsedAccessors::classify(Parser &P, AbstractStorageDecl *storage,
// was invalid.
if (invalid) {
for (auto accessor : Accessors) {
accessor->setInvalid();
flagInvalidAccessor(accessor);
}
}

Expand Down
2 changes: 2 additions & 0 deletions lib/Parse/ParseExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2656,6 +2656,8 @@ parseClosureSignatureIfPresent(SmallVectorImpl<CaptureListEntry> &captureList,
// to detect any tuple splat or destructuring as early as
// possible and give a proper fix-it. See SE-0110 for more details.
auto isTupleDestructuring = [](ParamDecl *param) -> bool {
if (!param->isInvalid())
return false;
if (auto *typeRepr = param->getTypeRepr())
return !param->hasName() && isa<TupleTypeRepr>(typeRepr);
return false;
Expand Down
20 changes: 16 additions & 4 deletions lib/Parse/ParsePattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -327,9 +327,6 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
// was invalid. Remember that.
if (type.isParseError() && !type.hasCodeCompletion())
param.isInvalid = true;
} else if (paramContext != Parser::ParameterContextKind::Closure) {
diagnose(Tok, diag::expected_parameter_colon);
param.isInvalid = true;
}
} else {
// Otherwise, we have invalid code. Check to see if this looks like a
Expand Down Expand Up @@ -361,6 +358,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
// on is most likely argument destructuring, we are going
// to diagnose that after all of the parameters are parsed.
if (param.Type) {
// Mark current parameter as invalid so it is possible
// to diagnose it as destructuring of the closure parameter list.
param.isInvalid = true;
if (!isClosure) {
// Unnamed parameters must be written as "_: Type".
Expand Down Expand Up @@ -479,6 +478,12 @@ mapParsedParameters(Parser &parser,
parser.CurDeclContext);
param->getAttrs() = paramInfo.Attrs;

auto setInvalid = [&]{
if (param->isInvalid())
return;
param->setInvalid();
};

bool parsingEnumElt
= (paramContext == Parser::ParameterContextKind::EnumElement);
// If we're not parsing an enum case, lack of a SourceLoc for both
Expand All @@ -488,7 +493,7 @@ mapParsedParameters(Parser &parser,

// If we diagnosed this parameter as a parse error, propagate to the decl.
if (paramInfo.isInvalid)
param->setInvalid();
setInvalid();

// If a type was provided, create the type for the parameter.
if (auto type = paramInfo.Type) {
Expand Down Expand Up @@ -519,6 +524,13 @@ mapParsedParameters(Parser &parser,
// or typealias with underlying function type.
param->setAutoClosure(attrs.has(TypeAttrKind::TAK_autoclosure));
}
} else if (paramContext != Parser::ParameterContextKind::Closure) {
// Non-closure parameters require a type.
if (!param->isInvalid())
parser.diagnose(param->getLoc(), diag::missing_parameter_type);

param->setSpecifier(ParamSpecifier::Default);
setInvalid();
} else if (paramInfo.SpecifierLoc.isValid()) {
StringRef specifier;
switch (paramInfo.SpecifierKind) {
Expand Down
4 changes: 2 additions & 2 deletions test/Parse/invalid.swift
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ protocol Animal<Food> { // expected-error {{protocols do not allow generic para
// SR-573 - Crash with invalid parameter declaration
class Starfish {}
struct Salmon {}
func f573(s Starfish, // expected-error {{expected ':' following argument label and parameter name}}
func f573(s Starfish, // expected-error {{parameter requires an explicit type}}
_ ss: Salmon) -> [Int] {}
func g573() { f573(Starfish(), Salmon()) }

Expand All @@ -89,7 +89,7 @@ func SR979b(inout inout b: Int) {} // expected-error {{inout' before a parameter
// expected-error@-1 {{parameter must not have multiple '__owned', 'inout', or '__shared' specifiers}} {{19-25=}}
func SR979d(let let a: Int) {} // expected-warning {{'let' in this position is interpreted as an argument label}} {{13-16=`let`}}
// expected-error @-1 {{expected ',' separator}} {{20-20=,}}
// expected-error @-2 {{expected ':' following argument label and parameter name}}
// expected-error @-2 {{parameter requires an explicit type}}
// expected-warning @-3 {{extraneous duplicate parameter name; 'let' already has an argument label}} {{13-17=}}
func SR979e(inout x: inout String) {} // expected-error {{parameter must not have multiple '__owned', 'inout', or '__shared' specifiers}} {{13-18=}}
func SR979g(inout i: inout Int) {} // expected-error {{parameter must not have multiple '__owned', 'inout', or '__shared' specifiers}} {{13-18=}}
Expand Down
2 changes: 1 addition & 1 deletion test/Sema/immutability.swift
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ func testSelectorStyleArguments1(_ x: Int, bar y: Int) {
func testSelectorStyleArguments2(let x: Int, // expected-warning {{'let' in this position is interpreted as an argument label}}{{34-37=`let`}}
let bar y: Int) { // expected-warning {{'let' in this position is interpreted as an argument label}}{{34-37=`let`}}
// expected-error @-1 {{expected ',' separator}}
// expected-error @-2 {{expected ':' following argument label and parameter name}}
// expected-error @-2 {{parameter requires an explicit type}}
}
func testSelectorStyleArguments3(_ x: Int, bar y: Int) {
++x // expected-error {{cannot pass immutable value to mutating operator: 'x' is a 'let' constant}}
Expand Down