Skip to content

Commit

Permalink
Define LineNumberSet as its own class (subclass of IntervalSet<int>).
Browse files Browse the repository at this point in the history
Combine near-equivalent typedefs in the codebase.

PiperOrigin-RevId: 316777543
  • Loading branch information
fangism authored and hzeller committed Jun 17, 2020
1 parent e1b660b commit 0dabd1b
Show file tree
Hide file tree
Showing 15 changed files with 50 additions and 22 deletions.
2 changes: 2 additions & 0 deletions common/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ cc_library(
"//bazel:flex",
"//common/lexer:flex_lexer_adapter",
"//common/lexer:token_stream_adapter",
"//common/strings:position",
"//common/text:text_structure",
"//common/text:token_info",
"//common/text:token_stream_view",
Expand All @@ -86,6 +87,7 @@ cc_library(
":command_file_lexer",
"//common/strings:comment_utils",
"//common/strings:line_column_map",
"//common/strings:position",
"//common/text:text_structure",
"//common/text:token_info",
"//common/text:token_stream_view",
Expand Down
4 changes: 2 additions & 2 deletions common/analysis/lint_waiver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void LintWaiver::WaiveOneLine(absl::string_view rule_name, size_t line_number) {

void LintWaiver::WaiveLineRange(absl::string_view rule_name, size_t line_begin,
size_t line_end) {
LineSet& line_set = waiver_map_[rule_name];
LineNumberSet& line_set = waiver_map_[rule_name];
line_set.Add({line_begin, line_end});
}

Expand Down Expand Up @@ -78,7 +78,7 @@ void LintWaiver::RegexToLines(absl::string_view contents,
bool LintWaiver::RuleIsWaivedOnLine(absl::string_view rule_name,
size_t line_number) const {
const auto* line_set = verible::container::FindOrNull(waiver_map_, rule_name);
return line_set != nullptr && LineSetContains(*line_set, line_number);
return line_set != nullptr && LineNumberSetContains(*line_set, line_number);
}

bool LintWaiver::Empty() const {
Expand Down
11 changes: 5 additions & 6 deletions common/analysis/lint_waiver.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <vector>

#include "absl/strings/string_view.h"
#include "common/strings/position.h"
#include "common/text/text_structure.h"
#include "common/text/token_stream_view.h"
#include "common/util/container_util.h"
Expand All @@ -32,9 +33,6 @@ namespace verible {
// LintWaiver maintains a set of line ranges per lint rule that should be
// exempt from each rule.
class LintWaiver {
// Compact set of line numbers.
// TODO(b/156991337): combine with other definition of LineNumberSet
using LineSet = IntervalSet<size_t>;
using RegexVector = std::vector<const std::regex*>;

public:
Expand Down Expand Up @@ -73,12 +71,13 @@ class LintWaiver {

// TODO(hzeller): The following methods break abstraction and are only
// for performance. Reconsider if this is worth it.
const LineSet* LookupLineSet(absl::string_view rule_name) const {
const LineNumberSet* LookupLineNumberSet(absl::string_view rule_name) const {
return verible::container::FindOrNull(waiver_map_, rule_name);
}

// Test if a particular line is included in the set.
static bool LineSetContains(const LineSet& line_set, size_t line) {
static bool LineNumberSetContains(const LineNumberSet& line_set,
size_t line) {
return line_set.Contains(line);
}

Expand All @@ -87,7 +86,7 @@ class LintWaiver {
// string_view because the static strings for each lint rule class exist,
// and will outlive all LintWaiver objects. This applies to both waiver_map_
// and waiver_re_map_.
std::map<absl::string_view, LineSet> waiver_map_;
std::map<absl::string_view, LineNumberSet> waiver_map_;
std::map<absl::string_view, RegexVector> waiver_re_map_;

std::map<std::string, std::regex> regex_cache_;
Expand Down
20 changes: 20 additions & 0 deletions common/strings/position.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
#ifndef VERIBLE_COMMON_STRINGS_POSITION_H_
#define VERIBLE_COMMON_STRINGS_POSITION_H_

#include <initializer_list>

#include "absl/strings/string_view.h"
#include "common/util/interval_set.h"

Expand Down Expand Up @@ -45,6 +47,24 @@ class ByteOffsetSet : public verible::IntervalSet<int> {
: impl_type(ranges) {}
};

// Collection of ranges of line numbers.
// Intentionally defining this as its own class instead of a typedef to
// avoid potential confusion with other IntervalSet<int>.
// Mismatches will be caught as type errors.
class LineNumberSet : public verible::IntervalSet<int> {
typedef verible::IntervalSet<int> impl_type;

public:
LineNumberSet() = default;

explicit LineNumberSet(const impl_type& iset) : impl_type(iset) {}

// This constructor can initialize from a sequence of pairs, e.g.
// LineNumberSet s{{0,1}, {4,7}, {8,10}};
LineNumberSet(std::initializer_list<verible::Interval<int>> ranges)
: impl_type(ranges) {}
};

} // namespace verible

#endif // VERIBLE_COMMON_STRINGS_POSITION_H_
5 changes: 3 additions & 2 deletions verilog/analysis/verilog_linter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,8 @@ static void AppendLintRuleStatuses(
std::vector<LintRuleStatus>* cumulative_statuses) {
for (const auto& status : new_statuses) {
cumulative_statuses->push_back(status);
const auto* waived_lines = waivers.LookupLineSet(status.lint_rule_name);
const auto* waived_lines =
waivers.LookupLineNumberSet(status.lint_rule_name);
if (waived_lines) {
cumulative_statuses->back().WaiveViolations(
[&](const verible::LintViolation& violation) {
Expand All @@ -208,7 +209,7 @@ static void AppendLintRuleStatuses(
const size_t line = line_map(offset).line;
// Check that line number against the set of waived lines.
const bool waived =
LintWaiver::LineSetContains(*waived_lines, line);
LintWaiver::LineNumberSetContains(*waived_lines, line);
VLOG(2) << "Violation of " << status.lint_rule_name
<< " rule on line " << line + 1
<< (waived ? " is waived." : " is not waived.");
Expand Down
2 changes: 2 additions & 0 deletions verilog/formatting/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ cc_test(
deps = [
":format_style",
":formatter",
"//common/strings:position",
"//common/text:text_structure",
"//common/util:logging",
"//verilog/analysis:verilog_analyzer",
Expand All @@ -175,6 +176,7 @@ cc_test(
deps = [
":format_style",
":formatter",
"//common/strings:position",
"//common/util:logging",
"@com_google_absl//absl/status",
"@com_google_absl//absl/strings",
Expand Down
2 changes: 1 addition & 1 deletion verilog/formatting/comment_controls.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ ByteOffsetSet DisableFormattingRanges(absl::string_view text,
}

ByteOffsetSet EnabledLinesToDisabledByteRanges(
const LineNumberSet& line_numbers,
const verible::LineNumberSet& line_numbers,
const verible::LineColumnMap& line_column_map) {
// Interpret empty line numbers as enabling all lines for formatting.
if (line_numbers.empty()) return ByteOffsetSet();
Expand Down
8 changes: 2 additions & 6 deletions verilog/formatting/comment_controls.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,13 @@

#include "absl/strings/string_view.h"
#include "common/strings/line_column_map.h"
#include "common/strings/position.h" // for ByteOffsetSet
#include "common/strings/position.h" // for ByteOffsetSet, LineNumberSet
#include "common/text/token_stream_view.h"
#include "common/util/interval_set.h"

namespace verilog {
namespace formatter {

// Collection of line numbers, 1-based.
// TODO(b/156991337): combine with other definition for LineSet
using LineNumberSet = verible::IntervalSet<int>;

// Returns a representation of byte offsets where true (membership) means
// formatting is disabled.
verible::ByteOffsetSet DisableFormattingRanges(
Expand All @@ -38,7 +34,7 @@ verible::ByteOffsetSet DisableFormattingRanges(

// Translates line numbers into a set of byte ranges to disable formatting.
verible::ByteOffsetSet EnabledLinesToDisabledByteRanges(
const LineNumberSet& line_numbers,
const verible::LineNumberSet& line_numbers,
const verible::LineColumnMap& line_column_map);

// Formats space between tokens while honoring formatting-disabled ranges.
Expand Down
1 change: 1 addition & 0 deletions verilog/formatting/comment_controls_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ using ::testing::ElementsAre;
using verible::ByteOffsetSet;
using verible::ExpectedTokenInfo;
using verible::LineColumnMap;
using verible::LineNumberSet;
using verible::TokenInfoTestData;

TEST(DisableFormattingRangesTest, EmptyFile) {
Expand Down
1 change: 1 addition & 0 deletions verilog/formatting/formatter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ using absl::StatusCode;

using verible::ByteOffsetSet;
using verible::ExpandableTreeView;
using verible::LineNumberSet;
using verible::MutableFormatTokenRange;
using verible::PartitionPolicyEnum;
using verible::PreFormatToken;
Expand Down
4 changes: 2 additions & 2 deletions verilog/formatting/formatter.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#include <vector>

#include "absl/status/status.h"
#include "verilog/formatting/comment_controls.h"
#include "common/strings/position.h"
#include "verilog/formatting/format_style.h"

namespace verilog {
Expand Down Expand Up @@ -71,7 +71,7 @@ struct ExecutionControl {
absl::Status FormatVerilog(absl::string_view text, absl::string_view filename,
const FormatStyle& style,
std::ostream& formatted_stream,
const LineNumberSet& lines = {},
const verible::LineNumberSet& lines = {},
const ExecutionControl& control = {});

} // namespace formatter
Expand Down
2 changes: 2 additions & 0 deletions verilog/formatting/formatter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "absl/status/status.h"
#include "absl/strings/match.h"
#include "absl/strings/string_view.h"
#include "common/strings/position.h"
#include "common/text/text_structure.h"
#include "common/util/logging.h"
#include "verilog/analysis/verilog_analyzer.h"
Expand All @@ -53,6 +54,7 @@ absl::Status VerifyFormatting(const verible::TextStructureView& text_structure,
namespace {

using absl::StatusCode;
using verible::LineNumberSet;

// Tests that clean output passes.
TEST(VerifyFormattingTest, NoError) {
Expand Down
3 changes: 2 additions & 1 deletion verilog/formatting/formatter_tuning_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "gtest/gtest.h"
#include "absl/status/status.h"
#include "absl/strings/string_view.h"
#include "common/strings/position.h"
#include "common/util/logging.h"
#include "verilog/formatting/format_style.h"

Expand All @@ -41,7 +42,7 @@ struct FormatterTestCase {
absl::string_view expected;
};

static const LineNumberSet kEnableAllLines;
static const verible::LineNumberSet kEnableAllLines;

// Tests in this file are intended to be sensitive to wrapping penalty tuning.
// These test cases should be kept short, small enough to be directed
Expand Down
1 change: 1 addition & 0 deletions verilog/tools/formatter/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ cc_binary(
srcs = ["verilog_format.cc"],
visibility = ["//visibility:public"], # for verilog_style_lint.bzl
deps = [
"//common/strings:position",
"//common/util:file_util",
"//common/util:init_command_line",
"//common/util:interval_set",
Expand Down
6 changes: 4 additions & 2 deletions verilog/tools/formatter/verilog_format.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include "absl/strings/str_join.h"
#include "absl/strings/str_split.h"
#include "absl/strings/string_view.h"
#include "common/strings/position.h"
#include "common/util/file_util.h"
#include "common/util/init_command_line.h"
#include "common/util/interval_set.h"
Expand All @@ -44,6 +45,7 @@
#include "verilog/formatting/formatter.h"

using absl::StatusCode;
using verible::LineNumberSet;
using verilog::formatter::ExecutionControl;
using verilog::formatter::FormatStyle;
using verilog::formatter::FormatVerilog;
Expand Down Expand Up @@ -130,7 +132,7 @@ std::ostream& FileMsg(absl::string_view filename) {
}

bool formatOneFile(absl::string_view filename,
const verilog::formatter::LineNumberSet& lines_to_format) {
const LineNumberSet& lines_to_format) {
const bool inplace = absl::GetFlag(FLAGS_inplace);
const bool is_stdin = filename == "-";
const auto& stdin_name = absl::GetFlag(FLAGS_stdin_name);
Expand Down Expand Up @@ -245,7 +247,7 @@ int main(int argc, char** argv) {
}

// Parse LineRanges into a line set, to validate the --lines flag(s)
verilog::formatter::LineNumberSet lines_to_format;
LineNumberSet lines_to_format;
if (!verible::ParseInclusiveRanges(
&lines_to_format, LineRanges::values.begin(),
LineRanges::values.end(), &std::cerr, '-')) {
Expand Down

0 comments on commit 0dabd1b

Please sign in to comment.