MuseScore violates the C++ One Definition Rule [-Wodr]

• Jun 13, 2021 - 16:19
Reported version
3.6
Type
Development
Frequency
Once
Severity
S4 - Minor
Reproducibility
Always
Status
closed
Regression
No
Workaround
No
Project

G++ now complains about the following:

I mscore/importmxmlpass1.h                                              R34  <142  C1  1240|4D8 u|0073 17:19

struct PageFormat {
QSizeF size;
qreal printableWidth; // _width - left margin - right margin
qreal evenLeftMargin; // values in inch
qreal oddLeftMargin;
qreal evenTopMargin;
qreal evenBottomMargin;
qreal oddTopMargin;
qreal oddBottomMargin;
bool twosided;
};

typedef QMap PartMap;
typedef std::map MusicXmlPartGroupMap;

I libmscore/read206.h                                                   R24  <91   C1  699 |2BB u|0063 17:19

class PageFormat {
QSizeF _size;
qreal _printableWidth; // _width - left margin - right margin
qreal _evenLeftMargin; // values in inch
qreal _oddLeftMargin;
qreal _evenTopMargin;
qreal _evenBottomMargin;
qreal _oddTopMargin;
qreal _oddBottomMargin;
bool _twosided;

public:
PageFormat() {}

  const QSizeF& size() const          { return _size;          }    // size in inch

This is because PageFormat now has two definitions which are incompatible with each other.


Comments

I accidentally copied from the editor I had the 3.2 files open. One of the files got moved to a different path, but it’s still there.

I haven’t looked at master regarding this. You’ll need a recent enough compiler (GCC in *buntu 21.04 seems to complain about this) to even see the messages.

It’s the case in the 3.x branch at least:

importexport/musicxml/importmxmlpass1.h:struct PageFormat {

libmscore/read206.h:class PageFormat {

This may be becoming a problem with LTO (link-time optimisation) because then the different translation units are mashed together. Debian/Ubuntu are considering enabling LTO by default (against my recommendation, and I’ll likely locally disable that for MuseScore), but in https://stackoverflow.com/q/40659435 someone has written that this may also trigger subtle bugs in Visual Studio-compiled output. (The suggested namespacing is probably not the best solution for MuseScore; rather, rename all (or all-but-one) occurrence of this, e.g. rename the first to struct MXMLPageFormat, the second maybe to class Read206PageFormat (maybe it can keep its name, but only if it’s the only one to use the name).

Both, struct pageFormat and class PageFormat do exist in master too, the former in src/importexport/musicxml/internal/musicxml/importmxmlpass1.h (and gets used 6 times in 2 files), similar to before in 3.x, the latter in src/engraving/compat/pageformat.h and gets used 10 times in 4 files)

I'd rename just the MusicXML one. Not sure yet whether to MxmlPageFormat or MusicXMLPageFormat.

They are in different namespaces though, struct mu::engraving::PageFormat and class mu::engraving::compat::PageFormatso should not collide at all, not in master.

Fix version
4.0.0