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

Implement sections using MultiValueProvider #43

Open
wants to merge 14 commits into
base: dev
Choose a base branch
from

Conversation

antis81
Copy link
Contributor

@antis81 antis81 commented Jul 16, 2023

(moved from PR #42)

The Problem:
All bass symbols (global and local) have to be defined in a section ("default" when no section is explicitly defined).
Currently SymbolTable is unaware of this dependency and cannot always differentiate the state of a symbol.

In some cases this leads to either of

  • "max number passes" -> this is the situation with my "real-world-project"
  • "false data" -> if a section's PC and/or start evaluate to an "unfortunate" address value - e.g. negative or 0)

The basic idea of MultiValueProvider is to define an "owner" (namely a Section instance) who keeps track of all its symbols while at the same time it is ensured that symbol identifiers are unique and not "already registered" by another MultiValueProvider.

Values can be

  • None values (e.g. a "yet to be defined" section start address)
  • global constant (sub-)symbols
    (variables are not implemented into bass at this point - ACME implements a !set meta command and also explains where this can be used)
  • built-in (implicit identifiers such as "section.{name}.start")
  • symbols added by bass users (defining global constant/label in the document)

MVP can also be used in 2 ways:

  1. ❌ Inherit class Section : public MultyValueProvider { … };
  2. ✔️ Add as class memebr class Section { MultyValueProvider mvp_0; … };

Currently I don't see a use case for the second approach. Starting with the first one and see if it reveals any restrictions…

Decided for variant #2 because it better integrates with current code (and also - theoretically - allows for more complex features such as multiple MVP's in the same class context…)

💡 To enable the implementation set USE_BASS_VALUEPROVIDER in CMakeLists.txt.

Have fun experimenting! 🔬

@antis81 antis81 changed the title Add MultiValueProvider Implement sections using MultiValueProvider Jul 16, 2023
@antis81
Copy link
Contributor Author

antis81 commented Jul 18, 2023

FYI: Will push current status and let you know when it is "done done". Very close to completing this one with "minimal invasive" approach, but requires a bit of "nasty" debugging in the details ("section_move" test throws with MVP enabled)…

antis81 added 6 commits July 18, 2023 20:18
Helps with "MultiValueProvider" integration. While this slightly changes
the logic (see "Machine::addSection -> emplace_back") I could
not detect any negative effects (all tests run)…
Values are always serialized to/from MVP (currently "uncached" -> copied).
Some tests throw -> needs debugging
Using transitional class "SectionInitializer" to
setup sections from "::parsArgs".
@antis81
Copy link
Contributor Author

antis81 commented Jul 19, 2023

As promised -> "Done Done"! (:partying_face:) The "magic" happens in this line where actually the section start is "synchronized" (copied from std::any -> int32_t) from MVP into the current SymbolTable.

FYI:
The Symbol::final case can be realized with ValueSerializer, which only needs to check if a value is None (if (mvp.hasValue("section.default.start") { throw std::runtime_error("Cannot reassing a const symbol") }). Currently not implemented because exactly of what is described above. SymbolTable simply fails here… 🙁

antis81 added 5 commits July 21, 2023 12:29
This way MVP is populated in parallel to the SymbolTable with slight
difference in value representation.

(ValueSerializer is not needed anymore
 -> use get-/setValue to read/store values in MVP).
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.

1 participant