Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(853)

Issue 251700043: Issue 4541: Introduce a Grob_interface class (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 7 months ago by Dan Eble
Modified:
7 years, 11 months ago
Reviewers:
dak, dan, c_sorensen
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Introduce a Grob_interface class. T::has_interface (grob) has become has_interface<T> (grob). (Like unsmob<T>, it has the advantage of not being inherited by derived classes of T.) DECLARE_GROB_INTERFACE() is no longer required. Patch set 1 adds Grob_interface and makes a few preparatory changes. Patch set 2 is mostly automated; see the replies section for the script used to make them.

Patch Set 1 #

Total comments: 1

Patch Set 2 : clean up (mostly automated) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+159 lines, -216 lines) Patch
M lily/accidental-placement.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M lily/align-interface.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/axis-group-interface.cc View 1 6 chunks +6 lines, -6 lines 0 comments Download
M lily/balloon.cc View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/beam.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/beam-collision-engraver.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M lily/beam-quanting.cc View 1 3 chunks +3 lines, -3 lines 0 comments Download
M lily/break-alignment-interface.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/clef-modifier.cc View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/cluster.cc View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/cluster-engraver.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/collision-engraver.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/dot-column.cc View 1 4 chunks +5 lines, -5 lines 0 comments Download
M lily/dynamic-align-engraver.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M lily/dynamic-engraver.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M lily/enclosing-bracket.cc View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/figured-bass-continuation.cc View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/flag.cc View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/grob.cc View 1 4 chunks +6 lines, -6 lines 0 comments Download
M lily/hairpin.cc View 1 3 chunks +3 lines, -3 lines 0 comments Download
M lily/include/accidental-interface.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/accidental-placement.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/align-interface.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/arpeggio.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/axis-group-interface.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/bar-line.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/beam.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/break-align-interface.hh View 1 2 chunks +0 lines, -3 lines 0 comments Download
M lily/include/breathing-sign.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/chord-name.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/clef.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/cluster.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/custos.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/dot-column.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/dots.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/fingering-column.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/font-interface.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/gregorian-ligature.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/grid-line-interface.hh View 1 1 chunk +0 lines, -2 lines 0 comments Download
M lily/include/grob.hh View 1 2 chunks +6 lines, -1 line 0 comments Download
M lily/include/grob-interface.hh View 1 2 chunks +37 lines, -15 lines 0 comments Download
M lily/include/hairpin.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/hara-kiri-group-spanner.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/horizontal-bracket.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/item.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/kievan-ligature.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/line-interface.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/lyric-extender.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/lyric-hyphen.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/measure-grouping-spanner.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/melody-spanner.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/mensural-ligature.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/multi-measure-rest.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/note-collision.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/note-column.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/note-head.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/note-spacing.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/paper-column.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/percent-repeat-item.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/pure-from-neighbor-interface.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/rest.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/rest-collision.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/rhythmic-head.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/script-column.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/script-interface.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/self-alignment-interface.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/semi-tie.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/semi-tie-column.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/separation-item.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/side-position-interface.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/slur.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/spaceable-grob.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/spacing-interface.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/spacing-spanner.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/spanner.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/staff-grouper-interface.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/staff-spacing.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/staff-symbol.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/staff-symbol-referencer.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/stem.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/stem-tremolo.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/system.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/system-start-delimiter.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/text-interface.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/tie.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/tie-column.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/tuplet-bracket.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/tuplet-number.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/vaticana-ligature.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/include/volta-bracket.hh View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/instrument-name-engraver.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/key-signature-interface.cc View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/ledger-line-spanner.cc View 1 2 chunks +0 lines, -2 lines 0 comments Download
M lily/line-spanner.cc View 1 2 chunks +2 lines, -3 lines 0 comments Download
M lily/lyric-engraver.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/note-collision.cc View 1 2 chunks +3 lines, -3 lines 0 comments Download
M lily/note-column.cc View 1 5 chunks +5 lines, -5 lines 0 comments Download
M lily/note-spacing.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/ottava-bracket.cc View 1 3 chunks +2 lines, -3 lines 0 comments Download
M lily/page-layout-problem.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/paper-column.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M lily/paper-column-engraver.cc View 1 1 chunk +3 lines, -3 lines 0 comments Download
M lily/piano-pedal-bracket.cc View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/pure-from-neighbor-engraver.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/rest-collision.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M lily/rest-collision-engraver.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M lily/script-column.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M lily/script-interface.cc View 1 1 chunk +0 lines, -1 line 0 comments Download
M lily/script-row-engraver.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/self-alignment-interface.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M lily/semi-tie.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/separating-line-group-engraver.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/separation-item.cc View 1 2 chunks +3 lines, -3 lines 0 comments Download
M lily/side-position-interface.cc View 1 4 chunks +4 lines, -4 lines 0 comments Download
M lily/slur.cc View 1 3 chunks +3 lines, -3 lines 0 comments Download
M lily/slur-configuration.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/slur-scoring.cc View 1 5 chunks +6 lines, -6 lines 0 comments Download
M lily/spacing-determine-loose-columns.cc View 1 3 chunks +3 lines, -3 lines 0 comments Download
M lily/spacing-interface.cc View 1 2 chunks +3 lines, -3 lines 0 comments Download
M lily/spacing-loose-columns.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/spacing-spanner.cc View 1 2 chunks +2 lines, -2 lines 0 comments Download
M lily/staff-spacing.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/staff-symbol-referencer.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/stem.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M lily/stem-tremolo.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/system.cc View 1 6 chunks +6 lines, -6 lines 0 comments Download
M lily/tie.cc View 1 3 chunks +5 lines, -5 lines 0 comments Download
M lily/tie-column.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/tie-formatting-problem.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/tuplet-bracket.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/tuplet-number.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/vertical-align-engraver.cc View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 9
Dan Eble
clean up (mostly automated)
8 years, 7 months ago (2015-08-08 00:28:32 UTC) #1
Dan Eble
On 2015/08/08 00:28:32, Dan Eble wrote: > clean up (mostly automated) Specifically... * Use has_interface<Interface> ...
8 years, 7 months ago (2015-08-08 00:30:16 UTC) #2
Dan Eble
8 years, 7 months ago (2015-08-08 00:48:41 UTC) #3
Dan Eble
https://codereview.appspot.com/251700043/diff/1/lily/include/grob-interface.hh File lily/include/grob-interface.hh (right): https://codereview.appspot.com/251700043/diff/1/lily/include/grob-interface.hh#newcode65 lily/include/grob-interface.hh:65: static SCM add_interface (); Whoops! I used this at ...
8 years, 7 months ago (2015-08-08 14:15:08 UTC) #4
Dan Eble
On 2015/08/08 14:15:08, Dan Eble wrote: > lily/include/grob-interface.hh:65: static SCM add_interface (); > Whoops! I ...
8 years, 7 months ago (2015-08-10 12:32:49 UTC) #5
dak
Ok, I need a refresher here. What was the ultimate aim of this patch/issue? As ...
7 years, 11 months ago (2016-04-30 09:37:05 UTC) #6
c_sorensen
On 4/30/16 3:37 AM, "lilypond-devel on behalf of dak@gnu.org" <lilypond-devel-bounces+c_sorensen=byu.edu@gnu.org on behalf of dak@gnu.org> wrote: ...
7 years, 11 months ago (2016-04-30 14:03:54 UTC) #7
dan_faithful.be
> On Apr 30, 2016, at 05:37 , dak@gnu.org wrote: > > Ok, I need ...
7 years, 11 months ago (2016-04-30 14:57:09 UTC) #8
dak
7 years, 11 months ago (2016-04-30 16:32:38 UTC) #9
Dan Eble <dan@faithful.be> writes:

