From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id Rwy/MkDf6mgScyoAWB0awg (envelope-from ) for ; Sat, 11 Oct 2025 18:50:40 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=IeD8cHs8; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id BA0A81E0B6; Sat, 11 Oct 2025 18:50:40 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 033B81E04C for ; Sat, 11 Oct 2025 18:50:39 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7BD5E3857739 for ; Sat, 11 Oct 2025 22:50:38 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7BD5E3857739 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=IeD8cHs8 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 01BA1385780D for ; Sat, 11 Oct 2025 22:49:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 01BA1385780D Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 01BA1385780D Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1760222996; cv=none; b=gooOu2XOEHT0PDdcAoXTYYBGW9zjUkJcC/Cm/9Xu02zZu2FM90qkkSmekDSJZlo0NKECeHM9CekvmiIDvbI/8nuUs9Q5OhmiJxbM99u2eQrYrXhmPsrFd1H5ZL/b9OMZCbcu9VMBzXm1iUljmi4ZlpxzdORwBdWq/qQuYAzrTJI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1760222996; c=relaxed/simple; bh=GbzGrzzGo+r7D0TvV1VTvCIoIdkPxfsMKiq0qvuqyrc=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=gfGNCpohTjqH+ogFx0w4q+Dyr8vDxUNcBJnWbco+QVI5LuHUNqoXSUblJyspbEAlcgTtW6CQcxID9oIIZ6sUhhY0cY3qbVPkXYpB92X8FmpmSTbEAfMLsd62On/OQsWMT0aYQkkZc2BVpuMhUYTCkJ/o4IfhxFVtQtVxKkjnPAY= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 01BA1385780D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1760222995; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WoSZqCLPsvGEmxVWqriapOTEQfKcxBMCDooS+pBSm0Y=; b=IeD8cHs8SrOKi2ydqhFZhICDDcDyXygUebTynOadQUmaAIBVhh4umnOYpE+7R1jrnkTXcr vXjwSeB+JW6+CPystOwhVhcome2a8XheMhCEs6Y6ZDRDgrpQabghyJEV+C4Bm2WxYohy2F rSCRTBk+QST/d0ErUX34UP1NQUetg6I= Received: from mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com (ec2-35-165-154-97.us-west-2.compute.amazonaws.com [35.165.154.97]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-269-jIvM6n67N2aiMErpboDDIw-1; Sat, 11 Oct 2025 18:49:54 -0400 X-MC-Unique: jIvM6n67N2aiMErpboDDIw-1 X-Mimecast-MFC-AGG-ID: jIvM6n67N2aiMErpboDDIw_1760222993 Received: from mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.12]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 37ACC1800578; Sat, 11 Oct 2025 22:49:53 +0000 (UTC) Received: from f41-zbm-amd (unknown [10.22.80.23]) by mx-prod-int-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 5294719560AB; Sat, 11 Oct 2025 22:49:52 +0000 (UTC) Date: Sat, 11 Oct 2025 15:49:49 -0700 From: Kevin Buettner To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v4] Add .clang-format Message-ID: <20251011154734.55f0208d@f41-zbm-amd> In-Reply-To: <20251006183901.1239002-1-tom@tromey.com> References: <20251006183901.1239002-1-tom@tromey.com> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.12 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: bMDvZ6qbWem3p08-VgqtjpxDsZX3Et8nYjVp9wc1Nfo_1760222993 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org On Mon, 6 Oct 2025 12:38:59 -0600 Tom Tromey 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 &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 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 §ion, 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, §ion, - sect_offset (sect_off), 0, - is_dwz)); - }; + return *units.emplace_back (new dwarf2_per_cu (dummy_per_bfd, §ion, + 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 §ion, 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 { §ion, 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 { §ion, 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 +template static gdb::array_view 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 *result, - gdb::unordered_set &all_children, - gdb::unordered_set &all_type_symtabs, - dwarf2_per_cu *per_cu, - dwarf2_per_objfile *per_objfile, - struct compunit_symtab *immediate_parent) +recursively_compute_inclusions ( + std::vector *result, + gdb::unordered_set &all_children, + gdb::unordered_set &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