Support for function pointers and references

Ondrej Oprala ooprala@redhat.com
Thu Jan 1 00:00:00 GMT 2015


Hi Dodji,
thank you for a lightning fast review!
I'll amend it and rebase against master.
I'll ping you on IRC when it's done.

Thanks again!
  Ondrej

On 28.09.2015 16:44, Dodji Seketeli wrote:
> Hello Ondrej,
>
> So I have been looking at this patch:
>
>      commit 5e9187ac43824a79c3e3d55ff71503aeb6718c41
>      Author: Ondrej Oprala <ooprala@redhat.com>
>      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 <ooprala@redhat.com>
>
>
> 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<decl_base>(
>
> 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,
>



More information about the Libabigail mailing list