Skip to content

Conversation

qingshi163
Copy link
Contributor

@qingshi163 qingshi163 commented Oct 15, 2024

closes #5299

@youknowone
Copy link
Member

I am glad to see this, awesome.

@coolreader18
Copy link
Member

I was also wondering if this would be possible! Very cool :)

@qingshi163
Copy link
Contributor Author

There is a proc macro panic witch I did not understand. Could you try to solve it? @youknowone

vm/src/vm/mod.rs Outdated
Comment on lines 907 to 911
// ext_modules!(
// iter,
// dir = "./Lib/python_builtins",
// crate_name = "rustpython_compiler_core"
// );
Copy link
Member

@youknowone youknowone Oct 30, 2024

Choose a reason for hiding this comment

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

I am sorry to be late.
The error caused when compiling Lib/python_builtins/__phello__/spam.py
Parsing step is ok, but compiling fails.

res.map_err(|e| e.into_codegen_error(source_path.to_owned()).into())
res.map_err(|e| e.into_codegen_error(source_code.path.to_owned()).into())
}

Copy link
Member

@youknowone youknowone Oct 30, 2024

Choose a reason for hiding this comment

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

I couldn't find the root cause yet, but adding this test will make using debugger or backtrace easy for the problem

Suggested change
#[test]
fn test_compile_phello() {
let code = r#"
initialized = True
def main():
print("Hello world!")
if __name__ == '__main__':
main()
"#;
let compiled = compile(&code, Mode::Exec, "<>", CompileOpts::default());
dbg!(compiled.expect("compile error"));
}

running with

cargo test --manifest-path compiler/Cargo.toml

@etaloof
Copy link
Contributor

etaloof commented Nov 12, 2024

Hi, what is the status on PR? I can work on this if needed.

@youknowone
Copy link
Member

@etaloof Could you please help? This PR need additonal works to make the upper commented new test passes

@etaloof
Copy link
Contributor

etaloof commented Nov 12, 2024

Sure, should I make a new PR when I am done? I don't think Github allows me to add my commits here.

Comment on lines 1304 to 1307
// If there are type params, we need to push a special symbol table just for them
if !type_params.is_empty() {
if !type_params.is_some() {
self.push_symbol_table();
}

Choose a reason for hiding this comment

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

@qingshi163 @youknowone: This is the source of the bug.
As the comment says, // If there are type params, we need to push a special symbol table just for them, but the change to !type_params.is_some() (as in type_params.is_none()) pushes if there are NO params, causing the panic! in push_symbol_table. On my machine changing to

  if type_params.is_some() {
      self.push_symbol_table();
  }

allows test_compile_phello to pass.

@youknowone
Copy link
Member

oh no... I forget to reset upstream to @etaloof's, now it is pushed here

@coolreader18
Copy link
Member

Superseded by #5494

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.

Replace parser
5 participants