Skip to content

Conversation

wawel37
Copy link
Contributor

@wawel37 wawel37 commented Oct 1, 2025

  1. We make it public as we need it in the cairo-lint and LS.
  2. We change the type to dyn Database, as we want to use this struct in LS, where we use the Database type from salsa already

@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.

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion


crates/cairo-lang-parser/src/db.rs line 53 at r1 (raw file):

/// Parses a file and returns the result and the generated [ParserDiagnostic].
#[salsa::tracked(returns(ref))]
pub fn file_syntax_data<'db>(db: &'db dyn Database, file_id: FileId<'db>) -> SyntaxData<'db> {

you don't need it - you have all the members of ParserGroup as the selectors for this data struct.

@wawel37 wawel37 force-pushed the file-syntax-public branch from f19356f to 7c9d8bf Compare October 2, 2025 11:45
Copy link
Contributor Author

@wawel37 wawel37 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @orizi)


crates/cairo-lang-parser/src/db.rs line 53 at r1 (raw file):

Previously, orizi wrote…

you don't need it - you have all the members of ParserGroup as the selectors for this data struct.

Ok, so cant we just make the syntax_file function part of the query group? As it's used in existing selectors anyway. And we don't want to duplicate logic of choosing right selector based on the FileId kind

@wawel37 wawel37 requested a review from orizi October 2, 2025 11:48
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.

Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @wawel37)


crates/cairo-lang-parser/src/db.rs line 53 at r1 (raw file):

Previously, wawel37 (Mateusz Kowalski) wrote…

Ok, so cant we just make the syntax_file function part of the query group? As it's used in existing selectors anyway. And we don't want to duplicate logic of choosing right selector based on the FileId kind

i still don't see WHY you need it - what is the actual context?

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