Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create reference wiki page for interface class step_definition_interface #114

Merged
merged 20 commits into from
Aug 31, 2024

Conversation

williaml33moore
Copy link
Owner

Closes issue #103

- modified:   src/bathtub_pkg/step_definition_interface.svh
  - New doc string for `step_definition_interface` class.
Modeling revealed that members should have restricted visibility and accessors.

- modified:   src/bathtub_pkg/step_definition_interface.svh
  - Fixed compilation errors.
  - Added `get_bathtub_object()` to model.

- modified:   src/bathtub_pkg/test_sequence.svh
  - Protect the members.
  - New `get_bathtub_object()` accessor.

- modified:   src/bathtub_pkg/test_sequence_interface.svh
  - New `get_bathtub_object()` accessor.
- modified:   src/bathtub_pkg/feature_sequence.svh
- modified:   src/bathtub_pkg/feature_sequence_interface.svh
- modified:   src/bathtub_pkg/rule_sequence_interface.svh
  - Changed member visibility to protected.
  - Supplied accessors.

- modified:   src/bathtub_pkg/step_definition_interface.svh
  - Updated model.
- modified:   src/bathtub_pkg/gherkin_document_runner/gherkin_document_runner.svh
  - Removed feature sequence from scenario sequence.

- modified:   src/bathtub_pkg/scenario_sequence.svh
  - Protected members.
  - Provided accessors.
  - Removed feature sequence reference.
    - It's redundant since you can get to the feature through parent sequence pointers.
    - Step sequences can access all the context sequences directly.

- modified:   src/bathtub_pkg/scenario_sequence_interface.svh
  - Removed feature sequence accessors.
  - Added scenario accessor.

- modified:   src/bathtub_pkg/step_definition_interface.svh
  - Updated the model.
- modified:   src/bathtub_macros.sv
  - Removed context sequence setters from the step definition.
  - They get set in the step attributes object.
  - Added null checks to the `step_parameter_get_args` block for easier debug.
    - These sanity checks make sense since the macro could potentially be used anywhere.

- modified:   src/bathtub_pkg/gherkin_document_runner/gherkin_document_runner.svh
  - `step_nurture` objects are now immutable, so configure them upon instantiation.

- modified:   src/bathtub_pkg/step_attributes_interface.svh
- modified:   src/bathtub_pkg/step_nurture.svh
  - Major clean-up.
  - Step attributes objects are now immutable, so removed setters.
  - Removed static attribute accessors.
    - I had made the static accessors available through the run-time object for convenience, but now I feel it's cleaner to keep them separate.

- modified:   src/bathtub_pkg/step_definition_interface.svh
  - Documentation. #WIP
Cleaning up the API.

- modified:   src/bathtub_macros.sv
  - Use updated methods.

- modified:   src/bathtub_pkg/step_attributes_interface.svh
- modified:   src/bathtub_pkg/step_nurture.svh
  - Replaced step attribute methods with an accessor to the step object.

- modified:   src/bathtub_pkg/step_definition_interface.svh
  - Documentation.
  - API development.
This was a tough one.
I had include recursions involving interface classes.
Classes can't implement and interface classes can't extend forward typedefs ("typedef interface class my_if;"), so that made it challenging to break the include loops.

Takeaways for BKMs:

* In a file declaring a class that implements an interface class, include the file that declares the interface class immediately before the class definition.

