[cfe-dev] [PATCH] Warn when an anonymous warning group is used more than once

[Richard Smith]

On Wed, Jan 9, 2013 at 5:19 PM, Jordan Rose <jordan_rose at apple.com> wrote:
> [+cfe-dev, -cfe-commits for the new "coding" convention]
>
> Hi, everyone. One thing that's bothered me about our diagnostic TableGen
> files is the anonymous use of warning groups:
>
> def warn_foo : Warning<"foo is bad">, InGroup<DiagGroup<"foo">>;
> def warn_foo_cxx : Warning<"foo is even worse in C++">,
> InGroup<DiagGroup<"foo">>;
>
> If we ever want to put the warning groups in a hierarchy, or change the
> name, there's a big chance we'll miss one.
>
> So, I wrote a new warning for clang-tblgen, which even has a fixit if the
> group already has a name:
>
> warning: group 'foo' is referred to anonymously
> def warn_foo : Warning<"foo is bad">, InGroup<DiagGroup<"foo">>;
>                                       ~~~~~~~~^~~~~~~~~~~~~~~~~
>                                       InGroup<Foo>
>
> And then fixed all the warnings, with the intent that we make this a new
> style requirement going forwards: groups that cover more than one diagnostic
> must be named.
>
>
> Sean has suggested making this an error instead of a warning, since we go
> for a warning-free build anyway. I'm not sure why I thought a warning was
> preferable; we'd obviously make anyone fix their code before committing if
> they triggered the warning. The only thing I can think of is that it might
> be better for quick testing when transitioning from one diagnostic to
> another, or when trying out a new diagnostic under an existing flag.
>
> Opinions? I'm fine either way.

Making this an error sounds fine to me.