Diagnose assignment to indexed property instead of ICE (#7160)

[expipiplus1]

Summary

• Replaces the SLANG_UNIMPLEMENTED_X("assignment") internal error with a proper diagnostic (E40017) when attempting to assign to an unsupported l-value form
• The most common trigger is assigning to an indexed element of an array-returning property (e.g. foo.b[0] = 1.0 where b is a property float b[2])
• Adds new diagnostic unsupported-assignment-target (40017)

Test plan

• New test tests/diagnostics/indexed-property-assign.slang verifies E40017 is emitted instead of crashing
• CI passes

Closes #7160

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR replaces an unimplemented assertion when encountering unsupported assignment target forms with a new diagnostic unsupported-assignment-target (40017) and emits it from the assignment lowering; a regression test verifies indexed property element assignment produces the diagnostic.

Changes

Unsupported assignment target diagnostics

Layer / File(s)	Summary
Diagnostic definition for unsupported assignment targets source/slang/slang-diagnostics.lua	New diagnostic unsupported-assignment-target (40017) is defined to report when an assignment target form is not supported.
Assignment codegen error handling and test source/slang/slang-lower-to-ir.cpp, tests/diagnostics/indexed-property-assign.slang	The assignment lowering default case now emits the diagnostic with a best-effort source location instead of calling the unimplemented assertion. A regression test validates that assigning to an indexed property element produces the expected diagnostic rather than causing an internal error.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: replacing an internal compiler error (ICE) when assigning to indexed properties with a proper diagnostic.
Description check	✅ Passed	The description is directly related to the changeset, explaining the replacement of SLANG_UNIMPLEMENTED_X with diagnostic E40017 and documenting the test plan.
Linked Issues check	✅ Passed	The PR fully addresses issue #7160's objectives: it prevents the internal crash by emitting a proper diagnostic (E40017) instead of an internal error when assigning to indexed properties.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the linked issue: adding the diagnostic definition, implementing error handling in the assignment lowering code, and adding a regression test.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR replaces an internal compiler error (ICE) during IR lowering for unsupported assignment targets with a proper diagnostic (E40017), and adds a regression test to ensure the compiler reports the error instead of crashing (notably for assignments like c.b[0] = ... when b is an array-returning property).

Changes:

• Add diagnostic unsupported-assignment-target (40017) with a guidance message.
• Update IR lowering assignment fallback to emit E40017 instead of SLANG_UNIMPLEMENTED_X("assignment").
• Add a new diagnostics regression test covering indexed assignment into an array-returning property.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
source/slang/slang-lower-to-ir.cpp	Replaces an ICE in assign() with a user-facing diagnostic for unsupported l-value assignment forms.
source/slang/slang-diagnostics.lua	Introduces the new E40017 diagnostic definition and message text.
tests/diagnostics/indexed-property-assign.slang	Adds a regression test intended to validate E40017 is emitted for indexed property assignment.

[coderabbitai]

♻️ Duplicate comments (1)

source/slang/slang-diagnostics.lua (1)

4477-4482: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Previously raised concern about workaround validity remains unaddressed.

The diagnostic message suggests "consider assigning to the whole value instead of an individual element," but this workaround may not be valid in all cases where this diagnostic fires. Specifically:

• For properties declared with only get (no set/ref), whole-value assignment will fail
• The test suite does not include a positive test case demonstrating that the suggested workaround actually compiles and runs

Consider either:

1. Softening the message to indicate the workaround requires a set accessor (e.g., "consider assigning to the whole value instead of an individual element (requires a set accessor)")
2. Adding a positive test case to validate the workaround, or
3. Removing the workaround suggestion if it's not universally applicable

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 66cec39b-beba-462c-be29-0c229831d5d5

📥 Commits

Reviewing files that changed from the base of the PR and between 0050fc3 and 01246a3.

📒 Files selected for processing (3)

• source/slang/slang-diagnostics.lua
• source/slang/slang-lower-to-ir.cpp
• tests/diagnostics/indexed-property-assign.slang

[coderabbitai]

Actionable comments posted: 3

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 00144807-7be4-4890-9abf-f3e9638f1966

📥 Commits

Reviewing files that changed from the base of the PR and between 01246a3 and 6694986.

📒 Files selected for processing (3)

• source/slang/slang-diagnostics.lua
• source/slang/slang-lower-to-ir.cpp
• tests/diagnostics/indexed-property-assign.slang

[github-actions]

Verdict: 🟡 Has issues — 4 gap(s)

Replaces an ICE in assign() with a recoverable diagnostic for the c.b[0] = 1.0 case where b is an array-typed property. The fix direction is right; concerns are around scope of the catch-all default: (it now also swallows Flavor::None / Flavor::Subscript that used to surface as compiler-bug ICEs), source-location recovery brittleness for non-assignExpr callers of assign(), and thin test coverage of the new behavior + missing positive test for the workaround the diagnostic message recommends.

Changes Overview

Diagnostic definition (source/slang/slang-diagnostics.lua)

• Adds unsupported-assignment-target (E40017) with primary message "assignment target is not supported" and span message suggesting whole-value assignment.

IR lowering recovery (source/slang/slang-lower-to-ir.cpp)

• Replaces SLANG_UNIMPLEMENTED_X("assignment") in the default: branch of void assign(IRGenContext*, LoweredValInfo const&, LoweredValInfo const&) with getSink()->diagnose(Diagnostics::UnsupportedAssignmentTarget{...}), recovering a SourceLoc by walking irBuilder->getSourceLocInfo() for the first non-zero entry.

Regression test (tests/diagnostics/indexed-property-assign.slang)

• New DIAGNOSTIC_TEST:SIMPLE exercising assignment to c.b[0] where b is property float b[2]. Asserts E40017 message text via two CHECK lines.

Findings (4 total)

Severity	Location	Finding
🟡 Gap	source/slang/slang-lower-to-ir.cpp:10083	default: now catches Flavor::None / Flavor::Subscript (previously ICE'd as compiler bugs); split out Flavor::Simple for the user-facing case and keep SLANG_UNEXPECTED for the rest.
🟡 Gap	source/slang/slang-lower-to-ir.cpp:10086-10095	Source-loc chain walk works only because assignExpr pushes an IRBuilderSourceLocRAII; other callers of assign() (applyOutArgumentFixups, addArg, recursive paths, global-var init) don't, so the diagnostic may emit with a stale or empty SourceLoc.
🟡 Gap	tests/diagnostics/indexed-property-assign.slang:30	Only c.b[0] = 1.0; is exercised; sibling shapes that hit the same default: (compound assignment, increment, function-return subscript, multi-dim) are not regressed.
🟡 Gap	tests/diagnostics/indexed-property-assign.slang:32	The diagnostic recommends "assign to the whole value", but no test asserts that whole-array assignment to an array-typed property setter actually compiles.