-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Clean up E0754 explanation #75626
Clean up E0754 explanation #75626
Conversation
``` | ||
|
||
```compile_fail,E0754 | ||
# #![feature(non_ascii_idents)] | ||
|
||
#[no_mangle] | ||
fn řųśť() {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not make it error
on the same line like ok
on the same line as below?
fn řųśť() {} | ||
// ^ error! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not make it error
on the same line like ok
on the same line as below?
fn řųśť() {} | |
// ^ error! | |
fn řųśť() {} // error! |
fn main() {} | ||
``` | ||
|
||
Non-ascii can be used as module names if it is inline | ||
or a #\[path\] attribute is specified. For example: | ||
Non-ascii can be used as module names if it is inline or a `#[path]` attribute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#[path]
attribute? What is that, should we do a link to that page?
Non-ascii can be used as module names if it is inline or a `#[path]` attribute | |
Non-ascii can be used as module names if it is inlined or a `#[path]` attribute |
Should it be "inlined" rather than "inline"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely! As for the path
attribute, it allows you to link to a file module by its system path and give it a different name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but should we link it to the reference to let the users know what is that? I feel like this is a less known item.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with external links is that they might change (for example in this case, this is: https://doc.rust-lang.org/reference/items/modules.html#the-path-attribute), even if they're part of the rust book.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But don't we have link check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only when referring to items inside the docs scope (so relative paths, not absolute ones).
Updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
@bors: r=pickfire rollup |
📌 Commit 4fa69cb has been approved by |
Rollup of 8 pull requests Successful merges: - rust-lang#75389 (attempt to improve span_label docs) - rust-lang#75392 (Add `as_uninit`-like methods to pointer types and unify documentation of `as_ref` methods) - rust-lang#75464 (Move to intra doc links for ascii.rs and panic.rs) - rust-lang#75578 (Allowing raw ptr dereference in const fn) - rust-lang#75613 (Add explanation for `&mut self` method call when expecting `-> Self`) - rust-lang#75626 (Clean up E0754 explanation) - rust-lang#75629 (Use intra-doc links in `std::env`, `std::alloc` and `std::error`) - rust-lang#75634 (Mark x86_64-linux-kernel as *) Failed merges: r? @ghost
r? @Dylan-DPC
cc @pickfire