[PATCH] D134831: [Clang][Sema] Add -Wcast-function-type-strict

[Kees Cook via Phabricator via cfe-commits]

kees added a subscriber: nathanchance.
kees added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8686
+def warn_cast_function_type_strict : Warning<warn_cast_function_type.Text>,
+  InGroup<CastFunctionTypeStrict>, DefaultIgnore;
 def err_cast_pointer_to_non_pointer_int : Error<
----------------
aaron.ballman wrote:
> samitolvanen wrote:
> > nickdesaulniers wrote:
> > > I don't think we need a new group for a warning that only contains one diagnostic kind?
> > > I don't think we need a new group for a warning that only contains one diagnostic kind?
> > 
> > I might have misunderstood something, but we seem to have plenty of groups with only one diagnostic kind. Is there a way to add a new warning flag without adding a diagnostic group? Most users of `-Wcast-function-type` wouldn't want to enable the stricter version, so I would prefer to keep this separate.
> > 
> > I did notice that some warnings don't define the group in DiagnosticGroups.td, but specify it directly here. For example, `InGroup<DiagGroup<"argument-outside-range">>`. I'm not sure if there are any benefits in doing so.
> Typically we only define a group in DiagnosticGroups.td when the group is going to be used by more than one diagnostic, otherwise we prefer using `InGroup<DiagGroup<"whatever">>` to form one-off diagnostic groups.
> 
> However, in this case, I am wondering if we want to add `CastFunctionTypeStrict` to be a subgroup of `CastFunctionType` so that users who enable `-Wcast-function-type` get the stricter checking by default, but still have a way to disable the stricter checking if it's too noisy for them?
> However, in this case, I am wondering if we want to add `CastFunctionTypeStrict` to be a subgroup of `CastFunctionType` so that users who enable `-Wcast-function-type` get the stricter checking by default, but still have a way to disable the stricter checking if it's too noisy for them?

I'd be for that. It'll be very noisy for the Linux kernel, but they are all instances we need to fix.

cc @nathanchance


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134831/new/

https://reviews.llvm.org/D134831