Type size v2

[joelimgu]

This pull requests continues the work of @langston-barrett in #34 . I have taken into account the concerns you expressed in that pull request and rebased it to the current main branch.

I was not able to to address this concern:
Can we have a comment pointing to the upstream function this is a port of, similar to how you did for getTypeSizeInBits? as I am not sure if this function is equivalent to the llvm function I have been able to identify (https://llvm.org/doxygen/DataLayout_8cpp_source.html#l00553).

And I would like feedback on how to deal with Type::TargetExtType as we don't have access to the type's data as explained in the enum.

I am no expert on llvm so any feedback is appreciated.

As a last note, I've seen the confusion between bits and bites multiple times in the code (I've hopefully fixed them), but upon successful validation of this code, a bit and byte types should be created moving forward.

[Benjins]

With LLVM 15, typed pointers were removed in favour of untyped pointers. Since the type doesn't matter here, you could have a helper function along the lines of:

fn get_pointer_type(types : &Types) -> TypeRef {
    #[cfg(feature = "llvm-14-or-lower")]
    {
        types.pointer_to(types.i8())
    }
    #[cfg(feature = "llvm-15-or-greater")]
    {
        types.pointer()
    }
}

or something like that, to get it to work with both typed and typeless pointers

[Benjins]

Not the maintainer, but wanted to note a few things:

1. I tried pulling the code, and noticed that there are a couple compiler errors depending on which version of LLVM it uses. I left some more specific comments where they are
2. For the upstream functions, they should all be in the DataLayout.h/DataLayout.cpp files. getTypeStoreSize and getTypeAllocSize for example
3. I did notice that there are some discrepancies in the results between this ported version and the native LLVM version, however those appear to be from differences in how alignment is computed rather than the code in this PR. Specifically, nested structs and some vector types will give different answers in some cases. I'm not entirely sure what the issue is, but it'd probably be good to get some more test cases in either way
4. For some reason CI didn't run? Github says it requires approval but I don't think it usually does. It builds w/ all the different LLVM versions, which catches things like missing version ranges

[joelimgu]

I've taken into account your comments, and added some tests, the CI is not working on the repo bc a maintainer needs to allow it but it runs on my repo for all llvm versions now. Let me know if you have any other remarks, I am happy to apply them!

If not we'll wait for the owner to respond and hopefully merge it if everything is ok! (I might need to clean the git history a bit I've made a mess).

[cdisselkoen]

Looks good, thanks for moving this forward. A few small comments.

[cdisselkoen]

Is this change intentional?

[cdisselkoen]

Is this change intentional?

[cdisselkoen]

What does the LLVM method for get-alignment-of-type do for target ext types? Neither the LangRef for target ext types nor the LangRef for data layout seem to specify the alignment of these types.

[cdisselkoen]

Can we get a doc comment on this struct?

[cdisselkoen]

Instead of quantity, can we have bits or quantity_bits to make clear this is bits not bytes? (Compare to get_type_store_size() above, where not specifying defaults to bytes, but here, not specifying seems to default to bits.)

[cdisselkoen]

I don't think this should be a From; it's not clear why types.into() should produce a pointer type and not some other kind of type. Is it sufficient to use the existing method Types::pointer_to(types.i8())?