[stdlib] List.__eq__ and List.__ne__ for Rhs_t:ComparableCollectionElement

[rd4com]

Hello,

here is an implementation of List.__eq__ and List.__ne__ that makes it possible to do:

var x = List(1,2,3)
var y = List(1,2,3)
if x == y: print("same values")
if x != y: print("not same values")

 

One of the downside is that part of the implementation is done with constrained,

this is not ideal because users won't be aware of some errors until m̀ojo build main.mojo shows the constrained failed.

But it is necessary to have the constrained because rhs could be a list[Bool] and self a list[Int], for example.

[ChristopherLR]

Suggested change

	assert_false(a == d)
	assert_false(a == d)
	assert_not_equal(a, d)

[rd4com]

assert_not_equal require the arguments to be Testable which includes EqualityComparable,

this would not work because List is not EqualityComparable and Stringable;

What the __eq__ implementation does is to exist if T of a specific list is ComparableCollectionElement.

 

(see fn __str__[U: RepresentableCollectionElement](self: List[U]) -> String: of List)

It would require the "conditional conformance"

[ChristopherLR]

Thanks for the info! I'm assuming that at some stage assert_equal will work for collections? I wonder If it's worth (in a separate MR) attempting to enhance the testing library to work for collections that contain ComparableCollectionElement?

[rd4com]

Not sure, @ChristopherLR , if you think it's a good idea you can always develop it

[rd4com]

Do not accept the PR please even if test passes,

@soraros pointed out that rebind should not work with memory-only types!

[JoeLoser]

Do not accept the PR please even if test passes,

@soraros pointed out that rebind should not work with memory-only types!

@soraros do you mind elaborating given the test case with a memory only type? @Mogball do you have any suggestions in not using rebind here and other places we're trying to implement these sort of things without conditional conformance?

[rd4com]

When @soraros offered that review on another PR, rebind was used on value:T(CollectionElement),
now, rebind is used on the Reference[T], which is register_passable (not memory only).

This is also why there is a: [stdlib] Fix Tuple.__contains__ PR (#2782 )

see VariadicPack.get_element:

mojo/stdlib/src/builtin/builtin_list.mojo

Line 579 in bf73717

fn get_element[

[JoeLoser]

When @soraros offered that review on another PR, rebind was used on value:T(CollectionElement), now, rebind is used on the Reference[T], which is register_passable (not memory only).

This is also why there is a: [stdlib] Fix Tuple.__contains__ PR (#2782 )

see VariadicPack.get_element:

mojo/stdlib/src/builtin/builtin_list.mojo

Line 579 in bf73717

fn get_element[

Makes sense, thanks for clarifying! So, if I understand correctly, this PR should be ready to go then, yeah? If so, do you mind resolving the merge conflicts?

[Mogball]

Do not accept the PR please even if test passes,
@soraros pointed out that rebind should not work with memory-only types!

@soraros do you mind elaborating given the test case with a memory only type? @Mogball do you have any suggestions in not using rebind here and other places we're trying to implement these sort of things without conditional conformance?

My suggestion is to never use rebind. We should rename it to __rebind

[rd4com]

I'm happy to follow @Mogball 's expert analysis,
let's just close the PR and revisit the idea once new features lands,
i would not be comfortable forming a reference out of an bitcasted UnsafePointer,
here is a suggestion for the renaming of rebind: UnsafeRebind 👍 (ping to @Mogball)
@JoeLoser can i close this PR ?

[JoeLoser]

Question Do we need ComparableCollectionElement or would EqualityCollectionElement work instead (CollectionElement + EqualityComparable)?

[JoeLoser]

Suggestion Want to use alias instead here for the variables and we can make sure eq and ne work in comp-time?

[rd4com]

@JoeLoser , did my best to implement both reviews, the tests do not passes