Skip to content

Add macro call module id. #7834

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

Open
wants to merge 3 commits into
base: spr/main/b5338905
Choose a base branch
from

Conversation

gilbens-starkware
Copy link
Contributor

@gilbens-starkware gilbens-starkware commented Jun 12, 2025

Stack:

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Collaborator

@dean-starkware dean-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 6 of 8 files at r1, all commit messages.
Reviewable status: 6 of 8 files reviewed, 3 unresolved discussions


crates/cairo-lang-defs/src/cache/mod.rs line 710 at r1 (raw file):

                ModuleIdCached::Submodule(SubmoduleIdCached::new(submodule_id, ctx))
            }
            ModuleId::MacroCall(_) => todo!(),

why todo!()?


crates/cairo-lang-semantic/src/resolve/mod.rs line 957 at r1 (raw file):

                }
                ModuleId::MacroCall(macro_call_id) => {
                    todo!("Is it possible to have `super` in a macro call module?");

I think it is possible, as long as the macro is invoked in a context where super resolves correctly.


crates/cairo-lang-semantic/src/lsp_helpers.rs line 282 at r1 (raw file):

                }
            }
            ModuleId::MacroCall(macro_call_id) => todo!(),

why todo!()?

@dean-starkware dean-starkware force-pushed the spr/main/b5338905 branch 3 times, most recently from 10bf157 to d7fb8b1 Compare July 9, 2025 12:53
@mkaput mkaput requested review from FroyaTheHen and removed request for mkaput July 9, 2025 13:06
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