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

frontend: add support for __MANGLED_FUNCTION__ special keyword #12252

Closed
wants to merge 5 commits into from

Conversation

ljmf00
Copy link
Member

@ljmf00 ljmf00 commented Mar 5, 2021

Signed-off-by: Luís Ferreira contact@lsferreira.net


This may need changes on the spec before the merge.

CC: @maxhaton

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @ljmf00! 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#12252"

@ljmf00 ljmf00 marked this pull request as draft March 5, 2021 18:30
@ljmf00 ljmf00 force-pushed the implement-mangled-func-init-exp branch 2 times, most recently from 60f8d1d to 4aa76b9 Compare March 5, 2021 18:52
@MoonlightSentinel
Copy link
Contributor

What's the use case?
And why does it need special casing when this feature can already be implemented using __FUNCTION__?

@ljmf00
Copy link
Member Author

ljmf00 commented Mar 5, 2021

What's the use case?
And why does it need special casing when this feature can already be implemented using __FUNCTION__?

That's problematic when mixin(callee).mangleof can't reach, like when the function is a nested function and it's inside another scope or a module. You can kinda do it with mangle(__PRETTY_FUNCTION__) but you are stuck with runtime, even at compile-time, and mangle!() need the type explicitly, which could be tricky and even impossible in some cases, e.g. when the type is from another module or on another scope.

Imagine this situation:

void main()
{
    int foo()
    {
        mixin(__FUNCTION__).mangleof.writeln;
        return 1;
    }

    foo();
}

@ljmf00 ljmf00 marked this pull request as ready for review March 5, 2021 19:38
ljmf00 added 5 commits March 5, 2021 19:50
Signed-off-by: Luís Ferreira <contact@lsferreira.net>
Signed-off-by: Luís Ferreira <contact@lsferreira.net>
Signed-off-by: Luís Ferreira <contact@lsferreira.net>
Signed-off-by: Luís Ferreira <contact@lsferreira.net>
Signed-off-by: Luís Ferreira <contact@lsferreira.net>
@maxhaton
Copy link
Member

maxhaton commented Mar 5, 2021

but you are stuck with runtime, even at compile-time

I'm not sure what you mean here

@ljmf00
Copy link
Member Author

ljmf00 commented Mar 5, 2021

but you are stuck with runtime, even at compile-time

I'm not sure what you mean here

I meant, an alternative would be using mangle from druntime, but still not doable, because it doesn't give you the full potential. https://dlang.org/phobos/core_demangle.html#.mangle

With mangle you need to specify the type of the function to mangle it.

So I don't see any alternative that can totally substitute __MANGLED_FUNCTION__ by combining __FUNCTION__ or __PRETTY_FUNCTION__.

@maxhaton
Copy link
Member

maxhaton commented Mar 6, 2021

What's the use case?
And why does it need special casing when this feature can already be implemented using __FUNCTION__?

That's problematic when mixin(callee).mangleof can't reach, like when the function is a nested function and it's inside another scope or a module. You can kinda do it with mangle(__PRETTY_FUNCTION__) but you are stuck with runtime, even at compile-time, and mangle!() need the type explicitly, which could be tricky and even impossible in some cases, e.g. when the type is from another module or on another scope.

Imagine this situation:

void main()
{
    int foo()
    {
        mixin(__FUNCTION__).mangleof.writeln;
        return 1;
    }

    foo();
}

You want the return type, right, but in this case in the nested function you have it with typeof(return). I'm not saying I don't see the utility here, but I'm just not sure whether it's a good route to go down (Code depending on mangling?).

