Disallow .~ in extended indexing operators.#2106
Conversation
|
(cc @Octachron) |
|
I am wondering if it would make sense to add |
|
That's an interesting idea. 5122818 makes |
|
@Octachron, @gasche: would either of you like to review this? (I realise that it needs a |
gasche
left a comment
There was a problem hiding this comment.
The implementation looks fine.
The feature looks very reasonable to me: it restricts a very-new feature that has apparently not been used in the wild yet (although it is part of a release OCaml version), I think the cost/benefit is reasonable.
@Octachron, extended operators are your expertise, so I will wait (not merge) to give you an opportunity to comment here. If you are against the change, I may have to reconsider.
|
You should document |
Octachron
left a comment
There was a problem hiding this comment.
I also think that the change is reasonable.
Another option might have be to just emit a DOTTILDE token if the dotop operator is.~ but removing ~ from the list of starting character for DOTOP is simpler. Moreover, it let MetaOcaml decides how to handle cases like .~!x .
Concerning the reserved error mechanism in the lexer, I have few remarks in term of explicitness, but it seems nice enough. (Initially, I was thinking of just importing the DOTTILDE token and the relevant lexer definition from the MetaOCaml patch; but the current implementation makes for a nicer error message. )
Done (02cf280) |
gasche
left a comment
There was a problem hiding this comment.
This looks good to me if the CI passes.
|
The check-typo CI failure:
looks spurious, since none of the |
|
This might be because of the recent changes to |
|
|
Disallow .~ in the lexer to preserve MetaOCaml compatibility.
* Fix problems with character literals in comments * Do not parse unclosed comments and quoted strings * Update known failures * Only allow line number directives with filename (ocaml/ocaml#931) * Rename dot_operator to indexing_operator * Disallow .~ in indexing operator (ocaml/ocaml#2106) * Add test for indexing operator * Support empty variants (ocaml/ocaml#1546) * Support binding operators (ocaml/ocaml#1947) * Use tree-sitter 0.14.0 * Cleanup trvis config
PR #1064 introduced support for user-defined indexing operators. The syntax is similar to the built-in operators (
.(),.[], etc.) but with symbol characters between the dot and the delimiters, like this:Unfortunately, allowing the sequence
.~is incompatible with MetaOCaml's longstanding splice syntax:.~expressionFortunately, there doesn't appear to be any code actually using
.~yet, so it's not too late to restore compatibility. The following table shows which user-defined indexing operators are currently in use by OCaml packages available via OPAM:!$%&?@This PR removes
~from the set of characters allowed in user-defined indexing operators, eliminating the incompatibility with MetaOCaml's syntax.