[PATCH] D18919: [Clang-tidy] Add check "modernize use using"

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Tue May 3 02:48:08 PDT 2016


hokein added inline comments.

================
Comment at: clang-tidy/modernize/UseUsingCheck.cpp:22
@@ +21,3 @@
+void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
----------------
Prazek wrote:
> hokein wrote:
> > Should be CplusPlus11 here.
> BTW is there any policy about that? I see that some checks from modernize require C++11 (e.g. modernize-make-unique which is in C++14) and other require just C++ (modernize-loop-convert), and it even has a comment
> 
> // Only register the matchers for C++. Because this checker is used for
>   // modernization, it is reasonable to run it on any C++ standard with the
>   // assumption the user is trying to modernize their codebase.
>   if (!getLangOpts().CPlusPlus)
>     return;
> 
> 
> I have 2 thoughts for this:
> 1. there should be note in documentation about it, so the user won't spend time debuging why the check doesn't do anything. e.g. "This check requires to compile code with C++11 or higher"
> 2. I would suggest modernize checks to require standard version the same or higher for C++ standars that doesn't break backwards compatibility:
> e.g. loop-convert should require C++11, make-shared C++14, this check also C++11,
> but for modernize-increment-bool, that is deprecated in C++17, it should require just C++ (because if someone need it, the he wont be able to compile his code with C++17).
This is a good point. 

As far as I know, we don't have detailed policy about the modernized checks. It depends on the check author. Basically the modern words means "C++11" feature.

I'm +1 on adding a note in each modernized check's document. 


Repository:
  rL LLVM

http://reviews.llvm.org/D18919





More information about the cfe-commits mailing list