[Lilypond-auto] [LilyIssues-auto] [testlilyissues:issues] Re: #5882 Refa

The current state is that I asked for opinions on the developer list. Everyone except Han-Wen is for it. Carl because he trusts my judgment, Dan sees no problem and side benefits in stopping a macro to behave like a member function: something unidiomatic for C++ that likely tripped up everyone who tried understanding the implementation. Nobody else chimed up, so while the voices that made themselves heard are pretty much on one side (for technical and not so technical reasons, both of which I am grateful for), not enough people got involved to make this convincingly overwhelming.

Han-Wen continues rejecting this patch on the basis that he does not think what I am planning to do with would be worth an extensive change (of the scope and kind that numerous developers did several times for various more or less successful reasons in the past years), never mind that the change in itself makes it easier to understand what happens. This objection goes to the degree that he single-handedly changes the status of other people's patches without anybody else agreeing with his rationale.

I don't think that it can or should be your job to make a decision in this case. I am currently in the process of submitting several other more confined changes of similar kind that make LilyPond code more readable and with more immediate and obvious benefits, rendering the code style Han-Wen defends in spite of its considerable limitations and lack of use elsewhere even more of an outlier than it already is.

So just leave it on review. It will require me to frequently rebase/recreate this patch while it stays relevant which is the reason I wanted to have this done early. I'll come back to it.

[issues:#5882] Refactor get/set_property to take the item as first argument

Status: Started
Created: Tue Apr 07, 2020 02:14 PM UTC by David Kastrup
Last Updated: Sun Apr 19, 2020 06:52 AM UTC
Owner: David Kastrup

Refactor get/set_property to take the item as first argument

This makes it possible in a future revision to make type-dependent
optimisations using memoisation techniques that cannot be factored
into member functions.

This consists of three commits, separately entered into the review so
that the humongous mechanical change in the middle commit can be
reviewed separately from the manual commits.

Commits are:

Issue 5882/1: Grob::flush_extent_cache: refactor del_property call

Issue 5882/2: Run script for get/set_property refactor

In order to allow for type-dependent speedups,
xxx->get_property ("prop") is converted to get_property (xxx, "prop").

The bulk of the work is done mechanically using the sed script

sed -i -n '1h
           1!H
           ${x
             s/\([a-z_A-Z][a-z_A-Z0-9]*\s*\(->\s*[a-z_A-Z][a-zA-Z_0-9]*\s*\|\.\s*[a-z_A-Z][a-zA-Z_0-9]*\s*\|\[[^]]*\]\s*\|([^()]*)\s*\|<[a-zA-Z_]\+>\s*\)*\)->\s*\(set_property\|get_property\|get_property_data\|get_object\|set_object\|get_pure_property\|get_maybe_pure_property\|del_property\)\(\s\+(\)/\3\4\1, /g
             s/\(set_property\|get_property\|get_property_data\|get_object\|set_object\|get_pure_property\|get_maybe_pure_property\|del_property\)\(\s\+(\)"/\1\2this, "/g
             p}' $(git ls-files '*.cc' '*.ll' '*.yy')

Issue 5882/3: Complete the set/get_property style conversion

This fixes oversights by the automated script and actually changes the
involved macros.  Functionally, there is no difference yet.

http://codereview.appspot.com/573670043

Sent from sourceforge.net because address@hidden is subscribed to https://sourceforge.net/p/testlilyissues/issues/

To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/testlilyissues/admin/issues/options. Or, if this is a mailing list, you can unsubscribe from the mailing list.

From:	Auto mailings of changes to Lily Issues via Testlilyissues-auto
Subject:	[Lilypond-auto] [LilyIssues-auto] [testlilyissues:issues] Re: #5882 Refactor get/set_property to take the item as first argument
Date:	Sun, 19 Apr 2020 08:19:51 -0000