ojeda added inline comments.
================
Comment at: clang/include/clang/Basic/Features.def:52
LangOpts.Sanitize.hasOneOf(SanitizerKind::Undefined))
+FEATURE(coverage_sanitizer, LangOpts.SanitizeCoverage)
FEATURE(assume_nonnull, true)
----------------
melver wrote:
> aaron.ballman wrote:
> > I think this is likely fine, but wanted to call it out explicitly in case
> > others had opinions.
> >
> > `FEATURE` is supposed to be used for standard features and `EXTENSION` used
> > for Clang extensions. This is an extension, not a standard feature, so it's
> > wrong in that way. However, it's following the same pattern as the other
> > sanitizers which is consistent. I think consistently wrong is better than
> > inconsistently right for this case, but I have no idea how others feel.
> Yes, you are correct of course, and I was pondering the same thing.
>
> In the end I'd like all sanitizers be queryable via `__has_feature()` and not
> have this be the odd one out requiring `__has_extension()` as that's also
> going to lead to confusion/errors on the user side.
Perhaps add both, deprecate `__has_feature()` for non-standard features like
these ones, and remove them after a couple releases? :)
Regardless of the way, //any// is better than a version check, so thanks!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103159/new/
https://reviews.llvm.org/D103159
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
