[PATCH] D73852: [clang] detect switch fallthrough marked by a comment (PR43465)

Wed Feb 12 05:54:44 PST 2020

thakis added a comment.

In D73852#1872044 <https://reviews.llvm.org/D73852#1872044>, @aaron.ballman wrote:

> In D73852#1872000 <https://reviews.llvm.org/D73852#1872000>, @thakis wrote:
>
> > I think giving comments a semantic meaning is a bad idea. Do we really have to do this?
>
>
> I think that ship has sailed, for instance, see // NOLINT comments that are also supported to silence diagnostics.

That's for linters, not for compilers.

> Also, the original bug report mentions that flex generates code using these constructs, so there are real world libraries using this construct.

So what? If we went down this road, we'd now have `/*OVERRIDE*/` instead of `override`, `/*FINAL*/` instead of `final`, etc. Real world libraries can be updated. That's what we already did everywhere in C++ mode. No reason it can't happen in C mode too.

The usual progression is:

- compiler-specific extensions (we now have `__attribute__((fallthrough))` attached to an empty statement)
- eventually, if it has legs, standardization

This disrupts this path.

>> In addition to this making comments have semantic meaning, the meaning is different from gcc.
> 
> We're covering the cases GCC does that make sense within Clang -- or have we deviated from GCC in a way that is significant to our users?

We give a not-well-specified and not documented subset of comments some meaning. We do this in a way that's different from GCC.

Also, as mentioned above, as-is I'd expect this patch to make builds measurably slower. That alone is revert reason enough.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73852