Skip to content

Conversation

@Centri3
Copy link
Member

@Centri3 Centri3 commented Jul 12, 2023

Closes #9212

changelog: New lint [four_forward_slashes]

@rustbot
Copy link
Collaborator

rustbot commented Jul 12, 2023

r? @xFrednet

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 12, 2023
@xFrednet
Copy link
Contributor

r$ @blyxyas

(xFrednet has picked a reviewer for you, use r$ to override)

Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

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

I cannot stop thinking about rustc_ast::util::comments::gather_comments, it returns a Vec<Comment> and that could be used to check if the comment starts with // or //// (I'm not sure Comment also includes the initial //).

It would be simpler, but we'd have to do a benchmark to verify speed. Maybe SPEEDTEST can help with that?

@Centri3
Copy link
Member Author

Centri3 commented Jul 12, 2023

gather_comments may work, but from what I can see it'd do a lot more work for the same result. Even just allocating an empty Vec, then increasing its capacity every time a comment is found, would be slow.

Perhaps we can use the lexer though. I hadn't thought to and that may be nicer

@Centri3
Copy link
Member Author

Centri3 commented Jul 13, 2023

Unfortunately gather_comments won't work. It doesn't give the span of the comment, only the column it starts at (Comment.pos).

Make it trim the contents
@Centri3 Centri3 force-pushed the four_forward_slashes branch from 30d8cf9 to 1bca699 Compare July 13, 2023 00:43
@Centri3 Centri3 force-pushed the four_forward_slashes branch from 1bca699 to 2e43d0c Compare July 13, 2023 00:55
Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks! ❤️
cc @xFrednet

@xFrednet
Copy link
Contributor

I'm sadly busy until Tuesday/Wednesday, but then I'll do the last check. :)

@xFrednet
Copy link
Contributor

I have full trust in your review:

@bors r=blyxyas

@bors
Copy link
Contributor

bors commented Jul 18, 2023

📌 Commit 2e43d0c has been approved by blyxyas

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jul 18, 2023

⌛ Testing commit 2e43d0c with merge 9f0cbfd...

@bors
Copy link
Contributor

bors commented Jul 18, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: blyxyas
Pushing 9f0cbfd to master...

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.

Lint for 4 / (////) to help detect broken doc comments

5 participants