New modname module
#23
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Octachron
reviewed
Feb 17, 2023
dinosaure
commented
Feb 17, 2023
Octachron
reviewed
Feb 17, 2023
dinosaure
force-pushed
the
modname
branch
3 times, most recently
from
e725764 to
d690075
Compare
|
I did the introduction of the new module
Tests did not change (but one test still is commented), just the dependencies graph was updated with the insertion of the new module. Again, I did not propagate the This PR integrates #26 too and it will probably be nice to merge #25 before this one. Especially when I updated the implementation of |
dinosaure
commented
Feb 27, 2023
dinosaure
commented
Feb 27, 2023
|
I have pushed few fixes, and one change in behavior in |
dinosaure
commented
May 3, 2023
Only for debugging, this function helps to knows which are the caller for an unexpected value, for instance.
The Modname module is like the Name but with an abstract type. The idea behind is to provide a nice API and codept as a library. The abstraction lets us to keep some invariants described into the documentation. The goal is to use this abstract type for any module names instead of the Name.t which can refers to several things (path, namespace, module path, etc.)
The order of dependencies is updated. We probably should implement and use a compare function for Modname.t and Namespaced.t instead of to use Stdlib.compare. Regardless the order, the reference still is the same.
The semantic of the Namespaced.t changes a bit. It has 2 views: - as a module path with x.namespace and x.name (Modname.t) - as a file path with x.file This patch update the way to construct Namespaced.t values according to these views and what we give to: - Namespaced.make - Namespaced.of_path
…n.ml The codept solver returns module names instead of a basic strings. To compare results and what we abstract, we must cast results to strings (due to the fact that Modname.t is an abstract type now). The comparison is also updated due to the new definition of Namespaced.t. We must use Namespaced.compare instead of Stdlib.(=) to select only one view of a namespace (the module path view instead of the file path view).
Due to the insertion and the usage of Modname, several modules have new dependencies. We update them according to the new module Modname.
Due to the Modname.t, we follow the module path view instead of the file path view. On the serialization process, solver.mli becomes Solver.
The objective of Unitname is to keep the path of a given module. Due to the namespaced view, it's possible that a path has a signification for the solver (as a part of the module path). To ensure the bijection on some elements, we must keep the file path for any modules which are concretized by a file. Unitname offers two views: - the module name (introduced by the Modname module) - the file path used to construct the Unitname By this way, we ensure the bijection on Unitname elements and we provide these views depending on the context.
This function, due to the transformation, is integrated into the Unitname module which keeps the file path given to construct the Modname value.
To concretize the goal of Unitname, we replace the usage of Modname by the Unitname module, especially for the Namespaced module to keep the file path view at some place. For the Namespaced module, the file record is not needed anymore because Unitname keeps this information internally.
Instead to have the file path view directly into the Namespaced module, we use the opportunity to handle extensions into the Unitname module. By this way, we factorize a bit the Namespaced module by the usage of Unitname.change_file_extension.
Due to the insertion and the usage of Unitname, several modules have new dependencies. We update them according to the new module Unitname.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR wants to solve the issue #14 and provide a new module:
Modnamewhich is basically a module to implement module names. As explained into the issue, the type is no longer a simplestringbut an abstract type to keep and enforce assertions about module names.Currently, only one test remains wrong from this deep change. Implications from this PR on tests are reference by the prefix
[test]. We denote 2 big changes:Modname.tbecomes abstract, we need to useModname.to_stringsometimes but the propagation of this type is not full. To keep the PR clear, I have only dwelt on the most important details and there are places where we could go further in the spread.Namespaced.ttype has 2 views that are useful in many contexts, one view as a module path and one view as a file path. I decided to decouple these views (with a new "file" field) to differentiate between when you want a module path and a file path.Finally, I haven't updated
bundle_refs(see this issue: #21).This is a draft but it is already a good step to clarify the information handled by codept throughout the resolution process. After this RP, it will be a matter of: