[PATCH] D68410: [AttrDocs] document always_inline
ojeda added a comment. Note that the latest GCC docs (rather than 4.1.2) are at: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html For reference, this was triggered from: https://lore.kernel.org/lkml/cakwvodm_gouedjayxtqctuvdl+9vwvfeofhv06mlmyva75c...@mail.gmail.com/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68410/new/ https://reviews.llvm.org/D68410 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D68410: [AttrDocs] document always_inline
ojeda added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:5498 + +See also `the MSDN Inline docs`_, `the GCC Common Function Attribute docs`_, +and `the GCC Inline docs`_. MSDN is gone now, Microsoft Docs is the new one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68410/new/ https://reviews.llvm.org/D68410 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103159: [Clang] Enable __has_feature(coverage_sanitizer)
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
[PATCH] D103159: [Clang] Enable __has_feature(coverage_sanitizer)
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
[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs
ojeda added a comment. Thanks Nick for the ping. Indeed, `rustc` allows to use a "target specification file" to create new targets that are not built-in, as a JSON file. They are unstable though, since they are fairly tied to LLVM, and I think the Rust compiler team prefers to stabilize flags instead. However, it would definitely be great for users and build systems if GCC, Clang and `rustc` could get together and decide on a new, common config file, even if it just covered the target specification. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134337/new/ https://reviews.llvm.org/D134337 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D139395: Add support for integer types normalization
ojeda added subscribers: nathanchance, nickdesaulniers. ojeda added a comment. Cc @nickdesaulniers @nathanchance Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139395/new/ https://reviews.llvm.org/D139395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
