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

Issue 157750043: Issue 4156: Define Smob<> constructors

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

Description

Issue 4156: Define Smob<> constructors The purpose of this patch is to stop the previous implicit copying of Smob<> data members in derived copy constructors. To that purpose, Smob<> has received a private copy constructor (which gcc unfortunately only considers worth a warning rather than an error when implicitly used in a default copy constructor) and an inline default constructor. Several classes contained virtual copy constructors without being actually copyable have had their copy constructor removed. Several copy constructors just containing a failing assertion were instead removed and left (privately) declared but not defined. This standard C++ practice leads to link time rather than run time errors in case an instance of such a class is copied. Other smob-defining base classes (Simple_smob and Smob{1,2,3}) do not actually have non-static data members that could be initialized. Prohibiting the copying of Simple_smob as a base class seems rather pointless as all of the data members of a derived class are under the control of the derived class. However, Smob{1,2,3} "misuse" the this pointer to contain an SCM value. Creating a copy would change the associated address, a very undesirable operation. So Smob{1,2,3} have its constructors private and/or disabled.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -41 lines) Patch
M lily/book.cc View 1 chunk +1 line, -0 lines 0 comments Download
M lily/context.cc View 1 chunk +0 lines, -5 lines 0 comments Download
M lily/context-def.cc View 1 chunk +1 line, -0 lines 0 comments Download
M lily/font-metric.cc View 1 chunk +1 line, -0 lines 0 comments Download
M lily/grob.cc View 1 chunk +1 line, -0 lines 0 comments Download
M lily/include/context.hh View 1 chunk +1 line, -1 line 0 comments Download
M lily/include/dispatcher.hh View 1 chunk +1 line, -0 lines 0 comments Download
M lily/include/engraver-group.hh View 1 chunk +1 line, -2 lines 0 comments Download
M lily/include/music-iterator.hh View 1 chunk +2 lines, -1 line 0 comments Download
M lily/include/paper-score.hh View 1 chunk +1 line, -1 line 0 comments Download
M lily/include/performer-group.hh View 1 chunk +1 line, -1 line 0 comments Download
M lily/include/score-performer.hh View 1 chunk +1 line, -1 line 0 comments Download
M lily/include/small-smobs.hh View 3 chunks +6 lines, -6 lines 0 comments Download
M lily/include/smobs.hh View 2 chunks +3 lines, -2 lines 0 comments Download
M lily/include/sources.hh View 1 chunk +1 line, -1 line 0 comments Download
M lily/include/translator-group.hh View 1 chunk +1 line, -1 line 0 comments Download
M lily/lily-parser.cc View 1 chunk +1 line, -0 lines 0 comments Download
M lily/music-iterator.cc View 1 chunk +0 lines, -5 lines 0 comments Download
M lily/output-def.cc View 1 chunk +1 line, -0 lines 0 comments Download
M lily/page-marker.cc View 1 chunk +1 line, -0 lines 0 comments Download
M lily/paper-score.cc View 1 chunk +0 lines, -6 lines 0 comments Download
M lily/prob.cc View 2 chunks +2 lines, -1 line 0 comments Download
M lily/scale.cc View 1 chunk +1 line, -0 lines 0 comments Download
M lily/scm-hash.cc View 1 chunk +1 line, -0 lines 0 comments Download
M lily/score.cc View 1 chunk +1 line, -0 lines 0 comments Download
M lily/sources.cc View 1 chunk +0 lines, -5 lines 0 comments Download
M lily/translator.cc View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 1
Dan Eble
9 years, 5 months ago (2014-10-13 13:51:01 UTC) #1
LGTM
Sign in to reply to this message.

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