Polish projection shorthand handling #82
Conversation
RAprogramm
deleted the
codex/update-projection-parsing-and-coverage-tests
branch
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| shorthand: &FormatArgShorthand, | ||
| placeholder: &TemplatePlaceholderSpec | ||
| ) -> Result<ResolvedPlaceholderExpr, Error> { | ||
| match shorthand { | ||
| FormatArgShorthand::Named(ident) => { | ||
| let field = fields.get_named(&ident.to_string()).ok_or_else(|| { | ||
| Error::new( | ||
| ident.span(), | ||
| format!("unknown field `{}` in format arguments", ident) | ||
| ) | ||
| })?; | ||
| Ok(struct_field_expr(field, placeholder.formatter)) | ||
| } | ||
| FormatArgShorthand::Positional { | ||
| index, | ||
| span | ||
| } => { | ||
| let field = fields.get_positional(*index).ok_or_else(|| { | ||
| Error::new( | ||
| *span, | ||
| format!("field `{}` is not available in format arguments", index) | ||
| ) | ||
| })?; | ||
| Ok(struct_field_expr(field, placeholder.formatter)) | ||
| } | ||
| let FormatArgShorthand::Projection(projection) = shorthand; | ||
|
|
||
| let (expr, first_field, has_tail) = struct_projection_expr(fields, projection)?; | ||
|
|
||
| if !has_tail && let Some(field) = first_field { | ||
| return Ok(struct_field_expr(field, placeholder.formatter)); | ||
| } | ||
|
|
||
| if needs_pointer_value(placeholder.formatter) { | ||
| Ok(ResolvedPlaceholderExpr::with(expr, false)) | ||
| } else { | ||
| Ok(ResolvedPlaceholderExpr::new(quote!(&(#expr)))) |
There was a problem hiding this comment.
[P1] Borrow nested projections before pointer formatting
When a shorthand projection resolves to more than one segment and the placeholder uses the pointer formatter ({:p}), the generated code returns the projection expression without taking a reference (ResolvedPlaceholderExpr::with(expr, false)). For projections like .inner.value where the final value is not itself a pointer, this expands to Pointer::fmt(self.inner.value, f) and fails to compile because i32 (and most values) do not implement Pointer. The simple-field path still borrows correctly, so the regression only appears for the new nested projections and affects both structs and variants. The expression should be wrapped in &(expr) unless the projection’s result is already a pointer type.
Useful? React with 👍 / 👎.
Summary
Testing
cargo +nightly fmt --cargo +nightly clippy -- -D warningscargo +nightly build --all-targetscargo +nightly test --allcargo +nightly doc --no-depscargo deny checkcargo audithttps://chatgpt.com/codex/tasks/task_e_68ce3655c450832b9578da25d26e83ce