[PATCH] cleanup: std::weak_ptr use: replace manual lock by std::weak_ptr::lock

[PATCH] cleanup: std::weak_ptr use: replace manual lock by std::weak_ptr::lock

[Matthias Maennich]

The pattern

	expired() ? shared_ptr<T>() : shared_ptr<T>(*this)

on std::weak_ptr<T> can be simplified by using std::weak_ptr<T>::lock.
Since weak_ptr::lock does this atomically, this patch also addresses
potential data races between the call to expired() and the construction
based on the assumption that the shared_ptr is still around.
Hence, apply this simplification/fix to the code base.

	* src/abg-comparison-priv.h (diff::priv::get_context): improve
	weak_ptr usage.
	(corpus_diff:diff_stats::priv::ctxt): Likewise.
	* src/abg-comparison.cc (corpus_diff::priv::get_context): Likewise.
	* src/abg-ir.cc (elf_symbol::get_next_alias): Likewise.
	(elf_symbol::get_next_common_instance): Likewise.
	(type_base::get_canonical_type): Likewise.
	(qualified_type_def::get_underlying_type): Likewise.
	(pointer_type_def::get_pointed_to_type): Likewise.
	(reference_type_def::get_pointed_to_type): Likewise.
	(array_type_def::subrange_type::get_underlying_type): Likewise.
	(array_type_def::get_element_type): Likewise.
	(typedef_decl::get_underlying_type): Likewise.
	(var_decl::get_type): Likewise.
	(function_type::get_return_type): Likewise.
	(function_decl::get_type): Likewise.
	(function_decl::parameter::get_type): Likewise.
	(class_or_union::get_naming_typedef): Likewise.
	(class_or_union::get_definition_of_declaration): Likewise.
	(class_decl::base_spec::get_base_class): Likewise.
	(template_parameter::get_enclosing_template_decl): Likewise.
	(non_type_tparameter::get_type): Likewise.
	(type_composition::get_composed_type): Likewise.

Signed-off-by: Matthias Maennich <maennich@google.com>
---
 src/abg-comparison-priv.h |   8 +--
 src/abg-comparison.cc     |   6 +-
 src/abg-ir.cc             | 114 +++++++-------------------------------
 3 files changed, 22 insertions(+), 106 deletions(-)

diff --git a/src/abg-comparison-priv.h b/src/abg-comparison-priv.h
index 420e7014421c..6f081b961f1a 100644
--- a/src/abg-comparison-priv.h
+++ b/src/abg-comparison-priv.h
@@ -305,11 +305,7 @@ public:
   /// @returnt a smart pointer to the diff context.
   diff_context_sptr
   get_context() const
-  {
-    if (ctxt_.expired())
-      return diff_context_sptr();
-    return diff_context_sptr(ctxt_);
-  }
+  { return ctxt_.lock(); }
 
   /// Check if a given categorization of a diff node should make it be
   /// filtered out.
@@ -1372,7 +1368,7 @@ struct corpus_diff::diff_stats::priv
 
   diff_context_sptr
   ctxt()
-  {return ctxt_.expired() ? diff_context_sptr() : diff_context_sptr(ctxt_);}
+  { return ctxt_.lock(); }
 }; // end class corpus_diff::diff_stats::priv
 
 void
diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index 5797c9dd7f3f..cb6989a2316c 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -8901,11 +8901,7 @@ corpus_diff::diff_stats::net_num_leaf_var_changes() const
 /// @return a smart pointer to the context associate with the corpus.
 diff_context_sptr
 corpus_diff::priv::get_context()
-{
-  if (ctxt_.expired())
-    return diff_context_sptr();
-  return diff_context_sptr(ctxt_);
-}
+{ return ctxt_.lock(); }
 
 /// Tests if the lookup tables are empty.
 ///
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 5cc39f59400a..69393028b93d 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -1725,11 +1725,7 @@ elf_symbol::is_main_symbol() const
 ///@return the alias, or NULL if there is no alias.
 elf_symbol_sptr
 elf_symbol::get_next_alias() const
-{
-  if (priv_->next_alias_.expired())
-    return elf_symbol_sptr();
-  return elf_symbol_sptr(priv_->next_alias_);
-}
+{ return priv_->next_alias_.lock(); }
 
 
 /// Check if the current elf_symbol has an alias.
@@ -1828,11 +1824,7 @@ elf_symbol::has_other_common_instances() const
 /// @return the next common instance, or nil if there is not any.
 elf_symbol_sptr
 elf_symbol::get_next_common_instance() const
-{
-  if (priv_->next_common_instance_.expired())
-    return elf_symbol_sptr();
-  return elf_symbol_sptr(priv_->next_common_instance_);
-}
+{ return priv_->next_common_instance_.lock(); }
 
 /// Add a common instance to the current common elf symbol.
 ///
@@ -12109,11 +12101,7 @@ type_base::type_base(const environment* e, size_t s, size_t a)
 /// type.
 type_base_sptr
 type_base::get_canonical_type() const
-{
-  if (priv_->canonical_type.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->canonical_type);
-}
+{ return priv_->canonical_type.lock(); }
 
 /// Getter of the canonical type pointer.
 ///
@@ -13400,11 +13388,7 @@ qualified_type_def::get_cv_quals_string_prefix() const
 /// Getter of the underlying type
 shared_ptr<type_base>
 qualified_type_def::get_underlying_type() const
-{
-  if (priv_->underlying_type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->underlying_type_);
-}
+{ return priv_->underlying_type_.lock(); }
 
 /// Non-member equality operator for @ref qualified_type_def
 ///
@@ -13639,11 +13623,7 @@ pointer_type_def::operator==(const pointer_type_def& other) const
 /// @return the pointed-to type.
 const type_base_sptr
 pointer_type_def::get_pointed_to_type() const
-{
-  if (priv_->pointed_to_type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->pointed_to_type_);
-}
+{ return priv_->pointed_to_type_.lock(); }
 
 /// Getter of a naked pointer to the pointed-to type.
 ///
@@ -13938,11 +13918,7 @@ reference_type_def::operator==(const reference_type_def& o) const
 
 type_base_sptr
 reference_type_def::get_pointed_to_type() const
