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

parser rejects valid: 'ubyte a[1]; ' #60

Closed
timotheecour opened this issue Oct 22, 2013 · 9 comments
Closed

parser rejects valid: 'ubyte a[1]; ' #60

timotheecour opened this issue Oct 22, 2013 · 9 comments

Comments

@timotheecour
Copy link

ubyte a[1];

=> error: C-style variable declarations are not supported.

@Hackerpilot
Copy link
Collaborator

I think that error message explains itself fairly well.

@timotheecour
Copy link
Author

I know, but it's still a bug: D code in production still relies on it (in rdmd etc); it should be a warning, not an error, and at least parse the rest of the file even if it skips this declaration.

@Hackerpilot
Copy link
Collaborator

The fact that DMD still supports C syntax at all is a bug because it doesn't do so properly. http://d.puremagic.com/issues/show_bug.cgi?id=953

@timotheecour
Copy link
Author

I agree it is a misfeature, but both ldc and dmd accept it, so dscanner should match their behavior.
phobos uses that in many files, and many D production 3rd party libs do too.
If dscanner deviates, it just won't be usable with those.

I see nothing wrong with just reporting a warning instead of errro (besides additional coding time of course :-) ).

@ghost
Copy link

ghost commented Feb 11, 2014

I don't think emitting a warning is enough, since you'd end up with a wrong AST tree. You need to actually implement parsing for this, but why bother doing all that work for a deprecated feature?

The only reason I could think of implementing this is if you wanted to auto-translate deprecated syntax into the proper syntax. But in that case such a conversion app can override parseDeclarator in its parser subclass and add this support. There's no need to add it to Dscanner itself.

I think we should close this. @Hackerpilot?

@callumenator
Copy link
Contributor

I agree with @timotheecour - in order to do AST->source conversion properly, dscanner should parse whatever dmd does (bugs in dmd aside). We can always warn about it, let the user decide whether they want to treat it as any error.

Far as I can tell, dmd only allows array decls (it doesn't do the semantic properly as noted, but it shouldn't matter):

Type ident TypeSuffix* (= init)? (, more)?

Where only arrays are allowed (syntactically). I have a possible patch for this here: https://github.com/callumenator/Dscanner/tree/c-style

Basically, it just adds a typesuffix array to the declarator, and parses typesuffixes after the identifier when it sees an opening [. In my tests on phobos AST->source, this has no adverse affects and allows c-style usage that appears in some unittests. Thoughts?

@callumenator
Copy link
Contributor

Note dmd also allows them for parameters:

void a(int b[]){}

This appears in phobos (regex at least)

@Hackerpilot
Copy link
Collaborator

Support for variables with c-style array syntax is present in libdparse now. I'm leaving this open since it seems dscanner does not dump the correct AST for c-style array function parameters.

@ghost
Copy link

ghost commented Apr 1, 2018

this is accepted now in dparse.

@ghost ghost closed this as completed Apr 1, 2018
This issue was closed.
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

3 participants