>> On Apr 30, 2016, at 05:37 , dak@gnu.org wrote:
>> 
>> Ok, I need a refresher here.  What was the ultimate aim of this
>> patch/issue?  As it stands, actual interface classes like
>> Axis_group_interface don't have _any_ connection to a type any more.
>> Instead, there is a Grob_interface template class depending on them with
>> an arbitrarily named single instance not used for anything but
>> initialization.
>
> I’m sorry I can’t give much of a refresher.  I’ve been busy on
> unrelated projects for months.
>
> This comment:
>
>> T::has_interface (grob) has become has_interface<T> (grob).  (Like
>> unsmob<T>, it has the advantage of not being inherited by derived
>> classes of T.)
>
> ... reminds me--if I recall correctly, which might not be the
> case--that a trigger for this patch was the belief that if one called
> Derived::has_interface(grob) and had neglected to implement
> has_interface() for the derived class, the question would be answered
> by the nearest base implementation, and that would be the wrong answer
> to the question.

No, I think that was for unsmob when used as a type checker.  Or
something like that.  I don't think that we have any inheritance with
regard to interfaces, and it would have been quite pointless anyway
since they only had static member functions so that there was nothing
worthwhile to inherit (apart from possibly protected static member
functions but I don't think there ever were any).

I think this was more in the course of having "this" be the respective
Grob* of the grob accessed by the interface.  But getting there would
really mess with the type system I think.

> If it hampers your greater clean-up, throw it away.

At the current point of time, I am changing the acknowledger macros from
something awkward to something awkward.  I was just brainstorming about
how this could ultimately be streamlined into something C++-like as
well, but I think it would be a bad idea to try refactoring this as well
right now.  It's more the temptation of doing something here that is
hampering the current work by having me brood over non-imminent stuff.

But the smart idea is to just stick with it here.  My problem is that
some acknowledgers are inherited right now, and this inheritance works
with a rather ugly combination of DECLARE_ACKNOWLEDGER inside of a base
class with ADD_ACKNOWLEDGER outside of a derived class, and each of
those macros only talks about its own class.  So the required
initialization function defined in ADD_ACKNOWLEDGER cannot be a static
member function since DECLARE_ACKNOWLEDGER (the only thing called
_inside_ of a class definition) would declare it in the base class.

I'll likely just do something boringly stupid, like redefine the
acknowledgers in the derived class to call the base class acknowledgers
and thus get rid of the whole "&T::grob_acknowledger is not of type void
(T::*)(Grob *) but of type void (B::*)(Grob *)" issue for this
iteration.  If I could have done the initialization with a static member
function, I could have used a class-local template override to do the
right thing (because then I would have been in the namespace of the
derived class), but the initialization happens at global namespace.

After the local acknowledger redefinitions, that template override
scheme would work but stop being necessary.  Well...

Redefining the local acknowledgers as deflectors to the base class
acknowledgers seems rather like defeating the whole idea of inheritance
in the first place.  But the half-baked macro system does not really
lend itself well in the current incarnation to anything else, and
replacing its incarnation with something else altogether seems
exaggerated.

So I am deadlocking between several different ugly approaches.  I'll
probably pick one of the uglier ones as long as it's simple.

-- 
David Kastrup
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b