Skip to content

Fix #35272: Don't replace typedefs in extern c scope #69102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Dec 26, 2023
Merged
31 changes: 25 additions & 6 deletions clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,39 @@
#include "clang/Lex/Lexer.h"

using namespace clang::ast_matchers;
namespace {

AST_MATCHER(clang::LinkageSpecDecl, isExternCLinkage) {
return Node.getLanguage() == clang::LinkageSpecDecl::lang_c;
}
} // namespace

namespace clang::tidy::modernize {

static constexpr llvm::StringLiteral ExternCDeclName = "extern-c-decl";
static constexpr llvm::StringLiteral ParentDeclName = "parent-decl";
static constexpr llvm::StringLiteral TagDeclName = "tag-decl";
static constexpr llvm::StringLiteral TypedefName = "typedef";

UseUsingCheck::UseUsingCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)),
IgnoreExternC(Options.get("IgnoreExternC", false)) {}

void UseUsingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "IgnoreMacros", IgnoreMacros);
Options.store(Opts, "IgnoreExternC", IgnoreExternC);
}

void UseUsingCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(typedefDecl(unless(isInstantiated()),
hasParent(decl().bind(ParentDeclName)))
.bind(TypedefName),
this);
Finder->addMatcher(
typedefDecl(
unless(isInstantiated()),
optionally(hasAncestor(
linkageSpecDecl(isExternCLinkage()).bind(ExternCDeclName))),
hasParent(decl().bind(ParentDeclName)))
.bind(TypedefName),
this);

// This matcher is used to find tag declarations in source code within
// typedefs. They appear in the AST just *prior* to the typedefs.
Expand Down Expand Up @@ -70,6 +83,11 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
if (MatchedDecl->getLocation().isInvalid())
return;

const auto *ExternCDecl =
Result.Nodes.getNodeAs<LinkageSpecDecl>(ExternCDeclName);
if (ExternCDecl && IgnoreExternC)
return;

SourceLocation StartLoc = MatchedDecl->getBeginLoc();

if (StartLoc.isMacroID() && IgnoreMacros)
Expand Down Expand Up @@ -122,7 +140,8 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
Type = FirstTypedefName + Type.substr(FirstTypedefType.size() + 1);
}
if (!ReplaceRange.getEnd().isMacroID()) {
const SourceLocation::IntTy Offset = MatchedDecl->getFunctionType() ? 0 : Name.size();
const SourceLocation::IntTy Offset =
MatchedDecl->getFunctionType() ? 0 : Name.size();
LastReplacementEnd = ReplaceRange.getEnd().getLocWithOffset(Offset);
}

Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ namespace clang::tidy::modernize {
class UseUsingCheck : public ClangTidyCheck {

const bool IgnoreMacros;
const bool IgnoreExternC;
SourceLocation LastReplacementEnd;
llvm::DenseMap<const Decl *, SourceRange> LastTagDeclRanges;

Expand Down
3 changes: 2 additions & 1 deletion clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,8 @@ Changes in existing checks

- Improved :doc:`modernize-use-using
<clang-tidy/checks/modernize/use-using>` check to fix function pointer and
forward declared ``typedef`` correctly.
forward declared ``typedef`` correctly. Added option `IgnoreExternC` to ignore ``typedef``
declaration in ``extern "C"`` scope.

- Improved :doc:`performance-faster-string-find
<clang-tidy/checks/performance/faster-string-find>` check to properly escape
Expand Down
13 changes: 13 additions & 0 deletions clang-tools-extra/docs/clang-tidy/checks/modernize/use-using.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ After:
using R_t = struct { int a; };
using R_p = R_t*;

The checker ignores `typedef` within `extern "C" { ... }` blocks.

.. code-block:: c++

extern "C" {
typedef int InExternC; // Left intact.
}

This check requires using C++11 or higher to run.

Options
Expand All @@ -37,3 +45,8 @@ Options

If set to `true`, the check will not give warnings inside macros. Default
is `true`.

.. option:: IgnoreExternC

If set to `true`, the check will not give warning inside `extern "C"`scope.
Default is `false`
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// RUN: %check_clang_tidy %s modernize-use-using %t -- -config="{CheckOptions: {modernize-use-using.IgnoreExternC: true}}" -- -I %S/Input/use-using/

// Some Header
extern "C" {

typedef int NewInt;
}

extern "C++" {

typedef int InExternCPP;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
// CHECK-FIXES: using InExternCPP = int;
}
17 changes: 17 additions & 0 deletions clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -325,3 +325,20 @@ typedef bool (*ISSUE_65055_2)(int);
typedef class ISSUE_67529_1 *ISSUE_67529;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
// CHECK-FIXES: using ISSUE_67529 = class ISSUE_67529_1 *;

// Some Header
extern "C" {

typedef int InExternC;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
// CHECK-FIXES: using InExternC = int;

}

extern "C++" {

typedef int InExternCPP;
// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef' [modernize-use-using]
// CHECK-FIXES: using InExternCPP = int;

}