-
Notifications
You must be signed in to change notification settings - Fork 63
Allow keywords as identifiers in extern(C) blocks for better C interopability #530
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
base: master
Are you sure you want to change the base?
Changes from all commits
594ac83
42e6a74
eb198fe
0740122
b7fc40e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,6 +118,16 @@ Module parseModule(F)(const(Token)[] tokens, string fileName, RollbackAllocator* | |
| */ | ||
| class Parser | ||
| { | ||
| /// Is it a interface file for a C source code | ||
| bool importC; | ||
| /// Depth of extern(C) blocks we're currently inside | ||
| int externCDepth; | ||
|
|
||
| bool isInExternC() const pure nothrow @safe @nogc | ||
| { | ||
| return importC || externCDepth > 0; | ||
| } | ||
|
|
||
| /** | ||
| * Parses an AddExpression. | ||
| * | ||
|
|
@@ -2294,26 +2304,43 @@ class Parser | |
| advance(); | ||
| break; | ||
| case tok!"{": | ||
| if (node.attributes.empty) | ||
| if (node.attributes.empty) | ||
| { | ||
| error("declaration expected instead of `{`"); | ||
| return null; | ||
| } | ||
| // Check if we're in an extern(C) block | ||
| bool isExternC = false; | ||
| foreach (attr; node.attributes) | ||
| { | ||
| if (attr.linkageAttribute !is null && | ||
| attr.linkageAttribute.identifier.text == "C") | ||
| { | ||
| error("declaration expected instead of `{`"); | ||
| return null; | ||
| isExternC = true; | ||
| break; | ||
| } | ||
| advance(); | ||
| alias declarations = attributes; | ||
| declarations.clear(); | ||
| while (moreTokens() && !currentIs(tok!"}")) | ||
| } | ||
| if (isExternC) | ||
| externCDepth++; | ||
| advance(); | ||
| alias declarations = attributes; | ||
| declarations.clear(); | ||
| while (moreTokens() && !currentIs(tok!"}")) | ||
| { | ||
| auto c = allocator.setCheckpoint(); | ||
| if (!declarations.put(parseDeclaration(strict, false, inTemplateDeclaration))) | ||
| { | ||
| auto c = allocator.setCheckpoint(); | ||
| if (!declarations.put(parseDeclaration(strict, false, inTemplateDeclaration))) | ||
| { | ||
| allocator.rollback(c); | ||
| return null; | ||
| } | ||
| allocator.rollback(c); | ||
| if (isExternC) | ||
| externCDepth--; | ||
| return null; | ||
| } | ||
| ownArray(node.declarations, declarations); | ||
| mixin(tokenCheck!"}"); | ||
| break; | ||
| } | ||
| if (isExternC) | ||
| externCDepth--; | ||
| ownArray(node.declarations, declarations); | ||
| mixin(tokenCheck!"}"); | ||
| break; | ||
| case tok!"alias": | ||
| if (startsWith(tok!"alias", tok!"identifier", tok!"this")) | ||
| mixin(parseNodeQ!(`node.aliasThisDeclaration`, `AliasThisDeclaration`)); | ||
|
|
@@ -2481,7 +2508,7 @@ class Parser | |
| foreach (B; BasicTypes) { case B: } | ||
| type: | ||
| Type t = parseType(); | ||
| if (t is null || !currentIsOneOf(tok!"identifier", tok!":")) | ||
| if (t is null || !(currentIs(tok!"identifier") || (isInExternC() && isKeyword(current.type)) || currentIs(tok!":"))) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should probably not allow all keywords, only what is allowed in C context (e.g. we need a new helper function instead of |
||
| { | ||
| if (t) | ||
| error("no identifier for declarator"); | ||
|
|
@@ -2654,7 +2681,17 @@ class Parser | |
| } | ||
| else | ||
| { | ||
| const id = expect(tok!"identifier"); | ||
| const(Token)* id; | ||
| if (currentIs(tok!"identifier") || (isInExternC() && isKeyword(current.type))) | ||
| id = &tokens[index++]; | ||
| else | ||
| { | ||
| error("Expected identifier instead of " ~ (index < tokens.length | ||
| ? (tokens[index].text is null ? "`" ~ str(tokens[index].type) ~ "`" | ||
| : str(tokens[index].type) ~ " `" ~ tokens[index].text ~ "`") | ||
| : "EOF")); | ||
| return null; | ||
| } | ||
| mixin (nullCheck!`id`); | ||
| node.name = *id; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need a conversion from keyword token to identifier token here, so that node.name.text is set (otherwise it's unset for keywords iirc) |
||
| if (currentIs(tok!"[")) // dmd doesn't accept pointer after identifier | ||
|
|
@@ -5495,7 +5532,7 @@ class Parser | |
|
|
||
| ownArray(node.parameterAttributes, parameterAttributes); | ||
| mixin(parseNodeQ!(`node.type`, `Type`)); | ||
| if (currentIs(tok!"identifier")) | ||
| if (currentIs(tok!"identifier") || (isInExternC() && isKeyword(current.type))) | ||
| { | ||
| node.name = advance(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need a conversion from keyword token to identifier token here, so that |
||
| if (currentIs(tok!"...")) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -313,34 +313,63 @@ void testArbitraryASTs() | |
| parser.messageDelegate = &msgDelegate; | ||
| parser.allocator = &rba; | ||
|
|
||
| parser.tokens = getTokensForParser("struct S {}", config, &cache); | ||
| assert(parser.parseCompileCondition() is null); | ||
| assert(errors == ["`version`, `debug`, or `static` expected (found token `struct`)"]); | ||
| errors = null; | ||
|
|
||
| parser = new Parser(); | ||
| parser.messageDelegate = &msgDelegate; | ||
| parser.allocator = &rba; | ||
| parser.tokens = getTokensForParser("~", config, &cache); | ||
| assert(parser.parseDestructor() is null); | ||
| assert(errors == ["`this` expected instead of EOF"]); | ||
| errors = null; | ||
|
|
||
| parser = new Parser(); | ||
| parser.messageDelegate = &msgDelegate; | ||
| parser.allocator = &rba; | ||
| parser.tokens = getTokensForParser("for (x; y; z) {}", config, &cache); | ||
| assert(parser.parseForeach() is null); | ||
| assert(errors == ["`foreach` or `foreach_reverse` expected (found token `for`)"]); | ||
| errors = null; | ||
|
|
||
| parser = new Parser(); | ||
| parser.messageDelegate = &msgDelegate; | ||
| parser.allocator = &rba; | ||
| parser.tokens = getTokensForParser("version =", config, &cache); | ||
| assert(parser.parseVersionSpecification() is null); | ||
| assert(errors == ["Identifier or integer literal expected (found EOF)"]); | ||
| errors = null; | ||
| { | ||
| parser.tokens = getTokensForParser("struct S {}", config, &cache); | ||
| assert(parser.parseCompileCondition() is null); | ||
| assert(errors == ["`version`, `debug`, or `static` expected (found token `struct`)"]); | ||
| errors = null; | ||
| } | ||
| { | ||
| parser = new Parser(); | ||
| parser.messageDelegate = &msgDelegate; | ||
| parser.allocator = &rba; | ||
| parser.tokens = getTokensForParser("~", config, &cache); | ||
| assert(parser.parseDestructor() is null); | ||
| assert(errors == ["`this` expected instead of EOF"]); | ||
| errors = null; | ||
| } | ||
| { | ||
| parser = new Parser(); | ||
| parser.messageDelegate = &msgDelegate; | ||
| parser.allocator = &rba; | ||
| parser.tokens = getTokensForParser("for (x; y; z) {}", config, &cache); | ||
| assert(parser.parseForeach() is null); | ||
| assert(errors == ["`foreach` or `foreach_reverse` expected (found token `for`)"]); | ||
| errors = null; | ||
| } | ||
| { | ||
| parser = new Parser(); | ||
| parser.messageDelegate = &msgDelegate; | ||
| parser.allocator = &rba; | ||
| parser.tokens = getTokensForParser("version =", config, &cache); | ||
| assert(parser.parseVersionSpecification() is null); | ||
| assert(errors == ["Identifier or integer literal expected (found EOF)"]); | ||
| errors = null; | ||
| } | ||
| { | ||
| parser = new Parser(); | ||
| parser.messageDelegate = &msgDelegate; | ||
| parser.allocator = &rba; | ||
| parser.importC = false; | ||
| parser.tokens = getTokensForParser("extern(C) { int normal = 1; int true = 1; }", config, &cache); | ||
| assert(parser.parseAttribute() !is null); | ||
| auto b = parser.parseBlockStatement(); | ||
| assert(b !is null); | ||
| assert(b.declarationsAndStatements.declarationsAndStatements.length == 1); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please add an |
||
| errors = null; | ||
| } | ||
| { | ||
| parser = new Parser(); | ||
| parser.messageDelegate = &msgDelegate; | ||
| parser.allocator = &rba; | ||
| parser.importC = true; | ||
| parser.tokens = getTokensForParser("extern(C) { int normal = 1; int true = 1; }", config, &cache); | ||
| assert(parser.parseAttribute() !is null); | ||
| auto b = parser.parseBlockStatement(); | ||
| assert(b !is null); | ||
| assert(b.declarationsAndStatements.declarationsAndStatements.length == 2); | ||
| errors = null; | ||
| } | ||
|
|
||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use
scope (exit)instead of duplicating theif (isExternC) externCDepth--;at both exit points