bugGNU Octave - Bugs: bug #48753, KeyboardTranslator.h macros...

 
 

bug #48753: KeyboardTranslator.h macros failing with Qt5 and GCC 4.8.4

Submitter:  Dan Sebald <sebald>
Submitted:  Wed 10 Aug 2016 08:08:49 PM UTC
   
 
Category:  Configuration and Build System Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Build Failure
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * GNU/Linux
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 11 Aug 2016 05:01:25 PM UTC, comment #13: 

I guess it was in the public header files but not officially intended to be a public function until 5.1?  I found some debate about whether to do that, and whether it was (at the time) exactly equivalent to K_GLOBAL_STATIC.

John W. Eaton <jwe>
Group administrator
Thu 11 Aug 2016 03:40:11 PM UTC, comment #12: 

Compiles and works.  Thanks.

Perhaps the macro has been around for quite a while, at least 4.8 here:

http://stackoverflow.com/questions/20644865/global-object-and-creation-order

(and Mike sees as early as 4.3) but this associated class QGlobalStatic has only been in existence since 5.1.  Maybe there was some underlying reason the 5.1 developers thought there should be a class for such a thing.

Dan Sebald <sebald>
Thu 11 Aug 2016 03:17:18 PM UTC, comment #11: 

I see.

I checked for the macro by looking at the header files in versions 5.0.2, 4.8.7, and as far back as 4.6.3 and 4.3 (from http://sources.debian.net/src/qt4-x11/) and saw Q_GLOBAL_STATE defined in all of them.

I was originally thinking of doing "#if defined (Q_GLOBAL_STATE)" since it is always a macro, but then saw that practically that will always be true.

Mike Miller <mtmiller>
Group Member
Thu 11 Aug 2016 12:19:34 PM UTC, comment #10: 

It looks like Q_GLOBAL_STATIC was not officially part of the public API until 5.1, so I used the version check.

John W. Eaton <jwe>
Group administrator
Thu 11 Aug 2016 12:18:05 PM UTC, comment #9: 

I pushed the following changeset:

http://hg.savannah.gnu.org/hgweb/octave/rev/05a8be08368e

Does this solve the problem for you?

John W. Eaton <jwe>
Group administrator
Thu 11 Aug 2016 11:28:00 AM UTC, comment #8: 

Where do you see that Q_GLOBAL_STATIC has been available since Qt 4.3?  This page says 5.1:

http://doc.qt.io/qt-5/qglobalstatic.html#Q_GLOBAL_STATIC

John W. Eaton <jwe>
Group administrator
Thu 11 Aug 2016 12:27:14 AM UTC, comment #7: 

Ok, good find. I like the idea of using Q_GLOBAL_STATIC if it is available.

I did some digging and Q_GLOBAL_STATIC has been around since Qt 4.3. Octave requires Qt 4.6 at a minimum. I think it should be safe to just use Q_GLOBAL_STATIC unconditionally and just delete the K_GLOBAL_STATIC cruft.

Mike Miller <mtmiller>
Group Member
Wed 10 Aug 2016 11:46:56 PM UTC, comment #6: 

From the file log history, it appears these files have been modified in Octave, so here's a patch for KeyboardTranslator.cpp.

(file #38170)

Dan Sebald <sebald>
Wed 10 Aug 2016 11:20:57 PM UTC, comment #5: 

Oy, I just picked the latest Mint a few months ago and it said it would be supported into 2019.  Well, maybe I'm a bit more willing to switch from V17 to V18 if it means that my Cinnamon settings all transfer over.  Not right now though.

OK, so my best guess is that this hunk of macro code that is failing was tossed in there by the terminal emulator person in order to make a global instance of KeyboardTranslator that can be used in a loadable library for this emulator.  So there really is no easy way of getting away from that approach.

However, I may have found a route to investigate.  In Qt 5.1 was introduced an equivalent type of macro/class:

http://doc.qt.io/qt-5/qglobalstatic.html

Ha!  Here's all I needed to do to make it compile:


diff --git a/libgui/qterminal/libqterminal/unix/KeyboardTranslator.cpp b/libgui/qterminal/libqterminal/unix/KeyboardTranslator.cpp
--- a/libgui/qterminal/libqterminal/unix/KeyboardTranslator.cpp
+++ b/libgui/qterminal/libqterminal/unix/KeyboardTranslator.cpp
@@ -889,7 +889,7 @@ bool KeyboardTranslatorManager::deleteTr
         return false;
     }
 }
-K_GLOBAL_STATIC( KeyboardTranslatorManager , theKeyboardTranslatorManager )
+Q_GLOBAL_STATIC( KeyboardTranslatorManager , theKeyboardTranslatorManager )
 KeyboardTranslatorManager* KeyboardTranslatorManager::instance()
 {
     return theKeyboardTranslatorManager;


And it appears to run.

So, if this is compiled with Qt 5, can we switch over to Q_GLOBAL_STATIC, otherwise use K_GLOBAL_STATIC?  Anyone see an easy way to redefine K_GLOBAL_STATIC without having to modify the source file?

Dan Sebald <sebald>
Wed 10 Aug 2016 10:03:05 PM UTC, comment #4: 

I will try to reproduce this in a Ubuntu 14.04 VM if I have the time, but I don't right now.

Dan - please try a clean build of a fresh clone with no local patches applied to make sure the error occurs, and I will see if I can reproduce when I get time.

The operator= is explicitly deleted in the qbasicatomic.h header file if you take a look there. The question is why is the compiler trying to synthesize a call to that operator at all, it is supposed to be deleted and it shouldn't be needed.

And the other error that you did not quote before is equally bizarre. QBasicAtomicPointer clearly has an "operator TYPE()" conversion operator, so no reason for this error.

Also note that Linux Mint 18 (Sarah) is the latest version, and is based on Ubuntu 16.04 instead of the aging Ubuntu 14.04 old LTS release.

Mike Miller <mtmiller>
Group Member
Wed 10 Aug 2016 09:48:04 PM UTC, comment #3: 

I looked up the KDE license, as my memory is that it isn't GPL, but it looks like multiple licenses for various parts of the code.

Anyway, your suggestion didn't change anything, just says:


         _k_static_##NAME = static_cast<TYPE *> (0);                                                  \
                          ^


It seems to me there are two types of errors.  One is this:


/home/sebald/octave/octave/octave/libgui/qterminal/libqterminal/unix/KeyboardTranslator.h:100:19: error: cannot convert 'QBasicAtomicPointer<KeyboardTranslatorManager>' to 'KeyboardTranslatorManager*' in initialization
         TYPE *x = _k_static_##NAME;                                            \
                   ^


My guess is that one is a Qt5 issue because of the QBasicAtomicPointer.  If I'm remembering correctly, that declaration is to make the pointer something for which signals/slots can be used, or passed within a signal/slot--I forget exactly.

And then there is this second group:


error: use of deleted function 'QBasicAtomicPointer<X>& QBasicAtomicPointer<X>::operator=(const QBasicAtomicPointer<X>&) [with X = KeyboardTranslatorManager]'


I think that is a C++11 issue:

http://stackoverflow.com/questions/27959496/why-c11-implicitly-deletes-my-default-constructor

Following what the checkmarked reply says, it's that "=" and other operators that is the problem.  The compiler doesn't know how to create those operators implicitly.  The operator definitions within this macro code must not be to the compiler's liking.

Is a static global translator manager necessary?  Couldn't there be a global pointer to a translator manager and then use "new" at the start of the main routine?

Dan Sebald <sebald>
Wed 10 Aug 2016 09:45:59 PM UTC, comment #2: 

It seems to work for me with GCC 4.8.5 and Qt 5.6.1 in Debian.

I don't have a problem installing a workaround or replacing this code with something that works properly in all versions of GCC and Qt4 and Qt5.  But I don't have the time to figure out what that is, especially since I can't duplicate the problem.

John W. Eaton <jwe>
Group administrator
Wed 10 Aug 2016 08:53:03 PM UTC, comment #1: 

This code was pulled in wholecloth from the qterminal project, and it has been modified only minimally from its origin.

There's nothing particular strange about these macros, they are declared as a public part of the kdelibs API, and pasted into the qterminal code so that it doesn't depend on kdelibs.

Looking at the error message, the compiler seems to be having a problem with the statement


some_var = 0;


where some_var is an instance of a class that has deleted its default assignment operator, but does have an assignment operator that accepts a pointer.

Can you try this quick hack to see if it resolves the error on your system?


diff --git a/libgui/qterminal/libqterminal/unix/KeyboardTranslator.h b/libgui/qterminal/libqterminal/unix/KeyboardTranslator.h
--- a/libgui/qterminal/libqterminal/unix/KeyboardTranslator.h
+++ b/libgui/qterminal/libqterminal/unix/KeyboardTranslator.h
@@ -98,7 +98,7 @@ static struct K_GLOBAL_STATIC_STRUCT_NAM
     {                                                                          \
         _k_static_##NAME##_destroyed = true;                                   \
         TYPE *x = _k_static_##NAME;                                            \
-        _k_static_##NAME = 0;                                                  \
+        _k_static_##NAME = static_cast<TYPE *> (0);                            \
         delete x;                                                              \
     }                                                                          \
 } NAME;


