HazardyKnusperkeks added a comment.

In D117416#3250415 <https://reviews.llvm.org/D117416#3250415>, @MyDeveloperDay 
wrote:

> I'm not a fan of this approach of adding a "C" language, mainly because of 
> the `.h` problem so ultimately it doesn't solve your problem.
>
> I think this is overkill for what is basically the subtle handling of 
> "public/private/protected/try/delete/new" being used as variables (which 
> frankly is at least in my view not a bright idea in the first place!)
>
> I think ultimately we can try to catch those corner cases but IMHO it doesn't 
> need the addition of a whole new language for that, (we are not at that much 
> on an impasse)
>
> This review contains .arclint that needs removing, this review contains no 
> tests

I was actually thinking that adding C as language is the right way, and still 
am. We have stuff only for arcane C variants, why not push these in the 
language C and keep C++ free from it?

But I'm fine with either solution.



================
Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:100
   int getIndentOffset(const FormatToken &RootToken) {
-    if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
+    if (Style.Language == FormatStyle::LK_C ||
+        Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||
----------------
MyDeveloperDay wrote:
> isC() these large OR/AND conditions become unreadable
I'm more of a `switch` fan and not your isXXX(), but that's preference. ;)


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1606
 
-      if (Style.isCpp() && FormatTok->is(TT_StatementMacro)) {
+      if ((Style.isCpp() || Style.isC()) && FormatTok->is(TT_StatementMacro)) {
         parseStatementMacro();
----------------
MyDeveloperDay wrote:
> I feel everywhere you put isCpp you'll end up doing isCpp() || is C()
> 
> This goes back to the arguments for a review I tried before {D80079} its 
> really isCStyle() if we WERE going to pursue this, I'd want that and not have 
> clauses added everywhere
I think one could use `isC()`, `isCpp()`, `isObjC()`, and what is now 
`isCpp()`: `isCish()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

Reply via email to