From a08a205b17585ead907e4f71423918d1201425ea Mon Sep 17 00:00:00 2001 From: Henner Zeller Date: Fri, 19 Apr 2024 11:40:49 -0700 Subject: [PATCH] Prefer 'final', only use 'override' where needed. This helps documenting the code for the reader. For the compiler, it might open devirtualization opportunities. Document all the methods that can not be set `final` because they are in fact overridden later. Mark as `// not yet final`. This will be useful in future automated cleanups. --- common/analysis/line_lint_rule.h | 2 +- common/analysis/syntax_tree_lint_rule.h | 2 +- common/analysis/text_structure_lint_rule.h | 2 +- common/analysis/token_stream_lint_rule.h | 2 +- common/analysis/violation_handler.h | 4 ++-- common/formatting/tree_unwrapper.h | 2 +- common/lexer/flex_lexer_adapter.h | 4 ++-- common/lexer/token_stream_adapter_test.cc | 4 +++- common/text/text_structure_test.cc | 2 +- common/text/tree_context_visitor.h | 4 ++-- verilog/tools/ls/verilog-language-server_test.cc | 8 ++++---- 11 files changed, 19 insertions(+), 17 deletions(-) diff --git a/common/analysis/line_lint_rule.h b/common/analysis/line_lint_rule.h index 9f4adf30e..ec6da5788 100644 --- a/common/analysis/line_lint_rule.h +++ b/common/analysis/line_lint_rule.h @@ -26,7 +26,7 @@ namespace verible { class LineLintRule : public LintRule { public: - ~LineLintRule() override = default; + ~LineLintRule() override = default; // not yet final // Scans a single line during analysis. virtual void HandleLine(absl::string_view line) = 0; diff --git a/common/analysis/syntax_tree_lint_rule.h b/common/analysis/syntax_tree_lint_rule.h index 184fcdcb3..90c96fed1 100644 --- a/common/analysis/syntax_tree_lint_rule.h +++ b/common/analysis/syntax_tree_lint_rule.h @@ -38,7 +38,7 @@ namespace verible { // top of the stack/back of vector. class SyntaxTreeLintRule : public LintRule { public: - ~SyntaxTreeLintRule() override = default; + ~SyntaxTreeLintRule() override = default; // not yet final virtual void HandleLeaf(const SyntaxTreeLeaf &leaf, const SyntaxTreeContext &context) {} diff --git a/common/analysis/text_structure_lint_rule.h b/common/analysis/text_structure_lint_rule.h index 0116257f4..eb44e6836 100644 --- a/common/analysis/text_structure_lint_rule.h +++ b/common/analysis/text_structure_lint_rule.h @@ -38,7 +38,7 @@ namespace verible { class TextStructureLintRule : public LintRule { public: - ~TextStructureLintRule() override = default; + ~TextStructureLintRule() override = default; // not yet final // Analyze text structure for violations. virtual void Lint(const TextStructureView &text_structure, diff --git a/common/analysis/token_stream_lint_rule.h b/common/analysis/token_stream_lint_rule.h index 381ee2dbd..dd1e978e2 100644 --- a/common/analysis/token_stream_lint_rule.h +++ b/common/analysis/token_stream_lint_rule.h @@ -26,7 +26,7 @@ namespace verible { class TokenStreamLintRule : public LintRule { public: - ~TokenStreamLintRule() override = default; + ~TokenStreamLintRule() override = default; // not yet final // Scans a single token during analysis. virtual void HandleToken(const TokenInfo &token) = 0; diff --git a/common/analysis/violation_handler.h b/common/analysis/violation_handler.h index 96994c3b7..8886b314a 100644 --- a/common/analysis/violation_handler.h +++ b/common/analysis/violation_handler.h @@ -51,7 +51,7 @@ class ViolationPrinter : public ViolationHandler { void HandleViolations( const std::set& violations, - absl::string_view base, absl::string_view path) override; + absl::string_view base, absl::string_view path) final; protected: std::ostream* const stream_; @@ -68,7 +68,7 @@ class ViolationWaiverPrinter : public ViolationHandler { void HandleViolations( const std::set& violations, - absl::string_view base, absl::string_view path) override; + absl::string_view base, absl::string_view path) final; protected: std::ostream* const message_stream_; diff --git a/common/formatting/tree_unwrapper.h b/common/formatting/tree_unwrapper.h index cd7ba1525..c3f723db5 100644 --- a/common/formatting/tree_unwrapper.h +++ b/common/formatting/tree_unwrapper.h @@ -52,7 +52,7 @@ class TreeUnwrapper : public TreeContextVisitor { TreeUnwrapper& operator=(const TreeUnwrapper&) = delete; TreeUnwrapper& operator=(TreeUnwrapper&&) = delete; - ~TreeUnwrapper() override = default; + ~TreeUnwrapper() override = default; // not yet final. // Partitions the token stream (in text_structure_view_) into // unwrapped_lines_ by traversing the syntax tree representation. diff --git a/common/lexer/flex_lexer_adapter.h b/common/lexer/flex_lexer_adapter.h index 7b0ab6899..57a6900dd 100644 --- a/common/lexer/flex_lexer_adapter.h +++ b/common/lexer/flex_lexer_adapter.h @@ -77,7 +77,7 @@ class FlexLexerAdapter : private CodeStreamHolder, protected L, public Lexer { const TokenInfo &GetLastToken() const final { return last_token_; } // Returns next token and updates its location. - const TokenInfo &DoNextToken() override { + const TokenInfo &DoNextToken() override { // not yet final if (at_eof_) { // Do not call yylex(), because that will result in the fatal error: // "fatal flex scanner internal error--end of buffer missed" @@ -108,7 +108,7 @@ class FlexLexerAdapter : private CodeStreamHolder, protected L, public Lexer { } // Restart lexer by pointing to new input stream, and reset all state. - void Restart(absl::string_view code) override { + void Restart(absl::string_view code) override { // not yet final at_eof_ = false; code_ = code; code_stream_.str(std::string(code_)); diff --git a/common/lexer/token_stream_adapter_test.cc b/common/lexer/token_stream_adapter_test.cc index fe2608270..1c9054154 100644 --- a/common/lexer/token_stream_adapter_test.cc +++ b/common/lexer/token_stream_adapter_test.cc @@ -38,7 +38,9 @@ class FakeTokenSequenceLexer : public Lexer, public FakeLexer { void Restart(absl::string_view) final {} - bool TokenIsError(const TokenInfo &) const override { return false; } + bool TokenIsError(const TokenInfo &) const override { // not yet final. + return false; + } }; TEST(MakeTokenGeneratorTest, Generate) { diff --git a/common/text/text_structure_test.cc b/common/text/text_structure_test.cc index 99d34849f..3d47784d0 100644 --- a/common/text/text_structure_test.cc +++ b/common/text/text_structure_test.cc @@ -363,7 +363,7 @@ class TextStructureViewInternalsTest : public TextStructureViewPublicTest { // modifications. This is only appropriate for tests on private or protected // methods; public methods should always leave the structure in a consistent // state. - ~TextStructureViewInternalsTest() override { Clear(); } + ~TextStructureViewInternalsTest() override { Clear(); } // not yet final }; // Test that whole tree is returned with offset 0. diff --git a/common/text/tree_context_visitor.h b/common/text/tree_context_visitor.h index b89ceb971..9b712436d 100644 --- a/common/text/tree_context_visitor.h +++ b/common/text/tree_context_visitor.h @@ -30,8 +30,8 @@ class TreeContextVisitor : public SymbolVisitor { TreeContextVisitor() = default; protected: - void Visit(const SyntaxTreeLeaf &leaf) override {} - void Visit(const SyntaxTreeNode &node) override; + void Visit(const SyntaxTreeLeaf &leaf) override {} // not yet final + void Visit(const SyntaxTreeNode &node) override; // not yet final const SyntaxTreeContext &Context() const { return current_context_; } diff --git a/verilog/tools/ls/verilog-language-server_test.cc b/verilog/tools/ls/verilog-language-server_test.cc index 2de8165d1..3d9c24fd9 100644 --- a/verilog/tools/ls/verilog-language-server_test.cc +++ b/verilog/tools/ls/verilog-language-server_test.cc @@ -123,7 +123,7 @@ class VerilogLanguageServerTest : public ::testing::Test { // Sets up the testing environment - creates Language Server object and // sends textDocument/initialize request. // It stores the response in initialize_response field for further processing - void SetUp() override { + void SetUp() override { // not yet final server_ = std::make_unique( [this](absl::string_view response) { response_stream_ << response; }); @@ -148,7 +148,7 @@ class VerilogLanguageServerTest : public ::testing::Test { class VerilogLanguageServerSymbolTableTest : public VerilogLanguageServerTest { public: - absl::Status InitializeCommunication() override { + absl::Status InitializeCommunication() final { json initialize_request = { {"jsonrpc", "2.0"}, {"id", 1}, @@ -158,7 +158,7 @@ class VerilogLanguageServerSymbolTableTest : public VerilogLanguageServerTest { } protected: - void SetUp() override { + void SetUp() final { absl::SetFlag(&FLAGS_rules_config_search, true); root_dir = verible::file::JoinPath( ::testing::TempDir(), @@ -168,7 +168,7 @@ class VerilogLanguageServerSymbolTableTest : public VerilogLanguageServerTest { VerilogLanguageServerTest::SetUp(); } - void TearDown() override { std::filesystem::remove(root_dir); } + void TearDown() final { std::filesystem::remove(root_dir); } // path to the project std::string root_dir;