public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Clean-ups
@ 2020-04-29 17:51 Giuliano Procida
  2020-04-29 17:51 ` [PATCH 1/6] Tabify code indentation Giuliano Procida
                   ` (7 more replies)
  0 siblings, 8 replies; 22+ messages in thread
From: Giuliano Procida @ 2020-04-29 17:51 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida

These are almost all independent. There's just 1 line of code ending:

? true : false;;

Regards,
Giuliano.

Giuliano Procida (6):
  Tabify code indentation.
  Remove excess whitespace.
  Remove stray semicolons.
  Eliminate redundant conditional operators.
  Make set_drops_artifact_from_ir non-const.
  Hoist some common expressions evaluating offsets.

 include/abg-diff-utils.h    |   2 +-
 include/abg-fwd.h           |   2 +-
 include/abg-suppression.h   |   2 +-
 src/abg-comparison.cc       |   6 +-
 src/abg-corpus.cc           |   4 +-
 src/abg-default-reporter.cc |   2 +-
 src/abg-dwarf-reader.cc     | 143 +++++++++++++++++-----------------
 src/abg-ir.cc               |   4 +-
 src/abg-reader.cc           |  20 ++---
 src/abg-suppression.cc      |  30 +++-----
 src/abg-tools-utils.cc      |  12 +--
 src/abg-writer.cc           |  42 +++++-----
 tools/abidiff.cc            |   2 +-
 tools/abipkgdiff.cc         | 148 ++++++++++++++++++------------------
 14 files changed, 205 insertions(+), 214 deletions(-)

-- 
2.26.2.303.gf8c07b1a785-goog


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/6] Tabify code indentation.
  2020-04-29 17:51 [PATCH 0/6] Clean-ups Giuliano Procida
@ 2020-04-29 17:51 ` Giuliano Procida
  2020-04-29 17:51 ` [PATCH 2/6] Remove excess whitespace Giuliano Procida
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: Giuliano Procida @ 2020-04-29 17:51 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida

Some sources file lines are not indented consistently with leading
8-space tabs. This patch updates the leading whitespace in those
cases.

	* src/abg-dwarf-reader.cc: At the start of lines, after any
	number of tabs, replace sequences of 8 spaces, or 1-7 spaces
	followed by a tab, with the same number of tabs.
	* src/abg-tools-utils.cc: Ditto.
	* src/abg-writer.cc: Ditto.
	* tools/abidiff.cc: Ditto.
	* tools/abipkgdiff.cc: Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-dwarf-reader.cc | 138 ++++++++++++++++++-------------------
 src/abg-tools-utils.cc  |  12 ++--
 src/abg-writer.cc       |  42 ++++++------
 tools/abidiff.cc        |   2 +-
 tools/abipkgdiff.cc     | 148 ++++++++++++++++++++--------------------
 5 files changed, 171 insertions(+), 171 deletions(-)

diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 850281ad..fe369fcf 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -1499,8 +1499,8 @@ lookup_public_variable_symbol_from_elf(const environment*		env,
 
 bool
 lookup_data_tag_from_dynamic_segment(Elf*                       elf,
-                                     Elf64_Sxword               data_tag,
-                                     vector<string>&            dt_tag_data)
+				     Elf64_Sxword               data_tag,
+				     vector<string>&            dt_tag_data)
 {
   size_t num_prog_headers = 0;
   bool found = false;
@@ -1513,7 +1513,7 @@ lookup_data_tag_from_dynamic_segment(Elf*                       elf,
       GElf_Phdr phdr_mem;
       GElf_Phdr *phdr = gelf_getphdr(elf, i, &phdr_mem);
       if (phdr == NULL || phdr->p_type != PT_DYNAMIC)
-        continue;
+	continue;
 
       // Poke at the dynamic segment like a section, so that we can
       // get its section header information; also we'd like to read
@@ -1527,48 +1527,48 @@ lookup_data_tag_from_dynamic_segment(Elf*                       elf,
       GElf_Shdr *dynamic_section_header = gelf_getshdr(dynamic_section,
 						       &shdr_mem);
       if (dynamic_section_header == NULL
-          || dynamic_section_header->sh_type != SHT_DYNAMIC)
-        continue;
+	  || dynamic_section_header->sh_type != SHT_DYNAMIC)
+	continue;
 
       // Get data of the dynamic segment (seen as a section).
       Elf_Data *data = elf_getdata(dynamic_section, NULL);
       if (data == NULL)
-        continue;
+	continue;
 
       // Get the index of the section headers string table.
       size_t string_table_index = 0;
       ABG_ASSERT (elf_getshdrstrndx(elf, &string_table_index) >= 0);
 
       size_t dynamic_section_header_entry_size = gelf_fsize(elf,
-                                                            ELF_T_DYN, 1,
-                                                            EV_CURRENT);
+							    ELF_T_DYN, 1,
+							    EV_CURRENT);
 
       GElf_Shdr link_mem;
       GElf_Shdr *link =
-        gelf_getshdr(elf_getscn(elf,
-                                dynamic_section_header->sh_link),
+	gelf_getshdr(elf_getscn(elf,
+				dynamic_section_header->sh_link),
 		     &link_mem);
       ABG_ASSERT(link != NULL);
 
       size_t num_dynamic_section_entries =
-        dynamic_section_header->sh_size / dynamic_section_header_entry_size;
+	dynamic_section_header->sh_size / dynamic_section_header_entry_size;
 
       // Now walk through all the DT_* data tags that are in the
       // segment/section
       for (size_t j = 0; j < num_dynamic_section_entries; ++j)
-        {
-          GElf_Dyn dynamic_section_mem;
-          GElf_Dyn *dynamic_section = gelf_getdyn(data,
-                                                  j,
-                                                  &dynamic_section_mem);
-          if (dynamic_section->d_tag == data_tag)
-            {
-              dt_tag_data.push_back(elf_strptr(elf,
-                                               dynamic_section_header->sh_link,
+	{
+	  GElf_Dyn dynamic_section_mem;
+	  GElf_Dyn *dynamic_section = gelf_getdyn(data,
+						  j,
+						  &dynamic_section_mem);
+	  if (dynamic_section->d_tag == data_tag)
+	    {
+	      dt_tag_data.push_back(elf_strptr(elf,
+					       dynamic_section_header->sh_link,
 					       dynamic_section->d_un.d_val));
-              found = true;
-            }
-        }
+	      found = true;
+	    }
+	}
     }
   return found;
 }
@@ -4885,7 +4885,7 @@ public:
 	 i != types_to_canonicalize(source).end();
 	 ++i)
       {
-        type_base_sptr t = lookup_type_from_die_offset(*i, source);
+	type_base_sptr t = lookup_type_from_die_offset(*i, source);
 	if (t->get_canonical_type())
 	  ++canonicalized;
 	else
@@ -4932,15 +4932,15 @@ public:
 	     << elf_path()
 	     << "\n";
 	cerr << "    # late canonicalized types: "
-             << num_canonicalized;
-        if (total)
-          cerr << " (" << num_canonicalized * 100 / total << "%)";
-        cerr << "\n"
+	     << num_canonicalized;
+	if (total)
+	  cerr << " (" << num_canonicalized * 100 / total << "%)";
+	cerr << "\n"
 	     << "    # missed canonicalization opportunities: "
-             << num_missed;
-        if (total)
-          cerr << " (" << num_missed * 100 / total << "%)";
-        cerr << "\n";
+	     << num_missed;
+	if (total)
+	  cerr << " (" << num_missed * 100 / total << "%)";
+	cerr << "\n";
       }
 
   }
@@ -6960,9 +6960,9 @@ public:
   /// @return true upon successful completion, false otherwise.
   bool
   populate_symbol_map_from_ksymtab(Elf_Scn *section,
-                                   address_set_sptr exported_fns_set,
-                                   address_set_sptr exported_vars_set,
-                                   size_t nb_entries)
+				   address_set_sptr exported_fns_set,
+				   address_set_sptr exported_vars_set,
+				   size_t nb_entries)
   {
     // The data of the section.
     Elf_Data *elf_data = elf_rawdata(section, 0);
@@ -7072,8 +7072,8 @@ public:
   /// @return true upon successful completion, false otherwise.
   bool
   populate_symbol_map_from_ksymtab_reloc(Elf_Scn *reloc_section,
-                                         address_set_sptr exported_fns_set,
-                                         address_set_sptr exported_vars_set)
+					 address_set_sptr exported_fns_set,
+					 address_set_sptr exported_vars_set)
   {
     GElf_Shdr reloc_section_mem;
     GElf_Shdr *reloc_section_shdr = gelf_getshdr(reloc_section,
@@ -7105,7 +7105,7 @@ public:
 
 	ABG_ASSERT(symbol);
 
-        // If the symbol is a linux string constant then ignore it.
+	// If the symbol is a linux string constant then ignore it.
 	if (symbol->get_is_linux_string_cst())
 	  continue;
 
@@ -7226,8 +7226,8 @@ public:
       }
     else
       return populate_symbol_map_from_ksymtab_reloc(reloc_section,
-                                                    linux_exported_fns_set,
-                                                    linux_exported_vars_set);
+						    linux_exported_fns_set,
+						    linux_exported_vars_set);
   }
 
   /// Load the special __ksymtab section. This is for linux kernel
@@ -17672,35 +17672,35 @@ get_soname_of_elf_file(const string& path, string &soname)
       GElf_Phdr* phdr = gelf_getphdr (elf, i, &phdr_mem);
 
       if (phdr != NULL && phdr->p_type == PT_DYNAMIC)
-        {
-          Elf_Scn* scn = gelf_offscn (elf, phdr->p_offset);
-          GElf_Shdr shdr_mem;
-          GElf_Shdr* shdr = gelf_getshdr (scn, &shdr_mem);
-          int maxcnt = (shdr != NULL
-                        ? shdr->sh_size / shdr->sh_entsize : INT_MAX);
-          ABG_ASSERT (shdr == NULL || shdr->sh_type == SHT_DYNAMIC);
-          Elf_Data* data = elf_getdata (scn, NULL);
-          if (data == NULL)
-            break;
-
-          for (int cnt = 0; cnt < maxcnt; ++cnt)
-            {
-              GElf_Dyn dynmem;
-              GElf_Dyn* dyn = gelf_getdyn (data, cnt, &dynmem);
-              if (dyn == NULL)
-                continue;
-
-              if (dyn->d_tag == DT_NULL)
-                break;
-
-              if (dyn->d_tag != DT_SONAME)
-                continue;
-
-              soname = elf_strptr (elf, shdr->sh_link, dyn->d_un.d_val);
-              break;
-            }
-          break;
-        }
+	{
+	  Elf_Scn* scn = gelf_offscn (elf, phdr->p_offset);
+	  GElf_Shdr shdr_mem;
+	  GElf_Shdr* shdr = gelf_getshdr (scn, &shdr_mem);
+	  int maxcnt = (shdr != NULL
+			? shdr->sh_size / shdr->sh_entsize : INT_MAX);
+	  ABG_ASSERT (shdr == NULL || shdr->sh_type == SHT_DYNAMIC);
+	  Elf_Data* data = elf_getdata (scn, NULL);
+	  if (data == NULL)
+	    break;
+
+	  for (int cnt = 0; cnt < maxcnt; ++cnt)
+	    {
+	      GElf_Dyn dynmem;
+	      GElf_Dyn* dyn = gelf_getdyn (data, cnt, &dynmem);
+	      if (dyn == NULL)
+		continue;
+
+	      if (dyn->d_tag == DT_NULL)
+		break;
+
+	      if (dyn->d_tag != DT_SONAME)
+		continue;
+
+	      soname = elf_strptr (elf, shdr->sh_link, dyn->d_un.d_val);
+	      break;
+	    }
+	  break;
+	}
     }
 
   elf_end(elf);
diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc
index a06e8615..9825dcef 100644
--- a/src/abg-tools-utils.cc
+++ b/src/abg-tools-utils.cc
@@ -1486,12 +1486,12 @@ guess_file_type(istream& in)
       && (unsigned char) buf[2] == 0xee
       && (unsigned char) buf[3] == 0xdb)
     {
-        if (buf[7] == 0x00)
-          return FILE_TYPE_RPM;
-        else if (buf[7] == 0x01)
-          return FILE_TYPE_SRPM;
-        else
-          return FILE_TYPE_UNKNOWN;
+	if (buf[7] == 0x00)
+	  return FILE_TYPE_RPM;
+	else if (buf[7] == 0x01)
+	  return FILE_TYPE_SRPM;
+	else
+	  return FILE_TYPE_UNKNOWN;
     }
 
   if (buf[257]    == 'u'
diff --git a/src/abg-writer.cc b/src/abg-writer.cc
index 8c6cc91a..afd99085 100644
--- a/src/abg-writer.cc
+++ b/src/abg-writer.cc
@@ -838,7 +838,7 @@ static bool write_pointer_type_def(const pointer_type_def_sptr&,
 static bool write_reference_type_def(const reference_type_def_sptr&,
 				     write_context&, unsigned);
 static bool write_array_type_def(const array_type_def_sptr&,
-			         write_context&, unsigned);
+				 write_context&, unsigned);
 static bool write_enum_type_decl(const enum_type_decl_sptr&,
 				 write_context&, unsigned);
 static bool write_typedef_decl(const typedef_decl_sptr&,
@@ -2766,8 +2766,8 @@ write_array_type_def(const array_type_def_sptr&	decl,
       vector<array_type_def::subrange_sptr>::const_iterator si;
 
       for (si = decl->get_subranges().begin();
-           si != decl->get_subranges().end(); ++si)
-        {
+	   si != decl->get_subranges().end(); ++si)
+	{
 	  unsigned local_indent =
 	    indent + ctxt.get_config().get_xml_element_indent();
 	  write_array_subrange_type(*si, ctxt, local_indent);
@@ -3184,18 +3184,18 @@ write_function_decl(const function_decl_sptr& decl, write_context& ctxt,
        ++pi)
     {
       if ((*pi)->get_variadic_marker())
-        {
-          do_indent(o, indent + ctxt.get_config().get_xml_element_indent());
-          o << "<parameter is-variadic='yes'";
-        }
+	{
+	  do_indent(o, indent + ctxt.get_config().get_xml_element_indent());
+	  o << "<parameter is-variadic='yes'";
+	}
       else
 	{
 	  parm_type = (*pi)->get_type();
 
-          annotate(*pi, ctxt,
+	  annotate(*pi, ctxt,
 		   indent + ctxt.get_config().get_xml_element_indent());
 
-          do_indent(o, indent + ctxt.get_config().get_xml_element_indent());
+	  do_indent(o, indent + ctxt.get_config().get_xml_element_indent());
 
 	  o << "<parameter type-id='"
 	    << ctxt.get_id_for_type(parm_type)
@@ -3278,17 +3278,17 @@ write_function_type(const function_type_sptr& fn_type,
     {
 
       if ((*pi)->get_variadic_marker())
-        {
-          do_indent(o, indent + ctxt.get_config().get_xml_element_indent());
-          o << "<parameter is-variadic='yes'";
-        }
+	{
+	  do_indent(o, indent + ctxt.get_config().get_xml_element_indent());
+	  o << "<parameter is-variadic='yes'";
+	}
       else
 	{
 	  parm_type = (*pi)->get_type();
 
-          annotate(*pi, ctxt, indent + ctxt.get_config().get_xml_element_indent());
+	  annotate(*pi, ctxt, indent + ctxt.get_config().get_xml_element_indent());
 
-          do_indent(o, indent + ctxt.get_config().get_xml_element_indent());
+	  do_indent(o, indent + ctxt.get_config().get_xml_element_indent());
 	  o << "<parameter type-id='"
 	    << ctxt.get_id_for_type(parm_type)
 	    << "'";
@@ -3502,7 +3502,7 @@ write_class_decl(const class_decl_sptr& decl,
 	   base != decl->get_base_specifiers().end();
 	   ++base)
 	{
-          annotate((*base)->get_base_class(), ctxt, indent);
+	  annotate((*base)->get_base_class(), ctxt, indent);
 	  do_indent(o, nb_ws);
 	  o << "<base-class";
 
@@ -3896,7 +3896,7 @@ write_member_type(const type_base_sptr& t, write_context& ctxt, unsigned indent)
 	 || write_reference_type_def(dynamic_pointer_cast<reference_type_def>(t),
 				     id, ctxt, nb_ws)
 	 || write_array_type_def(dynamic_pointer_cast<array_type_def>(t),
-			         id, ctxt, nb_ws)
+				 id, ctxt, nb_ws)
 	 || write_enum_type_decl(dynamic_pointer_cast<enum_type_decl>(t),
 				 id, ctxt, nb_ws)
 	 || write_typedef_decl(dynamic_pointer_cast<typedef_decl>(t),
@@ -4293,7 +4293,7 @@ create_archive_write_context(const string& archive_path)
 static bool
 write_translation_unit_to_archive(const translation_unit& tu,
 				  archive_write_ctxt& ctxt,
-                                  const bool annotate)
+				  const bool annotate)
 {
   if (!ctxt.archive)
     return false;
@@ -4335,7 +4335,7 @@ write_translation_unit_to_archive(const translation_unit& tu,
 static bool
 write_corpus_to_archive(const corpus& corp,
 			archive_write_ctxt& ctxt,
-                        const bool annotate)
+			const bool annotate)
 {
   for (translation_units::const_iterator i =
 	 corp.get_translation_units().begin();
@@ -4366,7 +4366,7 @@ write_corpus_to_archive(const corpus& corp,
 static bool
 write_corpus_to_archive(const corpus& corp,
 			archive_write_ctxt_sptr ctxt,
-                        const bool annotate)
+			const bool annotate)
 {return write_corpus_to_archive(corp, *ctxt, annotate);}
 
  /// Serialize the current corpus to disk in a file at a given path.
@@ -4382,7 +4382,7 @@ write_corpus_to_archive(const corpus& corp,
 bool
 write_corpus_to_archive(const corpus& corp,
 			const string& path,
-                        const bool annotate)
+			const bool annotate)
 {
   archive_write_ctxt_sptr ctxt = create_archive_write_context(path);
   ABG_ASSERT(ctxt);
diff --git a/tools/abidiff.cc b/tools/abidiff.cc
index 162d5ebc..4a9c4d1e 100644
--- a/tools/abidiff.cc
+++ b/tools/abidiff.cc
@@ -690,7 +690,7 @@ set_diff_context_from_opts(diff_context_sptr ctxt,
   // redundancy analysis pass altogether.  That could help save a
   // couple of CPU cycle here and there!
   ctxt->show_redundant_changes(opts.show_redundant_changes
-                               || opts.leaf_changes_only);
+			       || opts.leaf_changes_only);
   ctxt->show_symbols_unreferenced_by_debug_info
     (opts.show_symbols_not_referenced_by_debug_info);
   ctxt->show_added_symbols_unreferenced_by_debug_info
diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc
index 7d946e72..a5fc0a7c 100644
--- a/tools/abipkgdiff.cc
+++ b/tools/abipkgdiff.cc
@@ -360,7 +360,7 @@ public:
   /// @param pkg_kind the kind of package.
   package(const string&			path,
 	  const string&			dir,
-          kind					pkg_kind = package::KIND_MAIN)
+	  kind					pkg_kind = package::KIND_MAIN)
     : path_(path),
       kind_(pkg_kind)
   {
@@ -767,9 +767,9 @@ package::extracted_packages_parent_dir()
       const char *cachedir = getenv("XDG_CACHE_HOME");
 
       if (cachedir != NULL)
-        p = cachedir;
+	p = cachedir;
       else
-        {
+	{
 	  const char* s = getenv("HOME");
 	  if (s != NULL)
 	    p = s;
@@ -782,7 +782,7 @@ package::extracted_packages_parent_dir()
 		p = "/tmp";
 	    }
 	  p += "/.cache/libabigail";
-        }
+	}
 
       // Create the cache directory if it doesn't exist
       ABG_ASSERT(ensure_dir_path_created(p));
@@ -1057,11 +1057,11 @@ extract_package(const package& package,
     case abigail::tools_utils::FILE_TYPE_RPM:
 #ifdef WITH_RPM
       if (!extract_rpm(package.path(), package.extracted_dir_path(), opts))
-        {
-          emit_prefix("abipkgdiff", cerr)
+	{
+	  emit_prefix("abipkgdiff", cerr)
 	    << "Error while extracting package " << package.path() << "\n";
-          return false;
-        }
+	  return false;
+	}
       return true;
 #else
       emit_prefix("abipkgdiff", cerr)
@@ -1073,11 +1073,11 @@ extract_package(const package& package,
     case abigail::tools_utils::FILE_TYPE_DEB:
 #ifdef WITH_DEB
       if (!extract_deb(package.path(), package.extracted_dir_path(), opts))
-        {
-          emit_prefix("abipkgdiff", cerr)
+	{
+	  emit_prefix("abipkgdiff", cerr)
 	    << "Error while extracting package" << package.path() << "\n";
-          return false;
-        }
+	  return false;
+	}
       return true;
 #else
       emit_prefix("abipkgdiff", cerr)
@@ -1095,12 +1095,12 @@ extract_package(const package& package,
     case abigail::tools_utils::FILE_TYPE_TAR:
 #ifdef WITH_TAR
       if (!extract_tar(package.path(), package.extracted_dir_path(), opts))
-        {
-          emit_prefix("abipkgdiff", cerr)
+	{
+	  emit_prefix("abipkgdiff", cerr)
 	    << "Error while extracting GNU tar archive "
 	    << package.path() << "\n";
-          return false;
-        }
+	  return false;
+	}
       return true;
 #else
       emit_prefix("abipkgdiff", cerr)
@@ -1155,7 +1155,7 @@ set_diff_context_from_opts(diff_context_sptr ctxt,
   ctxt->error_output_stream(&cerr);
   // See comment in abidiff.cc's set_diff_context_from_opts.
   ctxt->show_redundant_changes(opts.show_redundant_changes
-                               || opts.leaf_changes_only);
+			       || opts.leaf_changes_only);
   ctxt->show_leaf_changes_only(opts.leaf_changes_only);
   ctxt->show_impacted_interfaces(opts.show_impacted_interfaces);
   ctxt->show_unreachable_types(opts.show_all_types);
@@ -2015,7 +2015,7 @@ create_maps_of_package_content(package& package, options& opts)
 	{
 	  if (e->type != abigail::dwarf_reader::ELF_TYPE_DSO
 	      && e->type != abigail::dwarf_reader::ELF_TYPE_EXEC
-              && e->type != abigail::dwarf_reader::ELF_TYPE_PI_EXEC)
+	      && e->type != abigail::dwarf_reader::ELF_TYPE_PI_EXEC)
 	    {
 	      if (is_linux_kernel_package)
 		{
@@ -2364,7 +2364,7 @@ compare_prepared_userspace_packages(package& first_package,
       if (iter != second_package.path_elf_file_sptr_map().end()
 	  && (iter->second->type == abigail::dwarf_reader::ELF_TYPE_DSO
 	      || iter->second->type == abigail::dwarf_reader::ELF_TYPE_EXEC
-              || iter->second->type == abigail::dwarf_reader::ELF_TYPE_PI_EXEC
+	      || iter->second->type == abigail::dwarf_reader::ELF_TYPE_PI_EXEC
 	      || iter->second->type == abigail::dwarf_reader::ELF_TYPE_RELOCATABLE))
 	{
 	  if (iter->second->type != abigail::dwarf_reader::ELF_TYPE_RELOCATABLE)
@@ -2697,85 +2697,85 @@ parse_command_line(int argc, char* argv[], options& opts)
   for (int i = 1; i < argc; ++i)
     {
       if (argv[i][0] != '-')
-        {
-          if (opts.package1.empty())
-            {
-              opts.package1 = make_path_absolute(argv[i]).get();
-              opts.nonexistent_file = !file_exists(opts.package1);
-            }
-          else if (opts.package2.empty())
-            {
-              opts.package2 = make_path_absolute(argv[i]).get();
-              opts.nonexistent_file = !file_exists(opts.package2);
-            }
-          else
+	{
+	  if (opts.package1.empty())
+	    {
+	      opts.package1 = make_path_absolute(argv[i]).get();
+	      opts.nonexistent_file = !file_exists(opts.package1);
+	    }
+	  else if (opts.package2.empty())
+	    {
+	      opts.package2 = make_path_absolute(argv[i]).get();
+	      opts.nonexistent_file = !file_exists(opts.package2);
+	    }
+	  else
 	    {
 	      opts.wrong_arg = argv[i];
 	      return false;
 	    }
 
-          if (opts.nonexistent_file)
-            {
-              opts.wrong_option = argv[i];
-              return true;
-            }
-        }
+	  if (opts.nonexistent_file)
+	    {
+	      opts.wrong_option = argv[i];
+	      return true;
+	    }
+	}
       else if (!strcmp(argv[i], "--debug-info-pkg1")
 	       || !strcmp(argv[i], "--d1"))
-        {
-          int j = i + 1;
-          if (j >= argc)
-            {
+	{
+	  int j = i + 1;
+	  if (j >= argc)
+	    {
 	      opts.missing_operand = true;
 	      opts.wrong_option = argv[i];
 	      return true;
-            }
-          opts.debug_packages1.push_back
+	    }
+	  opts.debug_packages1.push_back
 	    (abigail::tools_utils::make_path_absolute(argv[j]).get());
-          ++i;
-        }
+	  ++i;
+	}
       else if (!strcmp(argv[i], "--debug-info-pkg2")
 	       || !strcmp(argv[i], "--d2"))
-        {
-          int j = i + 1;
-          if (j >= argc)
-            {
+	{
+	  int j = i + 1;
+	  if (j >= argc)
+	    {
 	      opts.missing_operand = true;
 	      opts.wrong_option = argv[i];
 	      return true;
-            }
-          opts.debug_packages2.push_back
+	    }
+	  opts.debug_packages2.push_back
 	    (abigail::tools_utils::make_path_absolute(argv[j]).get());
-          ++i;
-        }
+	  ++i;
+	}
       else if (!strcmp(argv[i], "--devel-pkg1")
 	       || !strcmp(argv[i], "--devel1"))
-        {
-          int j = i + 1;
-          if (j >= argc)
-            {
+	{
+	  int j = i + 1;
+	  if (j >= argc)
+	    {
 	      opts.missing_operand = true;
 	      opts.wrong_option = argv[i];
 	      return true;
-            }
-          opts.devel_package1 =
+	    }
+	  opts.devel_package1 =
 	    abigail::tools_utils::make_path_absolute(argv[j]).get();
-          ++i;
-        }
+	  ++i;
+	}
       else if (!strcmp(argv[i], "--devel-pkg2")
 	       || !strcmp(argv[i], "--devel2"))
-        {
-          int j = i + 1;
-          if (j >= argc)
-            {
+	{
+	  int j = i + 1;
+	  if (j >= argc)
+	    {
 	      opts.missing_operand = true;
 	      opts.wrong_option = argv[i];
 	      return true;
-            }
-          opts.devel_package2 =
+	    }
+	  opts.devel_package2 =
 	    abigail::tools_utils::make_path_absolute(argv[j]).get();
-          ++i;
-        }
+	  ++i;
+	}
       else if (!strcmp(argv[i], "--drop-private-types"))
 	opts.drop_private_types = true;
       else if (!strcmp(argv[i], "--no-default-suppression"))
@@ -2877,10 +2877,10 @@ parse_command_line(int argc, char* argv[], options& opts)
 	}
       else if (!strcmp(argv[i], "--help")
 	       || !strcmp(argv[i], "-h"))
-        {
-          opts.display_usage = true;
-          return true;
-        }
+	{
+	  opts.display_usage = true;
+	  return true;
+	}
       else if (!strcmp(argv[i], "--version")
 	       || !strcmp(argv[i], "-v"))
 	{
@@ -2921,7 +2921,7 @@ main(int argc, char* argv[])
     {
       emit_prefix("abipkgdiff", cerr)
 	<< "missing operand\n"
-        "try the --help option for more information\n";
+	"try the --help option for more information\n";
       return (abigail::tools_utils::ABIDIFF_USAGE_ERROR
 	      | abigail::tools_utils::ABIDIFF_ERROR);
     }
-- 
2.26.2.303.gf8c07b1a785-goog


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 2/6] Remove excess whitespace.
  2020-04-29 17:51 [PATCH 0/6] Clean-ups Giuliano Procida
  2020-04-29 17:51 ` [PATCH 1/6] Tabify code indentation Giuliano Procida
@ 2020-04-29 17:51 ` Giuliano Procida
  2020-04-29 17:51 ` [PATCH 3/6] Remove stray semicolons Giuliano Procida
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: Giuliano Procida @ 2020-04-29 17:51 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida

This patch removes some excess blank lines and a space after the
prefix ++ operator.

	* src/abg-suppression.cc: Eliminate double blank lines.
	(read_parameter_spec_from_string): Eliminate space between
	++ operator and its operand.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-suppression.cc | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
index d3ccb63c..d9279c15 100644
--- a/src/abg-suppression.cc
+++ b/src/abg-suppression.cc
@@ -497,7 +497,6 @@ void
 type_suppression::set_consider_type_kind(bool f)
 {priv_->consider_type_kind_ = f;}
 
-
 /// Setter of the kind of type this suppression is about.
 ///
 /// Note that this will be considered during evaluation of the
@@ -802,7 +801,6 @@ type_suppression::suppresses_diff(const diff* diff) const
 		   range_end_val))
 		break;
 
-
 	      unsigned range_begin =
 		(range_begin_val < 0) ? first_type_size : range_begin_val;
 
@@ -1255,7 +1253,6 @@ type_suppression::insertion_range::integer_boundary_sptr
 type_suppression::insertion_range::create_integer_boundary(int value)
 {return integer_boundary_sptr(new integer_boundary(value));}
 
-
 /// Create a function call expression boundary.
 ///
 /// The return value of this function is to be used as a boundary for
@@ -3120,7 +3117,7 @@ read_parameter_spec_from_string(const string& str)
   if (str[cur] == '/')
     {
       is_regex = true;
-      ++ cur;
+      ++cur;
     }
 
   // look for the type name (regex)
@@ -3956,7 +3953,6 @@ variable_suppression::suppresses_variable_symbol(const elf_symbol* sym,
   else
     no_symbol_name = true;
 
-
   // Consider the symbol version.
   if (!get_symbol_version().empty())
     {
@@ -4383,7 +4379,6 @@ file_suppression_sptr
 is_file_suppression(const suppression_sptr s)
 {return dynamic_pointer_cast<file_suppression>(s);}
 
-
 /// Test if a given file path is "suppressed" by at least one file
 /// suppression specification among a vector of suppression
 /// specifications.
-- 
2.26.2.303.gf8c07b1a785-goog


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 3/6] Remove stray semicolons.
  2020-04-29 17:51 [PATCH 0/6] Clean-ups Giuliano Procida
  2020-04-29 17:51 ` [PATCH 1/6] Tabify code indentation Giuliano Procida
  2020-04-29 17:51 ` [PATCH 2/6] Remove excess whitespace Giuliano Procida
@ 2020-04-29 17:51 ` Giuliano Procida
  2020-04-30 21:32   ` Matthias Maennich
  2020-04-29 17:51 ` [PATCH 4/6] Eliminate redundant conditional operators Giuliano Procida
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: Giuliano Procida @ 2020-04-29 17:51 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida

This patch removes various stray semicolons.

	* include/abg-diff-utils.h (display_edit_script): Remove
	redundant semicolon.
	* include/abg-fwd.h (lookup_basic_type): Ditto.
	* src/abg-comparison.cc (mark_diff_as_visited):
	Ditto.	(array_diff::has_local_changes): Ditto.
	(class_diff::ensure_lookup_tables_populated): Ditto.
	* src/abg-corpus.cc
	(corpus::priv::build_unreferenced_symbols_tables): Ditto.
	* src/abg-default-reporter.cc (default_reporter::report):
	Ditto.
	* src/abg-dwarf-reader.cc (finish_member_function_reading):
	Ditto.
	* src/abg-ir.cc (is_compatible_with_class_type): Ditto.
	(enum_type_decl::enumerator::set_name): Ditto.
	* src/abg-reader.cc (read_corpus_from_input): Ditto.
	(build_function_type): Ditto.
	* src/abg-suppression.cc (type_suppression::suppresses_type):
	Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 include/abg-diff-utils.h    | 2 +-
 include/abg-fwd.h           | 2 +-
 src/abg-comparison.cc       | 6 +++---
 src/abg-corpus.cc           | 2 +-
 src/abg-default-reporter.cc | 2 +-
 src/abg-dwarf-reader.cc     | 3 +--
 src/abg-ir.cc               | 4 ++--
 src/abg-reader.cc           | 4 ++--
 src/abg-suppression.cc      | 4 ++--
 9 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/include/abg-diff-utils.h b/include/abg-diff-utils.h
index 92171a4a..3cbdbf33 100644
--- a/include/abg-diff-utils.h
+++ b/include/abg-diff-utils.h
@@ -2046,7 +2046,7 @@ display_edit_script(const edit_script& es,
   else if (es.num_deletions() == 1)
     {
       out << "1 deletion:\n"
-	  << "\t happened at index: ";;
+	  << "\t happened at index: ";
     }
   else
     {
diff --git a/include/abg-fwd.h b/include/abg-fwd.h
index 1aab70a6..c44d0f5d 100644
--- a/include/abg-fwd.h
+++ b/include/abg-fwd.h
@@ -993,7 +993,7 @@ type_decl_sptr
 lookup_basic_type(const string&, const translation_unit&);
 
 type_decl_sptr
-lookup_basic_type(const type_decl&, const corpus&);;
+lookup_basic_type(const type_decl&, const corpus&);
 
 type_decl_sptr
 lookup_basic_type(const string&, const corpus&);
diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index 46bf9e30..399c4b96 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -1320,7 +1320,7 @@ diff_context::mark_diff_as_visited(const diff* d)
   ABG_ASSERT(canonical);
 
    size_t canonical_ptr_value = reinterpret_cast<size_t>(canonical);
-   size_t diff_ptr_value = reinterpret_cast<size_t>(d);;
+   size_t diff_ptr_value = reinterpret_cast<size_t>(d);
    priv_->visited_diff_nodes_[canonical_ptr_value] = diff_ptr_value;
 }
 
@@ -3676,7 +3676,7 @@ array_diff::has_local_changes() const
   ir::change_kind k = ir::NO_CHANGE_KIND;
   if (!equals(*first_array(), *second_array(), &k))
     return k & ir::ALL_LOCAL_CHANGES_MASK;
-  return ir::NO_CHANGE_KIND;;
+  return ir::NO_CHANGE_KIND;
 }
 
 /// Report the diff in a serialized form.
@@ -5204,7 +5204,7 @@ class_diff::ensure_lookup_tables_populated(void) const
 
     vector<string> to_delete;
     corpus_sptr f = context()->get_first_corpus(),
-      s = context()->get_second_corpus();;
+      s = context()->get_second_corpus();
     if (s)
       for (string_member_function_sptr_map::const_iterator i =
 	     deleted_member_fns().begin();
diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc
index 12f44fd1..ebdf8f29 100644
--- a/src/abg-corpus.cc
+++ b/src/abg-corpus.cc
@@ -416,7 +416,7 @@ corpus::priv::build_unreferenced_symbols_tables()
 	    string sym_id = (*s)->get_id_string();
 	    if (refed_vars.find(sym_id) == refed_vars.end())
 	      {
-		bool keep = sym_id_vars_to_keep.empty() ? true : false;;
+		bool keep = sym_id_vars_to_keep.empty() ? true : false;
 		for (vector<string>::const_iterator i =
 		       sym_id_vars_to_keep.begin();
 		     i != sym_id_vars_to_keep.end();
diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc
index 04e2bb76..5892bec2 100644
--- a/src/abg-default-reporter.cc
+++ b/src/abg-default-reporter.cc
@@ -1713,7 +1713,7 @@ default_reporter::report(const corpus_diff& d, ostream& out,
 		  << c->get_pretty_representation()
 		  << "\n";
 	    }
-	  emitted = true;;
+	  emitted = true;
 	}
       if (emitted)
 	out << "\n";
diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index fe369fcf..8da5749d 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -13548,8 +13548,7 @@ finish_member_function_reading(Dwarf_Die*		  die,
     if (!f->get_parameters().empty())
       first_parm = f->get_parameters()[0];
 
-    bool is_artificial =
-      first_parm && first_parm->get_is_artificial();;
+    bool is_artificial = first_parm && first_parm->get_is_artificial();
     pointer_type_def_sptr this_ptr_type;
     type_base_sptr other_klass;
 
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 27831352..bfcaf5d3 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -7711,7 +7711,7 @@ is_compatible_with_class_type(const type_base_sptr& t)
   // CPU usage toll in exchange for finer filtering?
 
   // type_base_sptr ty = strip_typedef(t);
-  type_base_sptr ty = peel_typedef_type(t);;
+  type_base_sptr ty = peel_typedef_type(t);
   return is_class_type(ty);
 }
 
@@ -15203,7 +15203,7 @@ enum_type_decl::enumerator::set_name(const string& n)
 {
   const environment* env = get_environment();
   ABG_ASSERT(env);
-  priv_->name_ = env->intern(n);;
+  priv_->name_ = env->intern(n);
 }
 
 /// Getter for the value of @ref enum_type_decl::enumerator.
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 3727e044..4e303c80 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -2064,7 +2064,7 @@ read_corpus_from_input(read_context& ctxt)
       ctxt.set_corpus_node(node);
     }
 
-  return ctxt.get_corpus();;
+  return ctxt.get_corpus();
 }
 
 /// Parse the input XML document containing an ABI corpus group,
@@ -3840,7 +3840,7 @@ build_function_type(read_context&	ctxt,
   environment* env = ctxt.get_environment();
   ABG_ASSERT(env);
   std::vector<shared_ptr<function_decl::parameter> > parms;
-  type_base_sptr return_type = env->get_void_type();;
+  type_base_sptr return_type = env->get_void_type();
 
   class_decl_sptr method_class_type;
   if (is_method_t)
diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
index d9279c15..ab67f40a 100644
--- a/src/abg-suppression.cc
+++ b/src/abg-suppression.cc
@@ -1159,10 +1159,10 @@ bool
 type_suppression::suppresses_type(const type_base_sptr& type) const
 {
   if (!suppression_matches_type_no_name(*this, type))
-    return false;;
+    return false;
 
   if (!suppression_matches_type_name(*this, get_name(type)))
-    return false;;
+    return false;
 
   return true;
 }
-- 
2.26.2.303.gf8c07b1a785-goog


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 4/6] Eliminate redundant conditional operators.
  2020-04-29 17:51 [PATCH 0/6] Clean-ups Giuliano Procida
                   ` (2 preceding siblings ...)
  2020-04-29 17:51 ` [PATCH 3/6] Remove stray semicolons Giuliano Procida
@ 2020-04-29 17:51 ` Giuliano Procida
  2020-04-29 17:51 ` [PATCH 5/6] Make set_drops_artifact_from_ir non-const Giuliano Procida
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: Giuliano Procida @ 2020-04-29 17:51 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida

Code of the form

  bool x = expression ? true : false;

can be written more concisely as

  bool x = expression;

This patch does this. There are no occurences of "? false : true".

	* src/abg-corpus.cc (corpus::priv::build_unreferenced_symbols_tables):
	Eliminate redundant conditional operator.
	* src/abg-dwarf-reader.cc (build_reference_type): Ditto.
	* src/abg-reader.cc (read_static): Ditto.
	(read_is_artificial): Ditto. (build_function_parameter):
	Ditto. (build_function_decl): Ditto.
	(build_qualified_type_decl): Ditto.
	(build_reference_type_def): Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-corpus.cc       |  4 ++--
 src/abg-dwarf-reader.cc |  2 +-
 src/abg-reader.cc       | 16 ++++++++--------
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc
index ebdf8f29..147737d2 100644
--- a/src/abg-corpus.cc
+++ b/src/abg-corpus.cc
@@ -377,7 +377,7 @@ corpus::priv::build_unreferenced_symbols_tables()
 	    string sym_id = (*s)->get_id_string();
 	    if (refed_funs.find(sym_id) == refed_funs.end())
 	      {
-		bool keep = sym_id_fns_to_keep.empty() ? true : false;
+		bool keep = sym_id_fns_to_keep.empty();
 		for (vector<string>::const_iterator i =
 		       sym_id_fns_to_keep.begin();
 		     i != sym_id_fns_to_keep.end();
@@ -416,7 +416,7 @@ corpus::priv::build_unreferenced_symbols_tables()
 	    string sym_id = (*s)->get_id_string();
 	    if (refed_vars.find(sym_id) == refed_vars.end())
 	      {
-		bool keep = sym_id_vars_to_keep.empty() ? true : false;
+		bool keep = sym_id_vars_to_keep.empty();
 		for (vector<string>::const_iterator i =
 		       sym_id_vars_to_keep.begin();
 		     i != sym_id_vars_to_keep.end();
diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 8da5749d..937778a8 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -14779,7 +14779,7 @@ build_reference_type(read_context&	ctxt,
   // of the current translation unit.
   ABG_ASSERT((size_t) ctxt.cur_transl_unit()->get_address_size() == size);
 
-  bool is_lvalue = (tag == DW_TAG_reference_type) ? true : false;
+  bool is_lvalue = tag == DW_TAG_reference_type;
 
   result.reset(new reference_type_def(utype, is_lvalue, size,
 				      /*alignment=*/0,
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 4e303c80..2faf53f9 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -2442,7 +2442,7 @@ read_static(xmlNodePtr node, bool& is_static)
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "static"))
     {
       string b = CHAR_STR(s);
-      is_static = (b == "yes") ? true : false;
+      is_static = b == "yes";
       return true;
     }
   return false;
@@ -2567,7 +2567,7 @@ read_is_artificial(xmlNodePtr node, bool& is_artificial)
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "is-artificial"))
     {
       string is_artificial_str = CHAR_STR(s) ? CHAR_STR(s) : "";
-      is_artificial = (is_artificial_str == "yes") ? true : false;
+      is_artificial = is_artificial_str == "yes";
       return true;
     }
   return false;
@@ -3071,7 +3071,7 @@ build_function_parameter(read_context& ctxt, const xmlNodePtr node)
       xml::build_sptr(xmlGetProp(node, BAD_CAST("is-variadic"))))
     {
       is_variadic_str = CHAR_STR(s) ? CHAR_STR(s) : "";
-      is_variadic = (is_variadic_str == "yes") ? true : false;
+      is_variadic = is_variadic_str == "yes";
     }
 
   bool is_artificial = false;
@@ -3146,7 +3146,7 @@ build_function_decl(read_context&	ctxt,
   string inline_prop;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "declared-inline"))
     inline_prop = CHAR_STR(s);
-  bool declared_inline = inline_prop == "yes" ? true : false;
+  bool declared_inline = inline_prop == "yes";
 
   decl_base::visibility vis = decl_base::VISIBILITY_NONE;
   read_visibility(node, vis);
@@ -3583,17 +3583,17 @@ build_qualified_type_decl(read_context&	ctxt,
   string const_str;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "const"))
     const_str = CHAR_STR(s);
-  bool const_cv = const_str == "yes" ? true : false;
+  bool const_cv = const_str == "yes";
 
   string volatile_str;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "volatile"))
     volatile_str = CHAR_STR(s);
-  bool volatile_cv = volatile_str == "yes" ? true : false;
+  bool volatile_cv = volatile_str == "yes";
 
   string restrict_str;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "restrict"))
     restrict_str = CHAR_STR(s);
-  bool restrict_cv = restrict_str == "yes" ? true : false;
+  bool restrict_cv = restrict_str == "yes";
 
   qualified_type_def::CV cv = qualified_type_def::CV_NONE;
   if (const_cv)
@@ -3745,7 +3745,7 @@ build_reference_type_def(read_context&		ctxt,
   string kind;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "kind"))
     kind = CHAR_STR(s); // this should be either "lvalue" or "rvalue".
-  bool is_lvalue = kind == "lvalue" ? true : false;
+  bool is_lvalue = kind == "lvalue";
 
   string type_id;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "type-id"))
-- 
2.26.2.303.gf8c07b1a785-goog


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 5/6] Make set_drops_artifact_from_ir non-const.
  2020-04-29 17:51 [PATCH 0/6] Clean-ups Giuliano Procida
                   ` (3 preceding siblings ...)
  2020-04-29 17:51 ` [PATCH 4/6] Eliminate redundant conditional operators Giuliano Procida
@ 2020-04-29 17:51 ` Giuliano Procida
  2020-04-29 17:51 ` [PATCH 6/6] Hoist some common expressions evaluating offsets Giuliano Procida
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: Giuliano Procida @ 2020-04-29 17:51 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida

Setters should be non-const but set_drops_artifact_from_ir isn't.
This patch fixes this.

This reason this didn't cause a compilation failure is that const
shared_ptr<X> is equivalent to X *const, not const X*.

Note that resolving the apparent const-safety issue will require
std::experimental::propagate_const or similar.

	* include/abg-suppression.h
	(suppression_base::set_drops_artifact_from_ir):
	Drop const qualifier.
	* src/abg-suppression.cc
	(suppression_base::set_drops_artifact_from_ir):
	Drop const qualifier.

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

diff --git a/include/abg-suppression.h b/include/abg-suppression.h
index 4f1fb417..6383b932 100644
--- a/include/abg-suppression.h
+++ b/include/abg-suppression.h
@@ -71,7 +71,7 @@ public:
   get_drops_artifact_from_ir() const;
 
   void
-  set_drops_artifact_from_ir(bool) const;
+  set_drops_artifact_from_ir(bool);
 
   bool
   get_is_artificial() const;
diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
index ab67f40a..6d9a0f5f 100644
--- a/src/abg-suppression.cc
+++ b/src/abg-suppression.cc
@@ -96,7 +96,7 @@ suppression_base::get_drops_artifact_from_ir() const
 /// specification is to avoid adding the matched ABI artifact to the
 /// internal representation.
 void
-suppression_base::set_drops_artifact_from_ir(bool f) const
+suppression_base::set_drops_artifact_from_ir(bool f)
 {priv_->drops_artifact_ = f;}
 
 /// Test is the suppression specification is artificial.
-- 
2.26.2.303.gf8c07b1a785-goog


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 6/6] Hoist some common expressions evaluating offsets.
  2020-04-29 17:51 [PATCH 0/6] Clean-ups Giuliano Procida
                   ` (4 preceding siblings ...)
  2020-04-29 17:51 ` [PATCH 5/6] Make set_drops_artifact_from_ir non-const Giuliano Procida
@ 2020-04-29 17:51 ` Giuliano Procida
  2020-04-30 21:34 ` [PATCH 0/6] Clean-ups Matthias Maennich
  2020-05-01 15:51 ` [PATCH v2 0/5] Clean-ups Giuliano Procida
  7 siblings, 0 replies; 22+ messages in thread
From: Giuliano Procida @ 2020-04-29 17:51 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida

This is for readability.

	* src/abg-suppression.cc (type_suppression::suppresses_diff):
	Hoist some constant expressions out of loops.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-suppression.cc | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
index 6d9a0f5f..3b02182d 100644
--- a/src/abg-suppression.cc
+++ b/src/abg-suppression.cc
@@ -770,6 +770,11 @@ type_suppression::suppresses_diff(const diff* diff) const
       && (klass_diff->first_class_decl()->get_size_in_bits()
 	  <= klass_diff->second_class_decl()->get_size_in_bits()))
     {
+      const class_decl_sptr& first_type_decl = klass_diff->first_class_decl();
+      const class_decl_sptr& second_type_decl = klass_diff->second_class_decl();
+      size_t first_type_size = first_type_decl->get_size_in_bits();
+      size_t second_type_size = second_type_decl->get_size_in_bits();
+
       for (string_decl_base_sptr_map::const_iterator m =
 	     klass_diff->inserted_data_members().begin();
 	   m != klass_diff->inserted_data_members().end();
@@ -777,10 +782,6 @@ type_suppression::suppresses_diff(const diff* diff) const
 	{
 	  decl_base_sptr member = m->second;
 	  size_t dm_offset = get_data_member_offset(member);
-	  size_t first_type_size =
-	    klass_diff->first_class_decl()->get_size_in_bits();
-	  size_t second_type_size =
-	    klass_diff->second_class_decl()->get_size_in_bits();
 	  bool matched = false;
 
 	  for (insertion_ranges::const_iterator i =
@@ -791,14 +792,10 @@ type_suppression::suppresses_diff(const diff* diff) const
 	      type_suppression::insertion_range_sptr range = *i;
 	      ssize_t range_begin_val = 0,range_end_val = 0;
 	      if (!type_suppression::insertion_range::eval_boundary
-		  (range->begin(),
-		   klass_diff->first_class_decl(),
-		   range_begin_val))
+		  (range->begin(), first_type_decl, range_begin_val))
 		break;
 	      if (!type_suppression::insertion_range::eval_boundary
-		  (range->end(),
-		   klass_diff->first_class_decl(),
-		   range_end_val))
+		  (range->end(), first_type_decl, range_end_val))
 		break;
 
 	      unsigned range_begin =
-- 
2.26.2.303.gf8c07b1a785-goog


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/6] Remove stray semicolons.
  2020-04-29 17:51 ` [PATCH 3/6] Remove stray semicolons Giuliano Procida
@ 2020-04-30 21:32   ` Matthias Maennich
  2020-04-30 21:34     ` Matthias Maennich
  0 siblings, 1 reply; 22+ messages in thread
From: Matthias Maennich @ 2020-04-30 21:32 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: libabigail, dodji, kernel-team

On Wed, Apr 29, 2020 at 06:51:30PM +0100, Android Kernel Team wrote:
>This patch removes various stray semicolons.
>
>	* include/abg-diff-utils.h (display_edit_script): Remove
>	redundant semicolon.
>	* include/abg-fwd.h (lookup_basic_type): Ditto.
>	* src/abg-comparison.cc (mark_diff_as_visited):
>	Ditto.	(array_diff::has_local_changes): Ditto.
>	(class_diff::ensure_lookup_tables_populated): Ditto.
>	* src/abg-corpus.cc
>	(corpus::priv::build_unreferenced_symbols_tables): Ditto.
>	* src/abg-default-reporter.cc (default_reporter::report):
>	Ditto.
>	* src/abg-dwarf-reader.cc (finish_member_function_reading):
>	Ditto.
>	* src/abg-ir.cc (is_compatible_with_class_type): Ditto.
>	(enum_type_decl::enumerator::set_name): Ditto.
>	* src/abg-reader.cc (read_corpus_from_input): Ditto.
>	(build_function_type): Ditto.
>	* src/abg-suppression.cc (type_suppression::suppresses_type):
>	Ditto.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>
>---
> include/abg-diff-utils.h    | 2 +-
> include/abg-fwd.h           | 2 +-
> src/abg-comparison.cc       | 6 +++---
> src/abg-corpus.cc           | 2 +-
> src/abg-default-reporter.cc | 2 +-
> src/abg-dwarf-reader.cc     | 3 +--
> src/abg-ir.cc               | 4 ++--
> src/abg-reader.cc           | 4 ++--
> src/abg-suppression.cc      | 4 ++--
> 9 files changed, 14 insertions(+), 15 deletions(-)
>
>diff --git a/include/abg-diff-utils.h b/include/abg-diff-utils.h
>index 92171a4a..3cbdbf33 100644
>--- a/include/abg-diff-utils.h
>+++ b/include/abg-diff-utils.h
>@@ -2046,7 +2046,7 @@ display_edit_script(const edit_script& es,
>   else if (es.num_deletions() == 1)
>     {
>       out << "1 deletion:\n"
>-	  << "\t happened at index: ";;
>+	  << "\t happened at index: ";
>     }
>   else
>     {
>diff --git a/include/abg-fwd.h b/include/abg-fwd.h
>index 1aab70a6..c44d0f5d 100644
>--- a/include/abg-fwd.h
>+++ b/include/abg-fwd.h
>@@ -993,7 +993,7 @@ type_decl_sptr
> lookup_basic_type(const string&, const translation_unit&);
>
> type_decl_sptr
>-lookup_basic_type(const type_decl&, const corpus&);;
>+lookup_basic_type(const type_decl&, const corpus&);
>
> type_decl_sptr
> lookup_basic_type(const string&, const corpus&);
>diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
>index 46bf9e30..399c4b96 100644
>--- a/src/abg-comparison.cc
>+++ b/src/abg-comparison.cc
>@@ -1320,7 +1320,7 @@ diff_context::mark_diff_as_visited(const diff* d)
>   ABG_ASSERT(canonical);
>
>    size_t canonical_ptr_value = reinterpret_cast<size_t>(canonical);
>-   size_t diff_ptr_value = reinterpret_cast<size_t>(d);;
>+   size_t diff_ptr_value = reinterpret_cast<size_t>(d);
>    priv_->visited_diff_nodes_[canonical_ptr_value] = diff_ptr_value;
> }
>
>@@ -3676,7 +3676,7 @@ array_diff::has_local_changes() const
>   ir::change_kind k = ir::NO_CHANGE_KIND;
>   if (!equals(*first_array(), *second_array(), &k))
>     return k & ir::ALL_LOCAL_CHANGES_MASK;
>-  return ir::NO_CHANGE_KIND;;
>+  return ir::NO_CHANGE_KIND;
> }
>
> /// Report the diff in a serialized form.
>@@ -5204,7 +5204,7 @@ class_diff::ensure_lookup_tables_populated(void) const
>
>     vector<string> to_delete;
>     corpus_sptr f = context()->get_first_corpus(),
>-      s = context()->get_second_corpus();;
>+      s = context()->get_second_corpus();
>     if (s)
>       for (string_member_function_sptr_map::const_iterator i =
> 	     deleted_member_fns().begin();
>diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc
>index 12f44fd1..ebdf8f29 100644
>--- a/src/abg-corpus.cc
>+++ b/src/abg-corpus.cc
>@@ -416,7 +416,7 @@ corpus::priv::build_unreferenced_symbols_tables()
> 	    string sym_id = (*s)->get_id_string();
> 	    if (refed_vars.find(sym_id) == refed_vars.end())
> 	      {
>-		bool keep = sym_id_vars_to_keep.empty() ? true : false;;
>+		bool keep = sym_id_vars_to_keep.empty() ? true : false;

Ehm, can we clean this up along with this patch? bool = bool ? true : false ....

> 		for (vector<string>::const_iterator i =
> 		       sym_id_vars_to_keep.begin();
> 		     i != sym_id_vars_to_keep.end();
>diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc
>index 04e2bb76..5892bec2 100644
>--- a/src/abg-default-reporter.cc
>+++ b/src/abg-default-reporter.cc
>@@ -1713,7 +1713,7 @@ default_reporter::report(const corpus_diff& d, ostream& out,
> 		  << c->get_pretty_representation()
> 		  << "\n";
> 	    }
>-	  emitted = true;;
>+	  emitted = true;
> 	}
>       if (emitted)
> 	out << "\n";
>diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
>index fe369fcf..8da5749d 100644
>--- a/src/abg-dwarf-reader.cc
>+++ b/src/abg-dwarf-reader.cc
>@@ -13548,8 +13548,7 @@ finish_member_function_reading(Dwarf_Die*		  die,
>     if (!f->get_parameters().empty())
>       first_parm = f->get_parameters()[0];
>
>-    bool is_artificial =
>-      first_parm && first_parm->get_is_artificial();;
>+    bool is_artificial = first_parm && first_parm->get_is_artificial();
>     pointer_type_def_sptr this_ptr_type;
>     type_base_sptr other_klass;
>
>diff --git a/src/abg-ir.cc b/src/abg-ir.cc
>index 27831352..bfcaf5d3 100644
>--- a/src/abg-ir.cc
>+++ b/src/abg-ir.cc
>@@ -7711,7 +7711,7 @@ is_compatible_with_class_type(const type_base_sptr& t)
>   // CPU usage toll in exchange for finer filtering?
>
>   // type_base_sptr ty = strip_typedef(t);
>-  type_base_sptr ty = peel_typedef_type(t);;
>+  type_base_sptr ty = peel_typedef_type(t);
>   return is_class_type(ty);
> }
>
>@@ -15203,7 +15203,7 @@ enum_type_decl::enumerator::set_name(const string& n)
> {
>   const environment* env = get_environment();
>   ABG_ASSERT(env);
>-  priv_->name_ = env->intern(n);;
>+  priv_->name_ = env->intern(n);
> }
>
> /// Getter for the value of @ref enum_type_decl::enumerator.
>diff --git a/src/abg-reader.cc b/src/abg-reader.cc
>index 3727e044..4e303c80 100644
>--- a/src/abg-reader.cc
>+++ b/src/abg-reader.cc
>@@ -2064,7 +2064,7 @@ read_corpus_from_input(read_context& ctxt)
>       ctxt.set_corpus_node(node);
>     }
>
>-  return ctxt.get_corpus();;
>+  return ctxt.get_corpus();
> }
>
> /// Parse the input XML document containing an ABI corpus group,
>@@ -3840,7 +3840,7 @@ build_function_type(read_context&	ctxt,
>   environment* env = ctxt.get_environment();
>   ABG_ASSERT(env);
>   std::vector<shared_ptr<function_decl::parameter> > parms;
>-  type_base_sptr return_type = env->get_void_type();;
>+  type_base_sptr return_type = env->get_void_type();
>
>   class_decl_sptr method_class_type;
>   if (is_method_t)
>diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
>index d9279c15..ab67f40a 100644
>--- a/src/abg-suppression.cc
>+++ b/src/abg-suppression.cc
>@@ -1159,10 +1159,10 @@ bool
> type_suppression::suppresses_type(const type_base_sptr& type) const
> {
>   if (!suppression_matches_type_no_name(*this, type))
>-    return false;;
>+    return false;
>
>   if (!suppression_matches_type_name(*this, get_name(type)))
>-    return false;;
>+    return false;
>
>   return true;
> }
>-- 
>2.26.2.303.gf8c07b1a785-goog
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/6] Remove stray semicolons.
  2020-04-30 21:32   ` Matthias Maennich
@ 2020-04-30 21:34     ` Matthias Maennich
  0 siblings, 0 replies; 22+ messages in thread
From: Matthias Maennich @ 2020-04-30 21:34 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: libabigail, dodji, kernel-team

On Thu, Apr 30, 2020 at 11:32:36PM +0200, Matthias Maennich wrote:
>On Wed, Apr 29, 2020 at 06:51:30PM +0100, Android Kernel Team wrote:
>>This patch removes various stray semicolons.
>>
>>	* include/abg-diff-utils.h (display_edit_script): Remove
>>	redundant semicolon.
>>	* include/abg-fwd.h (lookup_basic_type): Ditto.
>>	* src/abg-comparison.cc (mark_diff_as_visited):
>>	Ditto.	(array_diff::has_local_changes): Ditto.
>>	(class_diff::ensure_lookup_tables_populated): Ditto.
>>	* src/abg-corpus.cc
>>	(corpus::priv::build_unreferenced_symbols_tables): Ditto.
>>	* src/abg-default-reporter.cc (default_reporter::report):
>>	Ditto.
>>	* src/abg-dwarf-reader.cc (finish_member_function_reading):
>>	Ditto.
>>	* src/abg-ir.cc (is_compatible_with_class_type): Ditto.
>>	(enum_type_decl::enumerator::set_name): Ditto.
>>	* src/abg-reader.cc (read_corpus_from_input): Ditto.
>>	(build_function_type): Ditto.
>>	* src/abg-suppression.cc (type_suppression::suppresses_type):
>>	Ditto.
>>
>>Signed-off-by: Giuliano Procida <gprocida@google.com>
>>---
>>include/abg-diff-utils.h    | 2 +-
>>include/abg-fwd.h           | 2 +-
>>src/abg-comparison.cc       | 6 +++---
>>src/abg-corpus.cc           | 2 +-
>>src/abg-default-reporter.cc | 2 +-
>>src/abg-dwarf-reader.cc     | 3 +--
>>src/abg-ir.cc               | 4 ++--
>>src/abg-reader.cc           | 4 ++--
>>src/abg-suppression.cc      | 4 ++--
>>9 files changed, 14 insertions(+), 15 deletions(-)
>>
>>diff --git a/include/abg-diff-utils.h b/include/abg-diff-utils.h
>>index 92171a4a..3cbdbf33 100644
>>--- a/include/abg-diff-utils.h
>>+++ b/include/abg-diff-utils.h
>>@@ -2046,7 +2046,7 @@ display_edit_script(const edit_script& es,
>>  else if (es.num_deletions() == 1)
>>    {
>>      out << "1 deletion:\n"
>>-	  << "\t happened at index: ";;
>>+	  << "\t happened at index: ";
>>    }
>>  else
>>    {
>>diff --git a/include/abg-fwd.h b/include/abg-fwd.h
>>index 1aab70a6..c44d0f5d 100644
>>--- a/include/abg-fwd.h
>>+++ b/include/abg-fwd.h
>>@@ -993,7 +993,7 @@ type_decl_sptr
>>lookup_basic_type(const string&, const translation_unit&);
>>
>>type_decl_sptr
>>-lookup_basic_type(const type_decl&, const corpus&);;
>>+lookup_basic_type(const type_decl&, const corpus&);
>>
>>type_decl_sptr
>>lookup_basic_type(const string&, const corpus&);
>>diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
>>index 46bf9e30..399c4b96 100644
>>--- a/src/abg-comparison.cc
>>+++ b/src/abg-comparison.cc
>>@@ -1320,7 +1320,7 @@ diff_context::mark_diff_as_visited(const diff* d)
>>  ABG_ASSERT(canonical);
>>
>>   size_t canonical_ptr_value = reinterpret_cast<size_t>(canonical);
>>-   size_t diff_ptr_value = reinterpret_cast<size_t>(d);;
>>+   size_t diff_ptr_value = reinterpret_cast<size_t>(d);
>>   priv_->visited_diff_nodes_[canonical_ptr_value] = diff_ptr_value;
>>}
>>
>>@@ -3676,7 +3676,7 @@ array_diff::has_local_changes() const
>>  ir::change_kind k = ir::NO_CHANGE_KIND;
>>  if (!equals(*first_array(), *second_array(), &k))
>>    return k & ir::ALL_LOCAL_CHANGES_MASK;
>>-  return ir::NO_CHANGE_KIND;;
>>+  return ir::NO_CHANGE_KIND;
>>}
>>
>>/// Report the diff in a serialized form.
>>@@ -5204,7 +5204,7 @@ class_diff::ensure_lookup_tables_populated(void) const
>>
>>    vector<string> to_delete;
>>    corpus_sptr f = context()->get_first_corpus(),
>>-      s = context()->get_second_corpus();;
>>+      s = context()->get_second_corpus();
>>    if (s)
>>      for (string_member_function_sptr_map::const_iterator i =
>>	     deleted_member_fns().begin();
>>diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc
>>index 12f44fd1..ebdf8f29 100644
>>--- a/src/abg-corpus.cc
>>+++ b/src/abg-corpus.cc
>>@@ -416,7 +416,7 @@ corpus::priv::build_unreferenced_symbols_tables()
>>	    string sym_id = (*s)->get_id_string();
>>	    if (refed_vars.find(sym_id) == refed_vars.end())
>>	      {
>>-		bool keep = sym_id_vars_to_keep.empty() ? true : false;;
>>+		bool keep = sym_id_vars_to_keep.empty() ? true : false;
>
>Ehm, can we clean this up along with this patch? bool = bool ? true : false ....
>

Sorry for the noise, a later patch does exactly that.

Cheers,
Matthias

>>		for (vector<string>::const_iterator i =
>>		       sym_id_vars_to_keep.begin();
>>		     i != sym_id_vars_to_keep.end();
>>diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc
>>index 04e2bb76..5892bec2 100644
>>--- a/src/abg-default-reporter.cc
>>+++ b/src/abg-default-reporter.cc
>>@@ -1713,7 +1713,7 @@ default_reporter::report(const corpus_diff& d, ostream& out,
>>		  << c->get_pretty_representation()
>>		  << "\n";
>>	    }
>>-	  emitted = true;;
>>+	  emitted = true;
>>	}
>>      if (emitted)
>>	out << "\n";
>>diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
>>index fe369fcf..8da5749d 100644
>>--- a/src/abg-dwarf-reader.cc
>>+++ b/src/abg-dwarf-reader.cc
>>@@ -13548,8 +13548,7 @@ finish_member_function_reading(Dwarf_Die*		  die,
>>    if (!f->get_parameters().empty())
>>      first_parm = f->get_parameters()[0];
>>
>>-    bool is_artificial =
>>-      first_parm && first_parm->get_is_artificial();;
>>+    bool is_artificial = first_parm && first_parm->get_is_artificial();
>>    pointer_type_def_sptr this_ptr_type;
>>    type_base_sptr other_klass;
>>
>>diff --git a/src/abg-ir.cc b/src/abg-ir.cc
>>index 27831352..bfcaf5d3 100644
>>--- a/src/abg-ir.cc
>>+++ b/src/abg-ir.cc
>>@@ -7711,7 +7711,7 @@ is_compatible_with_class_type(const type_base_sptr& t)
>>  // CPU usage toll in exchange for finer filtering?
>>
>>  // type_base_sptr ty = strip_typedef(t);
>>-  type_base_sptr ty = peel_typedef_type(t);;
>>+  type_base_sptr ty = peel_typedef_type(t);
>>  return is_class_type(ty);
>>}
>>
>>@@ -15203,7 +15203,7 @@ enum_type_decl::enumerator::set_name(const string& n)
>>{
>>  const environment* env = get_environment();
>>  ABG_ASSERT(env);
>>-  priv_->name_ = env->intern(n);;
>>+  priv_->name_ = env->intern(n);
>>}
>>
>>/// Getter for the value of @ref enum_type_decl::enumerator.
>>diff --git a/src/abg-reader.cc b/src/abg-reader.cc
>>index 3727e044..4e303c80 100644
>>--- a/src/abg-reader.cc
>>+++ b/src/abg-reader.cc
>>@@ -2064,7 +2064,7 @@ read_corpus_from_input(read_context& ctxt)
>>      ctxt.set_corpus_node(node);
>>    }
>>
>>-  return ctxt.get_corpus();;
>>+  return ctxt.get_corpus();
>>}
>>
>>/// Parse the input XML document containing an ABI corpus group,
>>@@ -3840,7 +3840,7 @@ build_function_type(read_context&	ctxt,
>>  environment* env = ctxt.get_environment();
>>  ABG_ASSERT(env);
>>  std::vector<shared_ptr<function_decl::parameter> > parms;
>>-  type_base_sptr return_type = env->get_void_type();;
>>+  type_base_sptr return_type = env->get_void_type();
>>
>>  class_decl_sptr method_class_type;
>>  if (is_method_t)
>>diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
>>index d9279c15..ab67f40a 100644
>>--- a/src/abg-suppression.cc
>>+++ b/src/abg-suppression.cc
>>@@ -1159,10 +1159,10 @@ bool
>>type_suppression::suppresses_type(const type_base_sptr& type) const
>>{
>>  if (!suppression_matches_type_no_name(*this, type))
>>-    return false;;
>>+    return false;
>>
>>  if (!suppression_matches_type_name(*this, get_name(type)))
>>-    return false;;
>>+    return false;
>>
>>  return true;
>>}
>>-- 
>>2.26.2.303.gf8c07b1a785-goog
>>
>>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] Clean-ups
  2020-04-29 17:51 [PATCH 0/6] Clean-ups Giuliano Procida
                   ` (5 preceding siblings ...)
  2020-04-29 17:51 ` [PATCH 6/6] Hoist some common expressions evaluating offsets Giuliano Procida
@ 2020-04-30 21:34 ` Matthias Maennich
  2020-05-01 15:45   ` Giuliano Procida
  2020-05-01 15:51 ` [PATCH v2 0/5] Clean-ups Giuliano Procida
  7 siblings, 1 reply; 22+ messages in thread
From: Matthias Maennich @ 2020-04-30 21:34 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: libabigail, dodji, kernel-team

On Wed, Apr 29, 2020 at 06:51:27PM +0100, Android Kernel Team wrote:
>These are almost all independent. There's just 1 line of code ending:
>
>? true : false;;
>
>Regards,
>Giuliano.

Hi Giuliano!

Those patches look all good to me and surely give my OCD a soothing
feeling :-)

Yet I would like to hold back at least 1/6 as it will collide with the
refactoring work I am currently doing in and around abg-corpus and
abg-dwarf-reader.

Nevertheless, for the whole series:
Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>
>Giuliano Procida (6):
>  Tabify code indentation.
>  Remove excess whitespace.
>  Remove stray semicolons.
>  Eliminate redundant conditional operators.
>  Make set_drops_artifact_from_ir non-const.
>  Hoist some common expressions evaluating offsets.
>
> include/abg-diff-utils.h    |   2 +-
> include/abg-fwd.h           |   2 +-
> include/abg-suppression.h   |   2 +-
> src/abg-comparison.cc       |   6 +-
> src/abg-corpus.cc           |   4 +-
> src/abg-default-reporter.cc |   2 +-
> src/abg-dwarf-reader.cc     | 143 +++++++++++++++++-----------------
> src/abg-ir.cc               |   4 +-
> src/abg-reader.cc           |  20 ++---
> src/abg-suppression.cc      |  30 +++-----
> src/abg-tools-utils.cc      |  12 +--
> src/abg-writer.cc           |  42 +++++-----
> tools/abidiff.cc            |   2 +-
> tools/abipkgdiff.cc         | 148 ++++++++++++++++++------------------
> 14 files changed, 205 insertions(+), 214 deletions(-)
>
>-- 
>2.26.2.303.gf8c07b1a785-goog
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] Clean-ups
  2020-04-30 21:34 ` [PATCH 0/6] Clean-ups Matthias Maennich
@ 2020-05-01 15:45   ` Giuliano Procida
  0 siblings, 0 replies; 22+ messages in thread
From: Giuliano Procida @ 2020-05-01 15:45 UTC (permalink / raw)
  To: Matthias Maennich; +Cc: libabigail, Dodji Seketeli, kernel-team

I'll drop the tabify patch and repost the other with any review updates.

Thanks,
Giuliano.

On Thu, 30 Apr 2020 at 22:34, Matthias Maennich <maennich@google.com> wrote:
>
> On Wed, Apr 29, 2020 at 06:51:27PM +0100, Android Kernel Team wrote:
> >These are almost all independent. There's just 1 line of code ending:
> >
> >? true : false;;
> >
> >Regards,
> >Giuliano.
>
> Hi Giuliano!
>
> Those patches look all good to me and surely give my OCD a soothing
> feeling :-)
>
> Yet I would like to hold back at least 1/6 as it will collide with the
> refactoring work I am currently doing in and around abg-corpus and
> abg-dwarf-reader.
>
> Nevertheless, for the whole series:
> Reviewed-by: Matthias Maennich <maennich@google.com>
>
> Cheers,
> Matthias
>
> >
> >Giuliano Procida (6):
> >  Tabify code indentation.
> >  Remove excess whitespace.
> >  Remove stray semicolons.
> >  Eliminate redundant conditional operators.
> >  Make set_drops_artifact_from_ir non-const.
> >  Hoist some common expressions evaluating offsets.
> >
> > include/abg-diff-utils.h    |   2 +-
> > include/abg-fwd.h           |   2 +-
> > include/abg-suppression.h   |   2 +-
> > src/abg-comparison.cc       |   6 +-
> > src/abg-corpus.cc           |   4 +-
> > src/abg-default-reporter.cc |   2 +-
> > src/abg-dwarf-reader.cc     | 143 +++++++++++++++++-----------------
> > src/abg-ir.cc               |   4 +-
> > src/abg-reader.cc           |  20 ++---
> > src/abg-suppression.cc      |  30 +++-----
> > src/abg-tools-utils.cc      |  12 +--
> > src/abg-writer.cc           |  42 +++++-----
> > tools/abidiff.cc            |   2 +-
> > tools/abipkgdiff.cc         | 148 ++++++++++++++++++------------------
> > 14 files changed, 205 insertions(+), 214 deletions(-)
> >
> >--
> >2.26.2.303.gf8c07b1a785-goog
> >
> >

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 0/5] Clean-ups.
  2020-04-29 17:51 [PATCH 0/6] Clean-ups Giuliano Procida
                   ` (6 preceding siblings ...)
  2020-04-30 21:34 ` [PATCH 0/6] Clean-ups Matthias Maennich
@ 2020-05-01 15:51 ` Giuliano Procida
  2020-05-01 15:51   ` [PATCH v2 1/5] Remove excess whitespace Giuliano Procida
                     ` (4 more replies)
  7 siblings, 5 replies; 22+ messages in thread
From: Giuliano Procida @ 2020-05-01 15:51 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida, maennich

Hi.

These are almost all independent. There's just 1 line of code ending
"? true : false;;" so patches and 2 and 3 cannot be reordered without
manual intervention.

This is a repost without the tabification commit, to avoid colliding
with ongoing work by Matthias.

Regards,
Giuliano.

Giuliano Procida (5):
  Remove excess whitespace.
  Remove stray semicolons.
  Eliminate redundant conditional operators.
  Make set_drops_artifact_from_ir non-const.
  Hoist some common expressions evaluating offsets.

 include/abg-diff-utils.h    |  2 +-
 include/abg-fwd.h           |  2 +-
 include/abg-suppression.h   |  2 +-
 src/abg-comparison.cc       |  6 +++---
 src/abg-corpus.cc           |  4 ++--
 src/abg-default-reporter.cc |  2 +-
 src/abg-dwarf-reader.cc     |  5 ++---
 src/abg-ir.cc               |  4 ++--
 src/abg-reader.cc           | 20 ++++++++++----------
 src/abg-suppression.cc      | 30 +++++++++++-------------------
 10 files changed, 34 insertions(+), 43 deletions(-)

-- 
2.26.2.526.g744177e7f7-goog


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 1/5] Remove excess whitespace.
  2020-05-01 15:51 ` [PATCH v2 0/5] Clean-ups Giuliano Procida
@ 2020-05-01 15:51   ` Giuliano Procida
  2020-05-11 12:06     ` Dodji Seketeli
  2020-05-01 15:51   ` [PATCH v2 2/5] Remove stray semicolons Giuliano Procida
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Giuliano Procida @ 2020-05-01 15:51 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida, maennich

This patch removes some excess blank lines and a space after the
prefix ++ operator.

	* src/abg-suppression.cc: Eliminate double blank lines.
	(read_parameter_spec_from_string): Eliminate space between
	++ operator and its operand.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-suppression.cc | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
index d3ccb63c..d9279c15 100644
--- a/src/abg-suppression.cc
+++ b/src/abg-suppression.cc
@@ -497,7 +497,6 @@ void
 type_suppression::set_consider_type_kind(bool f)
 {priv_->consider_type_kind_ = f;}
 
-
 /// Setter of the kind of type this suppression is about.
 ///
 /// Note that this will be considered during evaluation of the
@@ -802,7 +801,6 @@ type_suppression::suppresses_diff(const diff* diff) const
 		   range_end_val))
 		break;
 
-
 	      unsigned range_begin =
 		(range_begin_val < 0) ? first_type_size : range_begin_val;
 
@@ -1255,7 +1253,6 @@ type_suppression::insertion_range::integer_boundary_sptr
 type_suppression::insertion_range::create_integer_boundary(int value)
 {return integer_boundary_sptr(new integer_boundary(value));}
 
-
 /// Create a function call expression boundary.
 ///
 /// The return value of this function is to be used as a boundary for
@@ -3120,7 +3117,7 @@ read_parameter_spec_from_string(const string& str)
   if (str[cur] == '/')
     {
       is_regex = true;
-      ++ cur;
+      ++cur;
     }
 
   // look for the type name (regex)
@@ -3956,7 +3953,6 @@ variable_suppression::suppresses_variable_symbol(const elf_symbol* sym,
   else
     no_symbol_name = true;
 
-
   // Consider the symbol version.
   if (!get_symbol_version().empty())
     {
@@ -4383,7 +4379,6 @@ file_suppression_sptr
 is_file_suppression(const suppression_sptr s)
 {return dynamic_pointer_cast<file_suppression>(s);}
 
-
 /// Test if a given file path is "suppressed" by at least one file
 /// suppression specification among a vector of suppression
 /// specifications.
-- 
2.26.2.526.g744177e7f7-goog


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 2/5] Remove stray semicolons.
  2020-05-01 15:51 ` [PATCH v2 0/5] Clean-ups Giuliano Procida
  2020-05-01 15:51   ` [PATCH v2 1/5] Remove excess whitespace Giuliano Procida
@ 2020-05-01 15:51   ` Giuliano Procida
  2020-05-11 12:14     ` Dodji Seketeli
  2020-05-01 15:51   ` [PATCH v2 3/5] Eliminate redundant conditional operators Giuliano Procida
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Giuliano Procida @ 2020-05-01 15:51 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida, maennich

This patch removes various stray semicolons.

	* include/abg-diff-utils.h (display_edit_script): Remove
	redundant semicolon.
	* include/abg-fwd.h (lookup_basic_type): Ditto.
	* src/abg-comparison.cc (mark_diff_as_visited):
	Ditto.	(array_diff::has_local_changes): Ditto.
	(class_diff::ensure_lookup_tables_populated): Ditto.
	* src/abg-corpus.cc
	(corpus::priv::build_unreferenced_symbols_tables): Ditto.
	* src/abg-default-reporter.cc (default_reporter::report):
	Ditto.
	* src/abg-dwarf-reader.cc (finish_member_function_reading):
	Ditto.
	* src/abg-ir.cc (is_compatible_with_class_type): Ditto.
	(enum_type_decl::enumerator::set_name): Ditto.
	* src/abg-reader.cc (read_corpus_from_input): Ditto.
	(build_function_type): Ditto.
	* src/abg-suppression.cc (type_suppression::suppresses_type):
	Ditto.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 include/abg-diff-utils.h    | 2 +-
 include/abg-fwd.h           | 2 +-
 src/abg-comparison.cc       | 6 +++---
 src/abg-corpus.cc           | 2 +-
 src/abg-default-reporter.cc | 2 +-
 src/abg-dwarf-reader.cc     | 3 +--
 src/abg-ir.cc               | 4 ++--
 src/abg-reader.cc           | 4 ++--
 src/abg-suppression.cc      | 4 ++--
 9 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/include/abg-diff-utils.h b/include/abg-diff-utils.h
index 92171a4a..3cbdbf33 100644
--- a/include/abg-diff-utils.h
+++ b/include/abg-diff-utils.h
@@ -2046,7 +2046,7 @@ display_edit_script(const edit_script& es,
   else if (es.num_deletions() == 1)
     {
       out << "1 deletion:\n"
-	  << "\t happened at index: ";;
+	  << "\t happened at index: ";
     }
   else
     {
diff --git a/include/abg-fwd.h b/include/abg-fwd.h
index 1aab70a6..c44d0f5d 100644
--- a/include/abg-fwd.h
+++ b/include/abg-fwd.h
@@ -993,7 +993,7 @@ type_decl_sptr
 lookup_basic_type(const string&, const translation_unit&);
 
 type_decl_sptr
-lookup_basic_type(const type_decl&, const corpus&);;
+lookup_basic_type(const type_decl&, const corpus&);
 
 type_decl_sptr
 lookup_basic_type(const string&, const corpus&);
diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc
index 46bf9e30..399c4b96 100644
--- a/src/abg-comparison.cc
+++ b/src/abg-comparison.cc
@@ -1320,7 +1320,7 @@ diff_context::mark_diff_as_visited(const diff* d)
   ABG_ASSERT(canonical);
 
    size_t canonical_ptr_value = reinterpret_cast<size_t>(canonical);
-   size_t diff_ptr_value = reinterpret_cast<size_t>(d);;
+   size_t diff_ptr_value = reinterpret_cast<size_t>(d);
    priv_->visited_diff_nodes_[canonical_ptr_value] = diff_ptr_value;
 }
 
@@ -3676,7 +3676,7 @@ array_diff::has_local_changes() const
   ir::change_kind k = ir::NO_CHANGE_KIND;
   if (!equals(*first_array(), *second_array(), &k))
     return k & ir::ALL_LOCAL_CHANGES_MASK;
-  return ir::NO_CHANGE_KIND;;
+  return ir::NO_CHANGE_KIND;
 }
 
 /// Report the diff in a serialized form.
@@ -5204,7 +5204,7 @@ class_diff::ensure_lookup_tables_populated(void) const
 
     vector<string> to_delete;
     corpus_sptr f = context()->get_first_corpus(),
-      s = context()->get_second_corpus();;
+      s = context()->get_second_corpus();
     if (s)
       for (string_member_function_sptr_map::const_iterator i =
 	     deleted_member_fns().begin();
diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc
index 12f44fd1..ebdf8f29 100644
--- a/src/abg-corpus.cc
+++ b/src/abg-corpus.cc
@@ -416,7 +416,7 @@ corpus::priv::build_unreferenced_symbols_tables()
 	    string sym_id = (*s)->get_id_string();
 	    if (refed_vars.find(sym_id) == refed_vars.end())
 	      {
-		bool keep = sym_id_vars_to_keep.empty() ? true : false;;
+		bool keep = sym_id_vars_to_keep.empty() ? true : false;
 		for (vector<string>::const_iterator i =
 		       sym_id_vars_to_keep.begin();
 		     i != sym_id_vars_to_keep.end();
diff --git a/src/abg-default-reporter.cc b/src/abg-default-reporter.cc
index 04e2bb76..5892bec2 100644
--- a/src/abg-default-reporter.cc
+++ b/src/abg-default-reporter.cc
@@ -1713,7 +1713,7 @@ default_reporter::report(const corpus_diff& d, ostream& out,
 		  << c->get_pretty_representation()
 		  << "\n";
 	    }
-	  emitted = true;;
+	  emitted = true;
 	}
       if (emitted)
 	out << "\n";
diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 850281ad..8cbf0046 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -13548,8 +13548,7 @@ finish_member_function_reading(Dwarf_Die*		  die,
     if (!f->get_parameters().empty())
       first_parm = f->get_parameters()[0];
 
-    bool is_artificial =
-      first_parm && first_parm->get_is_artificial();;
+    bool is_artificial = first_parm && first_parm->get_is_artificial();
     pointer_type_def_sptr this_ptr_type;
     type_base_sptr other_klass;
 
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 27831352..bfcaf5d3 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -7711,7 +7711,7 @@ is_compatible_with_class_type(const type_base_sptr& t)
   // CPU usage toll in exchange for finer filtering?
 
   // type_base_sptr ty = strip_typedef(t);
-  type_base_sptr ty = peel_typedef_type(t);;
+  type_base_sptr ty = peel_typedef_type(t);
   return is_class_type(ty);
 }
 
@@ -15203,7 +15203,7 @@ enum_type_decl::enumerator::set_name(const string& n)
 {
   const environment* env = get_environment();
   ABG_ASSERT(env);
-  priv_->name_ = env->intern(n);;
+  priv_->name_ = env->intern(n);
 }
 
 /// Getter for the value of @ref enum_type_decl::enumerator.
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 255a200f..2e31f320 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -2064,7 +2064,7 @@ read_corpus_from_input(read_context& ctxt)
       ctxt.set_corpus_node(node);
     }
 
-  return ctxt.get_corpus();;
+  return ctxt.get_corpus();
 }
 
 /// Parse the input XML document containing an ABI corpus group,
@@ -3836,7 +3836,7 @@ build_function_type(read_context&	ctxt,
   environment* env = ctxt.get_environment();
   ABG_ASSERT(env);
   std::vector<shared_ptr<function_decl::parameter> > parms;
-  type_base_sptr return_type = env->get_void_type();;
+  type_base_sptr return_type = env->get_void_type();
 
   class_decl_sptr method_class_type;
   if (is_method_t)
diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
index d9279c15..ab67f40a 100644
--- a/src/abg-suppression.cc
+++ b/src/abg-suppression.cc
@@ -1159,10 +1159,10 @@ bool
 type_suppression::suppresses_type(const type_base_sptr& type) const
 {
   if (!suppression_matches_type_no_name(*this, type))
-    return false;;
+    return false;
 
   if (!suppression_matches_type_name(*this, get_name(type)))
-    return false;;
+    return false;
 
   return true;
 }
-- 
2.26.2.526.g744177e7f7-goog


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 3/5] Eliminate redundant conditional operators.
  2020-05-01 15:51 ` [PATCH v2 0/5] Clean-ups Giuliano Procida
  2020-05-01 15:51   ` [PATCH v2 1/5] Remove excess whitespace Giuliano Procida
  2020-05-01 15:51   ` [PATCH v2 2/5] Remove stray semicolons Giuliano Procida
@ 2020-05-01 15:51   ` Giuliano Procida
  2020-05-11 12:25     ` Dodji Seketeli
  2020-05-01 15:51   ` [PATCH v2 4/5] Make set_drops_artifact_from_ir non-const Giuliano Procida
  2020-05-01 15:51   ` [PATCH v2 5/5] Hoist some common expressions evaluating offsets Giuliano Procida
  4 siblings, 1 reply; 22+ messages in thread
From: Giuliano Procida @ 2020-05-01 15:51 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida, maennich

Code of the form

  bool x = expression ? true : false;

can be written more concisely as

  bool x = expression;

This patch does this. There are no occurences of "? false : true".

	* src/abg-corpus.cc (corpus::priv::build_unreferenced_symbols_tables):
	Eliminate redundant conditional operator.
	* src/abg-dwarf-reader.cc (build_reference_type): Ditto.
	* src/abg-reader.cc (read_static): Ditto.
	(read_is_artificial): Ditto. (build_function_parameter):
	Ditto. (build_function_decl): Ditto.
	(build_qualified_type_decl): Ditto.
	(build_reference_type_def): Ditto.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-corpus.cc       |  4 ++--
 src/abg-dwarf-reader.cc |  2 +-
 src/abg-reader.cc       | 16 ++++++++--------
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/abg-corpus.cc b/src/abg-corpus.cc
index ebdf8f29..147737d2 100644
--- a/src/abg-corpus.cc
+++ b/src/abg-corpus.cc
@@ -377,7 +377,7 @@ corpus::priv::build_unreferenced_symbols_tables()
 	    string sym_id = (*s)->get_id_string();
 	    if (refed_funs.find(sym_id) == refed_funs.end())
 	      {
-		bool keep = sym_id_fns_to_keep.empty() ? true : false;
+		bool keep = sym_id_fns_to_keep.empty();
 		for (vector<string>::const_iterator i =
 		       sym_id_fns_to_keep.begin();
 		     i != sym_id_fns_to_keep.end();
@@ -416,7 +416,7 @@ corpus::priv::build_unreferenced_symbols_tables()
 	    string sym_id = (*s)->get_id_string();
 	    if (refed_vars.find(sym_id) == refed_vars.end())
 	      {
-		bool keep = sym_id_vars_to_keep.empty() ? true : false;
+		bool keep = sym_id_vars_to_keep.empty();
 		for (vector<string>::const_iterator i =
 		       sym_id_vars_to_keep.begin();
 		     i != sym_id_vars_to_keep.end();
diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
index 8cbf0046..63837554 100644
--- a/src/abg-dwarf-reader.cc
+++ b/src/abg-dwarf-reader.cc
@@ -14779,7 +14779,7 @@ build_reference_type(read_context&	ctxt,
   // of the current translation unit.
   ABG_ASSERT((size_t) ctxt.cur_transl_unit()->get_address_size() == size);
 
-  bool is_lvalue = (tag == DW_TAG_reference_type) ? true : false;
+  bool is_lvalue = tag == DW_TAG_reference_type;
 
   result.reset(new reference_type_def(utype, is_lvalue, size,
 				      /*alignment=*/0,
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 2e31f320..47ac6229 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -2442,7 +2442,7 @@ read_static(xmlNodePtr node, bool& is_static)
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "static"))
     {
       string b = CHAR_STR(s);
-      is_static = (b == "yes") ? true : false;
+      is_static = b == "yes";
       return true;
     }
   return false;
@@ -2567,7 +2567,7 @@ read_is_artificial(xmlNodePtr node, bool& is_artificial)
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "is-artificial"))
     {
       string is_artificial_str = CHAR_STR(s) ? CHAR_STR(s) : "";
-      is_artificial = (is_artificial_str == "yes") ? true : false;
+      is_artificial = is_artificial_str == "yes";
       return true;
     }
   return false;
@@ -3071,7 +3071,7 @@ build_function_parameter(read_context& ctxt, const xmlNodePtr node)
       xml::build_sptr(xmlGetProp(node, BAD_CAST("is-variadic"))))
     {
       is_variadic_str = CHAR_STR(s) ? CHAR_STR(s) : "";
-      is_variadic = (is_variadic_str == "yes") ? true : false;
+      is_variadic = is_variadic_str == "yes";
     }
 
   bool is_artificial = false;
@@ -3146,7 +3146,7 @@ build_function_decl(read_context&	ctxt,
   string inline_prop;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "declared-inline"))
     inline_prop = CHAR_STR(s);
-  bool declared_inline = inline_prop == "yes" ? true : false;
+  bool declared_inline = inline_prop == "yes";
 
   decl_base::visibility vis = decl_base::VISIBILITY_NONE;
   read_visibility(node, vis);
@@ -3583,17 +3583,17 @@ build_qualified_type_decl(read_context&	ctxt,
   string const_str;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "const"))
     const_str = CHAR_STR(s);
-  bool const_cv = const_str == "yes" ? true : false;
+  bool const_cv = const_str == "yes";
 
   string volatile_str;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "volatile"))
     volatile_str = CHAR_STR(s);
-  bool volatile_cv = volatile_str == "yes" ? true : false;
+  bool volatile_cv = volatile_str == "yes";
 
   string restrict_str;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "restrict"))
     restrict_str = CHAR_STR(s);
-  bool restrict_cv = restrict_str == "yes" ? true : false;
+  bool restrict_cv = restrict_str == "yes";
 
   qualified_type_def::CV cv = qualified_type_def::CV_NONE;
   if (const_cv)
@@ -3743,7 +3743,7 @@ build_reference_type_def(read_context&		ctxt,
   string kind;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "kind"))
     kind = CHAR_STR(s); // this should be either "lvalue" or "rvalue".
-  bool is_lvalue = kind == "lvalue" ? true : false;
+  bool is_lvalue = kind == "lvalue";
 
   string type_id;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "type-id"))
-- 
2.26.2.526.g744177e7f7-goog


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 4/5] Make set_drops_artifact_from_ir non-const.
  2020-05-01 15:51 ` [PATCH v2 0/5] Clean-ups Giuliano Procida
                     ` (2 preceding siblings ...)
  2020-05-01 15:51   ` [PATCH v2 3/5] Eliminate redundant conditional operators Giuliano Procida
@ 2020-05-01 15:51   ` Giuliano Procida
  2020-05-11 13:03     ` Dodji Seketeli
  2020-05-01 15:51   ` [PATCH v2 5/5] Hoist some common expressions evaluating offsets Giuliano Procida
  4 siblings, 1 reply; 22+ messages in thread
From: Giuliano Procida @ 2020-05-01 15:51 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida, maennich

Setters should be non-const but set_drops_artifact_from_ir isn't.
This patch fixes this.

This reason this didn't cause a compilation failure is that const
shared_ptr<X> is equivalent to X *const, not const X*.

Note that resolving the apparent const-safety issue will require
std::experimental::propagate_const or similar.

	* include/abg-suppression.h
	(suppression_base::set_drops_artifact_from_ir):
	Drop const qualifier.
	* src/abg-suppression.cc
	(suppression_base::set_drops_artifact_from_ir):
	Drop const qualifier.

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

diff --git a/include/abg-suppression.h b/include/abg-suppression.h
index 4f1fb417..6383b932 100644
--- a/include/abg-suppression.h
+++ b/include/abg-suppression.h
@@ -71,7 +71,7 @@ public:
   get_drops_artifact_from_ir() const;
 
   void
-  set_drops_artifact_from_ir(bool) const;
+  set_drops_artifact_from_ir(bool);
 
   bool
   get_is_artificial() const;
diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
index ab67f40a..6d9a0f5f 100644
--- a/src/abg-suppression.cc
+++ b/src/abg-suppression.cc
@@ -96,7 +96,7 @@ suppression_base::get_drops_artifact_from_ir() const
 /// specification is to avoid adding the matched ABI artifact to the
 /// internal representation.
 void
-suppression_base::set_drops_artifact_from_ir(bool f) const
+suppression_base::set_drops_artifact_from_ir(bool f)
 {priv_->drops_artifact_ = f;}
 
 /// Test is the suppression specification is artificial.
-- 
2.26.2.526.g744177e7f7-goog


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 5/5] Hoist some common expressions evaluating offsets.
  2020-05-01 15:51 ` [PATCH v2 0/5] Clean-ups Giuliano Procida
                     ` (3 preceding siblings ...)
  2020-05-01 15:51   ` [PATCH v2 4/5] Make set_drops_artifact_from_ir non-const Giuliano Procida
@ 2020-05-01 15:51   ` Giuliano Procida
  2020-05-11 13:12     ` Dodji Seketeli
  4 siblings, 1 reply; 22+ messages in thread
From: Giuliano Procida @ 2020-05-01 15:51 UTC (permalink / raw)
  To: libabigail; +Cc: dodji, kernel-team, gprocida, maennich

This is for readability.

	* src/abg-suppression.cc (type_suppression::suppresses_diff):
	Hoist some constant expressions out of loops.

Reviewed-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-suppression.cc | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc
index 6d9a0f5f..3b02182d 100644
--- a/src/abg-suppression.cc
+++ b/src/abg-suppression.cc
@@ -770,6 +770,11 @@ type_suppression::suppresses_diff(const diff* diff) const
       && (klass_diff->first_class_decl()->get_size_in_bits()
 	  <= klass_diff->second_class_decl()->get_size_in_bits()))
     {
+      const class_decl_sptr& first_type_decl = klass_diff->first_class_decl();
+      const class_decl_sptr& second_type_decl = klass_diff->second_class_decl();
+      size_t first_type_size = first_type_decl->get_size_in_bits();
+      size_t second_type_size = second_type_decl->get_size_in_bits();
+
       for (string_decl_base_sptr_map::const_iterator m =
 	     klass_diff->inserted_data_members().begin();
 	   m != klass_diff->inserted_data_members().end();
@@ -777,10 +782,6 @@ type_suppression::suppresses_diff(const diff* diff) const
 	{
 	  decl_base_sptr member = m->second;
 	  size_t dm_offset = get_data_member_offset(member);
-	  size_t first_type_size =
-	    klass_diff->first_class_decl()->get_size_in_bits();
-	  size_t second_type_size =
-	    klass_diff->second_class_decl()->get_size_in_bits();
 	  bool matched = false;
 
 	  for (insertion_ranges::const_iterator i =
@@ -791,14 +792,10 @@ type_suppression::suppresses_diff(const diff* diff) const
 	      type_suppression::insertion_range_sptr range = *i;
 	      ssize_t range_begin_val = 0,range_end_val = 0;
 	      if (!type_suppression::insertion_range::eval_boundary
-		  (range->begin(),
-		   klass_diff->first_class_decl(),
-		   range_begin_val))
+		  (range->begin(), first_type_decl, range_begin_val))
 		break;
 	      if (!type_suppression::insertion_range::eval_boundary
-		  (range->end(),
-		   klass_diff->first_class_decl(),
-		   range_end_val))
+		  (range->end(), first_type_decl, range_end_val))
 		break;
 
 	      unsigned range_begin =
-- 
2.26.2.526.g744177e7f7-goog


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/5] Remove excess whitespace.
  2020-05-01 15:51   ` [PATCH v2 1/5] Remove excess whitespace Giuliano Procida
@ 2020-05-11 12:06     ` Dodji Seketeli
  0 siblings, 0 replies; 22+ messages in thread
From: Dodji Seketeli @ 2020-05-11 12:06 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: libabigail, kernel-team, maennich

Giuliano Procida <gprocida@google.com> a écrit:

> This patch removes some excess blank lines and a space after the
> prefix ++ operator.
>
> 	* src/abg-suppression.cc: Eliminate double blank lines.
> 	(read_parameter_spec_from_string): Eliminate space between
> 	++ operator and its operand.

Applied to master, thanks!

Cheers,

-- 
		Dodji

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 2/5] Remove stray semicolons.
  2020-05-01 15:51   ` [PATCH v2 2/5] Remove stray semicolons Giuliano Procida
@ 2020-05-11 12:14     ` Dodji Seketeli
  0 siblings, 0 replies; 22+ messages in thread
From: Dodji Seketeli @ 2020-05-11 12:14 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: libabigail, kernel-team, maennich

Giuliano Procida <gprocida@google.com> a écrit:

> This patch removes various stray semicolons.
>
> 	* include/abg-diff-utils.h (display_edit_script): Remove
> 	redundant semicolon.
> 	* include/abg-fwd.h (lookup_basic_type): Ditto.
> 	* src/abg-comparison.cc (mark_diff_as_visited):
> 	Ditto.	(array_diff::has_local_changes): Ditto.
> 	(class_diff::ensure_lookup_tables_populated): Ditto.
> 	* src/abg-corpus.cc
> 	(corpus::priv::build_unreferenced_symbols_tables): Ditto.
> 	* src/abg-default-reporter.cc (default_reporter::report):
> 	Ditto.
> 	* src/abg-dwarf-reader.cc (finish_member_function_reading):
> 	Ditto.
> 	* src/abg-ir.cc (is_compatible_with_class_type): Ditto.
> 	(enum_type_decl::enumerator::set_name): Ditto.
> 	* src/abg-reader.cc (read_corpus_from_input): Ditto.
> 	(build_function_type): Ditto.
> 	* src/abg-suppression.cc (type_suppression::suppresses_type):
> 	Ditto.

Applied to master, thanks!

Cheers,

-- 
		Dodji

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 3/5] Eliminate redundant conditional operators.
  2020-05-01 15:51   ` [PATCH v2 3/5] Eliminate redundant conditional operators Giuliano Procida
