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

Inconsistency in how Q-IDENTs are specified #2612

Closed
tobias-gutzmann-modelon opened this issue Jul 8, 2020 · 2 comments · Fixed by #2616
Closed

Inconsistency in how Q-IDENTs are specified #2612

tobias-gutzmann-modelon opened this issue Jul 8, 2020 · 2 comments · Fixed by #2616

Comments

@tobias-gutzmann-modelon
Copy link

There's an inconsistency in the spec in how Q-IDENTs are specified.

lexicalstructure.tex (Section 2.3.1):
Q-IDENT = "'" { Q-CHAR | S-ESCAPE | """ } "'"

syntax.tex (Appendix B):
Q-IDENT = "'" ( Q-CHAR | S-ESCAPE ) { Q-CHAR | S-ESCAPE | """ } "'"

The former definition allows '' and '"' as Q-IDENTs, the latter does not.

My proposal is to allow '' and '"', I cannot think of a reason to forbid them as the enclosing single quotes are part of the identifier anyway.

Suggested fix:

  • include quotation mark " in Q-CHAR
  • Change both places to Q-IDENT = "'" { Q-CHAR | S-ESCAPE } "'"
  • (optional) S-ESCAPE and Q-CHAR could be joined as well, as S-ESCAPE is not used anywhere else, but it may be desirable to keep them separated anyway.

I found that this was already mentioned in #2408 but neither discussed nor resolved as part of that issue.

@HansOlsson
Copy link
Collaborator

Obviously '"' should be allowed.
It seems " as part of quoted identifier was introduced in Modelica 3.4 together with the rule that \" and " are the same as part of #1176.
And the lexical variant was introduced in Modelica 3.0; and I believe it is just sloppy copying that led it to allow the empty quoted identifier.

I thus agree that it is simplest to extend Q-CHAR with " and simplify the rules. Having S-ESCAPE separate makes sense to me.
Obviously both variants of the rule should be identical.
Allowing the empty quoted identifier is less clear, but we might as well allow it. It would make the grammar slightly simpler; and if people find it unclear the obvious solution is to not use it, similarly as not mixing '1', 'I', 'l', '0', 'O'.

BTW: Dymola seems to support ''

HansOlsson added a commit to HansOlsson/ModelicaSpecification that referenced this issue Jul 10, 2020
Cleanups:
- Add " to Q-CHAR
- Unify Q-IDENT variants
Closes modelica#2612
@henrikt-ma
Copy link
Collaborator

I agree that there doesn't seem to be a compelling reason why the identifiers '' and '"' shouldn't be allowed. I can see how things could get confusing in an environment where only quoted identifiers are used, so that the enclosing apostrophes don't have to be displayed in a GUI, but I hope no such environments exist.

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 a pull request may close this issue.

3 participants