From patchwork Mon Aug 17 09:38:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 40277 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 879373858D37; Mon, 17 Aug 2020 09:38:45 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 879373858D37 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1597657125; bh=KDNGm19fXqfkvAMs2S+3uZHaSMpZgqZkcZz5UvgOa3k=; h=Date:In-Reply-To:References:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Help:List-Subscribe:From:Reply-To:Cc:From; b=Ug1ifw1Fg9s01Efc5P9bsL1J8g6WFUcy9AivLr7IyeMIfwJHPM4bBMyzYWRPMPfIT d44zPvMfid4MvjqjalN6pmU3rTHCJcBuzBPIgd+Etj7AghX4xUoG/ezb62zDZhju7K 25ECeRbg4+X9ipFOU9EmxNbVpPHl5O8Oy/ONrikc= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from mail-wr1-x449.google.com (mail-wr1-x449.google.com [IPv6:2a00:1450:4864:20::449]) by sourceware.org (Postfix) with ESMTPS id A8098385EC58 for ; Mon, 17 Aug 2020 09:38:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A8098385EC58 Received: by mail-wr1-x449.google.com with SMTP id b18so6799475wrn.6 for ; Mon, 17 Aug 2020 02:38:41 -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=KDNGm19fXqfkvAMs2S+3uZHaSMpZgqZkcZz5UvgOa3k=; b=t74mVYPkm2YzdMFVxT7BJaLaWiVTsv5yYGutkzEehmDe6lpVbnFoLIZEKo/bDDO498 d/9IDJIWiM9Hz3lAyqMckybTNgZa6ZIjirx9DfLhF6rQAKtRjlNqBAT0hfOV3dKew/Cf 7l/knypx+Cx+7EvoxC9uyR7C/SxQUjiLfxcn3OZlt2hvXyLq0mN69TCB0cT8EbXRJkIB WFyWiUqOfQXGtqC90Wk6FLEcsvK+esf5x2PPyFWIhDEdD0peYhTN5r9ayXq7OoNpuGt6 UNhBbK6+7Vwo4or3zU8JfBzWY7bECE45CpIvHvUFcB1A4zalYZDrxQFRn9LBZFIecogS 6cyg== X-Gm-Message-State: AOAM5337L4b+7mJHHykuN6Hi6jcmL571WZXYz9+ZioLfiv3KvV8ESVfE fT2qkg4MS33cB3BHdFyGHQX7GAjqDa+BLp5JFbZvqewpCfsB1Q3ggK1/gvAi2tcEl/WQ5tcSyD0 jdvQcS7Dc5j4OVqieVI02S3+M/gYGGr4mU+ccPNf6upgLsp+fxZHXtHC4V0mt7opgpfWdavY= X-Google-Smtp-Source: ABdhPJzVZ2sRyKk3DVPLNUcZsm4LArYbGHOoIGOluaHELq9TSyoIA7rBp6Wi6k7kI0UiYJqO0Dwg3OM/4myElA== X-Received: by 2002:a1c:dc86:: with SMTP id t128mr14248535wmg.6.1597657120727; Mon, 17 Aug 2020 02:38:40 -0700 (PDT) Date: Mon, 17 Aug 2020 10:38:18 +0100 In-Reply-To: <20200817093819.172380-1-gprocida@google.com> Message-Id: <20200817093819.172380-7-gprocida@google.com> Mime-Version: 1.0 References: <20200817093819.172380-1-gprocida@google.com> X-Mailer: git-send-email 2.28.0.220.ged08abb693-goog Subject: [PATCH 6/7] Default construct suppression types. To: libabigail@sourceware.org X-Spam-Status: No, score=-22.9 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: Matthias Maennich , kernel-team@android.com Errors-To: libabigail-bounces@sourceware.org Sender: "Libabigail" The current constructors for the various suppression types are not a good match for their current uses. Given that every member is effectively optional, default constructing these to a default unpopulated state makes the code simpler. There are no behavioural changes. * include/abg-suppression.h (suppression_base): Remove both non-default constructors, make default constructor public. (type_suppression): Ditto. (function_suppression): Remove non-default constructor. (variable_suppression): Remove non-default constructor, add default constructor. (file_suppression): Remove non-default constructor, make default constructor public. * src/abg-suppression-priv.h (variable_suppression::priv): Replace constructor with default constructor. (type_suppression::priv): Make default constructor public and define it. * src/abg-suppression.cc (suppression_base): Drop non-default constructors, define default constructor. (type_suppression): Drop non-default constructor, define default constructor. (function_suppression): Drop non-default constructor. (variable_suppression): Drop non-default constructor, define default constructor. (file_suppression): Drop non-default constructor, define default constructor. (read_type_suppression): Default construct result and populate initial fields using setters. (read_function_suppression): Ditto. (read_function_suppression): Ditto. (read_file_suppression): Ditto. * src/abg-tools-utils.cc (handle_file_entry): Default construct type_suppression suppr and use set_label to set initial field. Reviewed-by: Matthias Maennich Signed-off-by: Giuliano Procida --- include/abg-suppression.h | 44 +----- src/abg-suppression-priv.h | 28 ++-- src/abg-suppression.cc | 270 ++++++------------------------------- src/abg-tools-utils.cc | 6 +- 4 files changed, 59 insertions(+), 289 deletions(-) diff --git a/include/abg-suppression.h b/include/abg-suppression.h index dbb52de35..d1246a405 100644 --- a/include/abg-suppression.h +++ b/include/abg-suppression.h @@ -55,17 +55,10 @@ class suppression_base class priv; typedef shared_ptr priv_sptr; - // Forbid default constructor - suppression_base(); - public: priv_sptr priv_; - suppression_base(const string& label); - - suppression_base(const string& label, - const string& file_name_regex_str, - const string& file_name_not_regex_str); + suppression_base(); bool get_drops_artifact_from_ir() const; @@ -155,9 +148,6 @@ class type_suppression : public suppression_base class priv; typedef shared_ptr priv_sptr; - // Forbid this; - type_suppression(); - public: priv_sptr priv_; @@ -203,9 +193,7 @@ public: /// A convenience typedef for a vector of @ref insertion_range_sptr. typedef vector insertion_ranges; - type_suppression(const string& label, - const string& type_name_regexp, - const string& type_name); + type_suppression(); virtual ~type_suppression(); @@ -460,17 +448,6 @@ public: function_suppression(); - function_suppression(const string& label, - const string& name, - const string& name_regex, - const string& return_type_name, - const string& return_type_regex, - parameter_specs_type& parm_specs, - const string& symbol_name, - const string& symbol_name_regex, - const string& symbol_version, - const string& symbol_version_regex_str); - virtual ~function_suppression(); static change_kind @@ -677,15 +654,7 @@ public: priv_sptr priv_; - variable_suppression(const string& label = "", - const string& name = "", - const string& name_regex_str = "", - const string& symbol_name = "", - const string& symbol_name_regex_str = "", - const string& symbol_version = "", - const string& symbol_version_regex_str = "", - const string& type_name = "", - const string& type_name_regex_str = ""); + variable_suppression(); virtual ~variable_suppression(); @@ -810,14 +779,9 @@ class file_suppression: public suppression_base priv_sptr priv_; - // Forbid this - file_suppression(); - public: - file_suppression(const string& label, - const string& file_name_regex, - const string& file_name_not_regex); + file_suppression(); virtual bool suppresses_diff(const diff* diff) const; diff --git a/src/abg-suppression-priv.h b/src/abg-suppression-priv.h index deb08269c..658aeb143 100644 --- a/src/abg-suppression-priv.h +++ b/src/abg-suppression-priv.h @@ -497,23 +497,8 @@ struct variable_suppression::priv string type_name_regex_str_; mutable regex::regex_t_sptr type_name_regex_; - priv(const string& name, - const string& name_regex_str, - const string& symbol_name, - const string& symbol_name_regex_str, - const string& symbol_version, - const string& symbol_version_regex_str, - const string& type_name, - const string& type_name_regex_str) - : change_kind_(ALL_CHANGE_KIND), - name_(name), - name_regex_str_(name_regex_str), - symbol_name_(symbol_name), - symbol_name_regex_str_(symbol_name_regex_str), - symbol_version_(symbol_version), - symbol_version_regex_str_(symbol_version_regex_str), - type_name_(type_name), - type_name_regex_str_(type_name_regex_str) + priv() + : change_kind_(ALL_CHANGE_KIND) {} /// Getter for a pointer to a regular expression object built from @@ -666,9 +651,14 @@ class type_suppression::priv mutable regex::regex_t_sptr source_location_to_keep_regex_; mutable vector changed_enumerator_names_; - priv(); - public: + priv() + : consider_type_kind_(false), + type_kind_(CLASS_TYPE_KIND), + consider_reach_kind_(false), + reach_kind_(DIRECT_REACH_KIND) + {} + priv(const string& type_name_regexp, const string& type_name, bool consider_type_kind, diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc index fb97a124b..c9b2390e0 100644 --- a/src/abg-suppression.cc +++ b/src/abg-suppression.cc @@ -78,33 +78,11 @@ check_sufficient_props(const char *const * names, size_t count, // -/// Constructor for @ref suppression_base -/// -/// @param a label for the suppression. This represents just a -/// comment. -suppression_base::suppression_base(const string& label) - : priv_(new priv(label)) +/// Default constructor for @ref suppression_base +suppression_base::suppression_base() + : priv_(new priv()) {} -/// Constructor for @ref suppression_base -/// -/// @param a label for the suppression. This represents just a -/// comment. -/// -/// @param file_name_regex_str the regular expression that denotes the -/// file name to match. -/// -/// @param file_name_not_regex_str the regular expression that denotes -/// the file name to *NOT* match. -suppression_base::suppression_base(const string& label, - const string& file_name_regex_str, - const string& file_name_not_regex_str) - : priv_(new priv(label, - file_name_regex_str, - file_name_not_regex_str)) -{ -} - /// Tests if the current suppression specification is to avoid adding /// the matched ABI artifact to the internal representation or not. /// @@ -457,32 +435,9 @@ read_suppressions(const string& file_path, // -/// Constructor for @ref type_suppression. -/// -/// @param label the label of the suppression. This is just a free -/// form comment explaining what the suppression is about. -/// -/// @param type_name_regexp the regular expression describing the -/// types about which diff reports should be suppressed. If it's an -/// empty string, the parameter is ignored. -/// -/// @param type_name the name of the type about which diff reports -/// should be suppressed. If it's an empty string, the parameter is -/// ignored. -/// -/// Note that parameter @p type_name_regexp and @p type_name_regexp -/// should not necessarily be populated. It usually is either one or -/// the other that the user wants. -type_suppression::type_suppression(const string& label, - const string& type_name_regexp, - const string& type_name) - : suppression_base(label), - priv_(new priv(type_name_regexp, - type_name, - /*consider_type_kind=*/false, - /*type_kind=*/CLASS_TYPE_KIND, - /*consider_reach_kind=*/false, - /*reach_kind=*/DIRECT_REACH_KIND)) +/// Default constructor for @ref type_suppression. +type_suppression::type_suppression() + : suppression_base(), priv_(new priv) {} type_suppression::~type_suppression() @@ -1910,7 +1865,10 @@ read_type_suppression(const ini::config::section& section, changed_enumerator_names.push_back(p->get_value()->as_string()); } - type_suppression result(label_str, name_regex_str, name_str); + type_suppression result; + result.set_label(label_str); + result.set_type_name_regex_str(name_regex_str); + result.set_type_name(name_str); if (consider_type_kind) { @@ -2048,77 +2006,7 @@ function_suppression::parameter_spec::set_parameter_type_name_regex_str /// specified by using the various accessors of the @ref /// function_suppression type. function_suppression::function_suppression() - : suppression_base(/*label=*/""), priv_(new priv) -{} - -/// Constructor for the @ref function_suppression type. -/// -/// @param label an informative text string that the evalution code -/// might use to designate this function suppression specification in -/// error messages. This parameter might be empty, in which case it's -/// ignored at evaluation time. -/// -/// @param the name of the function the user wants the current -/// specification to designate. This parameter might be empty, in -/// which case it's ignored at evaluation time. -/// -/// @param nr if @p name is empty this parameter is a regular -/// expression for a family of names of functions the user wants the -/// current specification to designate. If @p name is not empty, this -/// parameter is ignored at specification evaluation time. This -/// parameter might be empty, in which case it's ignored at evaluation -/// time. -/// -/// @param ret_tn the name of the return type of the function the user -/// wants this specification to designate. This parameter might be -/// empty, in which case it's ignored at evaluation time. -/// -/// @param ret_tr if @p ret_tn is empty, then this is a regular -/// expression for a family of return type names for functions the -/// user wants the current specification to designate. If @p ret_tn -/// is not empty, then this parameter is ignored at specification -/// evaluation time. This parameter might be empty, in which case -/// it's ignored at evaluation time. -/// -/// @param ps a vector of parameter specifications to specify -/// properties of the parameters of the functions the user wants this -/// specification to designate. This parameter might be empty, in -/// which case it's ignored at evaluation time. -/// -/// @param sym_n the name of symbol of the function the user wants -/// this specification to designate. This parameter might be empty, -/// in which case it's ignored at evaluation time. -/// -/// @param sym_nr if the parameter @p sym_n is empty, then this -/// parameter is a regular expression for a family of names of symbols -/// of functions the user wants this specification to designate. If -/// the parameter @p sym_n is not empty, then this parameter is -/// ignored at specification evaluation time. This parameter might be -/// empty, in which case it's ignored at evaluation time. -/// -/// @param sym_v the name of the version of the symbol of the function -/// the user wants this specification to designate. This parameter -/// might be empty, in which case it's ignored at evaluation time. -/// -/// @param sym_vr if the parameter @p sym_v is empty, then this -/// parameter is a regular expression for a family of versions of -/// symbols of functions the user wants the current specification to -/// designate. If the parameter @p sym_v is non empty, then this -/// parameter is ignored. This parameter might be empty, in which -/// case it's ignored at evaluation time. -function_suppression::function_suppression(const string& label, - const string& name, - const string& nr, - const string& ret_tn, - const string& ret_tr, - parameter_specs_type& ps, - const string& sym_n, - const string& sym_nr, - const string& sym_v, - const string& sym_vr) - : suppression_base(label), - priv_(new priv(name, nr, ret_tn, ret_tr, ps, - sym_n, sym_nr, sym_v, sym_vr)) + : suppression_base(), priv_(new priv) {} function_suppression::~function_suppression() @@ -3342,16 +3230,17 @@ read_function_suppression(const ini::config::section& section, parms.push_back(parm); } - function_suppression result(label_str, - name, - name_regex_str, - return_type_name, - return_type_regex_str, - parms, - sym_name, - sym_name_regex_str, - sym_version, - sym_ver_regex_str); + function_suppression result; + result.set_label(label_str); + result.set_name(name); + result.set_name_regex_str(name_regex_str); + result.set_return_type_name(return_type_name); + result.set_return_type_regex_str(return_type_regex_str); + result.set_parameter_specs(parms); + result.set_symbol_name(sym_name); + result.set_symbol_name_regex_str(sym_name_regex_str); + result.set_symbol_version(sym_version); + result.set_symbol_version_regex_str(sym_ver_regex_str); if ((drop_artifact_str == "yes" || drop_artifact_str == "true") && (!name.empty() @@ -3396,71 +3285,13 @@ read_function_suppression(const ini::config::section& section, // -/// Constructor for the @ref variable_suppression type. -/// -/// @param label an informative text string that the evalution code -/// might use to designate this variable suppression specification in -/// error messages. This parameter might be empty, in which case it's -/// ignored at evaluation time. -/// -/// @param name the name of the variable the user wants the current -/// specification to designate. This parameter might be empty, in -/// which case it's ignored at evaluation time. -/// -/// @param name_regex_str if @p name is empty, this parameter is a -/// regular expression for a family of names of variables the user -/// wants the current specification to designate. If @p name is not -/// empty, then this parameter is ignored at evaluation time. This -/// parameter might be empty, in which case it's ignored at evaluation -/// time. -/// -/// @param symbol_name the name of the symbol of the variable the user -/// wants the current specification to designate. This parameter -/// might be empty, in which case it's ignored at evaluation time. -/// -/// @param symbol_name_str if @p symbol_name is empty, this parameter -/// is a regular expression for a family of names of symbols of -/// variables the user wants the current specification to designate. -/// If @p symbol_name is not empty, then this parameter is ignored at -/// evaluation time. This parameter might be empty, in which case -/// it's ignored at evaluation time. -/// -/// @param symbol_version the version of the symbol of the variable -/// the user wants the current specification to designate. This -/// parameter might be empty, in which case it's ignored at evaluation -/// time. -/// -/// @param symbol_version_regex if @p symbol_version is empty, then -/// this parameter is a regular expression for a family of versions of -/// symbol for the variables the user wants the current specification -/// to designate. If @p symbol_version is not empty, then this -/// parameter is ignored at evaluation time. This parameter might be -/// empty, in which case it's ignored at evaluation time. -/// -/// @param type_name the name of the type of the variable the user -/// wants the current specification to designate. This parameter -/// might be empty, in which case it's ignored at evaluation time. +/// Default constructor for the @ref variable_suppression type. /// -/// @param type_name_regex_str if @p type_name is empty, then this -/// parameter is a regular expression for a family of type names of -/// variables the user wants the current specification to designate. -/// If @p type_name is not empty, then this parameter is ignored at -/// evluation time. This parameter might be empty, in which case it's -/// ignored at evaluation time. -variable_suppression::variable_suppression(const string& label, - const string& name, - const string& name_regex_str, - const string& symbol_name, - const string& symbol_name_regex_str, - const string& symbol_version, - const string& symbol_version_regex, - const string& type_name, - const string& type_name_regex_str) - : suppression_base(label), - priv_(new priv(name, name_regex_str, - symbol_name, symbol_name_regex_str, - symbol_version, symbol_version_regex, - type_name, type_name_regex_str)) +/// It defines no suppression for now. Suppressions have to be +/// specified by using the various accessors of the @ref +/// variable_suppression type. +variable_suppression::variable_suppression() + : suppression_base(), priv_(new priv) {} /// Virtual destructor for the @erf variable_suppression type. @@ -4199,15 +4030,16 @@ read_variable_suppression(const ini::config::section& section, ? type_name_regex_prop->get_value()->as_string() : ""; - variable_suppression result(label_str, - name_str, - name_regex_str, - symbol_name, - symbol_name_regex_str, - symbol_version, - symbol_version_regex_str, - type_name_str, - type_name_regex_str); + variable_suppression result; + result.set_label(label_str); + result.set_name(name_str); + result.set_name_regex_str(name_regex_str); + result.set_symbol_name(symbol_name); + result.set_symbol_name_regex_str(symbol_name_regex_str); + result.set_symbol_version(symbol_version); + result.set_symbol_version_regex_str(symbol_version_regex_str); + result.set_type_name(type_name_str); + result.set_type_name_regex_str(type_name_regex_str); if ((drop_artifact_str == "yes" || drop_artifact_str == "true") && (!name_str.empty() @@ -4248,25 +4080,8 @@ read_variable_suppression(const ini::config::section& section, // -/// Constructor for the the @ref file_suppression type. -/// -/// @param label the label of the suppression directive. -/// -/// @param fname_regex_str the regular expression string that -/// designates the file name that instances of @ref file_suppression -/// should match. -/// -/// @param fname_not_regex_str the regular expression string that -/// designates the file name that instances of @ref file_suppression -/// shoult *NOT* match. In other words, this file_suppression should -/// be activated if its file name does not match the regular -/// expression @p fname_not_regex_str. -file_suppression::file_suppression(const string& label, - const string& fname_regex_str, - const string& fname_not_regex_str) - : suppression_base(label, - fname_regex_str, - fname_not_regex_str) +/// Default constructor for the the @ref file_suppression type. +file_suppression::file_suppression() {} /// Test if instances of this @ref file_suppression suppresses a @@ -4376,7 +4191,10 @@ read_file_suppression(const ini::config::section& section, ? soname_not_regex_prop->get_value()->as_string() : ""; - file_suppression result(label_str, file_name_regex_str, file_name_not_regex_str); + file_suppression result; + result.set_label(label_str); + result.set_file_name_regex_str(file_name_regex_str); + result.set_file_name_not_regex_str(file_name_not_regex_str); if (!soname_regex_str.empty()) { diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc index 38e2f1755..2e12206d6 100644 --- a/src/abg-tools-utils.cc +++ b/src/abg-tools-utils.cc @@ -1794,10 +1794,8 @@ handle_file_entry(const string& file_path, { if (!suppr) { - suppr.reset(new type_suppression(get_private_types_suppr_spec_label(), - /*type_name_regexp=*/"", - /*type_name=*/"")); - + suppr.reset(new type_suppression()); + suppr->set_label(get_private_types_suppr_spec_label()); // Types that are defined in system headers are usually // OK to be considered as public types. suppr->set_source_location_to_keep_regex_str("^/usr/include/");