Skip to content
New issue

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

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

Already on GitHub? # to your account

Error when #[doc(alias)] has same name as the item #80686

Merged
merged 2 commits into from
Jan 5, 2021

Conversation

GuillaumeGomez
Copy link
Member

Something I came across when reviewing some doc alias PRs.

r? @jyn514

@GuillaumeGomez GuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jan 4, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 4, 2021

#![crate_type = "lib"]

#[doc(alias = "Foo")] //~ ERROR
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I didn't realize this runs in rustc_attr itself. It shouldn't be changed here, but I wonder if it makes sense to run those checks in rustdoc instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's scheduled for later. :)

@GuillaumeGomez GuillaumeGomez force-pushed the error-doc-alias-same-name branch from 0ecf7da to 9714ac0 Compare January 4, 2021 15:31
@GuillaumeGomez
Copy link
Member Author

Updated!

@jyn514
Copy link
Member

jyn514 commented Jan 5, 2021

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 5, 2021

📌 Commit 9714ac0 has been approved by jyn514

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 5, 2021
@bors
Copy link
Collaborator

bors commented Jan 5, 2021

⌛ Testing commit 9714ac0 with merge f4b9d32...

@bors
Copy link
Collaborator

bors commented Jan 5, 2021

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing f4b9d32 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 5, 2021
@bors bors merged commit f4b9d32 into rust-lang:master Jan 5, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 5, 2021
@GuillaumeGomez GuillaumeGomez deleted the error-doc-alias-same-name branch January 5, 2021 12:30
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 6, 2021
…as-feature, r=jyn514

Remove useless doc_alias feature gate

As `@jyn514`  rightfully noted in rust-lang#80686 , this feature is no more. Therefore, cleanup time!

r? `@jyn514`
@@ -358,6 +358,17 @@ impl CheckAttrVisitor<'tcx> {
.emit();
return false;
}
let item_name = self.tcx.hir().name(hir_id);
if item_name.to_string() == doc_alias {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it really need to_string to compare between an Symbol and a string?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're absolutely right, I opened #80750 to fix it.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jan 7, 2021
…=lzutao

Don't use to_string on Symbol in rustc_passes/check_attr.rs

Improve code from rust-lang#80686.

r? `@lzutao`
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jan 7, 2021
…=lzutao

Don't use to_string on Symbol in rustc_passes/check_attr.rs

Improve code from rust-lang#80686.

r? ``@lzutao``
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants