Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Unconditional gnu extension usage in stddef.h breaks VS2008 headers #5460

Closed Quuxplusone closed 13 years ago

Quuxplusone commented 13 years ago
Bugzilla Link PR7796
Status RESOLVED FIXED
Importance P normal
Reported by Per Lindén (per@lumai.se)
Reported on 2010-08-03 05:20:51 -0700
Last modified on 2010-08-07 14:25:10 -0700
Version trunk
Hardware PC Windows XP
CC clattner@nondot.org, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
When using clang to compile a trivial C++ program using <iostream> in the
Visual Studio 2008 environment, clang/2.8/include/stddef.h unconditionally
#defines NULL to __null. This causes a problem in the VS2008 header <wchar.h>,
where the function

__inline int __CRTDECL mbsinit(_In_opt_ const mbstate_t *_P)
        {return (_P == NULL || *_P == 0); }

expands to

__inline int mbsinit( const mbstate_t *_P)
        {return (_P == || *_P == 0); }

Test case:

#include <iostream>

int main(int argc, char **argv)
{
    cout << "Hello C++ world" << endl;
    return 0;
}

Resulting error:

D:\Temp\llvm>clang++ -fno-exceptions test.cpp -ferror-limit=2
In file included from test.cpp:1:
In file included from c:\Program Files\VStudio 9.0\VC\include/iostream:6:
In file included from c:\Program Files\VStudio 9.0\VC\include/istream:6:
In file included from c:\Program Files\VStudio 9.0\VC\include/ostream:6:
In file included from c:\Program Files\VStudio 9.0\VC\include/ios:6:
In file included from c:\Program Files\VStudio 9.0\VC\include/xlocnum:9:
In file included from c:\Program Files\VStudio 9.0\VC\include/streambuf:6:
In file included from c:\Program Files\VStudio 9.0\VC\include/xiosbase:6:
In file included from c:\Program Files\VStudio 9.0\VC\include/xlocale:8:
In file included from c:\Program Files\VStudio 9.0\VC\include/stdexcept:7:
In file included from c:\Program Files\VStudio 9.0\VC\include/xstring:6:
In file included from c:\Program Files\VStudio 9.0\VC\include/xmemory:9:
In file included from c:\Program Files\VStudio 9.0\VC\include/xutility:7:
In file included from c:\Program Files\VStudio 9.0\VC\include/utility:6:
In file included from c:\Program Files\VStudio 9.0\VC\include/iosfwd:8:
In file included from c:\Program Files\VStudio 9.0\VC\include/cwchar:13:
c:\Program Files\VStudio 9.0\VC\include/wchar.h(1209) :  error: expected
expression
        {return (_P == NULL || *_P == 0); }
                            ^
Quuxplusone commented 13 years ago

stddef.h path should be "/cfe/trunk/lib/Headers/stddef.h". The change to use __null seems to have been made in commit 60355.

Quuxplusone commented 13 years ago

I don't get it, does a MSVC #define null to nothing? If so, does adding a #undef null to stddef.h fix it?

Quuxplusone commented 13 years ago
(In reply to comment #2)
> I don't get it, does a MSVC #define __null to nothing?  If so, does adding a
> #undef __null to stddef.h fix it?

Yup. That fixes it. I somehow missed the MSVC file "sal.h" that defined __null
and a slew of other __* macros. (I still wonder why a gnu extension such as
__null is used by default in the clang header. I was expecting something like
"#ifdef __GNUC__" around it.)

Should I close this bug now or should any of the issues above be patched?

---

My intention is to file more bugs concerning building MSVC code with clang. The
closer to a drop-in replacemant clang becomes, the happier I get. Hopefully I
will be more thorough next time tho...
Quuxplusone commented 13 years ago

Fixed in r110161

Quuxplusone commented 13 years ago
After further investigation I realized that this is not enough in the general
case. __null is redefined to <nothing> by the MSVC headers _after_ the
inclusion of stddef.h when including some other headers, such as when compiling
random.cpp from libcxx. (which pulls in sal.h after stddef.h)

I propose something along the lines of

#undef NULL
#ifdef __cplusplus
#ifdef __GNUC__
#define NULL __null
#else
#define NULL 0
#endif
#else
#define NULL ((void*)0)
#endif

Alternative solutions would be special-casing MSVC using #ifdef _MSC_VER
(ugly...?) or just not redefining NULL if it is already defined by system
headers. Is there a reason for always defining NULL here? At least the MSVC env
works fine without doing that.

Is there any interest in reopening this bug?
Quuxplusone commented 13 years ago

Defining NULL to 0 is not correct. Please find out what VC++'s headers define NULL to.

Quuxplusone commented 13 years ago
From MSVC <stdio.h> (replicated in several other files):

#ifndef NULL
#ifdef __cplusplus
#define NULL    0
#else
#define NULL    ((void *)0)
#endif
#endif

What's the correct definition then, if not using GNU extensions? (Stroustrup
says http://www2.research.att.com/~bs/bs_faq2.html#null)
Quuxplusone commented 13 years ago

Ok, I'm fine changing it to a #define to 0, but what should the #ifdef around it be? Remember that this file is being preprocessed by clang and windows, not MSVC, so GNUC is set and MSC_VER isn't.

Quuxplusone commented 13 years ago

Hmm. Tricky. Considering the MSVC-related discussion on clang.devel lately this seems to small an issue to pursue further atm. I don't see any easy not-looking-horrible fix anyway before the fate of _MSC_VER is decided. Sorry for the noise and thanks for the effort! I will post a patch to the list if a good solution comes up later.