Make typeRel behave to spec

[Graveflo]

The goal of this PR is to make typeRel accurate to it's definition for generics:

# 3) When used with two type classes, it will check whether the types
# matching the first type class (aOrig) are a strict subset of the types matching
# the other (f). This allows us to compare the signatures of generic procs in
# order to give preferrence to the most specific one:

I don't want this PR to break any code, and I want to preserve all of Nims current behaviors. I think that making this more accurate will help serve as ground work for the future. It may not be possible to not break anything but this is my attempt.

So that it is understood, this code was part of another PR (#22143) but that problem statement only needed this change by extension. It's more organized to split two problems into two PRs and this issue, being non-breaking, should be a more immediate improvement.

[Graveflo]

This is mostly concerning tyObject and tyBuiltInTypeClass
Eg

type
  A[T] = object
  C = object

C ⊂ object and A ⊂ object are true , but currently this will return isNone not isGeneric
Similarly A[int] ⊂ A:Object is true but returns isNone
Similar issues exist for tyPtr and tyRef and for types that are embedded in tyGenericInvocation for example

[Graveflo]

CI fail is Mac os servers shitting out again I think

Overview of changes:

• isObjectSubtype needed slightly more robustness since it is used in more situations.

In typeRel:

• tyArray: Since isGeneric is now more common a check for concreteness is needed for static evaluation
• tyObject: There is a check for tyAlias to keep concepts working. Otherwise, the object type of the subject is checked for membership instead of just checking the kind
• tyPtr, tyRef: Alignmnet of these nodes can be off if buried in generic params or bodies. getObjectTypeOrNil is meant to mitigate this. It is also used in tyBuiltInTypeClass
• tyGenericInvocation: tyGenericParam is skipped
• tyBuiltInTypeClass: Same extraction routine of type class as in tyPtr, tyRef except we have to skip tyBuiltInTypeClass to line up with f[0]
• tyGenericParam: doBind is used to differentiate between signature comparisons and parameter matching. There is a special case for non-concrete values in binding mode

tests/overload/tor_isnt_better.nim:
There is a comment in the file with an explination. The issue is that checkGeneric uses typeRel to determine D ⊂ D|E which should return isGeneric and the other way around D|E ⊂ D should return isNone, which makes proc g(a: D) preferred.

There is a case in this PR that should probably be reviewed by someone with authority on the matter. Consider:

type A[T] = object

proc d(x: A):bool= false
proc d(x: int | A[SomeInteger]):bool= true
echo d(A[5]())

Currently this will print true which does make sense when you look at it. The issue is that according to the way overload reslution is implemented, not just in circumstance but in design, the proc that returns false should be chosen. In the PR version of Nim, this is what happens. The question is, should more specific matches in or composited type classes be preferred as a special case? I will preserve this behavior and implement/document the special case if that is desired. I just want to make sure we are aware of this particular case.

I will continue to make pushes to this PR that attempt to implement the special case to preserve the current behavior, if it is not needed then the commit preceding this comment should be rolled back to.

[Araq]

A ⊂ object are true

No, A is a generic, not an object.

[Graveflo]

A ⊂ object are true

No, A is a generic, not an object.

I wasn't saying A is an object. I was trying to say that A consists only of specific objects and that set of objects is a subset of the set of objects that are members of object's set. If you were talking about the A:Object I was just mirroring how the compiler stringifies non-alias = object type definitions.

It was not a good idea for me to post this without clarifying the syntax I chose to use
"X ⊂Y" reads "X is a subset of Y"

[Araq]

Ok, but please add some tests that show what is now possible that wasn't before.

[Graveflo]

Ok, but please add some tests that show what is now possible that wasn't before.

I added to a test that highlights the only discrepancy. The behavior may be changed in the future. I think it is beyond the spec definitions, but that is a problem for another time.
Please note that this PR is a code-base improvement and was not meant to have noticeable effects / changes in the language. This PR was the result of findings from PR #22143 that were not specific to that PR's mission.

[Araq]

Why does tyAlias remain a tyAlias?! The rest of the logic does not consider tyAlias and aliases to object types are allowed and common.

[Graveflo]

I should have taken better notes, but I believe I tracked this down to an issue with bare type variables. You are right, this is not correct. I'll draft until I fix it.

[Graveflo]

To follow up on this, the reason checking for tyAlias was done here was to "keep concepts working" as I said in one of my comments that I forgot about. When a concept is evaluating matching, particularly with the var modifier, it is assumed that on to typeRel: f:tyObject, aOrig: tyTypedesc (typedesc[var Alias]) should return isNone. This means that in order for this functionality to work in the current version of Nim, the condition for tyAlias never matching with tyObject is acceptable (assuming there is no other legal way to get that type structure). Since this doesn't really make sense, as you pointed out, I switched the logic to try and explicitly detect concept modifier matching by checking for useTypeLoweringRuleInTypeClass instead. This is essentially the same approach as before, in that it just falls back on previous behavior for this case but it may read easier this way.

[Araq]

Neither the newly added tests nor the specification nor logic suggest that the handling of ptr and ref need to care about getObjectTypeOrNil.

[Graveflo]

As stated in my comment there are "alignment issues" with ptr and ref, but that only applied to my other PR about generic type variables. It seems evident that this is not the correct way to handle that issue anyway, so I'll just take note of this and make it a point to implement it the correct way in that PR.

[github-actions]

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from b2ca6be

Hint: mm: orc; opt: speed; options: -d:release
170145 lines; 8.657s; 620.066MiB peakmem