Re: Resolving standoffs

[David Kastrup]

Han-Wen Nienhuys <address@hidden> writes:

> In short, David is asking me to set aside my well-founded skepticism
> on his plans, so get_property can decltype to get at the type of the
> receiver, ie.
>
>   #define get_property(receiver, name) \
>      { something something (decltype)(*receiver_type) }
>
> I am concerned that the logical next step will require including the
> definitions of Music, Grob, Context etc. so the macro can actually do
> something different based on its type. This would make all of LilyPond
> depend on all of those headers, which will significantly slow down
> recompiles when working on the source code.

How about evaluating patches based on themselves rather than on future
expectations?

> If David wants to experiment to make something that compiles against
> current master, he could simply refactor the get_object/set_object()
> calls. They work exactly the same, and because the macro is already
> separate, there is no need to do a global refactoring. That wouldn't
> work to demonstrate improvements in benchmarks, but it would
> demonstrate the overall idea, so we know what we're talking about, and
> we can verify that it at least works correctly.

Obviously, we can verify that stuff works correctly with a more
encompassing patch, as well as judge the performance impacts.

> David has not mentioned two downsides to his refactoring:
>
> 1. it touches code all over the place making git-blame less useful.

Of course I have mentioned that it touches code all over the place.
That is the main reason that it is needed to facilitate parallel
development over a longer amount of time without constant rebasing.

> 2. all other pending changes become invalidated and need to be
> reorganized, including historical changes.

That is the case with every extensive change.  It hasn't kept us from
refactorings like that previously.

> In short, I see this as a poorly specified, ill-conceived plan, and to
> add insult to injury, nobody has tried to evaluate my extensive
> technical commentary on the plan at hand.

Because the plan at hand is not at debate.  At debate is a purely
syntactical change facilitating more work that does not require macros
to simulate member function overloading, a technique very unusual for
C++ and tripping up newcomers to the code.

> I also briefly wish to address Dan's point:
>
> get_property is a macro for performance reasons, but to the caller the
> distinction is invisible, and it would certainly be a member function
> if there no performance considerations.

The string-literal/variable distinction and memoization is achieved by
function overloading and the implementation of ly_scm2symbol .  More
relevant is that the macro can pass source file name and line numbers.

> This pattern is also common in the GUILE source code, for example,
> scm_is_eq looks like a function (lower case), but it is actually a
> macro for performance reasons.

That is a red herring since nobody complains about macro calls looking
like function calls: they always do.  The problem for understanding is
that this macro call looks like a member function call.

> For thinking about the readability of the source code at the caller
> side, it is better that it remains as a method.

But it isn't a method.  It is a macro.

-- 
David Kastrup