Issue 4846: Py_UNICODE_ISSPACE causes linker error (original) (raw)

Created on 2009-01-05 16:09 by ishimoto, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue4846.diff belopolsky,2009-01-05 17:54
Messages (8)
msg79160 - (view) Author: Atsuo Ishimoto (ishimoto) * Date: 2009-01-05 16:09
When I use Py_UNICODE_ISSPACE() in my C++ extension, I got following error. test.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) unsigned char const * const _Py_ascii_whitespace" (__imp_?_Py_ascii_whitespace@@3QBEB) referenced in function "struct _object * __cdecl fff(struct _object *,struct _object *)" (?fff@@YAPAU_object@@PAU1@0@Z) Py_ascii_whitespace defined in unicodeobject.h should be enclosed by 'extern "C" {' block for C++ support. Tested with Python 2.6.1/VS2008 express.
msg79163 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2009-01-05 16:44
On 2009-01-05 17:09, Atsuo Ishimoto wrote: > New submission from Atsuo Ishimoto <ishimoto@gembook.org>: > > When I use Py_UNICODE_ISSPACE() in my C++ extension, I got following error. > > test.obj : error LNK2019: unresolved external symbol > "__declspec(dllimport) unsigned char const * const _Py_ascii_whitespace" > (__imp_?_Py_ascii_whitespace@@3QBEB) referenced in function "struct > _object * __cdecl fff(struct _object *,struct _object *)" > (?fff@@YAPAU_object@@PAU1@0@Z) > > Py_ascii_whitespace defined in unicodeobject.h should be enclosed by > 'extern "C" {' block for C++ support. Agreed. There already is such a block in unicodeobject.h, so perhaps a little rearranging could do the trick.
msg79177 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2009-01-05 17:54
I believe attached .diff should fix the problem, but I don't have access to the affected platform (Windows?), so I did not test it. I also wonder whether it would be more appropriate to redefine PyAPI_* macros to declare API symbols with extern "C" when compiled under C++? This seems to be a better approach than cluttering all header files with #ifdef __cplusplus and worrying about the scope of extern "C" {} wrappers. Note that I considered whether _Py_ascii_whitespace declaration should be done inside if !defined(HAVE_USABLE_WCHAR_T) .. directive and concluded that it should not. My reasoning is as follows: _Py_ascii_whitespace must be defined unconditionally in the .c file because when python core is compiled, it is not known how an extension module would define HAVE_USABLE_WCHAR_T. While it is not strictly necessary to bring _Py_ascii_whitespace definition in an extension module that has "USABLE_WCHAR_T", there is little to be gained from hiding it because it is visible to the linker. Thus for the common uses there is little practical difference between the two options but unconditional declaration requires less code. On the other hand in the context of , having unconditional declaration would allow removing extern "C" wrappers from unicodeobject.c without loosing C++ compilability of python core.
msg79178 - (view) Author: Ulrich Eckhardt (eckhardt) Date: 2009-01-05 18:04
You beat me by 10 minutes, Alexander, otherwise I'd have had a similar patch, except moving the declaration. I agree with your rationale though, so yours is even better. Concerning including the extern "C" in the PyAPI_* macros, I had the same idea and think that that it would unclutter code a bit.
msg79185 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2009-01-05 19:10
Not that it would matter on a two-line patch, but I find it useful to add myself to the "nosy" list without any comments as so as I find the issue interesting even if I have nothing to contribute at the moment. This way I am more likely to notice when someone posts a patch before duplicating the effort. > Concerning including the extern "C" in the PyAPI_* macros, I had the > same idea and think that that it would unclutter code a bit. This would be a bigger change and getting it right would require expertise with non-standard declarations (__declspec(..) etc) that I don't have. Ulrich, you seem to be a Windows expert, can you suggest a patch?
msg79190 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2009-01-05 19:45
Fixed in r68344 on trunk. Backport and forward-port candidate.
msg79192 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2009-01-05 19:49
On 2009-01-05 18:54, Alexander Belopolsky wrote: > I also wonder whether it would be more appropriate to redefine PyAPI_* > macros to declare API symbols with extern "C" when compiled under C++? > This seems to be a better approach than cluttering all header files with > #ifdef __cplusplus and worrying about the scope of extern "C" {} > wrappers. This is standard practice, but of course has to be done with some care. In general, only macro and type definitions should live outside the extern "C" sections, #includes must live outside those sections. Changing the PyAPI_* macros would help with this, but I'm not sure whether that's a catch-all solution. If it's not, then it's better to stick with the extern "C" sections.
msg79193 - (view) Author: Ulrich Eckhardt (eckhardt) Date: 2009-01-05 19:57
Actually, providing the patch for PyAPI_Data/PyAPI_Func wouldn't be so hard, but there are lots of headers that need to be changed, i.e. all their existing extern "C" clauses removed. What I would do is build the Python shared library as it is and then use e.g. depends.exe (www.dependencywalker.com) to make sure the symbols before and after are the same. That's a different issue though and it's not pressing. Is it possible to clone this issue and make it a feature request (Include extern"C" in PyAPI_* macros)?
History
Date User Action Args
2022-04-11 14:56:43 admin set github: 49096
2009-01-05 19:57:16 eckhardt set messages: +
2009-01-05 19:49:40 lemburg set messages: +
2009-01-05 19:45:16 lemburg set status: open -> closedkeywords: + 26backport, - patchmessages: + versions: + Python 2.7
2009-01-05 19:10:44 belopolsky set messages: +
2009-01-05 18:04:06 eckhardt set nosy: + eckhardtmessages: +
2009-01-05 17:54:51 belopolsky set files: + issue4846.diffkeywords: + patchmessages: +
2009-01-05 16:54:52 belopolsky set nosy: + belopolsky
2009-01-05 16:44:56 lemburg set nosy: + lemburgmessages: +
2009-01-05 16:09:19 ishimoto create