melver 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)
----------------
ojeda wrote:
> 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!
I think realistically we have to pick one, and that's the one we keep for all
eternity. :-)
Because if we deprecate/remove something, some codebases would require version
checks, which is a non-starter again. Not every project is on top of what their
compilers deprecates/removes. (And, unlike the Linux kernel, some codebases
just never upgrade their compiler requirements, but still expect newer
compilers to work.)
So if we want consistency with other sanitizers, it has to be `__has_feature()`.
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
