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

ImportC: support #define IDENT ( expression ) #15871

Merged
merged 1 commit into from Dec 5, 2023

Conversation

WalterBright
Copy link
Member

This expands the kinds of macros ImportC can convert to usable D code. It relies on the sophisticated theory "if it quacks like a duck". I.e. if it will parse as an Expression without error, then it's an Expression. It the parse succeeds, it converts the Expression to a template function with Expression as the body.

Being a template means the semantic correctness is not checked until it is actually used, which saves the user from lots of spurious errors spawned by wacky macros.

If this works out, we can expand the technique to cover more cases, such as macros with ...wait for it... parameters!

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#15871"

Copy link
Contributor

@thewilsonator thewilsonator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to have some test cases of things this doesn't work on such as

#define foo do {} while (0)

otherwise looks fine.

@WalterBright
Copy link
Member Author

The example doesn't start with a '(' so the recognizer won't even get started on it. Things that don't quack like a duck are simply ignored.

@thewilsonator
Copy link
Contributor

Fair enough, carry on.

@WalterBright WalterBright force-pushed the expMacros branch 3 times, most recently from 4d0b767 to cda54ca Compare November 30, 2023 06:47
@WalterBright
Copy link
Member Author

This is having problems resynchronizing the token stream after an error. Will look into it tomorrow.

@tim-dlang
Copy link
Contributor

The pull request also allows macros, which start with '(', but don't end with the matching ')'. This can result in corner cases, where C and D would use different values:

#define A (1) + 2
#define B (3) * A
const int b = B;
import test;
import std.stdio;
void main()
{
    writeln(B); // prints 9
    writeln(b); // prints 5
}

@WalterBright
Copy link
Member Author

@tim-dlang Thanks, I'll look into that and make sure it doesn't happen.

@ibuclaw
Copy link
Member

ibuclaw commented Dec 1, 2023

I can't help but think mixins would be better suited to the job than templates.

@WalterBright
Copy link
Member Author

The next step here would be to add macro parameters as template parameters. I don't think that will work with mixins. Macros that are well-formed are suitable for turning into templates. Ones that aren't:

#define LP (
...
if LP 5+1)

are never going to be usable from the D side, and, that's the way it goes. (StatementExpressions are also converted into lambda templates, that works like a champ!) I don't see a workable path for token pasting macros.

@ibuclaw
Copy link
Member

ibuclaw commented Dec 1, 2023

The next step here would be to add macro parameters as template parameters.

I don't think that prevents from using mixins, I'd approach it by asking: what would be the equivalence of injecting the code inline into the current scope, and then reduce that answer to some simple reusable code the compiler could generate. I'd avoid introducing symbols if it could be helped.

enum A = /*lazily evaluated*/ mixin("(1) + 2");
enum B = /*lazily evaluated*/ mixin("(3) * A");
const int b = B;

With parameters:

#define B(A) (3) * A;
...
return B(1 + 2);

The return might as well be something like

return (alias A = 1 + 2,
        mixin("(3) * A"));

I don't think that will work with mixins. Macros that are well-formed are suitable for turning into templates. Ones that aren't:

#define LP (
...
if LP 5+1)

are never going to be usable from the D side, and, that's the way it goes.

Sure, much in the same way as you can't do if mixin("(") 5+1) in D. There's got to be some sane limitations.

@ibuclaw
Copy link
Member

ibuclaw commented Dec 1, 2023

The return might as well be something like

return (alias A = 1 + 2,
        mixin("(3) * A"));

Saying that, I could see parameters being turned into mixins too

return (alias A = mixin("1 + 2"),
        mixin("(3) * A"));

All I'm seeing is a special kind of declaration whose value is evaluated on demand. But maybe that'll fall apart with more complex examples.

@WalterBright
Copy link
Member Author

Your proposal is interesting, and it might be possible to make it work. But it would require multiple new syntactical constructions, and I don't see how it does anything different from templates. After all, aren't templates symbolic mixins?

There might be a case for translating the macros to mixin templates rather than templates, but I'm uneasy about that. mixin templates need to be used with caution as they derive their semantics from the instantiation context rather than the definition context. I expect that behavior would be surprising to users. I wouldn't want it in my code.

@tim-dlang
Copy link
Contributor

With parameters:

#define B(A) (3) * A;
...
return B(1 + 2);

The return might as well be something like

return (alias A = 1 + 2,
        mixin("(3) * A"));

That would be evaluated as 9 in D, but the macro in C would be 5. A mixin could have the same value like this:

string B(string A)
{
    return "(3) * " ~ A;
}
...
return mixin(B("1 + 2"));

Unfortunately the user code would be more verbose with this, making porting of C code to D harder.

The compiler could also enforce, that every parameter in a macro is in parenthesis like this:

#define B(A) (3 * (A))

In that case both template functions and mixins should work.

@WalterBright
Copy link
Member Author

It's well known that:

#define B(A) 3 * A

is an execrable practice in C, and in fact I don't plan to support it at all in ImportC (i.e. the macro will be ignored). The macro will have to be written as:

#define B(A) (3 * A)

Leaving the A unparenthesized in the macro is also almost always a bug in the C code. D will treat it as if it were parenthesized. We can talk about if A should be rejected if not parenthesized, but that's not relevant to this PR.

@WalterBright WalterBright added the Ready To Merge Let's get this merged label Dec 2, 2023
@WalterBright WalterBright merged commit 807373a into dlang:master Dec 5, 2023
42 of 43 checks passed
@WalterBright WalterBright deleted the expMacros branch December 5, 2023 06:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Easy Review Ready To Merge Let's get this merged
Projects
None yet
6 participants