Index: clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.h +++ clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.h @@ -18,15 +18,16 @@ /// Use C++11's `override` and remove `virtual` where applicable. class UseOverrideCheck : public ClangTidyCheck { public: - UseOverrideCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - IgnoreDestructors(Options.get("IgnoreDestructors", false)) {} + 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 bool IgnoreDestructors; + const std::string OverrideSpelling; + const std::string FinalSpelling; }; } // namespace modernize Index: clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/modernize/UseOverrideCheck.cpp @@ -17,8 +17,16 @@ namespace tidy { namespace modernize { +UseOverrideCheck::UseOverrideCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IgnoreDestructors(Options.get("IgnoreDestructors", false)), + OverrideSpelling(Options.get("OverrideSpelling", "override")), + FinalSpelling(Options.get("FinalSpelling", "final")) {} + void UseOverrideCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IgnoreDestructors", IgnoreDestructors); + Options.store(Opts, "OverrideSpelling", OverrideSpelling); + Options.store(Opts, "FinalSpelling", FinalSpelling); } void UseOverrideCheck::registerMatchers(MatchFinder *Finder) { @@ -78,6 +86,8 @@ const auto *Method = Result.Nodes.getNodeAs("method"); const SourceManager &Sources = *Result.SourceManager; + ASTContext &Context = *Result.Context; + assert(Method != nullptr); if (Method->getInstantiatedFromMemberFunction() != nullptr) Method = Method->getInstantiatedFromMemberFunction(); @@ -97,25 +107,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) + << OverrideSpelling << FinalSpelling; CharSourceRange FileRange = Lexer::makeFileCharRange( CharSourceRange::getTokenRange(Method->getSourceRange()), Sources, @@ -132,7 +141,7 @@ // Add 'override' on inline declarations that don't already have it. if (!HasFinal && !HasOverride) { SourceLocation InsertLoc; - StringRef ReplacementText = "override "; + std::string ReplacementText = OverrideSpelling + " "; SourceLocation MethodLoc = Method->getLocation(); for (Token T : Tokens) { @@ -162,7 +171,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 = " " + OverrideSpelling; auto LastTokenIter = std::prev(Tokens.end()); // When try statement is used instead of compound statement as // method body - insert override keyword before it. @@ -175,23 +184,30 @@ // 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 "; - } else if (GetText(Tokens.back(), Sources) == "ABSTRACT") { + ReplacementText = " " + OverrideSpelling + " "; + } else if (GetText(Tokens.back(), Sources) == "ABSTRACT") InsertLoc = Tokens.back().getLocation(); - } } if (!InsertLoc.isValid()) { InsertLoc = FileRange.getEnd(); - ReplacementText = " override"; + ReplacementText = " " + OverrideSpelling; } + + // If the override macro has been specified just ensure it exists, + // if not don't apply a fixit but keep the warning. + if (OverrideSpelling != "override" && + !Context.Idents.get(OverrideSpelling).hasMacroDefinition()) + return; + Diag << FixItHint::CreateInsertion(InsertLoc, ReplacementText); } Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/trunk/docs/ReleaseNotes.rst +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst @@ -105,7 +105,7 @@ - The :doc:`bugprone-argument-comment ` now supports - `CommentBoolLiterals`, `CommentIntegerLiterals`, `CommentFloatLiterals`, + `CommentBoolLiterals`, `CommentIntegerLiterals`, `CommentFloatLiterals`, `CommentUserDefiniedLiterals`, `CommentStringLiterals`, `CommentCharacterLiterals` & `CommentNullPtrs` options. @@ -113,6 +113,10 @@ :doc:`objc-property-declaration ` check have been removed. +- The :doc:`modernize-use-override + ` now supports `OverrideSpelling` + and `FinalSpelling` options. + Improvements to include-fixer ----------------------------- Index: clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-override.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-override.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-use-override.rst @@ -3,7 +3,22 @@ modernize-use-override ====================== -Use C++11's ``override`` and remove ``virtual`` where applicable. +Adds ``override`` (introduced in C++11) to overridden virtual functions and +removes ``virtual`` from those functions as it is not required. + +``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`` keywords were introduced to allow +overridden functions to be marked appropriately. Their 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 ------- @@ -11,3 +26,19 @@ .. option:: IgnoreDestructors If set to non-zero, this check will not diagnose destructors. Default is `0`. + +.. option:: OverrideSpelling + + 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:: FinalSpelling + + 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: clang-tools-extra/trunk/test/clang-tidy/modernize-use-override-with-macro.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/modernize-use-override-with-macro.cpp +++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-override-with-macro.cpp @@ -0,0 +1,70 @@ +// RUN: %check_clang_tidy %s modernize-use-override %t -- \ +// RUN: -config="{CheckOptions: [{key: modernize-use-override.OverrideSpelling, value: 'OVERRIDE'},{key: modernize-use-override.FinalSpelling, value: 'FINAL'}]}" \ +// RUN: -- -std=c++11 + +#define ABSTRACT = 0 + +#define OVERRIDE override +#define FINAL final +#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: clang-tools-extra/trunk/test/clang-tidy/modernize-use-override-with-no-macro-inscope.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/modernize-use-override-with-no-macro-inscope.cpp +++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-override-with-no-macro-inscope.cpp @@ -0,0 +1,28 @@ +// RUN: %check_clang_tidy %s modernize-use-override %t -- \ +// RUN: -config="{CheckOptions: [{key: modernize-use-override.OverrideSpelling, value: 'CUSTOM_OVERRIDE'},{key: modernize-use-override.FinalSpelling, value: 'CUSTOM_FINAL'}]}" \ +// RUN: -- -std=c++11 + +// As if the macro was not defined. +//#define CUSTOM_OVERRIDE override +//#define CUSTOM_FINAL override + +struct Base { + virtual ~Base() {} + virtual void a(); + virtual void b(); +}; + +struct SimpleCases : public Base { +public: + virtual ~SimpleCases(); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using 'CUSTOM_OVERRIDE' or (rarely) 'CUSTOM_FINAL' instead of 'virtual' [modernize-use-override] + // CHECK-FIXES: {{^}} virtual ~SimpleCases(); + + void a(); + // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: annotate this function with 'CUSTOM_OVERRIDE' or (rarely) 'CUSTOM_FINAL' [modernize-use-override] + // CHECK-FIXES: {{^}} void a(); + + virtual void b(); + // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using 'CUSTOM_OVERRIDE' or (rarely) 'CUSTOM_FINAL' instead of 'virtual' [modernize-use-override] + // CHECK-FIXES: {{^}} virtual void b(); +};