errors with getSymbolsByUDA to get a private members #5344

sysint64 · 2017-04-22T17:06:03Z

Resolve #15335 Remove ability in getSymbolsByUDA to get a private members If it is located in another module.
If you trying to get symbols from another module you will get errors. This commit resolve this. Now you can access to all attributes if you have access otherwise you will not get private members but also not throws errors.
examples:
uda.d

enum Attr;

struct HasPrivateMembers
{
    @Attr int a;
    int b;
    @Attr private int c;
    private int d;
}

main.d

unittest
{
    import uda : Attr, HasPrivateMembers;
    // Trying access to private member from another file therefore we do not have access
    // for this otherwise we get deprecation warning - not visible from module
    static assert(getSymbolsByUDA!(HasPrivateMembers, Attr).length == 1);
    static assert(hasUDA!(getSymbolsByUDA!(HasPrivateMembers, Attr)[0], Attr));

    struct A
    {
        @Attr int a;
        int b;
        @Attr private int c;
        private int d;
    }

    // Here everything is fine, we have access to private member c
    static assert(getSymbolsByUDA!(A, Attr).length == 2);
    static assert(hasUDA!(getSymbolsByUDA!(A, Attr)[0], Attr));
    static assert(hasUDA!(getSymbolsByUDA!(A, Attr)[1], Attr));
}

In my program I do not get deprecation warning instead of this I get a bunch of errors.
With this patch all works fine.

/usr/include/dlang/dmd/std/traits.d-mixin-7671(7671,1): Deprecation: editor.mapeditor.MyView.a is not visible from module traits
/usr/include/dlang/dmd/std/traits.d-mixin-7671(7671,1): Deprecation: ui.views.view.View.layoutData is not visible from class MyView
/usr/include/dlang/dmd/std/traits.d-mixin-7671(7671,1): Deprecation: ui.views.view.View.layoutData is not visible from module std.traits
/usr/include/dlang/dmd/std/traits.d-mixin-7671(7671,1): Error: class editor.mapeditor.MyView member layoutData is not accessible
/usr/include/dlang/dmd/std/meta.d(900,20): Error: template instance std.traits.getSymbolsByUDA!(MyView, OnClickListener).pred!"layoutData" error instantiating
/usr/include/dlang/dmd/std/traits.d(7672,39):        5 recursive instantiations from here: Filter!(hasSpecificUDA, "__ctor", "onOkButtonClick", "onCancelButtonClick", "a", "app", "createFromFile", "layoutData", "toString", "toHash", "opCmp", "opEquals", "Monitor", "factory")
src/ui/views/view.d(58,26):        instantiated from here: getSymbolsByUDA!(MyView, OnClickListener)
src/editor/mapeditor.d(34,14):        instantiated from here: __ctor!(MyView)
dmd failed with exit code 1.

UplinkCoder · 2017-04-22T17:08:07Z

std/traits.d

@@ -7664,9 +7664,13 @@ template getSymbolsByUDA(alias symbol, alias attribute) {
                  .format(names[0]));
    }

+    // filtering not compiled members such as not accessible members
+    enum noNotCompiledMembers(string name) = (__traits(compiles, __traits(getMember, symbol, name)));


this name is wired.

yes sorry I'm not very good with English, how about excludeInaccessibleMembers or noInaccessibleMembers?

sysint64 · 2017-04-23T07:51:42Z

found another problem - if put alias int INT or alias void function(INT) SomeFunction; application will crush. Fixed this.

ghost · 2017-04-24T02:34:26Z

You must use getSymbolsByUDA!(__trait(getMember, stuff), Attr) to access private members. __trait(getMember, stuff) is not constrained by the protection of stuff.

sysint64 · 2017-04-24T02:39:57Z

Totally agree with you, If I want to add some useful attribute like property I will get a lot of errors, because of the inability to access the symbols.

ghost · 2017-04-24T02:43:43Z

I've deleted previous comments so there might be a misunderstanding. Actually the bug you try to fix is not a bug anymore. You must use __traits(getMember) as symbol and not the symbol directly. This has for effect to bypass the protection.

