[cfe-dev] [clang][RFC] -Wmissing-prototypes: make suggested fix more generic?

[Carlos Galvez via cfe-dev]

Thanks for the feedback! Definitely, anonymous namespaces should only be
suggested in C++. That might complicate the logic and testing. An
alternative solution would be to not suggest any fix and provide a more
generic message, like "this function has external linkage but the
declaration is missing, please add it or make the function have internal
linkage if it's only meant to be used in this translation unit".

>  the fix cannot be automatically applied by an engine
Interesting, I wasn't aware there are tools that automatically fix compiler
warnings, is that an LLVM tool? If that's the case, I suppose removing the
fix-it would cause trouble to people relying on it. What concerns me is the
compiler implicitly making a style decision for the user when it has two
valid options to choose from - I think this fits better in e.g. clang-tidy.
The compiler can have one responsibility - detect functions that may
accidentally have external linkage, and then the linter would be in charge
of automatically applying one style or the other. A middle point could be
to keep the fix-it only for C (since "static" is the only choice) and not
have it for C++ (just have the generic message as above).

>  another solution for your situation might be a clang-tidy check to
enforce your company's coding guidelines
Nice idea, this works too! In order to not duplicate code, I was thinking
to have a check that enforces static -> anonymous namespace, still leaving
the responsibility of "detecting incorrect external linkage" to clang. This
might generate some developer friction though, as they first stumble across
the clang warning, fix it with "static" and later on in CI get the
clang-tidy warning telling them to put it in the anonymous namespace.

/Carlos

On Fri, Dec 10, 2021 at 7:04 PM Aaron Ballman <aaron at aaronballman.com>
wrote:

> On Fri, Dec 10, 2021 at 8:25 AM Carlos Galvez via cfe-dev
> <cfe-dev at lists.llvm.org> wrote:
> >
> > Hi,
> >
> > We want to enable the warning -Wmissing-prototypes to enforce internal
> linkage as much as possible. It works great, with one caveat - it suggests
> to use "static". Internal linkage can also be achieved with anonymous
> namespaces and Clang will not issue a warning.
> >
> > Now, the question as to what to use (static vs anonymous namespaces) is
> mostly a style issue (a "Microscopic Detail" in the LLVM Coding
> Guidelines). At our company we have decided to prefer anonymous namespaces
> over static, so we wouldn't want Clang to start suggesting people
> (especially junior devs that trust the tools blindly) to use static.
> >
> > Therefore I wanted to ask here - would it be possible to make the
> suggestion more generic, maybe displaying both alternatives? Best would be
> making this user-configurable but I'm guessing that's not a trivial thing
> to do.
> >
> > I'm happy to implement the changes given some pointers, but I want to
> check here first if you'd agree with it.
>
> There are a couple of things to keep in mind.
>
> The current fix-it is valid in both C and C++, but anonymous
> namespaces are C++ only and so that suggestion would be inappropriate
> to make in C.
> Multiple fixits means the fix cannot be automatically applied by an
> engine, whereas the current behavior can.
> Clang does not have a way to configure diagnostic options aside from
> the command line. I don't think it would be worth the effort to
> support something like `-Wmissing-prototypes=prefer-static` or
> something like that to parameterize this diagnostic.
>
> I don't think any of these are necessarily a show-stopper for the
> proposed functionality, but they're worth keeping in mind. However,
> for me personally, I think the current functionality is sufficient (it
> works in all language modes, it's automatable as a fix-it, etc) and
> that a perhaps another solution for your situation might be a
> clang-tidy check to enforce your company's coding guidelines (which
> should hopefully integrate nicely into a CI pipeline for you, can also
> automatically apply fix-its, can be tailored to your specific needs,
> etc).
>
> ~Aaron
>
> >
> > Thanks!
> > /Carlos
> >
> >
> >
> > _______________________________________________
> > cfe-dev mailing list
> > cfe-dev at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20211211/de657e24/attachment.html>