poelmanc updated this revision to Diff 229161.
poelmanc edited the summary of this revision.
poelmanc added a comment.
Update to add and use new `findNextTokenSkippingComments` and
`findNextTokenSkippingKind` utility functions. Since the former calls the
latter with just one token type, this required generalizing `Token::isOneOf()`
to work with 1-to-N token kinds versus requiring 2-to-N.
Also added a release note.
To me the change to `Token::isOneOf()` is an improvement (less code and more
flexibility when calling it from variadic templates), but if the team prefers
not to modify `Token::isOneOf()`, we could eliminate the
`findNextTokenSkippingKind` utility function and instead implement
`findNextTokenSkippingComments` directly.
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70144/new/
https://reviews.llvm.org/D70144
Files:
clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
clang-tools-extra/clang-tidy/utils/LexerUtils.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
clang/include/clang/Lex/Token.h
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
@@ -7,7 +7,7 @@
~OL();
};
-OL::OL() {}
+OL::OL() {};
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default' to define a trivial default constructor [modernize-use-equals-default]
// CHECK-FIXES: OL::OL() = default;
OL::~OL() {}
@@ -17,9 +17,9 @@
// Inline definitions.
class IL {
public:
- IL() {}
+ IL() {} ; // Note embedded tab on this line
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
- // CHECK-FIXES: IL() = default;
+ // CHECK-FIXES: IL() = default ; // Note embedded tab on this line
~IL() {}
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
// CHECK-FIXES: ~IL() = default;
@@ -46,18 +46,20 @@
// Default member initializer
class DMI {
public:
- DMI() {}
- // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
- // CHECK-FIXES: DMI() = default;
+ DMI() {} // Comment before semi-colon on next line
+ ;
+ // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'
+ // CHECK-FIXES: DMI() = default // Comment before semi-colon on next line
+ // CHECK-FIXES-NEXT: ;
int Field = 5;
};
// Class member
class CM {
public:
- CM() {}
+ CM() {} /* Comments */ /* before */ /* semicolon */;
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
- // CHECK-FIXES: CM() = default;
+ // CHECK-FIXES: CM() = default /* Comments */ /* before */ /* semicolon */;
OL o;
};
@@ -66,7 +68,7 @@
Priv() {}
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
// CHECK-FIXES: Priv() = default;
- ~Priv() {}
+ ~Priv() {};
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
// CHECK-FIXES: ~Priv() = default;
};
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
@@ -119,7 +119,7 @@
struct BF {
BF() = default;
BF(const BF &Other) : Field1(Other.Field1), Field2(Other.Field2), Field3(Other.Field3),
- Field4(Other.Field4) {}
+ Field4(Other.Field4) {};
// CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'
// CHECK-FIXES: BF(const BF &Other) {{$}}
// CHECK-FIXES: = default;
Index: clang/include/clang/Lex/Token.h
===================================================================
--- clang/include/clang/Lex/Token.h
+++ clang/include/clang/Lex/Token.h
@@ -96,12 +96,14 @@
/// "if (Tok.is(tok::l_brace)) {...}".
bool is(tok::TokenKind K) const { return Kind == K; }
bool isNot(tok::TokenKind K) const { return Kind != K; }
- bool isOneOf(tok::TokenKind K1, tok::TokenKind K2) const {
- return is(K1) || is(K2);
+
+ /// Single argument overload provides base case for recursive template below
+ bool isOneOf(tok::TokenKind K) const {
+ return is(K);
}
template <typename... Ts>
- bool isOneOf(tok::TokenKind K1, tok::TokenKind K2, Ts... Ks) const {
- return is(K1) || isOneOf(K2, Ks...);
+ bool isOneOf(tok::TokenKind K1, Ts... Ks) const {
+ return is(K1) || isOneOf(Ks...);
}
/// Return true if this is a raw identifier (when lexing
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -141,6 +141,10 @@
- The 'objc-avoid-spinlock' check was renamed to :doc:`darwin-avoid-spinlock
<clang-tidy/checks/darwin-avoid-spinlock>`
+- The :doc:`modernize-use-equals-default
+ <clang-tidy/checks/modernize-use-equals-default>` fix no longer adds
+ semicolons where they would be redundant.
+
- New :doc:`readability-redundant-access-specifiers
<clang-tidy/checks/readability-redundant-access-specifiers>` check.
Index: clang-tools-extra/clang-tidy/utils/LexerUtils.h
===================================================================
--- clang-tools-extra/clang-tidy/utils/LexerUtils.h
+++ clang-tools-extra/clang-tidy/utils/LexerUtils.h
@@ -80,6 +80,24 @@
}
}
+// Finds next token not of any given TokenKind.
+template <typename TokenKind, typename... TokenKinds>
+Optional<Token> findNextTokenSkippingKind(SourceLocation Start,
+ const SourceManager &SM,
+ const LangOptions &LangOpts,
+ TokenKind TK, TokenKinds... TKs) {
+ Optional<Token> CurrentToken;
+ do {
+ CurrentToken = Lexer::findNextToken(Start, SM, LangOpts);
+ } while (CurrentToken && CurrentToken->isOneOf(TK, TKs...));
+ return CurrentToken;
+}
+
+// Finds next token that's not a comment.
+Optional<Token> findNextTokenSkippingComments(SourceLocation Start,
+ const SourceManager &SM,
+ const LangOptions &LangOpts);
+
/// Re-lex the provide \p Range and return \c false if either a macro spans
/// multiple tokens, a pre-processor directive or failure to retrieve the
/// next token is found, otherwise \c true.
Index: clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
@@ -68,6 +68,12 @@
return findNextAnyTokenKind(Start, SM, LangOpts, tok::comma, tok::semi);
}
+Optional<Token> findNextTokenSkippingComments(SourceLocation Start,
+ const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ return findNextTokenSkippingKind(Start, SM, LangOpts, tok::comment);
+}
+
bool rangeContainsExpansionsOrDirectives(SourceRange Range,
const SourceManager &SM,
const LangOptions &LangOpts) {
Index: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -302,9 +302,19 @@
auto Diag = diag(Location, "use '= default' to define a trivial " +
SpecialFunctionName);
- if (ApplyFix)
- Diag << FixItHint::CreateReplacement(Body->getSourceRange(), "= default;")
+ if (ApplyFix) {
+ // Peek ahead to see if there's a semicolon after Body->getSourceRange()
+ Optional<Token> Token;
+ do {
+ Token = Lexer::findNextToken(
+ Body->getSourceRange().getEnd().getLocWithOffset(1),
+ Result.Context->getSourceManager(), Result.Context->getLangOpts());
+ } while (Token && Token->is(tok::comment));
+ StringRef Replacement =
+ Token && Token->is(tok::semi) ? "= default" : "= default;";
+ Diag << FixItHint::CreateReplacement(Body->getSourceRange(), Replacement)
<< RemoveInitializers;
+ }
}
} // namespace modernize
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
