Summary: | get rid of prex.h / postx.h wrapper headers | ||
---|---|---|---|
Product: | LibreOffice | Reporter: | Michael Stahl (allotropia) <michael.stahl> |
Component: | LibreOffice | Assignee: | Jorenz Paragas <j.paragas.237> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | h3734236, mentoring, sberg.fun |
Priority: | medium | Keywords: | difficultyBeginner, easyHack, skillCpp, topicCleanup |
Version: | Inherited From OOo | ||
Hardware: | Other | ||
OS: | Linux (All) | ||
Whiteboard: | ToBeReviewed target:5.2.0 | ||
Crash report or crash signature: | Regression By: |
Description
Michael Stahl (allotropia)
2014-08-13 20:06:22 UTC
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. |