@ 2020-05-11 12:25     ` Dodji Seketeli
  0 siblings, 0 replies; 22+ messages in thread
From: Dodji Seketeli @ 2020-05-11 12:25 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: libabigail, kernel-team, maennich

Giuliano Procida <gprocida@google.com> a écrit:

> Code of the form
>
>   bool x = expression ? true : false;
>
> can be written more concisely as
>
>   bool x = expression;
>
> This patch does this. There are no occurences of "? false : true".
>
> 	* src/abg-corpus.cc (corpus::priv::build_unreferenced_symbols_tables):
> 	Eliminate redundant conditional operator.
> 	* src/abg-dwarf-reader.cc (build_reference_type): Ditto.
> 	* src/abg-reader.cc (read_static): Ditto.
> 	(read_is_artificial): Ditto. (build_function_parameter):
> 	Ditto. (build_function_decl): Ditto.
> 	(build_qualified_type_decl): Ditto.
> 	(build_reference_type_def): Ditto.

Applied to master, thanks!

Cheers,

-- 
		Dodji

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 4/5] Make set_drops_artifact_from_ir non-const.
  2020-05-01 15:51   ` [PATCH v2 4/5] Make set_drops_artifact_from_ir non-const Giuliano Procida
@ 2020-05-11 13:03     ` Dodji Seketeli
  0 siblings, 0 replies; 22+ messages in thread
From: Dodji Seketeli @ 2020-05-11 13:03 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: libabigail, kernel-team, maennich

Giuliano Procida <gprocida@google.com> a écrit:

> Setters should be non-const but set_drops_artifact_from_ir isn't.
> This patch fixes this.
>
> This reason this didn't cause a compilation failure is that const
> shared_ptr<X> is equivalent to X *const, not const X*.
>
> Note that resolving the apparent const-safety issue will require
> std::experimental::propagate_const or similar.
>
> 	* include/abg-suppression.h
> 	(suppression_base::set_drops_artifact_from_ir):
> 	Drop const qualifier.
> 	* src/abg-suppression.cc
> 	(suppression_base::set_drops_artifact_from_ir):
> 	Drop const qualifier.

Applied to master, thanks!

Cheers,


-- 
		Dodji

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 5/5] Hoist some common expressions evaluating offsets.
  2020-05-01 15:51   ` [PATCH v2 5/5] Hoist some common expressions evaluating offsets Giuliano Procida
