From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (wildebeest.demon.nl [212.238.236.112]) by sourceware.org (Postfix) with ESMTPS id 0A4613854833 for ; Sun, 1 Aug 2021 22:37:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0A4613854833 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org Received: from reform (deer0x0b.wildebeest.org [172.31.17.141]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id 32E4130291A9; Mon, 2 Aug 2021 00:37:37 +0200 (CEST) Received: by reform (Postfix, from userid 1000) id A23A32E80D96; Mon, 2 Aug 2021 00:37:37 +0200 (CEST) Date: Mon, 2 Aug 2021 00:37:37 +0200 From: Mark Wielaard To: Philip Herron Cc: gcc-rust@gcc.gnu.org Subject: Re: [PATCH 2/2] WIP union hir-lowering and type support Message-ID: References: <20210722231902.7401-1-mark@klomp.org> <20210722231902.7401-3-mark@klomp.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="G2qXf8O0o5UsO8HA" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, JMQ_SPF_NEUTRAL, KAM_DMARC_STATUS, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-rust@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: gcc-rust mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 01 Aug 2021 22:37:42 -0000 --G2qXf8O0o5UsO8HA Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit 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 --G2qXf8O0o5UsO8HA Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-Add-locus-to-TupleField-and-pass-it-and-union-varian.patch" >From cd67e5d5f138dbdff4ec859e4020e8091cb03aa7 Mon Sep 17 00:00:00 2001 From: Mark Wielaard 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 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 field_type, Visibility vis, + TupleField (std::unique_ptr field_type, Visibility vis, Location locus, std::vector outer_attrs = std::vector ()) : 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 &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 (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 (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 (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::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 field_type = parse_type (); if (field_type == nullptr) @@ -4308,7 +4310,7 @@ Parser::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 --G2qXf8O0o5UsO8HA--