Upgrade the borrow pragma to work with generics and bracket operators on builtin types#25450
Open
Clemente90 wants to merge 26 commits intonim-lang:develfrom
Open
Upgrade the borrow pragma to work with generics and bracket operators on builtin types#25450Clemente90 wants to merge 26 commits intonim-lang:develfrom
Clemente90 wants to merge 26 commits intonim-lang:develfrom
Conversation
…unctionality tests: add barrow suite for borrowing operators on distinct types
…-distinct
Enable borrow support for distinct bracket operators
# Motivation
Allow distinct types to borrow bracket operators ([]) so borrowed bracket syntax on a distinct can reuse the base-type bracket implementations.
Ensure borrow resolution and type checks treat bracket borrowing consistently with the existing . borrow support for distinct types.
Keep tests as TDD: move existing barrow tests to tests/distinct and adapt minimal test code to exercise the new semantics.
# Description
Introduced a new type flag tfBorrowBrackets and wired it into the flag serialization/parsing logic (compiler/astdef.nim, compiler/ic/enum2nif.nim).
Extended the type pragma handling to accept {.borrow: [].} alongside {.borrow: ..} and mark the type with the new flag (compiler/pragmas.nim).
Updated type-finalization and error messages so only distinct types may borrow . or [], with precise messages depending on which operator was requested (compiler/semstmts.nim).
Adjusted subscript semantic checking to skip over distinct wrappers marked with tfBorrowBrackets when resolving built-in subscripts and overload resolution (compiler/semexprs.nim).
Enhanced borrow-resolution to treat generic distinct parameter cases and to special-case array magics (mArrGet / mArrPut) appropriately during borrow lookup and return-type matching (compiler/semcall.nim).
Propagated tfBorrowBrackets onto distinct parameter element types when a borrowed [] is attached, so nested borrowing works (compiler/semstmts.nim).
Moved and slightly adjusted tests from tests/barrow to tests/distinct, including tborrow_array_brackets.nim, tborrow_nested_brackets.nim, and updates to tinvalidborrow.nim to reflect new expectations and minimal test edits where necessary.
# Testing
Rebuilt the compiler and toolchain with bin/nim c ... koch and ./koch boot ..., both completed successfully.
Compiled and ran all modified borrow tests with for f in tests/distinct/tborrow_*.nim; do bin/nim c -r ... "$f"; done, and the testcases in tests/distinct completed successfully after the adjustments.
Ran bin/nim check --hints:off --warnings:off tests/distinct/tinvalidborrow.nim to verify the expected error messages for invalid borrow usages, and it reported the expected errors.
All automated semantic and compilation checks used in this rollout passed (see the rollout transcript for the exact command outputs).
This test case checks various aspects of distinct types and borrowing in Nim, including access to operators and borrowing from generic parent types.
…istinct-types-uuhdfn
Motivation
Recent changes allowed [] in the distinct type-level {.borrow.} pragma and introduced regressions in bracket-operator borrowing for builtin-like containers; the language manual limits type-level borrow to dot only and bracket operators must be borrowed via procs.
The compiler must correctly resolve proc-level {.borrow.} for magic bracket ops (array/seq/openArray/etc.) across multiple distinct types, generic instantiations and block scopes without creating global ambiguities.
Description
Restrict type-level borrow pragma to accept only . and emit a clear error for attempts to use [] in type declarations by updating compiler/pragmas.nim.
Stop treating borrowed bracket operators as a type-level flag by removing/controlling tfBorrowBrackets propagation and ensure []. borrows are handled via proc-level logic in compiler/semstmts.nim, compiler/semexprs.nim and compiler/semcall.nim.
Make overload/borrow resolution robust for magic bracket operators by skipping return-type checks for bracket-named borrows and preventing the automatic magic-match path from producing ambiguities for borrowed procs via changes in compiler/semcall.nim and compiler/sigmatch.nim.
Preserve correct handling for generic/instantiated distinct parameter types when marking borrowed [] proc parameters, including tyGenericInvocation cases, via compiler/semstmts.nim adjustments.
Update AST and encoding helpers so flags remain consistent (compiler/astdef.nim, compiler/ic/enum2nif.nim) and rework the semantic subscript handling to respect borrow flags in compiler/semexprs.nim.
Tests: remove the now-invalid type-pragma test tests/distinct/tborrow_type_pragma_brackets.nim, add tests/distinct/tborrow_seq_brackets.nim, and update several tests/distinct/* files to use proc-level {.borrow.} for brackets and exercise multiple distinct types, generics and block-scope cases.
Testing
Rebuilt the compiler and tools with ./bin/nim c ... koch and ./koch boot and the build succeeded.
Ran modified/added test cases individually with the new compiler: ./bin/nim r tests/distinct/tborrow_array_brackets.nim, ./bin/nim r tests/distinct/tborrow_negative_generics_and_blockscope.nim, and ./bin/nim r tests/distinct/tborrow_seq_brackets.nim, all of which executed successfully.
Ran the invalid/negative check ./bin/nim check --hints:off --warnings:off tests/distinct/tinvalidborrow.nim and it produced the expected reject diagnostics.
Overall: updated tests now pass and the reject-case emits the expected error messages.
This test case verifies the support for generics in borrow operations and checks for fixes to several related issues. It includes various structures and assertions to validate their behaviors.
…borrow-pragmas
# Motivation
The borrow-pragmas feature did not work when the borrowed routine involved generic (templated) containers, causing correct programs to fail to compile.
A new TDD test tests/distinct/tborrow_generics_support.nim exposed the issue (and had one wrong expectation which was corrected).
The intent is that a generic proc marked with {.borrow.} should cause the compiler to generate concrete borrowed instantiations with the same borrow semantics.
# Description
Allow generic-aware borrow lookup in searchForBorrowProc by adding logic to normalize distinct/generic invocation types and to match overloads that differ only by generic parameters (added baseTypeFromDistinctGeneric, borrowSearchType, normalizeBorrowType, borrowTypesCompatible).
When resolving borrow targets for generic signatures, prefer matching generic overloads and compare normalized return types (updated compiler/semcall.nim and compiler/semdata.nim).
During generic instantiation, generate concrete borrowed base procs when the instantiated body refers to a borrowed generic symbol, by instantiating the generic callee with inferred bindings (changes in compiler/seminst.nim).
Re-enabled generic borrows by removing the prior generic rejection in semBorrow (removed bsGeneric error) and fixed the test file tests/distinct/tborrow_generics_support.nim to assert the correct compile behavior and concrete type checks.
# Testing
Ran bin/nim c -d:release -r compiler/nim c -d:release -r tests/distinct/tborrow_generics_support.nim, which compiled and executed the test successfully.
Ran a full bootstrap build with ./koch boot -d:release --skipUserCfg --skipParentCfg --hints:off, which completed successfully (compiler rebuilt and executables matched).
The test tests/distinct/tborrow_generics_support.nim was corrected and now passes with the updated compiler behavior.
…borrow-pragmas # Motivation Borrow resolution produced incorrect matches, crashes and unintended instantiations for nested distinct and generic types; normalizing/peeling those wrappers and avoiding instantiation during borrow lookup is required. # Description Introduced guarded normalization and unwrapping helpers (normalizeBorrowType, baseTypeFromDistinctGeneric, borrowSearchType) and a scoring matcher borrowMatchScore to preference best overloads when resolving borrows, and integrated them into searchForBorrowProc (compiler/semcall.nim). Avoided recursive/erroneous instantiation during borrow resolution by adding the inBorrowSearch context guard and using borrowInstType to compare types without triggering generic instantiation (compiler/seminst.nim, compiler/semdata.nim). Adjusted distinct-base collapsing and generic-instantiation handling so nested distinct/generic wrappers are collapsed safely when computing borrow compatibility (compiler/seminst.nim, compiler/semcall.nim). Added focused tests exercising nested distinct/generic borrow edge cases: tests/distinct/tborrow_nested_distinct_len.nim, tborrow_static_array_nested.nim, tborrow_ref_object.nim, and tborrow_string_concat.nim. # Testing Built the compiler and ran focused compiles of the new tests with bin/nim c -d:release -r compiler/nim c -d:release -r tests/distinct/tborrow_nested_distinct_len.nim, .../tborrow_static_array_nested.nim, .../tborrow_ref_object.nim, and .../tborrow_string_concat.nim, all of which completed successfully. Ran the distinct test suite with bin/testament --nim:compiler/nim c tests/distinct, which reported 24 passed and 1 failure; the lone failure is a JS-backend invocation failing because the compiler binary in this environment was not built with the JS backend, not a regression introduced by these changes.
…-for-distinct-types Add additional test cases
…gma-in-distinct-types Make borrowing iterators work and add additional tests.
…comments-in-tests-0io3b5 Enable borrowed methods on distinct types
Author
|
Hello @Araq, This is a bit of an experiment. I have very little working knowledge of the Nim codebase. These edits were made with an ai agent where I mostly evaluated the test cases, not the changes. (Assuming CI passes) What is your opinion? Is this (A) "ai slop" and a waste of time and money, or (B) good and useful? |
Member
I know, don't worry about it.
It's not slop but it's very complex for what it does, most of the inherent complexity is due to other existing complexity. Too many special cases for |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This changeset hopes to address these issues for
distincttypes andborrowpragma usage:Error: no symbol to borrow from found #4121 distinct seq[T]
Borrowing from distinct generic results in:
no symbol to borrow from found#19097 distinct Table[A, B]distinct types with static[T] can't borrow #6026 type Amount[date: static[int]] = distinct float and +
Borrowing
[]for distinct array doesn't work #3564 distinct array[4, float32] and []Can't borrow array access operator #2684 distinct seq[T] and []
Fixes #4121 Fixes #19097 Fixes #6026 Fixes #3564 Fixes #2684
It is also a test of whether OpenAI's Codex Cloud Agent (5.2) is good enough to solve a problem of this complexity (pretty much) by itself.
Aims to make the {.borrow.} pragma work for bracket operators on arrays and seqs, on iterators, on methods, on generic types and functions.