[Parse] Split prefix operators from regex in the lexer

hamishknight · hamishknight · commit ba28b6a19b01 · 2022-04-29T10:53:56.000+01:00
Teach the lexer not to consider `/` an operator
character when attempting to re-lex a regex
literal. This allows us to split off a prefix
operator.

Previously this was done after-the-fact in the
parser, but that didn't cover the unapplied infix
operator case, and didn't form a `tok::amp_prefix`
for `foo(&amp;/.../)`, which led to a suboptimal
diagnostic.

This also now means we'll split an operator for
cases such as `foo(!/^/)` rather than treating it
as an unapplied infix operator.

rdar://92469917
diff --git a/include/swift/Parse/Lexer.h b/include/swift/Parse/Lexer.h
@@ -565,10 +565,17 @@ class Lexer {
     void operator=(const SILBodyRAII&) = delete;
   };
 
-  /// Attempt to re-lex a regex literal with forward slashes `/.../` from a
-  /// given lexing state. If \p mustBeRegex is set to true, a regex literal will
-  /// always be lexed. Otherwise, it will not be lexed if it may be ambiguous.
-  void tryLexForwardSlashRegexLiteralFrom(State S, bool mustBeRegex);
+  /// A RAII object for switching the lexer into forward slash regex `/.../`
+  /// lexing mode.
+  class ForwardSlashRegexRAII final {
+    llvm::SaveAndRestore<LexerForwardSlashRegexMode> Scope;
+
+  public:
+    ForwardSlashRegexRAII(Lexer &L, bool MustBeRegex)
+        : Scope(L.ForwardSlashRegexMode,
+                MustBeRegex ? LexerForwardSlashRegexMode::Always
+                            : LexerForwardSlashRegexMode::Tentative) {}
+  };
 
 private:
   /// Nul character meaning kind.
diff --git a/include/swift/Parse/Parser.h b/include/swift/Parse/Parser.h
@@ -1758,7 +1758,6 @@ class Parser {
   ParserResult<Expr>
   parseExprPoundCodeCompletion(Optional<StmtKind> ParentKind);
 
-  UnresolvedDeclRefExpr *makeExprOperator(const Token &opToken);
   UnresolvedDeclRefExpr *parseExprOperator();
 
   /// Try re-lex a '/' operator character as a regex literal. This should be
diff --git a/lib/Parse/Lexer.cpp b/lib/Parse/Lexer.cpp
@@ -815,6 +815,13 @@ void Lexer::lexOperatorIdentifier() {
         rangeContainsPlaceholderEnd(CurPtr + 2, BufferEnd)) {
       break;
     }
+
+    // If we are lexing a `/.../` regex literal, we don't consider `/` to be an
+    // operator character.
+    if (ForwardSlashRegexMode != LexerForwardSlashRegexMode::None &&
+        *CurPtr == '/') {
+      break;
+    }
   } while (advanceIfValidContinuationOfOperator(CurPtr, BufferEnd));
 
   if (CurPtr-TokStart > 2) {
@@ -2080,15 +2087,6 @@ bool Lexer::tryLexRegexLiteral(const char *TokStart) {
   return true;
 }
 
-void Lexer::tryLexForwardSlashRegexLiteralFrom(State S, bool mustBeRegex) {
-  // Try re-lex with forward slash enabled.
-  llvm::SaveAndRestore<LexerForwardSlashRegexMode> RegexLexingScope(
-      ForwardSlashRegexMode, mustBeRegex
-                                 ? LexerForwardSlashRegexMode::Always
-                                 : LexerForwardSlashRegexMode::Tentative);
-  restoreState(S, /*enableDiagnostics*/ true);
-}
-
 /// lexEscapedIdentifier:
 ///   identifier ::= '`' identifier '`'
 ///
diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp
@@ -546,19 +546,6 @@ ParserResult<Expr> Parser::parseExprUnary(Diag<> Message, bool isExprBasic) {
     break;
   }
   case tok::oper_prefix: {
-    // Check to see if we can split a prefix operator containing `/`, e.g `!/`,
-    // which might be a prefix operator on a regex literal.
-    if (Context.LangOpts.EnableBareSlashRegexLiterals) {
-      auto slashIdx = Tok.getText().find("/");
-      if (slashIdx != StringRef::npos) {
-        auto prefix = Tok.getText().take_front(slashIdx);
-        if (!prefix.empty()) {
-          Operator = makeExprOperator({Tok.getKind(), prefix});
-          consumeStartingCharacterOfCurrentToken(Tok.getKind(), prefix.size());
-          break;
-        }
-      }
-    }
     Operator = parseExprOperator();
     break;
   }
@@ -880,45 +867,65 @@ static DeclRefKind getDeclRefKindForOperator(tok kind) {
   }
 }
 
