Skip to content

Conversation

@Emmankoko
Copy link
Contributor

@Emmankoko Emmankoko commented Nov 28, 2025

for typedefs with ids that are D type keywords, maybe avoid aliasing them.

this ensures D semantics are not broken.

There's a need for new check keyword functions because of the inability for the one already implemented in D work well for C code. because the C ids are not parsed as tokens but identifiers.

@dkorpel @thewilsonator

has been discussed in forum:
https://forum.dlang.org/post/[email protected]

restore_symbols = false;
}


Copy link
Contributor

Choose a reason for hiding this comment

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

unrelated

@Emmankoko Emmankoko force-pushed the typedef branch 2 times, most recently from d3447c8 to f155bcd Compare November 30, 2025 20:29
@Emmankoko
Copy link
Contributor Author

I am a little skeptical about this. I would need to expand this for all D keywords in all type of code. but I don't think skipping them is the right call. maybe prepending a _ to them so they get parsed. and we make a loud noise to the D community
that any D keyword used in C code variably should be used in with _ prepended when imported into D.

@dkorpel @thewilsonator

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @Emmankoko! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

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.

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

  • In preparation for migrating from Bugzilla to GitHub Issues, the issue reference syntax has changed. Please add the word "Bugzilla" to issue references. For example, Fix Bugzilla Issue 12345 or Fix Bugzilla 12345.(Reminder: the edit needs to be done in the Git commit message, not the GitHub pull request.)

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#22154"

@Emmankoko Emmankoko force-pushed the typedef branch 3 times, most recently from dab28f9 to fa14379 Compare December 8, 2025 22:09
@thewilsonator
Copy link
Contributor

diff --git a/home/circleci/dmd/compiler/src/dmd/frontend.h b/home/circleci/dmd/generated/linux/release/64/frontend.h
index d0063f09f..8bb76e554 100644
--- a/home/circleci/dmd/compiler/src/dmd/frontend.h
+++ b/home/circleci/dmd/generated/linux/release/64/frontend.h
@@ -9018,10 +9018,10 @@ struct Token final
         };
         Identifier* ident;
     };
+    void checkKeyword();
     void setString(const char* ptr, size_t length);
     void setString(const OutBuffer& buf);
     void setString();
-    void checkKeyword();
     const char* toChars() const;
     static const char* toChars(TOK value);
     Token() :

@Emmankoko
Copy link
Contributor Author

diff --git a/home/circleci/dmd/compiler/src/dmd/frontend.h b/home/circleci/dmd/generated/linux/release/64/frontend.h
index d0063f09f..8bb76e554 100644
--- a/home/circleci/dmd/compiler/src/dmd/frontend.h
+++ b/home/circleci/dmd/generated/linux/release/64/frontend.h
@@ -9018,10 +9018,10 @@ struct Token final
         };
         Identifier* ident;
     };
+    void checkKeyword();
     void setString(const char* ptr, size_t length);
     void setString(const OutBuffer& buf);
     void setString();
-    void checkKeyword();
     const char* toChars() const;
     static const char* toChars(TOK value);
     Token() :

I just updated the header as well. but I don't know yet if that's a key factor too. or what am I doing wrong with the frontend.h file.

@dkorpel
Copy link
Contributor

dkorpel commented Dec 9, 2025

This is a good start but I have doubts about patching up identifiers in the C parser, it looks fragile and slow having to find and replace keywords in so many places. It also creates unexpected changes in the mangling of function symbols. Could you try creating aliases like alias version_ = version for global variables, struct fields and functions instead? Local variables don't need to change since they are not accessible from D.

@Emmankoko
Copy link
Contributor Author

Emmankoko commented Dec 9, 2025

This is a good start but I have doubts about patching up identifiers in the C parser, it looks fragile and slow having to find and replace keywords in so many places. It also creates unexpected changes in the mangling of function symbols. Could you try creating aliases like alias version_ = version for global variables, struct fields and functions instead? Local variables don't need to change since they are not accessible from D.

my only concern is that, if I create an alias and we have

int version;
alias version_ = version;

in the end, int version is still emitted. and even, doesn't alias version_ = version break D semantics ? aliasing version

@dkorpel
Copy link
Contributor

dkorpel commented Dec 9, 2025

in the end, int version is still emitted

Which matches C behavior, which I think is good

Doesn't alias version_ = version break D semantics ? aliasing version

This is not expressible in D code obviously, but internally, ImportC already allows creating identifiers that are D keywords in the AST so I assume it handles it just fine. If you encounter any trouble you can get back to me.

@Emmankoko
Copy link
Contributor Author

in the end, int version is still emitted

Which matches C behavior, which I think is good

Doesn't alias version_ = version break D semantics ? aliasing version

This is not expressible in D code obviously, but internally, ImportC already allows creating identifiers that are D keywords in the AST so I assume it handles it just fine. If you encounter any trouble you can get back to me.

Okay right! that's valid.

@ibuclaw
Copy link
Member

ibuclaw commented Dec 10, 2025

I'm sceptical about this. What are you going to do with the libm function creal? Suffix an underscore? That'll cause a linker error if anyone tries to call it.

Likewise it potentially introduces symbol conflicts where none existed in C - struct { int byte; int byte_; } boom;

I don't think ImportC should try to be smart. It only needs to parse C code, from the D user code interface anything less than WYSIWYG only seeks to cause confusion.

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 this pull request may close these issues.

5 participants