Skip to content

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Aug 23, 2023

Summary

Fix two bugs with typeRel that caused no relationship to be detected
when a formal range type's underlying type is dependent on an unresolved
expression.

Details

Problem One

In case they hadn't been resolved yet (because they depend on late-bound
type variables), the bounding values of the formal range type were
modified when computing the relationship between two range types,
which in effect meant that all following queries of the routine
signature saw an instantiated range type, instead of the original
generic one.

The resolved expressions are now stored locally and then directly passed
on to typeRangeRel.

Problem Two

Neither range-against-range nor non-range-against-range considered the
case where the range's underlying type was dependent on an unresolved
expression -- both returning isNone when the formal range type's base
is a tyFromExpr.

If the formal range type's underlying type is a tyFromExpr, the
dependent type is now resolved first. Were f was used previously
directly, the resolved underlying type is used instead.

Tests

An incorrect test case from the now-working tdependent_range_type test
is removed.

Summary
=======

Fix two bugs with `typeRel` that caused no relationship to be detected
when a formal range type's underlying type is dependent on an unresolved
expression.

Details
=======

Problem One
-----------

In case they hadn't been resolved yet (because they depend on late-bound
type variables), the bounding values of the formal range type were
*modified* when computing the relationship between two range types,
which in effect meant that all following queries of the routine
signature saw an instantiated range type, instead of the original
generic one.

The resolved expressions are now stored locally and then directly passed
on to `typeRangeRel`.

Problem Two
-----------

Neither range-against-range nor non-range-against-range considered the
case where the range's underlying type was dependent on an unresolved
expression -- both returning `isNone` when the formal range type's base
was a `tyFromExpr`.

If the formal range type's underlying type is a `tyFromExpr`, the
dependent type is now resolved first. Were `f` was used previously
directly, the resolved underlying type is used instead.

Tests
-----

An incorrect test case from the now-working `tdependent-range_type` test
is removed.
@zerbina zerbina added bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler labels Aug 23, 2023
Copy link
Collaborator

@saem saem left a comment

Choose a reason for hiding this comment

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

this makes ranges quite a bit more useful, yay!

@saem
Copy link
Collaborator

saem commented Aug 23, 2023

/merge

@github-actions
Copy link

Merge requested by: @saem

Contents after the first section break of the PR description has been removed and preserved below:


Notes for Reviewers

@chore-runner chore-runner bot added this pull request to the merge queue Aug 23, 2023
Merged via the queue into nim-works:devel with commit 2d79c7b Aug 23, 2023
@zerbina zerbina deleted the fix-range-type-matching branch August 24, 2023 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants