[clang-tools-extra] 441a18d - [clang-tidy] Preserve typedef comments in `modernize-use-using` (#180372)
via cfe-commits
cfe-commits at lists.llvm.org
Sun May 3 13:15:10 PDT 2026
Author: Daniil Dudkin
Date: 2026-05-03T20:15:05Z
New Revision: 441a18d980c10663412b701668dc2d6a790c5e38
URL: https://github.com/llvm/llvm-project/commit/441a18d980c10663412b701668dc2d6a790c5e38
DIFF: https://github.com/llvm/llvm-project/commit/441a18d980c10663412b701668dc2d6a790c5e38.diff
LOG: [clang-tidy] Preserve typedef comments in `modernize-use-using` (#180372)
Keep comment blocks between the typedef type and name by capturing the
raw lexer range and avoid injecting unrelated tokens into the
replacement.
Fixes https://github.com/llvm/llvm-project/issues/159518.
Added:
Modified:
clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
index aaba2d56389f8..327fe02735468 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -20,6 +20,8 @@ using namespace clang::ast_matchers;
namespace clang::tidy::modernize {
+namespace lexer = clang::tidy::utils::lexer;
+
namespace {
AST_MATCHER(LinkageSpecDecl, isExternCLinkage) {
@@ -28,6 +30,202 @@ AST_MATCHER(LinkageSpecDecl, isExternCLinkage) {
} // namespace
+namespace {
+
+struct TokenRangeInfo {
+ bool HasComment = false;
+ bool HasIdentifier = false;
+ bool HasPointerOrRef = false;
+};
+
+struct RangeTextInfo {
+ std::string Text;
+ TokenRangeInfo Tokens;
+};
+
+} // namespace
+
+static StringRef::size_type findFirstNonWhitespace(StringRef Text) {
+ return Text.find_first_not_of(" \t\n\r\f\v");
+}
+
+static std::optional<std::string> getSourceText(CharSourceRange Range,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ if (Range.isInvalid())
+ return std::nullopt;
+
+ const CharSourceRange FileRange =
+ Lexer::makeFileCharRange(Range, SM, LangOpts);
+ if (FileRange.isInvalid())
+ return std::nullopt;
+
+ bool IsInvalid = false;
+ const StringRef Text =
+ Lexer::getSourceText(FileRange, SM, LangOpts, &IsInvalid);
+ if (IsInvalid)
+ return std::nullopt;
+ return Text.str();
+}
+
+static TokenRangeInfo getTokenRangeInfo(CharSourceRange Range,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ TokenRangeInfo Info;
+ if (Range.isInvalid())
+ return Info;
+
+ const CharSourceRange FileRange =
+ Lexer::makeFileCharRange(Range, SM, LangOpts);
+ if (FileRange.isInvalid())
+ return Info;
+
+ const auto [BeginFID, BeginOffset] =
+ SM.getDecomposedLoc(FileRange.getBegin());
+ const auto [EndFID, EndOffset] = SM.getDecomposedLoc(FileRange.getEnd());
+ if (BeginFID != EndFID || BeginOffset > EndOffset)
+ return Info;
+
+ bool IsInvalid = false;
+ const StringRef Buffer = SM.getBufferData(BeginFID, &IsInvalid);
+ if (IsInvalid)
+ return Info;
+
+ const char *LexStart = Buffer.data() + BeginOffset;
+ Lexer TheLexer(SM.getLocForStartOfFile(BeginFID), LangOpts, Buffer.begin(),
+ LexStart, Buffer.end());
+ TheLexer.SetCommentRetentionState(true);
+
+ while (true) {
+ Token Tok;
+ if (TheLexer.LexFromRawLexer(Tok))
+ break;
+
+ if (Tok.is(tok::eof) || Tok.getLocation() == FileRange.getEnd() ||
+ SM.isBeforeInTranslationUnit(FileRange.getEnd(), Tok.getLocation()))
+ break;
+
+ if (Tok.is(tok::comment)) {
+ Info.HasComment = true;
+ continue;
+ }
+
+ if (Tok.isOneOf(tok::star, tok::amp))
+ Info.HasPointerOrRef = true;
+
+ if (tok::isAnyIdentifier(Tok.getKind()) ||
+ Tok.isOneOf(tok::kw_typedef, tok::kw_struct, tok::kw_class,
+ tok::kw_union, tok::kw_enum, tok::kw_typename,
+ tok::kw_template)) {
+ Info.HasIdentifier = true;
+ }
+ }
+
+ return Info;
+}
+
+static RangeTextInfo getRangeTextInfo(SourceLocation Begin, SourceLocation End,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ if (!Begin.isValid() || !End.isValid() || Begin.isMacroID() ||
+ End.isMacroID())
+ return {};
+
+ const CharSourceRange Range = CharSourceRange::getCharRange(Begin, End);
+ if (std::optional<std::string> Text = getSourceText(Range, SM, LangOpts))
+ return {*Text, getTokenRangeInfo(Range, SM, LangOpts)};
+ return {};
+}
+
+static std::optional<std::string>
+getFunctionPointerTypeText(SourceRange TypeRange, SourceLocation NameLoc,
+ const SourceManager &SM, const LangOptions &LO) {
+ SourceLocation StartLoc = NameLoc;
+ SourceLocation EndLoc = NameLoc;
+
+ while (true) {
+ const std::optional<Token> Prev = lexer::getPreviousToken(StartLoc, SM, LO);
+ const std::optional<Token> Next =
+ lexer::findNextTokenSkippingComments(EndLoc, SM, LO);
+ if (!Prev || Prev->isNot(tok::l_paren) || !Next ||
+ Next->isNot(tok::r_paren))
+ break;
+
+ StartLoc = Prev->getLocation();
+ EndLoc = Next->getLocation();
+ }
+
+ const CharSourceRange RangeLeftOfIdentifier =
+ CharSourceRange::getCharRange(TypeRange.getBegin(), StartLoc);
+ const CharSourceRange RangeRightOfIdentifier = CharSourceRange::getCharRange(
+ Lexer::getLocForEndOfToken(EndLoc, 0, SM, LO),
+ Lexer::getLocForEndOfToken(TypeRange.getEnd(), 0, SM, LO));
+
+ const std::optional<std::string> LeftText =
+ getSourceText(RangeLeftOfIdentifier, SM, LO);
+ if (!LeftText)
+ return std::nullopt;
+
+ const std::optional<std::string> RightText =
+ getSourceText(RangeRightOfIdentifier, SM, LO);
+ if (!RightText)
+ return std::nullopt;
+
+ return *LeftText + *RightText;
+}
+
+static RangeTextInfo getLeadingTextInfo(bool IsFirstTypedefInGroup,
+ SourceRange ReplaceRange,
+ SourceRange TypeRange,
+ const SourceManager &SM,
+ const LangOptions &LO) {
+ if (!IsFirstTypedefInGroup)
+ return {};
+
+ const SourceLocation TypedefEnd =
+ Lexer::getLocForEndOfToken(ReplaceRange.getBegin(), 0, SM, LO);
+ RangeTextInfo Info =
+ getRangeTextInfo(TypedefEnd, TypeRange.getBegin(), SM, LO);
+ if (!Info.Tokens.HasComment)
+ Info.Text.clear();
+ return Info;
+}
+
+static RangeTextInfo
+getSuffixTextInfo(bool FunctionPointerCase, bool IsFirstTypedefInGroup,
+ SourceLocation PrevReplacementEnd, SourceRange TypeRange,
+ SourceLocation NameLoc, const SourceManager &SM,
+ const LangOptions &LO) {
+ if (FunctionPointerCase)
+ return {};
+
+ if (IsFirstTypedefInGroup) {
+ const SourceLocation AfterType =
+ Lexer::getLocForEndOfToken(TypeRange.getEnd(), 0, SM, LO);
+ return getRangeTextInfo(AfterType, NameLoc, SM, LO);
+ }
+
+ if (!PrevReplacementEnd.isValid() || PrevReplacementEnd.isMacroID())
+ return {};
+
+ SourceLocation AfterComma = PrevReplacementEnd;
+ if (const std::optional<Token> NextTok =
+ lexer::findNextTokenSkippingComments(AfterComma, SM, LO)) {
+ if (NextTok->is(tok::comma)) {
+ AfterComma =
+ Lexer::getLocForEndOfToken(NextTok->getLocation(), 0, SM, LO);
+ }
+ }
+
+ return getRangeTextInfo(AfterComma, NameLoc, SM, LO);
+}
+
+static void stripLeadingComma(RangeTextInfo &Info) {
+ const StringRef::size_type NonWs = findFirstNonWhitespace(Info.Text);
+ if (NonWs != StringRef::npos && Info.Text[NonWs] == ',')
+ Info.Text.erase(0, NonWs + 1);
+}
+
static constexpr StringRef ExternCDeclName = "extern-c-decl";
static constexpr StringRef ParentDeclName = "parent-decl";
static constexpr StringRef TagDeclName = "tag-decl";
@@ -132,69 +330,74 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
const TypeLoc TL = MatchedDecl->getTypeSourceInfo()->getTypeLoc();
- bool FunctionPointerCase = false;
- auto [Type, QualifierStr] = [MatchedDecl, this, &TL, &FunctionPointerCase,
- &SM,
- &LO]() -> std::pair<std::string, std::string> {
- SourceRange TypeRange = TL.getSourceRange();
+ struct TypeInfo {
+ SourceRange Range;
+ bool FunctionPointerCase = false;
+ bool Valid = false;
+ std::string Type;
+ std::string Qualifier;
+ };
+
+ const TypeInfo TI = [&] {
+ TypeInfo Info;
+ Info.Range = TL.getSourceRange();
// Function pointer case, get the left and right side of the identifier
// without the identifier.
- if (TypeRange.fullyContains(MatchedDecl->getLocation())) {
- FunctionPointerCase = true;
- SourceLocation StartLoc = MatchedDecl->getLocation();
- SourceLocation EndLoc = MatchedDecl->getLocation();
-
- while (true) {
- const std::optional<Token> Prev =
- utils::lexer::getPreviousToken(StartLoc, SM, LO);
- const std::optional<Token> Next =
- utils::lexer::findNextTokenSkippingComments(EndLoc, SM, LO);
- if (!Prev || Prev->isNot(tok::l_paren) || !Next ||
- Next->isNot(tok::r_paren))
- break;
-
- StartLoc = Prev->getLocation();
- EndLoc = Next->getLocation();
+ if (Info.Range.fullyContains(MatchedDecl->getLocation())) {
+ Info.FunctionPointerCase = true;
+ if (std::optional<std::string> Type = getFunctionPointerTypeText(
+ Info.Range, MatchedDecl->getLocation(), SM, LO)) {
+ Info.Type = *Type;
+ Info.Valid = true;
}
-
- const auto RangeLeftOfIdentifier =
- CharSourceRange::getCharRange(TypeRange.getBegin(), StartLoc);
- const auto RangeRightOfIdentifier = CharSourceRange::getCharRange(
- Lexer::getLocForEndOfToken(EndLoc, 0, SM, LO),
- Lexer::getLocForEndOfToken(TypeRange.getEnd(), 0, SM, LO));
- const std::string VerbatimType =
- (Lexer::getSourceText(RangeLeftOfIdentifier, SM, LO) +
- Lexer::getSourceText(RangeRightOfIdentifier, SM, LO))
- .str();
- return {VerbatimType, ""};
+ return Info;
}
- StringRef ExtraReference = "";
- if (MainTypeEndLoc.isValid() && TypeRange.fullyContains(MainTypeEndLoc)) {
+ std::string ExtraReference;
+ if (MainTypeEndLoc.isValid() && Info.Range.fullyContains(MainTypeEndLoc)) {
// Each type introduced in a typedef can specify being a reference or
// pointer type separately, so we need to figure out if the new using-decl
// needs to be to a reference or pointer as well.
- const SourceLocation Tok = utils::lexer::findPreviousAnyTokenKind(
+ const SourceLocation Tok = lexer::findPreviousAnyTokenKind(
MatchedDecl->getLocation(), SM, LO, tok::TokenKind::star,
tok::TokenKind::amp, tok::TokenKind::comma,
tok::TokenKind::kw_typedef);
- ExtraReference = Lexer::getSourceText(
+ const std::optional<std::string> Reference = getSourceText(
CharSourceRange::getCharRange(Tok, Tok.getLocWithOffset(1)), SM, LO);
+ if (!Reference)
+ return Info;
+ ExtraReference = *Reference;
if (ExtraReference != "*" && ExtraReference != "&")
- ExtraReference = "";
+ ExtraReference.clear();
+
+ Info.Range.setEnd(MainTypeEndLoc);
+ }
- TypeRange.setEnd(MainTypeEndLoc);
+ if (std::optional<std::string> Type =
+ getSourceText(CharSourceRange::getTokenRange(Info.Range), SM, LO)) {
+ Info.Type = *Type;
+ Info.Qualifier = ExtraReference;
+ Info.Valid = true;
}
- return {
- Lexer::getSourceText(CharSourceRange::getTokenRange(TypeRange), SM, LO)
- .str(),
- ExtraReference.str()};
+ return Info;
}();
+
+ if (!TI.Valid) {
+ diag(StartLoc, UseUsingWarning);
+ return;
+ }
+
+ const SourceRange TypeRange = TI.Range;
+ const bool FunctionPointerCase = TI.FunctionPointerCase;
+ std::string Type = TI.Type;
+ const std::string QualifierStr = TI.Qualifier;
const StringRef Name = MatchedDecl->getName();
+ const SourceLocation NameLoc = MatchedDecl->getLocation();
SourceRange ReplaceRange = MatchedDecl->getSourceRange();
+ const SourceLocation PrevReplacementEnd = LastReplacementEnd;
// typedefs with multiple comma-separated definitions produce multiple
// consecutive TypedefDecl nodes whose SourceRanges overlap. Each range starts
@@ -203,10 +406,13 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
// But also we need to check that the ranges belong to the same file because
//
diff erent files may contain overlapping ranges.
std::string Using = "using ";
- if (ReplaceRange.getBegin().isMacroID() ||
+ const bool IsFirstTypedefInGroup =
+ ReplaceRange.getBegin().isMacroID() ||
(Result.SourceManager->getFileID(ReplaceRange.getBegin()) !=
Result.SourceManager->getFileID(LastReplacementEnd)) ||
- (ReplaceRange.getBegin() >= LastReplacementEnd)) {
+ (ReplaceRange.getBegin() >= LastReplacementEnd);
+
+ if (IsFirstTypedefInGroup) {
// This is the first (and possibly the only) TypedefDecl in a typedef. Save
// Type and Name in case we find subsequent TypedefDecl's in this typedef.
FirstTypedefType = Type;
@@ -225,6 +431,26 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
Type = FirstTypedefName;
}
+ const RangeTextInfo LeadingTextInfo = getLeadingTextInfo(
+ IsFirstTypedefInGroup, ReplaceRange, TypeRange, SM, LO);
+ RangeTextInfo SuffixTextInfo =
+ getSuffixTextInfo(FunctionPointerCase, IsFirstTypedefInGroup,
+ PrevReplacementEnd, TypeRange, NameLoc, SM, LO);
+ if (!IsFirstTypedefInGroup)
+ stripLeadingComma(SuffixTextInfo);
+
+ const bool SuffixHasComment = SuffixTextInfo.Tokens.HasComment;
+ std::string SuffixText;
+ if (SuffixHasComment) {
+ SuffixText = SuffixTextInfo.Text;
+ } else if (QualifierStr.empty() &&
+ findFirstNonWhitespace(SuffixTextInfo.Text) != StringRef::npos &&
+ SuffixTextInfo.Tokens.HasPointerOrRef &&
+ !SuffixTextInfo.Tokens.HasIdentifier) {
+ SuffixText = SuffixTextInfo.Text;
+ }
+ const std::string QualifierText = SuffixHasComment ? "" : QualifierStr;
+
if (!ReplaceRange.getEnd().isMacroID()) {
const SourceLocation::IntTy Offset = FunctionPointerCase ? 0 : Name.size();
LastReplacementEnd = ReplaceRange.getEnd().getLocWithOffset(Offset);
@@ -237,14 +463,22 @@ void UseUsingCheck::check(const MatchFinder::MatchResult &Result) {
if (LastTagDeclRange != LastTagDeclRanges.end() &&
LastTagDeclRange->second.isValid() &&
ReplaceRange.fullyContains(LastTagDeclRange->second)) {
- Type = std::string(Lexer::getSourceText(
- CharSourceRange::getTokenRange(LastTagDeclRange->second), SM, LO));
- if (Type.empty())
+ const std::optional<std::string> TagType = getSourceText(
+ CharSourceRange::getTokenRange(LastTagDeclRange->second), SM, LO);
+ if (!TagType)
return;
+ Type = *TagType;
}
- const std::string Replacement =
- (Using + Name + " = " + Type + QualifierStr).str();
+ std::string TypeExpr =
+ LeadingTextInfo.Text + Type + QualifierText + SuffixText;
+ TypeExpr = StringRef(TypeExpr).rtrim(" \t").str();
+ StringRef Assign = " = ";
+ if (!TypeExpr.empty() &&
+ (TypeExpr.front() == ' ' || TypeExpr.front() == '\t'))
+ Assign = " =";
+
+ const std::string Replacement = (Using + Name + Assign + TypeExpr).str();
Diag << FixItHint::CreateReplacement(ReplaceRange, Replacement);
}
} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c9749df481bcd..d4b81d0479e89 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -439,8 +439,12 @@ Changes in existing checks
macros appearing in the return type of a function.
- Improved :doc:`modernize-use-using
- <clang-tidy/checks/modernize/use-using>` check by avoiding the generation
- of invalid code for function types with redundant parentheses.
+ <clang-tidy/checks/modernize/use-using>` check:
+
+ - Avoid generating invalid code for function types with redundant
+ parentheses.
+
+ - Preserve inline comment blocks that appear between the ``typedef``'s parts.
- Improved :doc:`performance-enum-size
<clang-tidy/checks/performance/enum-size>` check:
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
index 324616d274646..232dd33f35f37 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-using.cpp
@@ -487,3 +487,70 @@ namespace GH176267 {
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef' [modernize-use-using]
// CHECK-FIXES: using f6 = int (double);
}
+
+namespace GH159518 {
+typedef int // start and end chunks for cells in a line
+ Commented; // (end is chunk beyond end of line)
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Commented = int // start and end chunks for cells in a line
+// CHECK-FIXES-NEXT: ; // (end is chunk beyond end of line)
+
+typedef /*prefix*/ int PrefixCommented;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using PrefixCommented = /*prefix*/ int;
+
+typedef const /*qual*/ int QualCommented;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using QualCommented = const /*qual*/ int;
+
+typedef int /*between*/ BetweenCommented;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using BetweenCommented = int /*between*/;
+
+typedef int /*multi-line
+comment*/ MultiLineCommented;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using MultiLineCommented = int /*multi-line
+// CHECK-FIXES-NEXT: comment*/;
+
+typedef int // line comment 1
+// line comment 2
+// line comment 3
+ MultiLineSlashCommented;
+// CHECK-MESSAGES: :[[@LINE-4]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using MultiLineSlashCommented = int // line comment 1
+// CHECK-FIXES-NEXT: // line comment 2
+// CHECK-FIXES-NEXT: // line comment 3
+// CHECK-FIXES-NEXT: ;
+
+typedef int * /*ptr*/ PtrCommented;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using PtrCommented = int * /*ptr*/;
+
+typedef int AfterNameCommented /*after*/;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using AfterNameCommented = int /*after*/;
+
+typedef int TrailingCommented; // trailing
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using TrailingCommented = int; // trailing
+
+typedef int MultiA, /*between comma*/ *MultiB;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-MESSAGES: :[[@LINE-2]]:{{[0-9]+}}: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using MultiA = int;
+// CHECK-FIXES-NEXT: using MultiB = MultiA /*between comma*/ *;
+
+struct TagCommented;
+typedef struct /*tag*/ TagCommented TagCommentedAlias;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using TagCommentedAlias = struct /*tag*/ TagCommented;
+
+typedef int (* /*fp*/ FuncPtrCommented)(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using FuncPtrCommented = int (* /*fp*/ )(int);
+
+typedef TwoArgTemplate</*tmpl*/ int, int> TemplateArgCommented;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using TemplateArgCommented = TwoArgTemplate</*tmpl*/ int, int>;
+} // namespace GH159518
More information about the cfe-commits
mailing list