Mike Miller <mtmiller>
Group Member
Wed 10 Aug 2016 08:08:49 PM UTC, original submission:  

With Qt5 build (gcc 4.8.4) I'm seeing the following error(s):


/home/sebald/octave/octave/octave/libgui/qterminal/libqterminal/unix/KeyboardTranslator.h:101:26: error: use of deleted function 'QBasicAtomicPointer<X>& QBasicAtomicPointer<X>::operator=(const QBasicAtomicPointer<X>&) [with X = KeyboardTranslatorManager]'
         _k_static_##NAME = 0;                                                  \
                          ^
/home/sebald/octave/octave/octave/libgui/qterminal/libqterminal/unix/KeyboardTranslator.h:61:37: note: in expansion of macro 'K_GLOBAL_STATIC_WITH_ARGS'
 #define K_GLOBAL_STATIC(TYPE, NAME) K_GLOBAL_STATIC_WITH_ARGS(TYPE, NAME, ())
                                     ^
/home/sebald/octave/octave/octave/libgui/qterminal/libqterminal/unix/KeyboardTranslator.cpp:892:1: note: in expansion of macro 'K_GLOBAL_STATIC'
 K_GLOBAL_STATIC( KeyboardTranslatorManager , theKeyboardTranslatorManager )
 ^
In file included from /usr/include/qt5/QtCore/qatomic.h:47:0,
                 from /usr/include/qt5/QtCore/qglobalstatic.h:47,
                 from /usr/include/qt5/QtCore/qglobal.h:1034,
                 from /usr/include/qt5/QtCore/qchar.h:45,
                 from /usr/include/qt5/QtCore/qhash.h:45,
                 from /usr/include/qt5/QtCore/QHash:1,
                 from /home/sebald/octave/octave/octave/libgui/qterminal/libqterminal/unix/KeyboardTranslator.h:28,
                 from /home/sebald/octave/octave/octave/libgui/qterminal/libqterminal/unix/KeyboardTranslator.cpp:25:
/usr/include/qt5/QtCore/qbasicatomic.h:258:26: error: declared here
     QBasicAtomicPointer &operator=(const QBasicAtomicPointer &) = delete;
                          ^
make[2]: *** [libgui/qterminal/libqterminal/unix/libgui_qterminal_libqterminal_la-KeyboardTranslator.lo] Error 1


which is attributable to C++11 compatibility issues.  See

https://savannah.gnu.org/bugs/?40252#comment19

I don't want to upgrade compilers to see if that fixes anything.  Other than it being more work than I want to attempt, this is the compiler used for a recent Mint Rosa (Ubuntu) bundle.  How can Octave expect to be accepted if it requires a compiler greater than what the main bundles are using?

Looking at the macro code, which probably shouldn't even be in the project, it seems to use some Qt macros (and if Qt version is too low, define its own Qt macro).  The following lines of code seem somewhat non Qt5/Template friendly:

+verbatim
    static void destroy()                                                      \
    {                                                                          \
        k_static##NAME##_destroyed = true;                                   \
        TYPE *x = k_static##NAME;                                            \
        k_static##NAME = 0;                                                  \
        delete x;                                                              \
    }                                                                          \
-verbatim-

For example, in one instance it is:


static QBasicAtomicPointer<TYPE > _k_static_##NAME = Q_BASIC_ATOMIC_INITIALIZER(0);


while above the line of code is:


        _k_static_##NAME = 0;


Doesn't seem consistent.  Qt5 headers probably use the macros in a more elegant way.

I'm going to search around to see if there is a better way to handle keyboard translation Qt.  To me, it seems it should be easier because that is the purpose of multiplatform Qt.

Dan Sebald <sebald>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by sebald (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-05-12 mtmiller Carbon-CopyRemoved 80942 -
    2016-08-11 jwe StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2016-08-11 jwe StatusIn Progress Ready For Test
    2016-08-11 mtmiller StatusWorks For Me In Progress
    2016-08-10 sebald Attached File- Added octave-qt5build_bug_40252-djs2016aug10.patch, #38170
    2016-08-10 mtmiller CategoryGUI Configuration and Build System
        StatusNone Works For Me

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code