Issue 108631 - Preprocessor defines in layout-pre.hxx to save typing namespace qualifiers break compile
Summary: Preprocessor defines in layout-pre.hxx to save typing namespace qualifiers br...
Status: UNCONFIRMED
Alias: None
Product: General
Classification: Code
Component: ui (show other issues)
Version: DEV300m70
Hardware: Sun Solaris
: P3 Trivial (vote)
Target Milestone: ---
Assignee: AOO issues mailing list
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-24 22:33 UTC by nstange
Modified: 2017-05-20 11:28 UTC (History)
4 users (show)

See Also:
Issue Type: PATCH
Latest Confirmation in: ---
Developer Difficulty: ---


Attachments
Patch against svn trunk rev. 277953 + sh-/GNU m4-script to create it (110.00 KB, application/x-tar)
2010-01-25 14:27 UTC, nstange
no flags Details

Note You need to log in before you can comment on or make changes to this issue.
Description nstange 2010-01-24 22:33:19 UTC
In layout-pre.hxx are preprocessor defines of the form
#define symbol layout::symbol

Those symbols are put as arguments into other macros (IMPL_LINK, zoom.cxx)
resulting in double expansion with Sun's cc:
symbol becomes layout::layout::symbol.

I think this technique undergoes the C++-namespace concept and in addition it 
prevents compilation. One should make it either explicit by 'using namespace
layout;' or should replace the macro instantiations within the code by their
definitions (my personal preference).
Comment 1 Olaf Felka 2010-01-25 06:55:34 UTC
@ hr: Please have a look.
Comment 2 nstange 2010-01-25 14:27:54 UTC
Created attachment 67401 [details]
Patch against svn trunk rev. 277953 + sh-/GNU m4-script to create it
Comment 3 nstange 2010-01-25 14:39:14 UTC
I created the patch above to replace all those bad defines by their definition
in all sources including a 'layout-pre.hxx' (determined through simple grepping).

In files including 'sfx2/layout-pre.hxx' I replaced 
SfxDialog, SfxModalDialog, SfxModelessDialog
by layout::SfxDialog each.

Since I haven't set up an environment to compile a trunk, I've not tested it,
but this technic works on OOO_310_m19 with some minor modifications. 

Please note that '::Window' has to be replaced by 'Windows' to make the source
compilable by Sun's cc although I'm not sure why and thus this change is not
cotnained within the patch.

Since the definition of LocalizedString depends on the preprocessor macro
ENABLE_LAYOUT, I left it as is, but it's still bad code (when a LocalizedString
is put as an argument into another macro the problem initially described
arises). Maybe one tests ENABLE_LAYOUT at all places a Localized string is used
or renames the macro, maybe prepending it with an underscore, to prevent its
multiple expansion. 
Comment 4 nstange 2010-01-25 14:40:06 UTC
sorry, of cource I mean '::Window' has to be replaces by 'Window'
Comment 5 nstange 2010-01-25 15:06:23 UTC
sorry for frequent posting, but I just want to note, that the m4 processor has
to be the GNU m4 as the '-P'-option is necessary!
Comment 6 jens-heiner.rechtien 2010-02-10 19:29:21 UTC
@nstange: May I ask which version of Sun CC you are using? We do compile all our
stuff Sun Studio 12 and I'm not aware of any breakage in this area.

@mba,@jcn: opinions?
Comment 7 nstange 2010-02-10 21:19:43 UTC
Sorry, here it is:
cc -V       
cc: Sun C 5.9 SunOS_sparc 2007/05/03

It's part of SunStudio12.
Comment 8 jens-heiner.rechtien 2010-02-15 12:55:49 UTC
@nstange: this is the compiler we use as well, albeit a slightly newer patch level:
CC: Sun C++ 5.9 SunOS_sparc Patch 124863-08 2008/10/16

The patch might explain while we got no problem with the "::Window" vs. "Window"
construct, I think I have a very weak recollection of something being
temporarily broken in this respect with FCS Sun C++ 5.9

I won't comment on the macro expansion vs. namespace issue as the responsible
engineers are more qualified to do so.

@mba: reassign
@mba, jcn: please comment
Comment 9 Mathias_Bauer 2010-02-16 08:13:14 UTC
@nstange: thanks for looking into this, in fact this code creates a problem (not
only) on Solaris.

Jan, I would like to get your opinion about that.
Comment 10 Rob Weir 2013-03-11 15:01:16 UTC
I'm adding this comment to all open issues with Issue Type == PATCH.  We have 220 such issues, many of them quite old.  I apologize for that.  

We need your help in prioritizing which patches should be integrated into our next release, Apache OpenOffice 4.0.

If you have submitted a patch and think it is applicable for AOO 4.0, please respond with a comment to let us know.

On the other hand, if the patch is no longer relevant, please let us know that as well.

If you have any general questions or want to discuss this further, please send a note to our dev mailing list:  dev@openoffice.apache.org

Thanks!

-Rob
Comment 11 Marcus 2017-05-20 11:28:07 UTC
Reset assigne to the default "issues@openoffice.apache.org".