Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Add @attribute syntax #1365

Merged
merged 4 commits into from

7 participants

@jacob-carlborg

Don't know if this is the best way to implement this, but it works and was a small change. I've run the DMD tests on Mac OS X 32bit and they all pass.

@deadalnix

Looked at the code and it does make sense to me.

src/parse.c
((5 lines not shown))
- goto Lstc;
+
+ else
+ {
+ if (isPredefinedAttribute(peek(&token)->ident))
+ {
+ stc = parseAttribute();
+ goto Lstc;
+ }
+
+ else
+ {
+ nextToken();
+
+ Expressions* arguments = new Expressions();
+ Expression* arg = parseAssignExp();
@WalterBright Owner

This will allow things like
@a+b
It would be better to have:
e = parsePrimaryExp();
if (token.value == TOKlparen)
e = new CallExp(loc, e, parseArguments());

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/parse.c
@@ -379,8 +379,29 @@ Dsymbols *Parser::parseDeclDefs(int once)
s = new UserAttributeDeclaration(exps, a);
break;
}
- stc = parseAttribute();
- goto Lstc;
+
+ else
@WalterBright Owner

It should be:

else if (peek(&token)->value == TOKident)

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

Walter: I've made the changes you mentioned in the comments.

@MartinNowak MartinNowak commented on the diff
src/parse.c
@@ -379,8 +379,32 @@ Dsymbols *Parser::parseDeclDefs(int once)
s = new UserAttributeDeclaration(exps, a);
break;
}
- stc = parseAttribute();
- goto Lstc;
+
+ else if (peek(&token)->value == TOKidentifier)
+ {
+ if (isPredefinedAttribute(peek(&token)->ident))
+ {
+ stc = parseAttribute();
+ goto Lstc;
+ }
+
@MartinNowak Collaborator

Why the space?

The empty line at line 390? I like to code with some extra space and empty newlines. I think it makes the code more readable if it's not packed.

@yebblies Collaborator

Sure, but before an else block...?

@ghost
ghost added a note

You need to conform to the existing style.

Then I guess I should use variable names with a single character, because that is soooo much clearer. Oh, and I must not forget to have code on the same line as an opening brace.

@ghost
ghost added a note

It's not about the rules, it's about consistency. I agree single-char variables suck, as does code in the opening brace. But don't introduce even more funky code like blank lines between if/else (I never saw this before in DMD or elsewhere).

@andralex Owner

yes please

I'm not introducing more funky code. I'm trying to make the existing code better, more readable. Since there are now written guidelines/style guides it seems quite arbitrary what style to follow from the existing code.

But sure, I can play ball and can conform to the existing "style". Is it anything else that need to be changed than the extra newline?

@ghost
ghost added a note

How does a blank line between if/else make code more readable? It's completely bizarre.

@andralex Owner

I agree with @AndrejMitrovic. At some level we must have a "tribal" shared view that goes beyond the written rules, and there will always be a lot that's not caught by the written rules but is silently agreed upon as not good. Our hope with keeping the written rules lax is to make for fewer, not more, discussions like this.

Then drop it, I said I will remove the newline. Again, is there anything else I need to change?

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

The code in isPredefinedAttribute is partly redundant to parseAttribute.
Maybe I've missed the discussion (any ref?) but why do user attributes parse differently than predefined attributes?

@jacob-carlborg

The code in isPredefinedAttribute is partly redundant to parseAttribute.

Yes, isPredefinedAttribute is a bit redundant. Preferably I would have liked to put my changes in parseAttribute. But the function would need to return a different type for UDA.

Maybe I've missed the discussion (any ref?) but why do user attributes parse differently than predefined attributes?

Predefined attributes only allow a symbol. UDA allows arbitrary expressions: @(1+ 2) int a;. My changes allow basiclly function calls without the parentheses: @foo(3, 4) int a;

@MartinNowak
Collaborator

There is an obvious issue then.

struct safe { int val; }
@safe(0) int a;
@MartinNowak
Collaborator

I'd like to see more UDA unit test.
There should be tests with storage class modifiers and predefined attributes, also the postfix ones.
There should tests for failing parses.
There should tests for failing semantics.

@MartinNowak
Collaborator

Predefined attributes only allow a symbol.

OK, got that.
But why can't I have void foo() @safe @bar {}?
The postfix stuff is not related to this pull though.

@WalterBright

There is an obvious issue then.

Right, and this was discussed on the n.g. The most pragmatic solution was simply "no, you cannot have a user defined attribute named safe, nothrow, etc.".

@MartinNowak
Collaborator

The most pragmatic solution was simply "no, you cannot have a user defined attribute named safe, nothrow, etc.".

Yes it makes sense to treat those as reserved after an @ but the parser detect it and print an appropriate error message.

@andralex
Owner

It would be a lot better if these didn't have built-in status, and instead appeared in object.di.

@jacob-carlborg

It would be a lot better if these didn't have built-in status, and instead appeared in object.di.

I would guess that is quite a lot bigger change than this one. The compiler still need to know about them.

src/parse.c
((5 lines not shown))
- goto Lstc;
+
+ else if (peek(&token)->value == TOKidentifier)
+ {
+ if (isPredefinedAttribute(peek(&token)->ident))
+ {
+ stc = parseAttribute();
+ goto Lstc;
+ }
+
+ else
+ {
+ nextToken();
+
+ Expressions* exprs = new Expressions();
+ Expression* expr = parsePrimaryExp();
@9rnsr Collaborator
9rnsr added a note

This parsePrimaryExp call would accept @a => a.
I'm not sure this is correct or not.

I think it's cool that you can use a function as an attribute, or rather what the function returns:

string foo ()
{
    return "bar";
}

@foo int a;

static assert(__traits(getAttributes, a)[0] == "bar");

void main () { }

Perhaps lambdas can be useful as well.

@9rnsr Collaborator
9rnsr added a note

@foo int a;

This is a irrelevant. That is an issue that UDA should interpret given function, or not.

I think that the parenthesis-less UDA should work syntactically as like the parenthesis omission in template instantiation.

Template Templ(alias a) { ... }
alias X1 = Templ!(a => a);   // OK
alias X2 = Templ!a => a;     // NG
@(a => a) void foo1() {} // OK
@a => a void foo() {}    // should be NG, and it is consistent with an illegal in X2
@9rnsr Collaborator
9rnsr added a note

But, we already think @xyz(args) void foo() is legal, so we should think the syntax around UDA more seriously.

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

I've removed the empty newlines.

@WalterBright WalterBright merged commit 15389cf into D-Programming-Language:master
@jacob-carlborg

Thanks.

@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
Commits on Dec 11, 2012
  1. @jacob-carlborg

    Add @attribute syntax.

    jacob-carlborg authored
  2. @jacob-carlborg
  3. @jacob-carlborg
Commits on Dec 13, 2012
  1. @jacob-carlborg
This page is out of date. Refresh to see the latest.
Showing with 152 additions and 3 deletions.
  1. +28 −3 src/parse.c
  2. +1 −0  src/parse.h
  3. +123 −0 test/runnable/uda.d
View
31 src/parse.c
@@ -379,8 +379,26 @@ Dsymbols *Parser::parseDeclDefs(int once)
s = new UserAttributeDeclaration(exps, a);
break;
}
- stc = parseAttribute();
- goto Lstc;
+ else if (peek(&token)->value == TOKidentifier)
+ {
+ if (isPredefinedAttribute(peek(&token)->ident))
+ {
+ stc = parseAttribute();
+ goto Lstc;
+ }
+ else
@MartinNowak Collaborator

Why the space?

The empty line at line 390? I like to code with some extra space and empty newlines. I think it makes the code more readable if it's not packed.

@yebblies Collaborator

Sure, but before an else block...?

@ghost
ghost added a note

You need to conform to the existing style.

Then I guess I should use variable names with a single character, because that is soooo much clearer. Oh, and I must not forget to have code on the same line as an opening brace.

@ghost
ghost added a note

It's not about the rules, it's about consistency. I agree single-char variables suck, as does code in the opening brace. But don't introduce even more funky code like blank lines between if/else (I never saw this before in DMD or elsewhere).

@andralex Owner

yes please

I'm not introducing more funky code. I'm trying to make the existing code better, more readable. Since there are now written guidelines/style guides it seems quite arbitrary what style to follow from the existing code.

But sure, I can play ball and can conform to the existing "style". Is it anything else that need to be changed than the extra newline?

@ghost
ghost added a note

How does a blank line between if/else make code more readable? It's completely bizarre.

@andralex Owner

I agree with @AndrejMitrovic. At some level we must have a "tribal" shared view that goes beyond the written rules, and there will always be a lot that's not caught by the written rules but is silently agreed upon as not good. Our hope with keeping the written rules lax is to make for fewer, not more, discussions like this.

Then drop it, I said I will remove the newline. Again, is there anything else I need to change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ {
+ nextToken();
+ Expressions* exprs = new Expressions();
+ Expression* expr = parsePrimaryExp();
+ if (token.value == TOKlparen)
+ expr = new CallExp(loc, expr, parseArguments());
+ exprs->push(expr);
+ a = parseBlock();
+ s = new UserAttributeDeclaration(exprs, a);
+ break;
+ }
+ }
#endif
Lstc:
@@ -6926,4 +6944,11 @@ void initPrecedence()
precedence[TOKdeclaration] = PREC_expr;
}
-
+bool isPredefinedAttribute (Identifier* identifier)
+{
+ return identifier == Id::property ||
+ identifier == Id::safe ||
+ identifier == Id::trusted ||
+ identifier == Id::system ||
+ identifier == Id::disable;
+}
View
1  src/parse.h
@@ -183,5 +183,6 @@ enum PREC
extern enum PREC precedence[TOKMAX];
void initPrecedence();
+bool isPredefinedAttribute (Identifier* identifier);
#endif /* DMD_PARSE_H */
View
123 test/runnable/uda.d
@@ -106,6 +106,123 @@ void test5()
/************************************************/
+enum Test6;
+@Test6 int x6;
+pragma(msg, __traits(getAttributes, x6));
+
+void test6()
+{
+ alias Tuple!(__traits(getAttributes, x6)) tp;
+
+ assert(tp.length == 1);
+
+ if (!is(Test6 == tp[0]))
+ assert(0);
+}
+
+/************************************************/
+
+struct Test7
+{
+ int a;
+ string b;
+}
+
+@Test7(3, "foo") int x7;
+pragma(msg, __traits(getAttributes, x7));
+
+void test7()
+{
+ alias Tuple!(__traits(getAttributes, x7)) tp;
+
+ assert(tp.length == 1);
+
+ if (!is(Test7 == typeof(tp[0])))
+ assert(0);
+
+ assert(tp[0] == Test7(3, "foo"));
+}
+
+/************************************************/
+
+struct Test8 (string foo) {}
+
+@Test8!"foo" int x8;
+pragma(msg, __traits(getAttributes, x8));
+
+void test8()
+{
+ alias Tuple!(__traits(getAttributes, x8)) tp;
+
+ assert(tp.length == 1);
+
+ if (!is(Test8!("foo") == tp[0]))
+ assert(0);
+}
+
+/************************************************/
+
+struct Test9 (string foo) {}
+
+@Test9!("foo") int x9;
+pragma(msg, __traits(getAttributes, x9));
+
+void test9()
+{
+ alias Tuple!(__traits(getAttributes, x9)) tp;
+
+ assert(tp.length == 1);
+
+ if (!is(Test9!("foo") == tp[0]))
+ assert(0);
+}
+
+/************************************************/
+
+struct Test10 (string foo)
+{
+ int a;
+}
+
+@Test10!"foo"(3) int x10;
+pragma(msg, __traits(getAttributes, x10));
+
+void test10()
+{
+ alias Tuple!(__traits(getAttributes, x10)) tp;
+
+ assert(tp.length == 1);
+
+ if (!is(Test10!("foo") == typeof(tp[0])))
+ assert(0);
+
+ assert(tp[0] == Test10!("foo")(3));
+}
+
+/************************************************/
+
+struct Test11 (string foo)
+{
+ int a;
+}
+
+@Test11!("foo")(3) int x11;
+pragma(msg, __traits(getAttributes, x11));
+
+void test11()
+{
+ alias Tuple!(__traits(getAttributes, x11)) tp;
+
+ assert(tp.length == 1);
+
+ if (!is(Test11!("foo") == typeof(tp[0])))
+ assert(0);
+
+ assert(tp[0] == Test11!("foo")(3));
+}
+
+/************************************************/
+
int main()
{
test1();
@@ -113,6 +230,12 @@ int main()
test3();
test4();
test5();
+ test6();
+ test7();
+ test8();
+ test9();
+ test10();
+ test11();
printf("Success\n");
return 0;
Something went wrong with that request. Please try again.