[PATCH] D72213: [clang-tidy] Add `bugprone-reserved-identifier` to flag usages of reserved C++ names
Eugene Zelenko via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sat Jan 4 13:55:45 PST 2020
Eugene.Zelenko added inline comments.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:49
+}
+static Optional<std::string> getDoubleUnderscoreFixup(StringRef Name) {
+ if (hasDoubleUnderscore(Name)) {
----------------
Please separate with empty line.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:60
+}
+static Optional<std::string> getUnderscoreCapitalFixup(const StringRef Name) {
+ if (startsWithUnderscoreCapital(Name)) {
----------------
Please separate with empty line.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:72
+}
+static Optional<std::string>
+getUnderscoreGlobalNamespaceFixup(const StringRef Name,
----------------
Please separate with empty line.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:114
+ };
+ if (auto Fixup = getDoubleUnderscoreFixup(inProgressFixup())) {
+ appendFailure("du", *std::move(Fixup));
----------------
Please don't use auto when type is spelled in same statement or iterator.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:117
+ }
+ if (auto Fixup = getUnderscoreCapitalFixup(inProgressFixup())) {
+ appendFailure("uc", *std::move(Fixup));
----------------
Please don't use auto when type is spelled in same statement or iterator.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:120
+ }
+ if (auto Fixup = getUnderscoreGlobalNamespaceFixup(inProgressFixup(),
+ IsInGlobalNamespace)) {
----------------
Please don't use auto when type is spelled in same statement or iterator.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:146
+}
+Optional<RenamerClangTidyCheck::FailureInfo>
+ReservedIdentifierCheck::GetMacroFailureInfo(const Token &MacroNameTok,
----------------
Please separate with empty line.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:152
+}
+RenamerClangTidyCheck::DiagInfo
+ReservedIdentifierCheck::GetDiagInfo(const NamingCheckId &ID,
----------------
Please separate with empty line.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:168
+};
+} // namespace bugprone
+
----------------
Please separate with empty line.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:169
+} // namespace bugprone
+
+} // namespace tidy
----------------
Unnecessary empty line.
================
Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h:13
+#include "../utils/RenamerClangTidyCheck.h"
+
+namespace clang {
----------------
Please include vector and llvm/Optional.h.
================
Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:645
}
-
-void IdentifierNamingCheck::checkMacro(SourceManager &SourceMgr,
- const Token &MacroNameTok,
- const MacroInfo *MI) {
+llvm::Optional<RenamerClangTidyCheck::FailureInfo>
+IdentifierNamingCheck::GetMacroFailureInfo(const Token &MacroNameTok,
----------------
Please separate with empty line.
================
Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:672
}
-
-void IdentifierNamingCheck::expandMacro(const Token &MacroNameTok,
- const MacroInfo *MI) {
- StringRef Name = MacroNameTok.getIdentifierInfo()->getName();
- NamingCheckId ID(MI->getDefinitionLoc(), Name);
-
- auto Failure = NamingCheckFailures.find(ID);
- if (Failure == NamingCheckFailures.end())
- return;
-
- SourceRange Range(MacroNameTok.getLocation(), MacroNameTok.getEndLoc());
- addUsage(NamingCheckFailures, ID, Range);
-}
-
-void IdentifierNamingCheck::onEndOfTranslationUnit() {
- for (const auto &Pair : NamingCheckFailures) {
- const NamingCheckId &Decl = Pair.first;
- const NamingCheckFailure &Failure = Pair.second;
-
- if (Failure.KindName.empty())
- continue;
-
- if (Failure.ShouldNotify()) {
- auto Diag =
- diag(Decl.first,
- "invalid case style for %0 '%1'%select{|" // Case 0 is empty on
- // purpose, because we
- // intent to provide a
- // fix
- "; cannot be fixed because '%3' would conflict with a keyword|"
- "; cannot be fixed because '%3' would conflict with a macro "
- "definition}2")
- << Failure.KindName << Decl.second
- << static_cast<int>(Failure.FixStatus) << Failure.Fixup;
-
- if (Failure.ShouldFix()) {
- for (const auto &Loc : Failure.RawUsageLocs) {
- // We assume that the identifier name is made of one token only. This
- // is always the case as we ignore usages in macros that could build
- // identifier names by combining multiple tokens.
- //
- // For destructors, we already take care of it by remembering the
- // location of the start of the identifier and not the start of the
- // tilde.
- //
- // Other multi-token identifiers, such as operators are not checked at
- // all.
- Diag << FixItHint::CreateReplacement(
- SourceRange(SourceLocation::getFromRawEncoding(Loc)),
- Failure.Fixup);
- }
- }
- }
- }
+RenamerClangTidyCheck::DiagInfo
+IdentifierNamingCheck::GetDiagInfo(const NamingCheckId &ID,
----------------
Please separate with empty line.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:10
+#include "RenamerClangTidyCheck.h"
+
+#include "ASTUtils.h"
----------------
Unnecessary empty line.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:59
+};
+} // namespace llvm
+
----------------
Please separate with empty line.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:65
+namespace {
+/// Callback supplies macros to RenamerClangTidyCheck::checkMacro
+class RenamerClangTidyCheckPPCallbacks : public PPCallbacks {
----------------
Please separate with empty line.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:89
+};
+} // namespace
+
----------------
Please separate with empty line.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:132
+ // is already in there
+ auto &Failure = Failures[Decl];
+ if (!Failure.RawUsageLocs.insert(FixLocation.getRawEncoding()).second)
----------------
Please don't use auto when type is spelled in same statement or iterator.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:165
+ continue;
+ if (const auto *FD = Init->getAnyMember())
+ addUsage(NamingCheckFailures, FD,
----------------
Please don't use auto when type is spelled in same statement or iterator.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:208
+ if (const auto &Ref = Loc->getAs<TemplateSpecializationTypeLoc>()) {
+ const auto *Decl =
+ Ref.getTypePtr()->getTemplateName().getAsTemplateDecl();
----------------
Please don't use auto when type is spelled in same statement or iterator.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:213
+ if (const auto *ClassDecl = dyn_cast<TemplateDecl>(Decl)) {
+ if (const auto *TemplDecl = ClassDecl->getTemplatedDecl())
+ addUsage(NamingCheckFailures, TemplDecl, Range);
----------------
Please don't use auto when type is spelled in same statement or iterator.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:229
+ Result.Nodes.getNodeAs<NestedNameSpecifierLoc>("nestedNameLoc")) {
+ if (NestedNameSpecifier *Spec = Loc->getNestedNameSpecifier()) {
+ if (NamespaceDecl *Decl = Spec->getAsNamespace()) {
----------------
Could it be const? Same below.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:258
+ if (const auto *Value = Result.Nodes.getNodeAs<ValueDecl>("decl")) {
+ if (const auto *TypePtr = Value->getType().getTypePtrOrNull()) {
+ if (const auto *Typedef = TypePtr->getAs<TypedefType>()) {
----------------
Please don't use auto when type is spelled in same statement or iterator.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:274
+ for (unsigned i = 0; i < Value->getNumParams(); ++i) {
+ if (const auto *Typedef = Value->parameters()[i]
+ ->getType()
----------------
Please don't use auto when type is spelled in same statement or iterator.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:349
+ if (FixStatus == RenamerClangTidyCheck::ShouldFixStatus::ShouldFix) {
+ return "";
+ }
----------------
Just {}. Same below.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:1
+//===--- RenamderClangTidyCheck.h - clang-tidy -------------------*- C++
+//-*-===//
----------------
Please merge two lines and make sure resulting line is 80 characters wide.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:14
+#include "../ClangTidyCheck.h"
+
+namespace clang {
----------------
Please include string, utility, DenseSet, Optional.
================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:124
+ };
+ /// Overridden by derived classes, returns a description of the diagnostic
+ /// that should be emitted for the given failure. The base class will then
----------------
Please separate with empty line.
================
Comment at: clang-tools-extra/docs/ReleaseNotes.rst:100
+
+ Finds usages of __names _Like ::_this which are reserved for
+ the C++ implementation.
----------------
Please synchronize with first statement in documentation.
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst:22
+
+
+The check can also be inverted, i.e. it can be configured to flag any identifier that is _not_ a reserved identifier. This mode is for use by e.g. standard library implementors, to ensure they don't infringe on the user namespace.
----------------
Unnecessary empty line.
================
Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst:30
+
+ If non-zero, inverts the check, i.e. flags names that are not reserved. Default is 0.
+
----------------
Please enclose 0 in single back-ticks.
================
Comment at: clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/system/system-header.h:34
+} // namespace std
\ No newline at end of file
----------------
Please add newline.
================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier-invert.cpp:71
+} // namespace std
\ No newline at end of file
----------------
Please add newline.
================
Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp:172
+using alias_ = int;
\ No newline at end of file
----------------
Please add newline.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72213/new/
https://reviews.llvm.org/D72213
More information about the cfe-commits
mailing list