-
Notifications
You must be signed in to change notification settings - Fork 173
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
Expand FormatArgs node as AST pass #2893
Expand FormatArgs node as AST pass #2893
Conversation
1478b6a
to
93192c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lots of good changes in there 👍 I think it still requires a bit of work before merging.
std::string | ||
ffi::RustHamster::to_string () const | ||
{ | ||
return std::string (ptr, len); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we implement a string view type instead ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think eventually yes, but not for now because this has FFI implications and this PR is already big enough IMO
auto it = spec_map.find (format_specifier.ty.to_string ()); | ||
|
||
if (it == spec_map.end ()) | ||
rust_unreachable (); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably emit an error ? Or at least report to the caller ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be addressed in a later PR. I'm not sure if the parser is responsible for handling these errors or if this is something we should indeed emit ourselves. We'll see as we move forward with the format_args!()
implementation
27ac75a
to
1fe32c0
Compare
b3dc88a
to
69bf48d
Compare
gcc/rust/ChangeLog: * Make-lang.in: Add new object. * hir/rust-ast-lower-expr.cc (ASTLoweringExpr::visit): Remove calls to FormatArgsLowering. * expand/rust-expand-format-args.cc: New file. * expand/rust-expand-format-args.h: New file.
gcc/rust/ChangeLog: * ast/rust-fmt.cc (ffi::RustHamster::to_string): New. (Pieces::collect): Adapt to use new handle API. (Pieces::~Pieces): Likewise. (Pieces::Pieces): Likewise. (Pieces::operator=): Likewise. * ast/rust-fmt.h (struct RustString): Add members. (struct FormatArgsHandle): New. (clone_pieces): Adapt for new FFI API. (destroy_pieces): Likewise. (struct Pieces): Store new FormatArgsHandle type. * expand/rust-expand-format-args.cc (expand_format_args): Use proper namespace. * resolve/rust-ast-resolve-base.cc (ResolverBase::visit): FormatArgs nodes are already resolved, so do nothing. libgrust/ChangeLog: * libformat_parser/src/lib.rs: Use new Handle struct and expose it.
gcc/rust/ChangeLog: * ast/rust-ast-builder.h: Rename AST::AstBuilder -> AST::Builder * ast/rust-ast-builder.cc: Likewise. * expand/rust-derive.cc: Use new AST::Builder name. * expand/rust-derive.h: Likewise. * ast/rust-builtin-ast-nodes.h: Add required getters. * expand/rust-expand-format-args.cc (format_arg): New. (get_trait_name): New. (expand_format_args): Properly expand basic format_args!() invocations. * expand/rust-expand-format-args.h (expand_format_args): Fix signature. * expand/rust-macro-builtins.cc (MacroBuiltin::format_args_handler): Call into expand_format_args().
gcc/testsuite/ChangeLog: * rust/compile/format_args_basic_expansion.rs: New test.
This fixes an issue we had where the generated code ended with more static pieces than its rustc counterpart. gcc/rust/ChangeLog: * expand/rust-macro-builtins.cc (struct FormatArgsInput): Store the format_str as a string instead of an AST::Expr. (format_args_parse_arguments): Transform format_expr into a format string properly - add note for handling eager macro invocations later on. (MacroBuiltin::format_args_handler): Parse the correct input, append newline to format_str if necessary.
Needs #2859
Needs #2917 for GCC 4.8 to have Rust 1.72 installed (for
String::leak
)