erik.pilkington added inline comments.
================ Comment at: clang/include/clang/Basic/Features.def:121 FEATURE(objc_class_property, LangOpts.ObjC) +FEATURE(objc_c_static_assert, true) +FEATURE(objc_cxx_static_assert, LangOpts.CPlusPlus11) ---------------- thakis wrote: > erik.pilkington wrote: > > We should only claim `_Static_assert` is a feature in languages where its > > actually a formal language feature (C11), you can also use EXTENSION to > > indicate what languages we accept it in (see the comment in the file > > header). > Sorry, I don't follow. clang accepts `_Static_assert` in C89. You're saying > you want `__has_feature(objc_c_static_assert)` to be false there? I'm > guessing because -pedantic warns on it? > > But if we do this, there's no way to differentiate clang-in-std=c89 mode with > this patch from clang-without-this-patch, is there? Is that what you want? > i.e. replace `true` with `LangOpts.C11`? > > (For now, I updated the test to check the __has_feature() is true in c89 too.) > > (That seems like another argument to restore the single feature to me since > this working in objc is orthogonal on if the language version supports static > asserts.) > Sorry, I don't follow. clang accepts _Static_assert in C89. You're saying you > want __has_feature(objc_c_static_assert) to be false there? I'm guessing > because -pedantic warns on it? Yeah, `__has_feature` is meant to check for standard language features. `__has_extension` checks whether clang will accept it, regardless of what respective standard says. So `_Static_assert` here is a "standard" language feature in ObjC + C11 and an extension in ObjC + C99. I'm suggesting you write this like: ``` FEATURE(objc_c_static_assert, LangOpts.C11) FEATURE(objc_cxx_static_assert, LangOpts.CPlusPlus11) EXTENSION(objc_c_static_assert, true) ``` Just like we do now for c_static_assert in general: ``` FEATURE(c_static_assert, LangOpts.C11) EXTENSION(c_static_assert, true) ``` That way, `__has_extension(objc_c_static_assert)` is always true (with a new compiler), and `__has_feature` only deals with standard language features. Does that make sense? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59223/new/ https://reviews.llvm.org/D59223 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
