Error compiling pdfdc29.inc with current wx-master

[wh11204]

After wxWidgets release 3.1.4 the developers added an overload for wxRound, and now the compiler gives an error at line 1070 of file pdfdc29.inc: claiming it can't decide which overload should be used.

m_pdfDocument->Rect(ScaleLogicalToPdfX(x), ScaleLogicalToPdfY(originalY + wxRound(lineNum*heightLine)), ScaleLogicalToPdfXRel(w), ScaleLogicalToPdfYRel(h), wxPDF_STYLE_FILL);

lineNum and heightLine are integers, so the call to wxRound() is unnecesary and can be removed.

[vadz]

Hmm, wxWidgets developers probably need to add overloads for the other types to prevent unnecessary breakage like this. Which doesn't change the fact that wxRound() is unnecessary here and could be removed, of course.

[wh11204]

BTW, the wxRound(float) overload is not documented

[utelle]

Thanks for reporting the issue.

I have to admit that I usually only test against actually released wxWidgets versions, and wxPdfDocument definitely works together with wxWidgets 3.1.4.

With only integer values involved the call to wxRound() seems indeed to be unnecessary at this code location. However, I will review the code again to make sure that removing the call is the right thing to do.

[vadz]

FWIW this was fixed in wx itself in the meanwhile, see wxWidgets/wxWidgets@c2e5f3520a315e51e0629cb147d4be36e9ce7848 (and wxWidgets/wxWidgets@1cf7c4793430fb514d7aef141b449704438dcb3d with the documentation improvements), so I think this can be safely closed now.

[utelle]

@vadz In fact, rounding doesn't make much sense for integer values, therefore it isn't bad to check my own code whether maybe something else was coded incorrectly.

[mxdamien]

Hi !

I think this issue this issue is not yet completely resolved. I updated both wxPdfDoc and wxWidgets to Master today giving me this error:

_1> pdfdc.cpp 1> c:\repos[...]\wxpdfdoc\src\pdfdc29.inc(1070): error C2668: 'wxRound': ambiguous call to overloaded function 1> c:\repos[...]\wxwidgets\include\wx\math.h(178): note: could be 'int wxRound(long)' 1> c:\repos[...]\wxwidgets\include\wx\math.h(175): note: or 'int wxRound(short)' 1> c:\repos[...]\wxwidgets\include\wx\math.h(172): note: or 'int wxRound(int)' 1> c:\repos[...]\wxwidgets\include\wx\math.h(165): note: or 'int wxRound(long double)' 1> c:\repos[...]\wxwidgets\include\wx\math.h(151): note: or 'int wxRound(float)' 1> c:\repos[...]\wxwidgets\include\wx\math.h(137): note: or 'int wxRound(double)' 1> c:\repos[...]\wxpdfdoc\src\pdfdc29.inc(1070): note: while trying to match the argument list '(sizet)'

size_t being unsigned int in my case, wxRound is missing the corresponding overload.

@vadz Do you think it's worth while adding unsigned overloads to the merry group of good for nothing functions?

• David

[utelle]

From my point of view it seems to be the wrong decision to introduce overloads for wxRound taking integer arguments, because there is nothing to round for integer arguments. In contrary, the "missing" overload allowed to detect this suspicious code in wxPdfDC.

I will certainly address this issue in the near future - as already implied by my previous post. As soon as a fix is available, it will be mentioned here. So, stay tuned. 😉

[mxdamien]

Thanks a lot ! 👍

[vadz]

Yes, I think we're going to add an overload taking size_t too. It's indeed not useful, but OTOH it doesn't do any harm neither and I prefer to make upgrading to 3.2 as frictionless as possible.

[vadz]

I've created https://github.com/wxWidgets/wxWidgets/pull/2119 which should fix the problem right now and will merge it once the CI builds pass.

Of course, I agree that in the long(er) term wxPdfDoc code should be changed, but it's probably not the only project using wxRound() with non floating-point types.

[utelle]

The call to wxRound in the wxPdfDC implementation which caused the error was removed.

[utelle]

@vadz

I've created wxWidgets/wxWidgets#2119 which should fix the problem right now and will merge it once the CI builds pass.

Although applying rounding to integer values (resulting in a no op) does no harm, I'm not in favor of adding useless overloads to avoid compile time errors caused by misuse of a function. At least I would prefer to get a compiler warning in such cases.

Of course, I agree that in the long(er) term wxPdfDoc code should be changed, but it's probably not the only project using wxRound() with non floating-point types.

Most likely you are right that there are other projects (mis)using wxRound, but I doubt that their number is very large. I changed the wxPdfDocument code accordingly. And that would be the best approach for other projects as well in my opinion.

[vadz]

Although applying rounding to integer values (resulting in a no op) does no harm, I'm not in favor of adding useless overloads to avoid compile time errors caused by misuse of a function. At least I would prefer to get a compiler warning in such cases.

These doing-nothing wxRound() overloads are deprecated, so you would indeed get a warning. Which is, arguably, better than before when the code compiled without any warnings (and still did nothing, because it converted int to double just to pass it to wxRound() where it got rounded back to -- hopefully the same! -- int).

Of course, I agree that in the long(er) term wxPdfDoc code should be changed, but it's probably not the only project using wxRound() with non floating-point types.

Most likely you are right that there are other projects (mis)using wxRound, but I doubt that their number is very large.

I hope it isn't, but, as I explained above, I'd rather facilitate the upgrade path to wx 3.2 as much as possible.

Anyhow, thanks for fixing this!

[utelle]

These doing-nothing wxRound() overloads are deprecated, so you would indeed get a warning.

That's good to hear. Thanks.

Which is, arguably, better than before when the code compiled without any warnings (and still did nothing, because it converted int to double just to pass it to wxRound() where it got rounded back to -- hopefully the same! -- int).

Well, problems could only arise, if rather large integers were involved.

Most likely you are right that there are other projects (mis)using wxRound, but I doubt that their number is very large.

I hope it isn't, but, as I explained above, I'd rather facilitate the upgrade path to wx 3.2 as much as possible.

With the addition of those overloads we will never know how many projects might have been affected. 😉 However, issuing warning messages is a good solution. And I'm looking forward to the first release of wx 3.2.