aaron.ballman added a comment.

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. Also, the original bug report mentions 
that flex generates code using these constructs, so there are real world 
libraries using this construct.

> 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?

> I don't think we should support this.

I think we should -- C did not have any other way to resolve this issue without 
C2x support or compiler extensions, and fallthrough is definitely an issue in 
C. While it's not my favorite solution to silencing the diagnostics, it solves 
a real problem that C programmers are hitting.

In D73852#1872013 <https://reviews.llvm.org/D73852#1872013>, @lebedev.ri wrote:

> Now this patch should be reverted.
>  This patch also omitted cfe-commits lists.


That is not an automatic reason to revert a patch, especially one that has been 
accepted by a code owner.



================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:1239
+              llvm::Regex("(/\\*[ \\t]*fall(s | |-)?thr(ough|u)\\.?[ 
\\t]*\\*/)"
+                          "|(//[ \\t]*fall(s | |-)?thr(ough|u)\\.?[ \\t]*)",
+                          llvm::Regex::IgnoreCase);
----------------
thakis wrote:
> Also, this adds a regex match for every comment line, yes? Isn't this 
> terrible for build performance? Did you do any benchmarking of this?
https://reviews.llvm.org/D73852#inline-671309


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73852



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to