Skip to content

Commit

Permalink
Merge pull request #1277 from hzeller/20220318-document-preprocess
Browse files Browse the repository at this point in the history 
Start documenting more behavior in preprocessor unit test.
  • Loading branch information
@hzeller
hzeller authored Mar 18, 2022
2 parents 409f3d9 + c87a161 commit 4cccc6b
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 32 deletions.
16 changes: 8 additions & 8 deletions verilog/preprocessor/verilog_preprocess.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,15 +192,15 @@ absl::Status VerilogPreprocess::HandleTokenIterator(
}

// Stores a macro definition for later use.
std::unique_ptr<VerilogPreprocessError>
VerilogPreprocess::RegisterMacroDefinition(const MacroDefinition& definition) {
void VerilogPreprocess::RegisterMacroDefinition(
const MacroDefinition& definition) {
// For now, unconditionally register the macro definition, keeping the last
// definition if macro is re-defined.
const bool inserted = InsertOrUpdate(&preprocess_data_.macro_definitions,
definition.Name(), definition);
if (inserted) return nullptr;
return std::make_unique<VerilogPreprocessError>(definition.NameToken(),
"Re-defining macro");
if (inserted) return;
preprocess_data_.warnings.emplace_back(
VerilogPreprocessError(definition.NameToken(), "Re-defining macro"));
// TODO(hzeller): multiline warning with 'previously defined here' location
}

Expand Down Expand Up @@ -228,13 +228,13 @@ absl::Status VerilogPreprocess::HandleDefine(
preprocess_data_.errors.push_back(*parse_error_ptr);
return absl::InvalidArgumentError("Error parsing macro definition.");
}
RegisterMacroDefinition(macro_definition);

// For now, forward all definition tokens.
if (auto warning = RegisterMacroDefinition(macro_definition); warning) {
preprocess_data_.warnings.push_back(*warning);
}
for (const auto& token : define_tokens) {
preprocess_data_.preprocessed_token_stream.push_back(token);
}

return absl::OkStatus();
}

