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:
> 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()`.
Agreed, any friction on updates is bad for users and will annoy someone
somewhere.
Having said that, any serious project migrating to a new toolchain needs to
revalidate regardless. And, after all, these are non-standard features. So in
practice I do not think it would matter too much if the deprecation notice is
long enough (several years).
But I may be saying something completely stupid, since I do not even know if
Clang promises forever-stability for options, like `rustc` does.
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
