Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Representation of 0-tuples #184

Closed
andyjansson opened this issue May 6, 2020 · 4 comments · Fixed by #185
Closed

Representation of 0-tuples #184

andyjansson opened this issue May 6, 2020 · 4 comments · Fixed by #185

Comments

@andyjansson
Copy link

Tuples can be represented with DebugStructType, which works fine with one exception: 0-tuples. When attempting to create an empty struct, I'm greeted with the argument exception 'Type must be sized to get target size information'. This leaves me with the question of how I would best go about representing this structure. Any advice?

@smaillet
Copy link
Member

smaillet commented May 6, 2020

Can you provide more specific examples of your code? Empty structs are a valid concept in many languages and should be possible in LLVM (and the DWARF inspired debug info it uses).

LLVM is unfortunately not particularly good about documenting it's parameter requirements and doesn't generate errors on invalid ones - it just crashes in release builds or asserts and aborts the process in a debug build. So a lot of the work in the .NET bindings is in doing parameter checks and creating meaningful exceptions and exception messages. It's possible this is a case of overly aggressive checks in the .NET projection.

@andyjansson
Copy link
Author

The issue is demonstrated by the following snippet:

new DebugStructType(
  module,
  "tuple",
  module.DICompileUnit,
  "tuple",
  null,
  0,
  DebugInfoFlags.None,
  Enumerable.Empty<DebugMemberInfo>()
);

Here's the relevant exception and stack trace:

Exception has occurred: CLR/System.ArgumentException
Exception thrown: 'System.ArgumentException' in Ubiquity.NET.Llvm.dll: 'Type must be sized to get target size information'
   at Ubiquity.NET.Llvm.DataLayout.VerifySized(ITypeRef type, String name)
   at Ubiquity.NET.Llvm.DataLayout.BitSizeOf(ITypeRef typeRef)
   at Ubiquity.NET.Llvm.DebugInfo.DebugStructType..ctor(BitcodeModule module, String nativeName, DIScope scope, String name, DIFile file, UInt32 line, DebugInfoFlags debugFlags, IEnumerable`1 debugElements, DIType derivedFrom, Boolean packed, Nullable`1 bitSize, UInt32 bitAlignment)

@smaillet smaillet added the bug label May 6, 2020
@smaillet
Copy link
Member

smaillet commented May 6, 2020

OK, that looks like a bug, Context.CreateStructType(string,bool,params ITypeRef[]) skips setting the body, when the element count is 0. This is unnecessary as the underlying LLVM APIs will accept the 0 length and mark the type as sized (with a size of 0).

Possible workaround: (I haven't tested it as I need to get to my day job [Been looking at this while waiting on a machine OS update]) But you should be able to create a native struct type, set the body to 0, then use that to create the DebugStruct type (e.g. attach the debug info from the existing native IR type that is a sized empty struct)

var lirType = module.Context.CreateStructType("tuple");
irType.SetBody(false);
var debugType = new DebugStructType(
  llvmType: irType,
 module: module,
 scope: null,
 name:"tuple",
 file: null,
 line: 0,
debugFlags: default
derivedFrom: null
elements: Enumerable.Empty<DIType>());

NOTE: There are two overloads of the DebugStructType constructor that take an IStructType as the first parameter. The one with fewer parameters creates a replaceable composite type, which you probably don't want. (Which is why the example above uses the named parameters to be clearer on which one you need)

@andyjansson
Copy link
Author

That worked a treat, thank you very much.

smaillet added a commit to smaillet/Llvm.NET that referenced this issue May 7, 2020
Corrected behavior and documentation for overloads of Context.CreateStructType to allow creating empty sized structures (Anonymous or named) This is technically a breaking change so the release notes are updated to reflect that. Though the expectation is that there isn't anything depending on the current and odd behavior (effectively declaring packed but not providing any members for the body is a contradiction and should not have been allowed to create a forward reference, especially since a simpler single param overload was already available)
@smaillet smaillet linked a pull request May 7, 2020 that will close this issue
smaillet added a commit that referenced this issue May 7, 2020
Corrected behavior and documentation for overloads of Context.CreateStructType to allow creating empty sized structures (Anonymous or named) This is technically a breaking change so the release notes are updated to reflect that. Though the expectation is that there isn't anything depending on the current and odd behavior (effectively declaring packed but not providing any members for the body is a contradiction and should not have been allowed to create a forward reference, especially since a simpler single param overload was already available)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants