Re: [PATCH v4] Add .clang-format

[Kevin Buettner <kevinb@redhat.com>]

On Mon,  6 Oct 2025 12:38:59 -0600
Tom Tromey <tom@tromey.com> wrote:

> This patch adds a .clang-format file to the gdb repository.
> 
> The resulting reformatting is what I'd describe as "ok but not great".
> There are a few variances from our normal style, some discussed in
> comments in the file, and some in the bug.
> 
> I've somewhat come around to the idea that some ugliness is
> acceptable, particularly because I regularly see code that's already
> ugly anyway -- either in formatting or along some other dimension.
> 
> I don't know of a way to enforce a particular version.  I have only
> tried clang-format 18 with this particular file.  I've documented this
> in the file.
> 
> I used "AllowShortFunctionsOnASingleLine: InlineOnly" as previously
> discussed.  I feel that the spirit of the GNU style is that vertical
> space is free, and we should use "None" here.  (This goes against
> something we previously decided on the list, though.)
> 
> The file is in the root directory for ease of use.
> 
> For the time being you should not bulk reformat files.  I think we
> should have a flag day for this, but at some later point.  See the
> earlier discussion for details.
> 
> New in v4:
> * Fix a comment
> * Remove ForEachMacros - no longer correct
> * Remove IncludeCategories - no longer correct
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30098
> ---
>  .clang-format | 174 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  .gitignore    |   1 -
>  2 files changed, 174 insertions(+), 1 deletion(-)
>  create mode 100644 .clang-format

I spent some time playing around with this.

I formatted gdb/dwarf2/read.c, which is one of the biggest GDB source
files.

I tried four versions of clang-format, versions 18, 19, 20, and 21.

This is the diff between v18 and v19:

--- read.c.v18	2025-10-10 20:55:13.151282243 -0700
+++ read.c.v19	2025-10-10 20:56:02.673050802 -0700
@@ -392,9 +392,7 @@
   /* The sections that make up this DWO file.
      Remember that for virtual DWO files in DWP V2 or DWP V5, these are
virtual sections (for lack of a better name).  */
-  struct dwo_sections sections
-  {
-  };
+  struct dwo_sections sections {};
 
   /* The CUs in the file.
 
@@ -601,9 +599,7 @@
   gdb_bfd_ref_ptr dbfd;
 
   /* Section info for this file.  */
-  struct dwp_sections sections
-  {
-  };
+  struct dwp_sections sections {};
 
   /* Table of CUs in the file.  */
   const struct dwp_hash_table *cus = nullptr;
@@ -679,9 +675,7 @@
      reference.  We track the section offset of each field to make
      this link.  */
   sect_offset offset;
-  struct field field
-  {
-  };
+  struct field field {};
 };
 
 struct fnfieldlist
@@ -7236,9 +7230,7 @@
 							 : dwp_file->cus;
   bfd *dbfd = dwp_file->dbfd.get ();
   const char *kind = is_debug_types ? "TU" : "CU";
-  struct virtual_v2_or_v5_dwo_sections sections
-  {
-  };
+  struct virtual_v2_or_v5_dwo_sections sections {};
 
   gdb_assert (dwp_file->version == 5);
 
There were no differences between v19 and v20.

--- read.c.v20	2025-10-10 20:58:13.960085581 -0700
+++ read.c.v21	2025-10-10 20:58:50.203646704 -0700
@@ -2489,9 +2489,8 @@
   /* Preserve the ordering of per_bfd->all_units.  */
   auto insert_it = std::lower_bound (
     per_bfd->all_units.begin (), per_bfd->all_units.end (), sig_type,
-    [] (const dwarf2_per_cu_up &lhs, const signatured_type *rhs) {
-      return all_units_less_than (*lhs, { rhs->section, rhs->sect_off });
-    });
+    [] (const dwarf2_per_cu_up &lhs, const signatured_type *rhs)
+      { return all_units_less_than (*lhs, { rhs->section, rhs->sect_off
}); }); 
   per_bfd->all_units.emplace (insert_it, sig_type_holder.release ());
   auto emplace_ret = per_bfd->signatured_types.emplace (sig_type);
@@ -2723,11 +2722,12 @@
   int next_attr_idx = 0;
 
   /* Push an element into ATTRIBUTES.  */
-  auto push_back = [&] (struct attribute *attr) {
-    gdb_assert (next_attr_idx < ARRAY_SIZE (attributes));
-    if (attr != nullptr)
-      attributes[next_attr_idx++] = attr;
-  };
+  auto push_back = [&] (struct attribute *attr)
+    {
+      gdb_assert (next_attr_idx < ARRAY_SIZE (attributes));
+      if (attr != nullptr)
+	attributes[next_attr_idx++] = attr;
+    };
 
   if (stub_comp_unit_die != NULL)
     {
@@ -3313,10 +3313,12 @@
   private:
     void process_one (dwarf2_per_cu &unit)
     {
-      m_thread_storage.catch_error ([&] () {
-	m_parent->process_unit (&unit, m_parent->m_per_objfile,
-				&m_thread_storage);
-      });
+      m_thread_storage.catch_error (
+	[&] ()
+	  {
+	    m_parent->process_unit (&unit, m_parent->m_per_objfile,
+				    &m_thread_storage);
+	  });
     }
 
     /* Measures the execution time of this worker.  */
@@ -3546,9 +3548,8 @@
       cutu_reader reader (*tu.sig_type, *per_objfile, abbrev_table.get (),
 			  nullptr, false, language_minimal);
       if (!reader.is_dummy ())
-	storage->catch_error ([&] () {
-	  process_type_unit (&reader, storage);
-	});
+	storage->catch_error ([&] ()
+				{ process_type_unit (&reader, storage); });
     }
 }
 
@@ -3590,9 +3591,12 @@
   if (per_objfile->per_bfd->dwp_file == nullptr)
     for (const dwo_file_up &file : per_objfile->per_bfd->dwo_files)
       for (const dwo_unit_up &unit : file->tus)
-	storage->catch_error ([&] () {
-	  process_skeletonless_type_unit (unit.get (), per_objfile,
storage);
-	});
+	storage->catch_error (
+	  [&] ()
+	    {
+	      process_skeletonless_type_unit (unit.get (), per_objfile,
+					      storage);
+	    });
 }
 
 void
@@ -3702,9 +3706,10 @@
   /* Ensure that the all_units vector is in the expected order for
      dwarf2_find_containing_unit to be able to perform a binary search.  */
   std::sort (per_bfd->all_units.begin (), per_bfd->all_units.end (),
-	     [] (const dwarf2_per_cu_up &a, const dwarf2_per_cu_up &b) {
-	       return all_units_less_than (*a, { b->section, b->sect_off
});
-	     });
+	     [] (const dwarf2_per_cu_up &a, const dwarf2_per_cu_up &b)
+	       {
+		 return all_units_less_than (*a, { b->section, b->sect_off
});
+	       });
 }
 
 /* See read.h.  */
@@ -9386,32 +9391,33 @@
 
   retval = dwarf2_ranges_process (
     offset, cu, tag,
-    [&] (unrelocated_addr range_beginning, unrelocated_addr range_end) {
-      if (map != nullptr)
-	{
-	  /* addrmap only accepts CORE_ADDR, so we must cast here.  */
-	  map->set_empty ((CORE_ADDR) range_beginning,
-			  (CORE_ADDR) range_end - 1, datum);
-	}
+    [&] (unrelocated_addr range_beginning, unrelocated_addr range_end)
+      {
+	if (map != nullptr)
+	  {
+	    /* addrmap only accepts CORE_ADDR, so we must cast here.  */
+	    map->set_empty ((CORE_ADDR) range_beginning,
+			    (CORE_ADDR) range_end - 1, datum);
+	  }
 
-      /* FIXME: This is recording everything as a low-high
+	/* FIXME: This is recording everything as a low-high
 	 segment of consecutive addresses.  We should have a
 	 data structure for discontiguous block ranges
 	 instead.  */
-      if (!low_set)
-	{
-	  low = range_beginning;
-	  high = range_end;
-	  low_set = 1;
-	}
-      else
-	{
-	  if (range_beginning < low)
+	if (!low_set)
+	  {
 	    low = range_beginning;
-	  if (range_end > high)
 	    high = range_end;
-	}
-    });
+	    low_set = 1;
+	  }
+	else
+	  {
+	    if (range_beginning < low)
+	      low = range_beginning;
+	    if (range_end > high)
+	      high = range_end;
+	  }
+      });
   if (!retval)
     return 0;
 
@@ -9435,9 +9441,8 @@
 			      std::vector<unrelocated_addr> &result)
 {
   dwarf2_ranges_process (offset, cu, tag,
-			 [&] (unrelocated_addr start, unrelocated_addr
end) {
-			   result.push_back (start);
-			 });
+			 [&] (unrelocated_addr start, unrelocated_addr end)
+			   { result.push_back (start); });
 }
 
 /* Determine the low and high pc of a DW_TAG_entry_point.  */
@@ -9874,13 +9879,14 @@
 	  std::vector<blockrange> blockvec;
 	  dwarf2_ranges_process (
 	    ranges_offset, cu, die->tag,
-	    [&] (unrelocated_addr start, unrelocated_addr end) {
-	      CORE_ADDR abs_start = per_objfile->relocate (start);
-	      CORE_ADDR abs_end = per_objfile->relocate (end);
-	      cu->get_builder ()->record_block_range (block, abs_start,
-						      abs_end - 1);
-	      blockvec.emplace_back (abs_start, abs_end);
-	    });
+	    [&] (unrelocated_addr start, unrelocated_addr end)
+	      {
+		CORE_ADDR abs_start = per_objfile->relocate (start);
+		CORE_ADDR abs_end = per_objfile->relocate (end);
+		cu->get_builder ()->record_block_range (block, abs_start,
+							abs_end - 1);
+		blockvec.emplace_back (abs_start, abs_end);
+	      });
 
 	  block->set_ranges (make_blockranges (objfile, blockvec));
 	}
@@ -18425,9 +18431,10 @@
 {
   auto it = std::lower_bound (all_units.begin (), all_units.end (), target,
 			      [] (const dwarf2_per_cu_up &per_cu,
-				  const section_and_offset &key) {
-				return all_units_less_than (*per_cu, key);
-			      });
+				  const section_and_offset &key)
+				{
+				  return all_units_less_than (*per_cu,
key);
+				});
 
   if (it == all_units.begin ())
     {
@@ -18466,11 +18473,12 @@
   dwarf2_per_bfd *per_bfd = per_objfile->per_bfd;
   dwarf2_per_cu *per_cu = dwarf2_find_containing_unit (target,
 						       per_bfd->all_units);
-  auto error_out = [&target, per_bfd] () {
-    error (_ (DWARF_ERROR_PREFIX
-	      "could not find unit containing offset %s [in module %s]"),
-	   sect_offset_str (target.offset), per_bfd->filename ());
-  };
+  auto error_out = [&target, per_bfd] ()
+    {
+      error (_ (DWARF_ERROR_PREFIX
+		"could not find unit containing offset %s [in module %s]"),
+	     sect_offset_str (target.offset), per_bfd->filename ());
+    };
 
   if (per_cu == nullptr)
     error_out ();
@@ -18500,12 +18508,10 @@
 dwarf2_per_cu *
 dwarf2_find_unit (const section_and_offset &start, dwarf2_per_bfd *per_bfd)
 {
-  auto it = std::lower_bound (per_bfd->all_units.begin (),
-			      per_bfd->all_units.end (), start,
-			      [] (const dwarf2_per_cu_up &per_cu,
-				  const section_and_offset &key) {
-				return all_units_less_than (*per_cu, key);
-			      });
+  auto it = std::lower_bound (
+    per_bfd->all_units.begin (), per_bfd->all_units.end (), start,
+    [] (const dwarf2_per_cu_up &per_cu, const section_and_offset &key)
+      { return all_units_less_than (*per_cu, key); });
 
   if (it == per_bfd->all_units.end ())
     return nullptr;
@@ -18537,13 +18543,14 @@
      reference.  */
   auto create_dummy_per_unit = [&] (dwarf2_section_info &section,
 				    unsigned int sect_off,
-				    bool is_dwz) -> dwarf2_per_cu & {
-    /* Omit the length, because dwarf2_find_containing_unit does not
consider
+				    bool is_dwz) -> dwarf2_per_cu &
+    {
+      /* Omit the length, because dwarf2_find_containing_unit does not
consider it.  */
-    return *units.emplace_back (new dwarf2_per_cu (dummy_per_bfd, &section,
-						   sect_offset (sect_off),
0,
-						   is_dwz));
-  };
+      return *units.emplace_back (new dwarf2_per_cu (dummy_per_bfd,
&section,
+						     sect_offset
(sect_off), 0,
+						     is_dwz));
+    };
 
   /* Create 2 units in the main file and 2 units in the supplementary (dwz)
      file.  */
@@ -18556,15 +18563,16 @@
      section SECTION finds EXPECTED.  */
   auto check_range = [&units] (dwarf2_section_info &section,
 			       unsigned int start, unsigned int end,
-			       dwarf2_per_cu *expected) {
-    for (unsigned int sect_off = start; sect_off < end; ++sect_off)
-      {
-	section_and_offset target { &section, sect_offset (sect_off) };
-	dwarf2_per_cu *result = dwarf2_find_containing_unit (target,
units);
+			       dwarf2_per_cu *expected)
+    {
+      for (unsigned int sect_off = start; sect_off < end; ++sect_off)
+	{
+	  section_and_offset target { &section, sect_offset (sect_off) };
+	  dwarf2_per_cu *result = dwarf2_find_containing_unit (target,
units); 
-	SELF_CHECK (result == expected);
-      }
-  };
+	  SELF_CHECK (result == expected);
+	}
+    };
 
   check_range (main_section, 0, 10, nullptr);
   check_range (main_section, 10, 20, &main1);

=========================================================

Just skimming those (above), I mostly like the v21 formatting better.

Next, I made a diff between the original source and the v18 formatted
version.  Below, I'll point out the odd things that I saw:

@@ -181,29 +181,27 @@
 /* Note that if the debugging section has been compressed, it might
    have a name like .zdebug_info.  */
 
