Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Protection trait #856

Merged
merged 2 commits into from

10 participants

src/traits.c
@@ -206,6 +206,36 @@ static int fptraits(void *param, FuncDeclaration *f)
StringExp *se = new StringExp(loc, s->ident->toChars());
return se->semantic(sc);
}
+ else if (ident == Id::getProtection)
+ {
+
+ if (dim != 1)
+ goto Ldimerror;
+ PROT protection = PROTundefined;
+ Object *o = args->tdata()[0];
@yebblies Collaborator
yebblies added a note

Walter will get mad if you use tdata!

(Finally looking at this again!) What should it be? Most the other code uses this same member. (I started this by copying the " else if (ident == Id::identifier)" branch)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/traits.c
@@ -206,6 +206,36 @@ static int fptraits(void *param, FuncDeclaration *f)
StringExp *se = new StringExp(loc, s->ident->toChars());
return se->semantic(sc);
}
+ else if (ident == Id::getProtection)
+ {
+
+ if (dim != 1)
+ goto Ldimerror;
+ PROT protection = PROTundefined;
+ Object *o = args->tdata()[0];
+ DotVarExp* d = dynamic_cast<DotVarExp*>(o);
@yebblies Collaborator
yebblies added a note

Don't use rtti, use the methods in Object (something like isExpression/isType) to get an Expression, then check e->op == TOKdotid before doing a c-style cast.

@yebblies Collaborator
yebblies added a note

Also, things other than DotVarExps should work with getProtection. VarExp, SymbolExp, etc.

Object doesn't have an isExpression method. Is there another way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/traits.c
((13 lines not shown))
+ {
+ Declaration *s = d->var->isDeclaration();
+ if (s)
+ {
+ protection = s->prot();
+ }
+ }
+ const char* protName =
+ protection == PROTundefined ? ""
+ : protection == PROTnone ? "none"
+ : protection == PROTprivate ? "private"
+ : protection == PROTpackage ? "package"
+ : protection == PROTprotected ? "protected"
+ : protection == PROTpublic ? "public"
+ : protection == PROTexport ? "export"
+ : (assert(0), ""); // unknown
@yebblies Collaborator
yebblies added a note

This should probably be a switch... and should possibly assert on 'none' as well as unknown, I think these should be replaced by the time they reach here?

@MartinNowak Collaborator

Protection to chars is already duplicated in JSON and HdrGen,
you might want to merge them, see MartinNowak@f3513e6.

@9rnsr Collaborator
9rnsr added a note

@adamdruppe , at least stop using hard tab.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/traits.c
@@ -206,6 +206,36 @@ static int fptraits(void *param, FuncDeclaration *f)
StringExp *se = new StringExp(loc, s->ident->toChars());
return se->semantic(sc);
}
+ else if (ident == Id::getProtection)
+ {
+
+ if (dim != 1)
+ goto Ldimerror;
@MartinNowak Collaborator

How about returning a tuple, so this can be used with multiple symbols?

I don't think that's needed because you can always just call it multiple times. I don't think any the other traits work that way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@MartinNowak
Collaborator

ping

@adamdruppe
@andralex
Owner

This is a useful trait. Ping @adamdruppe.

@adamdruppe

I went through and added some comments a while ago about details... I don't know how to solve all the comments' concerns myself.

@adamdruppe

I just pushed up some new code. I think this covers all the bases - both kinds of dot expressions and, of course, symbols. I also updated the style to match dmd git.

There's still the duplication of the protection names though. I just wanted to do one thing at a time though, we could DRY that in a separate little pull.

@AndrejMitrovic
Collaborator

Considering we have the ParameterStorageClass/FunctionAttribute enums I think we should also introduce a getProtection template in std.traits which would return an enum. Internally it could do string comparisons via __traits(getProtection).

@adamdruppe

Yeah, that'd be easy enough, though we'll sadly have to find names other that "public" etc. thanks to keywords. Ugh. But the phobos change can happen after the compiler is updated.

@AndrejMitrovic
Collaborator

Yup. I typically use uppercase names to avoid clashes. ParameterStorageClass uses underscores, maybe we should use that for consistency.

@AndrejMitrovic
Collaborator

Btw this pull will fix an undiscovered bug in std.typecons:

module inter;

interface Inter
{
    protected void test();
}
module test;

import inter;
import std.typecons;

void main()
{
    auto o = new BlackHole!Inter;
    o.test();  // works, o.test was declared public
}

BlackHole will be able to set the right protection attribute and disallow the above from compiling. I'd imagine this pull will be welcome for these sorts of templates that deal with classes.

@andralex
Owner

Great. Could you please file that so we keep track of it? Thanks!

@adamdruppe

Hmm, the autotester is saying a test fails, but it works on my box, and I don't think my changes here could be responsible anyway.

http://d.puremagic.com/test-results/pull-history.ghtml?repoid=1&pullid=856

It looks like it was passing until last night though, and I haven't changed anything.... is this something I should worry about?

@braddr
Owner

The master is broken, so it's not shocking that all pulls on top of that are also broken. It doesn't happen often, but when it does, it's annoying. Glare at @WalterBright for checking in broken code.

src/traits.c
((8 lines not shown))
+ Object *o = (*args)[0];
+ Dsymbol *s = getDsymbol(o);
+ if(!s)
+ {
+ // it might also be a trait getMember or something,
+ // which returns a dot expression rather than a symbol
+ if(o->dyncast() == DYNCAST_EXPRESSION)
+ {
+ Expression* e = (Expression*) o;
+
+ if (e->op == TOKdotvar)
+ {
+ DotVarExp *dv = (DotVarExp *)e;
+ s = dv->var->isDeclaration();
+ }
+ else if (e->op == TOKvar)
@9rnsr Collaborator
9rnsr added a note

This is redundant code, because getDsymbol catches TOKvar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
test/compilable/traitprot.d
((1 lines not shown))
+class Test {
+ public int a;
+ private int b;
+ export string c;
+ protected int d;
+ package void e() {}
+}
+
+void main() {
+ Test t;
+ static assert(__traits(getProtection, __traits(getMember, t, "a")) == "public");
+ static assert(__traits(getProtection, __traits(getMember, t, "b")) == "private");
+ static assert(__traits(getProtection, __traits(getMember, t, "c")) == "export");
+ static assert(__traits(getProtection, __traits(getMember, t, "d")) == "protected");
+ static assert(__traits(getProtection, __traits(getMember, t, "e")) == "package");
+}
@9rnsr Collaborator
9rnsr added a note

You should move this test to test/runnable/traits.d, it's a module for the __traits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
test/runnable/traits.d
@@ -764,6 +764,31 @@ void test7858()
static assert(__traits(isSame, C.sfunc, __traits(getOverloads, C, "sfunc")[0])); // NG
}
+void getProtection() {
@9rnsr Collaborator
9rnsr added a note

Please insert separator comment between each tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
test/runnable/traits.d
((13 lines not shown))
+
+ Test t;
+ // should work both directly and through getMember
+ static assert(__traits(getProtection, t.a) == "public");
+ static assert(__traits(getProtection, t.b) == "private");
+ static assert(__traits(getProtection, t.c) == "export");
+ static assert(__traits(getProtection, t.d) == "protected");
+ static assert(__traits(getProtection, t.e) == "package");
+
+ static assert(__traits(getProtection, __traits(getMember, t, "a")) == "public");
+ static assert(__traits(getProtection, __traits(getMember, t, "b")) == "private");
+ static assert(__traits(getProtection, __traits(getMember, t, "c")) == "export");
+ static assert(__traits(getProtection, __traits(getMember, t, "d")) == "protected");
+ static assert(__traits(getProtection, __traits(getMember, t, "e")) == "package");
+}
+
@9rnsr Collaborator
9rnsr added a note

More exhaustive test I think.

private   struct TestProt1 {}
package   struct TestProt2 {}
protected struct TestProt3 {}
public    struct TestProt4 {}
export    struct TestProt5 {}

void getProtection()
{
    class Test
    {
        private   { int va; void fa(){} }
        package   { int vb; void fb(){} }
        protected { int vc; void fc(){} }
        public    { int vd; void fd(){} }
        export    { int ve; void fe(){} }
    }
    Test t;

    // TOKvar and VarDeclaration
    static assert(__traits(getProtection, Test.va) == "private");
    static assert(__traits(getProtection, Test.vb) == "package");
    static assert(__traits(getProtection, Test.vc) == "protected");
    static assert(__traits(getProtection, Test.vd) == "public");
    static assert(__traits(getProtection, Test.ve) == "export");

    // TOKdotvar and VarDeclaration
    static assert(__traits(getProtection, t.va) == "private");
    static assert(__traits(getProtection, t.vb) == "package");
    static assert(__traits(getProtection, t.vc) == "protected");
    static assert(__traits(getProtection, t.vd) == "public");
    static assert(__traits(getProtection, t.ve) == "export");

    // TOKvar and FuncDeclaration
    static assert(__traits(getProtection, Test.fa) == "private");
    static assert(__traits(getProtection, Test.fb) == "package");
    static assert(__traits(getProtection, Test.fc) == "protected");
    static assert(__traits(getProtection, Test.fd) == "public");
    static assert(__traits(getProtection, Test.fe) == "export");

    // TOKdotvar and FuncDeclaration
    static assert(__traits(getProtection, t.fa) == "private");
    static assert(__traits(getProtection, t.fb) == "package");
    static assert(__traits(getProtection, t.fc) == "protected");
    static assert(__traits(getProtection, t.fd) == "public");
    static assert(__traits(getProtection, t.fe) == "export");

    // TOKtype
    static assert(__traits(getProtection, TestProt1) == "private");
    static assert(__traits(getProtection, TestProt2) == "package");
    static assert(__traits(getProtection, TestProt3) == "protected");
    static assert(__traits(getProtection, TestProt4) == "public");
    static assert(__traits(getProtection, TestProt5) == "export");
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@9rnsr
Collaborator

OK, now implementation looks good to me.
But, this is a language enhancement, and there is another pull request #952 for the design of protection traits feature.
So I can't merge this in my authority.
@WalterBright and @andralex , I'd like to assign this to you!

@klickverbot
Collaborator

Maybe squash the commits together?

@adamdruppe
@adamdruppe

eh i tried it and think got some success, but it was complaining about merge conflicts and rejected non-fast-forward and other stuff. But it still builds so I'm gonna leave it here.

@nazriel

@adamdruppe if you are on IRC I can help you with it

@adamdruppe
@adamdruppe

I read some more about the git rebase and think I got it right this time. The diff looks clean now... we should be good to go.

src/traits.c
@@ -206,6 +206,59 @@ Expression *TraitsExp::semantic(Scope *sc)
StringExp *se = new StringExp(loc, s->ident->toChars());
return se->semantic(sc);
}
+ else if (ident == Id::getProtection)
+ {
+ if (dim != 1)
+ goto Ldimerror;
+ Object *o = (*args)[0];
+ Dsymbol *s = getDsymbol(o);
+ if(!s)
+ {
+ // it might also be a trait getMember or something,
+ // which returns a dot expression rather than a symbol
+ if(o->dyncast() == DYNCAST_EXPRESSION)
+ {
+ Expression* e = (Expression*) o;
@yebblies Collaborator

Expression* e = (Expression*) o;
->
Expression *e = (Expression *)o;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/traits.c
((15 lines not shown))
+ {
+ Expression* e = (Expression*) o;
+
+ if (e->op == TOKdotvar)
+ {
+ DotVarExp *dv = (DotVarExp *)e;
+ s = dv->var->isDeclaration();
+ }
+ }
+ }
+ if(!s)
+ {
+ bool gagError = false;
+ if(o->dyncast() == DYNCAST_EXPRESSION)
+ {
+ Expression* e = (Expression*) o;
@yebblies Collaborator

Same here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/traits.c
((28 lines not shown))
+ if(o->dyncast() == DYNCAST_EXPRESSION)
+ {
+ Expression* e = (Expression*) o;
+ if(e->op == TOKerror)
+ gagError = true;
+ }
+
+ if(!gagError)
+ error("argument %s has no protection", o->toChars());
+
+ goto Lfalse;
+ }
+
+ PROT protection = s->prot();
+
+ const char* protName =
@yebblies Collaborator

I suspect this would be much nicer as a switch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/traits.c
((38 lines not shown))
+ goto Lfalse;
+ }
+
+ PROT protection = s->prot();
+
+ const char* protName =
+ protection == PROTundefined ? ""
+ : protection == PROTnone ? "none"
+ : protection == PROTprivate ? "private"
+ : protection == PROTpackage ? "package"
+ : protection == PROTprotected ? "protected"
+ : protection == PROTpublic ? "public"
+ : protection == PROTexport ? "export"
+ : (assert(0), ""); // unknown
+
+ StringExp *se = new StringExp(loc, (char*) protName);
@yebblies Collaborator

char*
char *

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@adamdruppe

We now have two commits: one is the protection trait, all changes squashed into one, and the other is removing some duplication on the protection names, by moving json.c's impl into a global area.

This should cover everyone's concerns.

@adamdruppe

Any chance we can get this pulled for the next release?

@WalterBright WalterBright merged commit 913b485 into from
@WalterBright WalterBright referenced this pull request from a commit
@WalterBright WalterBright fix D2 pull #856 931d305
@WalterBright WalterBright referenced this pull request from a commit
@WalterBright WalterBright fix D2 pull #856 b940d6d
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@ghost Unknown referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
15 src/attrib.c
@@ -683,17 +683,10 @@ void ProtDeclaration::protectionToCBuffer(OutBuffer *buf, enum PROT protection)
{
const char *p;
- switch (protection)
- {
- case PROTprivate: p = "private"; break;
- case PROTpackage: p = "package"; break;
- case PROTprotected: p = "protected"; break;
- case PROTpublic: p = "public"; break;
- case PROTexport: p = "export"; break;
- default:
- assert(0);
- break;
- }
+ p = Pprotectionnames[protection];
+
+ assert(p);
+
buf->writestring(p);
buf->writeByte(' ');
}
View
9 src/doc.c
@@ -553,15 +553,8 @@ void ScopeDsymbol::emitMemberComments(Scope *sc)
void emitProtection(OutBuffer *buf, PROT prot)
{
- const char *p;
+ const char *p = Pprotectionnames[prot];
- switch (prot)
- {
- case PROTpackage: p = "package"; break;
- case PROTprotected: p = "protected"; break;
- case PROTexport: p = "export"; break;
- default: p = NULL; break;
- }
if (p)
buf->printf("%s ", p);
}
View
3  src/dsymbol.h
@@ -96,6 +96,9 @@ enum PROT
PROTexport,
};
+// this is used for printing the protection in json, traits, docs, etc.
+static const char* Pprotectionnames[] = {NULL, "none", "private", "package", "protected", "public", "export"};
+
/* State of symbol in winding its way through the passes of the compiler
*/
enum PASS
View
1  src/idgen.c
@@ -322,6 +322,7 @@ Msgtable msgtable[] =
{ "isLazy" },
{ "hasMember" },
{ "identifier" },
+ { "getProtection" },
{ "parent" },
{ "getMember" },
{ "getOverloads" },
View
1  src/json.c
@@ -44,7 +44,6 @@ const char Ptype[] = "type";
const char Pcomment[] = "comment";
const char Pmembers[] = "members";
const char Pprotection[] = "protection";
-const char* Pprotectionnames[] = {NULL, "none", "private", "package", "protected", "public", "export"};
void JsonRemoveComma(OutBuffer *buf);
View
45 src/traits.c
@@ -206,6 +206,51 @@ Expression *TraitsExp::semantic(Scope *sc)
StringExp *se = new StringExp(loc, s->ident->toChars());
return se->semantic(sc);
}
+ else if (ident == Id::getProtection)
+ {
+ if (dim != 1)
+ goto Ldimerror;
+ Object *o = (*args)[0];
+ Dsymbol *s = getDsymbol(o);
+ if(!s)
+ {
+ // it might also be a trait getMember or something,
+ // which returns a dot expression rather than a symbol
+ if(o->dyncast() == DYNCAST_EXPRESSION)
+ {
+ Expression *e = (Expression *) o;
+
+ if (e->op == TOKdotvar)
+ {
+ DotVarExp *dv = (DotVarExp *)e;
+ s = dv->var->isDeclaration();
+ }
+ }
+ }
+ if(!s)
+ {
+ bool gagError = false;
+ if(o->dyncast() == DYNCAST_EXPRESSION)
+ {
+ Expression *e = (Expression *) o;
+ if(e->op == TOKerror)
+ gagError = true;
+ }
+
+ if(!gagError)
+ error("argument %s has no protection", o->toChars());
+
+ goto Lfalse;
+ }
+
+ PROT protection = s->prot();
+
+ const char *protName = Pprotectionnames[protection];
+
+ StringExp *se = new StringExp(loc, (char *) protName);
+ return se->semantic(sc);
+ }
+
else if (ident == Id::parent)
{
if (dim != 1)
View
69 test/runnable/traits.d
@@ -729,6 +729,75 @@ void test7608()
/********************************************************/
+private struct TestProt1 {}
+package struct TestProt2 {}
+protected struct TestProt3 {}
+public struct TestProt4 {}
+export struct TestProt5 {}
+
+void getProtection()
+{
+ class Test
+ {
+ private { int va; void fa(){} }
+ package { int vb; void fb(){} }
+ protected { int vc; void fc(){} }
+ public { int vd; void fd(){} }
+ export { int ve; void fe(){} }
+ }
+ Test t;
+
+ // TOKvar and VarDeclaration
+ static assert(__traits(getProtection, Test.va) == "private");
+ static assert(__traits(getProtection, Test.vb) == "package");
+ static assert(__traits(getProtection, Test.vc) == "protected");
+ static assert(__traits(getProtection, Test.vd) == "public");
+ static assert(__traits(getProtection, Test.ve) == "export");
+
+ // TOKdotvar and VarDeclaration
+ static assert(__traits(getProtection, t.va) == "private");
+ static assert(__traits(getProtection, t.vb) == "package");
+ static assert(__traits(getProtection, t.vc) == "protected");
+ static assert(__traits(getProtection, t.vd) == "public");
+ static assert(__traits(getProtection, t.ve) == "export");
+
+ // TOKvar and FuncDeclaration
+ static assert(__traits(getProtection, Test.fa) == "private");
+ static assert(__traits(getProtection, Test.fb) == "package");
+ static assert(__traits(getProtection, Test.fc) == "protected");
+ static assert(__traits(getProtection, Test.fd) == "public");
+ static assert(__traits(getProtection, Test.fe) == "export");
+
+ // TOKdotvar and FuncDeclaration
+ static assert(__traits(getProtection, t.fa) == "private");
+ static assert(__traits(getProtection, t.fb) == "package");
+ static assert(__traits(getProtection, t.fc) == "protected");
+ static assert(__traits(getProtection, t.fd) == "public");
+ static assert(__traits(getProtection, t.fe) == "export");
+
+ // TOKtype
+ static assert(__traits(getProtection, TestProt1) == "private");
+ static assert(__traits(getProtection, TestProt2) == "package");
+ static assert(__traits(getProtection, TestProt3) == "protected");
+ static assert(__traits(getProtection, TestProt4) == "public");
+ static assert(__traits(getProtection, TestProt5) == "export");
+
+ // This specific pattern is important to ensure it always works
+ // through reflection, however that becomes implemented
+ static assert(__traits(getProtection, __traits(getMember, t, "va")) == "private");
+ static assert(__traits(getProtection, __traits(getMember, t, "vb")) == "package");
+ static assert(__traits(getProtection, __traits(getMember, t, "vc")) == "protected");
+ static assert(__traits(getProtection, __traits(getMember, t, "vd")) == "public");
+ static assert(__traits(getProtection, __traits(getMember, t, "ve")) == "export");
+ static assert(__traits(getProtection, __traits(getMember, t, "fa")) == "private");
+ static assert(__traits(getProtection, __traits(getMember, t, "fb")) == "package");
+ static assert(__traits(getProtection, __traits(getMember, t, "fc")) == "protected");
+ static assert(__traits(getProtection, __traits(getMember, t, "fd")) == "public");
+ static assert(__traits(getProtection, __traits(getMember, t, "fe")) == "export");
+}
+
+/********************************************************/
+
int main()
{
test1();
Something went wrong with that request. Please try again.