[PATCH] D80079: [clang-format] [NFC] isCpp() is inconsistently used to mean both C++ and Objective C, add language specific isXXX() functions

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon May 18 02:05:58 PDT 2020


MyDeveloperDay added a comment.

- I personally find the current form (Style.Language == LK_XXX) very verbose and difficult to read, especially when its in the middle of a large clause.
- Its clear to me that isCpp() is not understood, its name doesn't quite suggest what it does and as such there is code that effectively does if (isCpp() || isObjectiveC()) (which you can't easily see because of the verbose form)
- I added isCSharp() (I knew what isCpp() did) but I added it so it was very clear where the C# handling code was, I didn't want to proliferate the `Style.Language == LK_CSharp` , and also its easier to search for all `!Style.isCSharp()` and `Style.isCSharp()` in one go rather than having to handle the `!=/==` cases

I think functions need to be named to say what they do, and isCpp() currently doesn't and it has/does already cause mistakes. I don't mind what its called I'm happy to change names, but I feel there is more clarity in the isXXX() calls.

This has been on of those refactorings I've wanted to do over the last year as I've been working on clang-format, I was expecting it to be controversial: ;-)

My 2p worth.


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

https://reviews.llvm.org/D80079





More information about the cfe-commits mailing list