+ if (LangOpts.MicrosoftMode)
+ Builder.defineMacro("__ms_extensions__", 1);
if (!LangOpts.CPlusPlus) {
if (LangOpts.C11)
Builder.defineMacro("__STDC_VERSION__", "201112L");
Looks fine to me, though you would also have to convince the gcc authors
to do the same. (That is, if you still want to use recent gcc's...
+ if (LangOpts.MicrosoftMode)
+ Builder.defineMacro("__ms_extensions__", 1);
if (!LangOpts.CPlusPlus) {
if (LangOpts.C11)
Builder.defineMacro("__STDC_VERSION__", "201112L");
Looks fine to me, though you would also have to convince the gcc authors
to do the same. (That is, if you still want to use recent gcc's...
Well, the licence means we care a little less about gcc for obvious reasons.
Working with the llvm/clang community makes more sense for us.
Another option, which works right away, without having to modify any
compiler, is to just define it on the command line, e.g. use:
-fms-extensions -D__ms_extensions__
Let's make it clear, I'm not looking for a work-around, I'm looking for a way
to fix things out-of-the-box. Having a "by default" define means we can
fix the includes once and for all, and have clang -fms-extensions work
out-of-the-box without any configure magic.
People who deal with portable code know the pain of having some code that
doesn't work on YOUR system because it requires one extra step that the
people upstream don't know about, so you have to make a patch, pass it
upstream... and often have things fail again down the line because upstream
dropped your change because no-one there knew what it was for.
Or try to figure out where you can inject the patch in some crazy build
system you have never worked with, and that actively fights you all the
way...
Just to make it clear where I'm coming from, over the past few weeks,
I've fixed about 200 stupid configury bugs for clang, between the
stuff that put int main(int argc, char *argv) in configure.ac,
the stuff that asserts that $(CC) -Wwhatever works since the compiler
didn't error out, the crazy mingw build shell script that won't
insert -fgnu89-inline just for the gcc3.4 bootstrap stage, or
python2.7 that by default disables multibyte functions required for
__bsd_locale_fallbacks.h ...
I think we already have _WCHAR_T_DEFINED for this:
if (LangOpts.MicrosoftExt) {
if (LangOpts.WChar) {
// wchar_t supported as a keyword.
Builder.defineMacro(“_WCHAR_T_DEFINED”);
Builder.defineMacro(“_NATIVE_WCHAR_T_DEFINED”);
}
}
Nope. I'm talking about the __wchar_t type.
In my case, even a C language "hello world" won't compile:
nausicaa$ clang -fms-extensions a.c
In file included from a.c:1:
In file included from /usr/include/stdio.h:43:
In file included from /usr/include/sys/_types.h:37:
/usr/include/machine/_types.h:132:15: error: cannot combine with previous 'int'
declaration specifier
typedef int __wchar_t;
^
1 error generated.
but actually based on the existing code, I would suggest following
the pattern.
my main concern would be: are names with ___ readable, or is there
a suggestion for better names ?
I’d rather not add new pre-defined macros if we can avoid it. There are already too many. You can probably detect this situation with: #if defined(_MSC_EXTENSIONS) && !defined(_WCHAR_T_DEFINED)
Or, what's the problem with moving _MSC_EXTENSIONS to be for all targets.
Look, this stuff is highly specific, I doubt many people are going to
use -fms-extensions who don't know what they're doing.
E.g., the exposure to _MSC_EXTENSIONS will be minimal.
Having a single define will mean replacing (for us) #ifndef __cplusplus
typedef int __wchar_t; #endif
with #if !defined(__cplusplus) && !defined(_MSC_EXTENSIONS)
typedef int __wchar_t; #endif
which is fairly acceptable.
Playing with __is_identifier is rather more awful, because there is no
simple construct, we can't simply write: #if !defined(__cplusplus) || __is_identifier(__wchar_t)
because this *also* has to work with other compilers, some of which
do not understand __is_identifier at all, so it can't be a single test
then.
It would become some atrocity like
#if !defined(__cplusplus)
# if defined(__is_identifier)
# if __is_identifier(__wchar_t)
# else
typedef int __wchar_t;
# endif
# else
typedef int __wchar_t;
# endif #endif
And that has about zero chance to ever be committed in our tree,
especially since this is in a md _types.h file, so this construct x10 times
in the tree.
>
>
> >
> >Â Â I'd rather not add new pre-defined macros if we can avoid it.
> There are
> >Â Â already too many. You can probably detect this situation
> with:
> >Â Â #if defined(_MSC_EXTENSIONS) && !defined(_WCHAR_T_DEFINED)
> >
> Was this added recently ?
> There is no _MSC_EXTENSIONS in the clang I'm using...
>
> Looks like we only define it for Windows targets.
> How about __is_identifier?
> $ cat t.cpp
> #if !__is_identifier(__wchar_t)
> #error "have __wchar_t"
> #else
> #error "no __wchar_t"
> #endif
Nope, that will trigger errors on non clang compilers.
And that leads to more or less the same shitty set of conditionals I already
posted about... as opposed to using _MSC_EXTENSIONS, which by the way is
already defined on windows, is unlikely to ever collide with anything, and
would only be on with -fms-extensions, which is not the most common option
ever...
In every single case, __is_identifier will replace a one-liner
with several lines of nested conditionals... not a good thing.
Date: Fri, 12 May 2017 08:53:53 -0700
From: Reid Kleckner via llvm-dev <llvm-dev@lists.llvm.org>
Moving to cfe-dev to discuss clang things.
I think we already have _WCHAR_T_DEFINED for this:
if (LangOpts.MicrosoftExt) {
if (LangOpts.WChar) {
// wchar_t supported as a keyword.
Builder.defineMacro("_WCHAR_T_DEFINED");
Builder.defineMacro("_NATIVE_WCHAR_T_DEFINED");
}
}
Unfortunately those aren't defined if you use -fms-extensions in C
mode as LangOpts.WChar is false in that case.
I do not want to define _MSC_EXTENSIONS more than we already do, and I
don't think we should add more pre-defined macros when we already have
these nicely factored feature test macros that don't pollute the global
namespace. The "simple solution" is not a good solution.
I believe our use of _MSC_EXTENSIONS is wrong anyway. It is controlled by
MSVC's /Ze and /Za flags, which do not relate to our -fms-extensions flag.
We reserve the right to provide __wchar_t in modes other than -fms-extensions in future, or to sometimes not provide it in that mode, so using _MSC_EXTENSIONS for this is simply a mistake.
If you want to detect whether we provide a builtin __wchar_t, __is_identifier is the mechanism we provide for that. It doesn’t make sense for us to add another.