sysint64 · 2017-04-24T04:24:06Z

I do not quite understand why this is not a bug - errors fall specifically in the getSymbolsByUDA even if there are no private attributes in the structure but have private members without attributes e.g.:

struct {
    @Field int testField = 10;
    private int member = 0;
}

When I try to call getSymbolsByUDA from another module - my application is down.

sysint64 · 2017-05-23T09:02:45Z

ping @bbasile

ghost · 2017-05-23T10:07:16Z

My wish to make most of the built-in traits free from protection attribute interferes. Consider my previous comments as non-existent.

sysint64 · 2017-05-26T03:51:42Z

Ok, @UplinkCoder all corrected, will there be more comments?

UplinkCoder · 2017-05-26T19:55:26Z

std/traits.d

@@ -7664,12 +7664,20 @@ template getSymbolsByUDA(alias symbol, alias attribute) {
                  .format(names[0]));
    }

+    // filtering inaccessible members
+    enum noInaccessibleMembers(string name) = (__traits(compiles, __traits(getMember, symbol, name)));


double negation is wired.
isAccessible
also the () around the __traits-Expression are not needed.

UplinkCoder · 2017-05-26T19:56:59Z

std/traits.d

    // filtering out nested class context
    enum noThisMember(string name) = (name != "this");
-    alias membersWithoutNestedCC = Filter!(noThisMember, __traits(allMembers, symbol));
+    alias membersWithoutNestedCC = Filter!(noThisMember, withoutInaccessibleMembers);


membersWithoutNestedCC ?
What does this stand for ?

@UplinkCoder this one was here before, but now it's redundant, fixed it.

sysint64

@UplinkCoder fixed

sysint64 · 2017-05-27T07:28:14Z

std/traits.d

    // filtering out nested class context
    enum noThisMember(string name) = (name != "this");
-    alias membersWithoutNestedCC = Filter!(noThisMember, __traits(allMembers, symbol));
+    alias membersWithoutNestedCC = Filter!(noThisMember, withoutInaccessibleMembers);


@UplinkCoder this one was here before, but now it's redundant, fixed it.

UplinkCoder · 2017-05-27T09:18:19Z

std/traits.d

    static assert(hasUDA!(getSymbolsByUDA!(HasPrivateMembers, Attr)[0], Attr));
-    static assert(hasUDA!(getSymbolsByUDA!(HasPrivateMembers, Attr)[1], Attr));
+
+    struct A


this could go in a unittest-block,

UplinkCoder

LGTM

UplinkCoder · 2017-05-27T09:20:39Z

std/traits.d

-    alias membersWithoutNestedCC = Filter!(noThisMember, __traits(allMembers, symbol));
+    // filtering inaccessible members
+    enum isAccessibleMember(string name) = __traits(compiles, __traits(getMember, symbol, name));
+    alias withoutInaccessibleMembers = Filter!(isAccessibleMember, __traits(allMembers, symbol));


again please no double negation.
accessibleMembers

UplinkCoder · 2017-05-27T09:21:29Z

std/traits.d

+    enum hasSpecificUDA(string name) = mixin("hasUDA!(symbol." ~ name ~ ", attribute)");
+    enum isCorrectMember(string name) = __traits(compiles, hasSpecificUDA!(name));
+
+    alias withoutIncorrectMembers = Filter!(isCorrectMember, withoutInaccessibleMembers);


correctMembers

UplinkCoder

please address the comments

sysint64 · 2017-05-27T11:05:40Z

@UplinkCoder corrected, but for some reason continuous-integration/jenkins/pr-merge and ci/circleci have not passed.

UplinkCoder · 2017-05-27T11:08:21Z

@sysint64 rebase on ~master.

another module

sysint64 · 2017-05-27T12:11:26Z

@UplinkCoder it hasn't helped, DAutoTest throw an error: /home/dtest/DAutoTest/work/repo/dlang.org/dlangspec.html: No such file or directory.
Don't actually understand, how these changes affect to it.

UplinkCoder · 2017-05-27T12:15:17Z

