Bug 82580 - get rid of prewin.h / postwin.h wrapper headers
Summary: get rid of prewin.h / postwin.h wrapper headers
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: Other Windows (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:5.4.0
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2014-08-13 20:12 UTC by Michael Stahl (CIB)
Modified: 2017-03-31 06:29 UTC (History)
1 user (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Stahl (CIB) 2014-08-13 20:12:30 UTC
includes of Win32 headers in LO are wrapped with
#include <prewin.h>
and
#include <postwin.h>

this is because old LO code, especially in "tools" and "vcl",
defines types that have the same names as Win32 types,
so the headers do a bunch of stupid macro hackery
to rename the Win32 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 Win32 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 Win32 type
   and the LO type visible in the same LO source file
   by not including both the defining LO and Win32 headers

3) if the Win32 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 Win32 type is a macro, then adding a namespace-like
   prefix to the LO type is probably required
Comment 1 Tor Lillqvist 2014-08-16 06:39:42 UTC
One note, that I sadly think needs to be spelled out: Please don't attempt to do this unless you actually have Windows, Visual Studio, etc and are able to build LibreOffice on Windows. Don't submit changes without verifying yourself that they don't break the compilation.
Comment 2 Commit Notification 2014-08-16 15:55:26 UTC
Tor Lillqvist committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a8669b56b194a7cd6da771dfb9904710da24c0b4

fdo#82580: Win32 GetObject() simplification



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.
Comment 3 Thomas Arnhold 2014-08-16 17:47:24 UTC
(In reply to comment #0)
> 2) in some cases it can be avoided to have the Win32 type
>    and the LO type visible in the same LO source file
>    by not including both the defining LO and Win32 headers

Note, that this could be complicated, because we (mostly) use precompiled headers on Windows, which include nearly all headers needed by a module. See for example oox/inc/pch/precompiled_oox.hxx, which includes all headers from the oox module. And thus avoiding to include one file could be problematic.
Comment 4 Noel Grandin 2014-08-17 08:43:40 UTC
Personally I think we should do what Cygwin have done and create our own copy of the win32 header that only contains the definitions we need, thus minimizing the odds of conflict.
Comment 5 Tor Lillqvist 2014-08-17 09:57:22 UTC
That is madness. We can't cross-compile a complete LibreOffice because the reverse-engineered headers provided by mingw-w64 are incomplete. And those headers have a very long history. So you expect us to be able to create something similar, and in a legally valid clean-room fashion, just like that?
Comment 6 Tor Lillqvist 2015-03-22 07:24:43 UTC
Please don't just sign a bug to yourself out of the blue. That is not how it works.
Comment 7 Fedor 2015-03-22 13:19:11 UTC
Assign this bug to me. I think it is good start before refactoring god objects at gsoc.
Comment 8 Tor Lillqvist 2015-03-22 16:57:50 UTC
It is enough to say you are working on this. If somebody else (another GSoC hopeful?) is rude enough to jump onto this task just at the same time (for a few weeks), we will notice, and take that as a sign that he/she is not good at cooperation.
Comment 9 Fernando Montes 2015-03-22 22:58:54 UTC
any chance you need help on this?
Comment 10 Fedor 2015-03-23 05:44:50 UTC
Still not need help.
Eclipse searchs well.
Comment 11 Fernando Montes 2015-03-23 07:04:33 UTC
Darn ok. I will find another easy hack to be my first
Comment 12 Robinson Tryon (qubit) 2015-12-14 04:59:03 UTC Comment hidden (obsolete)
Comment 13 Robinson Tryon (qubit) 2016-02-18 14:51:26 UTC Comment hidden (obsolete)
Comment 14 jani 2016-08-05 09:17:32 UTC
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.
Comment 15 Commit Notification 2017-03-31 06:29:25 UTC
Miklos Vajna committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a5a571307fb3306b74ab46b085cde6388270a770

tdf#82580 tools: rename Rectangle to tools::Rectangle

It will be available in 5.4.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.