```sv
// a.svh
`ifndef __A_SVH
`define __A_SVH
`include "a_if.svh" // Include this file last so there's no other includes between it and the class declaration.
class a implements a_if;
endclass : a
`endif // __A_SVH
---
// a_if.svh
`ifndef __A_IF_SVH
`define __A_IF_SVH
interface class a_if;
pure virtual function void do_a();
endclass : a_if
`endif //  __A_IF_SVH
```

* In a file declaring an interface class, put all the typedefs for regular classes and interface classes before the interface class declaration.
Do not put a typedef for the interface class' superclass, if any.
Use an include instead.

```sv
// b.svh
`ifndef __B_IF_SVH
`define __B_IF_SVH
typedef class c;
typedef interface class d;
`include "e_if.svh"
interface class b_if extends e_if;
pure virtual function c do_b(d arg);
endclass : b_if
`endif // __B_IF_SVH
```

* Since classes have forward declarations, it's tidy to move all includes to the end of the file.

* If there is include recursion, but ifdef guards around the include.
It looks redundant but it's not.
The guards inside the file guard against the file being read twice by unrelated files.
The guards around the include guard against include loops, i.e., recursion.

```sv
// f.svh
`ifndef __F_SVH
`define __F_SVH
typedef class g;
class f;
function void do_f(g arg);
endfunction : do_f
`ifndef __G_SVH
`include "g.svh"
`endif // __G_SVH
`endif // __F_SVH
---
// g.svh
`ifndef __G_SVH
`define __G_SVH
typedef class f;
class g;
function void do_g(f arg);
endfunction: do_g
`ifndef __F_SVH
`include "f.svh"
`endif // __F_SVH
`endif // __G_SVH
```

- modified:   src/bathtub_pkg/scenario_sequence_interface.svh
  - Removed vestigial dependency.

- modified:   src/bathtub_pkg/step_attributes_interface.svh
  - Moved includes to end of file.
  - Added `ifdef guards around the includes.

- modified:   src/bathtub_pkg/step_definition_interface.svh
  - Removed forward typedefs.
  - Actually, I might bring them back...

- modified:   src/bathtub_pkg/step_nurture.svh
- modified:   src/bathtub_pkg/test_sequence.svh
- modified:   src/bathtub_pkg/test_sequence_interface.svh
  - Forward typedefs at the top.
  - Interface include right before the class declaration.
  - Includes at the bottom.
  - This is the way.
- modified:   src/bathtub_pkg/step_definition_interface.svh
  - Typedefs at the top, includes at the bottom.
  - This is my new style.
Cleaned up APIs.

- modified:   src/bathtub_pkg/step_definition_interface.svh
  - Updated model in doc string.
All tests pass.

- modified:   src/bathtub_pkg/step_nurture.svh
  - Removed vestigial `step_seq` from constructor.
  - Attribute object no longer has a reference to the static attribute object, so this sequence argument is no longer used.
  - Removed sequences from message object; didn't compile.

- modified:   src/bathtub_pkg/gherkin_document_runner/gherkin_document_runner.svh
- modified:   test/e2e/gherkin_parser/tests/gherkin_parser_rules_test.svh
- modified:   test/e2e/plusargs/tests/plusarg_bathtub_include_exclude_test.svh
- modified:   test/e2e/plusargs/tests/plusarg_bathtub_start_stop_test.svh
- modified:   test/e2e/plusargs/tests/plusarg_bathtub_verbosity_test.svh
- modified:   test/resources/sequencing/sequence_items/mock_sequence_item/sequences/mock_step_definition_seqs_unit_test.sv
- modified:   test/resources/sequencing/sequence_items/uvm_sequence_item/uvm_sequence/step_definitions_unit_test.sv
- modified:   test/resources/sequencing/virtual/vsequences/mock_step_definition_vseqs_unit_test.sv
  - Updated with `step_nurture` API changes.
- modified:   src/bathtub_pkg/step_definition_interface.svh
  - Updated model to reflect step_static_attributes_interface API changes.

- modified:   src/bathtub_pkg/step_nature.svh
  - Made this class immutable.
  - Got rid of setters.
  - Added arguments to the constructor to initialize.

- modified:   src/bathtub_pkg/step_static_attributes_interface.svh
  - Removed setters.
- modified:   src/bathtub_pkg/step_definition_interface.svh
  - More doc strings.
