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,10 +17,35 @@ namespace tidy { namespace modernize { +static bool doesOverrideMacroExist(ASTContext &Context, + const llvm::StringRef &MacroId) { + // Don't check for the Macro existence if we are using ``override``. + if (MacroId == "override") + return true; + + // Otherwise look up the macro name in the context to see if its defined. + return Context.Idents.get(MacroId).hasMacroDefinition(); +} + +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) - Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this); + + // If we use ``override``, we require at least C++11. Use a + // macro with pre c++11 compilers by using OverrideMacro option. + if ((OverrideMacro == "override" && !getLangOpts().CPlusPlus11) || + !getLangOpts().CPlusPlus) + return; + + Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this); } // Re-lex the tokens to get precise locations to insert 'override' and remove @@ -67,6 +92,11 @@ const auto *Method = Result.Nodes.getNodeAs("method"); const SourceManager &Sources = *Result.SourceManager; + ASTContext &Context = *Result.Context; + + if (!doesOverrideMacroExist(Context, OverrideMacro)) + return; + assert(Method != nullptr); if (Method->getInstantiatedFromMemberFunction() != nullptr) Method = Method->getInstantiatedFromMemberFunction(); @@ -88,16 +118,19 @@ std::string Message; if (OnlyVirtualSpecified) { - Message = - "prefer using 'override' or (rarely) 'final' instead of 'virtual'"; + Message = "prefer using '" + OverrideMacro + "' or (rarely) '" + + FinalMacro + "' instead of 'virtual'"; } else if (KeywordCount == 0) { - Message = "annotate this function with 'override' or (rarely) 'final'"; + Message = "annotate this function with '" + OverrideMacro + + "' or (rarely) '" + FinalMacro + "'"; } else { StringRef Redundant = - HasVirtual ? (HasOverride && HasFinal ? "'virtual' and 'override' are" - : "'virtual' is") - : "'override' is"; - StringRef Correct = HasFinal ? "'final'" : "'override'"; + HasVirtual ? (HasOverride && HasFinal + ? "'virtual' and '" + OverrideMacro + "' are" + : "'virtual' is") + : "'" + OverrideMacro + "' is"; + StringRef Correct = + HasFinal ? "'" + FinalMacro + "'" : "'" + OverrideMacro + "'"; Message = (llvm::Twine(Redundant) + " redundant since the function is already declared " + Correct) @@ -121,7 +154,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 +184,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 +197,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 +214,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-cxx98-with-macro.cpp =================================================================== --- /dev/null +++ test/clang-tidy/modernize-use-override-cxx98-with-macro.cpp @@ -0,0 +1,95 @@ +// 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++98 + +#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 MUST_USE_RESULT MustUseResultObject {}; + +struct IntPair { + int First, Second; +}; + +struct Base { + virtual ~Base() {} + virtual void a(); + virtual void b(); + virtual void c(); + virtual void d(); + virtual void d2(); + virtual void e() = 0; + virtual void f() = 0; + virtual void f2() const = 0; + virtual void g() = 0; + + virtual void j() const; + virtual MustUseResultObject k(); + virtual bool l() MUST_USE_RESULT UNUSED; + virtual bool n() MUST_USE_RESULT UNUSED; + + virtual void m(); + virtual void m2(); + virtual void o() __attribute__((unused)); + + virtual void r() &; + virtual void rr() &&; + + virtual void cv() const volatile; + virtual void cv2() const volatile; + + virtual void ne() noexcept(false); + virtual void t() throw(); + + virtual void il(IntPair); +}; + +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 f()=0; + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using + // CHECK-FIXES: {{^}} void f() 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 MustUseResultObject k(); // Has an implicit attribute. + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: prefer using + // CHECK-FIXES: {{^}} MustUseResultObject k() OVERRIDE; +}; Index: test/clang-tidy/modernize-use-override-cxx98-with-no-macro-inscope.cpp =================================================================== --- /dev/null +++ test/clang-tidy/modernize-use-override-cxx98-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++98 + +// 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(); +};