Skip to content

Commit

Permalink
Fix a few things found using clang-tidy with the bant-generated compi…
Browse files Browse the repository at this point in the history
…lation DB.

Can't switch entirely yet, as it does not emit proper include directories
for targets that have an include = [] defined.
  • Loading branch information
hzeller committed Jun 19, 2024
1 parent 9250d3e commit d08ed3a
Show file tree
Hide file tree
Showing 23 changed files with 65 additions and 16 deletions.
8 changes: 8 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Checks: >
-readability-identifier-length,
-readability-implicit-bool-conversion,
-readability-magic-numbers,
-readability-named-parameter,
-readability-static-definition-in-anonymous-namespace,
-readability-uppercase-literal-suffix,
-readability-use-anyofallof,
Expand All @@ -38,3 +39,10 @@ Checks: >
-misc-no-recursion,
-misc-unused-parameters,
-misc-use-anonymous-namespace,
CheckOptions:
# Structs can not have non-public memebers, so they should be public.
# but clang-tidy warns about them anyway as being visible.
# clang-tidy can't distinguish between classes and structs, so approximate
# with this:
- key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic
value: '1'
3 changes: 3 additions & 0 deletions bant/explore/header-providers.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@
#include <optional>
#include <ostream>
#include <string>
#include <string_view>

#include "absl/container/btree_set.h"
#include "bant/frontend/parsed-project.h"
#include "bant/session.h"
#include "bant/types-bazel.h"
#include "bant/types.h"

// TODO: Given that this not only provides HeaderToLibMapping but also
Expand Down
7 changes: 3 additions & 4 deletions bant/explore/query-utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
#include <functional>
#include <initializer_list>
#include <string_view>
#include <vector>

#include "bant/frontend/ast.h"

namespace bant {
namespace query {
namespace bant::query {
// A Smörgåsbord of keyword parameters found for binaries, cc_libraries rules
// and other 'calls' to rules we look at. Starts to get a bit crowded (but
// is also cheap, as a instance is re-used and only passed by reference).
Expand Down Expand Up @@ -73,7 +73,6 @@ std::vector<std::string_view> ExtractStringList(List *list);
// Similar to ExtractStringList(), but append to vector.
void AppendStringList(List *list, std::vector<std::string_view> &append_to);

} // namespace query
} // namespace bant
} // namespace bant::query

#endif // BANT_QUERY_UTILS_
2 changes: 2 additions & 0 deletions bant/frontend/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -109,11 +109,13 @@ cc_library(
":parser",
":source-locator",
"//bant:session",
"//bant:types",
"//bant:types-bazel",
"//bant:workspace",
"//bant/explore:query-utils",
"//bant/util:disjoint-range-map",
"//bant/util:file-utils",
"//bant/util:memory",
"//bant/util:stat",
"@abseil-cpp//absl/log:check",
],
Expand Down
14 changes: 7 additions & 7 deletions bant/frontend/ast.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#ifndef BANT_AST_H_
#define BANT_AST_H_

#include <cstddef>
#include <cstdint>
#include <ostream>
#include <string_view>
Expand Down Expand Up @@ -150,6 +151,7 @@ class UnaryExpr : public Node {
friend class BaseNodeReplacementVisitor;
explicit UnaryExpr(TokenType op, Node *n) : node_(n), op_(op) {}

private:
Node *node_;
const TokenType op_;
};
Expand All @@ -163,8 +165,8 @@ class BinNode : public Node {
friend class BaseNodeReplacementVisitor;
BinNode(Node *lhs, Node *rhs) : left_(lhs), right_(rhs) {}

Node *left_;
Node *right_;
Node *left_; // NOLINT(misc-non-private-member-variables-in-classes)
Node *right_; // NOLINT(misc-non-private-member-variables-in-classes)
};

