Opened 15 years ago

Closed 15 years ago

Last modified 13 months ago

#8391 closed patch (fixed)

makeInstance with MS VC++7

Reported by: SF/h00ligan Owned by: fingolfin
Priority: normal Component: Port: Linux
Keywords: Cc:
Game:

Description

MS VC++7 stops compiling external function template
makeInstance.
workaround:
- explicitly indicate namespace with "::"
- move SingletonBaseType typedef to public section

Ticket imported from: #1095133. Ticket imported from: patches/496.

Attachments (4)

MakeInstance.patch.bz2 (860 bytes ) - added by SF/h00ligan 15 years ago.
makeInstance patch
makeInstance.patch (6.1 KB ) - added by fingolfin 15 years ago.
BuildLog.htm.bz2 (2.0 KB ) - added by SF/h00ligan 15 years ago.
makeinstance.patch (1.7 KB ) - added by fingolfin 15 years ago.
Yet another patch, maybe helps GCC 3.2.x?

Download all attachments as: .zip

Change History (23)

by SF/h00ligan, 15 years ago

Attachment: MakeInstance.patch.bz2 added

makeInstance patch

comment:1 by wjp, 15 years ago

The changes to the friend lines won't compile for me with
gcc 3.3.4 in linux (gentoo).

comment:2 by fingolfin, 15 years ago

Try this patch...

by fingolfin, 15 years ago

Attachment: makeInstance.patch added

comment:3 by SF/h00ligan, 15 years ago

after fingolfin patch apeared followed messages:
../../common\singleton.h(52) : error C2220: warning treated as
error - no object file generated
../../common\singleton.h(52) : warning C4506: no definition for
inline function 'OSystem *Common::Singleton<T>::
makeInstance(void)'
with
[
T=OSystem
]

after disabling (/wd4506) this message - scummvm runs well
may definition
"OSystem *Common::Singleton<OSystem>::makeInstance() {
....."
be moved to declaration section?

comment:4 by fingolfin, 15 years ago

I am not sure sure what you mean with "move to declaration
section" -- if you mean "move to the header file", then no,
that would not be OK.

Can you tell me the full error message? In particular, which files
is it compiling at that point?

comment:5 by SF/h00ligan, 15 years ago

i attached my build log (with enabled C4506)

1) you quite right - definition should rest in cpp, but ms
compiler assumed that "makeInstance" is inline method - and
warn that it can't find method definition
2)__declspec(noinline) - did't remove warning
3)only /wd4506 option makes scummvm be compiled (but is it
right solution?)

by SF/h00ligan, 15 years ago

Attachment: BuildLog.htm.bz2 added

comment:6 by fingolfin, 15 years ago

I googled for that warning, and the microsoft pages for it
suggest adding an "extern" before the declaration. Does that help?

template <>
extern OSystem *Common::Singleton<OSystem>::makeInstance();

(see also <http://msdn.microsoft.com/library/default.asp?url=/library/
en-us/vccore/html/C4506.asp>)

comment:7 by SF/h00ligan, 15 years ago

no it does not help:

../../common\system.h(38) : error C2220: warning treated as
error - no object file generated
../../common\system.h(38) : warning C4630: 'Common::
Singleton<T>::makeInstance' : 'extern' storage-class specifier
illegal on member definition
with
[
T=OSystem
]

comment:8 by fingolfin, 15 years ago

Well, I applied my patch for now -- at least with it you only get a
warning, not errors.

For now, I'd say just ignore warning 4506, as we don't want to inline that
piece of code anyway. Maybe you can use some #pragma surrounding
the offending code for that?

comment:9 by eriktorbjorn, 15 years ago

I wonder if that's why I can no longer compile ScummVM under
MinGW. I get the following linking errors:

ase/libbase.a(main.o)(.text+0x1424): In function `main':
c:/Privat/CygWin/home/torbjorn/Debian/CVS/ScummVM+hack/common/singleton.h:63:
undefined reference to
`Common::Singleton<OSystem>::makeInstance()'
base/libbase.a(engine.o)(.text+0x1407): In function
`Z5errorPKcz':
c:/Privat/CygWin/home/torbjorn/Debian/CVS/ScummVM+hack/common/singleton.h:63:
undefined reference to
`Common::Singleton<OSystem>::makeInstance()'
base/libbase.a(gameDetector.o)(.text+0x5d6f): In function
`ZN12GameDetector16parseCommandLineEiPPc':
c:/Privat/CygWin/home/torbjorn/Debian/CVS/ScummVM+hack/common/singleton.h:63:
undefined reference to
`Common::Singleton<OSystem>::makeInstance()'
scumm/libscumm.a(scumm.o)(.text+0xe69b): In function
`Z19Engine_SCUMM_createP12GameDetectorP7OSystem':
c:/Privat/CygWin/home/torbjorn/Debian/CVS/ScummVM+hack/common/singleton.h:63:
undefined reference to
`Common::Singleton<OSystem>::makeInstance()'
scumm/libscumm.a(dialogs.o)(.text+0x2e2b): In function
`ZN5Scumm14MainMenuDialog4saveEv':
c:/Privat/CygWin/home/torbjorn/Debian/CVS/ScummVM+hack/common/singleton.h:63:
undefined reference to
`Common::Singleton<OSystem>::makeInstance()'
scumm/libscumm.a(dialogs.o)(.text+0x2e6d):c:/Privat/CygWin/home/torbjorn/Debian/CVS/ScummVM+hack/common/singleton.h:63:
more undefined references to
`Common::Singleton<OSystem>::makeInstance()' follow
make: *** [scummvm.exe] Error 1

I'm using GCC 3.2.3, which is admittedly somewhat old, but
that's what I get when I install the latest stable MinGW and
MSYS packages.

It stopped working some time today (January 7). It worked
yesterday.

by fingolfin, 15 years ago

Attachment: makeinstance.patch added

Yet another patch, maybe helps GCC 3.2.x?

comment:10 by fingolfin, 15 years ago

OK folks, what about this new patch, does it help Mingw? (and what does
it do to the MSVC warning, I wonder...)

comment:11 by fingolfin, 15 years ago

Should have done a full build before submitting this: I get
link errors with my patch, because of course now code for
makeInstance is created multiple times, causing a duplicate
symbol link error.

Anyway, still would be interesting to know what effect the
patch has on Mingw / MSVC

comment:12 by fingolfin, 15 years ago

erik,

I discovered that the SF.net compile farm shell server (!)
runs GCC 3.2.3, so I was able to do some testing with it :-).
I really believe that this is a compile error, but at least I
found a work around. Sadly, that work around breaks
linking with GCC 3.3 <sigh>.

Anyway: just move the implementation of OSystem's
makeInstance from system.cpp to system.h (replace the
declaration of it in there, for example). That made it link
just fine on the test system.

comment:13 by eriktorbjorn, 15 years ago

Yes, that seems to work for me too. (But don't worry, I
won't commit it. :-)

comment:14 by wjp, 15 years ago

If specializations cause trouble, it should also be possible
to do this with inheritance:

in Common::Singleton<T>::instance(), call T::makeInstance().
Make Common::Singleton<T>::makeInstance() the default 'new
T()' function, and make (for instance)
OSystem::makeInstance() the specialized version.

comment:15 by eriktorbjorn, 15 years ago

I can compile ScummVM with GCC 3.2.3 again. Perhaps the
recent header file changes helped somehow?

comment:16 by fingolfin, 15 years ago

Well, I implemented wjp's suggestion, that's all. And I tested it on GCC
3.2.3, too (see also the commit message).

I wonder how MSVC copes with this -- and if maybe we don't need to
turn that warning off anymore.

comment:17 by SF/h00ligan, 15 years ago

last wjp suggested patch - helps to re enable
"no definition for inline function" warning in MSVC
scummvm compiles well

comment:18 by fingolfin, 15 years ago

Resolution: fixed
Status: newclosed

comment:19 by digitall, 13 months ago

Component: Port: Linux
Note: See TracTickets for help on using tickets.