Hygienic identifiers

[widberg]

This PR fixes a bug where deriving BinRead/BinWrite through a macro-expanded derive path could error in attributes like #[br(pre_assert(...))] with error[E0425]: cannot find value __binrw_generated_position_temp in this scope, help: an identifier with the same name is defined here, but is not accessible due to macro hygiene. This was revealed when I tried using derive-aliases, but it can happen with normal macro_rules!, see the tests. This PR makes the __binrw_generated_position_temp-style IdentStrs use Span::call_site() in to_ident and to_spanned_tokens while leaving the span untouched for other IdentStrs, which fixes the error for me. If there is another way you want this issue solved, I can rework it.

Also, I considered adding something like this to simplify the sanitization file:

ident_str! {
    pub(crate) hygienic POS = "__binrw_generated_position_temp";
}

But it seemed like overkill for a couple of identifiers. Let me know if you want me to put that back in or do something else.

Long-term, I think we should be more careful with spans; I saw some TODOs related to this. Even then, it's probably a good idea to keep these changes.

Finally, I fixed some new warnings and clippy lints to keep CI happy, but the nightly UI tests will probably still fail.

[widberg]

Looks like the 1.70 test is failing because syn bumped their rust-version to 1.71. Should we bump ours, too?

[csnover]

Looks like the 1.70 test is failing because syn bumped their rust-version to 1.71. Should we bump ours, too?

Oh, an easy question that my brain can handle! Yes. :-)

[widberg]

Ended up bumping it up to 1.76 because of trybuild. That should only be a dev dependency; I could try to maintain the 1.71 MSRV despite that, but it might get complicated, and 1.76 is already pretty old, so we'd likely have to bump again soon if we don't do it now. The nightly UI tests are still failing, expectedly. Do we want to disable the UI tests for nightly in CI if it just adds noise?

[csnover]

Do we want to disable the UI tests for nightly in CI if it just adds noise?

Just run TRYBUILD=overwrite cargo +nightly test, git add -u, git diff --staged to make sure it looks sane, and commit.

Once all the span stuff finally gets to the stable rustc the UI tests can run against stable instead. Of course with my limited ability to focus on any of this, and having no other co-maintainer, it might not make much of a difference in terms of how frequently the UI tests need to be updated :-)

[widberg]

Oh my bad, I thought nightly and stable were disagreeing about the output for the same files.

[csnover]

Hi! Did you close this because you don’t have time any more and I wasn’t able to participate whilst you were available, or because there is some other problem with the code itself? I am trying to get as many things done that people want done before I disappear again, so let me know, if you can. Thanks!

[widberg]

I got fed up with the consistently negative experience that comes with trying to engage with this repository. I fixed the UI tests the next day and then went a month with no comments before a different PR with identical CI fixes got merged immediately after it was opened. Fuck me, I guess. Not to mention all the other stalled PRs and issues that have been open for years with no comments. If you don't have time, then don't lead people on just to merge someone else who did the same thing a month later. What a waste of time. I'm done trying to participate in this dead project and will be working out of my fork from now on, at least until I can find something else.

[csnover]

Yes, the reviews are so sporadic, it’s frustrating for me and it’s frustrating when I have a similar experience sending patches into other projects that are black holes. But I don’t take offence to other OSS maintainers struggling or losing interest and I would sure appreciate a little more grace and compassion if you can muster it. You’re acting as if I deliberately tried to hurt you. What possible reason would I have to do that? “Ho ho, that widberg, sending these great PRs that I feel bad about not being able to review! Just wait until I really screw them over by mentioning my limited ability to focus on this, and then demonstrating that by not following up until five months!” Twirls moustache, cackles.

Beyond that, I don’t know what you are talking about. There are currently five open PRs. The oldest one is a draft of mine, one of them is an “it’s not clear if this is the right thing to do and no one else is weighing in”, and the three others all have a last comment from me where the author never followed up. Where are all these other stalled PRs with no comments? The last commits to the repository are also almost all your patches. Where is this other PR that duplicated your work and got merged preferentially? The pub_use_of_private_extern_crate? Is that a duplicate of something? I didn’t even do that.

It’s understandable to be frustrated by others’ schedules not aligning with yours, but I don’t know why you would burn bridges like this? Coming back now to delete your branches is shockingly petty and feels very out of line with the otherwise positive experience I’ve had working with you so far. And for what? So you too can eventually experience the joy of being an overburdened solo maintainer as the world burns around you? I wouldn’t wish that on you.

I’m pretty open to whatever, so I guess it’s up to you if you want collaborators or not, even if they are not as reliable as anyone wants. I’d sure like it if I didn’t have to worry about all the other things in my life that take away most of my free time. Be grateful that you’ve got health, I guess, and fill your life with people that bring you joy.