Skip to content

Conversation

@Fibonacci747
Copy link
Contributor

Summary

This change removes redundant work when formatting documentation signatures. The virtual file is now parsed only once using a local DiagnosticsBuilder, while keeping the same “allow a single parse error” behavior. The helper get_virtual_syntax_file_signature now takes &str instead of String, so we no longer clone signature strings just to intern them for the virtual file. Additionally, collect_green_nodes was simplified to fill a provided vector in place instead of returning a cloned copy, and its call sites were updated accordingly.


Type of change

Please check one:

  • Performance improvement

Why is this change needed?

These changes reduce unnecessary allocations and parsing overhead without affecting the observable behavior of location links.


@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

@orizi made 1 comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @Fibonacci747).


crates/cairo-lang-doc/src/location_links.rs line 34 at r1 (raw file):

/// Collects all [`cairo_lang_syntax::node::green::GreenNode`]s for a [`SyntaxNode`],
/// returns a vector of their [`SyntaxKind`] and text.
fn collect_green_nodes<'db>(

and fix doc.

Suggestion:

fn extend_green_nodes<'db>(

@Fibonacci747
Copy link
Contributor Author

@orizi made 1 comment.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @Fibonacci747).

crates/cairo-lang-doc/src/location_links.rs line 34 at r1 (raw file):

/// Collects all [`cairo_lang_syntax::node::green::GreenNode`]s for a [`SyntaxNode`],
/// returns a vector of their [`SyntaxKind`] and text.
fn collect_green_nodes<'db>(

and fix doc.

Suggestion:

fn extend_green_nodes<'db>(

fixed

Copy link
Collaborator

@orizi orizi left a comment

Choose a reason for hiding this comment

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

@orizi made 1 comment and resolved 1 discussion.
Reviewable status: 0 of 1 files reviewed, all discussions resolved.


a discussion (no related file):
@FroyaTheHen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants