Generate rustpython-ast based off of Python.asdl #2399
Conversation
0135b91 to
11a1255
Compare
|
🎉 |
bd599c6 to
869f2b9
Compare
|
@pca006132 this also implements AST mapping, since it's pretty easy when everything's autogenerated :) |
5a21123 to
dcea9e4
Compare
I just have a glance on it, so the python scripts are used for parsing the python asdl file, and generate the rust code for AST definitions? Considering the change would break `rustpython_parser' API, maybe a version bump would be needed? |
|
Yeah, I was planning to release all new versions next time we release, we make breaking changes often to all of our crates. |
dcea9e4 to
2ecc08f
Compare
|
@youknowone could you review this? |
| Self::Str(s) | ||
| } | ||
| } | ||
| impl From<Vec<u8>> for Constant { |
There was a problem hiding this comment.
is this intended to be a Vec not a Box<[u8]>?
There was a problem hiding this comment.
Yeah, I figured it doesn't make too much of a difference since it's not going to be "alive" for very long.
| match result { | ||
| Ok(mapped) => mapped, | ||
| Err(never) => match never {}, | ||
| } |
There was a problem hiding this comment.
if Err is Infallible, isn't it just result.unwrap()?
There was a problem hiding this comment.
Maybe, but this guarantees that no panic machinery gets pulled in.
| let ast = match parser::parse(source, mode) { | ||
| Ok(x) => x, | ||
| Err(e) => return Err(CompileError::from_parse(e, source, source_path)), |
There was a problem hiding this comment.
let ast = parser::parse(source, mode).map_err(|e| CompileError::from_parse(e, source, source_path))?;
There was a problem hiding this comment.
source_path is moved into from_parse(), and rust doesn't connect the closure to the early return, so I have to manually do the map_err.
| if let OptionalArg::Present(default) = default { | ||
| ret.or_else(|ex| catch_attr_exception(ex, default, vm)) | ||
| Ok(vm.get_attribute_opt(obj, attr)?.unwrap_or(default)) |
There was a problem hiding this comment.
is this better to be changed like this?
There was a problem hiding this comment.
I think so, I added the get_attribute_opt function for use in _ast, but I realized that the logic was being duplicated here as well.
cc @isidentical, I think you had mentioned being interested in this
Closes #2395, closes #2388