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

RFC: Add "function name macro" #466

Merged
merged 0 commits into from Nov 25, 2014

Conversation

@mitsuhiko
Copy link
Contributor

mitsuhiko commented Nov 17, 2014

This RFC proposes the addition of a function! macro that expands to the function it's used in. This will greatly help error reporting.

Rendered View

`Some(name)` where `name` is the name of the function. It is always the
local name of the function which means even if a function is contained in
another function, it's just the the actual name.
* when used inside a local block in a function (proc, closure or anything else) it

This comment has been minimized.

Copy link
@nrc

nrc Nov 17, 2014

Member

What name will you give for anonymous functions such as closures?

This comment has been minimized.

Copy link
@mitsuhiko

mitsuhiko Nov 17, 2014

Author Contributor

As mentioned the name of the function they are contained in is used.

This comment has been minimized.

Copy link
@bstrie

bstrie Nov 17, 2014

Contributor

The function in which they are defined, or the function in which they are invoked?

This comment has been minimized.

Copy link
@scialex

scialex Nov 18, 2014

It would need to be the one they are defined in, unless we want to hold onto that information and pass it through dynamically, which seems heavy for a debug feature.

* when used within a proper function `fn` (or trait method) it expands to
`Some(name)` where `name` is the name of the function. It is always the
local name of the function which means even if a function is contained in
another function, it's just the the actual name.

This comment has been minimized.

Copy link
@nrc

nrc Nov 17, 2014

Member

Why do you want the short name and not the fully qualified name?

This comment has been minimized.

Copy link
@huonw

huonw Nov 17, 2014

Member

module_path!() gives the full module qualification, so concat!(module_path!(), "::", function!()) would give the fully qualified name.

This comment has been minimized.

Copy link
@mitsuhiko

mitsuhiko Nov 17, 2014

Author Contributor

It was mentioned in IRC that it might make sense to add the contained trait (eg: FooBar::foo_bar()). Not sure though if that should be the default or not.

This comment has been minimized.

Copy link
@bstrie

bstrie Nov 17, 2014

Contributor

even if a function is contained in another function, it's just the the actual name

I'm not sure what this means. Can you elaborate? If foo contains bar, what does function!() within bar produce?

@thestinger

This comment has been minimized.

Copy link

thestinger commented Nov 17, 2014

I don't think Rust should continue down this road. I consider the usage of row / column numbers in failure messages to be a serious design flaw. It's a severe code bloat / performance issue along with resulting in terrible messages with only the local context (like Option::unwrap). It should just be using debug info to print out the source location and a traceback.

@mitsuhiko

This comment has been minimized.

Copy link
Contributor Author

mitsuhiko commented Nov 17, 2014

@thestinger what's your alternative? I rely on this error information to produce proper debug information. Taking it away without providing a replacement is not exactly a good idea. Obviously it's not fast, but that's hardly the point of a debug helper.

@dobkeratops

This comment has been minimized.

Copy link

dobkeratops commented Nov 17, 2014

+1 to this. Also I've always wondered if there could be a macro for the 'enclosing' function name in the case of inlined generic code.. you want to know where its' invoked from (e.g. iterators etc).
and as pointed out by the OP, just because something isn't useful for Release, doesn't mean it isn't useful . There's only so much you can determine at compile time... Rust doesn't obsolete the concept of debug builds with extra checks & so on.

@mitsuhiko

This comment has been minimized.

Copy link
Contributor Author

mitsuhiko commented Nov 17, 2014

I think this RFC should be seen separate to finding alternatives to line/file etc. I understand that @thestinger is concerned with compiling too much bloat into binaries but there are plenty of situations where this is actually a much more reasonable approach than using libbacktrace. For instance any logging system will probably see much higher performance from using line!() and others over trying to read DWARF info whenever it needs to log something.

@mitsuhiko mitsuhiko force-pushed the mitsuhiko:master branch from 18b158c to d5932dd Nov 17, 2014
@mitsuhiko mitsuhiko referenced this pull request Nov 17, 2014

# Drawbacks

Maybe `function!` is a bit generic of a name. Alternative names could be considered.

This comment has been minimized.

Copy link
@bstrie

bstrie Nov 17, 2014

Contributor

function!() does strike me as too generic. I much prefer the descriptiveness of function_name!(). I don't mind the underscore because ideally you'd have this wrapped in some more general error handling device.

@mitsuhiko mitsuhiko changed the title Add "function name macro" RFC: Add "function name macro" Nov 17, 2014
@andrew-d

This comment has been minimized.

Copy link

andrew-d commented Nov 18, 2014

+1 to this - would find this kind of information very useful, and there's precedent in systems languages already: C/C++ compilers provide the __func__ #define already.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Nov 21, 2014

cc rust-lang/rust#9668, quite an old PR which attempted this some time ago.

I do think that the actual output of this macro is somewhat orthogonal to considering its inclusion. Many uses of function!() have an obvious return value, and I would suspect that we are at liberty to alter the "flavorful" use cases throughout the lifetime of the compiler (e.g. change the output in rust 1.1 and then again in rust 1.4). So in that sense, I don't want to get too bogged down into the details of what precisely is being emitted just yet. Nevertheless, I have a few questions!

  • As a prelude macro (sorry about that!) we should take careful consideration of the name of this macro as well as its interface (primarily the output type). We won't be able to change it for forever most likely.
  • The return type of this macro is interesting, you propose Option<&'static str> but as @huonw points out it would be quite useful to use concat! with a pre-existing macro like module_path! in order to generate a "full path".
  • How useful is the vanilla name of a function? For example, a type could implement any number of methods named foo through a number of traits. I think that considering the scope of function may also be critical for this RFC (e.g. whether it's in an impl, whether it's a free function, whether it's a trait function, etc).

In general I don't think any of us have given an inordinate amount of thought to these "debugging related" macros in terms of long term stability. Most of them seem fairly harmless, but committing to provide all of them for all Rust programs forever is a strong commitment to make. We may want to briefly consider the story of these macros in conjunction with considering adding this new macro.

Regardless, thank you for taking the time to write this up @mitsuhiko! The work you're doing with error reporting is quite exciting, and I'd love to improve it even more!

@mitsuhiko mitsuhiko force-pushed the mitsuhiko:master branch from 18b158c to d5932dd Nov 24, 2014
@alexcrichton alexcrichton merged commit d5932dd into rust-lang:master Nov 25, 2014
@liigo

This comment has been minimized.

Copy link
Contributor

liigo commented Dec 3, 2014

Is this RFC merged normally? I can't find its content in both rfcs repo and @mitsuhiko 's "Rendered View" link. Github shows there is no commit and no file changed in this PR. @alexcrichton

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Dec 3, 2014

No this was an accident when @mitsuhiko updated his local master branch, I believe he was going to re-open the PR, or did I mis-hear @mitsuhiko?

@nstoddard

This comment has been minimized.

Copy link

nstoddard commented Jun 21, 2015

Is this still being worked on? The 'rendered view' link doesn't work.

@androm3da

This comment has been minimized.

Copy link

androm3da commented Sep 18, 2015

I support the addition of a macro like __func__. I recognize the challenge in deciding on what/how to report on the context/path of this particular function. What's needed to move forward? A proposal that addresses @alexcrichton 's points? An implementation?

@ghost

This comment has been minimized.

Copy link

ghost commented Jan 28, 2018

Rendered View

EDIT: Well, ignore this current issue, looks like the following one is overriding it AND still open: #1743

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.