Add C source information to ASTs #338
Conversation
|
Tests fail on Windows because the source paths in the Note that this issue is avoided in the One way to resolve this issue would be to change the We could mangle source path strings, converting Windows directory separators to POSIX directory separators. Perhaps it is better to display source paths using actual/native directory separators to users, though. I can implement it if we decide to go this route. My inclination is to not do special rendering in |
|
Re rendering of paths, see which I introduced for that precise reason. |
|
Indeed, that
Note that this branch already includes a change that makes the paths relative to the package directory when under that directory (#316 (comment)). |
|
Ah, sorry. Yes, I think we should go for something again to what we do with the |
Two tests fail because the source locations include absolute paths when data types are generated for included `typedef`s. This needs to be resolved before merge.
This commit changes name mangling so that there are minimal changes by default, not attempting to create Haskell-style names. One significant difference with the previous implementation is that it can now add a prefix if/when the first character is not valid, used to handle leading underscores when creating type constructors. `MkHsName` is removed, replaced by `mkHsNamePrefixInvalid` and `mkHsNameDropInvalid` functions that are now passed to `translateName` like other options. `ctxFieldVarSingleConstr` is removed since constructors now have the same name as types by default, and we do not generate sum types anyway. If we do so in the future, we need to decide how to do name mangling for constructors as well as accessors. `HsBindgen.C.Fold.Type.mkDefnName` is changed to follow the conventions of the new defaults.
The initial implementation allowed users to override name mangling by specifying the C name. This is problematic when the name of an anonymous structure/union needs to be overridden. Specifying a C name worked when some name mangling was performed while creating the C AST, but it does not work when all name mangling is done by the name mangler. With this change, users override by specifying the translated Haskell name. Overrides must now be performed after creating the Haskell name. The parameter order of `translateName` is updated for consistency.
This commit removes `DefnName` and instead tracks the "path" of a structure, renaming the type to `DeclPath`. `mkDefnName` is now implemented as part of name mangling (as `getDeclPathParts`), in a way that works with name mangling options. Context `AnonNamedFieldTypeConstrContext` is removed, replaced with `StructTypeConstrContext` that contains a `DeclPath` instead of a `CName`. New function `translateDeclPath` is a high-level function for translating `DeclPath`s.
At this point, there are no differences with the Hs AST, so those types are used as-is. If we ever introduce transformations, we should define SHs origin types that record the transformations made.
The original design threaded the sequence because I thought we would need to recursively generate sequential values. It sounds like we will not do that, however. With this design, the Haskell and C APIs match and can therefore be tested for equivalence. Two (unit test) assertions are added: * A value sequentially generated in Haskell has the expected value in C. * A value sequentially generated in C has the expected value in Haskell.
We need this functionality for test generation, so it should be moved out of the backend. I am putting it outside of the `HsBindgen` hierarchy to emphasize that it is the general part of pretty printing.
* `hs-bindgen` dependencies are removed * Documentation is updated * Context mutation is implemented using a function, for consistency * The `Pretty` `Natural` instance is removed
When generating C code, we have to know how to write each C name (such as `struct foo` vs. just `foo`). This commit adds type `DeclName` for use in the `DeclPathStruct` constructor of type `DeclPath` to do this. Note that this could alternatively be done by making `CName` a phantom type that specifies the namespace. I implemented `DeclName` for now as a simple/minimal change.
TravisCardwell
force-pushed
the
source-info
branch
from
4856da8 to
8762d74
Compare
The motivation for this is to normalize representations just for tests. `SourcePath` values are rendered as lists, like in the `tree-diff` fixtures. Note that `ediffGolden1` is *not* used because we do not have `Eq` instances for the `Hs` AST. We could use it if we decide to implement `Eq` instances. For now, per-line string comparisons are made. This is still an improvement over the previous implementation using `Show` because the `Expr`s are pretty-printed across multiple lines.
This commit essentially reverts the previous commit that made `typedef` source locations relative. Tests fail (gain) due to use of absolute paths. This will be addressed in the next commit.
TravisCardwell
force-pushed
the
source-info
branch
from
8ccbea0 to
ea986b6
Compare
|
I changed the Note that I did not use I imagine that we would like to make source locations relative in the tests only as well. I planned on doing this by moving the type class to the tests package. Doing so at this level requires a lot more instances, of course, since the AST has to be traversed. Unfortunately, I ran into the age-old Haskell issue of public API vs testing. It worked great for the Haskell representations because we export some modules specifically for testing, though I had to add some. It did not work for the C API, however, because our The Cabal file includes the following comment. Perhaps we can use an internal library to expose more things for testing? -- Exposed for the sake of tests
-- TODO: We should reconsider the proper way to export these.Making paths relative in general requires context (current working directory), but we do not have to worry about the general case since it is specific to our tests. I implemented a fix in the Alas, my initial implementation did not work in CI! Here is a diff: - ["hs-bindgen", "musl-include", "bits", "alltypes.h"],
+ ["hs-bindgen-0.1.0", "musl-include", "bits", "alltypes.h"],In the CI environment, Cabal appends the version number to the package directory names! I resolved this issue by normalizing package directory names. |
There was a problem hiding this comment.
We discussed this in a meet. Conclusions:
- Name mangling will get a
CName -> HsNamemap in addition to theHsName -> HsNamemap, or alternatively aMaybe CName -> HsName -> HsNamemap, where the latter allows the user to specify theCNameif they so desire (useful to state intent and useful for disambiguation). - We will make paths relative to the
cwdas we construct them, rather than doing this in tests. - We will use the
tree-diffinfrastructure for comparison, adding missingEqinstances (though it's not totally clear what they are required for).
Feel free to merge when ready.
We decided to implement relative path support when constructing a `SourcePath`, so that it is consistent across the whole application. The directory to make paths relative to is passed as configuration throughout the whole application, so that we can enable users to configure it. For now, I implemented it by consistently passing it as the first parameter to functions (`Reader`-style). Every parameter is named `relPath` and is documented in the same way. If we implement more configuration the future, we may want to consider using a `Reader` transformer. Threading configuration through an application changes the API for many functions. orz
TravisCardwell
force-pushed
the
source-info
branch
from
ea986b6 to
f979932
Compare
Since not all generated Haskell names have corresponding C names, overriding is primarily done based on the generated Haskell name. This changes adds an *optional* C name that allows users to create overrides in cases where more than one C name is being translated to the same Haskell name.
The primary goal of this PR is to add C source information to the ASTs (#316), for use in test and documentation generation. This is the initial attempt at implementing this so that we have something to discuss (#316 (comment)). We plan to discuss during the call on Thursday (December 19th).
This needed to build upon the new minimal default name mangling (#331), so the single commit from that branch is cherry-picked. Since a primary motivation is to support test generation (#22), my initial implementation of test generation is also implemented in this branch. Some commits are cherry-picked, some commits are rewritten from scratch with this different design, and new commits are added.
This comment serves as a guide to the various commits. Some commits have descriptions, and links to related comments are included below.
Fieldtype (cherry-picked)OpaqueStructandOpaqueEnumtypedefsource locations relative (Add C source information toHsandSHsASTs #316 (comment))structname manglingHsandSHsASTs #316 (comment))HsASTSHsASThs-bindgen-testlib(WIP)hs-bindgen-testlib(WIP)SameSemanticsSameSemanticsproperties/assertionsStorableproperties/assertionssizeofandalignofassertions/testsPreturbPreturbGenSeqHsGenSeq1SameSemanticsCGenSeq1clang-formatText.SimplePrettyPrintText.SimplePrettyPrintHsandSHsASTs #316 (comment))HsASTDeclName(Add C source information toHsandSHsASTs #316 (comment))hangsSpecandMainmodulesMainmodule to sample Cabal configHsAST usingData.TreeDiff.Exprtypedefsource locations relativeSourcePathrelative intoExprinstance