Merge branch 'upstream-main' into 'main'

* aosp/upstream-main:
  rust: add test where discriminant is offset from beginning in the variant data layout
  rust: add test with variant having a single member
  rust: add test with an empty enum and optional empty enum
  rust: model `Variant` discriminant as an optional member
  DWARF processor: fix whitespace in conditional expression

Signed-off-by: Giuliano Procida <gprocida@google.com>
Change-Id: Iec16ca936eb08da2c5e4a6423ce739c95592b191
diff --git a/comparison.cc b/comparison.cc
index 8e0dacb..0015fc8 100644
--- a/comparison.cc
+++ b/comparison.cc
@@ -570,9 +570,15 @@
   result.diff_.holds_changes = true;  // Anonymous variants are not allowed.
 
   result.MaybeAddNodeDiff("bytesize", x1.bytesize, x2.bytesize);
-  const auto type_diff =
-      (*this)(x1.discriminant_type_id, x2.discriminant_type_id);
-  result.MaybeAddEdgeDiff("discriminant", type_diff);
+  if (x1.discriminant.has_value() && x2.discriminant.has_value()) {
+    const auto type_diff =
+        (*this)(x1.discriminant.value(), x2.discriminant.value());
+    result.MaybeAddEdgeDiff("discriminant", type_diff);
+  } else if (x1.discriminant.has_value()) {
+    result.AddEdgeDiff("", Removed(x1.discriminant.value()));
+  } else if (x2.discriminant.has_value()) {
+    result.AddEdgeDiff("", Added(x2.discriminant.value()));
+  }
   CompareNodes(result, *this, x1.members, x2.members);
   return result;
 }
diff --git a/dwarf_processor.cc b/dwarf_processor.cc
index 9955b82..17fd682 100644
--- a/dwarf_processor.cc
+++ b/dwarf_processor.cc
@@ -568,7 +568,7 @@
       // struct or union.
       const Id id =
           AddProcessedNode<Variant>(entry, full_name, GetByteSize(entry),
-                                    variant_and_members->discriminant_type_id,
+                                    variant_and_members->discriminant,
                                     std::move(variant_and_members->members));
       AddNamedTypeNode(id);
       return;
