Re: [PATCH 2/2] WIP union hir-lowering and type support

[Mark Wielaard <mark@klomp.org>]

[-- Attachment #1: Type: text/plain, Size: 1122 bytes --]

On Sun, Aug 01, 2021 at 01:29:16PM +0200, Mark Wielaard wrote:
> I added two simple testcases to show the basic support for unions
> works now. But there are a couple of things that don't work correctly
> for unions yet. In particular when enabling warnings for the new
> union.rs testcase you'll get:
> 
> $ gcc/gccrs -Bgcc -g union.rs 
> union.rs:18:3: warning: field is never read: ‘f1’
>    18 |   f1: U,
>       |   ^
> union.rs:19:3: warning: field is never read: ‘f2’
>    19 |   f2: V
>       |   ^
> 
> But those (union) fields in the struct are read. Similarly unused
> union fields aren't detected.

This is why the testcase uses { dg-options "-w" } to suppress all
warnings.

Attached is a small followup patch to resolve two FIXMEs to add the
correct locus for the Union variant fields (the actual fixme was fixed
by Thomas Young in commit 6d7b87f9dd92 "make struct field carry the
location info"). But it was missing TupleField and (obviously)
Union variants since those weren't implemented yet.

Also on
https://code.wildebeest.org/git/user/mjw/gccrs/commit/?h=tuple-union-field-locus

Cheers,

Mark

[-- Attachment #2: 0001-Add-locus-to-TupleField-and-pass-it-and-union-varian.patch --]
[-- Type: text/x-diff, Size: 5635 bytes --]

From cd67e5d5f138dbdff4ec859e4020e8091cb03aa7 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Mon, 2 Aug 2021 00:33:02 +0200
Subject: [PATCH] Add locus to TupleField and pass it and union variants to HIR
 class

TupleField was missing a Location field and we dropped to locus when
lowering Union fields to HIR.
---
 gcc/rust/ast/rust-item.h           | 16 ++++++++++------
 gcc/rust/hir/rust-ast-lower-item.h |  6 ++----
 gcc/rust/hir/rust-ast-lower-stmt.h | 11 +++--------
 gcc/rust/parse/rust-parse-impl.h   |  4 +++-
 4 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/gcc/rust/ast/rust-item.h b/gcc/rust/ast/rust-item.h
index 5605b0bb79c..6b0021a8b11 100644
--- a/gcc/rust/ast/rust-item.h
+++ b/gcc/rust/ast/rust-item.h
@@ -2002,9 +2002,10 @@ private:
 
   std::unique_ptr<Type> field_type;
 
-  // should this store location info?
   NodeId node_id;
 
+  Location locus;
+
 public:
   // Returns whether tuple field has outer attributes.
   bool has_outer_attributes () const { return !outer_attrs.empty (); }
@@ -2014,17 +2015,17 @@ public:
   bool has_visibility () const { return !visibility.is_error (); }
 
   // Complete constructor
-  TupleField (std::unique_ptr<Type> field_type, Visibility vis,
+  TupleField (std::unique_ptr<Type> field_type, Visibility vis, Location locus,
 	      std::vector<Attribute> outer_attrs = std::vector<Attribute> ())
     : outer_attrs (std::move (outer_attrs)), visibility (std::move (vis)),
       field_type (std::move (field_type)),
-      node_id (Analysis::Mappings::get ()->get_next_node_id ())
+      node_id (Analysis::Mappings::get ()->get_next_node_id ()), locus (locus)
   {}
 
   // Copy constructor with clone
   TupleField (TupleField const &other)
     : outer_attrs (other.outer_attrs), visibility (other.visibility),
-      node_id (other.node_id)
+      node_id (other.node_id), locus (other.locus)
   {
     // guard to prevent null dereference (only required if error)
     if (other.field_type != nullptr)
@@ -2039,6 +2040,7 @@ public:
     visibility = other.visibility;
     outer_attrs = other.outer_attrs;
     node_id = other.node_id;
+    locus = other.locus;
 
     // guard to prevent null dereference (only required if error)
     if (other.field_type != nullptr)
@@ -2059,12 +2061,14 @@ public:
   // Creates an error state tuple field.
   static TupleField create_error ()
   {
-    return TupleField (nullptr, Visibility::create_error ());
+    return TupleField (nullptr, Visibility::create_error (), Location ());
   }
 
   std::string as_string () const;
 
-  NodeId get_node_id () const { return node_id; };
+  NodeId get_node_id () const { return node_id; }
+
+  Location get_locus () const { return locus; }
 
   // TODO: this mutable getter seems really dodgy. Think up better way.
   std::vector<Attribute> &get_outer_attrs () { return outer_attrs; }
diff --git a/gcc/rust/hir/rust-ast-lower-item.h b/gcc/rust/hir/rust-ast-lower-item.h
index f168c7d3e88..d49d2b22bf0 100644
--- a/gcc/rust/hir/rust-ast-lower-item.h
+++ b/gcc/rust/hir/rust-ast-lower-item.h
@@ -111,12 +111,10 @@ public:
 				     mappings->get_next_localdef_id (
 				       crate_num));
 
-      // FIXME
-      // AST::TupleField is missing Location info
-      Location field_locus;
       HIR::TupleField translated_field (mapping,
 					std::unique_ptr<HIR::Type> (type), vis,
-					field_locus, field.get_outer_attrs ());
+					field.get_locus (),
+					field.get_outer_attrs ());
       fields.push_back (std::move (translated_field));
       return true;
     });
diff --git a/gcc/rust/hir/rust-ast-lower-stmt.h b/gcc/rust/hir/rust-ast-lower-stmt.h
index 2e97ca63a13..c4c00ac0bee 100644
--- a/gcc/rust/hir/rust-ast-lower-stmt.h
+++ b/gcc/rust/hir/rust-ast-lower-stmt.h
@@ -187,12 +187,10 @@ public:
 				     mappings->get_next_localdef_id (
 				       crate_num));
 
