Skip to content

Commit 6e9d5f5

Browse files
xedinxedin
authored
Merge pull request #17078 from xedin/rdar-40945329
[Sema] Diagnose misplaced InOutExpr in `preCheckExpression`
2 parents 0f2de23 + 3149e2d commit 6e9d5f5

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
@@ -1099,8 +1099,6 @@ ERROR(expected_operator_ref,none,
10991099
"expected operator name in operator reference", ())
11001100
ERROR(invalid_postfix_operator,none,
11011101
"operator with postfix spacing cannot start a subexpression", ())
1102-
ERROR(extraneous_amp_prefix,none,
1103-
"use of extraneous '&'", ())
11041102

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

‎include/swift/Parse/Parser.h

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

11941194
//===--------------------------------------------------------------------===//
11951195
// Expression Parsing
1196-
ParserResult<Expr> parseExpr(Diag<> ID, bool allowAmpPrefix = false) {
1197-
return parseExprImpl(ID, /*isExprBasic=*/false, allowAmpPrefix);
1196+
ParserResult<Expr> parseExpr(Diag<> ID) {
1197+
return parseExprImpl(ID, /*isExprBasic=*/false);
11981198
}
11991199
ParserResult<Expr> parseExprBasic(Diag<> ID) {
12001200
return parseExprImpl(ID, /*isExprBasic=*/true);
12011201
}
1202-
ParserResult<Expr> parseExprImpl(Diag<> ID, bool isExprBasic,
1203-
bool allowAmpPrefix = false);
1202+
ParserResult<Expr> parseExprImpl(Diag<> ID, bool isExprBasic);
12041203
ParserResult<Expr> parseExprIs();
12051204
ParserResult<Expr> parseExprAs();
12061205
ParserResult<Expr> parseExprArrow();
12071206
ParserResult<Expr> parseExprSequence(Diag<> ID,
12081207
bool isExprBasic,
1209-
bool isForConditionalDirective = false,
1210-
bool allowAmpPrefix = false);
1208+
bool isForConditionalDirective = false);
12111209
ParserResult<Expr> parseExprSequenceElement(Diag<> ID,
12121210
bool isExprBasic);
12131211
ParserResult<Expr> parseExprPostfixSuffix(ParserResult<Expr> inner,
@@ -1311,8 +1309,7 @@ class Parser {
13111309
SmallVectorImpl<SourceLoc> &exprLabelLocs,
13121310
SourceLoc &rightLoc,
13131311
Expr *&trailingClosure,
1314-
syntax::SyntaxKind Kind,
1315-
bool allowAmpPrefix = false);
1312+
syntax::SyntaxKind Kind);
13161313

13171314
ParserResult<Expr> parseTrailingClosure(SourceRange calleeRange);
13181315

‎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

@@ -923,8 +906,7 @@ ParserResult<Expr> Parser::parseExprSuper(bool isExprBasic) {
923906
indexArgLabelLocs,
924907
rSquareLoc,
925908
trailingClosure,
926-
SyntaxKind::FunctionCallArgumentList,
927-
/*allowAmpPrefix*/true);
909+
SyntaxKind::FunctionCallArgumentList);
928910
if (status.hasCodeCompletion())
929911
return makeParserCodeCompletionResult<Expr>();
930912
if (status.isError())
@@ -1272,8 +1254,7 @@ Parser::parseExprPostfixSuffix(ParserResult<Expr> Result, bool isExprBasic,
12721254
tok::l_square, tok::r_square,
12731255
/*isPostfix=*/true, isExprBasic, lSquareLoc, indexArgs,
12741256
indexArgLabels, indexArgLabelLocs, rSquareLoc, trailingClosure,
1275-
SyntaxKind::FunctionCallArgumentList,
1276-
/*allowAmpPrefix*/true);
1257+
SyntaxKind::FunctionCallArgumentList);
12771258
if (status.hasCodeCompletion())
12781259
return makeParserCodeCompletionResult<Expr>();
12791260
if (status.isError() || Result.isNull())
@@ -1709,8 +1690,7 @@ ParserResult<Expr> Parser::parseExprPrimary(Diag<> ID, bool isExprBasic) {
17091690
argLabelLocs,
17101691
rParenLoc,
17111692
trailingClosure,
1712-
SyntaxKind::FunctionCallArgumentList,
1713-
/*allowAmpPrefix*/true);
1693+
SyntaxKind::FunctionCallArgumentList);
17141694
if (status.isError())
17151695
return nullptr;
17161696

