Clean up operator representations

[nnethercote]

Proposal

Current situation

There are several lexer and AST types that represent operators: lex::TokenKind, ast::TokenKind, ast::ExprKind, ast::AssocOp, ast::BinOpToken, and ast::BinOpKind. There is a lot of conceptual overlap between these types, but there are also various inconsistencies and weird things about them. It feels like the six types were created by six people that don't like each other.

• lex::TokenKind and ast::TokenKind

  • Both of them use syntactic names (e.g. Star, Caret, DotDot), good.
  • ast::TokenKind uses BinOp/BinOpEq instead of individual variants, which is different to lex::TokenKind, yuk1.
  • lex::TokenKind uses Bang and ast::TokenKind uses Not, yuk2.

• BinOpToken

  • This is a misleading name. It's not all binary ops, just the assignment ops, which excludes &&/|| and comparisons, yuk3.

• BinOpKind

  • Uses semantic names (e.g. Mul, Rem, Range), good.

• ExprKind

  • ExprKind::Binary: all the BinOpKind variants are used, good.
  • ExprKind::AssignOp: only the assignable op BinOpKind variants are used, yuk4.
    • E.g. ExprKind::AssignOp(BinOpKind::Lt) is expressible but invalid.
    • E.g. see the impossible cases in lang_item_for_op.
    • There's a similar story for assignment ops in HIR and MIR/THIR.

• AssocOp

  • This is a cut-down version of ExprKind.
  • Both ExprKind and AssocOp have an AssignOp variant, good.
    • But AssocOp::AssignOp's uses BinOpToken, which uses syntactic names, instead of BinOpKind, which uses semantic names, yuk5.
  • ExprKind has a Binary variant, while AssocOp has 10 individual variants covering those ops, yuk6.
    • Those variant names don't match the BinOpKind names (e.g. Multiply instead of Mul, Modulus instead of Rem), yuk7.
  • AssocOp has DotDot/DotDotEq variants but ExprKind has Range, yuk8.
  • AssocOp has an As variant but ExprKind has Cast, yuk9

Proposed changes

I propose to fix the nine distinct "yuk"s in three steps.

1. Make AssocOp more like ExprKind

• In AssocOp::AssignOp, use BinOpKind instead of BinOpToken
  • Fixes yuk5
  • Makes yuk4 worse, extends it to AssocOp::AssignOp
    • See Step 3 below
• Introduce AssocOp::Binary
  • Fixes yuk7 and yuk6
• Replace AssocOp::DotDot{,Eq} with AssocOp::Range
  • Fixes yuk8
• Rename AssocOp::As as AssocOp::Cast
  • Fixes yuk9

2. Make ast::TokenKind more like lexer::TokenKind

• Replace ast::TokenKind::BinOp{,Eq} and remove BinOpToken.
  • Fixes yuk1, yuk3
• Rename ast::TokenKind::Not as ast::TokenKind::Bang.
  • Fixes yuk2

3. Tighten up assignment operator representations.

• Fixes yuk4, at some complexity. I am uncertain if this is worth it, which is why it's the last step.

The following table summarizes the proposed changes.

Legend:
- `1->` indicates things changing in step 1
- `2->` indicates things changing in step 2
- `3->` indicates things changing in step 3
- `1,3->` indicates things changing in step 1 and 3
--------------------------------------------------------------------------------------------------------------------------------------------|
    |           syntactic                          |               semantic                                                                 |
--------------------------------------------------------------------------------------------------------------------------------------------|
op  | lex::     ast::TokenKind/      2-> ast::     | ast::ExprKind/      3-> ast::ExprKind/    ast::AssocOp  -> ast::AssocOp/               |
    | TokenKind ast::BinOpToken(BT)      TokenKind | ast::BinOpKind(BK)      ast::BinOpKind/                    ast::BinOpKind(BK)/         |
    |                                              |                         ast::AssignOpKind(AK)              ast::AssignOpKind(AK)       |
    |           (old)                    (new)     | (old)                   (new)             (old)                                        |
--------------------------------------------------------------------------------------------------------------------------------------------|
+   | Plus      BinOp(BT::Plus)      2-> Plus      | Binary(BK::Add)                           Add          1-> Binary(BK::Add)             |
-   | Minus     BinOp(BT::Minus)     2-> Minus     | Binary(BK::Sub)                           Subtract     1-> Binary(BK::Sub)             |
*   | Star      BinOp(BT::Star)      2-> Star      | Binary(BK::Mul)                           Multiply     1-> Binary(BK::Mul)             |
/   | Slash     BinOp(BT::Slash)     2-> Slash     | Binary(BK::Div)                           Divide       1-> Binary(BK::Div)             |
%   | Percent   BinOp(BT::Percent)   2-> Percent   | Binary(BK::Rem)                           Modulus      1-> Binary(BK::Rem)             |
^   | Caret     BinOp(BT::Caret)     2-> Caret     | Binary(BK::BitXor)                        BitXor       1-> Binary(BK::BitXor)          |
&   | And       BinOp(BT::And)       2-> And       | Binary(BK::BitAnd)                        BitAnd       1-> Binary(BK::BitAnd)          |
|   | Or        BinOp(BT::Or)        2-> Or        | Binary(BK::BitOr)                         BitOr        1-> Binary(BK::BitOr)           |
<<  |           BinOp(BT::Shl)       2-> Shl       | Binary(BK::Shl)                           ShiftLeft    1-> Binary(BK::Shl)             |
>>  |           BinOp(BT::Shr)       2-> Shr       | Binary(BK::Shr)                           ShiftRight   1-> Binary(BK::Shr)             |
    |                                              |                                                                                        |