@@ -774,16 +774,17 @@
   }
 
   struct VariantAndMembers {
-    Id discriminant_type_id;
+    std::optional<Id> discriminant;
     std::vector<Id> members;
   };
 
   VariantAndMembers GetVariantAndMembers(Entry& entry) {
     std::vector<Id> members;
-    std::optional<Id> discriminant_type_id = std::nullopt;
+    std::optional<Id> discriminant = std::nullopt;
     auto discriminant_entry = entry.MaybeGetReference(DW_AT_discr);
-    if (!discriminant_entry.has_value()) {
-      Die() << "Variant must have a discriminant: " << EntryToString(entry);
+    if (discriminant_entry.has_value()) {
+      discriminant = GetIdForEntry(*discriminant_entry);
+      ProcessMember(*discriminant_entry);
     }
 
     for (auto& child : entry.GetChildren()) {
@@ -791,14 +792,9 @@
       switch (child_tag) {
         case DW_TAG_member: {
           if (child.GetOffset() != discriminant_entry->GetOffset()) {
-            Die() << "Encountered unexpected member for variant: "
+            Die() << "Encountered rogue member for variant: "
                   << EntryToString(entry);
           }
-          discriminant_type_id = GetReferredTypeId(GetReferredType(child));
-          if (GetDataBitOffset(child, 0, is_little_endian_binary_) != 0) {
-            Die() << "Unexpected member location for variant discriminant: "
-                  << EntryToString(child);
-          }
           if (!child.GetFlag(DW_AT_artificial)) {
             Die() << "Variant discriminant must be an artificial member: "
                   << EntryToString(child);
@@ -814,12 +810,7 @@
                 << ", " << EntryToString(child);
       }
     }
-
-    if (!discriminant_type_id.has_value()) {
-      Die() << "No discriminant member found for variant: "
-            << EntryToString(entry);
-    }
-    return VariantAndMembers{.discriminant_type_id = *discriminant_type_id,
+    return VariantAndMembers{.discriminant = discriminant,
                              .members = std::move(members)};
   }
 
@@ -1007,7 +998,7 @@
           // As per this (rejected) proposal, GCC includes parameters as
           // children of this DIE.
           for (auto& child2 : child.GetChildren()) {
-            if (child2.GetTag() ==  DW_TAG_formal_parameter) {
+            if (child2.GetTag() == DW_TAG_formal_parameter) {
               parameters.push_back(GetReferredTypeId(GetReferredType(child2)));
             }
           }
diff --git a/equality.h b/equality.h
index eb47db9..6f96bc1 100644
--- a/equality.h
+++ b/equality.h
@@ -22,6 +22,7 @@
 
 #include <cstddef>
 #include <map>
+#include <optional>
 #include <vector>
 
 #include "graph.h"
@@ -79,6 +80,14 @@
     return result;
   }
 
+  bool operator()(const std::optional<Id>& opt1,
+                  const std::optional<Id>& opt2) {
+    if (opt1.has_value() && opt2.has_value()) {
+      return (*this)(opt1.value(), opt2.value());
+    }
+    return opt1.has_value() == opt2.has_value();
+  }
+
   bool operator()(const std::vector<Id>& ids1, const std::vector<Id>& ids2) {
     bool result = ids1.size() == ids2.size();
     for (size_t ix = 0; result && ix < ids1.size(); ++ix) {
@@ -197,7 +206,7 @@
   bool operator()(const Variant& x1, const Variant& x2) {
     return x1.name == x2.name
         && x1.bytesize == x2.bytesize
-        && (*this)(x1.discriminant_type_id, x2.discriminant_type_id)
+        && (*this)(x1.discriminant, x2.discriminant)
         && (*this)(x1.members, x2.members);
   }
 
diff --git a/fingerprint.cc b/fingerprint.cc
index 05a4cdc..740e237 100644
--- a/fingerprint.cc
+++ b/fingerprint.cc
@@ -139,7 +139,10 @@
   }
 
   HashValue operator()(const Variant& x) {
-    auto h = hash('v', x.name, x.bytesize, (*this)(x.discriminant_type_id));
+    auto h = hash('v', x.name, x.bytesize);
+    if (x.discriminant.has_value()) {
+      todo.insert(x.discriminant.value());
+    }
     ToDo(x.members);
     return h;
   }
diff --git a/graph.h b/graph.h
index f75173a..0955675 100644
--- a/graph.h
+++ b/graph.h
@@ -249,16 +249,16 @@
 };
 
 struct Variant {
-  Variant(const std::string& name, uint64_t bytesize, Id discriminant_type_id,
-          const std::vector<Id>& members)
+  Variant(const std::string& name, uint64_t bytesize,
+          std::optional<Id> discriminant, const std::vector<Id>& members)
       : name(name),
         bytesize(bytesize),
-        discriminant_type_id(discriminant_type_id),
+        discriminant(discriminant),
         members(members) {}
 
   std::string name;
   uint64_t bytesize;
-  Id discriminant_type_id;
+  std::optional<Id> discriminant;
   std::vector<Id> members;
 };
 
diff --git a/proto_reader.cc b/proto_reader.cc
index f2683ef..a83fd10 100644
--- a/proto_reader.cc
+++ b/proto_reader.cc
@@ -227,8 +227,11 @@
 }
 
 void Transformer::AddNode(const Variant& x) {
-  AddNode<stg::Variant>(GetId(x.id()), x.name(), x.bytesize(),
-                        GetId(x.discriminant_type_id()), x.member_id());
+  const auto& discriminant = x.has_discriminant()
+                                 ? std::make_optional(GetId(x.discriminant()))
+                                 : std::nullopt;
+  AddNode<stg::Variant>(GetId(x.id()), x.name(), x.bytesize(), discriminant,
+                        x.member_id());
 }
 
 void Transformer::AddNode(const Function& x) {
diff --git a/proto_writer.cc b/proto_writer.cc
index 919c5e8..16b1010 100644
--- a/proto_writer.cc
+++ b/proto_writer.cc
@@ -269,7 +269,9 @@
   variant.set_id(id);
   variant.set_name(x.name);
   variant.set_bytesize(x.bytesize);
-  variant.set_discriminant_type_id((*this)(x.discriminant_type_id));
+  if (x.discriminant.has_value()) {
+    variant.set_discriminant((*this)(x.discriminant.value()));
+  }
   for (const auto id : x.members) {
     variant.add_member_id((*this)(id));
   }
diff --git a/stable_hash.cc b/stable_hash.cc
index a8f9366..5982072 100644
--- a/stable_hash.cc
+++ b/stable_hash.cc
@@ -161,7 +161,9 @@
 
 HashValue StableHash::operator()(const Variant& x) {
   HashValue hash = hash_('V', x.name, x.bytesize);
-  hash = DecayHashCombine<8>(hash, (*this)(x.discriminant_type_id));
+  if (x.discriminant.has_value()) {
+    hash = DecayHashCombine<12>(hash, (*this)(x.discriminant.value()));
+  }
   return DecayHashCombine<2>(hash,
                              DecayHashCombineInReverse<8>(x.members, *this));
 }
diff --git a/stg.proto b/stg.proto
index 7703b79..5ba2d6c 100644
--- a/stg.proto
+++ b/stg.proto
@@ -205,7 +205,7 @@
   fixed32 id = 1;
   string name = 2;
   uint64 bytesize = 3;
-  fixed32 discriminant_type_id = 4;
+  optional fixed32 discriminant = 4;
   repeated fixed32 member_id = 5;
 }
 
diff --git a/substitution.h b/substitution.h
index 863115f..43768cf 100644
--- a/substitution.h
+++ b/substitution.h
@@ -120,7 +120,9 @@
   }
 
   void operator()(Variant& x) {
-    Update(x.discriminant_type_id);
+    if (x.discriminant.has_value()) {
+      Update(x.discriminant.value());
+    }
     Update(x.members);
   }
 
diff --git a/test_cases/info_tests/variant/expected/negative_discriminant_rs.elf_stg b/test_cases/info_tests/variant/expected/negative_discriminant_rs.elf_stg
index 748f73e..021539c 100644
--- a/test_cases/info_tests/variant/expected/negative_discriminant_rs.elf_stg
+++ b/test_cases/info_tests/variant/expected/negative_discriminant_rs.elf_stg
@@ -19,6 +19,10 @@
   bytesize: 0x00000008
 }
 member {
+  id: 0x18ba9281
+  type_id: 0xedc43a15  # i64
+}
+member {
   id: 0x976dcda7
   name: "__0"
   type_id: 0xd4bacb77  # u32
@@ -69,25 +73,25 @@
   }
 }
 variant {
-  id: 0x29673df8
+  id: 0x298b726b
   name: "negative_discriminant::Foo"
   bytesize: 16
-  discriminant_type_id: 0xedc43a15
+  discriminant: 0x18ba9281
   member_id: 0x72c55dce
   member_id: 0x528ee922
   member_id: 0x27839c0b
 }
 function {
-  id: 0xb409a4c0
+  id: 0xb432b724
   return_type_id: 0x62aebfd4  # bool
-  parameter_id: 0x29673df8  # variant negative_discriminant::Foo
+  parameter_id: 0x298b726b  # variant negative_discriminant::Foo
 }
 elf_symbol {
   id: 0x0f95dadc
   name: "is_minus_one"
   is_defined: true
   symbol_type: FUNCTION
-  type_id: 0xb409a4c0  # bool(variant negative_discriminant::Foo)
+  type_id: 0xb432b724  # bool(variant negative_discriminant::Foo)
   full_name: "negative_discriminant::is_minus_one"
 }
 interface {
diff --git a/test_cases/info_tests/variant/expected/offset_discriminant_rs.elf_stg b/test_cases/info_tests/variant/expected/offset_discriminant_rs.elf_stg
new file mode 100644
index 0000000..e7ab069
--- /dev/null
+++ b/test_cases/info_tests/variant/expected/offset_discriminant_rs.elf_stg
@@ -0,0 +1,106 @@
+version: 0x00000002
+root_id: 0x84ea5130  # interface
+primitive {
+  id: 0x384f7d7c
+  name: "char"
+  encoding: UTF
+  bytesize: 0x00000004
+}
+primitive {
+  id: 0x62aebfd4
+  name: "bool"
+  encoding: BOOLEAN
+  bytesize: 0x00000001
+}
+primitive {
+  id: 0xd4bacb77
+  name: "u32"
+  encoding: UNSIGNED_INTEGER
+  bytesize: 0x00000004
+}
+member {
+  id: 0x16e523e2
+  type_id: 0xd4bacb77  # u32
+  offset: 32
+}
+member {
+  id: 0x97813cf0
+  name: "__0"
+  type_id: 0x384f7d7c  # char
+}
+member {
+  id: 0xdbc0cd2f
+  name: "__1"
+  type_id: 0x384f7d7c  # char
+  offset: 32
+}
+variant_member {
+  id: 0x56b9f935
+  name: "Two"
+  type_id: 0x573dc947
+}
+variant_member {
+  id: 0x69cfc441
+  name: "One"
+  discriminant_value: 1114112
+  type_id: 0x988e2459
+}
+variant_member {
+  id: 0x276cfc01
+  name: "Zero"
+  discriminant_value: 1114113
+  type_id: 0xb2cd8432
+}
+struct_union {
+  id: 0x988e2459
+  kind: STRUCT
+  name: "offset_discriminant::Foo::One"
+  definition {
+    bytesize: 8
+    member_id: 0x97813cf0  # char __0
+  }
+}
+struct_union {
+  id: 0x573dc947
+  kind: STRUCT
+  name: "offset_discriminant::Foo::Two"
+  definition {
+    bytesize: 8
+    member_id: 0x97813cf0  # char __0
+    member_id: 0xdbc0cd2f  # char __1
+  }
+}
+struct_union {
+  id: 0xb2cd8432
+  kind: STRUCT
+  name: "offset_discriminant::Foo::Zero"
+  definition {
+    bytesize: 8
+  }
+}
+variant {
+  id: 0x82b2aa29
+  name: "offset_discriminant::Foo"
+  bytesize: 8
+  discriminant: 0x16e523e2
+  member_id: 0x56b9f935
+  member_id: 0x69cfc441
+  member_id: 0x276cfc01
+}
+function {
+  id: 0x9efcc134
+  return_type_id: 0x62aebfd4  # bool
+  parameter_id: 0x82b2aa29  # variant offset_discriminant::Foo
+}
+elf_symbol {
+  id: 0x699cebd9
+  name: "is_zero"
+  is_defined: true
+  symbol_type: FUNCTION
+  type_id: 0x9efcc134  # bool(variant offset_discriminant::Foo)
+  full_name: "offset_discriminant::is_zero"
+}
+interface {
+  id: 0x84ea5130
+  symbol_id: 0x699cebd9  # bool offset_discriminant::is_zero(variant offset_discriminant::Foo)
+}
diff --git a/test_cases/info_tests/variant/expected/optional_empty_rs.elf_stg b/test_cases/info_tests/variant/expected/optional_empty_rs.elf_stg
new file mode 100644
index 0000000..0584271
--- /dev/null
+++ b/test_cases/info_tests/variant/expected/optional_empty_rs.elf_stg
@@ -0,0 +1,65 @@
+version: 0x00000002
+root_id: 0x84ea5130  # interface
+primitive {
+  id: 0x62aebfd4
+  name: "bool"
+  encoding: BOOLEAN
+  bytesize: 0x00000001
+}
+member {
+  id: 0x9758c0a4
+  name: "__0"
+  type_id: 0xe1b3299b  # variant optional_empty::Empty
+}
+variant_member {
+  id: 0x5212c510
+  name: "None"
+  type_id: 0x3d2f2a96
+}
+variant_member {
+  id: 0x3107bf38
+  name: "Some"
+  type_id: 0x0148ce39
+}
+struct_union {
+  id: 0x3d2f2a96
+  kind: STRUCT
+  name: "core::option::Option<optional_empty::Empty>::None"
+  definition {
+  }
+}
+struct_union {
+  id: 0x0148ce39
+  kind: STRUCT
+  name: "core::option::Option<optional_empty::Empty>::Some"
+  definition {
+    member_id: 0x9758c0a4  # variant optional_empty::Empty __0
+  }
+}
+variant {
+  id: 0xc03763b2
+  name: "core::option::Option<optional_empty::Empty>"
+  member_id: 0x5212c510
+  member_id: 0x3107bf38
+}
+variant {
+  id: 0xe1b3299b
+  name: "optional_empty::Empty"
+}
+function {
+  id: 0x8e5db352
+  return_type_id: 0x62aebfd4  # bool
+  parameter_id: 0xc03763b2  # variant core::option::Option<optional_empty::Empty>
+}
+elf_symbol {
+  id: 0xbc9edef7
+  name: "is_none"
+  is_defined: true
+  symbol_type: FUNCTION
+  type_id: 0x8e5db352  # bool(variant core::option::Option<optional_empty::Empty>)
+  full_name: "optional_empty::is_none"
+}
+interface {
+  id: 0x84ea5130
+  symbol_id: 0xbc9edef7  # bool optional_empty::is_none(variant core::option::Option<optional_empty::Empty>)
+}
diff --git a/test_cases/info_tests/variant/expected/simple_rs.elf_stg b/test_cases/info_tests/variant/expected/simple_rs.elf_stg
index 0d051aa..8473682 100644
--- a/test_cases/info_tests/variant/expected/simple_rs.elf_stg
+++ b/test_cases/info_tests/variant/expected/simple_rs.elf_stg
@@ -25,6 +25,10 @@
   bytesize: 0x00000004
 }
 member {
+  id: 0x288078fc
+  type_id: 0x2d2f93e0  # u8
+}
+member {
   id: 0x976dc47d
   name: "__0"
   type_id: 0xd4bacb77  # u32
@@ -102,25 +106,25 @@
   }
 }
 variant {
-  id: 0x15514ee1
+  id: 0x157ee975
   name: "simple::Foo"
   bytesize: 12
-  discriminant_type_id: 0x2d2f93e0
+  discriminant: 0x288078fc
   member_id: 0xfc632c0e
   member_id: 0xad130615
   member_id: 0xc48d72bf
 }
 function {
-  id: 0xbb043806
+  id: 0xbb0fd1e3
   return_type_id: 0x62aebfd4  # bool
-  parameter_id: 0x15514ee1  # variant simple::Foo
+  parameter_id: 0x157ee975  # variant simple::Foo
 }
 elf_symbol {
   id: 0x4e2f2fc8
   name: "is_unit"
   is_defined: true
   symbol_type: FUNCTION
-  type_id: 0xbb043806  # bool(variant simple::Foo)
+  type_id: 0xbb0fd1e3  # bool(variant simple::Foo)
   full_name: "simple::is_unit"
 }
 interface {
diff --git a/test_cases/info_tests/variant/expected/singleton_rs.elf_stg b/test_cases/info_tests/variant/expected/singleton_rs.elf_stg
new file mode 100644
index 0000000..ed52414
--- /dev/null
+++ b/test_cases/info_tests/variant/expected/singleton_rs.elf_stg
@@ -0,0 +1,42 @@
+version: 0x00000002
+root_id: 0x84ea5130  # interface
+primitive {
+  id: 0x62aebfd4
+  name: "bool"
+  encoding: BOOLEAN
+  bytesize: 0x00000001
+}
+variant_member {
+  id: 0xfc304e1e
+  name: "Unit"
+  type_id: 0x312c753c
+}
+struct_union {
+  id: 0x312c753c
+  kind: STRUCT
+  name: "singleton::Singleton::Unit"
+  definition {
+  }
+}
+variant {
+  id: 0x9063b3ee
+  name: "singleton::Singleton"
+  member_id: 0xfc304e1e
+}
+function {
+  id: 0x9a488745
+  return_type_id: 0x62aebfd4  # bool
+  parameter_id: 0x9063b3ee  # variant singleton::Singleton
+}
+elf_symbol {
+  id: 0x4e2f2fc8
+  name: "is_unit"
+  is_defined: true
+  symbol_type: FUNCTION
+  type_id: 0x9a488745  # bool(variant singleton::Singleton)
+  full_name: "singleton::is_unit"
+}
+interface {
+  id: 0x84ea5130
+  symbol_id: 0x4e2f2fc8  # bool singleton::is_unit(variant singleton::Singleton)
+}
diff --git a/test_cases/info_tests/variant/offset_discriminant.rs b/test_cases/info_tests/variant/offset_discriminant.rs
new file mode 100644
index 0000000..a7aca13
--- /dev/null
+++ b/test_cases/info_tests/variant/offset_discriminant.rs
@@ -0,0 +1,13 @@
+pub enum Foo {
+    Two(char, char),
+    One(char),
+    Zero,
+}
+
+#[no_mangle]
+pub fn is_zero(foo: Foo) -> bool {
+    match foo {
+        Foo::Zero => true,
+        _ => false,
+    }
+}
diff --git a/test_cases/info_tests/variant/optional_empty.rs b/test_cases/info_tests/variant/optional_empty.rs
new file mode 100644
index 0000000..1cda07c
--- /dev/null
+++ b/test_cases/info_tests/variant/optional_empty.rs
@@ -0,0 +1,9 @@
+pub enum Empty {}
+
+#[no_mangle]
+pub fn is_none(opt: Option<Empty>) -> bool {
+    match opt {
+        None => true,
+        _ => false,
+    }
+}
diff --git a/test_cases/info_tests/variant/singleton.rs b/test_cases/info_tests/variant/singleton.rs
new file mode 100644
index 0000000..dd66c40
--- /dev/null
+++ b/test_cases/info_tests/variant/singleton.rs
@@ -0,0 +1,10 @@
+pub enum Singleton {
+    Unit,
+}
+
+#[no_mangle]
+pub fn is_unit(s: Singleton) -> bool {
+    match s {
+        Singleton::Unit => true,
+    }
+}
diff --git a/type_normalisation.cc b/type_normalisation.cc
index aeac699..292957b 100644
--- a/type_normalisation.cc
+++ b/type_normalisation.cc
@@ -146,7 +146,9 @@
   }
 
   void operator()(const Variant& x, Id) {
-    (*this)(x.discriminant_type_id);
+    if (x.discriminant.has_value()) {
+      (*this)(x.discriminant.value());
+    }
     (*this)(x.members);
   }
 
