ir: proposal: typecheck more things

[pwaller]

I occasionally get the order of params wrong to a store, passing the pointer in place of the value and vice versa. It would be neat if this panic'd.

Status: we agreed that typechecking is useful thing to do.
Progress: partially done, but only attacking things as they crop up. Feel free to comment to add to the list.

• trunc (done in Panic if trunc operands are not compatible #71)
• insertvalue (done in Type check elem in insertvalue #70)
• store (done in Type check NewStore operands #72 - BUT are vector operands legal?)
• load (beware vector operands)
• bitcast
• inttoptr, ptrtoint
• inlineasm type parameter must be a func type (or pointer(?) depending on ir: allow callee of function type in NewCall #74 discussion).

[mewmew]

I understand that the arguments are easy to mix up, since they are in the reverse order of e.g. memcpy and friends. The rationale for the given order is to mimic the order of occurrence in the LLVM assembly. https://llvm.org/docs/LangRef.html#store-instruction

The proposed sanity check sounds good to me. Essentially it would check that src.Type() is equal to dst.Type().(*types.PointerType).Elem, and if not panic, preferably with a user friendly error message.

func NewStore(src, dst value.Value) *InstStore

@pwaller Care to prepare a PR?

[mewmew]

Thinking a bit more about this, I remember earlier thoughts and design ideas to defer type checking to a dedicated semantic analysis stage.

We can add the proposed NewStore type check now, but it may go away in the future if we decide to implement sem.Verify.

The main rationale for separating IR construction and semantic analysis is that sometimes you may want to add an incomplete value as an argument to an instruction. The type of this incomplete value may not be readily available until later during IR construction, and forcing users to have it pre-computed, a-prior limits the use of the IR construction API.

To still provide the same safety guarantees, a later stage would then perform type checking and other semantic analysis and verification on the IR, once the users decides it is complete and ready for type checking (i.e. when the user decides to invoke sem.Verify.

// Package sem performs semantic analysis of LLVM IR modules.
package sem

// Verify performs type-checking and semantic analysis of the given module to
// verify consistency and correctness.
func Verify(m *ir.Module) error

I have run into this a few times in the past, and without that separation, you essentially run into a catch22 chicken or the egg problem. Which I had a more concrete example to provide, but it's just based on hours of struggling with these kind of issues in the past.

[pwaller]

So, the reason I proposed NewStore to panic is:

1. I want the code to panic at the point of the error, so that I can find the error early and easily.
2. We do already have access to a functioning verifier (opt -verify).

With respect to incomplete types, I understand where you're coming from. I had particular fun implementing recursive types.

What about accepting some sort of wrapper or sentinel when you really want to defer the typing?

[mewmew]

• I want the code to panic at the point of the error, so that I can find the error early and easily.

Make sense. Feel free to add the panic. If we later discover that this is limiting IR construction too much, we can figure out specifics related to the wrapper/sentinel that you mentioned.

• We do already have access to a functioning verifier (opt -verify).

Fair point.

[pwaller]

I'm also finding it would be useful to check that the arguments to bitcast, inttoptr and ptrtoint have the right form.

[pwaller]

I also added type checking of insertvalue. I am finding these really useful for catching bugs early. I figure if there are circumstances where it is useful to construct things with incomplete types, then those can be done by constructing the values of these directly rather than through their New* constructors. What do you think?

[mewmew]

I figure if there are circumstances where it is useful to construct things with incomplete types, then those can be done by constructing the values of these directly rather than through their New* constructors. What do you think?

That's a good point! I agree.

[mewmew]

Marking this as future milestone until we have a clear release target in mind.

[mewmew]

re (#65 (comment)): inlineasm type parameter must be a func type (or pointer(?) depending on #74 discussion).

Pointer to function it is.

Resource located by @dannypsnl:

From https://stackoverflow.com/questions/29184031/how-to-get-llvm-inline-asm-operands-type:

In LLVM, InlineAsm is a subclass of Value, and the associated value is always of type pointer-to-function.