From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 92583 invoked by alias); 28 Sep 2015 14:44:11 -0000 Mailing-List: contact libabigail-help@sourceware.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Subscribe: Sender: libabigail-owner@sourceware.org Received: (qmail 92563 invoked by uid 89); 28 Sep 2015 14:44:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.98.7 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 X-Spam-Status: No, score=-1.0 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on sourceware.org X-Spam-Level: X-HELO: ms.seketeli.fr From: Dodji Seketeli To: Ondrej Oprala Cc: libabigail@sourceware.org Subject: Re: Support for function pointers and references Organization: Me, myself and I References: <5603CF1E.7010605@redhat.com> X-Operating-System: Fedora 20 X-URL: http://www.seketeli.net/~dodji Date: Thu, 01 Jan 2015 00:00:00 -0000 In-Reply-To: <5603CF1E.7010605@redhat.com> (Ondrej Oprala's message of "Thu, 24 Sep 2015 12:23:26 +0200") Message-ID: <86mvw64n9o.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2015-q3/txt/msg00053.txt.bz2 Hello Ondrej, So I have been looking at this patch: commit 5e9187ac43824a79c3e3d55ff71503aeb6718c41 Author: Ondrej Oprala Date: Wed Sep 9 10:12:03 2015 +0200 Generalize some dwarf-reader functions to generate and return instances of type_or_decl_base_stpr to be able to propagate types occurring without an accompanying declaration. * src/abg-dwarf-reader.cc (build_ir_node_from_die): Return a type_or_decl_base_sptr instead. (get_scope_for_die): Likewise. (build_class_type_and_add_to_ir): Typecast the assignment from build_ir_node_from_die properly. (build_{qualified,reference,array,typedef}_type): Likewise. (build_pointer_type_def): Likewise. (build_{var,function}_decl): Likewise. Signed-off-by: Ondrej Oprala I just have some nitpicking comments about it :-) [...] @@ -6570,11 +6570,11 @@ build_class_type_and_add_to_ir(read_context& ctxt, type_die_is_in_alternate_debug_info)) continue; - decl_base_sptr base_type = + decl_base_sptr base_type = dynamic_pointer_cast( Just as a matter of style, I'd prefer that you use the is_decl() function (declared in include/abg-fwd.h), which basically does the dynamic_pointer_cast here, but which is arguably more legible, easier to call from the debugger (should the need arise) and more importantly, is what is used throughout the code base for this purpose. The same comment applies to the other similar calls to the dynamic_pointer_cast operator to cast type_or_decl_base_sptr to decl_base_sptr. build_ir_node_from_die(ctxt, &type_die, type_die_is_in_alternate_debug_info, called_from_public_decl, - where_offset); + where_offset)); [...] @@ -6892,8 +6895,8 @@ build_pointer_type_def(read_context& ctxt, assert(result); return result; } - type_base_sptr utype = is_type(utype_decl); + assert(utype); I think this is a useless white space change. This patch looks good to me with those changes. I can commit it to master once you've amended it, or you can do it yourself, as you wish :-) Thank you again for working on this. Cheers, -- Dodji