Review Request 129759: Fix the extraction of parameter initialisation.

Shaheed Haque srhaque at theiet.org
Fri Jan 6 09:37:24 UTC 2017


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129759/
-----------------------------------------------------------

(Updated Jan. 6, 2017, 9:37 a.m.)


Review request for Extra Cmake Modules.


Changes
-------

I was unable to create a self-contained repro, so the new test is a best guess "negative proof".


Repository: extra-cmake-modules


Description
-------

This involves reverting a change (see below). Without this reversion, we get for example:

sip: .../kfontutils.sip:14: Compulsory argument given after optional argument

Where the .h file says:

============
qreal KGUIADDONS_EXPORT adaptFontSize(QPainter &painter,
                                      const QString &text,
                                      qreal width,
                                      qreal height,
                                      qreal maxFontSize = 28.0,
                                      qreal minFontSize = 1.0,
                                      AdaptFontSizeOptions flags = NoFlags);
============

while the SIP file says (note the flags parameter is missing its initialiser):

    qreal adaptFontSize(QPainter & painter, ..., double minFontSize = 1.0, QFlags<KFontUtils::AdaptFontSizeOption> flags);

With this change, the SIP file says:

    qreal adaptFontSize(QPainter & painter, ..., double minFontSize = 1.0, QFlags<KFontUtils::AdaptFontSizeOption> flags = NoFlags);

There are many instances of this issue, but in only a handful of cases such
as this example from kguiaddons, the resulting SIP code does not compile. That
is the subject of a pending fix local to kguiaddons.

The details of the bug are that the test on "member.kind.is_expression()" does
not work reliably for unknown reasons. (In the case of KFontUtils::adaptFontSize's
flags argument, the member.kind is TYPE_REF). Also, the logic around the isQFlags
variable seems to be unclear and possibly broken.

The commit being reverted is in a github fork, and its original details are:

    Title: "Try to make a better implementation of determining init value"
    Commit ID: bcf1d6078f2c544d3908490ae538d2b97ca997d3


Diffs (updated)
-----

  find-modules/sip_generator.py 10be147711540437d38153fa8fe859bd34d7aa6c 
  tests/GenerateSipBindings/cpplib.h 0d63c30ed52b5d817c233e358b1ebce7a70c8218 
  tests/GenerateSipBindings/cpplib.cpp c5b7a5fbe6310b6b3742c3d35d2d04c67d77066f 

Diff: https://git.reviewboard.kde.org/r/129759/diff/


Testing
-------

Run across all of /usr/include/KF5, and reviewed the changed output.


Thanks,

Shaheed Haque

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-buildsystem/attachments/20170106/8254192c/attachment-0001.html>


More information about the Kde-buildsystem mailing list