Type scalar constants of struct type - #276
Conversation
MIR represents a constant of a struct with scalar layout, such as a single-field newtype, as ConstValue::Scalar. const_value_ty had no case for it and hit unimplemented!. Build the struct as a tuple of boxed fields, giving the scalar to the only non-ZST field. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WZXwt16ZiUmjT6hDYraDtb
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d422493897
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The paired UI tests must exercise scalar placement alongside ZST fields.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Adds support for MIR scalar constants represented as structs.
Changes:
- Reconstructs scalar structs from non-ZST and ZST fields.
- Adds paired pass/fail regression tests.
- ZST reconstruction still lacks test coverage.
| File | Description |
|---|---|
tests/ui/pass/const_scalar_struct.rs |
Adds successful scalar-struct verification coverage; should include a ZST field. |
tests/ui/fail/const_scalar_struct.rs |
Adds expected-failure coverage; should include a ZST field. |
src/analyze/basic_block.rs |
Converts scalar struct constants into boxed field tuples. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| let field_val = if field_layout.is_zst() { | ||
| ConstValue::ZeroSized |
A ZST constant of tuple or struct type became bare unit regardless of its declared shape, so passing e.g. a ((),) constant, or a ZST field of a scalar struct constant, to a function expecting ((),) panicked in relate_type. Build tuples and structs field by field for ZeroSized too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WZXwt16ZiUmjT6hDYraDtb
The catch-all arm gave any ZeroSized constant unit type whatever its declared type, silently producing a mistyped value. Tuples, structs and closures without captures are handled by their own arms; let the rest reach unimplemented!. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WZXwt16ZiUmjT6hDYraDtb
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5c4cd5391
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else { | ||
| *val | ||
| }; | ||
| let pt = self.const_value_ty(&field_val, &field_ty); |
There was a problem hiding this comment.
When a scalar-layout struct has a zero-sized, single-variant enum field (for example, enum Marker { M } in struct Wrap { scalar: i32, marker: Marker }), this passes ConstValue::ZeroSized back into const_value_ty, but the new match handles zero-sized tuples and structs only; the enum therefore reaches the fallback and panics instead of analyzing the constant. Fresh evidence after the prior fix is that this final match still reproduces not implemented: const: ZeroSized, ty: Marker; add construction of the enum's inhabited variant rather than recursively passing an unsupported value.
Useful? React with 👍 / 👎.
…casts) into forall-sort Brings in coord-e#114 (Rust expressions as thrust::predicate bodies, instantiated per generic arguments), coord-e#275 (newer PCSat in CI), coord-e#276, coord-e#277 and coord-e#278 on top of forall-sort. Resolution: - A predicate call whose instance resolves goes through Analyzer::predicate_with_args, so a Rust-body predicate is defined once per instantiation and a raw SMT-LIB2 predicate keeps its single definition. A call that does not resolve (it still depends on the owner's type parameters) keeps the forall predicate. - predicate_with_args takes the calling function as owner, since forall-sort translates type parameters relative to it; an instance whose arguments mention type parameters is keyed by that owner too. - UserDefinedPredDef keeps both the new body enum and the ForallPred dependency set. A formula body contributes its ForallPred atoms directly and its calls to other user-defined predicates as edges of the dependency graph; a raw body is still scanned by name. - The refinement clause builder records the value-variable origin only when the value term is a variable, as the singleton case substitutes a default term. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
MIR represents a constant of a struct with scalar layout, such as a single-field newtype (
Align::EIGHT = Align { pow2: 3 },Size::ZERO,Hash64::ZERO), asConstValue::Scalar.const_value_tyhad no case for it and hitunimplemented!("const: Scalar(0x03), ty: Align").This adds that case: the struct is built as a tuple of boxed fields, like other struct values, with the scalar given to the only non-ZST field and
ConstValue::ZeroSizedto ZST fields.Tests
tests/ui/{pass,fail}/const_scalar_struct.rs. Both hit the ICE before this change and pass after it.🤖 Generated with Claude Code
https://claude.ai/code/session_01WZXwt16ZiUmjT6hDYraDtb
Generated by Claude Code