Differential D3034
Add a test to distinguish between reserved tokens and normal identifiers Authored by ygao on Mar 10 2014, 3:07 PM.
Details Hi,
Diff Detail Event TimelineComment Actions Personally, I think this is a dangerous idea. This page lists some of the features that code can expect to see when _MSC_EXTENSIONS is enabled. Most notably, "Out of Class Definition of static const Integral (or enum) Members" is permitted which our -fms-extensions should not enable. This misfeature comes up a lot and _MSC_EXTENSIONS is often how people check for it. Comment Actions Hi David,
Comment Actions Consider the following: struct S {
static const int i = 42;
};
const int *use() { return &S::i; }Compiling with clang -cc1 -x c++ -S -o - -emit-llvm -triple i686-linux-gnu does not produce a definition of i. Compiling with clang -cc1 -x c++ -S -o - -emit-llvm -triple i686-pc-win32 produces i defined as @"\01?i@S@@2HB" = linkonce_odr constant i32 42, align 4. This matches MSVC's behavior: ?i@S@@2HB We won't define _MSC_EXTENSIONS unless we are targeting the MSVC ABI and enable the MSVC-specific keywords with -fms-extensions. Comment Actions Hi David,
Comment Actions In clang, -fms-extensions controls the conforming extensions. This is not a conforming extension. Comment Actions Hi David, To summarize our discussion:
not under control of -fms-extensions
but not the ABI-specific behaviors expected of "cl.exe /Ze" Since it is not feasible to define _MSC_EXTENSIONS for -fms-extensions, is
Comment Actions I think it would be better to use __has_extension instead: http://clang.llvm.org/docs/LanguageExtensions.html#has-feature-and-has-extension Comment Actions
We're trying to arrange that wchar_t is always available, but Comment Actions I don't think I understand the problem you are trying to solve. If you want to determine whether or not MSVC keywords are enabled, I don't see why it wouldn't be unreasonable to add a msvc_keywords argument for __has_extension. Comment Actions
We provide a header file to our licensees.
If "MSVC keywords" includes "predeclared types" I guess that would work. Comment Actions Hi David,
Comment Actions This change looks good to me, I'd like @rsmith's opinion on the spelling of msvc_keywords before you commit it. If/when you do commit this, please make sure to commit this with a different message than the current one.
Comment Actions Hi David, Richard,
Comment Actions The patch looks good. What should we do about the iso646 alternative tokens (and, or, compl, ...)? These are technically not keywords (and this implementation of __has_keyword will return 0 for them), but they are also not usable as identifiers, and it would be useful for code to detect them (because in MSVC compat mode we need to treat them weirdly). Comment Actions I do not know. Are these alternative tokens pretty much set or are we expecting to add more in the future? Are Comment Actions
In that case, something like the following should return true if the operator bool isSpecialOperatorToken(IdentifierInfo *II) if (LangOpts.MSVCCompat) return false; return II->isCPlusPlusOperatorKeyword(); }
True, but I would prefer if it is called something else. Comment Actions Hi,
| ||||||
I'm opposed to having something so vague and general. What happens when we add support for more MS keywords?
Please make the extension check be a test for a specific extension.