Parenthesize unary negations to avoid --

[ErichDonGubler]

Fixes #1564 and #2088. Most of the diff is snapshot test changes, have no fear!

Supersedes @kvark's PR at #1565, with major differences:

• Parenthesizes, rather than prefixes with spaces.
• Filed by an active contributor. 🙃
• Recently rebased onto current master.
• Simulates the consumption of Fix disassembly of constant signed int literals. rspirv#231 with a git dependency for now (thanks for the pointer, @teoxoy!). This should be easy to fixup! once upstream actually releases (presumably rspirv 0.12?); here's a checkbox to track it:
  • BLOCKER: Consume a new release of rspirv it comes out. Proposing working around it. EDIT: Accepted, just go with the Git dep for now.
• Split out the addition of new test case(s) into separate commits to make review easier.
• Also apply a fix to the glsl backend for this issue, which was missed originally.

Recommended review experience is commit-by-commit.

[ErichDonGubler]

The failures in CI seem orthogonal to the changes in this PR, but I'm not sure. @jimblandy, @teoxoy, do you have any ideas?

[teoxoy]

Both MSL and HLSL seem to complain about --3 not being assignable since it's a pre-decrement expression in those languages which is the cause of the orignal issue #1564 and the newly created one #2088.

I'd say we wrap negations in parentheses instead of relying on spaces since we already seem to do that in the WGSL backend.
What do you think?

[ErichDonGubler]

@teoxoy: Ah, okay, I obviously didn't cut through the other (unrelated?) warnings well enough to get to the errors you mention. Yep, that seems expected until we actually fix the offending cases. Sorry for the noise. 😅

[teoxoy]

No worries; yeah I always have to search for "error" in the CI terminal output.

[ErichDonGubler]

@teoxoy:

I'd say we wrap negations in parentheses instead of relying on spaces since we already seem to do that in the WGSL backend.
What do you think?

I do think that parentheses are going to be the simplest cross-target way of disambiguating operations. It will also be easier to intuit as a solution than spacing overall for both consumers and maintainers. @jimblandy, @teoxoy: Any objection to me switching this (and other negation fixes, like for #2088) PR to use parentheses, instead of spacing? (Is there a reason you may not have done this in the first place, @kvark?)

[ErichDonGubler]

This PR is now ready to go, I think, except for the upstream rspirv fix that hasn't been released yet. A few questions (CC @jimblandy, @teoxoy):

1. Can we break out the fix for rspirv into a separate PR? Merge-ability seems predicated on CI passing; what causes CI to fail without the fix is:

   1. adding test coverage for negative literals/negation expressions, and
   2. trying to consume our old version of rspirv is where the conflict starts.

   However:

   1. The bug in (ii) that causes CI to fail is already exposed to users; we just weren't testing it before.
   2. The referenced issues that this PR resolves are filed against other backends, not SPIR-V.

   So, it seems valuable to defer the conflicting changes if we have a sustained blocker here. And it looks like we can do so, given my test at DO NOT MERGE: Test: can we skip an rspirv to fix other backends' negation? #2118.

2. Whatever happens with (1), is there anything we can do to encourage rspirv to release? I don't fully understand this repo's community relationship to rspirv yet, so I don't know who to poke. I'm going to hazard a guess at @msiglreith, since they merged the fix.

EDIT: Tested my hypothesis at (1).

[ErichDonGubler]

@jimblandy: I know that #1565 was originally marked as Ready for review despite this blocker, but I don't feel comfortable doing so, until this is something we can actually merge.

Also, can we close the old PR, since we've already determined that it's not the direction we want? CC @kvark, @teoxoy.

[cwfitzgerald]

We can take a dependency on the git version of rspirv until they get around to releasing it. Next naga release is January 4th, so they have (exactly) two months.

[msiglreith]

I'm not an owner of rspirv just maintaining it on the side for the sporadic reviews. Someone else needs to do the release and publishing part.

[teoxoy]

Looks great!