From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.efficios.com (mail.efficios.com [167.114.26.124]) by sourceware.org (Postfix) with ESMTPS id 5D7383840C2D for ; Sat, 16 May 2020 16:35:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5D7383840C2D Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id E97B62B94C8; Sat, 16 May 2020 12:35:51 -0400 (EDT) Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id nj1UAitl9OKW; Sat, 16 May 2020 12:35:51 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mail.efficios.com (Postfix) with ESMTP id 7EBEB2B95ED; Sat, 16 May 2020 12:35:51 -0400 (EDT) DKIM-Filter: OpenDKIM Filter v2.10.3 mail.efficios.com 7EBEB2B95ED X-Virus-Scanned: amavisd-new at efficios.com Received: from mail.efficios.com ([127.0.0.1]) by localhost (mail03.efficios.com [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id QQ3hY2njauFE; Sat, 16 May 2020 12:35:51 -0400 (EDT) Received: from [10.0.0.193] (unknown [192.222.164.54]) by mail.efficios.com (Postfix) with ESMTPSA id 5BF212B94C7; Sat, 16 May 2020 12:35:51 -0400 (EDT) Subject: Re: [PATCH 2/2] gdb: remove TYPE_NAME macro To: Tom Tromey , Simon Marchi via Gdb-patches References: <20200514181812.25795-1-simon.marchi@efficios.com> <20200514181812.25795-2-simon.marchi@efficios.com> <877dxb7tuw.fsf@tromey.com> From: Simon Marchi Message-ID: <62d1efcd-01bf-a23b-7608-4ba1f1bd8ebe@efficios.com> Date: Sat, 16 May 2020 12:35:50 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <877dxb7tuw.fsf@tromey.com> Content-Type: text/plain; charset=utf-8 Content-Language: tl Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-7.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 16 May 2020 16:35:54 -0000 On 2020-05-16 12:00 p.m., Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi via Gdb-patches writes: > > Simon> Remove `TYPE_NAME`, changing all the call sites to use `type::name` > Simon> directly. This is quite a big diff, but this was mostly done using sed > Simon> and coccinelle. A few call sites were done by hand. > > Looks good. I didn't realize coccinelle worked on C++. Thanks, I'll push them in a moment. No, coccinelle doesn't work on C++ unfortunately. When there's a C++ construct in a function, it just skips it it seems. But our codebase still looks largely like C in a lot of places that it can still do a significant number of changes. But what this means is that it will become less and less effective for us as time goes by :(. Also, coccinelle doesn't really allow to specify the output code style, so it just never puts a space before the parenthesis. I worked around that by making it use `a_very_unique_string` as the function name in its output, so I could easily replace it with the desired value with sed: - thetype->a_very_unique_string(...) + thetype->set_name (...) The above can also leave out a space in other function calls in the modified code, for example turning TYPE_NAME (thetype) = a_function (); into thetype->set_name (a_function()); So the space after `a_function` needs to be added back by hand, or some sed to turn `a_function(` into `a_function (`. While at it, here's the complete method I used, maybe it will help you or somebody else when making similar changes: - Add getter / setters, change the TYPE_NAME macro to use the getter. The compiler will point out any spots that are using the macro to set the field. - A first sed pass to change the obvious spots to use the setter (not essential, but it's quick): sed -ri 's|TYPE_NAME \(([a-z0-9_]+)\) = ([a-z0-9_"]+)|\1->set_name (\2)|g' *.c */*.c - Run this coccinelle rule to change some less obvious spots to use the setter --- @@ expression type; expression value; @@ - TYPE_NAME(type) = value + type->a_very_unique_string(value) --- $ spatch --sp-file type-set-name.cocci --in-place Followed by this sed to replace `a_very_unique_string`: $ sed -i "s|a_very_unique_string|set_name |g" - Fix the remaining spots to use the setter by hand. That should be enough for the first patch. For the second patch: - Remove the TYPE_NAME macro. - A first sed pass: sed -ri 's|TYPE_NAME \(([a-z0-9_]+)\)|\1->name ()|g' - Run this coccinelle rule followed by sed, as mentioned above: @@ expression type; @@ - TYPE_NAME(type) + type->a_very_unique_string() Simon