www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - string file = __FILE__ considered harmful (and solution)

reply FeepingCreature <feepingcreature gmail.com> writes:
There's a very common idiom where in order to report line numbers 
of an error or a log line at the callsite of a function, you pass 
__FILE__ and __LINE__ as default parameters:

void foo(string file = __FILE__, size_t line = __LINE__);

What's wrong with this?

Say you add a string parameter, such as

void foo(string msg, string file = __FILE__, size_t line = 
__LINE__);

foo("Hello World");

Now when you accidentally grab an old version of the library, 
your new code will still run, but it will believe that it's being 
called from file "Hello World", line 15. Not good.

Luckily there's a fix. Just stick this in some common header file 
in your project:

struct CallerInfo
{
   string file;
   size_t line;
}

void foo(string msg, CallerInfo caller = CallerInfo(__FILE__, 
__LINE__));

Now you cannot accidentally invoke foo with a string, or in fact 
any type except another instance of CallerInfo.

This is such a basic type that it really belongs in phobos, 
arguably object.d. At which point we can shorten this to 
CallerInfo caller = __CALLER__, and be forward compatible for 
additional information about the callsite, such as, say, 
attributes of the calling function.
May 30 2018
next sibling parent rikki cattermole <rikki cattermole.co.nz> writes:
On 30/05/2018 8:27 PM, FeepingCreature wrote:

 There's a very common idiom where in order to report line numbers of an 
 error or a log line at the callsite of a function, you pass __FILE__ and 
 __LINE__ as default parameters:
 
 void foo(string file = __FILE__, size_t line = __LINE__);
 
 What's wrong with this?
 
 Say you add a string parameter, such as
 
 void foo(string msg, string file = __FILE__, size_t line = __LINE__);
 
 foo("Hello World");
 
 Now when you accidentally grab an old version of the library, your new 
 code will still run, but it will believe that it's being called from 
 file "Hello World", line 15. Not good.
 
 Luckily there's a fix. Just stick this in some common header file in 
 your project:
 
 struct CallerInfo
 {
    string file;
    size_t line;
 }
 
 void foo(string msg, CallerInfo caller = CallerInfo(__FILE__, __LINE__));
 
 Now you cannot accidentally invoke foo with a string, or in fact any 
 type except another instance of CallerInfo.
 
 This is such a basic type that it really belongs in phobos, arguably 
 object.d. At which point we can shorten this to CallerInfo caller = 
 __CALLER__, and be forward compatible for additional information about 
 the callsite, such as, say, attributes of the calling function.


ooo I have another solution.

Use a named argument[0]!

[0] https://github.com/rikkimax/DIPs/blob/named_args/DIPs/DIP1xxx-RC.md
May 30 2018
prev sibling next sibling parent reply FeepingCreature <feepingcreature gmail.com> writes:
Shit it doesn't work, I only checked if it compiled; it gives the 
wrong file/line number.

NEVERMIND ALL
May 30 2018
parent reply FeepingCreature <feepingcreature gmail.com> writes:
Updated subject to be visible at a glance.

Note that a compiler-based solution via __CALLER__ would still 
work.
May 30 2018
parent Steven Schveighoffer <schveiguy yahoo.com> writes:
On 5/30/18 4:46 AM, FeepingCreature wrote:

 Updated subject to be visible at a glance.
 
 Note that a compiler-based solution via __CALLER__ would still work.


I saw this after I replied. Oops :)

I think it's a good idea, and I'm also believing that the issue that 
causes it not to work really could be addressed quite easily.

-Steve
May 30 2018
prev sibling next sibling parent reply FeepingCreature <feepingcreature gmail.com> writes:
On Wednesday, 30 May 2018 at 08:27:16 UTC, FeepingCreature wrote:

 struct CallerInfo
 {
   string file;
   size_t line;
 }


Let me try to flesh this out.

void foo(CallContext caller = __CALL_CONTEXT__)
{
}

// in druntime object.d
struct CallContext
{
   public size_t line; // expected to change for every call
   private const FunctionContext* functionContext; // expected to 
be constant for many calls
   alias functionContext this;
}

