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

Issue 581270043: Issue 5607: typedef System::rank_type (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 5 months ago by Dan Eble
Modified:
4 years, 4 months ago
Reviewers:
lemzwerg
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

https://sourceforge.net/p/testlilyissues/issues/5607/ 1. break-substitution: typedef System_range to clarify code Call an interval of System rank a "System_range" rather than a "Slice." Though both of these are currently Interval_t<int>, there is code that stores System ranks in shorts. At some point, it might make sense to change System_range to Interval_t<short> for consistency. 2. typedef System::rank_type Code in break-substitution.cc previously assumed that there would not be more than 32k systems. This centralizes that assumption in System and checks for programming errors.

Patch Set 1 : typedef System_range to clarify code #

Total comments: 1

Patch Set 2 : typedef System::rank_type #

Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -28 lines) Patch
M lily/break-substitution.cc View 1 7 chunks +30 lines, -19 lines 0 comments Download
M lily/include/system.hh View 1 3 chunks +7 lines, -2 lines 0 comments Download
M lily/system.cc View 1 3 chunks +9 lines, -7 lines 0 comments Download

Messages

Total messages: 4
Dan Eble
typedef System::rank_type
4 years, 5 months ago (2019-11-19 17:37:48 UTC) #1
Dan Eble
4 years, 5 months ago (2019-11-19 17:41:59 UTC) #2
Dan Eble
https://codereview.appspot.com/581270043/diff/547260043/lily/break-substitution.cc File lily/break-substitution.cc (right): https://codereview.appspot.com/581270043/diff/547260043/lily/break-substitution.cc#newcode211 lily/break-substitution.cc:211: vector<Spanner *> const &bs = sp->broken_intos_; I hereby dedicate ...
4 years, 5 months ago (2019-11-19 17:50:00 UTC) #3
lemzwerg
4 years, 5 months ago (2019-11-19 21:49:36 UTC) #4
:-)
Sign in to reply to this message.

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