[PATCH] D42082: Add DWARF for discriminated unions

[Paul Robinson via Phabricator via llvm-commits]

probinson added a comment.

In https://reviews.llvm.org/D42082#988557, @tromey wrote:

> In https://reviews.llvm.org/D42082#988490, @probinson wrote:
>
> >
>
>
>
>
> > I found this example, which sure looks like multiple members in a variant, here:
> >  https://stackoverflow.com/questions/29248665/why-does-rust-not-have-unions
>
> In cases like this, a separate type is emitted for the contents of each variant.
>  So from the DWARF point of view there is just a single variant in the variant part, but this variant may be a structure.


Ok.

I am not super comfortable with the metadata APIs but a couple of things struck me, see inline comments.



================
Comment at: lib/Bitcode/Reader/MetadataLoader.cpp:1249
   case bitc::METADATA_COMPOSITE_TYPE: {
-    if (Record.size() != 16)
+    if (Record.size() < 16 && Record.size() > 17)
       return error("Invalid record");
----------------
You want `||` not `&&` here?


================
Comment at: lib/IR/Verifier.cpp:971
+
+  if (N.getRawDiscriminator()) {
+    AssertDI(isa<DIDerivedType>(N.getRawDiscriminator()) &&
----------------
```
if (auto *D = N.getRawDiscriminator()) {
  AssertDI(isa<DIDerivedType>(D) && ...
```



Repository:
  rL LLVM

https://reviews.llvm.org/D42082