Also needs a test of C++ mangling probably (haven't played with it yet)

@ljmf00
Copy link
Member Author

ljmf00 commented Mar 6, 2021

You want the return type, right, but in this case in the nested function you have it with typeof(return). I'm not saying I don't see the utility here, but I'm just not sure whether it's a good route to go down (Code depending on mangling?).

Also needs a test of C++ mangling probably (haven't played with it yet)

Yeah, on discord I wanted the mangled name but not sure if it's the best way to get the return type as a string, although, this could be easily added to the runtime demangler anyway. The easiest way to do it would be to get it from __PRETTY_FUNCTION__.

I didn't want to show that case here, because it was very specific and it might not fit here. There's a way to get a mangled function from __FUNCTION__ and __PRETTY_FUNCTION__, although it is very tricky, not optimal, and sometimes even impossible to fetch.

I know, the end-user shouldn't generally depend on the mangled name, like parsing it or something, but this could be useful for other things like doing something that externally interfaces with the D ABI.

@Geod24
Copy link
Member

Geod24 commented Mar 6, 2021

I know, the end-user shouldn't generally depend on the mangled name, like parsing it or something, but this could be useful for other things like doing something that externally interfaces with the D ABI.

"It could be useful" really doesn't meet the burden of proof for an addition to the language.
My opinion is that we should fix whatever's not working with the current ways of doing it rather than introduce yet another token. Either core.demangle needs to be improved, or we might need a trait to get an alias to the current function (have you tried __traits(getParent) ?), but not such a special-cased feature.

@iK4tsu
Copy link
Contributor

iK4tsu commented Mar 6, 2021

Using core.demangle for this shouldn't be a solution for this as we shouldn't depend on the runtime for such ends. Using __traits(getParent) ?) or creating a new trait isn't a solution either because either you would need to pass the type itself or have access to the function which isn't possible within a scope.

auto stuff(...)()
{
	// need foo's return type here
    // to do some fancy stuff
	// and return the appropriate type
}

void main()
{
	int fooInt()
	{
		return stuff();
	}

	float fooFloat()
	{
		return stuff();
	}
}

It would be possible to obtain the return type "directly" if both functions were declared in the global scope, however neither fooInt nor fooFloat are accessible. As @ljmf00 said it's possible to accomplish this with FUNCTION and PRETTY_FUNCTION as the latter contains the return type. If a valid reason for this implementation comes forward, as the above is not a reason just an example, we shouldn't depend on the runtime for it.

@ljmf00
Copy link
Member Author

ljmf00 commented Mar 6, 2021

"It could be useful" really doesn't meet the burden of proof for an addition to the language.

I didn't had a concrete example, but let see, the case I presented, externally interface with the D ABI. Currently, there's no feasible way to get the mangled name.

Mangling is useful in the user perspective for various reasons ranging from logging mangled names for debugging purposes to dynamically loading certainly used D symbols externally with dlopen. It could be dangerous to do that, but it shouldn't be impossible.

Internally, the D profiler logs the mangled symbols to a file for possible usage by a viewer but the user can't do that.

Maybe the biggest things you can have with it is instrumentation, like you do with __LINE__, __FUNCTION__, __MODULE__ and __PRETTY_FUNCTION__. Basically, those tokens are used across Phobos for error logging and such.

If we look to other compilers like C++, although it's not a standard across all compilers, MSVC as __FUNCDNAME__ for the same purpose. https://docs.microsoft.com/en-us/cpp/preprocessor/predefined-macros?redirectedfrom=MSDN&view=msvc-160

My opinion is that we should fix whatever's not working with the current ways of doing it rather than introduce yet another token. Either core.demangle needs to be improved, [...]

Depending on the runtime for this is not optimal. The worst thing we have on D is depending on the runtime for reasonable basic functionality. I'm not against other implementations, but rather about your solution.

[..] or we might need a trait to get an alias to the current function (have you tried __traits(getParent) ?), but not such a special-cased feature.

__traits(getParent) is just impossible. That trait gets information from the symbol, not form a string. Even if you use a mixin, you fall on the same issue I firstly described:

void main()
{
    int foo()
    {
        mixin(__FUNCTION__).mangleof.writeln;
        return 1;
    }

    foo();
}

@maxhaton
Copy link
Member

maxhaton commented Mar 6, 2021

The worst thing we have on D is depending on the runtime for reasonable basic

Surely the alternative to depending on the runtime in this case is just writing your own demangler anyway?

@ljmf00
Copy link
Member Author

ljmf00 commented Mar 6, 2021

Surely the alternative to depending on the runtime in this case is just writing your own demangler anyway?

No that's not the point here. You can use the runtime demangler externally, and not on the user application that uses __MANGLED_FUNCTION__. You can perfectly log that into a file and parse it externally, like you do with the profiler file.

@ljmf00
Copy link
Member Author

ljmf00 commented Mar 7, 2021

Why not ? If it's CTFEable, I don't see a problem.

For you, it's not a problem, but for a lot of people that don't want to depend on the actual runtime code or want to have a minimal runtime for whatever reason, it is. @FFY00 is an example of an embedded developer in D. In fact, the current runtime implementation doesn't even work with CTFE, AFAIK.

// dmd -defaultlib= app.d
extern(C) void _d_dso_registry() {}
extern(C) pragma(printf) int printf(scope const char* format, scope const ...);

extern(C) void main()
{
	import core.demangle : demangle;
    printf("%s", (demangle("_D7example12__ModuleInfoZ") ~ '\0').ptr);
}
onlineapp.d:9: error: undefined reference to '_D4core8demangleQjFNaNbNfAxaAaZQd'
onlineapp.d:9: error: undefined reference to '_D11TypeInfo_Aa6__initZ'
onlineapp.d:9: error: undefined reference to '_d_arraycatT'
collect2: error: ld returned 1 exit status
Error: linker exited with status 1

I guess this is basically because demangle("_D7example12__ModuleInfoZ") is horribly implemented with exceptions for normal control flow and other weird stuff that doesn't make sense. I'm rewriting it tho, but that's not the point here, because you are depending on the runtime source code anyway. And you can say: oh yeah, just copy the demangler implementation from the runtime. To which I reply: that's just stupid and an horrible practice.

Well, that's why I proposed a new traits. It seems a bit illogical to have a trait to get the parent of a symbol, but not a symbol to the current context, as it's a common need.

That doesn't solve the problem again. If you want to use it as a default argument, the user needs to specifically use the trait instead. I can show you another example that you can't do with the current traits, plus, bear in mind that traits can't evaluate strings nor using mixins to transform the strings to symbols for nested functions.

template bar(string mfunc = __MANGLED_FUNCTION__)
{
    auto bar()
    {
        return mfunc;
    }
}

void main()
{
    int foo()
    {
        bar!();
        // pragma(msg, bar!());
        return 1;
    }
}

Tell me how you can do instrumentation without explicitly pass the symbol or using a __trait, here, on calling bar!(). I'm doing it with a template but it can be perfectly a normal function, tho.

.mangleof on a symbol is quite alright. And it's not hard to use either. For example, I have a library that simulates an in-memory network, similar to what vibe.web.rest does (but for unittesting), and when calling a function, you simply get the .mangleof of the overload and send that to the remote thread, which then matches it with the server: Geod24/localrest@aa890bd/source/geod24/LocalRest.d#L627-L670

Your example is using the actual symbol, __traits(getOverloads, API, member). Also you are stuck with passing the class/struct/module symbol too. The problem here is not about working with symbols, if it was, .mangleof is the solution.

Show me an example of how you can get the mangling without the demangler runtime and by using plain strings, if you want, like __PRETTY_FUNCTION__. Also, there's a problem I just discovered on getting the mangle from __PRETTY_FUNCTION__ because of what @dkorpel addressed. __MANGLED_FUNCTION__ should be different and/or __PRETTY_FUNCTION__ should have a different behavior.

Once again, .mangleof will do the trick here.

Once again, you are dependent on the symbol. You can't pass the symbol implicitly as you do with __FUNCTION__.

You can also parse the binary, that's probably less trouble.

?? That's pointless. Instrumentation is about, for example, logging the mangled symbol on runtime function calls., like you do on the compiler profiler. Parsing the binary is either complex, tedious and doesn't fit here, anyway.

See more about applications of instrumentation here: https://en.wikipedia.org/wiki/Instrumentation_(computer_programming)#Output

Once again: What's the problem with depending on the runtime if the code is self contained and CTFEable ?

I proved to you that currently, CTFE does nothing to the current runtime implementation. And already talked about that above.

Well another issue that comes up is that even if you use foo.mangleof, you'll get this very explicit error message:

onlineapp.d(5): Error: function onlineapp.main.foo cannot retrieve its .mangleof while inferring attributes

So I don't see how this solution could work anyways.

I can investigate more about it. I guess it's possible to do it but needs another semantic stage.

@FFY00
Copy link

FFY00 commented Mar 7, 2021

We consider this case as well, and try to make D as pay-as-you-go as possible. There's a difference between depending on the runtime for core functionalities (e.g. assert) and telling people "use the runtime" for non-core functionalities (e.g. demangling).

sigh

This is how you run people out of your language. I have spent a lot of time and effort over the years trying to give D a real try for proper projects, I also spent a significant amount of time trying to fix some of the issues I was running into, but I gave up after consistently running up into issues like this and receiving this kind of friction trying to do anything. Every time I try to come back I run into the same kind of stuff... It's honestly very demotivating.

This problem is fairly simple. I want a mangled version of __FUNCTION__ at compile time without any having runtime implementation.

As of now, you failed to provide any real solution.

Also, in this case, the idea is that you probably want to call the demangler at CTFE, not runtime, so you can just drop core.demangle in your own runtime / project, perhaps add a few if (!__ctfe) assert(0); to make sure it's never called at runtime / doesn't do codegen, then just use it with -betterC.

No, I actually need to call the mangler. But neither of the variants available, core.demangle.mangle and core.demangle.mangleFunc are able to do this from __FUNCTION__, they need extra information.

And other big issue, where in the spec does it guarantee that what you propose will be CTFE-ed?
https://dlang.org/spec/function.html#interpretation

Let's take a simple example.

extern(C) pragma(printf) int printf(scope const char* format, scope const ...);

extern(C) void main()
{
    auto foo(immutable(char)* caller = (__MANGLED_FUNCTION__ ~ '\0').ptr)
    {
        printf("%s\n", caller);
    }

    foo();
}

Please show me how I can do that in a way that the spec guarantees will happen at compile time.

Well, that's why I proposed a new traits. It seems a bit illogical to have a trait to get the parent of a symbol, but not a symbol to the current context, as it's a common need.

Why is there __PRETTY_FUNCTION__ instead of it being done via traits then?
What would that approach look like? A mangle trait? That can somehow take __FUNCTION__? This seems like a really bad decision.

@ibuclaw
Copy link
Member

ibuclaw commented Mar 7, 2021

It would be possible to obtain the return type "directly" if both functions were declared in the global scope, however neither fooInt nor fooFloat are accessible.

I haven't seen all the discussion so risk sounding like a broken record. But...

return​ ​stuff!(typeof(return))​();

@ibuclaw
Copy link
Member

ibuclaw commented Mar 7, 2021

"It could be useful" really doesn't meet the burden of proof for an addition to the language.

I didn't had a concrete example, but let see, the case I presented, externally interface with the D ABI. Currently, there's no feasible way to get the mangled name.

For cross language binary interfaces, this is what you have extern(C) for. Every practical language - both compiled and interpreted - is capable of interfacing with extern(C).

Another alternative is pragma(mangle)

@FFY00
Copy link

FFY00 commented Mar 7, 2021

For cross language binary interfaces, this is what you have extern(C) for. Every practical language - both compiled and interpreted - is capable of interfacing with extern(C).

Another alternative is pragma(mangle)

We are aware, but that does not negate this. It is merely an alternative for some situations.

@ljmf00
Copy link
Member Author

ljmf00 commented Mar 7, 2021

How would you handle the circular dependency issue I described?

    static if (isNothrow(__MANGLED_FUNCTION__)) {
        throw new Exception(""); // creates contradiction
    }

You can check if the static if scope is throwable or not, as well as safety, nogc,... and decide after that. I'm not aware of how the compiler checks that tho, but the only situation that occurs is by using the __MANGLED_FUNCTION__ referring to the same scope.

@ibuclaw
Copy link
Member

ibuclaw commented Mar 8, 2021

For cross language binary interfaces, this is what you have extern(C) for. Every practical language - both compiled and interpreted - is capable of interfacing with extern(C).
Another alternative is pragma(mangle)

We are aware, but that does not negate this. It is merely an alternative for some situations.

Yes it does if the intent is to call from another language, as extern(D) functions pass data around differently to C functions. Integers aren't promoted, reals are returned on x87 registers, complex numbers are passed in reverse, arrays and built-in types are passed around differently to their equivalent struct types, and small structs may be passed by invisible reference instead of registers. Unless the other language is aware of how the D ABI works, the most reliable way to interface with an extern(D) function is with another D program.

@ljmf00
Copy link
Member Author

ljmf00 commented Mar 8, 2021

Yes it does if the intent is to call from another language, as extern(D) functions pass data around differently to C functions. Integers aren't promoted, reals are returned on x87 registers, complex numbers are passed in reverse, arrays and built-in types are passed around differently to their equivalent struct types, and small structs may be passed by invisible reference instead of registers. Unless the other language is aware of how the D ABI works, the most reliable way to interface with an extern(D) function is with another D program.

Yes, although, that shouldn't impossibilitate the user to do it, plus instrumentation doesn't necessarily need to deal with interfacing.

@Geod24
Copy link
Member

Geod24 commented Mar 8, 2021

In fact, the current runtime implementation doesn't even work with CTFE, AFAIK.

It does if you don't provide any flag, but the code example you gave is not correct. If you want something to be CTFEd, you need to assign it to a variable that is const / immutable / enum. Also, if you are going with -betterC, you do not want to use the GC, so appending a null byte to the string is a no go.

A more correct example would be:

extern(C) void _d_dso_registry() {}
extern(C) pragma(printf) int printf(scope const char* format, scope const ...);

import core.demangle : demangle;

immutable Mangling = demangle("_D7example12__ModuleInfoZ");

extern(C) void main()
{
    printf("%.*s", (cast(int) Mangling.length), Mangling.ptr);
}

Now there is a bug here: It seems that DMD chokes on exceptions in CTFE code with -betterC. A quick look on the issue tracker pointed to this issue. Fixing this issue would allow you to use the demangler as you wish, without importing its run time code into your application.

[...]

From the rest of your post, it seems you want to be able to pass it as a default argument, which is why you went with implementing it as a token ?

@ljmf00
Copy link
Member Author

ljmf00 commented Mar 8, 2021

[...]

From the rest of your post, it seems you want to be able to pass it as a default argument, which is why you went with implementing it as a token ?

Exactly!

I find no other way to do that without a token like that. If there is, it would be great.

Also, the problem about .mangleof it seems it's being evaluated on the wrong semantic phase, as function attributes and return types are forward references. I managed to implement it with the same restrictions of .mangleof but if anyone can help me with this, I can change this to work, but at least it now (not yet pushed) gives an error when forward reference found or attributes are in the inference proccess, so it always gives the right mangled name.

This forward reference problem is, right now, happening on __PRETTY_FUNCTION__ token, as it doesn't give the full pretty function when using inferred attributes or return type. At this point __PRETTY_FUNCTION__ shouldn't give an error as it is a breaking change, but can surely be improved to infer that types first.

@Geod24
Copy link
Member

Geod24 commented Mar 8, 2021

@FFY00 :

This is how you run people out of your language. I have spent a lot of time and effort over the years trying to give D a real try for proper projects, I also spent a significant amount of time trying to fix some of the issues I was running into, but I gave up after consistently running up into issues like this and receiving this kind of friction trying to do anything. Every time I try to come back I run into the same kind of stuff... It's honestly very demotivating.

Just to be clear here, I'm not trying to gatekeep here. I, and I'm sure other contributors, would really like to understand why D is currently not fitting your use case. But it seems that we're missing a piece of the story, as a conversation happened on Discord, and it seems that now, people are fixating on this approach as being the only possible alternative. It might be true, but the proposer needs to make a case for it. This PR was created without any explanation as to its use case, and why the established alternatives are not working. That's why you are seeing pushback.

Please show me how I can do that in a way that the spec guarantees will happen at compile time.

The easy way is to make it a template argument. To force the caller to CTFE the default argument, I currently can't think of anything else that would work with tokens.

What would that approach look like?

Something like:

void logCaller (string mangledName = __traits(getCurrentSymbol).mangleof)
{
}

Provided that the default argument is correctly evaluated in the caller's context, this would work without special case, and be useful for other people as well.


However, if your use case is instrumentation, there are other approaches that work, too. Other profilers, such as tracy, will just use the stack trace for that. We have bindings for LLVM's libunwind in druntime (https://github.com/dlang/druntime/blob/master/src/core/internal/backtrace/libunwind.d) which you might want to look into (that's what I was getting at with my "parse the binary" approach, sometimes a runtime approach is the best option).

@FFY00
Copy link

FFY00 commented Mar 8, 2021

Just to be clear here, I'm not trying to gatekeep here. I, and I'm sure other contributors, would really like to understand why D is currently not fitting your use case.

From our limited interactions, you are one of the few people that hasn't been doing this, but most of the people are often dismissive, which you can even see on this thread, being it intentional or not. At this point I can't be assuming good faith on everything because this happens so much.

But it seems that we're missing a piece of the story, as a conversation happened on Discord, and it seems that now, people are fixating on this approach as being the only possible alternative. It might be true, but the proposer needs to make a case for it.

I was not part of that conversation, I have been simply trying to make a point for this use-case here. I am not fixated on this approach, but it does seem like the clear obvious to me -- see my reply below.

This PR was created without any explanation as to its use case, and why the established alternatives are not working. That's why you are seeing pushback.

The use-case is pretty self-explanatory, easily accessing the mangled name of the current function.

The easy way is to make it a template argument. To force the caller to CTFE the default argument, I currently can't think of anything else that would work with tokens.

Again, embedded, I am working with limited space, so I am not templating a big function because of this. That is purely a work around that happens to work on desktop because you don't care about the object size, it is an incorrect approach to the problem -- the code is the same, I should not be forced to make multiple copies of it. When you start sacrificing correctness "because this way works", you are most likely dismissing use-cases, because "your way" that wasn't supposed to be used for this will a lot of the times not translate to other situations. This is generally what I think has been going wrong with D and its unfitness to be practically used for embedded development and other not-desktop use cases.

Something like:

void logCaller (string mangledName = __traits(getCurrentSymbol).mangleof)
{
}

Provided that the default argument is correctly evaluated in the caller's context, this would work without special case, and be useful for other people as well.

Again, why is there a __PRETTY_FUNCTION__ then and not just that?
AFAIK, no trait currently returns a value based on the context it was called, and I think that is a dangerous step to make.
Following your line of thought, why aren't the other tokens traits? What exactly makes it so that you think some things should be tokens and some should be traits? Is it the complexity in parsing? If so, I think that ship has already sailed. This seems completely arbitrary.
As no traits currently depend on the context they are called (please correct me if I am missing something), using a token seems like the obvious approach.

@FFY00
Copy link

FFY00 commented Mar 8, 2021

this would work without special case, and be useful for other people as well

Building on this, I think what would actually be useful would be

__traits(getSymbol, __MANGLED_FUNCTION__)

Instead of making it only able to get the symbol in the current context.

@dkorpel
Copy link
Contributor

dkorpel commented Mar 8, 2021

You can check if the static if scope is throwable or not, as well as safety, nogc,... and decide after that. I'm not aware of how the compiler checks that tho, but the only situation that occurs is by using the __MANGLED_FUNCTION__ referring to the same scope.

I think you're underestimating how difficult this is. I wouldn't know with what strategy dmd could infer a return type for this:

auto foo() {
    mixin(crazyCodeGeneratingTemplate!__MANGLED_FUNCTION__);
}

But I'll be happy if you could make it work.

@dkorpel
Copy link
Contributor

dkorpel commented Mar 8, 2021

The use-case is pretty self-explanatory, easily accessing the mangled name of the current function.

That's not a use-case, just a description of what it does. It's like saying "the use case for __FUNCTION_LINE_COUNT__ is when you want to know how many lines of code the current function consists of". It doesn't make a case for why it's a worthwhile addition.

I can sort of kind of see what this can be used for and can relate to why workarounds are unsatisfactory, but still have a hard time picturing how this would be used concretely in an actual project, so a specific complete example could be useful here.

@ghost
Copy link

ghost commented Mar 10, 2021

What's the use case?
And why does it need special casing when this feature can already be implemented using __FUNCTION__?

That's problematic when mixin(callee).mangleof can't reach, like when the function is a nested function and it's inside another scope or a module. You can kinda do it with mangle(__PRETTY_FUNCTION__) but you are stuck with runtime, even at compile-time, and mangle!() need the type explicitly, which could be tricky and even impossible in some cases, e.g. when the type is from another module or on another scope.

Imagine this situation:

void main()
{
    int foo()
    {
        mixin(__FUNCTION__).mangleof.writeln;
        return 1;
    }

    foo();
}

I wonder if this old PR allowed to get the right mangle,
e.g __traits(getCurrentFunction).mangleof.

@Geod24
Copy link
Member

Geod24 commented Mar 10, 2021

From our limited interactions, you are one of the few people that hasn't been doing this, but most of the people are often dismissive, which you can even see on this thread, being it intentional or not. At this point I can't be assuming good faith on everything because this happens so much.

Thanks for the compliment, but, and I'm sure you already know this, not assuming good faith isn't a workable basis to have a conversation on the merit's of an open source contribution. And that goes both ways, obviously, reviewers have to assume good faith from the contributors.

The use-case is pretty self-explanatory, easily accessing the mangled name of the current function.

As mentioned by @dkorpel , it's not a use case as much as it is a specific approach to a use case. And since there were a few use cases being mixed up (that discussion on Discord, your own, potentially others?), the conversation got quite confusing IMHO.

That is purely a work around that happens to work on desktop [...]

I agree, and I think it should work. I'm not even saying it's a good workaround. What I wanted to point out was that we needed to understand the use case, what work, and what didn't, to find the best fix that would fit within the big picture that D has.
There were a few things suggested in this thread:

To be clear, I (or any other reviewers) could just merge this PR, as it's fairly simple and @ljmf00 has been perfectly following the contributions guidelines (BTW: thanks for the quality of your contributions, past and present). Then we call it a day.
Until, a few days / weeks / months / years down the road, someone comes with a slightly different use case. They want some information to be usable as a default argument, but it doesn't work, so they implement a token. Maybe that use case is even extremely close to the current use case (what about wanting the __PRETTY_FUNCTION__ of a template declaration, instead of the instantiation for example?). And we merge it again, because we did it for __MANGLED_FUNCTION__, so why not __PRETTY_FUNCTION_DECLARATION__ ? And as you can guess, the story just keeps on repeating itself, because we haven't fixed the underlying issue (18919), just the symptoms.

So when there's a PR trying to introduce a new feature, my first question will always be "What's the use case?", so that we can be sure that the language actually is what you need it to be, not only what you ask it to be.

Again, why is there a __PRETTY_FUNCTION__ then and not just that?

I believe it is because __FUNCTION__ predates it.

AFAIK, no trait currently returns a value based on the context it was called, and I think that is a dangerous step to make.

There's a few that do. They take imports and visibility into account, if nothing else.

Following your line of thought, why aren't the other tokens traits? What exactly makes it so that you think some things should be tokens and some should be traits? Is it the complexity in parsing? If so, I think that ship has already sailed. This seems completely arbitrary.

The reason is likely historical: Tokens were implemented in D1, __traits came with D2. I think it's the right decision for __FILE__ and __LINE__, but I'd like to get a pseudo-struct for __VERSION__ (with the major, minor, patch, maybe other infos) instead of the current number. This could even be folded into __traits(getTargetInfo). But that's another topic.

@atilaneves
Copy link
Contributor

What's the use case?
And why does it need special casing when this feature can already be implemented using __FUNCTION__?

That's problematic when mixin(callee).mangleof can't reach, like when the function is a nested function and it's inside another scope or a module. You can kinda do it with mangle(__PRETTY_FUNCTION__) but you are stuck with runtime, even at compile-time, and mangle!() need the type explicitly, which could be tricky and even impossible in some cases, e.g. when the type is from another module or on another scope.

Imagine this situation:

void main()
{
    int foo()
    {
        mixin(__FUNCTION__).mangleof.writeln;
        return 1;
    }

    foo();
}

For a non-nested function, this works:

    int foo() {
        __traits(parent, {}).mangleof.writeln;
        return 1;
    }

If it's nested, however, we get this error:

d.d(8): Error: function d.main.foo cannot retrieve its .mangleof while inferring attributes

So, as I see it, there's a workaround for non-nested functions right now, and the example given wouldn't work with a __MANGLED_FUNCTION__ feature for the same reason the workaround doesn't work now - attribute inference.

@FFY00
Copy link

FFY00 commented Mar 10, 2021

Thanks for the compliment, but, and I'm sure you already know this, not assuming good faith isn't a workable basis to have a conversation on the merit's of an open source contribution. And that goes both ways, obviously, reviewers have to assume good faith from the contributors.

I still assume good faith in the interactions themselves, just not in the silent dismissal of certain points. Me assuming that "they just forgot" and not pressing for the points to be addressed, like I did here, is exactly what drove me out of the community.

As mentioned by @dkorpel , it's not a use case as much as it is a specific approach to a use case. And since there were a few use cases being mixed up (that discussion on Discord, your own, potentially others?), the conversation got quite confusing IMHO.

Sure. If you want a proper use-case I have, I want to cache some things based on the caller, this is helpful for embedded development. I can do this no problem with C, the names are not mangled, so I can use __func__.

I agree, and I think it should work. I'm not even saying it's a good workaround. What I wanted to point out was that we needed to understand the use case, what work, and what didn't, to find the best fix that would fit within the big picture that D has.
There were a few things suggested in this thread:

* Ability to get the current symbol - As you can see in #11538 I asked for use cases, and this would fit perfectly;

