Repository navigation
Conversation
The derives bound method parameters named `iter`, `v`, `s`, `payload`, `value`, `key` and `u`. An identifier pattern that names a constant in scope is a constant pattern, so a constant, const generic, static, unit struct or tuple struct with one of those names made the parameter a pattern of the wrong type (E0308 or E0530 inside the derive). A one-segment override path with one of those names resolved to the parameter. Every binding now starts with `__buffa_`, the prefix the crate already reserves. `Span::mixed_site()` does not help here: a constant is an item, and items resolve at the call site under mixed-site hygiene. Fixes anthropics#652. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #652.
What
The five
buffa-remote-derivederives, and theirarbitraryimpls, bound method parameters with plain names:iter,v,s,payload,value,keyandu. All of these now start with__buffa_(__buffa_iter,__buffa_vec,__buffa_str,__buffa_string,__buffa_payload,__buffa_value,__buffa_key,__buffa_unstructured). That is the prefix the crate already reserves for its generated lifetimes, type parameters and serde/arbitrary bindings.Why
An identifier pattern that names a constant in scope is a constant pattern, not a new binding. So any of these items beside a derive made a parameter a pattern of the wrong type:
const v: u8 = 0;struct L<T, const iter: usize>(Vec<T>)Each case failed with E0308 inside the derive, and the error did not name the cause. A static or tuple struct with one of those names fails the same way, with E0530.
payload, infrom_wireofProtoStringandProtoBytes, was affected as well, although the issue does not list it.A related problem: a one-segment override path with one of those names, such as
new = valueorinsert = key, resolved to the parameter. The crate docs asked for a path of two or more segments to work around it. This PR drops that restriction.Span::mixed_site()would not have fixed the constant case. Mixed-site hygiene resolves only locals, labels and$crateat the definition site. Constants are items, so they still resolve at the call site. Amacro_rules!macro, which has the same hygiene, hits the same E0308.Implementation
list.rs,string.rs,bytes.rs,map.rs,box_ptr.rs,forwarders.rs: rename the bindings.string.rslosesctor_from_wire, which was the same tokens asctor_from_strand can now share it.lib.rs: "Reserved identifiers" now covers every name the impls introduce, and every item in scope at the derive. The paragraph that asked for multi-segment override paths is removed.every_binding_starts_with_the_reserved_prefix. It walks each expansion withsyn::visit, covering tuple and named-field structs and every override key, and checks that eachPatIdentstarts with__buffa_. This means a future plain-named parameter, closure parameter orletfails a test.syn'svisitfeature is enabled for dev-dependencies only, so the shipped proc macro does not build it.tests/shadowed_names.rs. It declares lower-case constants, const generics, a static, a unit struct and a tuple struct with every old parameter name. It also uses one-segmentnew = value,new = uandinsert = keyoverrides, and exercises each derive at runtime. Before the fix it fails to compile with 64 errors.Testing
cargo fmt --all --checkcargo clippy --workspace --all-targets -- -D warningscargo test --workspace(protoc 33.5)RUSTDOCFLAGS="-D warnings" cargo doc -p buffa-remote-derive --no-depscargo clippy -p buffa-test --all-targets --features arbitrary -- -D warningsandcargo test -p buffa-test --features arbitrary --lib --testsFollow-ups (not in this PR)
Review turned up two older holes of a different kind, which I would file separately:
str,u8andusizewithout a path, so a user type with one of those names in scope breaks them. Codegen fixed the same class of problem in codegen: support types named after Rust keywords and primitives #556. Here the fix would be::core::primitive::*.__buffa_payload.to_str()and.as_slice()use method-call syntax. A trait in scope with a by-value method of the same name, implemented forWirePayload, would win method resolution. The fix is the fully qualified form the crate already uses foras_ref.🤖 Generated with Claude Code