extend const generics test suite #78916
Merged
Conversation
This file contains 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
rust-highfive
added
the
S-waiting-on-review
lcnr
force-pushed
the
const-generics-tests
branch
2 times, most recently
from
7bfa78e
to
cc3bea2
Compare
lcnr
commented
Nov 10, 2020
| let _: foo!({{ N }}); //[min]~ ERROR generic parameters may not | ||
| let _: bar!(N); | ||
| let _: bar!({ N }); //[min]~ ERROR generic parameters may not | ||
| let _: baz!(N); //~ ERROR expressions must be enclosed in braces |
There was a problem hiding this comment.
that one's surprising, Foo<N> should be allowed but apparently doesn't work inside of macros
There was a problem hiding this comment.
do you or @petrochenkov have an idea on where this might go wrong?
That's not really something I am familiar with
There was a problem hiding this comment.
I believe the issue is here:
expr_is_valid_const_arg returns false for an identifier like N, because we usually simply parse identifiers as type arguments instead, and then resolve them to const arguments later. (The comment at the top of handle_unambiguous_unbraced_const_arg is misleading and should probably be changed: it doesn't just handle an error code, it is also for parsing valid const arguments that are not surrounded in braces.) I think if you add the identifier case to expr_is_valid_const_arg, this might work as expected. I'm not quite sure off the top of my head why this is the only place we're hitting this issue, though, or why there's not an ambiguity issue with type arguments.
We can do this as a follow up, though.
lcnr
force-pushed
the
const-generics-tests
branch
from
cc3bea2
to
a8310e2
Compare
8 tasks
lcnr
force-pushed
the
const-generics-tests
branch
from
2f44b25
to
f1e456f
Compare
lcnr
commented
Nov 10, 2020
| let _: Foo<{ baz::<'a>(&()) }>; //~ ERROR a non-static lifetime | ||
| let _: Foo<{ faz::<'b>(&()) }>; //~ ERROR a non-static lifetime | ||
| let _: Foo<{ baz::<'b>(&()) }>; //~ ERROR a non-static lifetime | ||
| let _ = Foo::<{ foo::<T>() }>; //~ ERROR generic parameters may not |
There was a problem hiding this comment.
looks like this causes both the github and the vs code code highligher to break, what would be a good way to notify the relevant people here?
I personally don't think that this is blocking though.
There was a problem hiding this comment.
GitHub and Atom use https://github.com/zargony/atom-language-rust. VS Code's Rust syntax is here It would be good to also check how it looks in CodeMirror.
lcnr
force-pushed
the
const-generics-tests
branch
from
f1e456f
to
4ff95f7
Compare
jyn514
added
A-const-generics
jyn514
reviewed
Nov 10, 2020
| // 'pub trait Trait<const N: usize>' | ||
| // @has - '//*[@id="impl-Trait%3C1_usize%3E-for-u8"]//code' 'impl Trait<1_usize> for u8' | ||
| // @has - '//*[@id="impl-Trait%3C2_usize%3E-for-u8"]//code' 'impl Trait<2_usize> for u8' | ||
| // @has - '//*[@id="impl-Trait%3CN%3E-for-%5Bu8%3B%20N%5D"]//code' \ |
There was a problem hiding this comment.
😢 rustdoc really needs a better naming scheme for impls
Comment on lines
+76
to
+79
| // 'pub fn concrete() -> [u8; 22]' | ||
| pub fn concrete() -> [u8; 3 + std::mem::size_of::<u64>() << 1] { |
lcnr
force-pushed
the
const-generics-tests
branch
from
76c103a
to
0e25fbd
Compare
jyn514
reviewed
Nov 10, 2020
| loop {} | ||
| } | ||
|
|
||
| /// Document all the functions |
jyn514
approved these changes
Nov 10, 2020
lcnr
force-pushed
the
const-generics-tests
branch
from
0e25fbd
to
a9eacf3
Compare
|
Thanks, these look great. @bors r+ rollup |
|
📌 Commit a9eacf3 has been approved by |
bors
added
S-waiting-on-bors
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this pull request
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`
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Nov 12, 2020
…laumeGomez Rollup of 5 pull requests Successful merges: - rust-lang#78916 (extend const generics test suite) - rust-lang#78921 (Improve the page title switch handling between search and doc) - rust-lang#78933 (Don't print thread ids and names in `tracing` logs) - rust-lang#78960 (Test default values for const parameters.) - rust-lang#78971 (Update books) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
16 tasks
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.
should implement most of #78433, especially all parts of the hackmd which I did not explicitly mention in that issue.
r? @varkor