tests/ui/binop: add annotations for reference rules#153549
tests/ui/binop: add annotations for reference rules#153549DanielEScherzer wants to merge 1 commit intorust-lang:mainfrom
Conversation
| @@ -1,4 +1,5 @@ | |||
| //@ run-pass | |||
| //@ reference: patterns.literal.intro | |||
There was a problem hiding this comment.
This test looks like it is checking the parsing of 1-1 and negative literals in patterns. Perhaps also add patterns.literal.syntax and expr.arith-logic.syntax?
(In the long-term, I'd like to avoid using .intro rules since they are supposed to be introductions, and not specifications of behavior. But we're not there, yet.)
There was a problem hiding this comment.
Added patterns.literal.syntax, not sure about expr.arith-logic.syntax since this is mostly about the negative pattern and not the arithmetic syntax
about .intro rules - makes sense, but as long as they exist and are included in the test coverage summary I figured covering them would be useful
There was a problem hiding this comment.
Looking at the history of the file, the assert_eq!(1-1, 0) seems to be specifically saying does 1-1 work? Specifically #954 and f26e770 (which ultimately culminated in 7655b3c which removed lexing of negative literals).
There I believe they were dealing with the parsing of expr-expr where the parser was incorrectly handling negative literals as a 1 followed by a -1 literal.
There was a problem hiding this comment.
I didn't think to check the history, added expr.arith-logic.syntax too
| @@ -1,4 +1,5 @@ | |||
| //@ run-pass | |||
| //@ reference: expr.negate.results | |||
There was a problem hiding this comment.
It looks like this test is covering all operator overloading in general. I would expect to also include expr.arith-logic.behavior and expr.array.index.trait.
There was a problem hiding this comment.
For expr.arith-logic.behavior I didn't include that because it doesn't cover all of the rule cases, and I figured there is probably a better test to point to in another directory
similarly for expr.array.index.trait because this doesn't showcase a mutable reference I thought there would be a better test elsewhere
There was a problem hiding this comment.
Yea, this is part of the difficult of connecting the reference rules to the rustc test suite. I think the majority of the tests are not going to be a 100% coverage or connection to the rule.
We've talked a bunch about how to handle this, but have not settled on a better solution. For now I think we should just try to associate each test with whatever rule it is intending to cover, even if it isn't complete.
In the long term, we'll probably want to have some kind of hybrid approach where we write our own tests that very clearly and directly cover a reference rule. But we haven't settled on exactly how to do that.
There was a problem hiding this comment.
Writing our own tests for 100% coverage would make sense - e.g. I didn't add to https://github.com/rust-lang/rust/blob/2d76d9bc76f27b03b4899e72ce561c7ac2c5cf6b/tests/ui/binop/binops.rs because it didn't fully test any of the rules. I figured that once I had reviewed a majority of the ui directories I would maybe write tests for things that were not explicitly tested
ff77acd to
70db796
Compare
70db796 to
307cbc4
Compare
r? ehuss
@rustbot label +A-docs