Skip to content

KCL parser: Allow .prop or [index] to follow any expression#7371

Merged
adamchalmers merged 4 commits intomainfrom
achalmers/member-expr-in-more-places
Jun 5, 2025
Merged

KCL parser: Allow .prop or [index] to follow any expression#7371
adamchalmers merged 4 commits intomainfrom
achalmers/member-expr-in-more-places

Conversation

@adamchalmers
Copy link
Contributor

@adamchalmers adamchalmers commented Jun 4, 2025

Previously in a member expression like foo.x or foo[3], foo had to be an identifier. You could not do something like f().x (and if you tried, you got a cryptic error). Rather than make the error better, we should just accept any expression to be the LHS of a member expression (aka its 'object').

This does knock our "parse lots of function calls" from 58 to 55 calls before it stack overflows. But I think it's fine, we'll address this in #6226 when I get back to it.

This makes the parser a bit slower, which is fine, because the parser is stronger and more capable now.

Closes #7273

@vercel
Copy link

vercel bot commented Jun 4, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 4, 2025 9:44pm

@adamchalmers adamchalmers requested review from jtran and nrc June 4, 2025 19:05
Previously in a member expression like foo.x or foo[3], `foo` had to be
an identifier. You could not do something like f().x

Now you can.

Closes #7273
@codspeed-hq
Copy link

codspeed-hq bot commented Jun 4, 2025

CodSpeed Instrumentation Performance Report

Merging #7371 will degrade performances by 17.65%

Comparing achalmers/member-expr-in-more-places (4319ef6) with main (5235a73)

Summary

❌ 1 regressions
✅ 81 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
parse_food-service-spatula 125.5 ms 152.4 ms -17.65%

@adamchalmers adamchalmers force-pushed the achalmers/member-expr-in-more-places branch from 58a8f5b to 8ea3130 Compare June 4, 2025 20:48
@adamchalmers adamchalmers enabled auto-merge (squash) June 4, 2025 21:00
@adamchalmers adamchalmers merged commit 4575b32 into main Jun 5, 2025
92 of 94 checks passed
@adamchalmers adamchalmers deleted the achalmers/member-expr-in-more-places branch June 5, 2025 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CRYPTIC]: Unexpected token .

2 participants