[2/3] Fix inheritance of scope_decl::insert_member_decl

Message ID 20200709182250.1677238-3-gprocida@google.com
State Committed, archived
Headers
Series Bug 21485 - problems compiling with clang |

Commit Message

Giuliano Procida July 9, 2020, 6:22 p.m. UTC
  The classes class_decl, class_or_union and scope_decl derive from each
other. The method insert_member_decl is declared virtual and defined
in each of these. Unfortunately, it has different argument types in
the base scope_decl class.

Most calls to insert_member_decl are at a statically known class, but
in insert_decl_into_scope the method is called via a scope_decl
pointer. There is the possibility that this could be a type derived
from scope_decl rather than scope_decl itself, in which case the base
method would be called, not as intended.

This commit adjusts the type of the member argument to
scope_decl::insert_member_decl to match the other two classes and
eliminates the last trigger of Clang's -Werror-overloaded-virtual.

	* include/abg-ir.h (scope_decl::insert_member_decl): Change
	type of member argument from const decl_base_sptr& to plain
	decl_base_sptr.
	* src/abg-ir.cc (scope_decl::insert_member_decl): Likewise.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 include/abg-ir.h | 3 +--
 src/abg-ir.cc    | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)
  

Comments

Matthias Männich July 10, 2020, 4:27 p.m. UTC | #1
On Thu, Jul 09, 2020 at 07:22:49PM +0100, Giuliano Procida wrote:
>The classes class_decl, class_or_union and scope_decl derive from each
>other. The method insert_member_decl is declared virtual and defined
>in each of these. Unfortunately, it has different argument types in
>the base scope_decl class.
>
>Most calls to insert_member_decl are at a statically known class, but
>in insert_decl_into_scope the method is called via a scope_decl
>pointer. There is the possibility that this could be a type derived
>from scope_decl rather than scope_decl itself, in which case the base
>method would be called, not as intended.
>
>This commit adjusts the type of the member argument to
>scope_decl::insert_member_decl to match the other two classes and
>eliminates the last trigger of Clang's -Werror-overloaded-virtual.
>
>	* include/abg-ir.h (scope_decl::insert_member_decl): Change
>	type of member argument from const decl_base_sptr& to plain
>	decl_base_sptr.
>	* src/abg-ir.cc (scope_decl::insert_member_decl): Likewise.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>

Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>---
> include/abg-ir.h | 3 +--
> src/abg-ir.cc    | 2 +-
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
>diff --git a/include/abg-ir.h b/include/abg-ir.h
>index c2b66c4c..ea6a7ce4 100644
>--- a/include/abg-ir.h
>+++ b/include/abg-ir.h
>@@ -1653,8 +1653,7 @@ protected:
>   add_member_decl(const decl_base_sptr& member);
>
>   virtual decl_base_sptr
>-  insert_member_decl(const decl_base_sptr& member,
>-		     declarations::iterator before);
>+  insert_member_decl(decl_base_sptr member, declarations::iterator before);
>
>   virtual void
>   remove_member_decl(decl_base_sptr member);
>diff --git a/src/abg-ir.cc b/src/abg-ir.cc
>index a434ec69..a257cf67 100644
>--- a/src/abg-ir.cc
>+++ b/src/abg-ir.cc
>@@ -6081,7 +6081,7 @@ scope_decl::add_member_decl(const decl_base_sptr& member)
> /// @param before an interator pointing to the element before which
> /// the new member should be inserted.
> decl_base_sptr
>-scope_decl::insert_member_decl(const decl_base_sptr& member,
>+scope_decl::insert_member_decl(decl_base_sptr member,
> 			       declarations::iterator before)
> {
>   ABG_ASSERT(!member->get_scope());
>-- 
>2.27.0.383.g050319c2ae-goog
>
  
Dodji Seketeli July 27, 2020, 4:14 p.m. UTC | #2
Giuliano Procida <gprocida@google.com> a écrit:

> The classes class_decl, class_or_union and scope_decl derive from each
> other. The method insert_member_decl is declared virtual and defined
> in each of these. Unfortunately, it has different argument types in
> the base scope_decl class.
>
> Most calls to insert_member_decl are at a statically known class, but
> in insert_decl_into_scope the method is called via a scope_decl
> pointer. There is the possibility that this could be a type derived
> from scope_decl rather than scope_decl itself, in which case the base
> method would be called, not as intended.
>
> This commit adjusts the type of the member argument to
> scope_decl::insert_member_decl to match the other two classes and
> eliminates the last trigger of Clang's -Werror-overloaded-virtual.
>
> 	* include/abg-ir.h (scope_decl::insert_member_decl): Change
> 	type of member argument from const decl_base_sptr& to plain
> 	decl_base_sptr.
> 	* src/abg-ir.cc (scope_decl::insert_member_decl): Likewise.
>
> Signed-off-by: Giuliano Procida <gprocida@google.com>

Applied to master, thanks!

Cheers,
  

Patch

diff --git a/include/abg-ir.h b/include/abg-ir.h
index c2b66c4c..ea6a7ce4 100644
--- a/include/abg-ir.h
+++ b/include/abg-ir.h
@@ -1653,8 +1653,7 @@  protected:
   add_member_decl(const decl_base_sptr& member);
 
   virtual decl_base_sptr
-  insert_member_decl(const decl_base_sptr& member,
-		     declarations::iterator before);
+  insert_member_decl(decl_base_sptr member, declarations::iterator before);
 
   virtual void
   remove_member_decl(decl_base_sptr member);
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index a434ec69..a257cf67 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -6081,7 +6081,7 @@  scope_decl::add_member_decl(const decl_base_sptr& member)
 /// @param before an interator pointing to the element before which
 /// the new member should be inserted.
 decl_base_sptr
-scope_decl::insert_member_decl(const decl_base_sptr& member,
+scope_decl::insert_member_decl(decl_base_sptr member,
 			       declarations::iterator before)
 {
   ABG_ASSERT(!member->get_scope());