Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking Issue for improving min_const_generics test coverage #78433

Closed
7 of 8 tasks
lcnr opened this issue Oct 27, 2020 · 4 comments
Closed
7 of 8 tasks

Tracking Issue for improving min_const_generics test coverage #78433

lcnr opened this issue Oct 27, 2020 · 4 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@lcnr
Copy link
Contributor

lcnr commented Oct 27, 2020

This is a tracking issue for improving the test coverage of feature(min_const_generics).
We collected a collection of interesting test cases in a hackmd and
now have to design these tests and add them to the test suite.

Topics

For more info about these topics you can also look at the above hackmd which elaborates a bit more for some of them

  • rustdoc
    One interested fairly self-contained topic is probably rustdoc. We pretty much want tests for values 7, params N, associated consts fn test() -> [u8; Type::ASSOC] and more complex expressions (ConstKind::Unevaluated) 3 + 4 in different positions, like ret + arg impl trait, async fn ret + impl position, type alias, and defined inside of macros. Probably also nice to test reexports and usage of associated consts defined in a different crate. Consider also adding one test for a constant which contains a lot of weird stuff, like struct definitions and so on to see how that looks

  • default values for const parameters should error (claimed by @ethanboxx)
    I think this only needs one test which tries to use something like const N: usize = 23 and const N = 27: usize to check that the error is at least somewhat readable

  • ordering of const parameters (only after types)
    @JulianKnodt already added quite a few tests for that, so this is probably already good enough. At least I can't think of anything missing here

  • supertraits + dyn type upcasting with supertraits (claimed by @hameerabbasi, added in Add const generics tests for supertraits + dyn traits. #78478)
    This requires some knowledge about how all of this should work but is probably quite interesting to think about.

  • trait methods trait Foo { fn bar() }
    I think that's already fairly well tested thanks to the interaction with type dependent paths, at least I can't think of anything specific we are still missing here

  • associated type bounds
    hopefully one run-pass and one or two compile fail tests are enough here

  • invalid bool and char bit-pattern (claimed by @JulianKnodt)
    A test which tries to for example use 7 as a bool and 0xFF as a char

  • macros (claimed by @JulianKnodt, added in Add macro test for min-const-generics #78912)
    hopefully fairly self-contained, try defining a #[macro_export] macro inside of a const (which can be inside of async fn or impl trait or a repeat expression)


I think these are the more self-contained topics, for everything else it's probably easiest for someone - I expect and want this to be me - to just spend a few hours mixing stuff until it either breaks or there is a high confidence that it does not break.

@lcnr lcnr added C-enhancement Category: An issue proposing an enhancement or a PR with one. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC F-min_const_generics T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 27, 2020
@hameerabbasi
Copy link
Contributor

supertraits + dyn type upcasting with supertraits in #78478.

@lcnr
Copy link
Contributor Author

lcnr commented Nov 10, 2020

I ended up implementing pretty much everything that was still left over in #78916 so it does seem like my help request in #65819 was a bit unnecessary 😅

Thanks to everybody who both helped or wanted to help here, in case you are still interested in helping out in the future, feel free to join the #project-const-generics steam on zulip (https://rust-lang.zulipchat.com/#narrow/stream/260443-project-const-generics)

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Nov 12, 2020
extend const generics test suite

should implement most of rust-lang#78433, especially all parts of [the hackmd](https://hackmd.io/WnFmN4MjRCqAjGmYfYcu2A?view) which I did not explicitly mention in that issue.

r? `@varkor`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Nov 12, 2020
…ror, r=lcnr

Test default values for const parameters.

The last topic on rust-lang#78433

I originally intended to place these tests in a single file, however, due to them being parser errors that are fatal, they must be in separate files to be detected.

Thanks, `@lcnr` for mentoring me on this PR.

r? `@lcnr`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Nov 12, 2020
extend const generics test suite

should implement most of rust-lang#78433, especially all parts of [the hackmd](https://hackmd.io/WnFmN4MjRCqAjGmYfYcu2A?view) which I did not explicitly mention in that issue.

r? ``@varkor``
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Nov 12, 2020
…ror, r=lcnr

Test default values for const parameters.

The last topic on rust-lang#78433

I originally intended to place these tests in a single file, however, due to them being parser errors that are fatal, they must be in separate files to be detected.

Thanks, ``@lcnr`` for mentoring me on this PR.

r? ``@lcnr``
@alex
Copy link
Member

alex commented Nov 13, 2020

With the merger of #78960, is this complete?

@lcnr
Copy link
Contributor Author

lcnr commented Nov 13, 2020

yes, it is 🎉

@lcnr lcnr closed this as completed Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants