Implement partial support for #define objects#302
Open
davispuh wants to merge 1 commit intojacob-carlborg:masterfrom
Open
Implement partial support for #define objects#302davispuh wants to merge 1 commit intojacob-carlborg:masterfrom
davispuh wants to merge 1 commit intojacob-carlborg:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds partial handling for object-like #define macros that expand to C braced initializer lists (e.g. {1,2}), so dstep can preserve that initializer information in generated D output (as a placeholder with a FIXME marker).
Changes:
- Extend the macro expression AST with a new braced-initializer expression node (
ObjectExpr). - Parse
{ ... }token sequences as a standalone expression inMacroParser. - Translate this expression into a placeholder D value while retaining the original braced text in a
FIXMEcomment.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
dstep/translator/TypeInference.d |
Treats the new braced-initializer expression as Generic during type inference. |
dstep/translator/MacroParser.d |
Introduces ObjectExpr and parsing logic for { ... } object-style macro bodies. |
dstep/translator/MacroDefinition.d |
Adds translation support for ObjectExpr into a placeholder value with a FIXME marker. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
|
||
| string translate(ObjectExpr expr, ExpressionContext context) | ||
| { | ||
| return "null; // FIXME: " ~ expr.expr; |
Comment on lines
+506
to
+512
| struct ObjectExpr | ||
| { | ||
| string expr; | ||
|
|
||
| this (string expr) | ||
| { | ||
| this.expr = expr; |
Comment on lines
+1055
to
+1063
| import std.algorithm.iteration : fold; | ||
|
|
||
| if (!tokens.empty && | ||
| tokens.front.kind == TokenKind.punctuation && | ||
| tokens.front.spelling == "{" && | ||
| tokens.back.kind == TokenKind.punctuation && | ||
| tokens.back.spelling == "}") | ||
| { | ||
| auto expr = Expression(ObjectExpr(tokens.fold!((str, token) => str ~= token.spelling)(""))); |
Comment on lines
+1468
to
+1471
| auto objExpr = parseObjectExpr(tokens); | ||
| if (objExpr.hasValue) | ||
| return objExpr; | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Consider common C pattern like:
Currently
dstepwill ignoreS_INITwhich loses this "init" information.This PR implements partial support so that
Will be produced, which allows to create simple shell script with simple
sedcommand to afterwards fix up these case with something like this:$ sed -iE 's|null; // FIXME: {\([^}]*\)}|S(\1)|g' file.dWhich will make final version to be:
And that will work correctly when
S_INITis used.Unfortunately this 2nd step with
sedor another processing tool is needed because implementing usage-aware processing insidedstepis not trivial and would require significant effort to also parse macro usages so that's why I'm settling on this partial support.