-{
-  if (pointed_to_type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(pointed_to_type_);
-}
+{ return pointed_to_type_.lock(); }
 
 bool
 reference_type_def::is_lvalue() const
@@ -14258,11 +14234,7 @@ array_type_def::subrange_type::subrange_type(const environment* env,
 /// @return the underlying type.
 type_base_sptr
 array_type_def::subrange_type::get_underlying_type() const
-{
-  if (priv_->underlying_type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->underlying_type_);
-}
+{ return priv_->underlying_type_.lock(); }
 
 /// Setter of the underlying type of the subrange, that is, the type
 /// that defines the range.
@@ -14796,11 +14768,7 @@ array_type_def::operator==(const type_base& o) const
 /// @return the type of an array element.
 const type_base_sptr
 array_type_def::get_element_type() const
-{
-  if (priv_->element_type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->element_type_);
-}
+{ return priv_->element_type_.lock(); }
 
 /// Setter of the type of array element.
 ///
@@ -15666,11 +15634,7 @@ typedef_decl::get_pretty_representation(bool internal,
 /// @return the underlying_type.
 type_base_sptr
 typedef_decl::get_underlying_type() const
-{
-  if (priv_->underlying_type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->underlying_type_);
-}
+{ return priv_->underlying_type_.lock(); }
 
 /// This implements the ir_traversable_base::traverse pure virtual
 /// function.
@@ -15760,11 +15724,7 @@ var_decl::var_decl(const string&	name,
 /// @return the type of the variable.
 const type_base_sptr
 var_decl::get_type() const
-{
-  if (priv_->type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->type_);
-}
+{ return priv_->type_.lock(); }
 
 /// Getter of the type of the variable.
 ///
@@ -16357,11 +16317,7 @@ function_type::function_type(const environment* env,
 /// @return the return type.
 type_base_sptr
 function_type::get_return_type() const
-{
-  if (priv_->return_type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->return_type_);
-}
+{ return priv_->return_type_.lock(); }
 
 /// Setter of the return type of the current instance of @ref
 /// function_type.
@@ -17203,11 +17159,7 @@ function_decl::get_first_non_implicit_parm() const
 /// @return the type of the current instance of @ref function_decl.
 const shared_ptr<function_type>
 function_decl::get_type() const
-{
-  if (priv_->type_.expired())
-    return function_type_sptr();
-  return function_type_sptr(priv_->type_);
-}
+{ return priv_->type_.lock(); }
 
 /// Fast getter of the type of the current instance of @ref function_decl.
 ///
@@ -17727,11 +17679,7 @@ function_decl::parameter::parameter(const type_base_sptr	type,
 
 const type_base_sptr
 function_decl::parameter::get_type()const
-{
-  if (priv_->type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->type_);
-}
+{ return priv_->type_.lock(); }
 
 /// @return a copy of the type name of the parameter.
 interned_string
@@ -18567,11 +18515,7 @@ class_or_union::set_is_declaration_only(bool f)
 /// @return the naming typedef, if any.  Otherwise, returns nil.
 typedef_decl_sptr
 class_or_union::get_naming_typedef() const
-{
-  if (priv_->naming_typedef_.expired())
-    return typedef_decl_sptr();
-  return typedef_decl_sptr(priv_->naming_typedef_);
-}
+{ return priv_->naming_typedef_.lock(); }
 
 /// Set the naming typedef of the current instance of @ref class_decl.
 ///
@@ -18609,11 +18553,7 @@ class_or_union::set_definition_of_declaration(class_or_union_sptr d)
 /// @return the definition of this decl-only class.
 const class_or_union_sptr
 class_or_union::get_definition_of_declaration() const
-{
-  if (priv_->definition_of_declaration_.expired())
-    return class_or_union_sptr();
-  return class_or_union_sptr(priv_->definition_of_declaration_);
-}
+{ return priv_->definition_of_declaration_.lock(); }
 
 ///  If this @ref class_or_union is declaration-only, get its
 ///  definition, if any.
@@ -19916,11 +19856,7 @@ class_decl::base_spec::base_spec(const class_decl_sptr& base,
 /// @return the base class.
 class_decl_sptr
 class_decl::base_spec::get_base_class() const
-{
-  if (priv_->base_class_.expired())
-    return class_decl_sptr();
-  return class_decl_sptr(priv_->base_class_);
-}
+{ return priv_->base_class_.lock(); }
 
 /// Getter of the "is-virtual" proprerty of the base class specifier.
 ///
@@ -21896,11 +21832,7 @@ template_parameter::get_index() const
 
 const template_decl_sptr
 template_parameter::get_enclosing_template_decl() const
-{
-  if (priv_->template_decl_.expired())
-    return template_decl_sptr();
-  return template_decl_sptr(priv_->template_decl_);
-}
+{ return priv_->template_decl_.lock(); }
 
 bool
 template_parameter::get_hashing_has_started() const
@@ -22068,11 +22000,7 @@ non_type_tparameter::non_type_tparameter(unsigned		index,
 /// @return the type of the template parameter.
 const type_base_sptr
 non_type_tparameter::get_type() const
-{
-  if (priv_->type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->type_);
-}
+{ return priv_->type_.lock(); }
 
 /// Get the hash value of the current instance.
 ///
@@ -22241,11 +22169,7 @@ type_composition::type_composition(unsigned		index,
 /// @return the composed type.
 const type_base_sptr
 type_composition::get_composed_type() const
-{
-  if (priv_->type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->type_);
-}
+{ return priv_->type_.lock(); }
 
 /// Setter for the resulting composed type.
 ///
-- 
2.27.0.290.gba653c62da-goog

Re: [PATCH] cleanup: std::weak_ptr use: replace manual lock by std::weak_ptr::lock

[Giuliano Procida]

Hi.

Are there any remaining calls to expired() after this is applied? It
may be worth commenting on them or adjusting the commit message to
confirm their elimination.

On Mon, 15 Jun 2020 at 09:26, Matthias Maennich <maennich@google.com> wrote:
>
> The pattern
>
>         expired() ? shared_ptr<T>() : shared_ptr<T>(*this)
>
> on std::weak_ptr<T> can be simplified by using std::weak_ptr<T>::lock.
> Since weak_ptr::lock does this atomically, this patch also addresses
> potential data races between the call to expired() and the construction
> based on the assumption that the shared_ptr is still around.
> Hence, apply this simplification/fix to the code base.
>
>         * src/abg-comparison-priv.h (diff::priv::get_context): improve
>         weak_ptr usage.
>         (corpus_diff:diff_stats::priv::ctxt): Likewise.
>         * src/abg-comparison.cc (corpus_diff::priv::get_context): Likewise.
>         * src/abg-ir.cc (elf_symbol::get_next_alias): Likewise.
>         (elf_symbol::get_next_common_instance): Likewise.
>         (type_base::get_canonical_type): Likewise.
>         (qualified_type_def::get_underlying_type): Likewise.
>         (pointer_type_def::get_pointed_to_type): Likewise.
>         (reference_type_def::get_pointed_to_type): Likewise.
>         (array_type_def::subrange_type::get_underlying_type): Likewise.
>         (array_type_def::get_element_type): Likewise.
>         (typedef_decl::get_underlying_type): Likewise.
>         (var_decl::get_type): Likewise.
>         (function_type::get_return_type): Likewise.
>         (function_decl::get_type): Likewise.
>         (function_decl::parameter::get_type): Likewise.
>         (class_or_union::get_naming_typedef): Likewise.
>         (class_or_union::get_definition_of_declaration): Likewise.
>         (class_decl::base_spec::get_base_class): Likewise.
>         (template_parameter::get_enclosing_template_decl): Likewise.
>         (non_type_tparameter::get_type): Likewise.
>         (type_composition::get_composed_type): Likewise.
>
Reviewed-by: Giuliano Procida <gprocida@google.com>
> Signed-off-by: Matthias Maennich <maennich@google.com>
> ---
>  src/abg-comparison-priv.h |   8 +--
>  src/abg-comparison.cc     |   6 +-
>  src/abg-ir.cc             | 114 +++++++-------------------------------
>  3 files changed, 22 insertions(+), 106 deletions(-)
>
> diff --git a/src/abg-comparison-priv.h b/src/abg-comparison-priv.h
> index 420e7014421c..6f081b961f1a 100644
> --- a/src/abg-comparison-priv.h
> +++ b/src/abg-comparison-priv.h
> @@ -305,11 +305,7 @@ public:
>    /// @returnt a smart pointer to the diff context.
>    diff_context_sptr
>    get_context() const
> -  {
> -    if (ctxt_.expired())
> -      return diff_context_sptr();
> -    return diff_context_sptr(ctxt_);
> -  }
> +  { return ctxt_.lock(); }
>
>    /// Check if a given categorization of a diff node should make it be
>    /// filtered out.
> @@ -1372,7 +1368,7 @@ struct corpus_diff::diff_stats::priv
>
>    diff_context_sptr
>    ctxt()
> -  {return ctxt_.expired() ? diff_context_sptr() : diff_context_sptr(ctxt_);}
> +  { return ctxt_.lock(); }
>  }; // end class corpus_diff::diff_stats::priv
>
>  void
> diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
> index 5797c9dd7f3f..cb6989a2316c 100644
> --- a/src/abg-comparison.cc
> +++ b/src/abg-comparison.cc
> @@ -8901,11 +8901,7 @@ corpus_diff::diff_stats::net_num_leaf_var_changes() const
>  /// @return a smart pointer to the context associate with the corpus.
>  diff_context_sptr
>  corpus_diff::priv::get_context()
> -{
> -  if (ctxt_.expired())
> -    return diff_context_sptr();
> -  return diff_context_sptr(ctxt_);
> -}
> +{ return ctxt_.lock(); }
>
>  /// Tests if the lookup tables are empty.
>  ///
> diff --git a/src/abg-ir.cc b/src/abg-ir.cc
> index 5cc39f59400a..69393028b93d 100644
> --- a/src/abg-ir.cc
> +++ b/src/abg-ir.cc
> @@ -1725,11 +1725,7 @@ elf_symbol::is_main_symbol() const
>  ///@return the alias, or NULL if there is no alias.
>  elf_symbol_sptr
>  elf_symbol::get_next_alias() const
> -{
> -  if (priv_->next_alias_.expired())
> -    return elf_symbol_sptr();
> -  return elf_symbol_sptr(priv_->next_alias_);
> -}
> +{ return priv_->next_alias_.lock(); }
>
>
>  /// Check if the current elf_symbol has an alias.
> @@ -1828,11 +1824,7 @@ elf_symbol::has_other_common_instances() const
>  /// @return the next common instance, or nil if there is not any.
>  elf_symbol_sptr
>  elf_symbol::get_next_common_instance() const
> -{
> -  if (priv_->next_common_instance_.expired())
> -    return elf_symbol_sptr();
> -  return elf_symbol_sptr(priv_->next_common_instance_);
> -}
> +{ return priv_->next_common_instance_.lock(); }
>
>  /// Add a common instance to the current common elf symbol.
>  ///
> @@ -12109,11 +12101,7 @@ type_base::type_base(const environment* e, size_t s, size_t a)
>  /// type.
>  type_base_sptr
>  type_base::get_canonical_type() const
> -{
> -  if (priv_->canonical_type.expired())
> -    return type_base_sptr();
> -  return type_base_sptr(priv_->canonical_type);
> -}
> +{ return priv_->canonical_type.lock(); }
>
>  /// Getter of the canonical type pointer.
>  ///
> @@ -13400,11 +13388,7 @@ qualified_type_def::get_cv_quals_string_prefix() const
>  /// Getter of the underlying type
>  shared_ptr<type_base>
>  qualified_type_def::get_underlying_type() const
> -{
> -  if (priv_->underlying_type_.expired())
> -    return type_base_sptr();
> -  return type_base_sptr(priv_->underlying_type_);
> -}
> +{ return priv_->underlying_type_.lock(); }
>
>  /// Non-member equality operator for @ref qualified_type_def
>  ///
> @@ -13639,11 +13623,7 @@ pointer_type_def::operator==(const pointer_type_def& other) const
>  /// @return the pointed-to type.
>  const type_base_sptr
>  pointer_type_def::get_pointed_to_type() const
> -{
> -  if (priv_->pointed_to_type_.expired())
> -    return type_base_sptr();
> -  return type_base_sptr(priv_->pointed_to_type_);
> -}
> +{ return priv_->pointed_to_type_.lock(); }
>
>  /// Getter of a naked pointer to the pointed-to type.
>  ///
> @@ -13938,11 +13918,7 @@ reference_type_def::operator==(const reference_type_def& o) const
>
>  type_base_sptr
>  reference_type_def::get_pointed_to_type() const
> -{
> -  if (pointed_to_type_.expired())
> -    return type_base_sptr();
> -  return type_base_sptr(pointed_to_type_);
> -}
> +{ return pointed_to_type_.lock(); }
>
>  bool
>  reference_type_def::is_lvalue() const
> @@ -14258,11 +14234,7 @@ array_type_def::subrange_type::subrange_type(const environment* env,
>  /// @return the underlying type.
>  type_base_sptr
>  array_type_def::subrange_type::get_underlying_type() const
> -{
> -  if (priv_->underlying_type_.expired())
> -    return type_base_sptr();
> -  return type_base_sptr(priv_->underlying_type_);
> -}
> +{ return priv_->underlying_type_.lock(); }
>
>  /// Setter of the underlying type of the subrange, that is, the type
>  /// that defines the range.
> @@ -14796,11 +14768,7 @@ array_type_def::operator==(const type_base& o) const
>  /// @return the type of an array element.
>  const type_base_sptr
>  array_type_def::get_element_type() const
> -{
> -  if (priv_->element_type_.expired())
> -    return type_base_sptr();
> -  return type_base_sptr(priv_->element_type_);
> -}
> +{ return priv_->element_type_.lock(); }
>
>  /// Setter of the type of array element.
>  ///
> @@ -15666,11 +15634,7 @@ typedef_decl::get_pretty_representation(bool internal,
>  /// @return the underlying_type.
>  type_base_sptr
>  typedef_decl::get_underlying_type() const
> -{
> -  if (priv_->underlying_type_.expired())
> -    return type_base_sptr();
> -  return type_base_sptr(priv_->underlying_type_);
> -}
> +{ return priv_->underlying_type_.lock(); }
>
>  /// This implements the ir_traversable_base::traverse pure virtual
>  /// function.
> @@ -15760,11 +15724,7 @@ var_decl::var_decl(const string&       name,
>  /// @return the type of the variable.
>  const type_base_sptr
>  var_decl::get_type() const
> -{
> -  if (priv_->type_.expired())
> -    return type_base_sptr();
> -  return type_base_sptr(priv_->type_);
> -}
> +{ return priv_->type_.lock(); }
>
>  /// Getter of the type of the variable.
>  ///
> @@ -16357,11 +16317,7 @@ function_type::function_type(const environment* env,
>  /// @return the return type.
>  type_base_sptr
>  function_type::get_return_type() const
> -{
> -  if (priv_->return_type_.expired())
> -    return type_base_sptr();
> -  return type_base_sptr(priv_->return_type_);
> -}
> +{ return priv_->return_type_.lock(); }
>
>  /// Setter of the return type of the current instance of @ref
>  /// function_type.
> @@ -17203,11 +17159,7 @@ function_decl::get_first_non_implicit_parm() const
>  /// @return the type of the current instance of @ref function_decl.
>  const shared_ptr<function_type>
>  function_decl::get_type() const
> -{
> -  if (priv_->type_.expired())
> -    return function_type_sptr();
> -  return function_type_sptr(priv_->type_);
> -}
> +{ return priv_->type_.lock(); }
>
>  /// Fast getter of the type of the current instance of @ref function_decl.
>  ///
> @@ -17727,11 +17679,7 @@ function_decl::parameter::parameter(const type_base_sptr       type,
>
>  const type_base_sptr
>  function_decl::parameter::get_type()const
> -{
> -  if (priv_->type_.expired())
> -    return type_base_sptr();
> -  return type_base_sptr(priv_->type_);
> -}
> +{ return priv_->type_.lock(); }
>
>  /// @return a copy of the type name of the parameter.
>  interned_string
> @@ -18567,11 +18515,7 @@ class_or_union::set_is_declaration_only(bool f)
>  /// @return the naming typedef, if any.  Otherwise, returns nil.
>  typedef_decl_sptr
>  class_or_union::get_naming_typedef() const
> -{
> -  if (priv_->naming_typedef_.expired())
> -    return typedef_decl_sptr();
> -  return typedef_decl_sptr(priv_->naming_typedef_);
> -}
> +{ return priv_->naming_typedef_.lock(); }
>
>  /// Set the naming typedef of the current instance of @ref class_decl.
>  ///
> @@ -18609,11 +18553,7 @@ class_or_union::set_definition_of_declaration(class_or_union_sptr d)
>  /// @return the definition of this decl-only class.
>  const class_or_union_sptr
>  class_or_union::get_definition_of_declaration() const
> -{
> -  if (priv_->definition_of_declaration_.expired())
> -    return class_or_union_sptr();
> -  return class_or_union_sptr(priv_->definition_of_declaration_);
> -}
> +{ return priv_->definition_of_declaration_.lock(); }
>
>  ///  If this @ref class_or_union is declaration-only, get its
>  ///  definition, if any.
> @@ -19916,11 +19856,7 @@ class_decl::base_spec::base_spec(const class_decl_sptr& base,
>  /// @return the base class.
>  class_decl_sptr
>  class_decl::base_spec::get_base_class() const
> -{
> -  if (priv_->base_class_.expired())
> -    return class_decl_sptr();
> -  return class_decl_sptr(priv_->base_class_);
> -}
> +{ return priv_->base_class_.lock(); }
>
>  /// Getter of the "is-virtual" proprerty of the base class specifier.
>  ///
> @@ -21896,11 +21832,7 @@ template_parameter::get_index() const
>
>  const template_decl_sptr
>  template_parameter::get_enclosing_template_decl() const
> -{
> -  if (priv_->template_decl_.expired())
> -    return template_decl_sptr();
> -  return template_decl_sptr(priv_->template_decl_);
> -}
> +{ return priv_->template_decl_.lock(); }
>
>  bool
>  template_parameter::get_hashing_has_started() const
> @@ -22068,11 +22000,7 @@ non_type_tparameter::non_type_tparameter(unsigned              index,
>  /// @return the type of the template parameter.
>  const type_base_sptr
>  non_type_tparameter::get_type() const
> -{
> -  if (priv_->type_.expired())
> -    return type_base_sptr();
> -  return type_base_sptr(priv_->type_);
> -}
> +{ return priv_->type_.lock(); }
>
>  /// Get the hash value of the current instance.
>  ///
> @@ -22241,11 +22169,7 @@ type_composition::type_composition(unsigned            index,
>  /// @return the composed type.
>  const type_base_sptr
>  type_composition::get_composed_type() const
> -{
> -  if (priv_->type_.expired())
> -    return type_base_sptr();
> -  return type_base_sptr(priv_->type_);
> -}
> +{ return priv_->type_.lock(); }
>
>  /// Setter for the resulting composed type.
>  ///
> --
> 2.27.0.290.gba653c62da-goog
>

Re: [PATCH] cleanup: std::weak_ptr use: replace manual lock by std::weak_ptr::lock

[Matthias Maennich]

On Mon, Jun 15, 2020 at 11:49:30AM +0100, Giuliano Procida wrote:
>Hi.
>
>Are there any remaining calls to expired() after this is applied? It
>may be worth commenting on them or adjusting the commit message to
>confirm their elimination.

I transformed one more case for v2, but the remaining case is an
ABG_ASSERT in a setter. So, that is ok even without additional comment.

Cheers,
Matthias

>
>On Mon, 15 Jun 2020 at 09:26, Matthias Maennich <maennich@google.com> wrote:
>>
>> The pattern
>>
>>         expired() ? shared_ptr<T>() : shared_ptr<T>(*this)
>>
>> on std::weak_ptr<T> can be simplified by using std::weak_ptr<T>::lock.
>> Since weak_ptr::lock does this atomically, this patch also addresses
>> potential data races between the call to expired() and the construction
>> based on the assumption that the shared_ptr is still around.
>> Hence, apply this simplification/fix to the code base.
>>
>>         * src/abg-comparison-priv.h (diff::priv::get_context): improve
>>         weak_ptr usage.
>>         (corpus_diff:diff_stats::priv::ctxt): Likewise.
>>         * src/abg-comparison.cc (corpus_diff::priv::get_context): Likewise.
>>         * src/abg-ir.cc (elf_symbol::get_next_alias): Likewise.
>>         (elf_symbol::get_next_common_instance): Likewise.
>>         (type_base::get_canonical_type): Likewise.
>>         (qualified_type_def::get_underlying_type): Likewise.
>>         (pointer_type_def::get_pointed_to_type): Likewise.
>>         (reference_type_def::get_pointed_to_type): Likewise.
>>         (array_type_def::subrange_type::get_underlying_type): Likewise.
>>         (array_type_def::get_element_type): Likewise.
>>         (typedef_decl::get_underlying_type): Likewise.
>>         (var_decl::get_type): Likewise.
>>         (function_type::get_return_type): Likewise.
>>         (function_decl::get_type): Likewise.
>>         (function_decl::parameter::get_type): Likewise.
>>         (class_or_union::get_naming_typedef): Likewise.
>>         (class_or_union::get_definition_of_declaration): Likewise.
>>         (class_decl::base_spec::get_base_class): Likewise.
>>         (template_parameter::get_enclosing_template_decl): Likewise.
>>         (non_type_tparameter::get_type): Likewise.
>>         (type_composition::get_composed_type): Likewise.
>>
>Reviewed-by: Giuliano Procida <gprocida@google.com>
>> Signed-off-by: Matthias Maennich <maennich@google.com>
>> ---
>>  src/abg-comparison-priv.h |   8 +--
>>  src/abg-comparison.cc     |   6 +-
>>  src/abg-ir.cc             | 114 +++++++-------------------------------
>>  3 files changed, 22 insertions(+), 106 deletions(-)
>>
>> diff --git a/src/abg-comparison-priv.h b/src/abg-comparison-priv.h
>> index 420e7014421c..6f081b961f1a 100644
>> --- a/src/abg-comparison-priv.h
>> +++ b/src/abg-comparison-priv.h
>> @@ -305,11 +305,7 @@ public:
>>    /// @returnt a smart pointer to the diff context.
>>    diff_context_sptr
>>    get_context() const
>> -  {
>> -    if (ctxt_.expired())
>> -      return diff_context_sptr();
>> -    return diff_context_sptr(ctxt_);
>> -  }
>> +  { return ctxt_.lock(); }
>>
>>    /// Check if a given categorization of a diff node should make it be
>>    /// filtered out.
>> @@ -1372,7 +1368,7 @@ struct corpus_diff::diff_stats::priv
>>
>>    diff_context_sptr
>>    ctxt()
>> -  {return ctxt_.expired() ? diff_context_sptr() : diff_context_sptr(ctxt_);}
>> +  { return ctxt_.lock(); }
>>  }; // end class corpus_diff::diff_stats::priv
>>
>>  void
>> diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
>> index 5797c9dd7f3f..cb6989a2316c 100644
>> --- a/src/abg-comparison.cc
>> +++ b/src/abg-comparison.cc
>> @@ -8901,11 +8901,7 @@ corpus_diff::diff_stats::net_num_leaf_var_changes() const
>>  /// @return a smart pointer to the context associate with the corpus.
>>  diff_context_sptr
>>  corpus_diff::priv::get_context()
>> -{
>> -  if (ctxt_.expired())
>> -    return diff_context_sptr();
>> -  return diff_context_sptr(ctxt_);
>> -}
>> +{ return ctxt_.lock(); }
>>
>>  /// Tests if the lookup tables are empty.
>>  ///
>> diff --git a/src/abg-ir.cc b/src/abg-ir.cc
>> index 5cc39f59400a..69393028b93d 100644
>> --- a/src/abg-ir.cc
>> +++ b/src/abg-ir.cc
>> @@ -1725,11 +1725,7 @@ elf_symbol::is_main_symbol() const
>>  ///@return the alias, or NULL if there is no alias.
>>  elf_symbol_sptr
>>  elf_symbol::get_next_alias() const
>> -{
>> -  if (priv_->next_alias_.expired())
>> -    return elf_symbol_sptr();
>> -  return elf_symbol_sptr(priv_->next_alias_);
>> -}
>> +{ return priv_->next_alias_.lock(); }
>>
>>
>>  /// Check if the current elf_symbol has an alias.
>> @@ -1828,11 +1824,7 @@ elf_symbol::has_other_common_instances() const
>>  /// @return the next common instance, or nil if there is not any.
>>  elf_symbol_sptr
>>  elf_symbol::get_next_common_instance() const
>> -{
>> -  if (priv_->next_common_instance_.expired())
>> -    return elf_symbol_sptr();
>> -  return elf_symbol_sptr(priv_->next_common_instance_);
>> -}
>> +{ return priv_->next_common_instance_.lock(); }
>>
>>  /// Add a common instance to the current common elf symbol.
>>  ///
>> @@ -12109,11 +12101,7 @@ type_base::type_base(const environment* e, size_t s, size_t a)
>>  /// type.
>>  type_base_sptr
>>  type_base::get_canonical_type() const
>> -{
>> -  if (priv_->canonical_type.expired())
>> -    return type_base_sptr();
>> -  return type_base_sptr(priv_->canonical_type);
>> -}
>> +{ return priv_->canonical_type.lock(); }
>>
>>  /// Getter of the canonical type pointer.
>>  ///
>> @@ -13400,11 +13388,7 @@ qualified_type_def::get_cv_quals_string_prefix() const
>>  /// Getter of the underlying type
>>  shared_ptr<type_base>
>>  qualified_type_def::get_underlying_type() const
>> -{
>> -  if (priv_->underlying_type_.expired())
>> -    return type_base_sptr();
>> -  return type_base_sptr(priv_->underlying_type_);
>> -}
>> +{ return priv_->underlying_type_.lock(); }
>>
>>  /// Non-member equality operator for @ref qualified_type_def
>>  ///
>> @@ -13639,11 +13623,7 @@ pointer_type_def::operator==(const pointer_type_def& other) const
>>  /// @return the pointed-to type.
>>  const type_base_sptr
>>  pointer_type_def::get_pointed_to_type() const
>> -{
>> -  if (priv_->pointed_to_type_.expired())
>> -    return type_base_sptr();
>> -  return type_base_sptr(priv_->pointed_to_type_);
>> -}
>> +{ return priv_->pointed_to_type_.lock(); }
>>
>>  /// Getter of a naked pointer to the pointed-to type.
>>  ///
>> @@ -13938,11 +13918,7 @@ reference_type_def::operator==(const reference_type_def& o) const
>>
>>  type_base_sptr
>>  reference_type_def::get_pointed_to_type() const
>> -{
>> -  if (pointed_to_type_.expired())
>> -    return type_base_sptr();
>> -  return type_base_sptr(pointed_to_type_);
>> -}
>> +{ return pointed_to_type_.lock(); }
>>
>>  bool
>>  reference_type_def::is_lvalue() const
>> @@ -14258,11 +14234,7 @@ array_type_def::subrange_type::subrange_type(const environment* env,
>>  /// @return the underlying type.
>>  type_base_sptr
>>  array_type_def::subrange_type::get_underlying_type() const
>> -{
>> -  if (priv_->underlying_type_.expired())
>> -    return type_base_sptr();
>> -  return type_base_sptr(priv_->underlying_type_);
>> -}
>> +{ return priv_->underlying_type_.lock(); }
>>
>>  /// Setter of the underlying type of the subrange, that is, the type
>>  /// that defines the range.
>> @@ -14796,11 +14768,7 @@ array_type_def::operator==(const type_base& o) const
>>  /// @return the type of an array element.
>>  const type_base_sptr
>>  array_type_def::get_element_type() const
>> -{
>> -  if (priv_->element_type_.expired())
>> -    return type_base_sptr();
>> -  return type_base_sptr(priv_->element_type_);
>> -}
>> +{ return priv_->element_type_.lock(); }
>>
>>  /// Setter of the type of array element.
>>  ///
>> @@ -15666,11 +15634,7 @@ typedef_decl::get_pretty_representation(bool internal,
>>  /// @return the underlying_type.
>>  type_base_sptr
>>  typedef_decl::get_underlying_type() const
>> -{
>> -  if (priv_->underlying_type_.expired())
>> -    return type_base_sptr();
>> -  return type_base_sptr(priv_->underlying_type_);
>> -}
>> +{ return priv_->underlying_type_.lock(); }
>>
>>  /// This implements the ir_traversable_base::traverse pure virtual
>>  /// function.
>> @@ -15760,11 +15724,7 @@ var_decl::var_decl(const string&       name,
>>  /// @return the type of the variable.
>>  const type_base_sptr
>>  var_decl::get_type() const
>> -{
>> -  if (priv_->type_.expired())
>> -    return type_base_sptr();
>> -  return type_base_sptr(priv_->type_);
>> -}
>> +{ return priv_->type_.lock(); }
>>
>>  /// Getter of the type of the variable.
>>  ///
>> @@ -16357,11 +16317,7 @@ function_type::function_type(const environment* env,
>>  /// @return the return type.
>>  type_base_sptr
>>  function_type::get_return_type() const
>> -{
>> -  if (priv_->return_type_.expired())
>> -    return type_base_sptr();
>> -  return type_base_sptr(priv_->return_type_);
>> -}
>> +{ return priv_->return_type_.lock(); }
>>
>>  /// Setter of the return type of the current instance of @ref
>>  /// function_type.
>> @@ -17203,11 +17159,7 @@ function_decl::get_first_non_implicit_parm() const
>>  /// @return the type of the current instance of @ref function_decl.
>>  const shared_ptr<function_type>
>>  function_decl::get_type() const
>> -{
>> -  if (priv_->type_.expired())
>> -    return function_type_sptr();
>> -  return function_type_sptr(priv_->type_);
>> -}
>> +{ return priv_->type_.lock(); }
>>
>>  /// Fast getter of the type of the current instance of @ref function_decl.
>>  ///
>> @@ -17727,11 +17679,7 @@ function_decl::parameter::parameter(const type_base_sptr       type,
>>
>>  const type_base_sptr
>>  function_decl::parameter::get_type()const
>> -{
>> -  if (priv_->type_.expired())
>> -    return type_base_sptr();
>> -  return type_base_sptr(priv_->type_);
>> -}
>> +{ return priv_->type_.lock(); }
>>
>>  /// @return a copy of the type name of the parameter.
>>  interned_string
>> @@ -18567,11 +18515,7 @@ class_or_union::set_is_declaration_only(bool f)
>>  /// @return the naming typedef, if any.  Otherwise, returns nil.
>>  typedef_decl_sptr
>>  class_or_union::get_naming_typedef() const
>> -{
>> -  if (priv_->naming_typedef_.expired())
>> -    return typedef_decl_sptr();
>> -  return typedef_decl_sptr(priv_->naming_typedef_);
>> -}
>> +{ return priv_->naming_typedef_.lock(); }
>>
>>  /// Set the naming typedef of the current instance of @ref class_decl.
>>  ///
>> @@ -18609,11 +18553,7 @@ class_or_union::set_definition_of_declaration(class_or_union_sptr d)
>>  /// @return the definition of this decl-only class.
>>  const class_or_union_sptr
>>  class_or_union::get_definition_of_declaration() const
>> -{
>> -  if (priv_->definition_of_declaration_.expired())
>> -    return class_or_union_sptr();
>> -  return class_or_union_sptr(priv_->definition_of_declaration_);
>> -}
>> +{ return priv_->definition_of_declaration_.lock(); }
>>
>>  ///  If this @ref class_or_union is declaration-only, get its
>>  ///  definition, if any.
>> @@ -19916,11 +19856,7 @@ class_decl::base_spec::base_spec(const class_decl_sptr& base,
>>  /// @return the base class.
>>  class_decl_sptr
>>  class_decl::base_spec::get_base_class() const
>> -{
>> -  if (priv_->base_class_.expired())
>> -    return class_decl_sptr();
>> -  return class_decl_sptr(priv_->base_class_);
>> -}
>> +{ return priv_->base_class_.lock(); }
>>
>>  /// Getter of the "is-virtual" proprerty of the base class specifier.
>>  ///
>> @@ -21896,11 +21832,7 @@ template_parameter::get_index() const
>>
>>  const template_decl_sptr
>>  template_parameter::get_enclosing_template_decl() const
>> -{
>> -  if (priv_->template_decl_.expired())
>> -    return template_decl_sptr();
>> -  return template_decl_sptr(priv_->template_decl_);
>> -}
>> +{ return priv_->template_decl_.lock(); }
>>
>>  bool
>>  template_parameter::get_hashing_has_started() const
>> @@ -22068,11 +22000,7 @@ non_type_tparameter::non_type_tparameter(unsigned              index,
>>  /// @return the type of the template parameter.
>>  const type_base_sptr
>>  non_type_tparameter::get_type() const
>> -{
>> -  if (priv_->type_.expired())
>> -    return type_base_sptr();
>> -  return type_base_sptr(priv_->type_);
>> -}
>> +{ return priv_->type_.lock(); }
>>
>>  /// Get the hash value of the current instance.
>>  ///
>> @@ -22241,11 +22169,7 @@ type_composition::type_composition(unsigned            index,
>>  /// @return the composed type.
>>  const type_base_sptr
>>  type_composition::get_composed_type() const
>> -{
>> -  if (priv_->type_.expired())
>> -    return type_base_sptr();
>> -  return type_base_sptr(priv_->type_);
>> -}
>> +{ return priv_->type_.lock(); }
>>
>>  /// Setter for the resulting composed type.
>>  ///
>> --
>> 2.27.0.290.gba653c62da-goog
>>

[PATCH v2] cleanup: std::weak_ptr use: replace manual lock by std::weak_ptr::lock

[Matthias Maennich]

The pattern

	expired() ? shared_ptr<T>() : shared_ptr<T>(*this)

on std::weak_ptr<T> can be simplified by using std::weak_ptr<T>::lock.
Since weak_ptr::lock does this atomically, this patch also addresses
potential data races between the call to expired() and the construction
based on the assumption that the shared_ptr is still around.
Hence, apply this simplification/fix to the code base.

	* src/abg-comparison-priv.h (diff::priv::get_context): improve
	weak_ptr usage.
	(corpus_diff:diff_stats::priv::ctxt): Likewise.
	* src/abg-comparison.cc (corpus_diff::priv::get_context): Likewise.
	(var_diff::type_diff): Likewise.
	* src/abg-ir.cc (elf_symbol::get_next_alias): Likewise.
	(elf_symbol::get_next_common_instance): Likewise.
	(type_base::get_canonical_type): Likewise.
	(qualified_type_def::get_underlying_type): Likewise.
	(pointer_type_def::get_pointed_to_type): Likewise.
	(reference_type_def::get_pointed_to_type): Likewise.
	(array_type_def::subrange_type::get_underlying_type): Likewise.
	(array_type_def::get_element_type): Likewise.
	(typedef_decl::get_underlying_type): Likewise.
	(var_decl::get_type): Likewise.
	(function_type::get_return_type): Likewise.
	(function_decl::get_type): Likewise.
	(function_decl::parameter::get_type): Likewise.
	(class_or_union::get_naming_typedef): Likewise.
	(class_or_union::get_definition_of_declaration): Likewise.
	(class_decl::base_spec::get_base_class): Likewise.
	(template_parameter::get_enclosing_template_decl): Likewise.
	(non_type_tparameter::get_type): Likewise.
	(type_composition::get_composed_type): Likewise.

Signed-off-by: Matthias Maennich <maennich@google.com>
---
 src/abg-comparison-priv.h |   8 +--
 src/abg-comparison.cc     |  21 +++----
 src/abg-ir.cc             | 114 +++++++-------------------------------
 3 files changed, 30 insertions(+), 113 deletions(-)

diff --git a/src/abg-comparison-priv.h b/src/abg-comparison-priv.h
index 420e7014421c..6f081b961f1a 100644
--- a/src/abg-comparison-priv.h
+++ b/src/abg-comparison-priv.h
@@ -305,11 +305,7 @@ public:
   /// @returnt a smart pointer to the diff context.
   diff_context_sptr
   get_context() const
-  {
-    if (ctxt_.expired())
-      return diff_context_sptr();
-    return diff_context_sptr(ctxt_);
-  }
+  { return ctxt_.lock(); }
 
   /// Check if a given categorization of a diff node should make it be
   /// filtered out.
@@ -1372,7 +1368,7 @@ struct corpus_diff::diff_stats::priv
 
   diff_context_sptr
   ctxt()
-  {return ctxt_.expired() ? diff_context_sptr() : diff_context_sptr(ctxt_);}
+  { return ctxt_.lock(); }
 }; // end class corpus_diff::diff_stats::priv
 
 void
diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index 5797c9dd7f3f..81aed36a1f28 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -3355,15 +3355,16 @@ var_diff::second_var() const
 diff_sptr
 var_diff::type_diff() const
 {
-  if (priv_->type_diff_.expired())
+  if (diff_sptr result = priv_->type_diff_.lock())
+    return result;
+  else
     {
-      diff_sptr d = compute_diff(first_var()->get_type(),
-				       second_var()->get_type(),
-				       context());
-      context()->keep_diff_alive(d);
-      priv_->type_diff_ = d;
+      result = compute_diff(first_var()->get_type(), second_var()->get_type(),
+			    context());
+      context()->keep_diff_alive(result);
+      priv_->type_diff_ = result;
+      return result;
     }
-  return diff_sptr(priv_->type_diff_);
 }
 
 /// Return true iff the diff node has a change.
@@ -8901,11 +8902,7 @@ corpus_diff::diff_stats::net_num_leaf_var_changes() const
 /// @return a smart pointer to the context associate with the corpus.
 diff_context_sptr
 corpus_diff::priv::get_context()
-{
-  if (ctxt_.expired())
-    return diff_context_sptr();
-  return diff_context_sptr(ctxt_);
-}
+{ return ctxt_.lock(); }
 
 /// Tests if the lookup tables are empty.
 ///
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 5cc39f59400a..69393028b93d 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -1725,11 +1725,7 @@ elf_symbol::is_main_symbol() const
 ///@return the alias, or NULL if there is no alias.
 elf_symbol_sptr
 elf_symbol::get_next_alias() const
-{
-  if (priv_->next_alias_.expired())
-    return elf_symbol_sptr();
-  return elf_symbol_sptr(priv_->next_alias_);
-}
+{ return priv_->next_alias_.lock(); }
 
 
 /// Check if the current elf_symbol has an alias.
@@ -1828,11 +1824,7 @@ elf_symbol::has_other_common_instances() const
 /// @return the next common instance, or nil if there is not any.
 elf_symbol_sptr
 elf_symbol::get_next_common_instance() const
-{
-  if (priv_->next_common_instance_.expired())
-    return elf_symbol_sptr();
-  return elf_symbol_sptr(priv_->next_common_instance_);
-}
+{ return priv_->next_common_instance_.lock(); }
 
 /// Add a common instance to the current common elf symbol.
 ///
@@ -12109,11 +12101,7 @@ type_base::type_base(const environment* e, size_t s, size_t a)
 /// type.
 type_base_sptr
 type_base::get_canonical_type() const
-{
-  if (priv_->canonical_type.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->canonical_type);
-}
+{ return priv_->canonical_type.lock(); }
 
 /// Getter of the canonical type pointer.
 ///
@@ -13400,11 +13388,7 @@ qualified_type_def::get_cv_quals_string_prefix() const
 /// Getter of the underlying type
 shared_ptr<type_base>
 qualified_type_def::get_underlying_type() const
-{
-  if (priv_->underlying_type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->underlying_type_);
-}
+{ return priv_->underlying_type_.lock(); }
 
 /// Non-member equality operator for @ref qualified_type_def
 ///
@@ -13639,11 +13623,7 @@ pointer_type_def::operator==(const pointer_type_def& other) const
 /// @return the pointed-to type.
 const type_base_sptr
 pointer_type_def::get_pointed_to_type() const
-{
-  if (priv_->pointed_to_type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->pointed_to_type_);
-}
+{ return priv_->pointed_to_type_.lock(); }
 
 /// Getter of a naked pointer to the pointed-to type.
 ///
@@ -13938,11 +13918,7 @@ reference_type_def::operator==(const reference_type_def& o) const
 
 type_base_sptr
 reference_type_def::get_pointed_to_type() const
-{
-  if (pointed_to_type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(pointed_to_type_);
-}
+{ return pointed_to_type_.lock(); }
 
 bool
 reference_type_def::is_lvalue() const
@@ -14258,11 +14234,7 @@ array_type_def::subrange_type::subrange_type(const environment* env,
 /// @return the underlying type.
 type_base_sptr
 array_type_def::subrange_type::get_underlying_type() const
-{
-  if (priv_->underlying_type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->underlying_type_);
-}
+{ return priv_->underlying_type_.lock(); }
 
 /// Setter of the underlying type of the subrange, that is, the type
 /// that defines the range.
@@ -14796,11 +14768,7 @@ array_type_def::operator==(const type_base& o) const
 /// @return the type of an array element.
 const type_base_sptr
 array_type_def::get_element_type() const
-{
-  if (priv_->element_type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->element_type_);
-}
+{ return priv_->element_type_.lock(); }
 
 /// Setter of the type of array element.
 ///
@@ -15666,11 +15634,7 @@ typedef_decl::get_pretty_representation(bool internal,
 /// @return the underlying_type.
 type_base_sptr
 typedef_decl::get_underlying_type() const
-{
-  if (priv_->underlying_type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->underlying_type_);
-}
+{ return priv_->underlying_type_.lock(); }
 
 /// This implements the ir_traversable_base::traverse pure virtual
 /// function.
@@ -15760,11 +15724,7 @@ var_decl::var_decl(const string&	name,
 /// @return the type of the variable.
 const type_base_sptr
 var_decl::get_type() const
-{
-  if (priv_->type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->type_);
-}
+{ return priv_->type_.lock(); }
 
 /// Getter of the type of the variable.
 ///
@@ -16357,11 +16317,7 @@ function_type::function_type(const environment* env,
 /// @return the return type.
 type_base_sptr
 function_type::get_return_type() const
-{
-  if (priv_->return_type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->return_type_);
-}
+{ return priv_->return_type_.lock(); }
 
 /// Setter of the return type of the current instance of @ref
 /// function_type.
@@ -17203,11 +17159,7 @@ function_decl::get_first_non_implicit_parm() const
 /// @return the type of the current instance of @ref function_decl.
 const shared_ptr<function_type>
 function_decl::get_type() const
-{
-  if (priv_->type_.expired())
-    return function_type_sptr();
-  return function_type_sptr(priv_->type_);
-}
+{ return priv_->type_.lock(); }
 
 /// Fast getter of the type of the current instance of @ref function_decl.
 ///
@@ -17727,11 +17679,7 @@ function_decl::parameter::parameter(const type_base_sptr	type,
 
 const type_base_sptr
 function_decl::parameter::get_type()const
-{
-  if (priv_->type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->type_);
-}
+{ return priv_->type_.lock(); }
 
 /// @return a copy of the type name of the parameter.
 interned_string
@@ -18567,11 +18515,7 @@ class_or_union::set_is_declaration_only(bool f)
 /// @return the naming typedef, if any.  Otherwise, returns nil.
 typedef_decl_sptr
 class_or_union::get_naming_typedef() const
-{
-  if (priv_->naming_typedef_.expired())
-    return typedef_decl_sptr();
-  return typedef_decl_sptr(priv_->naming_typedef_);
-}
+{ return priv_->naming_typedef_.lock(); }
 
 /// Set the naming typedef of the current instance of @ref class_decl.
 ///
@@ -18609,11 +18553,7 @@ class_or_union::set_definition_of_declaration(class_or_union_sptr d)
 /// @return the definition of this decl-only class.
 const class_or_union_sptr
 class_or_union::get_definition_of_declaration() const
-{
-  if (priv_->definition_of_declaration_.expired())
-    return class_or_union_sptr();
-  return class_or_union_sptr(priv_->definition_of_declaration_);
-}
+{ return priv_->definition_of_declaration_.lock(); }
 
 ///  If this @ref class_or_union is declaration-only, get its
 ///  definition, if any.
@@ -19916,11 +19856,7 @@ class_decl::base_spec::base_spec(const class_decl_sptr& base,
 /// @return the base class.
 class_decl_sptr
 class_decl::base_spec::get_base_class() const
-{
-  if (priv_->base_class_.expired())
-    return class_decl_sptr();
-  return class_decl_sptr(priv_->base_class_);
-}
+{ return priv_->base_class_.lock(); }
 
 /// Getter of the "is-virtual" proprerty of the base class specifier.
 ///
@@ -21896,11 +21832,7 @@ template_parameter::get_index() const
 
 const template_decl_sptr
 template_parameter::get_enclosing_template_decl() const
-{
-  if (priv_->template_decl_.expired())
-    return template_decl_sptr();
-  return template_decl_sptr(priv_->template_decl_);
-}
+{ return priv_->template_decl_.lock(); }
 
 bool
 template_parameter::get_hashing_has_started() const
@@ -22068,11 +22000,7 @@ non_type_tparameter::non_type_tparameter(unsigned		index,
 /// @return the type of the template parameter.
 const type_base_sptr
 non_type_tparameter::get_type() const
-{
-  if (priv_->type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->type_);
-}
+{ return priv_->type_.lock(); }
 
 /// Get the hash value of the current instance.
 ///
@@ -22241,11 +22169,7 @@ type_composition::type_composition(unsigned		index,
 /// @return the composed type.
 const type_base_sptr
 type_composition::get_composed_type() const
-{
-  if (priv_->type_.expired())
-    return type_base_sptr();
-  return type_base_sptr(priv_->type_);
-}
+{ return priv_->type_.lock(); }
 
 /// Setter for the resulting composed type.
 ///
-- 
2.27.0.290.gba653c62da-goog

Re: [PATCH v2] cleanup: std::weak_ptr use: replace manual lock by std::weak_ptr::lock

[Dodji Seketeli]

Matthias Maennich <maennich@google.com> a écrit:

> The pattern
>
> 	expired() ? shared_ptr<T>() : shared_ptr<T>(*this)
>
> on std::weak_ptr<T> can be simplified by using std::weak_ptr<T>::lock.
> Since weak_ptr::lock does this atomically, this patch also addresses
> potential data races between the call to expired() and the construction
> based on the assumption that the shared_ptr is still around.
> Hence, apply this simplification/fix to the code base.
>
> 	* src/abg-comparison-priv.h (diff::priv::get_context): improve
> 	weak_ptr usage.
> 	(corpus_diff:diff_stats::priv::ctxt): Likewise.
> 	* src/abg-comparison.cc (corpus_diff::priv::get_context): Likewise.
> 	(var_diff::type_diff): Likewise.
> 	* src/abg-ir.cc (elf_symbol::get_next_alias): Likewise.
> 	(elf_symbol::get_next_common_instance): Likewise.
> 	(type_base::get_canonical_type): Likewise.
> 	(qualified_type_def::get_underlying_type): Likewise.
> 	(pointer_type_def::get_pointed_to_type): Likewise.
> 	(reference_type_def::get_pointed_to_type): Likewise.
> 	(array_type_def::subrange_type::get_underlying_type): Likewise.
> 	(array_type_def::get_element_type): Likewise.
> 	(typedef_decl::get_underlying_type): Likewise.
> 	(var_decl::get_type): Likewise.
> 	(function_type::get_return_type): Likewise.
> 	(function_decl::get_type): Likewise.
> 	(function_decl::parameter::get_type): Likewise.
> 	(class_or_union::get_naming_typedef): Likewise.
> 	(class_or_union::get_definition_of_declaration): Likewise.
> 	(class_decl::base_spec::get_base_class): Likewise.
> 	(template_parameter::get_enclosing_template_decl): Likewise.
> 	(non_type_tparameter::get_type): Likewise.
> 	(type_composition::get_composed_type): Likewise.
>
> Signed-off-by: Matthias Maennich <maennich@google.com>
Applied to master, thanks!

Cheers,

-- 
		Dodji