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

Gram.token_info abstraction is too restrictrive, could be moved to Token #5221

Closed
vicuna opened this issue Feb 5, 2011 · 2 comments
Closed

Comments

@vicuna
Copy link

vicuna commented Feb 5, 2011

Original bug ID: 5221
Reporter: bluestorm
Assigned to: @xclerc
Status: closed (set by @diml on 2016-12-07T17:27:42Z)
Resolution: open
Priority: normal
Severity: feature
Version: 3.12.0
Category: -for Camlp4 use https://github.com/ocaml/camlp4/issues
Monitored by: @gasche

Bug description

I'm having a type regression under 3.12 due to a formerly public type which is now private in Camlp4 interface.

My use case is to compose the Gram1.Entry.of_parser and Gram2.parse_tokens_after_input, to define a grammar entry from the action of a grammar entry of a different grammar. This was possible in 3.10/11 as both functions relied on the following type:

(Token.t * Loc.t) Stream.t

As both grammars relied on the same Lexer module -- of course it doesn't make sense to apply one grammar to an other input stream if the lexers are different -- the Token and Loc module are shared, the types are compatible and this works fine.

At 3.12 the type was changed to:

(Token.t * token_info) Stream.t

where token_info is a private type of the Gram module. Two private types of distinct modules are incompatible, so my code doesn't compile anymore.

I suggest the token_info type definition could be moved into the Token module. I don't think its semantic place it necessarily in the Grammar module, it's mostly a per-token information. If it was defined in Token, even abstractly, different grammars sharing the same Token and Loc module could communicate token_info again.

This change wouldn't break anything as it would only expose more sharing of the token_info type. Of course, for backward-compatibility, an alias inside Gram (type token_info = Token.token_info) would be fine.

Additional information

Attached is a simple example file exhibiting the problem: it compiles under 3.10/11, but not under 3.12. It is actually faithfully reproduces my use case in the Macaque project.

File attachments

@vicuna
Copy link
Author

vicuna commented Feb 5, 2011

Comment author: ertai

I committed a fix in the version/3.12 branch. I prefered to keep the token_info type close to the grammar code. However thanks to applicative functors, I've written the type in such a way that the type only depends on the location type.

Please try this commit and tell us if it fixes your issue.

@vicuna
Copy link
Author

vicuna commented Dec 7, 2016

Comment author: @diml

Camlp4 is now a separate project. Please re-open a ticket on github [1] if you are still interested in seeing the original issue fixed.

[1] https://github.com/ocaml/camlp4/issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant