Skip to content

Remove The Last Vestiges of isInvalid from Parse Harder This Time #27862

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 5 commits into from
Oct 26, 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: 0 additions & 2 deletions include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -870,8 +870,6 @@ 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: 4 additions & 10 deletions lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4933,12 +4933,6 @@ 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 @@ -4949,7 +4943,7 @@ static void diagnoseConflictingAccessors(Parser &P, AccessorDecl *first,
P.diagnose(first->getLoc(), diag::previous_accessor,
getAccessorNameForDiagnostic(first, /*article*/ false),
/*already*/ false);
flagInvalidAccessor(second);
second->setInvalid();
}

template <class... DiagArgs>
Expand All @@ -4959,11 +4953,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...);
flagInvalidAccessor(accessor);
accessor->setInvalid();
}
if (auto &accessor = accessors.DidSet) {
P.diagnose(accessor->getLoc(), diagnostic, /*didSet*/ 1, args...);
flagInvalidAccessor(accessor);
accessor->setInvalid();
}
}

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

Expand Down
7 changes: 3 additions & 4 deletions lib/Parse/ParseExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2656,11 +2656,10 @@ 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())
auto *typeRepr = param->getTypeRepr();
if (!(typeRepr && typeRepr->isInvalid()))
return false;
if (auto *typeRepr = param->getTypeRepr())
return !param->hasName() && isa<TupleTypeRepr>(typeRepr);
return false;
return !param->hasName() && isa<TupleTypeRepr>(typeRepr);
};

for (unsigned i = 0, e = params->size(); i != e; ++i) {
Expand Down
23 changes: 7 additions & 16 deletions lib/Parse/ParsePattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,9 @@ 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 @@ -358,8 +361,9 @@ 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
// Mark current parameter type as invalid so it is possible
// to diagnose it as destructuring of the closure parameter list.
param.Type->setInvalid();
param.isInvalid = true;
if (!isClosure) {
// Unnamed parameters must be written as "_: Type".
Expand Down Expand Up @@ -478,12 +482,6 @@ 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 @@ -493,7 +491,7 @@ mapParsedParameters(Parser &parser,

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

// If a type was provided, create the type for the parameter.
if (auto type = paramInfo.Type) {
Expand Down Expand Up @@ -524,13 +522,6 @@ 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 Expand Up @@ -708,7 +699,7 @@ Parser::parseSingleParameterClause(ParameterContextKind paramContext,
// Parse the parameter clause.
status |= parseParameterClause(leftParenLoc, params, rightParenLoc,
defaultArgs, paramContext);

// Turn the parameter clause into argument and body patterns.
auto paramList = mapParsedParameters(*this, leftParenLoc, params,
rightParenLoc, namePieces, paramContext);
Expand Down
6 changes: 2 additions & 4 deletions test/Constraints/tuple_arguments.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1468,8 +1468,6 @@ let _ = sr4745.enumerated().map { (count, element) in "\(count): \(element)" }
// SR-4738

let sr4738 = (1, (2, 3))
// expected-error@+2{{use of undeclared type 'y'}}
// expected-error@+1{{use of undeclared type 'z'}}
[sr4738].map { (x, (y, z)) -> Int in x + y + z }
// expected-error@-1 {{closure tuple parameter does not support destructuring}} {{20-26=arg1}} {{38-38=let (y, z) = arg1; }}

Expand All @@ -1478,10 +1476,8 @@ let r31892961_1 = [1: 1, 2: 2]
r31892961_1.forEach { (k, v) in print(k + v) }

let r31892961_2 = [1, 2, 3]
// expected-error@+1{{use of undeclared type 'val'}}
let _: [Int] = r31892961_2.enumerated().map { ((index, val)) in
// expected-error@-1 {{closure tuple parameter does not support destructuring}} {{48-60=arg0}} {{3-3=\n let (index, val) = arg0\n }}
// expected-error@-2 {{use of undeclared type 'index'}}
val + 1
}

Expand Down Expand Up @@ -1690,6 +1686,7 @@ class Mappable<T> {

let x = Mappable(())
_ = x.map { (_: Void) in return () }
_ = x.map { (_: ()) in () }

// https://bugs.swift.org/browse/SR-9470
do {
Expand Down Expand Up @@ -1756,3 +1753,4 @@ func noescapeSplat() {
takesEscaping(t.0)
}
}

4 changes: 1 addition & 3 deletions test/IDE/complete_default_arguments.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=DEFAULT_ARG_INIT_1 | %FileCheck %s -check-prefix=DEFAULT_ARG_INIT
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=DEFAULT_ARG_INIT_2 | %FileCheck %s -check-prefix=DEFAULT_ARG_INIT_INTCONTEXT
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=DEFAULT_ARG_INIT_3 | %FileCheck %s -check-prefix=DEFAULT_ARG_INIT_INTCONTEXT
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=DEFAULT_ARG_INIT_4 | %FileCheck %s -check-prefix=DEFAULT_ARG_INIT_INTCONTEXT

func freeFuncWithDefaultArgs1(
_ a: Int, b: Int = 42, file: String = #file, line: Int = #line,
Expand Down Expand Up @@ -135,8 +134,7 @@ func testDefaultArgs9(_ x: C2) {
let globalVar = 1
func testDefaultArgInit1(x = #^DEFAULT_ARG_INIT_1^#) { }
func testDefaultArgInit2(_: Int = #^DEFAULT_ARG_INIT_2^#) { }
func testDefaultArgInit3(Int = #^DEFAULT_ARG_INIT_3^#) { }
func testDefaultArgInit4(_ x: Int = #^DEFAULT_ARG_INIT_4^#) { }
func testDefaultArgInit3(_ x: Int = #^DEFAULT_ARG_INIT_3^#) { }
// DEFAULT_ARG_INIT: Begin completions
// DEFAULT_ARG_INIT: Decl[GlobalVar]/CurrModule: globalVar[#Int#]{{; name=.+$}}
// DEFAULT_ARG_INIT: End completions
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 {{parameter requires an explicit type}}
func f573(s Starfish, // expected-error {{expected ':' following argument label and parameter name}}
_ 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 {{parameter requires an explicit type}}
// expected-error @-2 {{expected ':' following argument label and parameter name}}
// 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 {{parameter requires an explicit type}}
// expected-error @-2 {{expected ':' following argument label and parameter name}}
}
func testSelectorStyleArguments3(_ x: Int, bar y: Int) {
++x // expected-error {{cannot pass immutable value to mutating operator: 'x' is a 'let' constant}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors

// rdar://56144412
// XFAIL: asserts
// RUN: not %target-swift-frontend %s -emit-ir
func t(UInt=__FUNCTION__
func&t(