Skip to content

fix: review findings that don't involve Any, and simplifications 🧹 - #345

Merged
timfennis merged 2 commits into
feat/type-checkerfrom
fix/review-findings-not-any
Nov 13, 2026
Merged

timfennis merged 2 commits into
feat/type-checkerfrom
fix/review-findings-not-any

Conversation

@timfennis

Copy link
Copy Markdown
Owner

A review of the type-checker branch (PR #291) found these, all independent of how Any works (which is being redesigned separately, #331).

Bug fixes:

  • A pure fn shared one cache between 1 and 1.0, which are equal, so same(1.0) returned the cached Int: each set of argument types of a pure fn now gets an instance, and a cache, of its own.
  • A tuple whose elements still waited on a literal iterated as Any, so (x - 1, y + 1).map(abs) was rejected while the list form worked. A single-overload call now waits while a sequence argument waits to iterate.
  • A later call could take a nested function away from an earlier one: in fn run2() { fn d(x) => "inner"; let r = d(1); d(2.5); r }, d(1) ended up calling an outer fn d(x: Int). When the nearest scope that takes a call has one overload whose parameters take whatever its literals become, the call commits to it.
  • A range's bounds weren't required to be Ints, so let xs = [0.5, n] could turn n into a Float and 0..n failed as the program ran. The bounds are now Ints, and a Float or Number bound is an analysis error.
  • xs[1..3] = 5..7 passed analysis and failed as the program ran: a list slice now takes any sequence, as its signature says.

Simplifications (from the review, −84 lines): delete the unused StaticType::with_element_type and a slice branch of validate_assignment_write that could never run, replace TypeVars::try_settle with try_unify, define held_literal as the first of held_literals, and give a value callee the same checks path as other calls.

Analysis of the large benchmark files is 2–3% slower, from the tuple fix's extra check per sequence parameter; the VM is unchanged.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-13T20:16:05.540034Z d857a24 Manual request
ℹ️ 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 95f02cf8cd

ℹ️ 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".

Comment on lines +1374 to +1377
&& let [only] = nearest[..]
&& self.takes_any_literal(only, arguments)
{
return EarlyOverload::Only(only);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Wait for nonliteral arguments before committing to a scope

When another argument is an unresolved nonliteral type variable, takes_any_literal does not establish that this overload will still accept the call once that argument is known. For example, fn d(x: Int, y: String) => "outer"; fn run(y) { fn d(x, y: Int) => "inner"; d(1, y) } print(run("s")); now commits to the inner d while analysing run, forcing y to Int and rejecting the String call. Previously, both scopes remained viable until the call supplied String, allowing the outer overload to be selected. Keep this call undecided unless the nonliteral arguments also cannot change which scope accepts it.

Useful? React with 👍 / 👎.

…cope takes it 🎯

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@timfennis

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: d857a24df9

ℹ️ 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".

@timfennis
timfennis merged commit 56086aa into feat/type-checker Nov 13, 2026
1 check passed
@timfennis
timfennis deleted the fix/review-findings-not-any branch November 13, 2026 21:04
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.

1 participant