From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29904 invoked by alias); 17 Jul 2014 22:36:25 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 29885 invoked by uid 89); 17 Jul 2014 22:36:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.6 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 17 Jul 2014 22:36:23 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s6HMaEDc008313 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 17 Jul 2014 18:36:14 -0400 Received: from [10.36.116.97] (ovpn-116-97.ams2.redhat.com [10.36.116.97]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s6HMaCqR018296; Thu, 17 Jul 2014 18:36:13 -0400 Subject: Re: [PATCH] DWARFv5 DW_TAG_aligned_type. From: Mark Wielaard To: Tom Tromey Cc: gdb-patches@sourceware.org, Joel Brobecker In-Reply-To: <87sim77wce.fsf@fleche.redhat.com> References: <1404944457-4500-1-git-send-email-mjw@redhat.com> <87sim77wce.fsf@fleche.redhat.com> Content-Type: text/plain; charset="UTF-8" Date: Thu, 17 Jul 2014 23:11:00 -0000 Message-ID: <1405636572.17759.216.camel@bordewijk.wildebeest.org> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-SW-Source: 2014-07/txt/msg00487.txt.bz2 On Fri, 2014-07-11 at 09:23 -0600, Tom Tromey wrote: > Mark> + if (TYPE_USER_ALIGN (domain) != 0) > Mark> + fprintf_filtered (stream, " _Alignas (%u)", TYPE_USER_ALIGN (domain)); > > I think the preferred spelling in C++ is "alignas", not "_Alignas". > It would be a bit prettier if this were language-dependent. > What do you think? It affects a few spots. Yes, C++ uses a keyword for this with slightly different, but almost equal, semantics than the C _Alignas qualifier (C11 does define a header stdalign.h that has #define alignas _Alignas so that you can use alignas almost like in C++). How does the c-typeprint.c code determine the current language of the type? > Mark> +/* Add the given user alignment to the element type of the array. GCC > Mark> + outputs DWARF type qualifiers that apply to an array, not the > Mark> + element type. But GDB relies on the array element type to carry > Mark> + the cv-qualifiers. This is mimics section 6.7.3, point 9 of the > Mark> + C11 specification (n1570). */ > Mark> +static struct type * > Mark> +add_array_cv_aligned_type (struct die_info *die, struct dwarf2_cu *cu, > Mark> + struct type *base_type, unsigned int user_align) > Mark> +{ > > gdb rules put a blank line between the comment and the start of the > function. Fixed. > Mark> + TYPE_TARGET_TYPE (inner_array) = > Mark> + copy_type (TYPE_TARGET_TYPE (inner_array)); > > The "=" goes on the start of the next line. Fixed. > Mark> +static struct type * > Mark> +read_tag_aligned_type (struct die_info *die, struct dwarf2_cu *cu) > Mark> +{ > > Needs some kind of intro comment. Added: /* Handle DW_TAG_aligned_type and DW_AT_alignment. */ > Mark> +/* Make a '_Alignas'-qualified version of TYPE (if user_align is > Mark> + stricter than the user alignment of TYPE). */ > Mark> + > Mark> +struct type * > Mark> +make_aligned_type (struct type *type, unsigned int user_align) > Mark> +{ > Mark> + if (user_align > TYPE_USER_ALIGN (type)) > Mark> + return make_qualified_aligned_type (type, TYPE_INSTANCE_FLAGS (type), > Mark> + user_align, NULL); > > Like Joel I am curious about the need for this. > > I thought maybe it was just following the (C & C++) language standard. > But would DWARF like this really be emitted by the compiler? > It wasn't clear to me. Yeah, it wouldn't actually be emitted by my gcc patch. As I explained to Joel I just wanted to handle the "nested alignment" case somehow and assumed C11 semantics. We will define something for this and handle it properly. > Mark> + int new_user_align = TYPE_USER_ALIGN (type); > > Most spots are using unsigned for this. > > Occasionally I'm tempted to adopt the C++ style of ubiquitous typedefs > to avoid this kind of mismatch. But I've never really brought that up, > so I suggest just changing this instance. Changed to unsigned. Thanks, Mark