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

Issue 341320043: Issue 5335: Use covariant return types on virtual Grob::clone() (Closed)

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

Description

https://sourceforge.net/p/testlilyissues/issues/5335/ Use covariant return types on virtual Grob::clone() and remove unnecessary dynamic_casts. The casts in the parser were not Grob-related, but since they turned up during my search, I removed them too.

Patch Set 1 #

Patch Set 2 : fix style #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -34 lines) Patch
M lily/include/item.hh View 1 1 chunk +1 line, -1 line 0 comments Download
M lily/include/paper-column.hh View 1 chunk +1 line, -1 line 0 comments Download
M lily/include/spanner.hh View 1 chunk +1 line, -1 line 0 comments Download
M lily/include/system.hh View 1 chunk +1 line, -1 line 0 comments Download
M lily/item.cc View 1 chunk +0 lines, -6 lines 0 comments Download
M lily/lily-parser.cc View 1 chunk +1 line, -2 lines 0 comments Download
M lily/paper-column.cc View 1 chunk +0 lines, -6 lines 0 comments Download
M lily/parser.yy View 1 chunk +1 line, -1 line 0 comments Download
M lily/spanner.cc View 3 chunks +2 lines, -8 lines 0 comments Download
M lily/system.cc View 2 chunks +1 line, -7 lines 1 comment Download

Messages

Total messages: 3
Dan Eble
fix style
5 years, 10 months ago (2018-06-04 00:41:41 UTC) #1
dak
At any rate: LGTM I think that covariant return types predate C++11, right? https://codereview.appspot.com/341320043/diff/20001/lily/system.cc File ...
5 years, 10 months ago (2018-06-04 07:00:58 UTC) #2
Dan Eble
5 years, 10 months ago (2018-06-04 10:41:33 UTC) #3
On 2018/06/04 07:00:58, dak wrote:
> I think that covariant return types predate C++11, right?

1998: http://www.drdobbs.com/whats-new-in-standard-c/184403580

> Not really in the scope of this patch/issue any more, but ...

I can probably afford to spend some time on that and submit a separate patch.
Sign in to reply to this message.

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