diff --git a/unification.cc b/unification.cc
index 0d77012..21ba792 100644
--- a/unification.cc
+++ b/unification.cc
@@ -20,6 +20,7 @@
 #include "unification.h"
 
 #include <cstddef>
+#include <optional>
 #include <utility>
 
 #include "graph.h"
@@ -79,6 +80,14 @@
     return true;
   }
 
+  bool operator()(const std::optional<Id>& opt1,
+                  const std::optional<Id>& opt2) {
+    if (opt1.has_value() && opt2.has_value()) {
+      return (*this)(opt1.value(), opt2.value());
+    }
+    return opt1.has_value() == opt2.has_value();
+  }
+
   bool operator()(const std::vector<Id>& ids1, const std::vector<Id>& ids2) {
     bool result = ids1.size() == ids2.size();
     for (size_t ix = 0; result && ix < ids1.size(); ++ix) {
@@ -208,7 +217,7 @@
   Winner operator()(const Variant& x1, const Variant& x2) {
     return x1.name == x2.name
         && x1.bytesize == x2.bytesize
-        && (*this)(x1.discriminant_type_id, x2.discriminant_type_id)
+        && (*this)(x1.discriminant, x2.discriminant)
         && (*this)(x1.members, x2.members)
         ? Right : Neither;
   }