canonical type equality constraint #8445
Conversation
source/slang/slang-check-decl.cpp
Outdated
| } | ||
| else if (!(subOk || supOk)) | ||
| { | ||
| // None is qualified, the generic constraint is not valid anyway. |
There was a problem hiding this comment.
do we diagnose in this case?
There was a problem hiding this comment.
It will be diagnosed right after, but it will only point that sub is not valid.
To be more clear we could instead emit a diagnostic that at least one operarand must be valid, and that none were here.
source/slang/slang-check-decl.cpp
Outdated
| auto ancestor = findDeclsLowestCommonAncestor(subAncestor, supAncestor); | ||
| if (!ancestor) | ||
| { | ||
| return 0; |
There was a problem hiding this comment.
Should we sort by mangled name if they belong to two different modules?
There was a problem hiding this comment.
I don't think this case can present itself. To be valid, a type must depend on the parent generic decl, which is necessarily in the same module. So the if (subOk != supOk) // Only one is qualified logic should have already treated this case.
But if the case presents itself, yes we can compare the mangled name.
In the current implementation, if we fail to compare the two types and sub is not valid, a note is diagnosed to point that we failed to find the canonical order. This is because I am not sure all cases were handled properly without introducing a breaking change.
source/slang/slang-check-decl.cpp
Outdated
| { | ||
| if (!validSub && !equalityCannon) | ||
| { | ||
| getSink()->diagnose(decl, Diagnostics::failedEqualityConstraintCanonicalOrder); |
There was a problem hiding this comment.
If !validSub, then we are already diagnosing an error during validateSybType. Why do we need another diagnostic here?
In principle there shouldn't be any cases where we cannot determine canonical order, and we should always be able to sort the types one way or the other.
There was a problem hiding this comment.
In principle there shouldn't be any cases where we cannot determine canonical order, and we should always be able to sort the types one way or the other.
Yes, but if new cases arise in the future then we will be notified.
|
Actually I was going to push some improvements based on your comments... |
Head branch was pushed to by a user without write access
SuperRonan
force-pushed
the
ronan/feature/canonical-type-equality-constraint
branch
from
95f052b to
b02b31f
Compare
| @@ -0,0 +1,41 @@ | |||
| //TEST:SIMPLE(filecheck=CHECK): | |||
There was a problem hiding this comment.
It is better to have a test that computes something meaning out of it and make it an executable test (simplest form being //TEST:INTERPRET:.
SuperRonan
force-pushed
the
ronan/feature/canonical-type-equality-constraint
branch
from
e20b1f5 to
6dd988a
Compare
|
The CI failing tests pass when rebased / merged on master (locally for me) |
…straint operator token rather than the sup one.
…ds order in a GenericTypeConstraintDecl. - Added failedEqualityConstraintCanonicalOrder diagnostic note. - type equality constraints are commutative. - Added a test
…ng` with an interpreted test.
Head branch was pushed to by a user without write access
SuperRonan
force-pushed
the
ronan/feature/canonical-type-equality-constraint
branch
from
6dd988a to
af1810d
Compare
Fixes #8439
When checked, generic type equality constraints types are now in a canonical order, allowing for a commutative type equality operator.