Skip to content

Conversation

rdaly525
Copy link
Collaborator

depends on #4 and #5

This branch fully fleshes out constant expressions and adds a test that fully describes a parameterized verilog module.

@rdaly525 rdaly525 requested a review from leonardt June 18, 2019 21:31
@leonardt
Copy link
Owner

I'm going to change the base of the PR to #5 so I can review the changes w.r.t. to that branch

@leonardt leonardt changed the base branch from master to always June 18, 2019 22:26
Copy link
Owner

@leonardt leonardt left a comment

Choose a reason for hiding this comment

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

What is the value in having a separate ConstantExpression versus regular expression? Are there different rules about what you can have? Or is it the same as the expressions you can use normally?

If there's nothing different about them, I would strongly prefer to re-use the expression logic for both so we don't have to duplicate code.

add_executable(parameterized_module tests/parameterized_module.cpp)
target_link_libraries(parameterized_module gtest_main ${LIBRARY_NAME})
add_test(NAME parameterized_module_tests COMMAND parameterized_module)

Copy link
Owner

Choose a reason for hiding this comment

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

(nit) we can review this new line?

@rdaly525 rdaly525 closed this Jun 18, 2019
@leonardt leonardt deleted the const-expr branch December 2, 2019 15:21
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.

2 participants