==  |           EqEq                               | Binary(BK::Eq)                            Equal        1-> Binary(BK::Eq)              |
<   | Lt        Lt                                 | Binary(BK::Lt)                            Less         1-> Binary(BK::Lt)              |
<=  |           Le                                 | Binary(BK::Le)                            LessEqual    1-> Binary(BK::Le)              |
!=  |           Ne                                 | Binary(BK::Ne)                            NotEqual     1-> Binary(BK::Ne)              |
>=  |           Ge                                 | Binary(BK::Ge)                            GreaterEqual 1-> Binary(BK::Ge)              |
>   | Gt        Gt                                 | Binary(BK::Gt)                            Greater      1-> Binary(BK::Gt)              |
    |                                              |                                                                                        |
&&  |           AndAnd                             | Binary(BK::And)                           LAnd         1-> Binary(BK::And)             |
||  |           OrOr                               | Binary(BK::Or)                            LOr          1-> Binary(BK::Or)              |
    |                                              |                                                                                        |
+=  |           BinOpEq(BT::Plus)    2-> PlusEq    | AssignOp(BK::Add    3-> AK::AddAssign)    AssignOp(BT::Plus    1,3-> AK::AddAssign)    |
-=  |           BinOpEq(BT::Minus)   2-> MinusEq   | AssignOp(BK::Sub    3-> AK::SubAssign)    AssignOp(BT::Minus   1,3-> AK::SubAssign)    |
*=  |           BinOpEq(BT::Star)    2-> StarEq    | AssignOp(BK::Mul    3-> AK::MulAssign)    AssignOp(BT::Star    1,3-> AK::MulAssign)    |
/=  |           BinOpEq(BT::Slash)   2-> SlashEq   | AssignOp(BK::Div    3-> AK::DivAssign)    AssignOp(BT::Slash   1,3-> AK::DivAssign)    |
%=  |           BinOpEq(BT::Percent) 2-> PercentEq | AssignOp(BK::Rem    3-> AK::RemAssign)    AssignOp(BT::Percent 1,3-> AK::RemAssign)    |
^=  |           BinOpEq(BT::Caret)   2-> CaretEq   | AssignOp(BK::BitXor 3-> AK::BitXorAssign) AssignOp(BT::Caret   1,3-> AK::BitXorAssign) |
&=  |           BinOpEq(BT::And)     2-> AndEq     | AssignOp(BK::BitAnd 3-> AK::BitandAssign) AssignOp(BT::And     1,3-> AK::BitAndAssign) |
|=  |           BinOpEq(BT::Or)      2-> OrEq      | AssignOp(BK::BitOr  3-> AK::BitOrAssign)  AssignOp(BT::Or      1,3-> AK::BitOrAssign)  |
<<= |           BinOpEq(BT::Shl)     2-> ShlEq     | AssignOp(BK::Shl    3-> AK::ShlAssign)    AssignOp(BT::Shl     1,3-> AK::ShlAssign)    |
>>= |           BinOpEq(BT::Shr)     2-> ShrEq     | AssignOp(BK::Shr    3-> AK::ShrAssign)    AssignOp(BT::Shr     1,3-> AK::ShrAssign)    |
    |                                              |                                                                                        |
=   | Eq        Eq                                 | Assign                                    Assign                                       |
    |                                              |                                                                                        |
as  | Ident     Ident                              | Cast                                      As           1-> Cast                        |
..  |           DotDot                             | Range(HalfOpen)                           DotDot       1-> Range(HalfOpen)             |
..= |           DotDotEq                           | Range(Closed)                             DotDotEq     1-> Range(Closed)               |
    |                                              |                                                                                        |
!   | Bang      Not                  2-> Bang      | Unary(UnOp::Not)                                                                       |
--------------------------------------------------------------------------------------------------------------------------------------------|

In summary: BinOpToken is removed entirely, almost every variant of AssocOp changes, a lot of ast::TokenKind variants change, one variant of ast::ExprKind changes (AssignOp), and lex::TokenKind and ast::BinOpToken are unchanged.

Mentors or Reviewers

I have draft code up at rust-lang/rust#134552. It's all in one PR, though I would split it into three PRs (the steps listed above) before merging.

Process

The main points of the Major Change Process are as follows:

• File an issue describing the proposal.
• A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
  • Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
  • Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
• Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

[rustbot]

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

Concerns or objections to the proposal should be discussed on Zulip and formally registered here by adding a comment with the following syntax:

@rfcbot concern reason-for-concern 
<description of the concern> 

Concerns can be lifted with:

@rfcbot resolve reason-for-concern 

See documentation at https://forge.rust-lang.org

cc @rust-lang/compiler

[estebank]

@rustbot second

[apiraino]

@rustbot label -final-comment-period +major-change-accepted