[clang-tidy] Fix modernize-use-using in extern C code

[njames93]

Summary

The check currently erroneously flags typedefs in extern "C" blocks.
Addresses https://bugs.llvm.org/show_bug.cgi?id=49181

[njames93]

Newline.

[steveire]

clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp

305

Can you add tests for typedefs in other scopes like

extern "C" {
typedef int CType;

struct CAnother {
};

typedef struct {
  int b;
  typedef struct CAnother AStruct;
} CStruct;

void foo()
{
  typedef struct CAnother AStruct;
}
}

[njames93]

clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp

305

I'm not sure those tests add any value. For the struct, you can't have a typedef in a struct in c. The function typedef is valid in c, but I still can't see a reason to use extern C when defining a function in c++

[Eugene.Zelenko]

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

19

This matcher may be useful for other checks, for example, modernize-use-nullptr.

[steveire]

clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp

305

Fair enough - Clang accepts the typedef in the struct with a warning, but it doesn't get transformed by the check anyway. The one in the function doesn't get transformed either, and if functions are so unlikely to appear in extern C, that it doesn't need to be covered, that's fine with me.

[aaron.ballman]

clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp

305

I still can't see a reason to use extern C when defining a function in c++

There are a few reasons.

1. You're defining an extern linkage function with C language linkage and not providing the declaration or you're being consistent between the declaration and the definition.

2. You have an extern "C" block that contains function definitions.

3. Pedantically, if the function declaration is extern "C", the function definition must be as well (http://eel.is/c++draft/dcl.link#1.sentence-5, http://eel.is/c++draft/dcl.link#6), however, many compilers ignore these requirements because otherwise the STL has issues.

So it definitely does happen in the wild. As some examples:

https://codesearch.isocpp.org/actcd19/main/b/bali-phy/bali-phy_3.4+dfsg-1/src/builtins/SModel.cc
https://codesearch.isocpp.org/actcd19/main/o/openjdk-11/openjdk-11_11.0.3+1-1/test/hotspot/jtreg/vmTestbase/nsk/jvmti/AttachOnDemand/attach015/attach015Agent00.c
https://codesearch.isocpp.org/actcd19/main/m/mp3fs/mp3fs_0.91-1/src/transcode.cc
https://codesearch.isocpp.org/actcd19/main/t/theano/theano_0.8.2-6/theano/sandbox/cuda/cnmem.cpp

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

 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace modernize {
 
+AST_MATCHER(Decl, isInExternC) {
+  return Node.getDeclContext()->isExternCContext();
+}
+
 UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
 
 void UseUsingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
 void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(typedefDecl(unless(isInstantiated())).bind("typedef"),
-                     this);
+  Finder->addMatcher(
+      typedefDecl(unless(isInstantiated()), unless(isInExternC()))
+          .bind("typedef"),
   // This matcher used to find tag declarations in source code within typedefs.
   // They appear in the AST just *prior* to the typedefs.
   Finder->addMatcher(tagDecl(unless(isImplicit())).bind("tagdecl"), this);
 }
 
 void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
   // Match CXXRecordDecl only to store the range of the last non-implicit full
   // declaration, to later check whether it's within the typdef itself.

clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp

 };
 
 template <int>
 struct InjectedClassNameWithUnnamedArgument {
   typedef InjectedClassNameWithUnnamedArgument b;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
   // CHECK-FIXES: using b = InjectedClassNameWithUnnamedArgument;
 };
+
+extern "C" {
+typedef int CType;
+typedef struct {
+  int b;
+} CStruct;
+}