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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits