Skip to content

Conversation

@Emmankoko
Copy link
Contributor

Tried compiling s7 library and __FUNCTION__ was reportedly unresolved.
define it as __func__ which literally does the same thing.

@Emmankoko Emmankoko requested a review from ibuclaw as a code owner December 15, 2025 21:45
@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.

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

@Emmankoko Emmankoko changed the title define __FUNCTION__ define __FUNCTION__ macro Dec 15, 2025
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.

What about __PRETTY_FUNCTION__. Also tests?

@Emmankoko
Copy link
Contributor Author

What about __PRETTY_FUNCTION__. Also tests?

Added!

void foo()
{
assert(strcmp(__FUNCTION__, "foo") == 0);
assert(strcmp(__PRETTY_FUNCTION__, "func.foo") == 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

From a quick Google search, it seems that gcc would actually print void foo() here, not func.foo. However, since this macro is intended as an informative debug string it probably doesn't matter what it is exactly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe, was easier to use what D already does and not reinventing the wheel.
good to go ?
@thewilsonator @dkorpel

Copy link
Contributor

@dkorpel dkorpel left a comment

Choose a reason for hiding this comment

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

The test must be runnable to test the asserts. I suspect it will fail, because in the parser code __FUNCTION__ and __PRETTY_FUNCTION__ are treated identically while the test asserts two different strings are produced.

@Emmankoko
Copy link
Contributor Author

Emmankoko commented Dec 16, 2025

The test must be runnable to test the asserts. I suspect it will fail, because in the parser code __FUNCTION__ and __PRETTY_FUNCTION__ are treated identically while the test asserts two different strings are produced.

wait, I think that's true. I just re run printf on __FUNCTION__ and it is giving me filename.function. I was thinking that should give function name

@dkorpel
Copy link
Contributor

dkorpel commented Dec 16, 2025

Maybe instead of generating a FuncInitExp() you can directly generate a StringExp in the C parser. The FuncInitExp is only there to make it work with default parameters, but that's a feature of D that's not in C.

@Emmankoko
Copy link
Contributor Author

Emmankoko commented Dec 16, 2025

Maybe instead of generating a FuncInitExp() you can directly generate a StringExp in the C parser. The FuncInitExp is only there to make it work with default parameters, but that's a feature of D that's not in C.

A stringExp of the function name ? maybe that works. that could make it actually behave like C

@dkorpel
Copy link
Contributor

dkorpel commented Dec 16, 2025

A stringExp of the function name ? maybe that works. that could make it actually behave like C

Yes exactly, since C behaves differently than D here it makes sense to give it its own implementation rather than trying to re-use the D implementation.

@Emmankoko
Copy link
Contributor Author

Emmankoko commented Dec 16, 2025

A stringExp of the function name ? maybe that works. that could make it actually behave like C

Yes exactly, since C behaves differently than D here it makes sense to give it its own implementation rather than trying to re-use the D implementation.

okay, latest commit handles the __FUNCTION__ well now. the parser treats references to __func__ as a static const char and creates stringexp and stores in a variable and adds to the function body. done in createfuncname. createfuncname was very specific to __func__ and I have now made it general.

@Emmankoko
Copy link
Contributor Author

okay, latest commit handles the FUNCTION well now. the parser treats references to func as a static const char and creates stringexp and stores in a variable and adds to the function body. done in createfuncname. createfuncname was very specific to func and I have now made it general.

void foo()
{
assert(strcmp(__FUNCTION__, "foo") == 0);
//assert(strcmp(__PRETTY_FUNCTION__, "void foo") == 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this test be enabled again?

Copy link
Contributor Author

@Emmankoko Emmankoko Dec 16, 2025

Choose a reason for hiding this comment

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

I have it working for __FUNCTION__

I am now about to make it work for __PRETTY_FUNCTION__. I just wanted to be sure I was on the right path. That's why pretty function is commended out. I think it will be good if I get pretty function work exactly as it is in C. so I will enable in my next commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

enabled

@Emmankoko
Copy link
Contributor Author

@dkorpel I installed a linux VM locally today to check why ubuntu fails and realized ubuntu treats __PRETTY_FUNCTION__ differently from other platforms. __PRETTY_FUNCTION__ on linux is just the function name and not the full signature. I runt the test locally and inserted a printf and it printed foo whiles Mac and the BSDs print the whole signature. looks like when we create the variable, at runtime, it picks the linked C compiler version. so I think checking that we can compile C code with __PRETTY_FUNCTION and __FUNCTION__ is probably enough. I have a simple compile unit on ubuntu with gcc that prints only foo for pretty function. and macOS printing void foo(). it is not a C standard and also runtime specific.

@dkorpel
Copy link
Contributor

dkorpel commented Dec 18, 2025

Your test file is supposed to be testing what you implemented for dmd. How does that give different results on different platforms, are the tests being run with gcc as well? Or is the preprocessor intercepting it?

@Emmankoko
Copy link
Contributor Author

Emmankoko commented Dec 18, 2025

Your test file is supposed to be testing what you implemented for dmd. How does that give different results on different platforms, are the tests being run with gcc as well? Or is the preprocessor intercepting it?

I think its because my test is moved to runtime. and That's because they are generated by the systems based on its own implementation and our internally created variable is extern C and so at runtime it picks the system generated version. I printed them at printed them at compile time and they don't change.

I run separately with gcc and I get exactly what it is expecting from DMD. only function name for PRETTY_FUNCTION on ubuntu.

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.

4 participants