/** a variable with this type is lazily allocated once per 
function */
struct FunctionContext
{
   string file, fileFullPath, module, function, prettyFunction;
}

I've looked at DMDFE and I don't think I know enough to do this, 
annoyingly.
May 30 2018
parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
What about this?

------
struct EndOfArgs { }
EndOfArgs eoa;

void func(string s, EndOfArgs _ = eoa,
	string file = __FILE__, size_t line = __LINE__)
{
	import std.stdio;
	writefln("%s:%d: msg=%s", file, line, s);
}

void main() {
	func("hello");
	func("there");
}
------


Basically, use a dummy empty struct to differentiate between real
arguments and context info.


T

-- 
Don't drink and derive. Alcohol and algebra don't mix.
May 30 2018
parent reply FeepingCreature <feepingcreature gmail.com> writes:
On Wednesday, 30 May 2018 at 10:05:42 UTC, H. S. Teoh wrote:

 What about this?

 ------
 struct EndOfArgs { }
 EndOfArgs eoa;

 void func(string s, EndOfArgs _ = eoa,
 	string file = __FILE__, size_t line = __LINE__)
 {
 	import std.stdio;
 	writefln("%s:%d: msg=%s", file, line, s);
 }

 void main() {
 	func("hello");
 	func("there");
 }
 ------


 Basically, use a dummy empty struct to differentiate between 
 real arguments and context info.


 T


Thank you, this seems to work well!

We're using struct Fence { } Fence _ = Fence(), and it doesn't 
add much overhead.

Barring the proposed compiler change, this seems the cleanest fix.
May 30 2018
parent reply Daniel N <no public.email> writes:
On Wednesday, 30 May 2018 at 12:22:28 UTC, FeepingCreature wrote:

 On Wednesday, 30 May 2018 at 10:05:42 UTC, H. S. Teoh wrote:

 What about this?

 ------
 struct EndOfArgs { }
 EndOfArgs eoa;

 void func(string s, EndOfArgs _ = eoa,
 	string file = __FILE__, size_t line = __LINE__)
 {
 	import std.stdio;
 	writefln("%s:%d: msg=%s", file, line, s);
 }

 void main() {
 	func("hello");
 	func("there");
 }
 ------


 Basically, use a dummy empty struct to differentiate between 
 real arguments and context info.


 T


 Thank you, this seems to work well!

 We're using struct Fence { } Fence _ = Fence(), and it doesn't 
 add much overhead.

 Barring the proposed compiler change, this seems the cleanest 
 fix.


void func(NONE...)(string s, NONE, string file = __FILE__, size_t 
line = __LINE__) if(!NONE.length)
{
	import std.stdio;
	writefln("%s:%d: msg=%s", file, line, s);
}

void main() {func("hello"); func("there");
}
May 30 2018
parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 5/30/18 3:05 PM, Daniel N wrote:


 void func(NONE...)(string s, NONE, string file = __FILE__, size_t line = 
 __LINE__) if(!NONE.length)
 {
      import std.stdio;
      writefln("%s:%d: msg=%s", file, line, s);
 }
 
 void main() {func("hello"); func("there");
 }


Very cool and interesting pattern. If you now wanted to wrap the 
function (let's say the caller of this wants to forward it's own called 
file/line to it), you could do this as well:

func!()("boo", file, line);

I still think we should be able to wrap __FILE__ and __LINE__ into 
another call without having the wrapping take over the file/line combo.

-Steve
May 30 2018
next sibling parent Daniel N <no public.email> writes:
On Wednesday, 30 May 2018 at 19:34:55 UTC, Steven Schveighoffer 
wrote:

 On 5/30/18 3:05 PM, Daniel N wrote:


 void func(NONE...)(string s, NONE, string file = __FILE__, 
 size_t line = __LINE__) if(!NONE.length)
 {
      import std.stdio;
      writefln("%s:%d: msg=%s", file, line, s);
 }
 
 void main() {func("hello"); func("there");
 }


 Very cool and interesting pattern. If you now wanted to wrap 
 the function (let's say the caller of this wants to forward 
 it's own called file/line to it), you could do this as well:

 func!()("boo", file, line);


Heh, didn't consider that use-case, cool indeed!
How about we name the pattern "You shall not pass"/"None shall 
pass"? ;)


 I still think we should be able to wrap __FILE__ and __LINE__ 
 into another call without having the wrapping take over the 
 file/line combo.

 -Steve


Agree, adheres to the Principle of least astonishment.
May 30 2018
prev sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Wed, May 30, 2018 at 03:34:55PM -0400, Steven Schveighoffer via
Digitalmars-d wrote:

 On 5/30/18 3:05 PM, Daniel N wrote:
 

 void func(NONE...)(string s, NONE, string file = __FILE__, size_t
 line = __LINE__) if(!NONE.length)
 {
      import std.stdio;
      writefln("%s:%d: msg=%s", file, line, s);
 }
 
 void main() {func("hello"); func("there");
 }


 
 Very cool and interesting pattern.


[...]

Note that this requires a recent version of DMDFE, because earlier
versions are unable to handle additional arguments after a "..."
parameter.


T

-- 
Why ask rhetorical questions? -- JC
May 30 2018
prev sibling next sibling parent reply bauss <jj_1337 live.dk> writes:
On Wednesday, 30 May 2018 at 08:27:16 UTC, FeepingCreature wrote:

 There's a very common idiom where in order to report line 
 numbers of an error or a log line at the callsite of a 
 function, you pass __FILE__ and __LINE__ as default parameters:

 [...]


void foo(string file = __FILE__, size_t line = __LINE__)(string 
msg);

Wouldn't this solve it?
May 30 2018
parent FeepingCreature <feepingcreature gmail.com> writes:
On Wednesday, 30 May 2018 at 11:59:05 UTC, bauss wrote:

 On Wednesday, 30 May 2018 at 08:27:16 UTC, FeepingCreature 
 wrote:

 There's a very common idiom where in order to report line 
 numbers of an error or a log line at the callsite of a 
 function, you pass __FILE__ and __LINE__ as default parameters:

 [...]


 void foo(string file = __FILE__, size_t line = __LINE__)(string 
 msg);

 Wouldn't this solve it?


No, because a) then you're completely pointlessly making a foo 
for every line it's called in, b) you're not future compatible 
with, say, call column, and c) you get exactly the same problem 
with template value parameters, ie. foo!"some ct argument".
May 30 2018
prev sibling next sibling parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 5/30/18 4:27 AM, FeepingCreature wrote:

 There's a very common idiom where in order to report line numbers of an 
 error or a log line at the callsite of a function, you pass __FILE__ and 
 __LINE__ as default parameters:
 
 void foo(string file = __FILE__, size_t line = __LINE__);
 
 What's wrong with this?
 
 Say you add a string parameter, such as
 
 void foo(string msg, string file = __FILE__, size_t line = __LINE__);
 
 foo("Hello World");
 
 Now when you accidentally grab an old version of the library, your new 
 code will still run, but it will believe that it's being called from 
 file "Hello World", line 15. Not good.
 
 Luckily there's a fix. Just stick this in some common header file in 
 your project:
 
 struct CallerInfo
 {
    string file;
    size_t line;
 }
 
 void foo(string msg, CallerInfo caller = CallerInfo(__FILE__, __LINE__));
 
 Now you cannot accidentally invoke foo with a string, or in fact any 
 type except another instance of CallerInfo.


Awesome idea! Unfortunately, it doesn't work. The __FILE__ and __LINE__ 
there are not from the caller, but from the line that defines foo.

See here: https://run.dlang.io/is/siz9YZ


 At which point we can shorten this to CallerInfo caller = 
 __CALLER__, and be forward compatible for additional information about 
 the callsite, such as, say, attributes of the calling function.


Hm.. I don't like this too much. Adding more magic to the compiler seems 
unnecessary.

But if we fixed the behavior that causes your idea not to work, then we 
could probably easily define a function like so:

CallerInfo __CALLER__(string file = __FILE__, size_t line = __LINE__)
{
     return CallerInfo(file, line);
}

-Steve
May 30 2018
next sibling parent Steven Schveighoffer <schveiguy yahoo.com> writes:
On 5/30/18 10:40 AM, Steven Schveighoffer wrote:

 But if we fixed the behavior that causes your idea not to work, then we 
 could probably easily define a function like so:
 
 CallerInfo __CALLER__(string file = __FILE__, size_t line = __LINE__)
 {
      return CallerInfo(file, line);
 }


Filed an issue so it's not forgotten:

https://issues.dlang.org/show_bug.cgi?id=18919

-Steve
May 30 2018
prev sibling next sibling parent "Nick Sabalausky (Abscissa)" <SeeWebsiteToContactMe semitwist.com> writes:
On 05/30/2018 10:40 AM, Steven Schveighoffer wrote:

 
 But if we fixed the behavior that causes your idea not to work, then we 
 could probably easily define a function like so:
 
 CallerInfo __CALLER__(string file = __FILE__, size_t line = __LINE__)
 {
      return CallerInfo(file, line);
 }
 


Love it!
May 30 2018
prev sibling next sibling parent reply John Colvin <john.loughran.colvin gmail.com> writes:
On Wednesday, 30 May 2018 at 14:40:50 UTC, Steven Schveighoffer 
wrote:

 On 5/30/18 4:27 AM, FeepingCreature wrote:

 There's a very common idiom where in order to report line 
 numbers of an error or a log line at the callsite of a 
 function, you pass __FILE__ and __LINE__ as default parameters:
 
 void foo(string file = __FILE__, size_t line = __LINE__);
 
 What's wrong with this?
 
 Say you add a string parameter, such as
 
 void foo(string msg, string file = __FILE__, size_t line = 
 __LINE__);
 
 foo("Hello World");
 
 Now when you accidentally grab an old version of the library, 
 your new code will still run, but it will believe that it's 
 being called from file "Hello World", line 15. Not good.
 
 Luckily there's a fix. Just stick this in some common header 
 file in your project:
 
 struct CallerInfo
 {
    string file;
    size_t line;
 }
 
 void foo(string msg, CallerInfo caller = CallerInfo(__FILE__, 
 __LINE__));
 
 Now you cannot accidentally invoke foo with a string, or in 
 fact any type except another instance of CallerInfo.


 Awesome idea! Unfortunately, it doesn't work. The __FILE__ and 
 __LINE__ there are not from the caller, but from the line that 
 defines foo.

 See here: https://run.dlang.io/is/siz9YZ


https://run.dlang.io/is/oMe7KQ

Less elegant, but solves the problem of accidental argument 
adding (CallerFile acts as a barrier). Unfortunately, while it 
works in theory, in practice the compiler crashes.... LOL
May 30 2018
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 5/30/2018 2:45 PM, John Colvin wrote:

 https://run.dlang.io/is/oMe7KQ
 
 Less elegant, but solves the problem of accidental argument adding (CallerFile 
 acts as a barrier). Unfortunately, while it works in theory, in practice the 
 compiler crashes.... LOL


Please post bug reports when you find compiler crashes. Thanks!
May 31 2018
parent reply Mike Franklin <slavo5150 yahoo.com> writes:
On Friday, 1 June 2018 at 03:14:11 UTC, Walter Bright wrote:

 On 5/30/2018 2:45 PM, John Colvin wrote:

 https://run.dlang.io/is/oMe7KQ
 
 Less elegant, but solves the problem of accidental argument 
 adding (CallerFile acts as a barrier). Unfortunately, while it 
 works in theory, in practice the compiler crashes.... LOL


 Please post bug reports when you find compiler crashes. Thanks!