-const struct dwarf2_debug_sections dwarf2_elf_names =
-{
-  { ".debug_info", ".zdebug_info" },
...
+const struct dwarf2_debug_sections dwarf2_elf_names
+  = { { ".debug_info", ".zdebug_info" },
...

I do not like the double left curly braces on the same line.

There are a bunch of changes like this one:

@@ -441,7 +448,9 @@
 bool
 dwo_file_eq::operator() (const dwo_file_up &a,
 			 const dwo_file_up &b) const noexcept
-{ return (*this) ({ a->dwo_name.c_str (), a->comp_dir }, b); }
+{
+  return (*this) ({ a->dwo_name.c_str (), a->comp_dir }, b);
+}
 
 /* These sections are what may appear in a DWP file.  */
 
This change is fine with me - I like it better.

Here's another change that I like better. clang-format has placed the
backslashes all in the same column...

@@ -542,12 +551,12 @@
     {
       /* This is indexed by column number and gives the id of the section
 	 in that column.  */
-#define MAX_NR_V2_DWO_SECTIONS \
-  (1 /* .debug_info or .debug_types */ \
-   + 1 /* .debug_abbrev */ \
-   + 1 /* .debug_line */ \
-   + 1 /* .debug_loc */ \
-   + 1 /* .debug_str_offsets */ \
+#define MAX_NR_V2_DWO_SECTIONS           \
+  (1   /* .debug_info or .debug_types */ \
+   + 1 /* .debug_abbrev */               \
+   + 1 /* .debug_line */                 \
+   + 1 /* .debug_loc */                  \
+   + 1 /* .debug_str_offsets */          \
    + 1 /* .debug_macro or .debug_macinfo */)
       int section_ids[MAX_NR_V2_DWO_SECTIONS];
       const gdb_byte *offsets;

(There's more than one change which makes that same kind of adjustment
where the backslash continuation character are lined up in the same
column.)

Here's one where I prefer the original formatting:

@@ -592,7 +601,9 @@
   gdb_bfd_ref_ptr dbfd;
 
   /* Section info for this file.  */
-  struct dwp_sections sections {};
+  struct dwp_sections sections
+  {
+  };
 
   /* Table of CUs in the file.  */
   const struct dwp_hash_table *cus = nullptr;

There's two things going on in this one:

 struct fnfieldlist
@@ -723,8 +736,9 @@
 show_dwarf_max_cache_age (struct ui_file *file, int from_tty,
 			  struct cmd_list_element *c, const char *value)
 {
-  gdb_printf (file, _("The upper bound on the age of cached "
-		      "DWARF compilation units is %s.\n"),
+  gdb_printf (file,
+	      _ ("The upper bound on the age of cached "
+		 "DWARF compilation units is %s.\n"),
 	      value);
 }
 
First, the string has been moved to a line of its own, which I think is
okay.

Second, it puts a space between the _ and the left paren, which I don't
especially like.  Technically, it's the right thing to do, but I'm
accustomed to see it formatted as '_(...'.

There's a bunch of changes like this one - I'm okay with it:

@@ -754,9 +767,9 @@
 static sect_offset read_abbrev_offset (dwarf2_per_objfile *per_objfile,
 				       dwarf2_section_info *, sect_offset);
 
-static const char *read_indirect_string (dwarf2_per_objfile *per_objfile,
bfd *,
-					 const gdb_byte *, const unit_head
*,
-					 unsigned int *);
+static const char *read_indirect_string (dwarf2_per_objfile *per_objfile,
+					 bfd *, const gdb_byte *,
+					 const unit_head *, unsigned int
*); 
 static unrelocated_addr read_addr_index_from_leb128 (struct dwarf2_cu *,
 						     const gdb_byte *,

For this one, I slightly prefer the clang-format version:

@@ -1804,7 +1803,7 @@
 
       /* We may have already read in this line header (TU line header
sharing). If we have we're done.  */
-      stmt_list_hash_key = {cu->dwo_unit, line_offset};
+      stmt_list_hash_key = { cu->dwo_unit, line_offset };
 
       if (auto it = per_bfd->quick_file_names_table.find
(*stmt_list_hash_key); it != per_bfd->quick_file_names_table.end ())

I definitely prefer the clang-format version for this one:

@@ -1867,7 +1866,7 @@
 dw2_get_file_names (dwarf2_per_cu *this_cu, dwarf2_per_objfile
*per_objfile) {
   /* This should never be called for TUs.  */
-  gdb_assert (! this_cu->is_debug_types);
+  gdb_assert (!this_cu->is_debug_types);
 
   if (this_cu->files_read)
     return this_cu->file_names;

I like the original better for this one:

@@ -1944,8 +1944,8 @@
 }
 
 void
-dwarf2_base_index_functions::forget_cached_source_info
-     (struct objfile *objfile)
+dwarf2_base_index_functions::forget_cached_source_info (
+  struct objfile *objfile)
 {
   dwarf2_per_objfile *per_objfile = get_dwarf2_per_objfile (objfile);
 
I think the clang-format version makes more sense here too:

@@ -2032,10 +2029,9 @@
    based on FILE_MATCHER.  */
 
 static void
-dw_search_file_matcher
-  (dwarf2_per_objfile *per_objfile,
-   auto_bool_vector &cus_to_skip,
-   search_symtabs_file_matcher file_matcher)
+dw_search_file_matcher (dwarf2_per_objfile *per_objfile,
+			auto_bool_vector &cus_to_skip,
+			search_symtabs_file_matcher file_matcher)
 {
   if (file_matcher == NULL)
     return;

For this one, I find the original to be more readable:

@@ -2153,12 +2147,9 @@
 }
 
 struct compunit_symtab *
-dwarf2_base_index_functions::find_pc_sect_compunit_symtab
-     (struct objfile *objfile,
-      bound_minimal_symbol msymbol,
-      CORE_ADDR pc,
-      struct obj_section *section,
-      int warn_if_readin)
+dwarf2_base_index_functions::find_pc_sect_compunit_symtab (
+  struct objfile *objfile, bound_minimal_symbol msymbol, CORE_ADDR pc,
+  struct obj_section *section, int warn_if_readin)
 {
   struct compunit_symtab *result;
 
The GCC C++ conventions specify that spaces should be placed outside
of the angle brackets - therefore the original complies with that
convention and the clang-format version does not.

@@ -2291,7 +2280,7 @@
 /* Get the content of the .gdb_index section of OBJ.  SECTION_OWNER should
point to either a dwarf2_per_bfd or dwz_file object.  */
 
-template <typename T>
+template<typename T>
 static gdb::array_view<const gdb_byte>
 get_gdb_index_contents_from_section (objfile *obj, T *section_owner)
 {

I like the original formatting better for this lambda:

@@ -2743,12 +2729,11 @@
   int next_attr_idx = 0;
 
   /* Push an element into ATTRIBUTES.  */
-  auto push_back = [&] (struct attribute *attr)
-    {
-      gdb_assert (next_attr_idx < ARRAY_SIZE (attributes));
-      if (attr != nullptr)
-	attributes[next_attr_idx++] = attr;
-    };
+  auto push_back = [&] (struct attribute *attr) {
+    gdb_assert (next_attr_idx < ARRAY_SIZE (attributes));
+    if (attr != nullptr)
+      attributes[next_attr_idx++] = attr;
+  };
 
   if (stub_comp_unit_die != NULL)
     {

For this type of thing, the left paren for the function call should
have been moved to new line too, right?

@@ -3206,11 +3190,9 @@
 
   m_info_ptr = section->buffer + to_underlying (this_cu.sect_off);
   const gdb_byte *begin_info_ptr = m_info_ptr;
-  m_info_ptr = read_and_check_unit_head (&m_new_cu->header, section,
-					 abbrev_section, m_info_ptr,
-					 (this_cu.is_debug_types
-					  ? ruh_kind::TYPE
-					  : ruh_kind::COMPILE));
+  m_info_ptr = read_and_check_unit_head (
+    &m_new_cu->header, section, abbrev_section, m_info_ptr,
+    (this_cu.is_debug_types ? ruh_kind::TYPE : ruh_kind::COMPILE));
 
   m_new_cu->str_offsets_base = parent_cu.str_offsets_base;
   m_new_cu->addr_base = parent_cu.addr_base;

I like the original formatting better:

@@ -3957,8 +3934,9 @@
       /* We only handle DW_FORM_ref4 here.  */
       const gdb_byte *sibling_data = info_ptr + abbrev->sibling_offset;
       unsigned int offset = read_4_bytes (m_abfd, sibling_data);
-      const gdb_byte *sibling_ptr
-	= m_buffer + to_underlying (m_cu->header.sect_off) + offset;
+      const gdb_byte *sibling_ptr = m_buffer
+				    + to_underlying (m_cu->header.sect_off)
+				    + offset;
       if (sibling_ptr >= info_ptr && sibling_ptr < m_buffer_end)
 	return sibling_ptr;
       /* Fall through to the slow way.  */

While I'm certain about the indentation being used in the original, I
do like placing the left paren on the same line as the parameters:

@@ -4713,16 +4687,14 @@
    included by PER_CU.  */
 
 static void
-recursively_compute_inclusions
-     (std::vector<compunit_symtab *> *result,
-      gdb::unordered_set<dwarf2_per_cu *> &all_children,
-      gdb::unordered_set<compunit_symtab *> &all_type_symtabs,
-      dwarf2_per_cu *per_cu,
-      dwarf2_per_objfile *per_objfile,
-      struct compunit_symtab *immediate_parent)
+recursively_compute_inclusions (
+  std::vector<compunit_symtab *> *result,
+  gdb::unordered_set<dwarf2_per_cu *> &all_children,
+  gdb::unordered_set<compunit_symtab *> &all_type_symtabs,
+  dwarf2_per_cu *per_cu, dwarf2_per_objfile *per_objfile,
+  struct compunit_symtab *immediate_parent)
 {
-  if (bool inserted = all_children.emplace (per_cu).second;
-      !inserted)
+  if (bool inserted = all_children.emplace (per_cu).second; !inserted)
     {
       /* This inclusion and its children have been processed.  */
       return;

Here's a case where clang-format chose to use more lines than necessary:

@@ -5052,7 +5023,8 @@
 {
 public:
   process_die_scope (die_info *die, dwarf2_cu *cu)
-    : m_die (die), m_cu (cu)
+    : m_die (die),
+      m_cu (cu)
   {
     /* We should only be processing DIEs not already in process.  */
     gdb_assert (!m_die->in_process);

Versus something like this:

@@ -5926,14 +5893,9 @@
 	process_die (child_die, cu);
       }
 
-  add_using_directive (using_directives (cu),
-		       import_prefix,
-		       canonical_name,
-		       import_alias,
-		       imported_declaration,
-		       excludes,
-		       read_decl_line (die, cu),
-		       &objfile->objfile_obstack);
+  add_using_directive (using_directives (cu), import_prefix,
canonical_name,
+		       import_alias, imported_declaration, excludes,
+		       read_decl_line (die, cu),
&objfile->objfile_obstack); }
 
 file_and_directory &

I like the clang-format version better on this one:

@@ -10562,8 +10496,9 @@
   for (int i = 0; i < nfields; ++i)
     {
       struct nextfield &field
-	= ((i < fip->baseclasses.size ()) ? fip->baseclasses[i]
-	   : fip->fields[i - fip->baseclasses.size ()]);
+	= ((i < fip->baseclasses.size ())
+	     ? fip->baseclasses[i]
+	     : fip->fields[i - fip->baseclasses.size ()]);
 
       type->field (i) = field.field;
     }

I think the way that clang-format formats the cases for a switch is
correct for the GNU coding style:

@@ -14950,46 +14835,45 @@
 {
   switch (attr->form)
     {
-      case DW_FORM_addrx:
-      case DW_FORM_GNU_addr_index:
-	attr->set_address (read_addr_index (m_cu,
-					    attr->as_unsigned_reprocess
()));
-	break;
-      case DW_FORM_loclistx:
-	{
-	  sect_offset loclists_sect_off
-	    = read_loclist_index (m_cu, attr->as_unsigned_reprocess ());
+    case DW_FORM_addrx:
+    case DW_FORM_GNU_addr_index:
+      attr->set_address (read_addr_index (m_cu,
+					  attr->as_unsigned_reprocess ()));
+      break;
+    case DW_FORM_loclistx:
+      {
+	sect_offset loclists_sect_off
+	  = read_loclist_index (m_cu, attr->as_unsigned_reprocess ());
...


I think the original is more readable here:

@@ -16376,10 +16254,8 @@
 	case DW_TAG_union_type:
 	case DW_TAG_set_type:
 	case DW_TAG_enumeration_type:
-	  if (cu->lang () == language_c
-	      || cu->lang () == language_cplus
-	      || cu->lang () == language_objc
-	      || cu->lang () == language_opencl
+	  if (cu->lang () == language_c || cu->lang () == language_cplus
+	      || cu->lang () == language_objc || cu->lang () ==
language_opencl || cu->lang () == language_minimal)
 	    {
 	      /* These languages have a tag namespace.  Note that

Bottom line - while I don't necessarily like the way that clang-format
does everything, it did well enough - I could live with it.  This is
something that could be endlessly bike-shedded.  If you see some easy
fixes for some of the things I noticed, by all means make them, otherwise,
I think this version will work and should be checked in.

Kevin