Replace pointer address checks in the parser

[Noratrieb]

The parser used to check the address of the P<Expr> to determine whether a method it called had changed the value. This is undesirable for serveral reasons:

• The P::map function allows changes while still maintaining address stability, which could break this code (although it isn't a problem right now)
• Replacing it with a boolean flag is really simple
• This is a Rust parser and I don't like casting pointers to integers in my Rust parser
• I really dont.

r? @compiler-errors because he told me to fix it after i made fun of it

[WaffleLapkin]

maybe a (doc) comment describing what this bool means would be nice,,

[compiler-errors]

galaxy brain: enum ParseDotOrCall { Changed(P<Expr>), Unchanged(P<Expr>) } 😅

but a bool is fine.

[Noratrieb]

enum ParseDotOrCall {
  Changed(P<Expr>),
  Unchanged(P<Expr>),
  CheckTheAddressJustToBeSure(P<Expr>),
}

[compiler-errors]

Just being extra defensive, this maybe could use a perf run... But I will rollup- if it ends up with no results.

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 1a043b2 with merge 75b17f6d708e321f05dd86ccae511d29ff954d97...

[compiler-errors]

anyways r=me once this perf run comes back clean, if it doesn't then i'll give a look at the results.

@bors delegate+

[bors]

✌️ @Nilstrieb can now approve this pull request

[compiler-errors]

You forgot a changed_expr on the previous line.

[compiler-errors]

Add the corresponding UI test please:

fn main() {
    (1,) as (i32,) .0
    //~^ ERROR cast cannot be followed by a field access
}

[compiler-errors]

Actually, I feel like changed is redundant with the check on line 836:

if !matches!(with_postfix.kind, ExprKind::Cast(_, _) | ExprKind::Type(_, _)) || changed {

Why did we need changed in the first place??

[WaffleLapkin]

Status update, as far as I can tell: we don't

[Noratrieb]

i like this

[compiler-errors]

@bors delegate-

[bors]

☀️ Try build successful - checks-actions
Build commit: 75b17f6d708e321f05dd86ccae511d29ff954d97 (75b17f6d708e321f05dd86ccae511d29ff954d97)

[rust-timer]

Queued 75b17f6d708e321f05dd86ccae511d29ff954d97 with parent 77e7b74, future comparison URL.

[rust-timer]

Finished benchmarking commit (75b17f6d708e321f05dd86ccae511d29ff954d97): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.7%, 0.7%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.7%	[0.7%, 0.7%]	1

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.7%	[-4.3%, -3.4%]	3
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

[compiler-errors]

(probably noise)

[Noratrieb]

I forgot about one location the body and it turns out this location is very nested and non-trivial. I'm sure it can be fixed, but that's too much effort for me right. If anyone wants to pick it up, feel free.