// Generally some tree element that takes two nodes
Expand Down Expand Up @@ -317,7 +319,7 @@ class VoidVisitor {
virtual void VisitIdentifier(Identifier *) = 0; // Leaf.

// Utility function: if node exists, walk and return 'true'.
inline bool WalkNonNull(Node *node) {
bool WalkNonNull(Node *node) {
if (node) node->Accept(this);
return node;
}
Expand Down Expand Up @@ -368,9 +370,7 @@ class NodeVisitor {
virtual Node *VisitIdentifier(Identifier *) = 0; // Leaf.

// Utility function: if node exists, walk and return valud from visit.
inline Node *WalkNonNull(Node *node) {
return node ? node->Accept(this) : node;
}
Node *WalkNonNull(Node *node) { return node ? node->Accept(this) : node; }
};

// Replace nodes with whatever walk on that node yielded.
Expand Down Expand Up @@ -429,7 +429,7 @@ class BaseNodeReplacementVisitor : public NodeVisitor {
Node *VisitIdentifier(Identifier *i) override { return i; }

private:
inline void ReplaceWalk(Node **n) { *n = WalkNonNull(*n); }
void ReplaceWalk(Node **n) { *n = WalkNonNull(*n); }
};

class PrintVisitor : public BaseVoidVisitor {
Expand Down
1 change: 1 addition & 0 deletions bant/frontend/elaboration.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "bant/frontend/ast.h"
#include "bant/frontend/parsed-project.h"
#include "bant/session.h"
#include "bant/types-bazel.h"

namespace bant {

Expand Down
2 changes: 2 additions & 0 deletions bant/frontend/linecolumn-map.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
#ifndef BANT_LINE_COLUMN_MAP_
#define BANT_LINE_COLUMN_MAP_

#include <cstddef>
#include <string_view>
#include <vector>

#include "bant/frontend/source-locator.h" // Provides LineColumn/Range
Expand Down
1 change: 1 addition & 0 deletions bant/frontend/named-content.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#ifndef BANT_NAMED_CONTENT_
#define BANT_NAMED_CONTENT_

#include <cstddef>
#include <string>
#include <string_view>

Expand Down
7 changes: 7 additions & 0 deletions bant/frontend/parsed-project.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,19 @@
#ifndef BANT_PROJECT_PARDER_
#define BANT_PROJECT_PARDER_

#include <memory>
#include <ostream>
#include <string>
#include <string_view>
#include <utility>

#include "bant/frontend/ast.h"
#include "bant/frontend/named-content.h"
#include "bant/frontend/source-locator.h"
#include "bant/session.h"
#include "bant/types-bazel.h"
#include "bant/types.h"
#include "bant/util/arena.h"
#include "bant/util/disjoint-range-map.h"
#include "bant/util/file-utils.h"
#include "bant/workspace.h"
Expand All @@ -45,9 +50,11 @@ class ParsedBuildFile {

std::string_view name() const { return source_.source_name(); }

// NOLINTBEGIN(misc-non-private-member-variables-in-classes) // TODO: fix
BazelPackage package;
List *ast; // parsed AST. Content owned by arena in ParsedProject
std::string errors; // List of errors if observed (todo: make actual list)
// NOLINTEND(misc-non-private-member-variables-in-classes)

private:
friend class ParsedProject; // It is allowed to access source_ directly.
Expand Down
6 changes: 5 additions & 1 deletion bant/frontend/parsed-project_testutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
#ifndef BANT_PARSED_PROJECT_TESTUTIL_
#define BANT_PARSED_PROJECT_TESTUTIL_

#include <iostream>
#include <string>
#include <string_view>

#include "absl/strings/str_cat.h"
#include "bant/frontend/elaboration.h"
#include "bant/frontend/parsed-project.h"
Expand All @@ -34,7 +38,7 @@ class ParsedProjectTestUtil {
auto package_or = BazelPackage::ParseFrom(package_str);
if (!package_or.has_value()) return nullptr;
SessionStreams streams(&std::cerr, &std::cerr);
std::string fake_filename = absl::StrCat(package_str, "/BUILD");
const std::string fake_filename = absl::StrCat(package_str, "/BUILD");
return project_.AddBuildFileContent(streams, *package_or, fake_filename,
std::string(content));
}
Expand Down
2 changes: 1 addition & 1 deletion bant/frontend/scanner.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class Scanner {
// All tokens returned by the Scanner are sub-string_views of the larger
// content; this allows correspondence with the original text to extract
// source.Loc() information.
Scanner(NamedLineIndexedContent &source);
explicit Scanner(NamedLineIndexedContent &source);

// Advance to next token and return it.
Token Next();
Expand Down
2 changes: 2 additions & 0 deletions bant/frontend/source-locator.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
#ifndef BANT_SOURCE_LOCATOR_
#define BANT_SOURCE_LOCATOR_

#include <ostream>
#include <string>
#include <string_view>

namespace bant {
Expand Down
1 change: 1 addition & 0 deletions bant/session.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <memory>
#include <ostream>
#include <string_view>
#include <vector>

#include "bant/output-format.h"
#include "bant/types.h"
Expand Down
1 change: 1 addition & 0 deletions bant/tool/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ cc_library(
hdrs = ["edit-callback_testutil.h"],
deps = [
":edit-callback",
"//bant:types-bazel",
"@abseil-cpp//absl/container:flat_hash_set",
"@abseil-cpp//absl/strings",
"@googletest//:gtest",
Expand Down
1 change: 1 addition & 0 deletions bant/tool/canon-targets.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <cstdlib>

#include "bant/frontend/parsed-project.h"
#include "bant/session.h"
#include "bant/tool/edit-callback.h"
#include "bant/types-bazel.h"

Expand Down
2 changes: 2 additions & 0 deletions bant/tool/compilation-db.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
// - compilation db should also include all external projects.
// - more readable output of json with indentation and stuff without having
// to resort to external lib.
// - if there are any cc-libraries with includes = [], add these
namespace bant {

// Make quoted strings a little less painful to read and write in C++
Expand Down Expand Up @@ -81,6 +82,7 @@ static std::string CollectAllExternallIncDirs(const ParsedProject &project) {
std::stringstream out;
const BazelWorkspace &workspace = project.workspace();
absl::flat_hash_set<std::string> already_seen;
// TODO: if there are any cc-libraries with includes = [], add these
for (const auto &[_, parsed_package] : project.ParsedFiles()) {
const BazelPackage &current_package = parsed_package->package;
query::FindTargets(
Expand Down
7 changes: 6 additions & 1 deletion bant/tool/dwyu-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@
#ifndef BANT_TOOL_DWYU_INTERNAL_
#define BANT_TOOL_DWYU_INTERNAL_

#include <set>
#include <cstddef>
#include <optional>
#include <string>
#include <string_view>
#include <vector>

#include "absl/container/btree_map.h"
#include "absl/container/btree_set.h"
Expand All @@ -28,6 +32,7 @@
#include "bant/session.h"
#include "bant/tool/edit-callback.h"
#include "bant/types-bazel.h"
#include "bant/types.h"

namespace bant {
// The DWYUGenerator is the underlying implementation, for which
Expand Down
2 changes: 2 additions & 0 deletions bant/tool/dwyu.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@
#ifndef BANT_TOOL_DWYU_
#define BANT_TOOL_DWYU_

#include <cstddef>
#include <string_view>
#include <vector>

#include "bant/frontend/named-content.h"
#include "bant/frontend/parsed-project.h"
#include "bant/session.h"
#include "bant/tool/edit-callback.h"
Expand Down
1 change: 1 addition & 0 deletions bant/tool/edit-callback.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#define BANT_TOOL_EDIT_CALLBACK_

#include <functional>
#include <ostream>
#include <string_view>

#include "bant/types-bazel.h"
Expand Down
4 changes: 4 additions & 0 deletions bant/tool/edit-callback_testutil.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@
#ifndef BANT_TOOL_EDIT_CALLBACK_TESTUTIL_
#define BANT_TOOL_EDIT_CALLBACK_TESTUTIL_

#include <string>
#include <string_view>

#include "absl/container/flat_hash_set.h"
#include "absl/strings/str_cat.h"
#include "bant/tool/edit-callback.h"
#include "bant/types-bazel.h"
#include "gtest/gtest.h"

namespace bant {
Expand Down
1 change: 1 addition & 0 deletions bant/types-bazel.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <ostream>
#include <string>
#include <string_view>
#include <utility>

#include "re2/re2.h"

Expand Down
1 change: 1 addition & 0 deletions bant/util/arena-container.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#define BANT_ARENA_CONTAINER_H

#include <cassert>
#include <cstddef>
#include <cstdint>

#include "bant/util/arena.h"
Expand Down
5 changes: 3 additions & 2 deletions bant/util/stat.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include <cstddef>
#include <optional>
#include <ostream>
#include <string>
#include <string_view>

Expand All @@ -31,7 +32,7 @@ namespace bant {
// Add time encountered in the scope to duration.
class ScopedTimer {
public:
ScopedTimer(absl::Duration *to_update)
explicit ScopedTimer(absl::Duration *to_update)
: to_update_(to_update), start_(absl::Now()) {}
~ScopedTimer() { *to_update_ += absl::Now() - start_; }

Expand All @@ -41,7 +42,7 @@ class ScopedTimer {
};

struct Stat {
Stat(std::string_view subject) : subject(subject) {}
explicit Stat(std::string_view subject) : subject(subject) {}
std::string_view subject; // Descriptive name this stat is counting.

int count = 0;
Expand Down

0 comments on commit d08ed3a

Please sign in to comment.