includes of Mac OS X headers in LO are wrapped with #include <premac.h> and #include <postmac.h> this is because old LO code, especially in "tools" and "vcl", defines types that have the same names as Mac OS X types, so the headers do a bunch of stupid macro hackery to rename the Mac OS X 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 Mac OS X 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 Mac OS X type and the LO type visible in the same LO source file by not including both the defining LO and Mac OS X headers 3) if the Mac OS X 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 Mac OS X type is a macro, then adding a namespace-like prefix to the LO type is probably required
Tor Lillqvist committed a patch related to this issue. It has been pushed to "master": http://cgit.freedesktop.org/libreoffice/core/commit/?id=c2d3b7e4aa0480ed2c66b7240ef05cfff6a07125 fdo#82579: Only Point, Polygon, Size and TimeValue need to be handled 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=95a5d7733190d66b1c4761a5bbaa5a2cc2361dd0 fdo#82579: The SDK used by the "TDF" tb needs Cursor and Line to be handled 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.
I guess handling TimeValue will be problematic here, as it is defined in a stable API header, <osl/time.h>. OK to break API (and ABI?) by renaming TimeValue there to OslTimeValue? Or is something more complicated needed to hide the osl TimeValue in those (relatively few?) source files that include both the <osl/time.h> and Mac and iOS system headers? A start of an attempt at that would be as below: diff --git a/include/postmac.h b/include/postmac.h index a0cc11b..b489d67 100644 --- a/include/postmac.h +++ b/include/postmac.h @@ -21,6 +21,5 @@ #undef Point #undef Polygon #undef Size -#undef TimeValue /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/include/premac.h b/include/premac.h index f22c193..ee201c9 100644 --- a/include/premac.h +++ b/include/premac.h @@ -21,6 +21,5 @@ #define Point MacOSPoint #define Polygon MacOSPolygon #define Size MacOSSize -#define TimeValue MacOSTimeValue /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/vcl/inc/osx/salinst.h b/vcl/inc/osx/salinst.h index 8a88a26..2a3032d 100644 --- a/vcl/inc/osx/salinst.h +++ b/vcl/inc/osx/salinst.h @@ -20,6 +20,8 @@ #ifndef INCLUDED_VCL_INC_OSX_SALINST_H #define INCLUDED_VCL_INC_OSX_SALINST_H +#include "quartz/hide-osl-timevalue.h" + #include "comphelper/solarmutex.hxx" #include "osl/thread.hxx" #include "osl/conditn.h" diff --git a/vcl/osx/a11ycomponentwrapper.mm b/vcl/osx/a11ycomponentwrapper.mm index b91029d..5a2d52a 100644 --- a/vcl/osx/a11ycomponentwrapper.mm +++ b/vcl/osx/a11ycomponentwrapper.mm @@ -17,6 +17,8 @@ * the License at http://www.apache.org/licenses/LICENSE-2.0 . */ +#include "quartz/hide-osl-timevalue.h" + #include "quartz/utils.h" #include "a11ycomponentwrapper.h" #include "a11yrolehelper.h" diff --git a/vcl/osx/a11yfactory.mm b/vcl/osx/a11yfactory.mm index 30437a8..3437565 100644 --- a/vcl/osx/a11yfactory.mm +++ b/vcl/osx/a11yfactory.mm @@ -17,7 +17,6 @@ * the License at http://www.apache.org/licenses/LICENSE-2.0 . */ - #include "osx/salinst.h" #include "osx/a11yfactory.h" #include "osx/a11yfocustracker.hxx" diff --git a/vcl/osx/a11yrolehelper.mm b/vcl/osx/a11yrolehelper.mm index 45423db..2e1aee3 100644 --- a/vcl/osx/a11yrolehelper.mm +++ b/vcl/osx/a11yrolehelper.mm @@ -17,6 +17,7 @@ * the License at http://www.apache.org/licenses/LICENSE-2.0 . */ +#include "quartz/hide-osl-timevalue.h" #include "osx/a11yfactory.h" diff --git a/vcl/osx/printaccessoryview.mm b/vcl/osx/printaccessoryview.mm index e2db4c3..35fc28a 100644 --- a/vcl/osx/printaccessoryview.mm +++ b/vcl/osx/printaccessoryview.mm @@ -19,6 +19,8 @@ #include "sal/config.h" +#include "quartz/hide-osl-timevalue.h" + #include "tools/resary.hxx" #include "vcl/print.hxx" diff --git a/vcl/osx/printview.mm b/vcl/osx/printview.mm index 8b324b9..498e9ed 100644 --- a/vcl/osx/printview.mm +++ b/vcl/osx/printview.mm @@ -17,6 +17,7 @@ * the License at http://www.apache.org/licenses/LICENSE-2.0 . */ +#include "quartz/hide-osl-timevalue.h" #include "vcl/print.hxx" diff --git a/vcl/osx/saldata.cxx b/vcl/osx/saldata.cxx index f4dc179..0a94673 100644 --- a/vcl/osx/saldata.cxx +++ b/vcl/osx/saldata.cxx @@ -19,6 +19,8 @@ #include <config_features.h> +#include "quartz/hide-osl-timevalue.h" + #include "osx/saldata.hxx" #include "osx/salnsmenu.h" #include "osx/salinst.h" diff --git a/vcl/osx/salframeview.mm b/vcl/osx/salframeview.mm index 8e74086..fbedacf 100644 --- a/vcl/osx/salframeview.mm +++ b/vcl/osx/salframeview.mm @@ -17,6 +17,8 @@ * the License at http://www.apache.org/licenses/LICENSE-2.0 . */ +#include "quartz/hide-osl-timevalue.h" + #include <sal/alloca.h> #include <sal/macros.h> diff --git a/vcl/osx/salinst.cxx b/vcl/osx/salinst.cxx index d8adf03..8223214 100644 --- a/vcl/osx/salinst.cxx +++ b/vcl/osx/salinst.cxx @@ -21,6 +21,8 @@ #include <stdio.h> +#include "quartz/hide-osl-timevalue.h" + #include <tools/solarmutex.hxx> #include "osl/process.h" @@ -665,7 +667,7 @@ void AquaSalInstance::Yield( bool bWait, bool bHandleAllCurrentEvents ) // wait until any thread (most likely the main thread) // has dispatched an event, cop out at 200 ms osl_resetCondition( maWaitingYieldCond ); - TimeValue aVal = { 0, 200000000 }; + OslTimeValue aVal = { 0, 200000000 }; sal_uLong nCount = ReleaseYieldMutex(); osl_waitCondition( maWaitingYieldCond, &aVal ); AcquireYieldMutex( nCount ); diff --git a/vcl/osx/salsys.cxx b/vcl/osx/salsys.cxx index f10ba61..b75d249 100644 --- a/vcl/osx/salsys.cxx +++ b/vcl/osx/salsys.cxx @@ -17,6 +17,8 @@ * the License at http://www.apache.org/licenses/LICENSE-2.0 . */ +#include "quartz/hide-osl-timevalue.h" + #include "rtl/ustrbuf.hxx" #include "vcl/button.hxx" diff --git a/vcl/osx/saltimer.cxx b/vcl/osx/saltimer.cxx index 17c4d80..ff3fd49 100644 --- a/vcl/osx/saltimer.cxx +++ b/vcl/osx/saltimer.cxx @@ -17,6 +17,8 @@ * the License at http://www.apache.org/licenses/LICENSE-2.0 . */ +#include "quartz/hide-osl-timevalue.h" + #include "osx/saltimer.h" #include "osx/salnstimer.h" #include "osx/saldata.hxx" diff --git a/vcl/osx/vclnsapp.mm b/vcl/osx/vclnsapp.mm index d1d086f..4955ae2 100644 --- a/vcl/osx/vclnsapp.mm +++ b/vcl/osx/vclnsapp.mm @@ -19,6 +19,8 @@ #include <config_features.h> +#include "quartz/hide-osl-timevalue.h" + #include "sal/config.h" #include <vector> diff --git a/vcl/quartz/ctlayout.cxx b/vcl/quartz/ctlayout.cxx index cd9771a..de076e6 100644 --- a/vcl/quartz/ctlayout.cxx +++ b/vcl/quartz/ctlayout.cxx @@ -17,6 +17,8 @@ * the License at http://www.apache.org/licenses/LICENSE-2.0 . */ +#include "quartz/hide-osl-timevalue.h" + #include <sal/types.h> #include <boost/ptr_container/ptr_vector.hpp> #include "tools/debug.hxx" diff --git a/vcl/quartz/salgdi.cxx b/vcl/quartz/salgdi.cxx index 80d6d5e..7130f7d 100644 --- a/vcl/quartz/salgdi.cxx +++ b/vcl/quartz/salgdi.cxx @@ -21,10 +21,7 @@ #include "sal/config.h" -#include "osl/file.hxx" -#include "osl/process.h" - -#include "osl/mutex.hxx" +#include "quartz/hide-osl-timevalue.h" #include "rtl/bootstrap.h" #include "rtl/strbuf.hxx" diff --git a/vcl/source/app/dbggui.cxx b/vcl/source/app/dbggui.cxx index 85b4528..d32d61e 100644 --- a/vcl/source/app/dbggui.cxx +++ b/vcl/source/app/dbggui.cxx @@ -52,7 +52,6 @@ #include "vcl/unohelp2.hxx" #include "salinst.hxx" -#include "svsys.h" #include "com/sun/star/i18n/XCharacterClassification.hpp" diff --git a/vcl/source/app/settings.cxx b/vcl/source/app/settings.cxx index a30fd34..90015f6 100644 --- a/vcl/source/app/settings.cxx +++ b/vcl/source/app/settings.cxx @@ -21,7 +21,6 @@ #include <officecfg/Office/Common.hxx> -#include <svsys.h> #include "comphelper/processfactory.hxx" #include <rtl/bootstrap.hxx> #include "tools/debug.hxx" diff --git a/vcl/source/window/accessibility.cxx b/vcl/source/window/accessibility.cxx index 0f3b416..720dd4f 100644 --- a/vcl/source/window/accessibility.cxx +++ b/vcl/source/window/accessibility.cxx @@ -54,10 +54,6 @@ #include "vcl/virdev.hxx" #include "vcl/settings.hxx" -// declare system types in sysdata.hxx -#include "svsys.h" -#include "vcl/sysdata.hxx" - #include "salframe.hxx" #include "salobj.hxx" #include "salinst.hxx" diff --git a/vcl/source/window/syschild.cxx b/vcl/source/window/syschild.cxx index 78c31f1..d930c3d 100644 --- a/vcl/source/window/syschild.cxx +++ b/vcl/source/window/syschild.cxx @@ -24,9 +24,6 @@ #include <tools/rc.h> -// declare system types in sysdata.hxx -#include <svsys.h> - #include <vcl/window.hxx> #include <vcl/sysdata.hxx> #include <vcl/svapp.hxx>
Forgot the quartz/hide-osl-timevalue.h, would be something like this. (But the above was just one test approach, not sure at all that it is sane.) #define TimeValue OslTimeValue #include <osl/conditn.h> #include <osl/file.hxx> #include <osl/mutex.hxx> #include <osl/process.h> #include <osl/thread.hxx> #include <osl/time.h> #undef TimeValue
(In reply to Tor Lillqvist from comment #3) > I guess handling TimeValue will be problematic here, as it is defined in a > stable API header, <osl/time.h>. > > OK to break API (and ABI?) by renaming TimeValue there to OslTimeValue? Even though certainly very poor choice of non-prefixed name in osl/time.h, it would be better if we can work around the issue without breaking API (as outlined in your patch).
Or we could just say that having premac.h/postmac.h is not that bad after all and close this bug as WONTFIX. After all, the above approach at least is hardly any cleaner...
Well i am going to try to do what is left of this. Hopefully i can get ride of both headers. Though i doubt getting rid of them will improve performance by much.
What made you think this bug has anything to do with performance?
I don't know. I don't really understand what the bug really is. I understand its about types conflicting with each other. But much like you said I can't think of a clear way to do it not using a wrapper. I even talked to my professor about it. The only thing he could think of was making a namespace for the conflicting types. But would that be any cleaner then headers?
Migrating Whiteboard tags to Keywords: (EasyHack DifficultyBeginner SkillCpp TopicCleanup ) [NinjaEdit]
JanI is default CC for Easy Hacks (Add Jan; remove LibreOffice Dev List from CC) [NinjaEdit]
Please be aware, that this easyhack is considered an important but large scale cosmetic change as described in https://wiki.documentfoundation.org/Development/LargeScaleChanges It was in decided by the ESC to close this kind of easyhacks, and send them directly as mail, to new contributors. https://lists.freedesktop.org/archives/libreoffice/2016-August/074920.html Please do not submit patches with many files !! This particular easyhack is kept open as an exception to the rule.
Re-evaluating the EasyHack in 2023 This enhancement/refactoring is still relevant, as the wrapper headers premac.h / postmac.h are still there. The remaining instances are more difficult as discussed in comment 3.