refactor: Use trailing return types for void functions; Use auto to simplify type declarations in the touched files.

[SharafMohamed]

References

• Depends on PR#60.
• To review in parallel with PR#60, diff against PR#60 locally. In the repo run:

git fetch upstream pull/60/head:pr-60
git fetch upstream pull/61/head:pr-61
git diff pr-60 pr-61

Description

• Main focus of this PR is to to update older code to follow the auto -> return_value standard. Ideally we catch all these in thi PR.
• Secondary focus of this PR is to use auto for variable types where possible. Will catch any missed instances of this in individual clang-tidy PRs when we go file-by-file in the future.

Validation performed

Previously existing tests succeed.

Summary by CodeRabbit

Release Notes

• Refactor

  • Updated method signatures across multiple files to use auto return types and trailing return type syntax.
  • Renamed template parameters for improved clarity.
  • Replaced specific regex state types with more generic byte state types.

• Code Quality

  • Simplified type declarations using auto for variable inference.
  • Enhanced type consistency in regex and finite automata handling.

• Performance

  • Maintained existing logic while improving type management.
  • No significant functional changes introduced.

These updates focus on modernizing the codebase's type system and improving code readability without altering core functionality.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (4)

src/log_surgeon/Lalr1Parser.hpp (1)

168-168: Consider adding null check for robustness.

While at() provides bounds checking, consider adding an assertion or null check for m_production to prevent undefined behaviour.

-    [[nodiscard]] auto next_symbol() const -> uint32_t { return m_production->m_body.at(m_dot); }
+    [[nodiscard]] auto next_symbol() const -> uint32_t {
+        assert(m_production != nullptr);
+        return m_production->m_body.at(m_dot);
+    }

src/log_surgeon/Lexer.tpp (3)

243-243: Consider using const qualification for the loop variable.

The pointer is only used for reading, so it could benefit from const qualification:

-                        auto* next_state = state->next(byte);
+                        auto const* next_state = state->next(byte);

---

349-352: Add const qualification to range-based for loop variables.

Both loop variables are only used for reading, so they should be const-qualified:

-    for (auto& i : m_is_delimiter) {
+    for (auto const& i : m_is_delimiter) {
        i = false;
    }
-    for (auto delimiter : delimiters) {
+    for (auto const delimiter : delimiters) {
        m_is_delimiter[delimiter] = true;
    }

---

470-470: Use const reference for the range-based for loop.

Since the loop variable is only used for reading, it should use a const reference to avoid unnecessary copying:

-            auto* dest_state = next_dfa_state(kv.second);
+            auto const* dest_state = next_dfa_state(kv.second);

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee46719 and ea78ed2.

📒 Files selected for processing (2)

• src/log_surgeon/Lalr1Parser.hpp (3 hunks)
• src/log_surgeon/Lexer.tpp (12 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

src/log_surgeon/Lalr1Parser.hpp (1)

Pattern **/*.{cpp,hpp,java,js,jsx,ts,tsx}: - Prefer false == <expression> rather than !<expression>.

⏰ Context from checks skipped due to timeout of 90000ms (2)

• GitHub Check: build (ubuntu-latest, Release)
• GitHub Check: build (ubuntu-latest, Debug)

🔇 Additional comments (7)

src/log_surgeon/Lalr1Parser.hpp (3)

123-123: LGTM! Clean implementation following modern C++ practices.

The implementation correctly uses trailing return type syntax and the [[nodiscard]] attribute.

---

161-161: LGTM! Correct implementation with proper null safety.

The implementation safely compares the dot position with the production body size.

---

396-397: LGTM! Proper use of using declaration.

The using declaration appropriately brings the base class member into scope, improving code readability.

src/log_surgeon/Lexer.tpp (4)

21-21: LGTM! Trailing return type syntax properly implemented.

The change to use auto -> void aligns with the modern C++ style and the PR objectives.

---

44-44: Well-structured type inference with proper const-correctness!

The use of auto with appropriate const qualifiers (auto const* for read-only pointers and auto* for mutable ones) demonstrates good attention to const-correctness while simplifying the type declarations.

Also applies to: 68-71, 84-84

---

307-307: LGTM! Clear and concise type inference.

The use of auto with a descriptive variable name maintains code clarity while reducing verbosity.

---

382-382: LGTM! Proper const qualification with type inference.

The use of auto const* correctly indicates a read-only pointer to the DFA root state.

[LinZhihao-723]

Reviewed the changes to ensure all void returns are in auto xxx -> void form.
One note is that we shouldn't include variable type changes in this PR (but since they're already included, let's keep them for this PR). Two reasons:

• It makes this PR harder to review
• Some of these changes are incomplete: we still need to add const keyword to make them correct. It might be better to do them once in a clang-tidy PR, so that reviewers don't need to review these type changes twice.

[LinZhihao-723]

Last one missed suggestion and we're ready to merge.

[LinZhihao-723]

For the PR title, how about:

refactor: Use trailing return types for `void` functions; Use `auto` to simplify type declarations in the touched files.