Upon reviewing the step definition interface, I didn't want to expose the `set_step_attributes()` method to the user.
That is strictly for use by the runner in creating the step definition object, not for use inside the step definition object.
But if `set_step_attributes()` is removed, how can the runner provide the step definition object with its step attributes object?
The solution here is to use a UVM global pool.
I added a typedef to `bathtub_pkg`:
```sv
typedef uvm_pool#(uvm_sequence_base, step_nurture) step_attributes_pool_t;
```
When the runner creates a step definition object and a step attributes object, it stores the attributes object in the global pool with the definition object as the lookup key.
When the step definition's `get_step_attributes()` method is called, it retrieves the attributes from the global pool, and deletes the attributes from the global pool to keep the pool tidy.

This is not ideal because the global pool is, well, global.
Anyone could mess with the global pool.
But that's a problem for another day.

As of now, the objective of removing set_step_attrubutes() has been achieved.
The `step_definition` interface is now immutable; it has no setters.

- modified:   src/bathtub_macros.sv
  - Removed `set_step_attributes()` implementation as it's no longer required.
  - Moved the `__step_static_attributes` member into `get_step_attributes()` as a static variable, so not even user's step definition code can reach it.
  - `get_step_attributes()` retrieves the attributes object from the global pool, then deletes it from the pool.
  - Added checks and warnings in case the attributes object is not found in the pool or is null.

- modified:   src/bathtub_pkg/bathtub_pkg.svh
  - New typedef `step_attributes_pool_t` for the global step attributes pool.

- modified:   src/bathtub_pkg/gherkin_document_runner/gherkin_document_runner.svh
  - Pass step attributes to the step definition object indirectly through the global pool, not directly.

- modified:   src/bathtub_pkg/step_definition_interface.svh
  - Removed `set_step_attributes()` from the interface as it's no longer desired.
Upon reflection, I'm rethinking my earlier decision to push step definition functions into the first line of attributes and context sequence objects.
The primary purpose of the step_definition_interface is to be useful to the step definition writer.
Therefore it shall be a facade that contains as many useful methods as reasonable.
The implementations will delegate to the sub-objects as necessary, hiding that complexity from the user.

- modified:   src/bathtub_macros.sv
  - Implemented step accessors in step definition and virtual step definition macros.
  - Removed vestigial setters from virtual step definition macro.

- modified:   src/bathtub_pkg/step_definition_interface.svh
  - Cleaned up model. #WIP
  - New step accessors
    - `get_step_keyword()`
    - `get_step_text()`
    - `get_step_argument_data_table()`
    - `get_step_argument_doc_string()`

- modified:   src/bathtub_pkg/step_definition_seq.svh
  - Implement new step accessors.
- modified:   src/bathtub_macros.sv
  - Implemented new static accessors in step definition and virtual step definition macros.

- modified:   src/bathtub_pkg/step_definition_interface.svh
  - New step definition accessors
    - get_step_definition_keyword()
    - get_step_definition_expression()
    - get_step_definition_regexp()
  - Rearranged order of methods for the sake of the class reference page.
  - Edited the doc strings.

- modified:   src/bathtub_pkg/step_definition_seq.svh
  - Implemented new static accessors.
- modified:   src/bathtub_pkg/step_definition_interface.svh
  - Edited the doc strings.
  - Updated the models.
All tests pass now.

- modified:   src/bathtub_macros.sv
  - Fixed a bug.
  - The static variable in the method was too static, so made it a regular protected member again.

- modified:   src/bathtub_pkg/bathtub_pkg.svh
- modified:   src/bathtub_pkg/step_definition_interface.svh
  - Fix compile error.

- modified:   src/bathtub_pkg/gherkin_document_runner/gherkin_document_runner.svh
  - Use the typedef.

- modified:   test/resources/sequencing/sequence_items/mock_sequence_item/sequences/mock_step_definition_seqs_unit_test.sv
- modified:   test/resources/sequencing/sequence_items/uvm_sequence_item/uvm_sequence/step_definitions_unit_test.sv
- modified:   test/resources/sequencing/virtual/vsequences/mock_step_definition_vseqs_unit_test.sv
  - Use global attributes pool instead of deleted `set_step_attributes()` method to fix compile errors.
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.

Create reference wiki page for interface class step_definition_interface
1 participant