Thread overview
[Issue 10258] New: Add hasAccess trait
Jun 03, 2013
Andrej Mitrovic
Jun 03, 2013
Andrej Mitrovic
Jun 03, 2013
Jonathan M Davis
Jun 03, 2013
Andrej Mitrovic
Jul 19, 2013
Kenji Hara
Jul 19, 2013
Jonathan M Davis
Jul 19, 2013
Andrej Mitrovic
June 03, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=10258

           Summary: Add hasAccess trait
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: andrej.mitrovich@gmail.com


--- Comment #0 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-06-03 15:53:20 PDT ---
One of the most common implementation problems in my template code is that I rarely properly check for access issues. Take the following for example:

b.d:
-----
module b;
import std.conv;
import std.stdio;

string[] getValues(T)(T var)
{
    string[] values;

    foreach (member; __traits(allMembers, T))
        values ~= to!string(__traits(getMember, var, member));

    return values;
}

struct S
{
    int x;
    private int y;
}

void main()
{
    S s;
    writeln(getValues(s));
}
-----

All is well and good. But now try moving the S struct and main function into another module:

-----
module a;
import b;

import std.stdio;

struct S
{
    int x;
    private int y;
}

void main()
{
    S s;
    writeln(getValues(s));
}
-----

And now we get: Error: struct test.S member y is not accessible

This is a commonly overlooked problem because unittests for templates are usually written right alongside them, and "private" extends to the entire module.

We currently have a "getProtection" attribute, but it's awkward to use it because if a symbol has a package or protected attribute we still won't know whether we actually have access to it *from the call point*.

Using is(typeof()) to check accessibility is a hacky workaround, so instead I
propose that we implement a new trait, called hasAccess. It would be used like
so:

-----
string[] getValues(T)(T var)
{
    string[] values;

    foreach (member; __traits(allMembers, T))
    {
        // new
        static if (__traits(hasAccess, __traits(getMember, var, member))
        {
            values ~= to!string(__traits(getMember, var, member));
        }
    }

    return values;
}
-----

The compiler would then check if the current scope has access to a symbol and return true or false.

The true benefit of the 'hasAccess' trait is that it will automatically work for templates, mixin templates, and string mixin code.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
June 03, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=10258


Andrej Mitrovic <andrej.mitrovich@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrei@erdani.com


--- Comment #1 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-06-03 15:57:37 PDT ---
@andralex: CC'ing you to get an idea if you like this enhancement proposal
before I try implementing it (I'm eager to start hacking on it :) ).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
June 03, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=10258


Jonathan M Davis <jmdavisProg@gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg@gmx.com


--- Comment #2 from Jonathan M Davis <jmdavisProg@gmx.com> 2013-06-03 16:09:56 PDT ---
> Using is(typeof()) to check accessibility is a hacky workaround

Actually, according to Don (from what I recall in a discussion on unusable init
values), I don't think that that necessarily even works. You may be forced to
use __traits(compiles, foo) instead.

Regardless of that though, I'm not sure that it's really a hacky workaround. What if accessibility isn't the only problem? It seems to me that specifically checking for whether you have access is like checking file permissions before trying to read a file. Even if you have permissions, there are a host of other reasons that reading the file could fail, so checking for permissions explicitly doesn't generally buy you much - if anything. You only care why you can't read the file when you're trying to react differently for different error conditions, and I doubt that you generally care that you can't use a symbol due to access level as opposed to something else, so I would expect that checking for whether something compiles would be enough and might even save you having additional checks.

That doesn't mean that we shouldn't add something like this, but I question its general utility.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
June 03, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=10258



--- Comment #3 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-06-03 16:12:28 PDT ---
(In reply to comment #2)
> What if accessibility isn't the only problem?

That's exactly why we need this. is(typeof()) silences the problem altogether,
meaning if your template fails for a reason *other* than accessibility problem,
you won't know about it because is(typeof()) has silenced the error for you.

If on the other hand you check access, and you have access but then your template still fails, you will *know* why it failed since the compiler will tell you what went wrong, and then you can fix your template.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
June 10, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=10258



--- Comment #4 from Andrei Alexandrescu <andrei@erdani.com> 2013-06-09 17:38:07 PDT ---
I'd say we should define a trait in std.traits, implement it with what we have, and defer the decision on a built-in trait. If needed it'll come up.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
July 19, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=10258



--- Comment #5 from Kenji Hara <k.hara.pg@gmail.com> 2013-07-19 11:11:13 PDT ---
I think hasAccess traits would be completely redundant feature.

We can use __traits(compiles) + __traits(getMember).

module a;
struct S { int x;  private int y; }

module b;
import a;
void getValues(T)(T t)
{
    import std.stdio;
    foreach (name; __traits(allMembers, T))
    {
        static if (__traits(compiles, __traits(getMember, t, name)))
            writeln(name, ": ", mixin("t."~name));
    }
}
void main()
{
    S s;
    getValues(s);
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
July 19, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=10258



--- Comment #6 from Jonathan M Davis <jmdavisProg@gmx.com> 2013-07-19 11:38:58 PDT ---
If what you really need to check is whether code has access to a particular symbol (as opposed to whether using it works), then it would be much more idiomatic if there were a trait for it rather than combining __traits(compiles, ...) with __traits(getMember, ...). So, from the standpoint of code clarity, a trait would be preferable. However, in most cases, I would think that it would be better to check whether using a symbol works rather than checking whether it's accessible, since that's generally what you care about. But if we have a strong enough use case for checking explicitly for access, then I think that it would be better to have trait for it because of what it gives you in code clarity.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
July 19, 2013
http://d.puremagic.com/issues/show_bug.cgi?id=10258



--- Comment #7 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-07-19 14:08:55 PDT ---
(In reply to comment #5)
> I think hasAccess traits would be completely redundant feature.

This isn't a good workaround for two reasons:

1. It is not clear enough what is being done (same as JMD's argument)
2. __traits(compiles) silences all compiler errors.

#2 is the most important reason. Here's an example:

-----
struct S
{
    int a;
}

void main()
{
    static if (__traits(compiles, __traits(getMember, T, "a")))
    {
        pragma(msg, "Can access a!");
    }
}
-----

This will not print anything, because the aggregate type is wrong (it doesn't
even exist!). It should have been 'S', not 'T', but __traits(compiles) has
gagged the error message.

Tracking down bugs like these are hard when there are no error messages. If instead we had a trait, the above would become:

-----
static if (__traits(hasAccess, __traits(getMember, T, "b")))
-----

And the error message is clear:

> Error: undefined identifier T, did you mean struct S?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------