-      // FIXME
-      // AST::StructField is missing Location info
-      Location field_locus;
       HIR::StructField translated_field (mapping, field.get_field_name (),
 					 std::unique_ptr<HIR::Type> (type), vis,
-					 field_locus, field.get_outer_attrs ());
+					 field.get_locus (),
+					 field.get_outer_attrs ());
       fields.push_back (std::move (translated_field));
       return true;
     });
@@ -240,12 +238,9 @@ public:
 				     mappings->get_next_localdef_id (
 				       crate_num));
 
-      // FIXME
-      // AST::StructField is missing Location info
-      Location variant_locus;
       HIR::StructField translated_variant (mapping, variant.get_field_name (),
 					   std::unique_ptr<HIR::Type> (type),
-					   vis, variant_locus,
+					   vis, variant.get_locus (),
 					   variant.get_outer_attrs ());
       variants.push_back (std::move (translated_variant));
       return true;
diff --git a/gcc/rust/parse/rust-parse-impl.h b/gcc/rust/parse/rust-parse-impl.h
index 122a3c361b0..9eb212b4e72 100644
--- a/gcc/rust/parse/rust-parse-impl.h
+++ b/gcc/rust/parse/rust-parse-impl.h
@@ -4295,6 +4295,8 @@ Parser<ManagedTokenSource>::parse_tuple_field ()
   // parse visibility if it exists
   AST::Visibility vis = parse_visibility ();
 
+  Location locus = lexer.peek_token ()->get_locus ();
+
   // parse type, which is required
   std::unique_ptr<AST::Type> field_type = parse_type ();
   if (field_type == nullptr)
@@ -4308,7 +4310,7 @@ Parser<ManagedTokenSource>::parse_tuple_field ()
       return AST::TupleField::create_error ();
     }
 
-  return AST::TupleField (std::move (field_type), std::move (vis),
+  return AST::TupleField (std::move (field_type), std::move (vis), locus,
 			  std::move (outer_attrs));
 }
 
-- 
2.32.0