-UnresolvedDeclRefExpr *Parser::makeExprOperator(const Token &Tok) {
+/// parseExprOperator - Parse an operator reference expression.  These
+/// are not "proper" expressions; they can only appear in binary/unary
+/// operators.
+UnresolvedDeclRefExpr *Parser::parseExprOperator() {
   assert(Tok.isAnyOperator());
   DeclRefKind refKind = getDeclRefKindForOperator(Tok.getKind());
   SourceLoc loc = Tok.getLoc();
   DeclNameRef name(Context.getIdentifier(Tok.getText()));
+  consumeToken();
   // Bypass local lookup.
   return new (Context) UnresolvedDeclRefExpr(name, refKind, DeclNameLoc(loc));
 }
 
-/// parseExprOperator - Parse an operator reference expression.  These
-/// are not "proper" expressions; they can only appear in binary/unary
-/// operators.
-UnresolvedDeclRefExpr *Parser::parseExprOperator() {
-  auto *op = makeExprOperator(Tok);
-  consumeToken();
-  return op;
-}
-
 void Parser::tryLexRegexLiteral(bool mustBeRegex) {
   if (!Context.LangOpts.EnableBareSlashRegexLiterals)
     return;
 
-  // Check to see if we have the start of a regex literal `/.../`.
+  // Check to see if we have a regex literal `/.../`, optionally with a prefix
+  // operator e.g `!/.../`.
   switch (Tok.getKind()) {
   case tok::oper_prefix:
   case tok::oper_binary_spaced:
   case tok::oper_binary_unspaced: {
-    if (!Tok.getText().startswith("/"))
+    // Check to see if we have an operator containing '/'.
+    auto slashIdx = Tok.getText().find("/");
+    if (slashIdx == StringRef::npos)
       break;
 
-    // Try re-lex as a `/.../` regex literal.
-    auto state = getParserPosition().LS;
-    L->tryLexForwardSlashRegexLiteralFrom(state, mustBeRegex);
-
-    // Discard the current token, which will be replaced by the re-lexed token,
-    // which may or may not be a regex literal token.
-    discardToken();
+    CancellableBacktrackingScope backtrack(*this);
+    {
+      Optional<Lexer::ForwardSlashRegexRAII> regexScope;
+      regexScope.emplace(*L, mustBeRegex);
+
+      // Try re-lex as a `/.../` regex literal, this will split an operator if
+      // necessary.
+      L->restoreState(getParserPosition().LS, /*enableDiagnostics*/ true);
+
+      // If we didn't split a prefix operator, reset the regex lexing scope.
+      // Otherwise, we want to keep it in place for the next token.
+      auto didSplit = L->peekNextToken().getLength() == slashIdx;
+      if (!didSplit)
+        regexScope.reset();
+
+      // Discard the current token, which will be replaced by the re-lexed
+      // token, which will either be a regex literal token, a prefix operator,
+      // or the original unchanged token.
+      discardToken();
+
+      // If we split a prefix operator from the regex literal, and are not sure
+      // whether this should be a regex, backtrack if we didn't end up lexing a
+      // regex literal.
+      if (didSplit && !mustBeRegex &&
+          !L->peekNextToken().is(tok::regex_literal)) {
+        return;
+      }
 
-    assert(Tok.getText().startswith("/"));
+      // Otherwise, accept the result.
+      backtrack.cancelBacktrack();
+    }
     break;
   }
   default:
diff --git a/test/StringProcessing/Parse/forward-slash-regex.swift b/test/StringProcessing/Parse/forward-slash-regex.swift
@@ -35,18 +35,23 @@ _ = /x/.self
 _ = /\//
 _ = /\\/
 
-// These unfortunately become infix `=/`. We could likely improve the diagnostic
-// though.
-let z=/0/
-// expected-error@-1 {{type annotation missing in pattern}}
-// expected-error@-2 {{consecutive statements on a line must be separated by ';'}}
-// expected-error@-3 {{expected expression after unary operator}}
-// expected-error@-4 {{cannot find operator '=/' in scope}}
-// expected-error@-5 {{'/' is not a postfix unary operator}}
-_=/0/
-// expected-error@-1 {{'_' can only appear in a pattern or on the left side of an assignment}}
-// expected-error@-2 {{cannot find operator '=/' in scope}}
-// expected-error@-3 {{'/' is not a postfix unary operator}}
+// This is just here to appease typo correction.
+let y = 0
+
+// These unfortunately become prefix `=` and infix `=/` respectively. We could
+// likely improve the diagnostic though.
+do {
+  let z=/0/
+  // expected-error@-1 {{type annotation missing in pattern}}
+  // expected-error@-2 {{consecutive statements on a line must be separated by ';'}}
+  // expected-error@-3 {{expected expression}}
+}
+do {
+  _=/0/
+  // expected-error@-1 {{'_' can only appear in a pattern or on the left side of an assignment}}
+  // expected-error@-2 {{cannot find operator '=/' in scope}}
+  // expected-error@-3 {{'/' is not a postfix unary operator}}
+}
 
 _ = /x
 // expected-error@-1 {{unterminated regex literal}}
@@ -58,9 +63,11 @@ _ = (!/x/)
 // expected-error@-1 {{cannot convert value of type 'Regex<Substring>' to expected argument type 'Bool'}}
 
 _ = !/ /
-// expected-error@-1 {{unary operator cannot be separated from its operand}}
-// expected-error@-2 {{cannot find operator '!/' in scope}}
-// expected-error@-3 {{unterminated regex literal}}
+// expected-error@-1 {{regex literal may not start with space; add backslash to escape}}
+// expected-error@-2 {{cannot convert value of type 'Regex<Substring>' to expected argument type 'Bool'}}
+
+_ = !!/ /
+// expected-error@-1 {{regex literal may not start with space; add backslash to escape}}
 
 _ = !!/x/
 _ = (!!/x/)
@@ -75,7 +82,6 @@ _ = /x/ + /y/ // expected-error {{binary operator '+' cannot be applied to two '
 _ = /x/+/y/
 // expected-error@-1 {{cannot find operator '+/' in scope}}
 // expected-error@-2 {{'/' is not a postfix unary operator}}
-// expected-error@-3 {{cannot find 'y' in scope}}
 
 _ = /x/?.blah
 // expected-error@-1 {{cannot use optional chaining on non-optional value of type 'Regex<Substring>'}}
@@ -102,7 +108,6 @@ _ = /x/ ... /y/ // expected-error {{referencing operator function '...' on 'Comp
 _ = /x/.../y/
 // expected-error@-1 {{missing whitespace between '...' and '/' operators}}
 // expected-error@-2 {{'/' is not a postfix unary operator}}
-// expected-error@-3 {{cannot find 'y' in scope}}
 
 _ = /x /...
 // expected-error@-1 {{unary operator '...' cannot be applied to an operand of type 'Regex<Substring>'}}
@@ -120,12 +125,7 @@ func foo<T>(_ x: T, y: T) {}
 foo(/abc/, y: /abc /)
 
 func bar<T>(_ x: inout T) {}
-
-// TODO: We split this into a prefix '&', but inout is handled specially when
-// parsing an argument list. This shouldn't matter anyway, but we should at
-// least have a custom diagnostic.
-bar(&/x/)
-// expected-error@-1 {{'&' is not a prefix unary operator}}
+bar(&/x/) // expected-error {{cannot pass immutable value as inout argument: literals are not mutable}}
 
 struct S {
   subscript(x: Regex<Substring>) -> Void { () }
@@ -259,7 +259,7 @@ _ = /x/*comment*/
 // expected-error@-1 {{unterminated regex literal}}
 
 // These become regex literals, unless surrounded in parens.
-func baz(_ x: (Int, Int) -> Int, _ y: (Int, Int) -> Int) {} // expected-note 3{{'baz' declared here}}
+func baz(_ x: (Int, Int) -> Int, _ y: (Int, Int) -> Int) {} // expected-note 4{{'baz' declared here}}
 baz(/, /)
 // expected-error@-1 {{cannot convert value of type 'Regex<Substring>' to expected argument type '(Int, Int) -> Int'}}
 // expected-error@-2 {{missing argument for parameter #2 in call}}
@@ -278,8 +278,7 @@ do {
   // expected-note@-2 {{to match this opening '('}}
 } // expected-error {{expected ')' in expression list}}
 
-// TODO: Should we do prefix operator splitting here?
-baz(^^/, /)
+baz(^^/, /) // expected-error {{missing argument for parameter #2 in call}}
 baz((^^/), /)
 
 func bazbaz(_ x: (Int, Int) -> Int, _ y: Int) {}
@@ -305,8 +304,9 @@ quxqux(/^/) // expected-error {{cannot convert value of type 'Regex<Substring>'
 quxqux((/^/)) // expected-error {{cannot convert value of type 'Regex<Substring>' to expected argument type '(Int, Int) -> Int'}}
 quxqux({ $0 /^/ $1 })
 
-// FIXME: We should be able to do operator splitting here and form `!(/^/)`.
-quxqux(!/^/) // expected-error {{unary operators must not be juxtaposed; parenthesize inner expression}}
+quxqux(!/^/)
+// expected-error@-1 {{cannot convert value of type 'Bool' to expected argument type '(Int, Int) -> Int'}}
+// expected-error@-2 {{cannot convert value of type 'Regex<Substring>' to expected argument type 'Bool'}}
 
 quxqux(/^)
 
