Skip to content

Commit 78fb745

Browse files
xedinxedin
authored
Merge pull request #17142 from xedin/rdar-40945329-4.2
[4.2][Sema] Diagnose misplaced InOutExpr in `preCheckExpression`
2 parents e1927ec + 76accbb commit 78fb745

File tree

6 files changed

+99
-65
lines changed

6 files changed

+99
-65
lines changed

‎include/swift/AST/DiagnosticsParse.def

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,8 +1098,6 @@ ERROR(expected_operator_ref,none,
10981098
"expected operator name in operator reference", ())
10991099
ERROR(invalid_postfix_operator,none,
11001100
"operator with postfix spacing cannot start a subexpression", ())
1101-
ERROR(extraneous_amp_prefix,none,
1102-
"use of extraneous '&'", ())
11031101

11041102
ERROR(expected_member_name,PointsToFirstBadToken,
11051103
"expected member name following '.'", ())

‎include/swift/Parse/Parser.h

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1160,21 +1160,19 @@ class Parser {
11601160

11611161
//===--------------------------------------------------------------------===//
11621162
// Expression Parsing
1163-
ParserResult<Expr> parseExpr(Diag<> ID, bool allowAmpPrefix = false) {
1164-
return parseExprImpl(ID, /*isExprBasic=*/false, allowAmpPrefix);
1163+
ParserResult<Expr> parseExpr(Diag<> ID) {
1164+
return parseExprImpl(ID, /*isExprBasic=*/false);
11651165
}
11661166
ParserResult<Expr> parseExprBasic(Diag<> ID) {
11671167
return parseExprImpl(ID, /*isExprBasic=*/true);
11681168
}
1169-
ParserResult<Expr> parseExprImpl(Diag<> ID, bool isExprBasic,
1170-
bool allowAmpPrefix = false);
1169+
ParserResult<Expr> parseExprImpl(Diag<> ID, bool isExprBasic);
11711170
ParserResult<Expr> parseExprIs();
11721171
ParserResult<Expr> parseExprAs();
11731172
ParserResult<Expr> parseExprArrow();
11741173
ParserResult<Expr> parseExprSequence(Diag<> ID,
11751174
bool isExprBasic,
1176-
bool isForConditionalDirective = false,
1177-
bool allowAmpPrefix = false);
1175+
bool isForConditionalDirective = false);
11781176
ParserResult<Expr> parseExprSequenceElement(Diag<> ID,
11791177
bool isExprBasic);
11801178
ParserResult<Expr> parseExprPostfixSuffix(ParserResult<Expr> inner,
@@ -1278,8 +1276,7 @@ class Parser {
12781276
SmallVectorImpl<SourceLoc> &exprLabelLocs,
12791277
SourceLoc &rightLoc,
12801278
Expr *&trailingClosure,
1281-
syntax::SyntaxKind Kind,
1282-
bool allowAmpPrefix = false);
1279+
syntax::SyntaxKind Kind);
12831280

12841281
ParserResult<Expr> parseTrailingClosure(SourceRange calleeRange);
12851282

‎lib/Parse/ParseExpr.cpp

Lines changed: 27 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,7 @@ using namespace swift::syntax;
4141
///
4242
/// \param isExprBasic Whether we're only parsing an expr-basic.
4343
ParserResult<Expr> Parser::parseExprImpl(Diag<> Message,
44-
bool isExprBasic,
45-
bool allowAmpPrefix) {
44+
bool isExprBasic) {
4645
// Start a context for creating expression syntax.
4746
SyntaxParsingContext ExprParsingContext(SyntaxContext, SyntaxContextKind::Expr);
4847

@@ -64,8 +63,7 @@ ParserResult<Expr> Parser::parseExprImpl(Diag<> Message,
6463
}
6564

6665
auto expr = parseExprSequence(Message, isExprBasic,
67-
/*forConditionalDirective*/false,
68-
allowAmpPrefix);
66+
/*forConditionalDirective*/false);
6967
if (expr.hasCodeCompletion())
7068
return expr;
7169
if (expr.isNull())
@@ -170,8 +168,7 @@ ParserResult<Expr> Parser::parseExprArrow() {
170168
/// apply to everything to its right.
171169
ParserResult<Expr> Parser::parseExprSequence(Diag<> Message,
172170
bool isExprBasic,
173-
bool isForConditionalDirective,
174-
bool allowAmpPrefix) {
171+
bool isForConditionalDirective) {
175172
SyntaxParsingContext ExprSequnceContext(SyntaxContext, SyntaxContextKind::Expr);
176173

177174
SmallVector<Expr*, 8> SequencedExprs;
@@ -182,36 +179,10 @@ ParserResult<Expr> Parser::parseExprSequence(Diag<> Message,
182179
while (true) {
183180
if (isForConditionalDirective && Tok.isAtStartOfLine())
184181
break;
185-
186-
ParserResult<Expr> Primary;
187-
188-
SourceLoc AmpPrefix;
189-
if (Tok.is(tok::amp_prefix)) {
190-
SyntaxParsingContext AmpCtx(SyntaxContext, SyntaxKind::InOutExpr);
191-
AmpPrefix = consumeToken(tok::amp_prefix);
192-
193-
auto SubExpr = parseExprUnary(Message, isExprBasic);
194-
auto allowNextAmpPrefix = Tok.isBinaryOperator();
195-
if (SubExpr.hasCodeCompletion()) {
196-
Primary = makeParserCodeCompletionResult<Expr>();
197-
} else if (SubExpr.isNull()) {
198-
Primary = nullptr;
199-
} else if (allowAmpPrefix || allowNextAmpPrefix) {
200-
Primary = makeParserResult(
201-
new (Context) InOutExpr(AmpPrefix, SubExpr.get(), Type()));
202-
} else {
203-
diagnose(AmpPrefix, diag::extraneous_amp_prefix);
204-
// In the long run, we should assign SubExpr to Primary to improve
205-
// single-pass diagnostic completeness, but for now, doing so exposes
206-
// diagnostic bugs in Sema where '&' is wrongly suggested as a fix.
207-
Primary = makeParserErrorResult(new (Context) ErrorExpr(
208-
{AmpPrefix, SubExpr.get()->getSourceRange().End}));
209-
}
210-
allowAmpPrefix = allowNextAmpPrefix;
211-
} else {
212-
// Parse a unary expression.
213-
Primary = parseExprSequenceElement(Message, isExprBasic);
214-
}
182+
183+
// Parse a unary expression.
184+
ParserResult<Expr> Primary =
185+
parseExprSequenceElement(Message, isExprBasic);
215186

216187
HasCodeCompletion |= Primary.hasCodeCompletion();
217188
if (Primary.isNull()) {
@@ -256,8 +227,7 @@ ParserResult<Expr> Parser::parseExprSequence(Diag<> Message,
256227
SyntaxKind::BinaryOperatorExpr);
257228
Expr *Operator = parseExprOperator();
258229
SequencedExprs.push_back(Operator);
259-
allowAmpPrefix = true;
260-
230+
261231
// The message is only valid for the first subexpr.
262232
Message = diag::expected_expr_after_operator;
263233
break;
@@ -508,6 +478,19 @@ ParserResult<Expr> Parser::parseExprUnary(Diag<> Message, bool isExprBasic) {
508478
// If the next token is not an operator, just parse this as expr-postfix.
509479
return parseExprPostfix(Message, isExprBasic);
510480

481+
case tok::amp_prefix: {
482+
SyntaxParsingContext AmpCtx(SyntaxContext, SyntaxKind::InOutExpr);
483+
SourceLoc Loc = consumeToken(tok::amp_prefix);
484+
485+
ParserResult<Expr> SubExpr = parseExprUnary(Message, isExprBasic);
486+
if (SubExpr.hasCodeCompletion())
487+
return makeParserCodeCompletionResult<Expr>();
488+
if (SubExpr.isNull())
489+
return nullptr;
490+
return makeParserResult(
491+
new (Context) InOutExpr(Loc, SubExpr.get(), Type()));
492+
}
493+
511494
case tok::backslash:
512495
return parseExprKeyPath();
513496

@@ -919,8 +902,7 @@ ParserResult<Expr> Parser::parseExprSuper(bool isExprBasic) {
919902
indexArgLabelLocs,
920903
rSquareLoc,
921904
trailingClosure,
922-
SyntaxKind::FunctionCallArgumentList,
923-
/*allowAmpPrefix*/true);
905+
SyntaxKind::FunctionCallArgumentList);
924906
if (status.hasCodeCompletion())
925907
return makeParserCodeCompletionResult<Expr>();
926908
if (status.isError())
@@ -1268,8 +1250,7 @@ Parser::parseExprPostfixSuffix(ParserResult<Expr> Result, bool isExprBasic,
12681250
tok::l_square, tok::r_square,
12691251
/*isPostfix=*/true, isExprBasic, lSquareLoc, indexArgs,
12701252
indexArgLabels, indexArgLabelLocs, rSquareLoc, trailingClosure,
1271-
SyntaxKind::FunctionCallArgumentList,
1272-
/*allowAmpPrefix*/true);
1253+
SyntaxKind::FunctionCallArgumentList);
12731254
if (status.hasCodeCompletion())
12741255
return makeParserCodeCompletionResult<Expr>();
12751256
if (status.isError() || Result.isNull())
@@ -1705,8 +1686,7 @@ ParserResult<Expr> Parser::parseExprPrimary(Diag<> ID, bool isExprBasic) {
17051686
argLabelLocs,
17061687
rParenLoc,
17071688
trailingClosure,
1708-
SyntaxKind::FunctionCallArgumentList,
1709-
/*allowAmpPrefix*/true);
1689+
SyntaxKind::FunctionCallArgumentList);
17101690
if (status.isError())
17111691
return nullptr;
17121692

@@ -2942,8 +2922,7 @@ ParserStatus Parser::parseExprList(tok leftTok, tok rightTok,
29422922
SmallVectorImpl<SourceLoc> &exprLabelLocs,
29432923
SourceLoc &rightLoc,
29442924
Expr *&trailingClosure,
2945-
SyntaxKind Kind,
2946-
bool allowAmpPrefix) {
2925+
SyntaxKind Kind) {
29472926
trailingClosure = nullptr;
29482927

29492928
StructureMarkerRAII ParsingExprList(*this, Tok);
@@ -2980,8 +2959,7 @@ ParserStatus Parser::parseExprList(tok leftTok, tok rightTok,
29802959
DeclRefKind::Ordinary,
29812960
DeclNameLoc(Loc));
29822961
} else {
2983-
auto ParsedSubExpr = parseExpr(diag::expected_expr_in_expr_list,
2984-
/*allowAmpPrefix*/allowAmpPrefix);
2962+
auto ParsedSubExpr = parseExpr(diag::expected_expr_in_expr_list);
29852963
SubExpr = ParsedSubExpr.getPtrOrNull();
29862964
Status = ParsedSubExpr;
29872965
}
@@ -3225,8 +3203,7 @@ Parser::parseExprCallSuffix(ParserResult<Expr> fn, bool isExprBasic) {
32253203
argLabelLocs,
32263204
rParenLoc,
32273205
trailingClosure,
3228-
SyntaxKind::FunctionCallArgumentList,
3229-
/*allowAmpPrefix*/true);
3206+
SyntaxKind::FunctionCallArgumentList);
32303207

32313208
// Form the call.
32323209
auto Result = makeParserResult(status | fn,

‎lib/Sema/TypeCheckConstraints.cpp

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,8 @@ namespace {
852852
TypeChecker &TC;
853853
DeclContext *DC;
854854

855+
Expr *ParentExpr;
856+
855857
/// A stack of expressions being walked, used to determine where to
856858
/// insert RebindSelfInConstructorExpr nodes.
857859
llvm::SmallVector<Expr *, 8> ExprStack;
@@ -880,7 +882,8 @@ namespace {
880882
void resolveKeyPathExpr(KeyPathExpr *KPE);
881883

882884
public:
883-
PreCheckExpression(TypeChecker &tc, DeclContext *dc) : TC(tc), DC(dc) { }
885+
PreCheckExpression(TypeChecker &tc, DeclContext *dc, Expr *parent)
886+
: TC(tc), DC(dc), ParentExpr(parent) {}
884887

885888
bool walkToClosureExprPre(ClosureExpr *expr);
886889

@@ -941,6 +944,43 @@ namespace {
941944
return finish(true, expr);
942945
}
943946

947+
// Let's try to figure out if `InOutExpr` is out of place early
948+
// otherwise there is a risk of producing solutions which can't
949+
// be later applied to AST and would result in the crash in some
950+
// cases. Such expressions are only allowed in argument positions
951+
// of function/operator calls.
952+
if (isa<InOutExpr>(expr)) {
953+
// If this is an implicit `inout` expression we assume that
954+
// compiler knowns what it's doing.
955+
if (expr->isImplicit())
956+
return finish(true, expr);
957+
958+
if (TC.isExprBeingDiagnosed(ParentExpr) ||
959+
TC.isExprBeingDiagnosed(expr))
960+
return finish(true, expr);
961+
962+
auto parents = ParentExpr->getParentMap();
963+
964+
auto result = parents.find(expr);
965+
if (result != parents.end()) {
966+
auto *parent = result->getSecond();
967+
968+
if (isa<SequenceExpr>(parent))
969+
return finish(true, expr);
970+
971+
if (isa<TupleExpr>(parent) || isa<ParenExpr>(parent)) {
972+
auto call = parents.find(parent);
973+
if (call != parents.end() &&
974+
(isa<ApplyExpr>(call->getSecond()) ||
975+
isa<UnresolvedMemberExpr>(call->getSecond())))
976+
return finish(true, expr);
977+
}
978+
}
979+
980+
TC.diagnose(expr->getStartLoc(), diag::extraneous_address_of);
981+
return finish(false, nullptr);
982+
}
983+
944984
return finish(true, expr);
945985
}
946986

@@ -1699,7 +1739,7 @@ CleanupIllFormedExpressionRAII::~CleanupIllFormedExpressionRAII() {
16991739
/// Pre-check the expression, validating any types that occur in the
17001740
/// expression and folding sequence expressions.
17011741
bool TypeChecker::preCheckExpression(Expr *&expr, DeclContext *dc) {
1702-
PreCheckExpression preCheck(*this, dc);
1742+
PreCheckExpression preCheck(*this, dc, expr);
17031743
// Perform the pre-check.
17041744
if (auto result = expr->walk(preCheck)) {
17051745
expr = result;

‎test/Constraints/rdar40945329.swift

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
class A {
4+
static var a: Int = 0
5+
static var b: Int = 42
6+
7+
func foo(_ ptr: UnsafeMutableRawPointer?) {
8+
switch ptr {
9+
case (&A.a)?: break
10+
case (&A.b)?: break
11+
default: break
12+
}
13+
}
14+
15+
func bar(_ ptr: UnsafeRawPointer) {
16+
switch ptr {
17+
case &A.a: break
18+
case &A.b: break
19+
default: break
20+
}
21+
}
22+
}

‎test/expr/expressions.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -814,8 +814,8 @@ public struct TestPropMethodOverloadGroup {
814814
// <rdar://problem/18496742> Passing ternary operator expression as inout crashes Swift compiler
815815
func inoutTests(_ arr: inout Int) {
816816
var x = 1, y = 2
817-
(true ? &x : &y) // expected-error 2 {{use of extraneous '&'}}
818-
let a = (true ? &x : &y) // expected-error 2 {{use of extraneous '&'}}
817+
(true ? &x : &y) // expected-error {{use of extraneous '&'}}
818+
let a = (true ? &x : &y) // expected-error {{use of extraneous '&'}}
819819

820820
inoutTests(true ? &x : &y) // expected-error {{use of extraneous '&'}}
821821

@@ -827,7 +827,7 @@ func inoutTests(_ arr: inout Int) {
827827
inoutTests(&x)
828828

829829
// <rdar://problem/17489894> inout not rejected as operand to assignment operator
830-
&x += y // expected-error {{'&' can only appear immediately in a call argument list}}}
830+
&x += y // expected-error {{'&' can only appear immediately in a call argument list}}
831831

832832
// <rdar://problem/23249098>
833833
func takeAny(_ x: Any) {}

0 commit comments

Comments
 (0)