It's the same issue as this 
(https://issues.dlang.org/show_bug.cgi?id=18916) only for 
__FILE__.

I already have a pending PR at 
https://github.com/dlang/dmd/pull/8310 which I can modify to fix 
for both __LINE__ and __FILE_ if I can get clarification on how 
it *should* work.

Mike
Jun 01 2018
parent Walter Bright <newshound2 digitalmars.com> writes:
On 6/1/2018 12:30 AM, Mike Franklin wrote:

 I already have a pending PR at https://github.com/dlang/dmd/pull/8310 which I 
 can modify to fix for both __LINE__ and __FILE_ if I can get clarification on 
 how it *should* work.


I followed up there. Thanks for taking care of this!
Jun 01 2018
prev sibling parent reply bauss <jj_1337 live.dk> writes:
On Wednesday, 30 May 2018 at 14:40:50 UTC, Steven Schveighoffer 
wrote:

 On 5/30/18 4:27 AM, FeepingCreature wrote:

 There's a very common idiom where in order to report line 
 numbers of an error or a log line at the callsite of a 
 function, you pass __FILE__ and __LINE__ as default parameters:
 
 void foo(string file = __FILE__, size_t line = __LINE__);
 
 What's wrong with this?
 
 Say you add a string parameter, such as
 
 void foo(string msg, string file = __FILE__, size_t line = 
 __LINE__);
 
 foo("Hello World");
 
 Now when you accidentally grab an old version of the library, 
 your new code will still run, but it will believe that it's 
 being called from file "Hello World", line 15. Not good.
 
 Luckily there's a fix. Just stick this in some common header 
 file in your project:
 
 struct CallerInfo
 {
    string file;
    size_t line;
 }
 
 void foo(string msg, CallerInfo caller = CallerInfo(__FILE__, 
 __LINE__));
 
 Now you cannot accidentally invoke foo with a string, or in 
 fact any type except another instance of CallerInfo.


 Awesome idea! Unfortunately, it doesn't work. The __FILE__ and 
 __LINE__ there are not from the caller, but from the line that 
 defines foo.

 See here: https://run.dlang.io/is/siz9YZ


 At which point we can shorten this to CallerInfo caller = 
 __CALLER__, and be forward compatible for additional 
 information about the callsite, such as, say, attributes of 
 the calling function.


 Hm.. I don't like this too much. Adding more magic to the 
 compiler seems unnecessary.

 But if we fixed the behavior that causes your idea not to work, 
 then we could probably easily define a function like so:

 CallerInfo __CALLER__(string file = __FILE__, size_t line = 
 __LINE__)
 {
     return CallerInfo(file, line);
 }

 -Steve


Instead of these hack keywords.

Perhaps a __traits() in the compiler with the information would 
be better suited like:


void foo()
{
     enum caller = __traits(getCaller);

     ....
}

getCaller would return a compile-time struct with additional 
information about the current module and the module/function it 
was called from.

Alternatively you can use the following traits.

true/false as secondary argument for whether it should be its 
current module or the call module/function etc. This argument 
should be optional and when omitted should default to the callee.

     __FILE__ -- __traits(getFile);
     __FILE_FULL_PATH__ -- __traits(getFilePath);
     __MODULE__ -- __traits(getModule);
     __LINE__  -- __traits(getLine);
     __FUNCTION__ -- __traits(getFunction);
     __PRETTY_FUNCTION__ -- __traits(getPrettyFunction);
May 31 2018
parent Steven Schveighoffer <schveiguy yahoo.com> writes:
On 5/31/18 7:14 AM, bauss wrote:

 Instead of these hack keywords.
 
 Perhaps a __traits() in the compiler with the information would be 
 better suited like:
 
 
 void foo()
 {
      enum caller = __traits(getCaller);
 
      ....
 }
 
 getCaller would return a compile-time struct with additional information 
 about the current module and the module/function it was called from.


How can this be possible? When inside the function, you have no idea 
where you were called from.

-Steve
May 31 2018
prev sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 5/30/2018 1:27 AM, FeepingCreature wrote:

 There's a very common idiom where in order to report line numbers of an error
or 
 a log line at the callsite of a function, you pass __FILE__ and __LINE__ as 
 default parameters:
 
 void foo(string file = __FILE__, size_t line = __LINE__);
 
 What's wrong with this?
 
 Say you add a string parameter, such as
 
 void foo(string msg, string file = __FILE__, size_t line = __LINE__);
 
 foo("Hello World");
 
 Now when you accidentally grab an old version of the library, your new code
will 
 still run, but it will believe that it's being called from file "Hello World", 
 line 15.


A solution:

   enum E { reserved }

   void foo(E e = E.reserved, string file = __FILE__, size_t line = __LINE__);
   void foo(string msg, E e = E.reserved, string file = __FILE__, size_t line = 
__LINE__);
May 31 2018