[clang-tidy] minor improvements in modernise-deprecated-headers check

This patch introduces a minor list of changes as proposed by Richard Smith in
the mailing list.

See original comments with an impact on the future check state below:

[comments.begin

> +                          {"complex.h", "ccomplex"},

It'd be better to convert this one to <complex>, or leave it alone.
<ccomplex> is an unnecessary wart.

(The contents of C++11's <complex.h> / <ccomplex> / <complex> (all of
which are identical) aren't comparable to C99's <complex.h>, so if
this was C++98 code using the C99 header, the code will be broken with
or without this transformation.)

> +                          {"iso646.h", "ciso646"},

Just delete #includes of this one. <ciso646> does nothing.

> +              {"stdalign.h", "cstdalign"},
> +              {"stdbool.h", "cstdbool"},

We should just delete these two includes. These headers do nothing in C++.

comments.end]

Reviewers: alexfh, aaron.ballman

Differential Revision: https://reviews.llvm.org/D17990

llvm-svn: 278254

clang-tools-extra/clang-tidy/modernize/DeprecatedHeadersCheck.cpp

@@ -12,6 +12,7 @@
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/ADT/StringSet.h"
 
 #include <vector>
 
@@ -35,6 +36,7 @@ class IncludeModernizePPCallbacks : public PPCallbacks {
   ClangTidyCheck &Check;
   LangOptions LangOpts;
   llvm::StringMap<std::string> CStyledHeaderToCxx;
+  llvm::StringSet<> DeleteHeaders;
 };
 } // namespace
 
@@ -51,16 +53,23 @@ IncludeModernizePPCallbacks::IncludeModernizePPCallbacks(ClangTidyCheck &Check,
     : Check(Check), LangOpts(LangOpts) {
   for (const auto &KeyValue :
        std::vector<std::pair<llvm::StringRef, std::string>>(
-           {{"assert.h", "cassert"}, {"complex.h", "ccomplex"},
-            {"ctype.h", "cctype"},   {"errno.h", "cerrno"},
-            {"float.h", "cfloat"},   {"inttypes.h", "cinttypes"},
-            {"iso646.h", "ciso646"}, {"limits.h", "climits"},
-            {"locale.h", "clocale"}, {"math.h", "cmath"},
-            {"setjmp.h", "csetjmp"}, {"signal.h", "csignal"},
-            {"stdarg.h", "cstdarg"}, {"stddef.h", "cstddef"},
-            {"stdint.h", "cstdint"}, {"stdio.h", "cstdio"},
-            {"stdlib.h", "cstdlib"}, {"string.h", "cstring"},
-            {"time.h", "ctime"},     {"wchar.h", "cwchar"},
+           {{"assert.h", "cassert"},
+            {"complex.h", "complex"},
+            {"ctype.h", "cctype"},
+            {"errno.h", "cerrno"},
+            {"float.h", "cfloat"},
+            {"limits.h", "climits"},
+            {"locale.h", "clocale"},
+            {"math.h", "cmath"},
+            {"setjmp.h", "csetjmp"},
+            {"signal.h", "csignal"},
+            {"stdarg.h", "cstdarg"},
+            {"stddef.h", "cstddef"},
+            {"stdio.h", "cstdio"},
+            {"stdlib.h", "cstdlib"},
+            {"string.h", "cstring"},
+            {"time.h", "ctime"},
+            {"wchar.h", "cwchar"},
             {"wctype.h", "cwctype"}})) {
     CStyledHeaderToCxx.insert(KeyValue);
   }
@@ -69,13 +78,17 @@ IncludeModernizePPCallbacks::IncludeModernizePPCallbacks(ClangTidyCheck &Check,
     for (const auto &KeyValue :
          std::vector<std::pair<llvm::StringRef, std::string>>(
              {{"fenv.h", "cfenv"},
-              {"stdalign.h", "cstdalign"},
-              {"stdbool.h", "cstdbool"},
+              {"stdint.h", "cstdint"},
+              {"inttypes.h", "cinttypes"},
               {"tgmath.h", "ctgmath"},
               {"uchar.h", "cuchar"}})) {
       CStyledHeaderToCxx.insert(KeyValue);
     }
   }
+  for (const auto &Key :
+       std::vector<std::string>({"stdalign.h", "stdbool.h", "iso646.h"})) {
+    DeleteHeaders.insert(Key);
+  }
 }
 
 void IncludeModernizePPCallbacks::InclusionDirective(
@@ -86,17 +99,22 @@ void IncludeModernizePPCallbacks::InclusionDirective(
   //
   // Reasonable options for the check:
   //
-  // 1. Insert std prefix for every such symbol occurance.
+  // 1. Insert std prefix for every such symbol occurrence.
   // 2. Insert `using namespace std;` to the beginning of TU.
   // 3. Do nothing and let the user deal with the migration himself.
   if (CStyledHeaderToCxx.count(FileName) != 0) {
     std::string Replacement =
         (llvm::Twine("<") + CStyledHeaderToCxx[FileName] + ">").str();
-    Check.diag(FilenameRange.getBegin(),
-               "inclusion of deprecated C++ header '%0'; consider using '%1' instead")
+    Check.diag(FilenameRange.getBegin(), "inclusion of deprecated C++ header "
+                                         "'%0'; consider using '%1' instead")
         << FileName << CStyledHeaderToCxx[FileName]
         << FixItHint::CreateReplacement(FilenameRange.getAsRange(),
                                         Replacement);
+  } else if (DeleteHeaders.count(FileName) != 0) {
+    Check.diag(FilenameRange.getBegin(),
+               "including '%0' has no effect in C++; consider removing it")
+        << FileName << FixItHint::CreateRemoval(
+                           SourceRange(HashLoc, FilenameRange.getEnd()));
   }
 }
 

clang-tools-extra/docs/clang-tidy/checks/modernize-deprecated-headers.rst

@@ -4,10 +4,11 @@ modernize-deprecated-headers
 ============================
 
 Some headers from C library were deprecated in C++ and are no longer welcome in
-C++ codebases. For more details refer to the C++ 14 Standard [depr.c.headers]
-section.
+C++ codebases. Some have no effect in C++. For more details refer to the C++ 14
+Standard [depr.c.headers] section.
 
-This check replaces C standard library headers with their C++ alternatives.
+This check replaces C standard library headers with their C++ alternatives and
+removes redundant ones.
 
 Improtant note: the Standard doesn't guarantee that the C++ headers declare all
 the same functions in the global namespace. The check in its current form can
@@ -20,15 +21,12 @@ break the code that uses library symbols from the global namespace.
 * `<fenv.h>`     // deprecated since C++11
 * `<float.h>`
 * `<inttypes.h>`
-* `<iso646.h>`
 * `<limits.h>`
 * `<locale.h>`
 * `<math.h>`
 * `<setjmp.h>`
 * `<signal.h>`
-* `<stdalign.h>` // deprecated since C++11
 * `<stdarg.h>`
-* `<stdbool.h>`  // deprecated since C++11
 * `<stddef.h>`
 * `<stdint.h>`
 * `<stdio.h>`
@@ -42,4 +40,10 @@ break the code that uses library symbols from the global namespace.
 
 If the specified standard is older than C++11 the check will only replace
 headers deprecated before C++11, otherwise -- every header that appeared in
-the list.
+the previous list.
+
+These headers don't have effect in C++:
+
+* `<iso646.h>`
+* `<stdalign.h>`
+* `<stdbool.h>`