@ 2020-05-11 13:12     ` Dodji Seketeli
  0 siblings, 0 replies; 22+ messages in thread
From: Dodji Seketeli @ 2020-05-11 13:12 UTC (permalink / raw)
  To: Giuliano Procida; +Cc: libabigail, kernel-team, maennich

Giuliano Procida <gprocida@google.com> a écrit:

> This is for readability.
>
> 	* src/abg-suppression.cc (type_suppression::suppresses_diff):
> 	Hoist some constant expressions out of loops.

Applied to master, thanks!

Cheers,

-- 
		Dodji

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2020-05-11 13:12 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-29 17:51 [PATCH 0/6] Clean-ups Giuliano Procida
2020-04-29 17:51 ` [PATCH 1/6] Tabify code indentation Giuliano Procida
2020-04-29 17:51 ` [PATCH 2/6] Remove excess whitespace Giuliano Procida
2020-04-29 17:51 ` [PATCH 3/6] Remove stray semicolons Giuliano Procida
2020-04-30 21:32   ` Matthias Maennich
2020-04-30 21:34     ` Matthias Maennich
2020-04-29 17:51 ` [PATCH 4/6] Eliminate redundant conditional operators Giuliano Procida
2020-04-29 17:51 ` [PATCH 5/6] Make set_drops_artifact_from_ir non-const Giuliano Procida
2020-04-29 17:51 ` [PATCH 6/6] Hoist some common expressions evaluating offsets Giuliano Procida
2020-04-30 21:34 ` [PATCH 0/6] Clean-ups Matthias Maennich
2020-05-01 15:45   ` Giuliano Procida
2020-05-01 15:51 ` [PATCH v2 0/5] Clean-ups Giuliano Procida
2020-05-01 15:51   ` [PATCH v2 1/5] Remove excess whitespace Giuliano Procida
2020-05-11 12:06     ` Dodji Seketeli
2020-05-01 15:51   ` [PATCH v2 2/5] Remove stray semicolons Giuliano Procida
2020-05-11 12:14     ` Dodji Seketeli
2020-05-01 15:51   ` [PATCH v2 3/5] Eliminate redundant conditional operators Giuliano Procida
2020-05-11 12:25     ` Dodji Seketeli
2020-05-01 15:51   ` [PATCH v2 4/5] Make set_drops_artifact_from_ir non-const Giuliano Procida
2020-05-11 13:03     ` Dodji Seketeli
2020-05-01 15:51   ` [PATCH v2 5/5] Hoist some common expressions evaluating offsets Giuliano Procida
2020-05-11 13:12     ` Dodji Seketeli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).