[cfe-commits] [PATCH RFC] Stop defining __GNUC__ for MSVC builds

[Aaron Ballman]

On Fri, Mar 9, 2012 at 2:23 PM, Douglas Gregor <dgregor at apple.com> wrote:
>
> On Mar 6, 2012, at 5:31 AM, Aaron Ballman wrote:
>
>> On Mon, Mar 5, 2012 at 10:32 PM, Douglas Gregor <dgregor at apple.com> wrote:
>>>
>>> On Mar 2, 2012, at 4:16 PM, Aaron Ballman wrote:
>>>
>>>> There are a lot of references on the web which relate __GNUC__ to GCC
>>>> for compiler discovery
>>>> (<http://sourceforge.net/apps/mediawiki/predef/index.php?title=Compilers#GCC_C.2FC.2B.2B>,
>>>> <http://www.ocf.berkeley.edu/~pad/tigcc/doc/html/cpp_SEC15_GNUC.html>,
>>>> etc).  This translates into a fair number of cross-compiler projects
>>>> with code like this:
>>>>
>>>> #if defined(__GNUC__)
>>>>  // Do GCC things
>>>> #elif defined(_MSC_VER)
>>>>  // Do MSVC things
>>>> ...
>>>> #endif
>>>>
>>>> This currently leads to problems when compiling with Clang because
>>>> __GNUC__ (and friends) are always defined, even when compiling for MS
>>>> compatibility (PR 11790).  I've attached a patch which addresses this
>>>> by only defining __GNUC__ et al when MSVC mode is not set.  This means
>>>> code like the above will work as expected.
>>>>
>>>> Thoughts?
>>>
>>> -  // Currently claim to be compatible with GCC 4.2.1-5621.
>>> -  Builder.defineMacro("__GNUC_MINOR__", "2");
>>> -  Builder.defineMacro("__GNUC_PATCHLEVEL__", "1");
>>> -  Builder.defineMacro("__GNUC__", "4");
>>> -  Builder.defineMacro("__GXX_ABI_VERSION", "1002");
>>> +  if (!LangOpts.MSCVersion) {
>>>
>>> This should probably just check LangOpts.MicrosoftMode.
>>>
>>> I'm not sure about the general approach, though. Even when compiling in Microsoft-compatible mode, Clang is still much more GCC-like than MSVC-like. I would venture a guess that
>>>
>>>        // Do GCC things
>>>
>>> is far more likely to be standards-conforming code that Clang will accept than
>>>
>>>        // Do MSVC things
>>>
>>> especially when dealing with template libraries in C++.
>>
>> I think your guess may be off base.  A lot of those things are
>> includes.  At work, we have several sources like:
>>
>> #if defined( _MSC_VER )
>> #include <Windows.h>
>> #elif defined( __GNUC__ )
>> #include <gtk/gtk.h>
>> #endif
>
> So the assumption is that _MSC_VER/__GNUC__ are used more as a platform indicator than a compiler indicator. Broken as that is, I agree that this could be the common case.
>
>> I think the correct thing to do is define __clang__ and MSVC in
>> MS-compat mode, like we already do for __clang__ and __GNUC__ for GNU
>> mode.  It seems like a reasonable pattern.  If someone cares about
>> clang, that's turned on appropriately, as is the compiler they're
>> emulating.
>
> Fair enough. Patch is fine with the change to check for LangOpts.MicrosoftMode.
>
>> But I do wonder what should happen with -std=gnu++98
>> -fms-compatibility as a combination.  Seems almost like an error?
>
>
> I think it's fine to have GNU extensions and MS extensions available at the same time. -fms-compatibility essentially takes precedence when it is provided.

Thank you for the review -- I've committed (with the MS mode changes) in r152512

~Aaron