@sysint64 squash please.
This is ready for merge.

sysint64 · 2017-05-27T17:15:29Z

@UplinkCoder sorry for the stupid question: what does the "squash please" mean?

UplinkCoder · 2017-05-27T17:22:05Z

Squashing refers to merging the individual commits into one.
https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History

wilzbach · 2017-05-27T17:23:49Z

@UplinkCoder sorry for the stupid question: what does the "squash please" mean?

That instead of merging 6 commits, we prefer to merge only one to keep the history nice and clean.
However, @UplinkCoder there's absolutely no need to request this from submitters (and thus complicate their flow) since a couple of months as:

maintainers can do manual squashes easily
the dlang-bot does support auto-merge-squash (!)

@UplinkCoder: Do you know this page: https://wiki.dlang.org/Guidelines_for_maintainers

UplinkCoder · 2017-05-27T17:46:25Z

@sysint64 there seems to be a problem when building ggplot

sysint64 · 2017-05-27T19:02:14Z

@UplinkCoder how can I reproduce it? jenkins/pr-merge log does not say anything except that this is linking error (collect2: fatal error: ld terminated with signal 7 [Bus error]).

wilzbach · 2017-05-27T19:05:41Z

how can I reproduce it?

Clone ggplotd and pass your locally build DMD to DUB with --compiler when running the tests (dub test)

sysint64 · 2017-05-27T19:55:10Z

@UplinkCoder, @wilzbach I checked ggplotd and everything was compiled fine and tests passed.

command:
dub build --force --compiler=/home/andrey/projects/dmd/generated/linux/release/64/dmd
dub test --force --compiler=/home/andrey/projects/dmd/generated/linux/release/64/dmd

dmd.conf:

[Environment64]
DFLAGS=-I/home/andrey/projects/phobos -I/home/andrey/projects/druntime/import -L-L/home/andrey/projects/phobos/generated/linux/release/64 -L-L/home/andrey/projects/druntime/generated/linux/release/64 -L--export-dynamic -fPIC

sysint64 · 2017-05-28T06:16:00Z

Jenkins falls when he tries to compile with flag -c ggplotd-gtk, I have tried to compile with this flag, and on my computer everything is compiled normally. I suppose problem with linking gtk-d:gtkd.

sysint64 · 2017-05-29T04:48:44Z

@UplinkCoder, @wilzbach I checked ggplot on another computer where only one dmd - v2.075.0-devel-e5ebfc5 and one phobos with my changes, and everything builds correctrly and all tests pass.
Maybe the problem is not in the changes but in jenkins configuration?

wilzbach · 2017-05-29T05:18:45Z

Maybe the problem is not in the changes but in jenkins configuration?

Quite likely, unfortunately I don't have access to restart jobs at Jenkins. I will try to restart manually by force-pushing the same changeset.

sysint64 · 2017-06-01T03:23:27Z

@wilzbach, @UplinkCoder all checks have been passing, but for some reason doesn't run the jenkins/pr-merge

CyberShadow · 2017-07-15T05:54:15Z

This pull request may have introduced a regression:
https://issues.dlang.org/show_bug.cgi?id=17643

Your PR description says:

Resolve #15335 Remove ability in getSymbolsByUDA to get a private members If it is located in another module.

However, as per the above issue, it also affects symbols within the same module.

sysint64 · 2017-07-15T09:18:18Z

@CyberShadow

However, as per the above issue, it also affects symbols within the same module.

there is test on this case which successfully passed:

@safe unittest
{
    enum Attr;
    struct A
    {
        alias int INT;
        alias void function(INT) SomeFunction;
        @Attr int a;
        int b;
        @Attr private int c;
        private int d;
    }

    // Here everything is fine, we have access to private member c
    static assert(getSymbolsByUDA!(A, Attr).length == 2);
    static assert(hasUDA!(getSymbolsByUDA!(A, Attr)[0], Attr));
    static assert(hasUDA!(getSymbolsByUDA!(A, Attr)[1], Attr));
}

You can't access to symbol only if this accessing leads to compilation error

