carbon-lang icon indicating copy to clipboard operation
carbon-lang copied to clipboard

Remove indirection for builtin operator tests

Open jonmeow opened this issue 3 years ago • 4 comments

Not sure if there was something behind the indirection...?

jonmeow avatar Aug 25 '22 16:08 jonmeow

The idea of these tests was to make sure these interfaces are properly implemented for builtin types. The revised tests would pass if the interfaces don't work for builtin types but the operators are instead special-cased for those types.

zygoloid avatar Sep 02 '22 00:09 zygoloid

The idea of these tests was to make sure these interfaces are properly implemented for builtin types. The revised tests would pass if the interfaces don't work for builtin types but the operators are instead special-cased for those types.

Can you explain to me how that differs from the existing template code? How does the existing template code force the i32 builtin to be used? (that is, how does it prevent writing an interface for the type if the alternative doesn't?)

jonmeow avatar Sep 02 '22 16:09 jonmeow

// CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/operators/mul_builtin.carbon:[[@LINE+2]]: type error in `*`:
// CHECK: could not find implementation of interface MulWith(U = T:! MulWith(i32) where .Self.Result == i32) for T:! MulWith(i32) where .Self.Result == i32
fn DoMul[T:! MulWith(i32) where .Result == i32](x: T, y: T) -> T { return x * y; }

I think I'm starting to understand what you mean, but this makes me wonder if this is behaving as intended...

jonmeow avatar Sep 06 '22 05:09 jonmeow

// CHECK: COMPILATION ERROR: {{.*}}/explorer/testdata/operators/mul_builtin.carbon:[[@LINE+2]]: type error in `*`:
// CHECK: could not find implementation of interface MulWith(U = T:! MulWith(i32) where .Self.Result == i32) for T:! MulWith(i32) where .Self.Result == i32
fn DoMul[T:! MulWith(i32) where .Result == i32](x: T, y: T) -> T { return x * y; }

I think I'm starting to understand what you mean, but this makes me wonder if this is behaving as intended...

I think the problem here was that you're asking for a T that can be multiplied with an i32, but you're multiplying it with a T instead. The error message isn't great but I think explorer is correct to reject this.

zygoloid avatar Sep 08 '22 22:09 zygoloid

@zygoloid Pinging here, did you have any remaining concerns?

jonmeow avatar Sep 14 '22 17:09 jonmeow