aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticGroups.td:190
: DiagGroup<"deprecated-dynamic-exception-spec">;
+def DeprecatedHasBuiltins : DiagGroup<"deprecated-has-builtins">;
def DeprecatedImplementations :DiagGroup<"deprecated-implementations">;
----------------
I wonder if we want to rename this to `deprecated-builtins` so it applies to
any builtin we elect to deprecate, not just ones that start with `has`. WDYT?
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5561
+def warn_deprecated_has_builtins : Warning<
+ "the %0 compiler builtin is deprecated from C++11 onwards. Use %1 instead.">,
+ InGroup<DeprecatedHasBuiltins>;
----------------
erichkeane wrote:
> cor3ntin wrote:
> > Should we say something like "and will be removed in the future"?
> >
> > "%0 is deprecated from C++11 onward. Use %1 instead." might be sufficient
> >
> >
> Diagnostics arent sentences. Also, we wouldn't say "C++11 onward", we can
> just say C++11. I might suggest:
>
> `builtin %0 is deprecated in C++11, use %1 instead`
>
> BUT @aaron.ballman always does great at this level of bikeshedding.
>
I think we might want to rename this to `warn_deprecated_builtin` and make it
slightly more general. I think we want to drop the mention of C++11 because the
documentation says these are deprecated (not deprecated in a specific version)
and the replacement APIs are all available pre C++11 anyway (at least in
Clang). How about: `builtin %0 is deprecated; use %1 instead`?
================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:5400-5401
+ SourceLocation KWLoc) {
+ if (!S.getLangOpts().CPlusPlus11)
+ return;
+
----------------
I think we should always warn on these, not just in C++11.
================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:5426-5428
+ case UTT_HasTrivialDefaultConstructor:
+ Replacement = TT_IsTriviallyConstructible;
+ break;
----------------
This one is not documented as being deprecated (or documented at all). I think
it's reasonable to deprecate it, but it should probably be documented in the
language extensions page.
================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:5433
+ // FIXME: GCC don't implement __is_trivially_destructible: Warning for this
+ // might be too noisy for now.
+ // case UTT_HasTrivialDestructor:
----------------
I'd like to know how plausible that "might" is -- Clang flags it as being
deprecated, so it seems very reasonable to diagnose it as such. These
diagnostics won't be triggered from system headers by default anyway, so I'm
not certain if this will be too noisy in practice.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129170/new/
https://reviews.llvm.org/D129170
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