CyberShadow · 2017-07-15T09:21:11Z

@sysint64 I think that test passes only because it's located inside the std.traits module.

Suffice to say, it's not useful that we can access private members of types declared in std.traits specifically.

sysint64 · 2017-07-15T09:44:05Z

@CyberShadow you are right it's not compiled, I suppose it's because all computations are produced inside the function in std.traits, old version of this function also in the future will stop working due to deprecation such access.

sysint64 · 2017-07-15T10:05:52Z

@CyberShadow on current version of D not only does this method not work with private members also do not work getUDAs and hasUDA.

UplinkCoder reviewed Apr 22, 2017

View reviewed changes

UplinkCoder reviewed May 26, 2017

View reviewed changes

sysint64 commented May 27, 2017

View reviewed changes

UplinkCoder reviewed May 27, 2017

View reviewed changes

UplinkCoder approved these changes May 27, 2017

View reviewed changes

UplinkCoder reviewed May 27, 2017

View reviewed changes

sysint64 added 5 commits May 27, 2017 18:34

Remove the ability for getSymbolsByUDA to get a private method from

51a3555

another module

rename weird name

e70e1c1

avoid error if struct has alias of basic or function types

d96c817

reduction of code repetition

5ec4312

rm redundant code, avoid double negation in names

c445477

sysint64 force-pushed the avoid-error-in-getSymbolsByUDA branch from 0a25eb4 to c233122 Compare May 27, 2017 11:38

UplinkCoder added Merge:auto-merge and removed Merge:auto-merge labels May 27, 2017

wilzbach closed this May 29, 2017

wilzbach reopened this May 29, 2017

avoid double negation, add unittest block

cf668ab

wilzbach force-pushed the avoid-error-in-getSymbolsByUDA branch from c233122 to cf668ab Compare May 29, 2017 05:23

UplinkCoder merged commit 23c4f0f into dlang:master Jun 1, 2017

sysint64 deleted the avoid-error-in-getSymbolsByUDA branch June 1, 2017 11:45

errors with getSymbolsByUDA to get a private members #5344

errors with getSymbolsByUDA to get a private members #5344

Conversation

sysint64 commented Apr 22, 2017 • edited Loading

Choose a reason for hiding this comment

sysint64 Apr 22, 2017 • edited Loading

Choose a reason for hiding this comment

sysint64 commented Apr 23, 2017

ghost commented Apr 24, 2017

sysint64 commented Apr 24, 2017

ghost commented Apr 24, 2017

sysint64 commented Apr 24, 2017

sysint64 commented May 23, 2017

ghost commented May 23, 2017 • edited by ghost Loading

sysint64 commented May 26, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sysint64 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UplinkCoder left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

UplinkCoder left a comment

Choose a reason for hiding this comment

sysint64 commented May 27, 2017 • edited Loading

UplinkCoder commented May 27, 2017

sysint64 commented May 27, 2017 • edited Loading

UplinkCoder commented May 27, 2017

sysint64 commented May 27, 2017

UplinkCoder commented May 27, 2017

wilzbach commented May 27, 2017

UplinkCoder commented May 27, 2017

sysint64 commented May 27, 2017

wilzbach commented May 27, 2017

sysint64 commented May 27, 2017

sysint64 commented May 28, 2017

sysint64 commented May 29, 2017

wilzbach commented May 29, 2017

sysint64 commented Jun 1, 2017

CyberShadow commented Jul 15, 2017

sysint64 commented Jul 15, 2017 • edited Loading

CyberShadow commented Jul 15, 2017

sysint64 commented Jul 15, 2017

sysint64 commented Jul 15, 2017

sysint64 commented Apr 22, 2017 •

edited

Loading

sysint64 Apr 22, 2017 •

edited

Loading

ghost commented May 23, 2017 •

edited by ghost

Loading

sysint64 left a comment •

edited

Loading

sysint64 commented May 27, 2017 •

edited

Loading

sysint64 commented May 27, 2017 •

edited

Loading

sysint64 commented Jul 15, 2017 •

edited

Loading