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

fix: resolve doc path from parent module if outer comments exist on module #19507

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Hmikihiro
Copy link
Contributor

@Hmikihiro Hmikihiro commented Apr 2, 2025

fix: #19506
#19471
スクリーンショット 2025-04-03 1 40 38

I set placement information about inner or outer in attribute.
When I check link on module, I search from parent module if attribute of doc item have a outer comment.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 2, 2025
@Hmikihiro Hmikihiro changed the title fix: resolve doc path if outer comments exist on module fix: resolve doc path from parent module if outer comments exist on module Apr 2, 2025
@Hmikihiro Hmikihiro force-pushed the fix_module_doc_links branch from 8035999 to 4db0156 Compare April 2, 2025 17:31
@Hmikihiro Hmikihiro force-pushed the fix_module_doc_links branch from 4db0156 to de69c73 Compare April 4, 2025 09:10
@@ -211,6 +215,15 @@ pub struct Attr {
pub path: Interned<ModPath>,
pub input: Option<Box<AttrInput>>,
pub ctxt: SyntaxContext,
pub place: AttrPlacement,
Copy link
Member

Choose a reason for hiding this comment

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

This grows attributes by 8 bytes which will be noticable, we'll need to avoid that somehow if we can. Note quite sure how yet. Probably by using one of the AttrId bits (we already do something there for cfg_attr but that is not really used so we can replace that with this likely). Will write oiut better instructions later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

impl AttrId {
const CFG_ATTR_BITS: usize = 7;
const AST_INDEX_MASK: usize = 0x00FF_FFFF;
const AST_INDEX_BITS: usize = Self::AST_INDEX_MASK.count_ones() as usize;
const CFG_ATTR_SET_BITS: u32 = 1 << 31;

Right, that means AttrId currently uses bits like this:

  • Bit 31 → is_cfg_attr
  • Bits 24–30 → cfg_attr_index (7 bits)
  • Bits 0–23 → ast_index (24 bits)

So, we could either remove or reduce the size of cfg_attr_index, and reuse the freed bit for placement information.

  • Bit 31 → inner_or_outer
  • Bits 24–30 → (unused)or (is_cfg_attr & cfg_attr_index(6bit))
  • Bits 0–23 → ast_index

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let mut it = it.clone();
it.id.id = (it.id.ast_index() as u32 + last_ast_index)
| ((it.id.cfg_attr_index().unwrap_or(0) as u32)

And cfg_attr was accessed through cfg_attr_index() but this function was only used to copy it when creating a new [Attr]. So would we remove cfg_attr?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inner comment links of module point out a different item with cargo doc, If the module have a outer comment.
3 participants