* Ability to force CTFE on default argument while retaining call-site evaluation - [We have a bug report for it](https://issues.dlang.org/show_bug.cgi?id=18919). And if you look at the duplicate, you can see [I was struck by it myself](https://issues.dlang.org/show_bug.cgi?id=21211).

To be clear, I (or any other reviewers) could just merge this PR, as it's fairly simple and @ljmf00 has been perfectly following the contributions guidelines (BTW: thanks for the quality of your contributions, past and present). Then we call it a day.
Until, a few days / weeks / months / years down the road, someone comes with a slightly different use case. They want some information to be usable as a default argument, but it doesn't work, so they implement a token. Maybe that use case is even extremely close to the current use case (what about wanting the __PRETTY_FUNCTION__ of a template declaration, instead of the instantiation for example?). And we merge it again, because we did it for __MANGLED_FUNCTION__, so why not __PRETTY_FUNCTION_DECLARATION__ ? And as you can guess, the story just keeps on repeating itself, because we haven't fixed the underlying issue (18919), just the symptoms.

So when there's a PR trying to introduce a new feature, my first question will always be "What's the use case?", so that we can be sure that the language actually is what you need it to be, not only what you ask it to be.

That is fine then, if you think introducing this in traits is the correct approach, go for it. It just wasn't to me from the context I had. Also, please note that my original comment was a reply to you telling us to use core.demangle at CTFE, which is very different from using traits. Traits works for my use-case, I just had doubts about it being the right choice.

I believe it is because __FUNCTION__ predates it.

I thought __FUNCTION__ and __PRETTY_FUNCTION__ were added at the same time, perhaps I am mistaken.

There's a few that do. They take imports and visibility into account, if nothing else.

Okay, that makes sense.

The reason is likely historical: Tokens were implemented in D1, __traits came with D2.

Shouldn't the current tokens be added as traits and marked for removal in a future version then?

I think it's the right decision for __FILE__ and __LINE__, but I'd like to get a pseudo-struct for __VERSION__ (with the major, minor, patch, maybe other infos) instead of the current number. This could even be folded into __traits(getTargetInfo). But that's another topic.

I share this opinion, I had actually written this in my previous reply but choose to remove it.

@ljmf00
Copy link
Member Author

ljmf00 commented Mar 10, 2021

But it seems that we're missing a piece of the story, as a conversation happened on Discord, and it seems that now, people are fixating on this approach as being the only possible alternative.

Yeah. The conversation on Discord kinda deviates from the usefulness of this approach. I understand that people without the context may feel a bit confused and I miss some rationale here.

I think you're underestimating how difficult this is. I wouldn't know with what strategy dmd could infer a return type for this:

auto foo() {
    mixin(crazyCodeGeneratingTemplate!__MANGLED_FUNCTION__);
}

But I'll be happy if you could make it work.

Yes. I don't think DMD can do it right now. I can give an error if some forward reference occurs in that situation.

I wonder if this old PR allowed to get the right mangle,
e.g __traits(getCurrentFunction).mangleof.

I've been looking for that. I've seen one thing there that could lead to inconsistency with the current approach, using tokens.

To be clear, I (or any other reviewers) could just merge this PR, as it's fairly simple and @ljmf00 has been perfectly following the contributions guidelines (BTW: thanks for the quality of your contributions, past and present). Then we call it a day.
Until, a few days / weeks / months / years down the road, someone comes with a slightly different use case. They want some information to be usable as a default argument, but it doesn't work, so they implement a token. Maybe that use case is even extremely close to the current use case (what about wanting the PRETTY_FUNCTION of a template declaration, instead of the instantiation for example?). And we merge it again, because we did it for MANGLED_FUNCTION, so why not PRETTY_FUNCTION_DECLARATION ? And as you can guess, the story just keeps on repeating itself, because we haven't fixed the underlying issue (18919), just the symptoms.

Thanks. Appreciate :)

Ok. I guess we are getting somewhere with this. Now, I totally understand your point here.

This case could introduce more complexity and keep the same old problem about getting the raw symbol. Maybe a good approach is to use traits and getting rid of those tokens, except, essential ones like __LINE__, __FILE__ or even __MODULE__.

The reason is likely historical: Tokens were implemented in D1, __traits came with D2. I think it's the right decision for FILE and LINE, but I'd like to get a pseudo-struct for VERSION (with the major, minor, patch, maybe other infos) instead of the current number. This could even be folded into __traits(getTargetInfo). But that's another topic.

This makes even more sense. I didn't think that this came from D1, tho.

Removing __FUNCTION__, __PRETTY_FUNCTION__ with a deprecation process is the right path, as you can fetch either the fully qualified name of the symbol, the stringof or the mangleof.

@ljmf00
Copy link
Member Author

ljmf00 commented Mar 10, 2021

The ghost user contacted me via email. If you want to come up with his idea on #11538 , I can rebase it and work from those commits with the original authorshipp. I can ask him/her some context too.

@dkorpel
Copy link
Contributor

dkorpel commented Mar 24, 2022

Is this still being pursued?

@ljmf00
Copy link
Member Author

ljmf00 commented Mar 24, 2022

Is this still being pursued?

I don't think so. This will eventually be superseded by the proposed __trait to get the current function symbol.

@dkorpel
Copy link
Contributor

dkorpel commented Mar 24, 2022

Okay, then I'll close it for now in favor of someone rebooting #11538

@dkorpel dkorpel closed this Mar 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review:Needs Rebase Review:Needs Spec PR A PR updating the language specification needs to be submitted to dlang.org Review:Needs Work Review:stalled
Projects
None yet
Development

Successfully merging this pull request may close these issues.