includes of X11 headers in LO are wrapped with #include <prex.h> and #include <postx.h> this is because old LO code, especially in "tools" and "vcl", defines types that have the same names as X11 types, so the headers do a bunch of stupid macro hackery to rename the X11 types so they don't collide with LO types. it would be obviously much simpler if the LO types simply had names that don't collide with X11 types. "git grep" or ctags or opengrok.libreoffice.org should find the definitions of types in LO that are re-defined in the wrapper headers. there are several ways to clean up collisions: 1) if there is no equivalent in LO of the redefined type, it can just be removed from the wrapper header 2) in some cases it can be avoided to have the X11 type and the LO type visible in the same LO source file by not including both the defining LO and X11 headers 3) if the X11 type is not a macro, then putting the LO type into a namespace and namespace-qualifying the uses should avoid the collisions 4) if the X11 type is a macro, then adding a namespace prefix to the LO type is probably required
Is there a compiler flag that we can turn on that will automatically report type name collisions?
(In reply to comment #1) > Is there a compiler flag that we can turn on that will automatically report > type name collisions? Even easier: New type names should be in a namespace. That should make practically rule out collisions of types (up until we collide a full namespace somewhere).
One thing that we need consensus on is whether to use a "real" C++ namespace or a "C-style" one, i.e. just prefixing the conlicting LibreOffice type names with some short string, like "Vcl". I guess the C++ namespace would be better from a C++ orthodoxy point of view, but what should the namespace be? "vcl::" ? "org::libreoffice::vcl::" (brrr)? In any case, we *don't* want to repeat the current disaster of inconsistent "using" declaration, varying from one source file to another. Would using a C++ namespace have the benefit that it would be enough to just add a "using namespace vcl" (or whatever) in some header, and only those few source files that actually refer to the identically-named X11 types would need to add a :: prefix to those? Personally I wouldn't mind using a "C-style" prefix, but then I am well known to not really be that huge a C++ fan. For the cases where there are clashes with *macros* (I guess mostly for Win32?), the "C-style" identifier prefix (or even renaming our identifier completely) is the only solution, right?
(In reply to comment #3) > One thing that we need consensus on is whether to use a "real" C++ namespace > or a "C-style" one, i.e. just prefixing the conlicting LibreOffice type > names with some short string, like "Vcl". IHMO C++ namespaces as its better for tooling, e.g. doxygen: http://docs.libreoffice.org/sw/html/namespaces.html > I guess the C++ namespace would be better from a C++ orthodoxy point of > view, but what should the namespace be? "vcl::" ? "org::libreoffice::vcl::" > (brrr)? No, not a java-like "lets prefix my full home address" thing. We already have "com::sun::star::" as our public API. There were discussions of aliasing that as "libreoffice::" (llunak wanted something like that IIRC). For our private/internal API, having "vcl::Foo" should already be less risky than "VclFoo" -- and in the long term, if needed, we could move "vcl::" to something-like "libreoffice::private::vcl::" or "libreoffice::internal::vcl::", when there are conflicts. It will be a lot easier to move a namespace than to rename hundreds of classes. > In any case, we *don't* want to repeat the current disaster of > inconsistent "using" declaration, varying from one source file to another. IMHO that is mostly a symptom of having unmanageable 10KLOC or more per source file, so that its not easy to see what a using (or #include for the matter) is for and thus they are rarely touched and just grow. > Would using a C++ namespace have the benefit that it would be enough to just > add a "using namespace vcl" (or whatever) in some header, and only those few > source files that actually refer to the identically-named X11 types would > need to add a :: prefix to those? using in header files is so harmful that most coding styles disencourage it, e.g. Sutter/Alexandrescus "C++ Coding Standards" rule 59: "Don’t write namespace usings in a header file or before an #include." http://stackoverflow.com/questions/5849457/using-namespace-in-c-headers Anyway, this is more of a discussion for the mailing list or the ESC, lets take it there if there is further need.
Oh, one more thing: Making everything C++ mangled names (or other long prefixes) might have an impact on dynamic linking and thus startup time. mmeeks would have some words about that, Im sure.
Tor Lillqvist committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=708fc1c187986796861c4dcecba2861ce272dd57 fdo#82577, fdo#82579: Handle Cursor The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Tor Lillqvist committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=30ae83c268125383866d47a7ee3e4a5dfcf59f71 fdo#82577: Handle KeyCode The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Noel Grandin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=60e78fbb806bb45e635ba1de45ceffe187938ac0 fdo#82577: Handle Font The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
see <http://lists.freedesktop.org/archives/libreoffice/2014-September/063545.html> "Re: Performance samples for LibreOffice ..." for another benefit of fixing this fully
Noel Grandin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=827c46e7d75000cb03b0ce21759f9d0825f0c096 fdo#82577: Handle Window The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Noel Grandin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=8dbde0845a3989528614addb9bd0333f60c522a5 fdo#82577: Handle Region The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Noel Grandin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=c9d4a2887c13a5df244022276dd79a5bef8af0ea fdo#82577: Handle PolyPolygon The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Noel Grandin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=fc04f76336fdf8c96e35382cdeb497e2f939705c fdo#82577: Handle Time The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Noel Grandin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=7bacb89bb955f4985e435c33dde629099dab744b fdo#82577: Handle Icon The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Noel Grandin committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=544c4fc14bca0e670ed1d59569f22c5d4a643c72 fdo#82577: Handle KeyPress The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Migrating Whiteboard tags to Keywords: (EasyHack DifficultyBeginner SkillCpp TopicCleanup ) [NinjaEdit]
I'm going to work on this task.
Jorenz Paragas committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=7915bd1929afe6ae2a3fea670c3310462c7e766a tdf#82577: Handle DestroyAll, InitializeToken, NextRequest It will be available in 5.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
Jorenz Paragas committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=0d65526d397aff9bff367060f19f19476de748c2 tdf#82577: Remove remaining #undefs in postx.h It will be available in 5.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
JanI is default CC for Easy Hacks (Add Jan; remove LibreOffice Dev List from CC) [NinjaEdit]
Jorenz Paragas committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=e565d346fc949782703bdefa4d3ce8777b7940a9 tdf#82577: Remove prex.h and postx.h wrapper headers It will be available in 5.2.0. The patch should be included in the daily builds available at http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More information about daily builds can be found at: http://wiki.documentfoundation.org/Testing_Daily_Builds Affected users are encouraged to test the fix and report feedback.
With the above commit, this task is finished.