Compiler crash trying foreach on distinct pointers

[chri-k]

Bug

If you try to use foreach on a distinct pointer implementing .len() (such as ZString), the compiler crashes with

FATAL ERROR Should be unreachable -> in llvm_emit_len_for_expr @ in /usr/src/debug/c3c-git/c3c/src/compiler/llvm_codegen_expr.c:2834 

Occurs on latest version (git c8018c5)

Reproduction

distinct Type = char*;
fn usz Type.len(str)
{
	return 0;
}
fn int main(String[] args)
{
  Type x = "AAAAA";  // Or ZString
  foreach(y : x)
  {
    
  }
  return 0;
}

[lerno]

This should now work, please check.

[chri-k]

The crash is fixed.

I realized i made a slight mistake in my code, i forgot to add @operator(len)

In that circumstance the behavior is correct, but now an error is emitted even with corrected code, which directly contradicts

In order to use a type with foreach, e.g. foreach(d : foo), at a minimum [] and len need to be implemented.

[Caleb-o]

@chri-k I was able to get this to compile:

distinct Type = char*;

fn char Type.get(self, usz index) @operator([]) {
	return self[index];
}

fn usz Type.len(self) @operator(len)
{
	return 0;
}

fn int main() {
	Type x = "AAAAA";  // Or ZString
	foreach(y : x)
	{
		
	}
	return 0;
}

[Caleb-o]

I have found an odd issue after playing with this, using this as an example:

distinct TypeA = char[];

fn char TypeA.get(self, usz i) @operator([]) {
	io::printfn("index = %s", i);
	return self[i];
}

fn usz TypeA.len(self) @operator(len)
{
	return self.len;
}

fn int main() {
	TypeA x = "AAAAA";  // Or ZString
	foreach(y : x)
	{
		io::printfn("%s", y);
	}
	return 0;
}

Which then results in a stack overflow:

Did I not do this right?

[chri-k]

@chri-k I was able to get this to compile:

distinct Type = char*;

fn char Type.get(self, usz index) @operator([]) {
	return self[index];
}

fn usz Type.len(self) @operator(len)
{
	return 0;
}

fn int main() {
	Type x = "AAAAA";  // Or ZString
	foreach(y : x)
	{
		
	}
	return 0;
}

Oh, i tried doing that and got some weird compilation issue so i assumed you can't index self inside of []. Apparently i was accidentally using &self, so that failing makes perfect sense.

I still don't think an explicit "[] should do self[]" should be needed

[Caleb-o]

@chri-k it seems I can get rid of the get function for it to work, but I feel like this should be an error as I was allowed to implement it.

[lerno]

Ok, there is a bug, but it's not really in the output @Caleb-o!

What would happen would be that the code would ignore the operator(len) code, which was a bug, but then it would correctly call operator([])

The bug in your code is here:

fn char Type.get(self, usz index) @operator([]) {
	return self[index];
}

Since that one is equal to:

fn char Type.get(self, usz index) @operator([]) {
	return self.get(index);
}

So infinite recursion. The correct way to write it is:

fn char Type.get(self, usz index) @operator([]) {
	return ((char[])self)[index];
}

As this bypasses the distinct (and its operator overload).

The bug with "len" is fixed now though. Try it again.

[Caleb-o]

Ok, there is a bug, but it's not really in the output @Caleb-o!

What would happen would be that the code would ignore the operator(len) code, which was a bug, but then it would correctly call operator([])

The bug in your code is here:

fn char Type.get(self, usz index) @operator([]) {
	return self[index];
}

Since that one is equal to:

fn char Type.get(self, usz index) @operator([]) {
	return self.get(index);
}

So infinite recursion. The correct way to write it is:

fn char Type.get(self, usz index) @operator([]) {
	return ((char[])self)[index];
}

As this bypasses the distinct (and its operator overload).

The bug with "len" is fixed now though. Try it again.

Ah okay! That makes sense to why it would just spam index 0 then blow out the stack. I'm glad it still managed to point out a bug though!

[chri-k]

So my assumption that using self[] in [] is wrong was correct, but it should also be a warning or error since it's a really easy mistake (and if you want recursion you can call the method by name). But that's a separate issue.

[lerno]

It can't be an error, because then String foo; char x = foo[2]; would stop working.

[lerno]

One obvious thing one could do was to prohibit overriding get for distinct variants of types that in themselves may be iterable, but I don't know if that is ideal.

[Caleb-o]

If the distinct variant itself is iterable, does it make much sense for it to allow an implementation of iterable?

[lerno]

It could. For example, we could create an "even slice" where get maps to i * 2 (and len is half of the wrapped type).

[Caleb-o]

Hmmm. With my case, how might you ensure that it doesn't get written as I did? My thought was, "I'm passing self, which is just a slice, so I must be able to simply index it". But that wasn't the case and I had to change it so make it work as expected and not recurse.

[lerno]

Well you DO pass in the self, but the self is a distinct type TypeA. So it will always prefer the override that TypeA has put, right?

This is just a pitfall when overriding [] and len by the way.

[Caleb-o]

Is there a way with contracts or reflection etc, to know whether something is iterable?

[lerno]

$defined(foo[0]) &&& ($defined(foo.len) ||| $defined(foo.len())) should work well.

[lerno]

I was thinking of a type property as well, but it's not available right now.

[Caleb-o]

Since it's something more specific, as it's the only thing that allows overloading, it might be nice to have a type property and such.

[chri-k]

It can't be an error, because then String foo; char x = foo[2]; would stop working.

...it would not? i think you misunderstood something.

I meant, add a warning when self[] would cause recursion. But still, separate issue.

[chri-k]

This is getting a bit derailed.
What's the answer on counting the implicit [] of distinct pointer types as a suitable @operator([]) for foreach, so that you can foreach a ZString for example?

[lerno]

Would that not require fairly detailed analysis? For example:

fn char Type.get(self, usz index) @operator([]) {
    if (index % 2 == 0) return ((char[])self)[index];
   return self[index - 1];
}

This would not have infinite recursion but is recursive.

[chri-k]

I meant to always warn about self[]. But i guess there's no nice way to silence the warning (besides using self.get by name), so maybe just put a disclaimer in the documentation on overloading []?

[lerno]

If you want to add foreach to ZString:

macro usz ZString.actual_len(self) @operator(len)
{
  return self.len;
}

macro char ZString.get_element(self, usz index) @operator([])
{
  return ((char*)self)[index];
}

[lerno]

BUT I would consider that a bad idea, since zstr.str_view() is already iterable.

[chri-k]

i guess you could argue that there is no real reason to iterate over a pointer. I mostly-accept that argument ( that section of the docs is still inaccurate though )

In any case, this issue is resolved now.

[lerno]

Which section @chri-k

[chri-k]

The one that says you need to have [] and len defined for foreach to work.
Inaccurate was the wrong word, it's accurate but just misleading.
( distinct pointer with len does have len and you can use [] on it, but it doesn't qualify because [] was not defined on it ).

[lerno]

Is it in some other place than this:

by implementing .len and [] methods and annotating them using the @operator attribute

[chri-k]

I quoted it earlier:

In order to use a type with foreach, e.g. foreach(d : foo), at a minimum [] and len need to be implemented.

It's the first sentence of the section ( and people often only read that ).
Or did you fix that one? if so, sorry for not checking!

[lerno]

My quote was from elsewhere.

I tried to clarify it somewhat on the page you found, I hope it's better now.

[chri-k]

thank you!