@@ -2954,8 +2934,7 @@ ParserStatus Parser::parseExprList(tok leftTok, tok rightTok,
29542934
SmallVectorImpl<SourceLoc> &exprLabelLocs,
29552935
SourceLoc &rightLoc,
29562936
Expr *&trailingClosure,
2957-
SyntaxKind Kind,
2958-
bool allowAmpPrefix) {
2937+
SyntaxKind Kind) {
29592938
trailingClosure = nullptr;
29602939

29612940
StructureMarkerRAII ParsingExprList(*this, Tok);
@@ -2992,8 +2971,7 @@ ParserStatus Parser::parseExprList(tok leftTok, tok rightTok,
29922971
DeclRefKind::Ordinary,
29932972
DeclNameLoc(Loc));
29942973
} else {
2995-
auto ParsedSubExpr = parseExpr(diag::expected_expr_in_expr_list,
2996-
/*allowAmpPrefix*/allowAmpPrefix);
2974+
auto ParsedSubExpr = parseExpr(diag::expected_expr_in_expr_list);
29972975
SubExpr = ParsedSubExpr.getPtrOrNull();
29982976
Status = ParsedSubExpr;
29992977
}
@@ -3237,8 +3215,7 @@ Parser::parseExprCallSuffix(ParserResult<Expr> fn, bool isExprBasic) {
32373215
argLabelLocs,
32383216
rParenLoc,
32393217
trailingClosure,
3240-
SyntaxKind::FunctionCallArgumentList,
3241-
/*allowAmpPrefix*/true);
3218+
SyntaxKind::FunctionCallArgumentList);
32423219

32433220
// Form the call.
32443221
auto Result = makeParserResult(status | fn,

‎lib/Sema/TypeCheckConstraints.cpp

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -858,6 +858,8 @@ namespace {
858858
TypeChecker &TC;
859859
DeclContext *DC;
860860

861+
Expr *ParentExpr;
862+
861863
/// A stack of expressions being walked, used to determine where to
862864
/// insert RebindSelfInConstructorExpr nodes.
863865
llvm::SmallVector<Expr *, 8> ExprStack;
@@ -886,7 +888,8 @@ namespace {
886888
void resolveKeyPathExpr(KeyPathExpr *KPE);
887889

888890
public:
889-
PreCheckExpression(TypeChecker &tc, DeclContext *dc) : TC(tc), DC(dc) { }
891+
PreCheckExpression(TypeChecker &tc, DeclContext *dc, Expr *parent)
892+
: TC(tc), DC(dc), ParentExpr(parent) {}
890893

891894
bool walkToClosureExprPre(ClosureExpr *expr);
892895

@@ -947,6 +950,43 @@ namespace {
947950
return finish(true, expr);
948951
}
949952

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

@@ -1683,7 +1723,7 @@ CleanupIllFormedExpressionRAII::~CleanupIllFormedExpressionRAII() {
16831723
/// Pre-check the expression, validating any types that occur in the
16841724
/// expression and folding sequence expressions.
16851725
bool TypeChecker::preCheckExpression(Expr *&expr, DeclContext *dc) {
1686-
PreCheckExpression preCheck(*this, dc);
1726+
PreCheckExpression preCheck(*this, dc, expr);
16871727
// Perform the pre-check.
16881728
if (auto result = expr->walk(preCheck)) {
16891729
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)