Skip to content

Commit

Permalink
Prefer 'final', only use 'override' where needed.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hzeller committed Apr 19, 2024
1 parent e3ef2a3 commit a08a205
Show file tree
Hide file tree
Showing 11 changed files with 19 additions and 17 deletions.
2 changes: 1 addition & 1 deletion common/analysis/line_lint_rule.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion common/analysis/syntax_tree_lint_rule.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand Down
2 changes: 1 addition & 1 deletion common/analysis/text_structure_lint_rule.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion common/analysis/token_stream_lint_rule.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions common/analysis/violation_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class ViolationPrinter : public ViolationHandler {

void HandleViolations(
const std::set<verible::LintViolationWithStatus>& violations,
absl::string_view base, absl::string_view path) override;
absl::string_view base, absl::string_view path) final;

protected:
std::ostream* const stream_;
Expand All @@ -68,7 +68,7 @@ class ViolationWaiverPrinter : public ViolationHandler {

void HandleViolations(
const std::set<verible::LintViolationWithStatus>& 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_;
Expand Down
2 changes: 1 addition & 1 deletion common/formatting/tree_unwrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions common/lexer/flex_lexer_adapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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_));
Expand Down
4 changes: 3 additions & 1 deletion common/lexer/token_stream_adapter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion common/text/text_structure_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions common/text/tree_context_visitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_; }

Expand Down
8 changes: 4 additions & 4 deletions verilog/tools/ls/verilog-language-server_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<VerilogLanguageServer>(
[this](absl::string_view response) { response_stream_ << response; });

Expand All @@ -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},
Expand All @@ -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(),
Expand All @@ -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;
Expand Down

0 comments on commit a08a205

Please sign in to comment.