Skip to content

[Parse] Do not parse generic arguments of a cast destination if the token is more than a < #60088

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

Closed
wants to merge 9 commits into from
Closed
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
8 changes: 6 additions & 2 deletions include/swift/Parse/Parser.h
Original file line number Diff line number Diff line change
@@ -1259,6 +1259,9 @@ class Parser {

/// Whether the type is for a closure attribute.
CustomAttribute,

/// Whether the type is a destination of a cast.
CastDestination,
};

ParserResult<TypeRepr> parseType();
@@ -1297,8 +1300,9 @@ class Parser {
/// positioned at '.f'.
/// - If there is no base type qualifier (e.g. when parsing just 'f'), returns
/// an empty parser error.
ParserResult<TypeRepr> parseTypeIdentifier(
bool isParsingQualifiedDeclBaseType = false);
ParserResult<TypeRepr>
parseTypeIdentifier(bool isParsingQualifiedDeclBaseType = false,
ParseTypeReason reason = ParseTypeReason::Unspecified);
ParserResult<TypeRepr> parseOldStyleProtocolComposition();
ParserResult<TypeRepr> parseAnyType();
ParserResult<TypeRepr> parseSILBoxType(GenericParamList *generics,
6 changes: 4 additions & 2 deletions lib/Parse/ParseExpr.cpp
Original file line number Diff line number Diff line change
@@ -74,7 +74,8 @@ ParserResult<Expr> Parser::parseExprImpl(Diag<> Message,
ParserResult<Expr> Parser::parseExprIs() {
SourceLoc isLoc = consumeToken(tok::kw_is);

ParserResult<TypeRepr> type = parseType(diag::expected_type_after_is);
ParserResult<TypeRepr> type =
parseType(diag::expected_type_after_is, ParseTypeReason::CastDestination);
if (type.hasCodeCompletion())
return makeParserCodeCompletionResult<Expr>();
if (type.isNull())
@@ -101,7 +102,8 @@ ParserResult<Expr> Parser::parseExprAs() {
exclaimLoc = consumeToken(tok::exclaim_postfix);
}

ParserResult<TypeRepr> type = parseType(diag::expected_type_after_as);
ParserResult<TypeRepr> type =
parseType(diag::expected_type_after_as, ParseTypeReason::CastDestination);

if (type.hasCodeCompletion())
return makeParserCodeCompletionResult<Expr>();
24 changes: 16 additions & 8 deletions lib/Parse/ParseType.cpp
Original file line number Diff line number Diff line change
@@ -14,16 +14,16 @@
//
//===----------------------------------------------------------------------===//

#include "swift/Parse/Parser.h"
#include "swift/AST/ASTWalker.h"
#include "swift/AST/Attr.h"
#include "swift/AST/GenericParamList.h"
#include "swift/AST/TypeRepr.h"
#include "swift/Parse/Lexer.h"
#include "swift/Parse/CodeCompletionCallbacks.h"
#include "swift/Parse/SyntaxParsingContext.h"
#include "swift/Parse/Lexer.h"
#include "swift/Parse/ParsedSyntaxBuilders.h"
#include "swift/Parse/ParsedSyntaxRecorder.h"
#include "swift/Parse/Parser.h"
#include "swift/Parse/SyntaxParsingContext.h"
#include "llvm/ADT/APInt.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/Twine.h"
@@ -180,7 +180,7 @@ ParserResult<TypeRepr> Parser::parseTypeSimple(
case tok::kw_Self:
case tok::kw_Any:
case tok::identifier: {
ty = parseTypeIdentifier();
ty = parseTypeIdentifier(/*isParsingQualifiedDeclBaseType=*/false, reason);
break;
}
case tok::l_paren:
@@ -679,7 +679,8 @@ ParserStatus Parser::parseGenericArguments(SmallVectorImpl<TypeRepr *> &Args,
/// identifier generic-args? ('.' identifier generic-args?)*
///
ParserResult<TypeRepr>
Parser::parseTypeIdentifier(bool isParsingQualifiedDeclBaseType) {
Parser::parseTypeIdentifier(bool isParsingQualifiedDeclBaseType,
ParseTypeReason reason) {
// If parsing a qualified declaration name, return error if base type cannot
// be parsed.
if (isParsingQualifiedDeclBaseType && !canParseBaseTypeForQualifiedDeclName())
@@ -722,9 +723,16 @@ Parser::parseTypeIdentifier(bool isParsingQualifiedDeclBaseType) {
SourceLoc LAngle, RAngle;
SmallVector<TypeRepr*, 8> GenericArgs;
if (startsWithLess(Tok)) {
auto genericArgsStatus = parseGenericArguments(GenericArgs, LAngle, RAngle);
if (genericArgsStatus.isErrorOrHasCompletion())
return genericArgsStatus;
// Only attempt to parse a generic argument list in a cast destination
// type if the token text is just "<", because it can be an operator,
// for example: "1 as Int16 << 7".
if (Tok.getText().equals("<") ||
reason != ParseTypeReason::CastDestination) {
Comment on lines 725 to +730
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we just use canParseAsGenericArgumentList() instead of startsWithLess(Tok) like expression parsing?
It only accepts single <, but I'm not sure there's a place we need to accept << for a generic arguments in the first place. If we cannot use canParseAsGenericArgumentList(), could you add some code-comment to explain why not?

Copy link
Member

@rintaro rintaro Jul 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see we don't use canParseAsGenericArgumentList() probably because of the SIL type parsing which has generic function types like <T> (T) -> Void. And for non-expression position, we probably want to diagnose errors inside <...> as a generic argument error, but not as a bogus random tokens after a type. E.g.

let a: Dictionary<String: Int>

The error in <String: Int> should be diagnosed as like expected ',' instead of ':' inside parseGenericArguments(). But if we used canParseAsGenericArgumentList() here, it would be parsed as a random tokens after let a: Dictionary.

So I think having ParseTypeReason::CastDestination actually makes sense. And we can use canParseAsGenericArgumentList() only for CastDestination case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback!

auto genericArgsStatus =
parseGenericArguments(GenericArgs, LAngle, RAngle);
if (genericArgsStatus.isErrorOrHasCompletion())
return genericArgsStatus;
}
}
EndLoc = Loc.getEndLoc();

19 changes: 19 additions & 0 deletions test/Parse/generic_disambiguation.swift
Original file line number Diff line number Diff line change
@@ -76,3 +76,22 @@ A<B?>(x: 0) // parses as type // expected-warning{{unused}}
_ = a < b ? c : d

A<(B) throws -> D>(x: 0) // expected-warning{{unused}}

// https://github.com/apple/swift/issues/43053
do {
1 as! A<B>.F << 2 // expected-warning {{cast from 'Int' to unrelated type 'A<B>.F' always fails}}
// expected-error@-1 {{referencing operator function '<<' on 'BinaryInteger' requires that 'A<B>.F' conform to 'BinaryInteger'}}
1 as! A<B> << 2 // expected-warning {{cast from 'Int' to unrelated type 'A<B>' always fails}}
// expected-error@-1 {{referencing operator function '<<' on 'BinaryInteger' requires that 'A<B>' conform to 'BinaryInteger'}}
let _ = 1 as Int16 << 7
let _ = 1 as? Int16 << 7 // expected-error {{value of optional type 'Int16?' must be unwrapped to a value of type 'Int16'}}
// expected-warning@-1 {{conditional downcast from literal to 'Int16' always fails; consider using 'as' coercion}}
// expected-note@-2 {{coalesce using '??' to provide a default when the optional value contains 'nil'}}
// expected-note@-3 {{force-unwrap using '!' to abort execution if the optional value contains 'nil'}}
let _ = 1 is Int16 << 7 // expected-warning {{conditional downcast from literal to 'Int16' always fails; consider using 'as' coercion}}
// expected-error@-1 {{binary operator '<<' cannot be applied to operands of type 'Bool' and 'Int'}}
let _ = 1 as! Int16 << 7 // expected-warning {{conditional downcast from literal to 'Int16' always fails; consider using 'as' coercion}}
let _ = 1 as Int16 <= 7
// FIXME: this should not produce any errors.
let _ = 1 as Int16 < 7 // expected-error {{expected type}}
}