I found this why looking at bug 7482, and it seems to be a fundamental problem. Surely other people have hit this before.

Something like:
deprecated int x;
is parsed as  StorageDeclaration( STCdeprecated, new VarDeclaration(x))

note that the VarDeclaration is initially created without the
'deprecated' storage class.
Then, when semantic is run on all the declarations in the module, it
runs on StorageDeclaration, which then runs semanticNewSc on the
VarDeclaration.

The problem is, if 'x' is forward referenced, the storage declaration hasn't run yet. It gets found with the original, wrong storage class. I don't understand why these AttribDeclarations exist at all. Why isn't the whole thing dealt with in the parser?

(ie, create a PendingAttribs struct, and pass it to all the children
in a block).

Attributes don't always apply to everything inside them - eg a const declaration doesn't make everything parsed inside it automatically const, and this is sorted out during semantic as it should be.  And if you pass it to the top-level declarations only, the same problem will exist if you try and forward-reference sub-declarations.  Maybe this could be done in a pre-semantic pass, though I'm not sure if it should be.

I think the PendingAttribs approach might work for deprecated, but will it for the others?  Being able to pass a bunch of attributes all nested declarations without modifying semantic gives an easy way to implement new attributes (even user-defined, if we ever go there) that would simplify some things such as adding a message to deprecated.

On Fri, Feb 17, 2012 at 7:48 PM, Don Clugston <dclugston at googlemail.com> wrote:
> I found this why looking at bug 7482, and it seems to be a fundamental problem. Surely other people have hit this before.
>
> Something like:
> deprecated int x;
> is parsed as ?StorageDeclaration( STCdeprecated, new VarDeclaration(x))
>
> note that the VarDeclaration is initially created without the
> 'deprecated' storage class.
> Then, when semantic is run on all the declarations in the module, it
> runs on StorageDeclaration, which then runs semanticNewSc on the
> VarDeclaration.
>
> The problem is, if 'x' is forward referenced, the storage declaration hasn't run yet. It gets found with the original, wrong storage class. I don't understand why these AttribDeclarations exist at all. Why isn't the whole thing dealt with in the parser?
>
> (ie, create a PendingAttribs struct, and pass it to all the children
> in a block).
> _______________________________________________
> dmd-internals mailing list
> dmd-internals at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals

On 17 February 2012 10:49, Daniel Murphy <yebblies at gmail.com> wrote:
> Attributes don't always apply to everything inside them - eg a const declaration doesn't make everything parsed inside it automatically const, and this is sorted out during semantic as it should be.

Yes, but that's dealt with in StorageDeclaration::semantic, which, as
far as I can tell, does nothing which couldn't be done better in the
parser.
In particular, I do not think the parser should be creating
declarations with attributes which it knows are wrong.
The attributes relevant for a declaration are always known when that
declaration is parsed.

Interestingly declarations involving type inference don't work in the same way.
For example
deprecated auto x = 5;
is marked as deprecated immediately.
whereas
deprecated int x = 5;
is not.

I think the existing approach is the reason the compiler has accepts nonsensical combinations of attributes without complaint.

>?And if
> you pass it to the top-level declarations only, the same problem will exist if you try and forward-reference sub-declarations. ?Maybe this could be done in a pre-semantic pass, though I'm not sure if it should be.

There's no problem there, it isn't possible to reach a sub-declaration
of an aggregate type without going through its parent.
I would envisage that each level of nesting would have its own
PendingAttribs, based on the one it received from its parent.

> I think the PendingAttribs approach might work for deprecated, but will it for the others? ?Being able to pass a bunch of attributes all nested declarations without modifying semantic gives an easy way to implement new attributes (even user-defined, if we ever go there) that would simplify some things such as adding a message to deprecated.

But it doesn't work. The declaration needs to know its attributes, in
the case where it is forward referenced.
I've patched a few of these bugs before, and generally, the semantic
needs to be modified to take the pending attributes into account.

>
> On Fri, Feb 17, 2012 at 7:48 PM, Don Clugston <dclugston at googlemail.com> wrote:
>> I found this why looking at bug 7482, and it seems to be a fundamental problem. Surely other people have hit this before.
>>
>> Something like:
>> deprecated int x;
>> is parsed as ?StorageDeclaration( STCdeprecated, new VarDeclaration(x))
>>
>> note that the VarDeclaration is initially created without the
>> 'deprecated' storage class.
>> Then, when semantic is run on all the declarations in the module, it
>> runs on StorageDeclaration, which then runs semanticNewSc on the
>> VarDeclaration.
>>
>> The problem is, if 'x' is forward referenced, the storage declaration hasn't run yet. It gets found with the original, wrong storage class. I don't understand why these AttribDeclarations exist at all. Why isn't the whole thing dealt with in the parser?
>>
>> (ie, create a PendingAttribs struct, and pass it to all the children
>> in a block).
>> _______________________________________________
>> dmd-internals mailing list
>> dmd-internals at puremagic.com
>> http://lists.puremagic.com/mailman/listinfo/dmd-internals
> _______________________________________________
> dmd-internals mailing list
> dmd-internals at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/dmd-internals

On Fri, Feb 17, 2012 at 9:44 PM, Don Clugston <dclugston at googlemail.com> wrote:
> Yes, but that's dealt with in StorageDeclaration::semantic, which, as far as I can tell, does nothing which couldn't be done better in the parser.

Good point, I can't think of any way that storage classes depend on semantic.

> I think the existing approach is the reason the compiler has accepts nonsensical combinations of attributes without complaint.

Yeah, I never realized this could be done in the parser - I thought storage classes etc had to be propagated/filtered during semantic.

I think the rest of what I said was due to this misunderstanding.

One problem I've run into with the current setup is that AttribDeclarations don't exist by the time semantic is run on the members.  This means that eg a PragmaDeclaration has to inject information recursively into its members, or add it to the scope. Having declarations be able to query the attributes attached to it would make some things I've tried to do much easier.

So yeah, if it can be done in the parser, I see no reason why it shouldn't be done in the parser.

On Fri, 17 Feb 2012 12:19:40 +0100, Daniel Murphy <yebblies@gmail.com> wrote:

> On Fri, Feb 17, 2012 at 9:44 PM, Don Clugston <dclugston@googlemail.com> wrote:
>> Yes, but that's dealt with in StorageDeclaration::semantic, which, as
>> far as I can tell, does nothing which couldn't be done better in the
>> parser.
>
> Good point, I can't think of any way that storage classes depend on semantic.
>
++i
_______________________________________________
dmd-internals mailing list
dmd-internals@puremagic.com
http://lists.puremagic.com/mailman/listinfo/dmd-internals