Skip to content

Conversation

@hendrikmuhs
Copy link
Contributor

add cpp linter workflow to run clang-tidy on PRs

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 74. Check the log or trigger a new build to see more.

std::string GetStatistics() const {
return dictionary_properties_->GetStatistics();
}
std::string GetStatistics() const { return dictionary_properties_->GetStatistics(); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'GetStatistics' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
std::string GetStatistics() const { return dictionary_properties_->GetStatistics(); }
[[nodiscard]] std::string GetStatistics() const { return dictionary_properties_->GetStatistics(); }

const std::string& GetManifest() const {
return dictionary_properties_->GetManifest();
}
const std::string& GetManifest() const { return dictionary_properties_->GetManifest(); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'GetManifest' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
const std::string& GetManifest() const { return dictionary_properties_->GetManifest(); }
[[nodiscard]] const std::string& GetManifest() const { return dictionary_properties_->GetManifest(); }

const uint64_t GetVersion() const {
return dictionary_properties_->GetVersion();
}
const uint64_t GetVersion() const { return dictionary_properties_->GetVersion(); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'GetVersion' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
const uint64_t GetVersion() const { return dictionary_properties_->GetVersion(); }
[[nodiscard]] const uint64_t GetVersion() const { return dictionary_properties_->GetVersion(); }

const uint64_t GetVersion() const {
return dictionary_properties_->GetVersion();
}
const uint64_t GetVersion() const { return dictionary_properties_->GetVersion(); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: return type 'const uint64_t' (aka 'const unsigned long') is 'const'-qualified at the top level, which may reduce code readability without improving const correctness [readability-const-return-type]

Suggested change
const uint64_t GetVersion() const { return dictionary_properties_->GetVersion(); }
uint64_t GetVersion() const { return dictionary_properties_->GetVersion(); }

const dictionary_properties_t& GetDictionaryProperties() const {
return dictionary_properties_;
}
const dictionary_properties_t& GetDictionaryProperties() const { return dictionary_properties_; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'GetDictionaryProperties' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
const dictionary_properties_t& GetDictionaryProperties() const { return dictionary_properties_; }
[[nodiscard]] const dictionary_properties_t& GetDictionaryProperties() const { return dictionary_properties_; }


explicit ComparableStateTraverser(const automata_t f, const uint64_t start_state,
traversal::TraversalPayload<transition_t> &&payload, const bool advance = true,
traversal::TraversalPayload<transition_t>&& payload, const bool advance = true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "keyvi::dictionary::fsa::traversal::TraversalPayload" is directly included [misc-include-cleaner]

keyvi/include/keyvi/dictionary/fsa/comparable_state_traverser.h:33:

- #include "keyvi/dictionary/fsa/traverser_types.h"
+ #include "keyvi/dictionary/fsa/traversal/traversal_base.h"
+ #include "keyvi/dictionary/fsa/traverser_types.h"

label_t GetStateLabel() const { return state_traverser_.GetStateLabel(); }

const std::vector<label_t> &GetStateLabels() const { return label_stack_; }
const std::vector<label_t>& GetStateLabels() const { return label_stack_; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'GetStateLabels' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
const std::vector<label_t>& GetStateLabels() const { return label_stack_; }
[[nodiscard]] const std::vector<label_t>& GetStateLabels() const { return label_stack_; }

friend class matching::NearMatching;

traversal::TraversalState<transition_t> &GetStates() { return state_traverser_.GetStates(); }
traversal::TraversalState<transition_t>& GetStates() { return state_traverser_.GetStates(); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "keyvi::dictionary::fsa::traversal::TraversalState" is directly included [misc-include-cleaner]

  traversal::TraversalState<transition_t>& GetStates() { return state_traverser_.GetStates(); }
             ^

traversal::TraversalPayload<transition_t>& GetTraversalPayload() { return state_traverser_.GetTraversalPayload(); }

const traversal::TraversalPayload<transition_t> &GetTraversalPayload() const {
const traversal::TraversalPayload<transition_t>& GetTraversalPayload() const {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'GetTraversalPayload' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
const traversal::TraversalPayload<transition_t>& GetTraversalPayload() const {
[[nodiscard]] const traversal::TraversalPayload<transition_t>& GetTraversalPayload() const {


inline bool CompareWeights(const traversal::TraversalState<traversal::WeightedTransition> &i,
const traversal::TraversalState<traversal::WeightedTransition> &j) {
inline bool CompareWeights(const traversal::TraversalState<traversal::WeightedTransition>& i,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "keyvi::dictionary::fsa::traversal::TraversalState" is directly included [misc-include-cleaner]

inline bool CompareWeights(const traversal::TraversalState<traversal::WeightedTransition>& i,
                                            ^

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 64. Check the log or trigger a new build to see more.


inline bool CompareWeights(const traversal::TraversalState<traversal::WeightedTransition> &i,
const traversal::TraversalState<traversal::WeightedTransition> &j) {
inline bool CompareWeights(const traversal::TraversalState<traversal::WeightedTransition>& i,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "keyvi::dictionary::fsa::traversal::WeightedTransition" is directly included [misc-include-cleaner]

keyvi/include/keyvi/dictionary/fsa/comparable_state_traverser.h:33:

- #include "keyvi/dictionary/fsa/traverser_types.h"
+ #include "keyvi/dictionary/fsa/traversal/weighted_traversal.h"
+ #include "keyvi/dictionary/fsa/traverser_types.h"


inline bool CompareWeights(const traversal::TraversalState<traversal::WeightedTransition> &i,
const traversal::TraversalState<traversal::WeightedTransition> &j) {
inline bool CompareWeights(const traversal::TraversalState<traversal::WeightedTransition>& i,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter name 'i' is too short, expected at least 3 characters [readability-identifier-length]

inline bool CompareWeights(const traversal::TraversalState<traversal::WeightedTransition>& i,
                                                                                           ^

inline bool CompareWeights(const traversal::TraversalState<traversal::WeightedTransition> &i,
const traversal::TraversalState<traversal::WeightedTransition> &j) {
inline bool CompareWeights(const traversal::TraversalState<traversal::WeightedTransition>& i,
const traversal::TraversalState<traversal::WeightedTransition>& j) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter name 'j' is too short, expected at least 3 characters [readability-identifier-length]

                           const traversal::TraversalState<traversal::WeightedTransition>& j) {
                                                                                           ^

delete current_generation_;
for (MinimizationHash<EntryT> *generation : generations_) {
for (MinimizationHash<EntryT>* generation : generations_) {
delete generation;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

      delete generation;
      ^
Additional context

keyvi/include/keyvi/dictionary/fsa/internal/lru_generation_cache.h:68: variable declared here

    for (MinimizationHash<EntryT>* generation : generations_) {
         ^

current_generation_->Clear();
for (MinimizationHash<EntryT> *generation : generations_) {
for (MinimizationHash<EntryT>* generation : generations_) {
delete generation;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: deleting a pointer through a type that is not marked 'gsl::owner<>'; consider using a smart pointer instead [cppcoreguidelines-owning-memory]

      delete generation;
      ^
Additional context

keyvi/include/keyvi/dictionary/fsa/internal/lru_generation_cache.h:128: variable declared here

    for (MinimizationHash<EntryT>* generation : generations_) {
         ^

size_t GetChunkSizeExternalTransitions() const {
return transitions_extern_->GetChunkSize();
}
size_t GetChunkSizeExternalTransitions() const { return transitions_extern_->GetChunkSize(); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'GetChunkSizeExternalTransitions' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
size_t GetChunkSizeExternalTransitions() const { return transitions_extern_->GetChunkSize(); }
[[nodiscard]] size_t GetChunkSizeExternalTransitions() const { return transitions_extern_->GetChunkSize(); }

}

StateTraverser(automata_t f, const uint64_t start_state, traversal::TraversalPayload<TransitionT> &&payload,
StateTraverser(automata_t f, const uint64_t start_state, traversal::TraversalPayload<TransitionT>&& payload,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "uint64_t" is directly included [misc-include-cleaner]

keyvi/include/keyvi/dictionary/fsa/state_traverser.h:27:

- #include <utility>
+ #include <cstdint>
+ #include <utility>

}

StateTraverser(automata_t f, const uint64_t start_state, traversal::TraversalPayload<TransitionT> &&payload,
StateTraverser(automata_t f, const uint64_t start_state, traversal::TraversalPayload<TransitionT>&& payload,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter name 'f' is too short, expected at least 3 characters [readability-identifier-length]

  StateTraverser(automata_t f, const uint64_t start_state, traversal::TraversalPayload<TransitionT>&& payload,
                            ^

}

StateTraverser(automata_t f, const uint64_t start_state, traversal::TraversalPayload<TransitionT> &&payload,
StateTraverser(automata_t f, const uint64_t start_state, traversal::TraversalPayload<TransitionT>&& payload,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: the parameter 'f' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param]

Suggested change
StateTraverser(automata_t f, const uint64_t start_state, traversal::TraversalPayload<TransitionT>&& payload,
StateTraverser(const automata_t& f, const uint64_t start_state, traversal::TraversalPayload<TransitionT>&& payload,

StateTraverser(const StateTraverser& that) = delete;

StateTraverser(StateTraverser &&other)
StateTraverser(StateTraverser&& other)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: move constructors should be marked noexcept [cppcoreguidelines-noexcept-move-operations]

keyvi/include/keyvi/dictionary/fsa/state_traverser.h:87:

-       : fsa_(other.fsa_),
+  noexcept       : fsa_(other.fsa_),

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 56. Check the log or trigger a new build to see more.

const uint64_t GetVersion() const {
return dictionary_properties_->GetVersion();
}
[[nodiscard]] const uint64_t GetVersion() const { return dictionary_properties_->GetVersion(); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: return type 'const uint64_t' (aka 'const unsigned long') is 'const'-qualified at the top level, which may reduce code readability without improving const correctness [readability-const-return-type]

Suggested change
[[nodiscard]] const uint64_t GetVersion() const { return dictionary_properties_->GetVersion(); }
[[nodiscard]] uint64_t GetVersion() const { return dictionary_properties_->GetVersion(); }


inline bool CompareWeights(const traversal::TraversalState<traversal::WeightedTransition> &i,
const traversal::TraversalState<traversal::WeightedTransition> &j) {
inline bool CompareWeights(const traversal::TraversalState<traversal::WeightedTransition>& i,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "keyvi::dictionary::fsa::traversal::WeightedTransition" is directly included [misc-include-cleaner]

keyvi/include/keyvi/dictionary/fsa/comparable_state_traverser.h:34:

- #include "keyvi/dictionary/fsa/traverser_types.h"
+ #include "keyvi/dictionary/fsa/traversal/weighted_traversal.h"
+ #include "keyvi/dictionary/fsa/traverser_types.h"

template <class innerTraverserType>
friend class ComparableStateTraverser;
const traversal::TraversalStack<TransitionT> &GetStack() const { return stack_; }
const traversal::TraversalStack<TransitionT>& GetStack() const { return stack_; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'GetStack' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
const traversal::TraversalStack<TransitionT>& GetStack() const { return stack_; }
[[nodiscard]] const traversal::TraversalStack<TransitionT>& GetStack() const { return stack_; }

traversal::TraversalPayload<TransitionT>& GetTraversalPayload() { return stack_.traversal_stack_payload; }

const traversal::TraversalPayload<TransitionT> &GetTraversalPayload() const { return stack_.traversal_stack_payload; }
const traversal::TraversalPayload<TransitionT>& GetTraversalPayload() const { return stack_.traversal_stack_payload; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'GetTraversalPayload' should be marked [[nodiscard]] [modernize-use-nodiscard]

Suggested change
const traversal::TraversalPayload<TransitionT>& GetTraversalPayload() const { return stack_.traversal_stack_payload; }
[[nodiscard]] const traversal::TraversalPayload<TransitionT>& GetTraversalPayload() const { return stack_.traversal_stack_payload; }


struct TraverserCompare {
bool operator()(const traverser_t &t1, const traverser_t &t2) const { return *t1 > *t2; }
bool operator()(const traverser_t& t1, const traverser_t& t2) const { return *t1 > *t2; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter name 't1' is too short, expected at least 3 characters [readability-identifier-length]

    bool operator()(const traverser_t& t1, const traverser_t& t2) const { return *t1 > *t2; }
                                       ^


struct TraverserCompare {
bool operator()(const traverser_t &t1, const traverser_t &t2) const { return *t1 > *t2; }
bool operator()(const traverser_t& t1, const traverser_t& t2) const { return *t1 > *t2; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: parameter name 't2' is too short, expected at least 3 characters [readability-identifier-length]

    bool operator()(const traverser_t& t1, const traverser_t& t2) const { return *t1 > *t2; }
                                                              ^

using heap_t =
boost::heap::skew_heap<traverser_t, boost::heap::compare<TraverserCompare>, boost::heap::mutable_<true>>;
explicit ZipStateTraverser(const std::vector<automata_t> &fsas, const bool advance = true) {
explicit ZipStateTraverser(const std::vector<automata_t>& fsas, const bool advance = true) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: constructor does not initialize these fields: order_ [cppcoreguidelines-pro-type-member-init]

keyvi/include/keyvi/dictionary/fsa/zip_state_traverser.h:233:

-   size_t order_;
+   size_t order_{};

explicit ZipStateTraverser(const std::vector<automata_t>& fsas, const bool advance = true) {
size_t order = 0;
for (const automata_t &f : fsas) {
for (const automata_t& f : fsas) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable name 'f' is too short, expected at least 3 characters [readability-identifier-length]

    for (const automata_t& f : fsas) {
                           ^

}

explicit ZipStateTraverser(const std::vector<std::pair<automata_t, uint64_t>> &fsa_start_state_pairs,
explicit ZipStateTraverser(const std::vector<std::pair<automata_t, uint64_t>>& fsa_start_state_pairs,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: constructor does not initialize these fields: order_ [cppcoreguidelines-pro-type-member-init]

  explicit ZipStateTraverser(const std::vector<std::pair<automata_t, uint64_t>>& fsa_start_state_pairs,
  ^

}

explicit ZipStateTraverser(const std::vector<std::pair<automata_t, uint64_t>> &fsa_start_state_pairs,
explicit ZipStateTraverser(const std::vector<std::pair<automata_t, uint64_t>>& fsa_start_state_pairs,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: no header providing "uint64_t" is directly included [misc-include-cleaner]

keyvi/include/keyvi/dictionary/fsa/zip_state_traverser.h:28:

- #include <cstring>
+ #include <cstdint>
+ #include <cstring>

@hendrikmuhs
Copy link
Contributor Author

close for #376

@hendrikmuhs hendrikmuhs deleted the clang-tidy-workflow branch October 21, 2025 05:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant