Skip to content

Enable getSymbolsByUDA to retrieve private members. #3827

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.

Sign up for GitHub

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

Merged
Hackerpilot merged 1 commit into from
Nov 29, 2015
Merged

Enable getSymbolsByUDA to retrieve private members. #3827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions std/internal/test/uda.d
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
For testing only.
Provides a struct with UDA's defined in an external module.
Useful for validating behavior with member privacy.
*/
module std.internal.test.uda;

enum Attr;

struct HasPrivateMembers
{
@Attr int a;
int b;
@Attr private int c;
private int d;
}
39 changes: 32 additions & 7 deletions std/traits.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6727,14 +6727,29 @@ unittest
* This is not recursive; it will not search for symbols within symbols such as
* nested structs or unions.
*/
template getSymbolsByUDA(alias symbol, alias attribute)
{
import std.typetuple : Filter, staticMap, TypeTuple;
template getSymbolsByUDA(alias symbol, alias attribute) {
import std.string : format;
import std.meta : AliasSeq, Filter;

// translate a list of strings into symbols. mixing in the entire alias
// avoids trying to access the symbol, which could cause a privacy violation
template toSymbols(names...) {
static if (names.length == 1)
mixin("alias toSymbols = AliasSeq!(symbol.%s);".format(names[0]));
else
mixin("alias toSymbols = AliasSeq!(symbol.%s, toSymbols!(names[1..$]));"
.format(names[0]));
Copy link
Member

@MartinNowak MartinNowak Aug 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Yikes, this does not work. Not all type names are correctly printable and you always use an alias instead of a mixin string to get them.
Could we somehow replace this with __traits(getMember, Type, name)?
Or maybe we have to change the API to return a bunch of strings like __traits(allMembers, Type) and leave it to the people to access the fields.

Copy link
Contributor Author

@rcorre rcorre Aug 10, 2016

Choose a reason for hiding this comment

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

Could we somehow replace this with __traits(getMember, Type, name)

Only if we change the API to only retrieve publically accessible members (even if those members are visible in the caller's module)

Or maybe we have to change the API to return a bunch of strings like __traits(allMembers, Type)

I don't think this matters as we'd still be using a mixin for hasSpecificUDA. Get rid of that mixin, and we can only support members visible to std.traits.

I'm not completely sure I understand the problem here, can you elaborate or give an example?

Copy link
Contributor Author

@rcorre rcorre Aug 10, 2016

Choose a reason for hiding this comment

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

Oh, there are deprecation warnings even with the code as-is. I think we'll have to give up on exposing members that std.traits can't access. Which could really limit the usefulness of this. Users will have to implement their own getSymbolsByUDA in every module they want to use it (if they want it to return non-public members with the UDA).

Copy link
Member

@MartinNowak MartinNowak Aug 12, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Only if we change the API to only retrieve publically accessible members (even if those members are visible in the caller's module)

I think __traits(getMember) should bypass visibility checks, b/c it's a meta programming tool and similar use cases are needed in many places.

Copy link
Member

@MartinNowak MartinNowak Aug 12, 2016

Choose a reason for hiding this comment

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

Maybe I mixed that up with using formatted strings to rebuild the type, which doesn't work in many places.

auto test()
{
    struct S
    {
    }
    return S();
}

pragma(msg, typeof(test()).stringof);

But here you're just accessing the members of sth. which are always named.

Copy link
Member

@MartinNowak MartinNowak Aug 12, 2016

Choose a reason for hiding this comment

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

A mixin template might be viable alternative as well.

Copy link
Member

@schveiguy schveiguy Aug 12, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I think __traits(getMember) should bypass visibility checks, b/c it's a meta programming tool and similar use cases are needed in many places.

I'm not sure this is the best idea. __traits(getMember, x, "foo") is as easy to type as x.foo and can have disastrous consequences (if you mess with private data, you can screw up the expected invariants for a type).

I think the best mechanism is to allow access to private symbols if you have the alias already. That is, I can alias my private symbol, hand it off to some other module, and because it now has that key, it can unlock the data. This allows the module to retain control over private data.

Copy link
Contributor Author

@rcorre rcorre Aug 12, 2016

Choose a reason for hiding this comment

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

think the best mechanism is to allow access to private symbols if you have the alias already

I'd like this as well, but it won't be useful in this particular case. getSymbolsByUDA only gets the 'parent' symbol, so we still wouldn't be able to access private child symbols.

A mixin template might be viable alternative as well.

I think that would work, but it is a bit uglier on the calling end.

}

enum hasSpecificUDA(string name) = mixin("hasUDA!(symbol.%s, attribute)".format(name));

alias membersWithUDA = toSymbols!(Filter!(hasSpecificUDA, __traits(allMembers, symbol)));

static enum hasSpecificUDA(alias S) = hasUDA!(S, attribute);
alias StringToSymbol(alias Name) = Identity!(__traits(getMember, symbol, Name));
alias getSymbolsByUDA = Filter!(hasSpecificUDA, TypeTuple!(symbol,
staticMap!(StringToSymbol, __traits(allMembers, symbol))));
// if the symbol itself has the UDA, tack it on to the front of the list
static if (hasUDA!(symbol, attribute))
alias getSymbolsByUDA = AliasSeq!(symbol, membersWithUDA);
else
alias getSymbolsByUDA = membersWithUDA;
}

///
Expand Down Expand Up @@ -6794,6 +6809,16 @@ unittest
static assert(getSymbolsByUDA!(C, UDA)[1].stringof == "d");
}

// #15335: getSymbolsByUDA fails if type has private members
unittest
{
// HasPrivateMembers has, well, private members, one of which has a UDA.
import std.internal.test.uda;
static assert(getSymbolsByUDA!(HasPrivateMembers, Attr).length == 2);
static assert(hasUDA!(getSymbolsByUDA!(HasPrivateMembers, Attr)[0], Attr));
static assert(hasUDA!(getSymbolsByUDA!(HasPrivateMembers, Attr)[1], Attr));
}

/**
Returns: $(D true) iff all types $(D T) are the same.
*/
Expand Down