Index: clang-tidy/modernize/UseOverrideCheck.h =================================================================== --- clang-tidy/modernize/UseOverrideCheck.h +++ clang-tidy/modernize/UseOverrideCheck.h @@ -18,10 +18,14 @@ /// Use C++11's `override` and remove `virtual` where applicable. class UseOverrideCheck : public ClangTidyCheck { public: - UseOverrideCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + UseOverrideCheck(StringRef Name, ClangTidyContext *Context); void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + const std::string OverrideMacro; + const std::string FinalMacro; }; } // namespace modernize Index: clang-tidy/modernize/UseOverrideCheck.cpp =================================================================== --- clang-tidy/modernize/UseOverrideCheck.cpp +++ clang-tidy/modernize/UseOverrideCheck.cpp @@ -17,6 +17,16 @@ namespace tidy { namespace modernize { +UseOverrideCheck::UseOverrideCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + OverrideMacro(Options.get("OverrideMacro", "override")), + FinalMacro(Options.get("FinalMacro", "final")) {} + +void UseOverrideCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "OverrideMacro", OverrideMacro); + Options.store(Opts, "FinalMacro", FinalMacro); +} + void UseOverrideCheck::registerMatchers(MatchFinder *Finder) { // Only register the matcher for C++11. if (getLangOpts().CPlusPlus11) @@ -67,6 +77,13 @@ const auto *Method = Result.Nodes.getNodeAs("method"); const SourceManager &Sources = *Result.SourceManager; + ASTContext &Context = *Result.Context; + + // If the override macro has been specified just ensure it exists. + if (OverrideMacro != "override" && + !Context.Idents.get(OverrideMacro).hasMacroDefinition()) + return; + assert(Method != nullptr); if (Method->getInstantiatedFromMemberFunction() != nullptr) Method = Method->getInstantiatedFromMemberFunction(); @@ -86,25 +103,24 @@ return; // Nothing to do. std::string Message; - if (OnlyVirtualSpecified) { - Message = - "prefer using 'override' or (rarely) 'final' instead of 'virtual'"; + Message = "prefer using '%0' or (rarely) '%1' instead of 'virtual'"; } else if (KeywordCount == 0) { - Message = "annotate this function with 'override' or (rarely) 'final'"; + Message = "annotate this function with '%0' or (rarely) '%1'"; } else { StringRef Redundant = - HasVirtual ? (HasOverride && HasFinal ? "'virtual' and 'override' are" + HasVirtual ? (HasOverride && HasFinal ? "'virtual' and '%0' are" : "'virtual' is") - : "'override' is"; - StringRef Correct = HasFinal ? "'final'" : "'override'"; + : "'%0' is"; + StringRef Correct = HasFinal ? "'%1'" : "'%0'"; Message = (llvm::Twine(Redundant) + " redundant since the function is already declared " + Correct) .str(); } - DiagnosticBuilder Diag = diag(Method->getLocation(), Message); + auto Diag = diag(Method->getLocation(), Message) + << OverrideMacro << FinalMacro; CharSourceRange FileRange = Lexer::makeFileCharRange( CharSourceRange::getTokenRange(Method->getSourceRange()), Sources, @@ -121,7 +137,7 @@ // Add 'override' on inline declarations that don't already have it. if (!HasFinal && !HasOverride) { SourceLocation InsertLoc; - StringRef ReplacementText = "override "; + std::string ReplacementText = OverrideMacro + " "; SourceLocation MethodLoc = Method->getLocation(); for (Token T : Tokens) { @@ -151,7 +167,7 @@ // end of the declaration of the function, but prefer to put it on the // same line as the declaration if the beginning brace for the start of // the body falls on the next line. - ReplacementText = " override"; + ReplacementText = " " + OverrideMacro; auto LastTokenIter = std::prev(Tokens.end()); // When try statement is used instead of compound statement as // method body - insert override keyword before it. @@ -164,14 +180,16 @@ // For declarations marked with "= 0" or "= [default|delete]", the end // location will point until after those markings. Therefore, the override // keyword shouldn't be inserted at the end, but before the '='. - if (Tokens.size() > 2 && (GetText(Tokens.back(), Sources) == "0" || - Tokens.back().is(tok::kw_default) || - Tokens.back().is(tok::kw_delete)) && + if (Tokens.size() > 2 && + (GetText(Tokens.back(), Sources) == "0" || + Tokens.back().is(tok::kw_default) || + Tokens.back().is(tok::kw_delete)) && GetText(Tokens[Tokens.size() - 2], Sources) == "=") { InsertLoc = Tokens[Tokens.size() - 2].getLocation(); // Check if we need to insert a space. - if ((Tokens[Tokens.size() - 2].getFlags() & Token::LeadingSpace) == 0) - ReplacementText = " override "; + if ((Tokens[Tokens.size() - 2].getFlags() & Token::LeadingSpace) == 0) { + ReplacementText = " " + OverrideMacro + " "; + } } else if (GetText(Tokens.back(), Sources) == "ABSTRACT") { InsertLoc = Tokens.back().getLocation(); } @@ -179,7 +197,7 @@ if (!InsertLoc.isValid()) { InsertLoc = FileRange.getEnd(); - ReplacementText = " override"; + ReplacementText = " " + OverrideMacro; } Diag << FixItHint::CreateInsertion(InsertLoc, ReplacementText); } Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -73,6 +73,10 @@ Checks for casts of ``absl::Duration`` conversion functions, and recommends the right conversion function instead. +- The :doc:`modernize-use-override + ` now supports `OverrideMacro` + and `FinalMacro` options. + Improvements to include-fixer ----------------------------- Index: docs/clang-tidy/checks/modernize-use-override.rst =================================================================== --- docs/clang-tidy/checks/modernize-use-override.rst +++ docs/clang-tidy/checks/modernize-use-override.rst @@ -3,5 +3,37 @@ modernize-use-override ====================== +Adds ``override`` (introduced in C++11) to overridden virtual functions and +removes ``virtual`` from those functions as it is not required. -Use C++11's ``override`` and remove ``virtual`` where applicable. +virtual on non base class implementations was used to help indiciate to the user +that a function was virtual. C++ compilers did not use the presence of this to +signify an overriden function. + +In C++ 11 ``override`` and ``final`` were introduced to allow overridden +functions to be marked appropriately. There presence allows compilers to verify +that an overridden function correctly overrides a base class implementation. + +This can be useful as compilers can generate a compile time error when: + + - The base class implementation function signature changes. + - The user has not created the override with the correct signature. + +Options +------- + +.. option:: OverrideMacro + + Specifies a macro to use instead of ``override``. This is useful when + maintaining source code that also needs to compile with a pre-C++11 + compiler. + +.. option:: FinalMacro + + Specifies a macro to use instead of ``final``. This is useful when + maintaining source code that also needs to compile with a pre-C++11 + compiler. + +.. note:: + + For more information on the use of override see https://en.cppreference.com/w/cpp/language/override Index: test/clang-tidy/modernize-use-override-with-macro.cpp =================================================================== --- /dev/null +++ test/clang-tidy/modernize-use-override-with-macro.cpp @@ -0,0 +1,69 @@ +// RUN: %check_clang_tidy %s modernize-use-override %t -- \ +// RUN: -config="{CheckOptions: [{key: modernize-use-nodiscard.OverrideMacro, value: 'OVERRIDE'},{key: modernize-use-nodiscard.FinalMacro, value: 'FINAL'}]}" \ +// RUN: -- -std=c++11 + +#define ABSTRACT = 0 + +#define OVERRIDE override +#define VIRTUAL virtual +#define NOT_VIRTUAL +#define NOT_OVERRIDE + +#define MUST_USE_RESULT __attribute__((warn_unused_result)) +#define UNUSED __attribute__((unused)) + +struct Base { + virtual ~Base() {} + virtual void a(); + virtual void b(); + virtual void c(); + virtual void e() = 0; + virtual void f2() const = 0; + virtual void g() = 0; + virtual void j() const; + virtual void k() = 0; + virtual void l() const; +}; + +struct SimpleCases : public Base { +public: + virtual ~SimpleCases(); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using 'OVERRIDE' or (rarely) 'FINAL' instead of 'virtual' [modernize-use-override] + // CHECK-FIXES: {{^}} ~SimpleCases() OVERRIDE; + + void a(); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: annotate this function with 'OVERRIDE' or (rarely) 'FINAL' [modernize-use-override] + // CHECK-FIXES: {{^}} void a() OVERRIDE; + + virtual void b(); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using 'OVERRIDE' or (rarely) 'FINAL' instead of 'virtual' [modernize-use-override] + // CHECK-FIXES: {{^}} void b() OVERRIDE; + + virtual void c(); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using + // CHECK-FIXES: {{^}} void c() OVERRIDE; + + virtual void e() = 0; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using + // CHECK-FIXES: {{^}} void e() OVERRIDE = 0; + + virtual void f2() const = 0; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using + // CHECK-FIXES: {{^}} void f2() const OVERRIDE = 0; + + virtual void g() ABSTRACT; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using + // CHECK-FIXES: {{^}} void g() OVERRIDE ABSTRACT; + + virtual void j() const; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using + // CHECK-FIXES: {{^}} void j() const OVERRIDE; + + virtual void k() OVERRIDE; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'OVERRIDE' [modernize-use-override] + // CHECK-FIXES: {{^}} void k() OVERRIDE; + + virtual void l() const OVERRIDE; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'OVERRIDE' [modernize-use-override] + // CHECK-FIXES: {{^}} void l() const OVERRIDE; +}; Index: test/clang-tidy/modernize-use-override-with-no-macro-inscope.cpp =================================================================== --- /dev/null +++ test/clang-tidy/modernize-use-override-with-no-macro-inscope.cpp @@ -0,0 +1,24 @@ +// RUN: %check_clang_tidy %s modernize-use-override %t -- \ +// RUN: -config="{CheckOptions: [{key: modernize-use-nodiscard.OverrideMacro, value: 'CUSTOM_OVERRIDE'},{key: modernize-use-nodiscard.FinalMacro, value: 'FINAL'}]}" \ +// RUN: -- -std=c++11 + +// As if the macro was not defined. +//#define CUSTOM_OVERRIDE override + +struct Base { + virtual ~Base() {} + virtual void a(); + virtual void b(); +}; + +struct SimpleCases : public Base { +public: + virtual ~SimpleCases(); + // CHECK-FIXES: {{^}} virtual ~SimpleCases(); + + void a(); + // CHECK-FIXES: {{^}} void a(); + + virtual void b(); + // CHECK-FIXES: {{^}} virtual void b(); +};