From patchwork Wed Jul 8 09:53:13 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 39953 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A1B9F386180A; Wed, 8 Jul 2020 09:53:31 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A1B9F386180A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1594202011; bh=IGgkOaZQL489eI6GZSYwz0h/Ltq6BC+paHKQ80gpmTE=; h=Date:In-Reply-To:References:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Help:List-Subscribe:From:Reply-To:Cc:From; b=xyhmbRQCZWh/RgRgPVCi+ETzv8PZDCmAuM1gxKhlFFyg/DzQBump2X0gELG7xqlwP HK8gYzmbGuz4UvnLqxc9g8BtOvpOTD9USPyx1GCS5c+BMhaZF/BotqEVrfD59b3LVz W0XVNxll9PXjwGX6S5O/tscTQuFFfF/wyi8u4tq0= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from mail-wr1-x44a.google.com (mail-wr1-x44a.google.com [IPv6:2a00:1450:4864:20::44a]) by sourceware.org (Postfix) with ESMTPS id 65E0D3861815 for ; Wed, 8 Jul 2020 09:53:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 65E0D3861815 Received: by mail-wr1-x44a.google.com with SMTP id o12so51262954wrj.23 for ; Wed, 08 Jul 2020 02:53:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=IGgkOaZQL489eI6GZSYwz0h/Ltq6BC+paHKQ80gpmTE=; b=BKiF/rjmgi8q/Yx5aPcQJ3fJZOa6rGvumj0Wpd25agX/ymuOwSziGwRbax3m5jF5Dp AqUEPLzxzHvc2XSuMtaaUzo5a7m822C7BL0JEFKSY75MjjlD3qsXALtZpXnPXF1tcGvz /2InE6Clano5b89Loies2IHvOw+a5S5OSKalcUv/q/T97ItvFmXQaRYTQZktlI5Gr1B+ 5O0xPGJ2D8jLwmL3x18qmShld6OAWf0tK98BkmusIRLLLPLNoaiEQgdlyZPHFvBFMXV7 qIlTeuYn/pB8VNQZhf24mojvJHw7ubl4MfAUnQ2EJo5e68HR/vPAZi+f29beAeJ1pSrf Ku2Q== X-Gm-Message-State: AOAM531+ov2HPLh4QofVTQWDxoV+HFWDQVzNrZ6xiuCAydngNZvDLvbu egcNmtTwWMGz1S4rVTh8xUuQdBnmcINAuPAREY/wXFfG1JFf8w2E00KWN2OK5vURmsU2RNxggyA qNy3NDwUhT2fEtbV04OBPM18u/8HvnHynKC5pYn8KqLD89omOy74kCvP2k8UctWgjip0ceOg= X-Google-Smtp-Source: ABdhPJxhSnifZ/W8ktNBnPn1L9cqNI3bOCq7TyzJmOEn1tlXwG98r3WZ99IBcFHnyncfmg6IODbI0LBYojlO/w== X-Received: by 2002:a1c:a4c6:: with SMTP id n189mr8230415wme.173.1594202006358; Wed, 08 Jul 2020 02:53:26 -0700 (PDT) Date: Wed, 8 Jul 2020 10:53:13 +0100 In-Reply-To: <20200708095315.948634-1-gprocida@google.com> Message-Id: <20200708095315.948634-2-gprocida@google.com> Mime-Version: 1.0 References: <20200708095315.948634-1-gprocida@google.com> X-Mailer: git-send-email 2.27.0.383.g050319c2ae-goog Subject: [PATCH 1/3] abg-ir.cc: Tidy some operator== definitions To: libabigail@sourceware.org X-Spam-Status: No, score=-23.4 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-Patchwork-Original-From: Giuliano Procida via Libabigail From: Giuliano Procida Reply-To: Giuliano Procida Cc: maennich@google.com, kernel-team@android.com Errors-To: libabigail-bounces@sourceware.org Sender: "Libabigail" Many of the operator== definitions in this source file follow the same pattern: - the address of the argument is dynamic_cast to type of 'this' - naked canonical type pointers are compared, if both present - the types are compared structurally with 'equals' In a couple of cases extra work is done to fetch the canonical type of the definition of a declaration. This commit adjusts a few cases so they more closely follow the common form. This is to make the next refactoring trivial. There are no behavioural changes. * src/abg-irc.cc (scope_type_decl::operator==): Compare naked canonical type pointers instead of the shared pointers. (qualified_type_def::operator==): Remove excess blank line. (function_type::operator==): Do dynamic_cast and check of argument before comparing naked canonical type pointers. (class_or_union::operator==): Eliminate temporary reference. (class_decl::operator==): Likewise. (union_decl::operator==): Likewise. Signed-off-by: Giuliano Procida --- src/abg-ir.cc | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 37f6bbdf..41e2f00e 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -12872,8 +12872,8 @@ scope_type_decl::operator==(const decl_base& o) const if (!other) return false; - if (get_canonical_type() && other->get_canonical_type()) - return get_canonical_type().get() == other->get_canonical_type().get(); + if (get_naked_canonical_type() && other->get_naked_canonical_type()) + return get_naked_canonical_type() == other->get_naked_canonical_type(); return equals(*this, *other, 0); } @@ -13243,7 +13243,6 @@ qualified_type_def::operator==(const decl_base& o) const if (get_naked_canonical_type() && other->get_naked_canonical_type()) return get_naked_canonical_type() == other->get_naked_canonical_type(); - return equals(*this, *other, 0); } @@ -16723,16 +16722,16 @@ function_type::get_cached_name(bool internal) const bool function_type::operator==(const type_base& other) const { + const function_type* o = dynamic_cast(&other); + if (!o) + return false; + type_base* canonical_type = get_naked_canonical_type(); type_base* other_canonical_type = other.get_naked_canonical_type(); if (canonical_type && other_canonical_type) return canonical_type == other_canonical_type; - const function_type* o = dynamic_cast(&other); - if (!o) - return false; - return equals(*this, *o, 0); } @@ -19111,8 +19110,7 @@ class_or_union::operator==(const decl_base& other) const if (canonical_type && other_canonical_type) return canonical_type == other_canonical_type; - const class_or_union& o = *op; - return equals(*this, o, 0); + return equals(*this, *op, 0); } /// Equality operator. @@ -20966,8 +20964,7 @@ class_decl::operator==(const decl_base& other) const if (canonical_type && other_canonical_type) return canonical_type == other_canonical_type; - const class_decl& o = *op; - return equals(*this, o, 0); + return equals(*this, *op, 0); } /// Equality operator for class_decl. @@ -21755,8 +21752,7 @@ union_decl::operator==(const decl_base& other) const if (canonical_type && other_canonical_type) return canonical_type == other_canonical_type; - const union_decl &o = *op; - return equals(*this, o, 0); + return equals(*this, *op, 0); } /// Equality operator for union_decl. From patchwork Wed Jul 8 09:53:14 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 39954 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EA5AD3861841; Wed, 8 Jul 2020 09:53:31 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EA5AD3861841 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1594202012; bh=S+bdQLR7O1xny2qvOs5HyD3+faLEhFDEQp4+nZ/NAiI=; h=Date:In-Reply-To:References:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Help:List-Subscribe:From:Reply-To:Cc:From; b=QIE1zwaCpMyjknnKGUvLiZrM70a5gN7xVZtYuMBPnte+kzRHEYaA8APSn0Rp0CGhX SKBFwfymdXwkPoZ4K5aUrMhAWnDXbwMtrcyxjJZjYykHgjKnKK16MHu7aylaBe7qdV VGMBNs9vKrT8OW/fqycan18dxZEW2SmZEsGyvegk= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from mail-qv1-xf4a.google.com (mail-qv1-xf4a.google.com [IPv6:2607:f8b0:4864:20::f4a]) by sourceware.org (Postfix) with ESMTPS id 233A9386180A for ; Wed, 8 Jul 2020 09:53:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 233A9386180A Received: by mail-qv1-xf4a.google.com with SMTP id m8so11431037qvv.10 for ; Wed, 08 Jul 2020 02:53:29 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=S+bdQLR7O1xny2qvOs5HyD3+faLEhFDEQp4+nZ/NAiI=; b=Jk6smIhWowrWEALF+GvVT/M3AFJqBZHzjxYPCoAG4fbslIbffrh1EWdj2YtY/W3xjz hiy7vsOhz5rRSkBfJQYj1HA4E8ev2epFA4CdLQj/KptolfbVaD6DMu0TPzbeMaiNzMq5 e/OZ9tkYHQtG9t61LURuo88fEJ0rbdCx8LmGbDq5YIwfTl3OL8xy0ZV7e5kYFX2/FuWM SNbGixmLb8zaWeVO3liPBuG29aN8zwiXND0HCFHPIBHgqhhiMFuaciuPmqGZojVUbggg c6rW9GGgySDOSWEnpqq4ciL0FDjLerBh/Pt/s97cUycil/UxblUI0L+Rq5dMJ2aSLFLR Yyaw== X-Gm-Message-State: AOAM533NZRHih4XkPa36e14VP0xeVh5gnTMO3ImOVNCpaPT60cOHzY9d hQt2o6G+edV+rm48aL5jjyuXSrnGeTWmNv0JESGOYz15A2wayMJfUaKp6fMHgxWpASbOwlSrIe7 fJh1Mlz99SBePGMlGfgT1hXvfZzBVggTIwkXRH873Z+3T/KVO1uDyGO/GUuBWxmmdOhCe47E= X-Google-Smtp-Source: ABdhPJwW9hw4W90KTD+m8MoPWI7JZebSil3faO3mYF/VZp93pQgoH3IRFu1srJq/YImLIS3MyVFNH/xSsSWU3w== X-Received: by 2002:a05:6214:a85:: with SMTP id ev5mr54273635qvb.153.1594202008590; Wed, 08 Jul 2020 02:53:28 -0700 (PDT) Date: Wed, 8 Jul 2020 10:53:14 +0100 In-Reply-To: <20200708095315.948634-1-gprocida@google.com> Message-Id: <20200708095315.948634-3-gprocida@google.com> Mime-Version: 1.0 References: <20200708095315.948634-1-gprocida@google.com> X-Mailer: git-send-email 2.27.0.383.g050319c2ae-goog Subject: [PATCH 2/3] abg-ir.cc: Refactor operator== methods with helper To: libabigail@sourceware.org X-Spam-Status: No, score=-23.0 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-Patchwork-Original-From: Giuliano Procida via Libabigail From: Giuliano Procida Reply-To: Giuliano Procida Cc: maennich@google.com, kernel-team@android.com Errors-To: libabigail-bounces@sourceware.org Sender: "Libabigail" Many of the operator== definitions in this source file follow the same pattern: - the address of the argument is dynamic_cast to type of 'this' - naked canonical type pointers are compared, if both present - the types are compared structurally with 'equals' In a couple of cases extra work is done to fetch the canonical type of the definition of a declaration. This commit refactors all the common logic into a couple of templated helper functions. There are no behavioural changes. * src/abg-ir.cc (equality_helper): Add an overloaded function to perform the common actions needed for operator==. The first overload takes two extra canonical type pointer arguments while the second obtains these from the types being compared. (type_decl::operator==): Call equality_helper to perform canonical type pointer and 'equals' comparisons. (scope_type_decl::operator==): Likewise. (qualified_type_def::operator==): Likewise. (pointer_type_def::operator==): Likewise. (reference_type_def::operator==): Likewise. (array_type_def::subrange_type::operator==): Likewise. (array_type_def::operator==): Likewise. (enum_type_decl::operator==): Likewise. (typedef_decl::operator==): Likewise. (function_type::operator==): Likewise. (class_or_union::operator==): Likewise. (class_decl::operator==): Likewise. (union_decl::operator==): Likewise. Signed-off-by: Giuliano Procida --- src/abg-ir.cc | 104 ++++++++++++++------------------------------------ 1 file changed, 29 insertions(+), 75 deletions(-) diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 41e2f00e..4b7e180d 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -651,6 +651,22 @@ struct type_name_comp {return operator()(type_base_sptr(l), type_base_sptr(r));} }; // end struct type_name_comp +template +bool equality_helper(const T* lptr, const T* rptr, + const type_base* lcanon, + const type_base* rcanon) +{ + return lcanon && rcanon ? lcanon == rcanon : equals(*lptr, *rptr, 0); +} + +template +bool equality_helper(const T* lptr, const T* rptr) +{ + return equality_helper(lptr, rptr, + lptr->get_naked_canonical_type(), + rptr->get_naked_canonical_type()); +} + /// Getter of all types types sorted by their pretty representation. /// /// @return a sorted vector of all types sorted by their pretty @@ -12690,11 +12706,7 @@ type_decl::operator==(const decl_base& o) const const type_decl* other = dynamic_cast(&o); if (!other) return false; - - if (get_naked_canonical_type() && other->get_naked_canonical_type()) - return get_naked_canonical_type() == other->get_naked_canonical_type(); - - return equals(*this, *other, 0); + return equality_helper(this, other); } /// Return true if both types equals. @@ -12871,11 +12883,7 @@ scope_type_decl::operator==(const decl_base& o) const const scope_type_decl* other = dynamic_cast(&o); if (!other) return false; - - if (get_naked_canonical_type() && other->get_naked_canonical_type()) - return get_naked_canonical_type() == other->get_naked_canonical_type(); - - return equals(*this, *other, 0); + return equality_helper(this, other); } /// Equality operator between two scope_type_decl. @@ -13239,11 +13247,7 @@ qualified_type_def::operator==(const decl_base& o) const dynamic_cast(&o); if (!other) return false; - - if (get_naked_canonical_type() && other->get_naked_canonical_type()) - return get_naked_canonical_type() == other->get_naked_canonical_type(); - - return equals(*this, *other, 0); + return equality_helper(this, other); } /// Equality operator for qualified types. @@ -13610,14 +13614,7 @@ pointer_type_def::operator==(const decl_base& o) const const pointer_type_def* other = is_pointer_type(&o); if (!other) return false; - - type_base* canonical_type = get_naked_canonical_type(); - type_base* other_canonical_type = other->get_naked_canonical_type(); - - if (canonical_type && other_canonical_type) - return canonical_type == other_canonical_type; - - return equals(*this, *other, 0); + return equality_helper(this, other); } /// Return true iff both instances of pointer_type_def are equal. @@ -13921,14 +13918,7 @@ reference_type_def::operator==(const decl_base& o) const dynamic_cast(&o); if (!other) return false; - - type_base* canonical_type = get_naked_canonical_type(); - type_base* other_canonical_type = other->get_naked_canonical_type(); - - if (canonical_type && other_canonical_type) - return canonical_type == other_canonical_type; - - return equals(*this, *other, 0); + return equality_helper(this, other); } /// Equality operator of the @ref reference_type_def type. @@ -14470,11 +14460,7 @@ array_type_def::subrange_type::operator==(const decl_base& o) const dynamic_cast(&o); if (!other) return false; - - if (get_naked_canonical_type() && other->get_naked_canonical_type()) - return get_naked_canonical_type() == other->get_naked_canonical_type(); - - return equals(*this, *other, 0); + return equality_helper(this, other); } /// Equality operator. @@ -14792,11 +14778,7 @@ array_type_def::operator==(const decl_base& o) const dynamic_cast(&o); if (!other) return false; - - if (get_naked_canonical_type() && other->get_naked_canonical_type()) - return get_naked_canonical_type() == other->get_naked_canonical_type(); - - return equals(*this, *other, 0); + return equality_helper(this, other); } bool @@ -15272,11 +15254,7 @@ enum_type_decl::operator==(const decl_base& o) const const enum_type_decl* op = dynamic_cast(&o); if (!op) return false; - - if (get_naked_canonical_type() && op->get_naked_canonical_type()) - return get_naked_canonical_type() == op->get_naked_canonical_type(); - - return equals(*this, *op, 0); + return equality_helper(this, op); } /// Equality operator. @@ -15626,11 +15604,7 @@ typedef_decl::operator==(const decl_base& o) const const typedef_decl* other = dynamic_cast(&o); if (!other) return false; - - if (get_naked_canonical_type() && other->get_naked_canonical_type()) - return get_naked_canonical_type() == other->get_naked_canonical_type(); - - return equals(*this, *other, 0); + return equality_helper(this, other); } /// Equality operator @@ -16725,14 +16699,7 @@ function_type::operator==(const type_base& other) const const function_type* o = dynamic_cast(&other); if (!o) return false; - - type_base* canonical_type = get_naked_canonical_type(); - type_base* other_canonical_type = other.get_naked_canonical_type(); - - if (canonical_type && other_canonical_type) - return canonical_type == other_canonical_type; - - return equals(*this, *o, 0); + return equality_helper(this, o); } /// Return a copy of the pretty representation of the current @ref @@ -19107,10 +19074,7 @@ class_or_union::operator==(const decl_base& other) const other_canonical_type = op->get_naked_definition_of_declaration()->get_naked_canonical_type(); - if (canonical_type && other_canonical_type) - return canonical_type == other_canonical_type; - - return equals(*this, *op, 0); + return equality_helper(this, op, canonical_type, other_canonical_type); } /// Equality operator. @@ -20961,10 +20925,7 @@ class_decl::operator==(const decl_base& other) const other_canonical_type = op->get_naked_definition_of_declaration()->get_naked_canonical_type(); - if (canonical_type && other_canonical_type) - return canonical_type == other_canonical_type; - - return equals(*this, *op, 0); + return equality_helper(this, op, canonical_type, other_canonical_type); } /// Equality operator for class_decl. @@ -21745,14 +21706,7 @@ union_decl::operator==(const decl_base& other) const const union_decl* op = dynamic_cast(&other); if (!op) return false; - - type_base *canonical_type = get_naked_canonical_type(), - *other_canonical_type = op->get_naked_canonical_type(); - - if (canonical_type && other_canonical_type) - return canonical_type == other_canonical_type; - - return equals(*this, *op, 0); + return equality_helper(this, op); } /// Equality operator for union_decl. From patchwork Wed Jul 8 09:53:15 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 39955 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3F54A386181A; Wed, 8 Jul 2020 09:53:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 3F54A386181A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1594202014; bh=vU8A7TrGZnk5CMMiYxkmPIYdHSdcecAfXMMAIjWkPqU=; h=Date:In-Reply-To:References:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Help:List-Subscribe:From:Reply-To:Cc:From; b=BASMScqCoQ/BiGEAh4g+Oy8avmcKrNyW82wAwVCspz8ocXjIJf4nweabdGmTQUbDg 4V8g4EGJ2wzYp0wfj4zz/eeC3RcTJZIgr/8VNQCG2fF+hrpaMfvXDPBJU2je9we11b mTZqrO1rO8ETF0vCgyzRplxJrZFr2D4HYdQfV4jU= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from mail-wr1-x44a.google.com (mail-wr1-x44a.google.com [IPv6:2a00:1450:4864:20::44a]) by sourceware.org (Postfix) with ESMTPS id F09A43861834 for ; Wed, 8 Jul 2020 09:53:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org F09A43861834 Received: by mail-wr1-x44a.google.com with SMTP id v3so21703933wrq.10 for ; Wed, 08 Jul 2020 02:53:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=vU8A7TrGZnk5CMMiYxkmPIYdHSdcecAfXMMAIjWkPqU=; b=NacI15FuT76hXm80rsbxiTzib9/BHgF+5+pgA1kuwhFj5f9Gs6EevCPD922++LwoiX aAuDiGNXe6DhM0XUVdidbK/wmYA51rI1gIyEgHk8OAIP+s6LFRB8H0XULyw3vZcSrdCJ +OPl+bk6w4Gd20cJMxt8iPsv4uyIW8Mn/ZzdC3ZxnNL3VcFO1AkQmWreGzk1rfbnHfT3 VI4C9/U3+8kQh3cxHpW9m9Z3g79mpvTV4MmrBeSaOJYD7Sn0ZiMWkYMLjwIlutLeu+KO 4L7jIwczKS4cHjr3RNcesajyEa8Efo31WTmLRhafY2Lrq/dX0eCpouRoAwgHAQyLe1Kw eSeA== X-Gm-Message-State: AOAM5319HDo4gZKtd+QIuYC/MxGsqQk+g60Rv2/lOPd71VJ8XtAWKtiP DleeYbXxuntbF339S0rCMFWWpjNXJFdwZ8d5BHiwElAoxpQdvahkCjnpHSiJ/CnZm3H2beKAGhp NUyg4ClFMOT4ueE7YQrPW8bcRLf1hdQ4NJs31ISpBmoR0I8biYqGVl01iWbQMXtcUB/2+wHY= X-Google-Smtp-Source: ABdhPJwSJjryicZyS3Kcaze1YSislYKIy6pEl5SzKXM1/WiBr93HYIDrHlXlp/YKRiaGXqG/xcyAv5oclkxT2Q== X-Received: by 2002:adf:8521:: with SMTP id 30mr54870367wrh.238.1594202010893; Wed, 08 Jul 2020 02:53:30 -0700 (PDT) Date: Wed, 8 Jul 2020 10:53:15 +0100 In-Reply-To: <20200708095315.948634-1-gprocida@google.com> Message-Id: <20200708095315.948634-4-gprocida@google.com> Mime-Version: 1.0 References: <20200708095315.948634-1-gprocida@google.com> X-Mailer: git-send-email 2.27.0.383.g050319c2ae-goog Subject: [PATCH 3/3] Add some type equality paranoia. To: libabigail@sourceware.org X-Spam-Status: No, score=-23.3 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-Patchwork-Original-From: Giuliano Procida via Libabigail From: Giuliano Procida Reply-To: Giuliano Procida Cc: maennich@google.com, kernel-team@android.com Errors-To: libabigail-bounces@sourceware.org Sender: "Libabigail" This commit is an attempt to answer the question "how do canonical types relate to type equality?" and in particular "are they just an optimisation?". It adds some instrumentation to the equality_helper methods. - they take an extra line (__LINE__) argument - both canonical type pointer and plain comparisons are done - comparisons are skipped if the pointers are identical - in-progress comparisons are recorded to avoid infinite recursion - discrepancies are logged The commit also adds some comments to reflect behaviour seen in practice with libabigail's test suite. Note that this is not quite cross-checking canonical pointer comparison with deep equality as the (recursive) equality tests are themselves influenced by canonical pointer comparisons in the direction of considering more things to be equal. Note that thread-local storage is used to avoid corruption of the comparison stack. This is not portable to older compilers in its current form. While the behaviour should be the same, the code runs much more slowly and there can be considerable logging to stderr depending on what you choose to log. Signed-off-by: Giuliano Procida --- src/abg-ir.cc | 111 ++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 94 insertions(+), 17 deletions(-) diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 4b7e180d..e797a341 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -651,18 +651,81 @@ struct type_name_comp {return operator()(type_base_sptr(l), type_base_sptr(r));} }; // end struct type_name_comp +class hold +{ + public: + hold(const void* l, const void* r) + : p_(std::make_pair(l, r)) + { + st_.push_back(p_); + } + ~hold() + { + ABG_ASSERT(st_.size()); + ABG_ASSERT(st_.back() == p_); + st_.pop_back(); + } + static bool has(const void* l, const void* r) + { + return find(st_.begin(), st_.end(), std::make_pair(l, r)) != st_.end(); + } + private: + const std::pair p_; + thread_local static std::vector> st_; +}; + +thread_local std::vector> hold::st_; + +const bool debug_equality = true; + template -bool equality_helper(const T* lptr, const T* rptr, +bool equality_helper(size_t line, const T* lptr, const T* rptr, const type_base* lcanon, const type_base* rcanon) { - return lcanon && rcanon ? lcanon == rcanon : equals(*lptr, *rptr, 0); + if (debug_equality) + { + // Short cut equality with identical pointers to avoid wasteful deep + // comparisons. + if (lptr == rptr) + return true; + // Record outcome of canonical type pointer comparison. + char c = lcanon ? rcanon ? lcanon == rcanon ? '1' : '0' : 'L' : rcanon ? 'R' : 'X'; + // If we are already holding this pair of type_base pointers we should + // assume they are equal in recursive calls. + bool held = hold::has(lptr, rptr); + bool eq; + if (held) + eq = true; + else + { + hold it(static_cast(lptr), + static_cast(rptr)); + eq = equals(*lptr, *rptr, 0); + } + // Record outcome of plain comparison. + char p = held ? 'H' : eq ? '1' : '0'; + if (c + p == '0' + '1') + { + // Avoid tearing the output. + std::ostringstream os; + os << line << " canon=" << c << " plain=" << p + << " '" << lptr->get_cached_pretty_representation(true) + << "' '" << rptr->get_cached_pretty_representation(true) << "'\n"; + std::cerr << os.str(); + } + return lcanon && rcanon ? lcanon == rcanon : eq; + } + else + { + return lcanon && rcanon ? lcanon == rcanon : equals(*lptr, *rptr, 0); + } } template -bool equality_helper(const T* lptr, const T* rptr) +bool equality_helper(size_t line, const T* lptr, const T* rptr) { - return equality_helper(lptr, rptr, + return equality_helper(line, lptr, rptr, lptr->get_naked_canonical_type(), rptr->get_naked_canonical_type()); } @@ -12706,7 +12769,7 @@ type_decl::operator==(const decl_base& o) const const type_decl* other = dynamic_cast(&o); if (!other) return false; - return equality_helper(this, other); + return equality_helper(__LINE__, this, other); } /// Return true if both types equals. @@ -12883,7 +12946,7 @@ scope_type_decl::operator==(const decl_base& o) const const scope_type_decl* other = dynamic_cast(&o); if (!other) return false; - return equality_helper(this, other); + return equality_helper(__LINE__, this, other); } /// Equality operator between two scope_type_decl. @@ -13247,7 +13310,7 @@ qualified_type_def::operator==(const decl_base& o) const dynamic_cast(&o); if (!other) return false; - return equality_helper(this, other); + return equality_helper(__LINE__, this, other); } /// Equality operator for qualified types. @@ -13614,7 +13677,8 @@ pointer_type_def::operator==(const decl_base& o) const const pointer_type_def* other = is_pointer_type(&o); if (!other) return false; - return equality_helper(this, other); + // Sometimes canonically distinct but compare as equal. + return equality_helper(__LINE__, this, other); } /// Return true iff both instances of pointer_type_def are equal. @@ -13918,7 +13982,7 @@ reference_type_def::operator==(const decl_base& o) const dynamic_cast(&o); if (!other) return false; - return equality_helper(this, other); + return equality_helper(__LINE__, this, other); } /// Equality operator of the @ref reference_type_def type. @@ -14460,7 +14524,7 @@ array_type_def::subrange_type::operator==(const decl_base& o) const dynamic_cast(&o); if (!other) return false; - return equality_helper(this, other); + return equality_helper(__LINE__, this, other); } /// Equality operator. @@ -14778,7 +14842,8 @@ array_type_def::operator==(const decl_base& o) const dynamic_cast(&o); if (!other) return false; - return equality_helper(this, other); + // Sometimes canonically distinct but compare as equal. + return equality_helper(__LINE__, this, other); } bool @@ -15254,7 +15319,8 @@ enum_type_decl::operator==(const decl_base& o) const const enum_type_decl* op = dynamic_cast(&o); if (!op) return false; - return equality_helper(this, op); + // Sometimes canonically distinct but compare as equal. + return equality_helper(__LINE__, this, op); } /// Equality operator. @@ -15604,7 +15670,9 @@ typedef_decl::operator==(const decl_base& o) const const typedef_decl* other = dynamic_cast(&o); if (!other) return false; - return equality_helper(this, other); + // Sometimes canonically distinct but compare as equal. + // Sometimes canonically equal but compare as distinct. + return equality_helper(__LINE__, this, other); } /// Equality operator @@ -16699,7 +16767,9 @@ function_type::operator==(const type_base& other) const const function_type* o = dynamic_cast(&other); if (!o) return false; - return equality_helper(this, o); + // Sometimes canonically distinct but compare as equal. + // Sometimes canonically equal but compare as distinct. + return equality_helper(__LINE__, this, o); } /// Return a copy of the pretty representation of the current @ref @@ -19074,7 +19144,9 @@ class_or_union::operator==(const decl_base& other) const other_canonical_type = op->get_naked_definition_of_declaration()->get_naked_canonical_type(); - return equality_helper(this, op, canonical_type, other_canonical_type); + // Or should this just be definition_of_declaration equality? + // Sometimes canonically equal but compare as distinct. + return equality_helper(__LINE__, this, op, canonical_type, other_canonical_type); } /// Equality operator. @@ -20925,7 +20997,10 @@ class_decl::operator==(const decl_base& other) const other_canonical_type = op->get_naked_definition_of_declaration()->get_naked_canonical_type(); - return equality_helper(this, op, canonical_type, other_canonical_type); + // Or should this just be definition_of_declaration equality? + // Sometimes canonically distinct but compare as equal. + // Sometimes canonically equal but compare as distinct. + return equality_helper(__LINE__, this, op, canonical_type, other_canonical_type); } /// Equality operator for class_decl. @@ -21706,7 +21781,9 @@ union_decl::operator==(const decl_base& other) const const union_decl* op = dynamic_cast(&other); if (!op) return false; - return equality_helper(this, op); + // Sometimes canonically distinct but compare as equal. + // Sometimes canonically equal but compare as distinct. + return equality_helper(__LINE__, this, op); } /// Equality operator for union_decl.