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

Issue 363740043: Prob::equal_p: discard "origin" property

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 5 months ago by dak
Modified:
6 years, 5 months ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Prob::equal_p: discard "origin" property Previously elements of class Input was considered equal when compared as part of Music property lists but this did not take into account that some music might store its origin (if any) at different locations in its property lists. So we just discard any "origin" property completely, regardless of position in the property list and its actual value type. Also contains commit: Don't set origin on copied music explicitly There would be some mild incentive if the origin were accessed an inordinary amount of times (and thus leaving it unset would maximize access times to it) but there is no evidence for that. One reason might be to ensure greater structural similarity between copies of music, but Prob::equal_p has been changed to be robust against differences here.

Patch Set 1 #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -13 lines) Patch
M lily/music.cc View 1 chunk +0 lines, -3 lines 0 comments Download
M lily/prob.cc View 1 chunk +21 lines, -10 lines 2 comments Download

Messages

Total messages: 3
thomasmorley651
Can't review C++. But from a little testing: http://lilypond.1069038.n5.nabble.com/Iterating-music-for-voiceOne-Checking-with-equal-td215135.html#a215158 it works as wished. A question ...
6 years, 5 months ago (2018-07-27 09:32:56 UTC) #1
dak
https://codereview.appspot.com/363740043/diff/1/lily/prob.cc File lily/prob.cc (right): https://codereview.appspot.com/363740043/diff/1/lily/prob.cc#newcode36 lily/prob.cc:36: equality; e.g., that two probs are equal iff they ...
6 years, 5 months ago (2018-07-27 09:37:25 UTC) #2
kolejorzdhl088
6 years, 5 months ago (2018-07-27 16:31:23 UTC) #3

          
Sign in to reply to this message.

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