Skip to content
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

Remove an old diagnostic for 'var' in param position #10709

Merged
merged 1 commit into from
Jun 29, 2017
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
6 changes: 3 additions & 3 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -4509,7 +4509,7 @@ class VarDecl : public AbstractStorageDecl {
class ParamDecl : public VarDecl {
Identifier ArgumentName;
SourceLoc ArgumentNameLoc;
SourceLoc LetVarInOutLoc;
SourceLoc SpecifierLoc;

struct StoredDefaultArgument {
Expr *DefaultArg = nullptr;
Expand All @@ -4530,7 +4530,7 @@ class ParamDecl : public VarDecl {
DefaultArgumentKind defaultArgumentKind = DefaultArgumentKind::None;

public:
ParamDecl(bool isLet, SourceLoc letVarInOutLoc, SourceLoc argumentNameLoc,
ParamDecl(bool isLet, SourceLoc specifierLoc, SourceLoc argumentNameLoc,
Identifier argumentName, SourceLoc parameterNameLoc,
Identifier parameterName, Type ty, DeclContext *dc);

Expand All @@ -4548,7 +4548,7 @@ class ParamDecl : public VarDecl {
/// was specified separately from the parameter name.
SourceLoc getArgumentNameLoc() const { return ArgumentNameLoc; }

SourceLoc getLetVarInOutLoc() const { return LetVarInOutLoc; }
SourceLoc getSpecifierLoc() const { return SpecifierLoc; }

bool isTypeLocImplicit() const { return IsTypeLocImplicit; }
void setIsTypeLocImplicit(bool val) { IsTypeLocImplicit = val; }
Expand Down
4 changes: 2 additions & 2 deletions include/swift/AST/DiagnosticsParse.def
Original file line number Diff line number Diff line change
Expand Up @@ -814,8 +814,8 @@ ERROR(inout_as_attr_disallowed,none,
ERROR(parameter_inout_var_let_repeated,none,
"parameter may not have multiple 'inout', 'var', or 'let' specifiers",
())
ERROR(parameter_let_as_attr,none,
"'let' as a parameter attribute is not allowed", ())
ERROR(parameter_let_var_as_attr,none,
"'%select{var|let}0' as a parameter attribute is not allowed", (unsigned))


ERROR(expected_behavior_name,none,
Expand Down
4 changes: 0 additions & 4 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1006,10 +1006,6 @@ ERROR(attribute_requires_operator_identifier,none,
ERROR(attribute_requires_single_argument,none,
"'%0' requires a function with one argument", (StringRef))

ERROR(var_parameter_not_allowed,none,
"parameters may not have the 'var' specifier", ())


ERROR(mutating_invalid_global_scope,none,
"'mutating' is only valid on methods", ())
ERROR(mutating_invalid_classes,none,
Expand Down
9 changes: 4 additions & 5 deletions include/swift/Parse/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -977,15 +977,14 @@ class Parser {
/// Any declaration attributes attached to the parameter.
DeclAttributes Attrs;

/// The location of the 'let', 'var', or 'inout' keyword, if present.
SourceLoc LetVarInOutLoc;
/// The location of the 'inout' keyword, if present.
SourceLoc SpecifierLoc;

enum SpecifierKindTy {
Let,
Var,
None,
InOut
};
SpecifierKindTy SpecifierKind = Let; // Defaults to let.
SpecifierKindTy SpecifierKind = None; // Defaults to 'no value'.

/// The location of the first name.
///
Expand Down
6 changes: 3 additions & 3 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4114,13 +4114,13 @@ void VarDecl::emitLetToVarNoteIfSimple(DeclContext *UseDC) const {
}

ParamDecl::ParamDecl(bool isLet,
SourceLoc letVarInOutLoc, SourceLoc argumentNameLoc,
SourceLoc specifierLoc, SourceLoc argumentNameLoc,
Identifier argumentName, SourceLoc parameterNameLoc,
Identifier parameterName, Type ty, DeclContext *dc)
: VarDecl(DeclKind::Param, /*IsStatic*/false, /*IsLet*/isLet,
/*IsCaptureList*/false, parameterNameLoc, parameterName, ty, dc),
ArgumentName(argumentName), ArgumentNameLoc(argumentNameLoc),
LetVarInOutLoc(letVarInOutLoc) {
SpecifierLoc(specifierLoc) {
}

/// Clone constructor, allocates a new ParamDecl identical to the first.
Expand All @@ -4131,7 +4131,7 @@ ParamDecl::ParamDecl(ParamDecl *PD)
PD->hasType() ? PD->getType() : Type(), PD->getDeclContext()),
ArgumentName(PD->getArgumentName()),
ArgumentNameLoc(PD->getArgumentNameLoc()),
LetVarInOutLoc(PD->getLetVarInOutLoc()),
SpecifierLoc(PD->getSpecifierLoc()),
DefaultValueAndIsVariadic(nullptr, PD->DefaultValueAndIsVariadic.getInt()),
IsTypeLocImplicit(PD->IsTypeLocImplicit),
defaultArgumentKind(PD->defaultArgumentKind) {
Expand Down
32 changes: 16 additions & 16 deletions lib/Parse/ParsePattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,20 +184,20 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
bool hasSpecifier = false;
while (Tok.isAny(tok::kw_inout, tok::kw_let, tok::kw_var)) {
if (!hasSpecifier) {
if (Tok.is(tok::kw_let)) {
diagnose(Tok, diag::parameter_let_as_attr)
.fixItRemove(Tok.getLoc());
if (Tok.is(tok::kw_inout)) {
// This case is handled later when mapping to ParamDecls for
// better fixits.
param.SpecifierKind = ParsedParameter::InOut;
} else {
// We handle the var error in sema for a better fixit and inout is
// handled later in this function for better fixits.
param.SpecifierKind = Tok.is(tok::kw_inout) ? ParsedParameter::InOut :
ParsedParameter::Var;
diagnose(Tok, diag::parameter_let_var_as_attr,
unsigned(Tok.is(tok::kw_let)))
.fixItRemove(Tok.getLoc());
}
param.LetVarInOutLoc = consumeToken();
param.SpecifierLoc = consumeToken();
hasSpecifier = true;
} else {
// Redundant specifiers are fairly common, recognize, reject, and recover
// from this gracefully.
// Redundant specifiers are fairly common, recognize, reject, and
// recover from this gracefully.
diagnose(Tok, diag::parameter_inout_var_let_repeated)
.fixItRemove(Tok.getLoc());
consumeToken();
Expand Down Expand Up @@ -342,8 +342,8 @@ mapParsedParameters(Parser &parser,
Identifier paramName, SourceLoc paramNameLoc)
-> ParamDecl * {
auto specifierKind = paramInfo.SpecifierKind;
bool isLet = specifierKind == Parser::ParsedParameter::Let;
auto param = new (ctx) ParamDecl(isLet, paramInfo.LetVarInOutLoc,
bool isLet = specifierKind == Parser::ParsedParameter::None;
auto param = new (ctx) ParamDecl(isLet, paramInfo.SpecifierLoc,
argNameLoc, argName,
paramNameLoc, paramName, Type(),
parser.CurDeclContext);
Expand All @@ -360,7 +360,7 @@ mapParsedParameters(Parser &parser,
if (auto type = paramInfo.Type) {
// If 'inout' was specified, turn the type into an in-out type.
if (specifierKind == Parser::ParsedParameter::InOut) {
auto InOutLoc = paramInfo.LetVarInOutLoc;
auto InOutLoc = paramInfo.SpecifierLoc;
if (isa<InOutTypeRepr>(type)) {
parser.diagnose(InOutLoc, diag::parameter_inout_var_let_repeated)
.fixItRemove(InOutLoc);
Expand All @@ -380,9 +380,9 @@ mapParsedParameters(Parser &parser,
param->getTypeLoc() = TypeLoc::withoutLoc(ErrorType::get(ctx));
param->setInvalid();
} else if (specifierKind == Parser::ParsedParameter::InOut) {
parser.diagnose(paramInfo.LetVarInOutLoc, diag::inout_must_have_type);
paramInfo.LetVarInOutLoc = SourceLoc();
specifierKind = Parser::ParsedParameter::Let;
parser.diagnose(paramInfo.SpecifierLoc, diag::inout_must_have_type);
paramInfo.SpecifierLoc = SourceLoc();
specifierKind = Parser::ParsedParameter::None;
}
return param;
};
Expand Down
72 changes: 1 addition & 71 deletions lib/Sema/TypeCheckPattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -671,74 +671,6 @@ static bool validateTypedPattern(TypeChecker &TC, DeclContext *DC,
return hadError;
}

static void diagnoseAndMigrateVarParameterToBody(ParamDecl *decl,
AbstractFunctionDecl *func,
TypeChecker &TC) {
if (!func || !func->hasBody()) {
// If there is no function body, just suggest removal.
TC.diagnose(decl->getLetVarInOutLoc(),
diag::var_parameter_not_allowed)
.fixItRemove(decl->getLetVarInOutLoc());
return;
}
// Insert the shadow copy. The computations that follow attempt to
// 'best guess' the indentation and new lines so that the user
// doesn't have to add any whitespace.
auto declBody = func->getBody();

auto &SM = TC.Context.SourceMgr;

SourceLoc insertionStartLoc;
std::string start;
std::string end;

auto lBraceLine = SM.getLineNumber(declBody->getLBraceLoc());
auto rBraceLine = SM.getLineNumber(declBody->getRBraceLoc());

if (!declBody->getNumElements()) {

// Empty function body.
insertionStartLoc = declBody->getRBraceLoc();

if (lBraceLine == rBraceLine) {
// Same line braces, means we probably have something
// like {} as the func body. Insert directly into body with spaces.
start = " ";
end = " ";
} else {
// Different line braces, so use RBrace's indentation.
end = "\n" + Lexer::getIndentationForLine(SM, declBody->
getRBraceLoc()).str();
start = " "; // Guess 4 spaces as extra indentation.
}
} else {
auto firstLine = declBody->getElement(0);
insertionStartLoc = firstLine.getStartLoc();
if (lBraceLine == SM.getLineNumber(firstLine.getStartLoc())) {
// Function on same line, insert with semi-colon. Not ideal but
// better than weird space alignment.
start = "";
end = "; ";
} else {
start = "";
end = "\n" + Lexer::getIndentationForLine(SM, firstLine.
getStartLoc()).str();
}
}
if (insertionStartLoc.isInvalid()) {
TC.diagnose(decl->getLetVarInOutLoc(),
diag::var_parameter_not_allowed)
.fixItRemove(decl->getLetVarInOutLoc());
return;
}
auto parameterName = decl->getNameStr().str();
TC.diagnose(decl->getLetVarInOutLoc(),
diag::var_parameter_not_allowed)
.fixItRemove(decl->getLetVarInOutLoc())
.fixItInsert(insertionStartLoc, start + "var " + parameterName + " = " +
parameterName + end);
}

static bool validateParameterType(ParamDecl *decl, DeclContext *DC,
TypeResolutionOptions options,
GenericTypeResolver &resolver,
Expand Down Expand Up @@ -772,16 +704,14 @@ static bool validateParameterType(ParamDecl *decl, DeclContext *DC,

// If the user did not explicitly write 'let', 'var', or 'inout', we'll let
// type inference figure out what went wrong in detail.
if (decl->getLetVarInOutLoc().isValid()) {
if (decl->getSpecifierLoc().isValid()) {
// If the param is not a 'let' and it is not an 'inout'.
// It must be a 'var'. Provide helpful diagnostics like a shadow copy
// in the function body to fix the 'var' attribute.
if (!decl->isLet() &&
!decl->isImplicit() &&
(Ty.isNull() || !Ty->is<InOutType>()) &&
!hadError) {
auto func = dyn_cast_or_null<AbstractFunctionDecl>(DC);
diagnoseAndMigrateVarParameterToBody(decl, func, TC);
decl->setInvalid();
hadError = true;
}
Expand Down
14 changes: 7 additions & 7 deletions test/Parse/invalid.swift
Original file line number Diff line number Diff line change
Expand Up @@ -82,18 +82,18 @@ func SR979d(let let a: Int) {} // expected-error {{'let' as a parameter attribu
// expected-error @-1 {{parameter may not have multiple 'inout', 'var', or 'let' specifiers}} {{17-21=}}
func SR979e(inout x: inout String) {} // expected-error {{parameter may not have multiple 'inout', 'var', or 'let' specifiers}} {{13-18=}}
func SR979f(var inout x : Int) { // expected-error {{parameter may not have multiple 'inout', 'var', or 'let' specifiers}} {{17-23=}}
// expected-error @-1 {{parameters may not have the 'var' specifier}} {{13-16=}}{{3-3=var x = x\n }}
x += 10
// expected-error @-1 {{'var' as a parameter attribute is not allowed}}
x += 10 // expected-error {{left side of mutating operator isn't mutable: 'x' is a 'let' constant}}
}
func SR979g(inout i: inout Int) {} // expected-error {{parameter may not have multiple 'inout', 'var', or 'let' specifiers}} {{13-18=}}
func SR979h(let inout x : Int) {} // expected-error {{parameter may not have multiple 'inout', 'var', or 'let' specifiers}} {{17-23=}}
// expected-error @-1 {{'let' as a parameter attribute is not allowed}}
class VarTester {
init(var a: Int, var b: Int) {} // expected-error {{parameters may not have the 'var' specifier}} {{8-11=}} {{33-33= var a = a }}
// expected-error @-1 {{parameters may not have the 'var' specifier}} {{20-24=}} {{33-33= var b = b }}
func x(var b: Int) { //expected-error {{parameters may not have the 'var' specifier}} {{12-15=}} {{9-9=var b = b\n }}
b += 10
}
init(var a: Int, var b: Int) {} // expected-error {{'var' as a parameter attribute is not allowed}}
// expected-error @-1 {{'var' as a parameter attribute is not allowed}}
func x(var b: Int) { //expected-error {{'var' as a parameter attribute is not allowed}}
b += 10 // expected-error {{left side of mutating operator isn't mutable: 'b' is a 'let' constant}}
}
}

func repeat() {}
Expand Down
4 changes: 2 additions & 2 deletions test/Sema/immutability.swift
Original file line number Diff line number Diff line change
Expand Up @@ -348,11 +348,11 @@ func testSelectorStyleArguments3(_ x: Int, bar y: Int) {
func invalid_inout(inout var x : Int) { // expected-error {{parameter may not have multiple 'inout', 'var', or 'let' specifiers}} {{26-30=}}
// expected-error @-1 {{'inout' before a parameter name is not allowed, place it before the parameter type instead}}{{20-25=}}{{34-34=inout }}
}
func invalid_var(var x: Int) { // expected-error {{parameters may not have the 'var' specifier}}{{18-21=}} {{1-1= var x = x\n}}
func invalid_var(var x: Int) { // expected-error {{'var' as a parameter attribute is not allowed}}

}
func takesClosure(_: (Int) -> Int) {
takesClosure { (var d) in d } // expected-error {{parameters may not have the 'var' specifier}}
takesClosure { (var d) in d } // expected-error {{'var' as a parameter attribute is not allowed}}
}

func updateInt(_ x : inout Int) {}
Expand Down
4 changes: 2 additions & 2 deletions test/decl/class/override.swift
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@ class H : G {

func manyA(_: AnyObject, _: AnyObject) {}
func manyB(_ a: AnyObject, b: AnyObject) {}
func manyC(var a: AnyObject, // expected-error {{parameters may not have the 'var' specifier}} {{14-17=}}
var b: AnyObject) {} // expected-error {{parameters may not have the 'var' specifier}} {{14-18=}}
func manyC(var a: AnyObject, // expected-error {{'var' as a parameter attribute is not allowed}}
var b: AnyObject) {} // expected-error {{'var' as a parameter attribute is not allowed}}

func result() -> AnyObject? { return nil }
func both(_ x: AnyObject) -> AnyObject? { return x }
Expand Down
6 changes: 3 additions & 3 deletions test/decl/var/usage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ func basicTests() -> Int {
return y
}

func mutableParameter(_ a : Int, h : Int, var i : Int, j: Int, g: Int) -> Int { // expected-error {{parameters may not have the 'var' specifier}}
i += 1
func mutableParameter(_ a : Int, h : Int, var i : Int, j: Int, g: Int) -> Int { // expected-error {{'var' as a parameter attribute is not allowed}}
i += 1 // expected-error {{left side of mutating operator isn't mutable: 'i' is a 'let' constant}}
var j = j
swap(&i, &j)
swap(&i, &j) // expected-error {{cannot pass immutable value as inout argument: 'i' is a 'let' constant}}
return i+g
}

Expand Down