Expand Down
3 changes: 1 addition & 2 deletions verilog/preprocessor/verilog_preprocess.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,7 @@ class VerilogPreprocess {
static std::unique_ptr<VerilogPreprocessError> ParseMacroParameter(
TokenStreamView::const_iterator*, MacroParameterInfo*);

std::unique_ptr<VerilogPreprocessError> RegisterMacroDefinition(
const MacroDefinition&);
void RegisterMacroDefinition(const MacroDefinition&);

// Results of preprocessing
VerilogPreprocessData preprocess_data_;
Expand Down
84 changes: 62 additions & 22 deletions verilog/preprocessor/verilog_preprocess_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#include "verilog/preprocessor/verilog_preprocess.h"

#include <algorithm>
#include <map>
#include <vector>

Expand All @@ -24,6 +25,7 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "verilog/analysis/verilog_analyzer.h"
#include "verilog/parser/verilog_token_enum.h"

namespace verilog {
namespace {
Expand All @@ -35,21 +37,21 @@ using verible::container::FindOrNull;
class PreprocessorTester {
public:
explicit PreprocessorTester(const char* text)
: analyzer_(text, "<<inline-file>>"), status_() {
status_ = analyzer_.Analyze();
}
: analyzer_(text, "<<inline-file>>"), status_(analyzer_.Analyze()) {}

const VerilogPreprocessData& PreprocessorData() const {
return analyzer_.PreprocessorData();
}

const verible::TextStructureView& Data() const { return analyzer_.Data(); }

const absl::Status& Status() const { return status_; }

const VerilogAnalyzer& Analyzer() const { return analyzer_; }

private:
VerilogAnalyzer analyzer_;
absl::Status status_;
const absl::Status status_;
};

struct FailTest {
Expand Down Expand Up @@ -99,6 +101,13 @@ TEST(VerilogPreprocessTest, InvalidPreprocessorInputs) {
}
}

#define EXPECT_PARSE_OK() \
do { \
EXPECT_TRUE(tester.Status().ok()) << "Unexpected analyzer failure."; \
EXPECT_TRUE(tester.PreprocessorData().errors.empty()); \
EXPECT_TRUE(tester.Analyzer().GetRejectedTokens().empty()); \
} while (false)

// Verify that VerilogPreprocess works without any directives.
TEST(VerilogPreprocessTest, WorksWithoutDefinitions) {
const char* test_cases[] = {
Expand All @@ -109,11 +118,10 @@ TEST(VerilogPreprocessTest, WorksWithoutDefinitions) {
};
for (const auto& test_case : test_cases) {
PreprocessorTester tester(test_case);
EXPECT_TRUE(tester.Status().ok());
EXPECT_PARSE_OK();

const auto& definitions = tester.PreprocessorData().macro_definitions;
EXPECT_TRUE(definitions.empty());
EXPECT_TRUE(tester.PreprocessorData().errors.empty());
EXPECT_TRUE(tester.Analyzer().GetRejectedTokens().empty());
}
}

Expand All @@ -128,9 +136,8 @@ TEST(VerilogPreprocessTest, OneMacroDefinitionNoParamsNoValue) {
};
for (const auto& test_case : test_cases) {
PreprocessorTester tester(test_case);
EXPECT_TRUE(tester.Status().ok());
EXPECT_TRUE(tester.PreprocessorData().errors.empty());
EXPECT_TRUE(tester.Analyzer().GetRejectedTokens().empty());
EXPECT_PARSE_OK();

const auto& definitions = tester.PreprocessorData().macro_definitions;
EXPECT_THAT(definitions, ElementsAre(Pair("FOOOO", testing::_)));
auto macro = FindOrNull(definitions, "FOOOO");
Expand All @@ -145,10 +152,9 @@ TEST(VerilogPreprocessTest, OneMacroDefinitionNoParamsSimpleValue) {
PreprocessorTester tester(
"module foo;\nendmodule\n"
"`define FOOOO \"bar\"\n");
EXPECT_PARSE_OK();

const auto& definitions = tester.PreprocessorData().macro_definitions;
EXPECT_TRUE(tester.Status().ok()) << "Unexpected analyzer failure.";
EXPECT_TRUE(tester.PreprocessorData().errors.empty());
EXPECT_TRUE(tester.Analyzer().GetRejectedTokens().empty());
EXPECT_THAT(definitions, ElementsAre(Pair("FOOOO", testing::_)));
auto macro = FindOrNull(definitions, "FOOOO");
ASSERT_NE(macro, nullptr);
Expand All @@ -161,10 +167,9 @@ TEST(VerilogPreprocessTest, OneMacroDefinitionOneParamWithValue) {
PreprocessorTester tester(
"module foo;\nendmodule\n"
"`define FOOOO(x) (x+1)\n");
EXPECT_PARSE_OK();

const auto& definitions = tester.PreprocessorData().macro_definitions;
EXPECT_TRUE(tester.Status().ok()) << "Unexpected analyzer failure.";
EXPECT_TRUE(tester.PreprocessorData().errors.empty());
EXPECT_TRUE(tester.Analyzer().GetRejectedTokens().empty());
EXPECT_THAT(definitions, ElementsAre(Pair("FOOOO", testing::_)));
auto macro = FindOrNull(definitions, "FOOOO");
ASSERT_NE(macro, nullptr);
Expand All @@ -181,10 +186,9 @@ TEST(VerilogPreprocessTest, OneMacroDefinitionOneParamDefaultWithValue) {
PreprocessorTester tester(
"module foo;\nendmodule\n"
"`define FOOOO(x=22) (x+3)\n");
EXPECT_PARSE_OK();

const auto& definitions = tester.PreprocessorData().macro_definitions;
EXPECT_TRUE(tester.Status().ok()) << "Unexpected analyzer failure.";
EXPECT_TRUE(tester.PreprocessorData().errors.empty());
EXPECT_TRUE(tester.Analyzer().GetRejectedTokens().empty());
EXPECT_THAT(definitions, ElementsAre(Pair("FOOOO", testing::_)));
auto macro = FindOrNull(definitions, "FOOOO");
ASSERT_NE(macro, nullptr);
Expand All @@ -202,10 +206,9 @@ TEST(VerilogPreprocessTest, TwoMacroDefinitions) {
PreprocessorTester tester(
"`define BAAAAR(y, z) (y*z)\n"
"`define FOOOO(x=22) (x+3)\n");
EXPECT_PARSE_OK();

const auto& definitions = tester.PreprocessorData().macro_definitions;
EXPECT_TRUE(tester.Status().ok()) << "Unexpected analyzer failure.";
EXPECT_TRUE(tester.PreprocessorData().errors.empty());
EXPECT_TRUE(tester.Analyzer().GetRejectedTokens().empty());
EXPECT_THAT(definitions, ElementsAre(Pair("BAAAAR", testing::_),
Pair("FOOOO", testing::_)));
{
Expand All @@ -224,5 +227,42 @@ TEST(VerilogPreprocessTest, TwoMacroDefinitions) {
}
}

TEST(VerilogPreprocessTest, RedefineMacroWarning) {
PreprocessorTester tester(
"`define FOO 1\n"
"`define FOO 2\n");
EXPECT_PARSE_OK();

const auto& definitions = tester.PreprocessorData().macro_definitions;
EXPECT_EQ(definitions.size(), 1);

const auto& warnings = tester.PreprocessorData().warnings;
EXPECT_EQ(warnings.size(), 1);
EXPECT_EQ(warnings.begin()->error_message, "Re-defining macro");
}

// We might have different modes later, in which we remove the define tokens
// from the stream. Document the current default which registeres all the
// defines, but also does not filter out the define calls.
TEST(VerilogPreprocessTest, DefaultPreprocessorKeepsDefineInStream) {
PreprocessorTester tester(
"`define FOO\n"
"`define BAR(x) (x)\n"
"module x(); endmodule\n");
EXPECT_PARSE_OK();

const auto& definitions = tester.PreprocessorData().macro_definitions;
EXPECT_EQ(definitions.size(), 2);

// The original `define tokens are still in the stream
const auto& token_stream = tester.Data().GetTokenStreamView();
const int count_defines =
std::count_if(token_stream.begin(), token_stream.end(),
[](verible::TokenSequence::const_iterator t) {
return t->token_enum() == verilog_tokentype::PP_define;
});
EXPECT_EQ(count_defines, 2);
}

} // namespace
} // namespace verilog

0 comments on commit 4cccc6b

Please sign in to comment.