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) ---------------- 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. 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