Tune precedence of type operators #4920
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
`argText` uses `toTextGlobal` to change priority; instead, call `toText(arg)` and leave the priority up to the caller. - call-sites where `GlobalPrec` makes sense use now `argsText`. - to ensure this is a refactoring, other call-sites now use `atPrec(GlobalPrec)` explicitly; all those calls will be corrected in subsequent commits.
Using `atPrec(GlobalPrec)` ensures that `argText(args.head)` never adds parentheses around its output, but that's inappropriate for a function type `T1 => T2`. The nested calls to `atPrec` make no sense, but `argText` hid that before it was refactored.
Encapsulate the precedence rules in `parsing.precedence` while allowing for different precedence between term- and type-level operators, as mandated by the spec. This is just a refactoring, the actual priority changes are separate. * Add an `isType` flag to `parsing.precedence` and add constants for `AndTypePrec` and `OrTypePrec` (distinct from `OrPrec` and `AndPrec`, which is now unused and dropped). * In the parser, consider `isType` to decide priority. * In the pretty-printer, use `AndTypePrec` and `OrTypePrec` where appropriate.
While the operators should be associative, they aren't actually guaranteed to be, so correct pretty-printing will help users. * PrinterTests: Add first test for TypeTree printing. * PrinterTests: disable colors, following ErrorMessagesTest.scala.
This way, printing an infix type need not inspect its arguments; that ensures we don't need to handle *all* the possible forms of types that need to be wrapped in parentheses. I did this purely as a refactoring, based on how pretty-printers are supposed to be written. But the new test case, combining infix type operators and builting connectives, didn't work with the old code. It also doesn't work in Scala 2: ```scala Welcome to Scala 2.12.6 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_172). Type in expressions for evaluation. Or try :help. scala> class &&[T,U] defined class $amp$amp scala> def foo: Int && Boolean = ??? foo: Int && Boolean scala> def foo: Int && Boolean with String = ??? foo: Int && Boolean with String scala> def foo: Int && (Boolean with String) = ??? foo: Int && Boolean with String scala> def foo: (Int && Boolean) with String = ??? foo: Int && Boolean with String ```
* RefinedPrinter: Handle associativity mismatches for Type operators. * Add Documentation/tutorial on how to use atPrec and changePrec correctly, since so much code got it wrong and it takes a while to get it. * Extend tests for Type printing.
Refines 8b941d6: classSymbol is unnecessarily aggressive here.
Unlike others (which here follow Scala 2 precedence), they should follow the term-level precedence.
|
SIP-33 mandates closing this, but #4072 is still in! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change the precedence of type operators to comply with the spec.
Only the last two commits are new relative to #4072, they offer different options for cherry-picking or squashing; each of them passes tests.
EDIT: just learned that our current precedence was already accepted as SIP-33!
https://docs.scala-lang.org/sips/priority-based-infix-type-precedence.html