add location for storage class declaration #14720
Conversation
|
Thanks for your pull request and interest in making D better, @lucica28! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#14720" |
lucica28
force-pushed
the
scd-loc
branch
2 times, most recently
from
b33b3ff
to
c81cdfc
Compare
compiler/src/dmd/astbase.d
Outdated
| @@ -1274,6 +1274,13 @@ struct ASTBase | |||
| this.stc = stc; | |||
| } | |||
|
|
|||
| final extern (D) this(StorageClass stc, Dsymbols* decl, const ref Loc loc) | |||
There was a problem hiding this comment.
The convention in dmd is normally Loc as the first constructor argument
compiler/src/dmd/dsymbol.d
Outdated
| @@ -269,6 +269,12 @@ extern (C++) class Dsymbol : ASTNode | |||
| loc = Loc(null, 0, 0); | |||
| } | |||
|
|
|||
| final extern (D) this(const ref Loc loc) nothrow | |||
There was a problem hiding this comment.
I don't understand, why is this needed? Can't you just set the loc in the constructor in attrib.d?
There was a problem hiding this comment.
No, because loc field is declared const, so it can be set only by a constructor
https://github.com/dlang/dmd/blob/master/compiler/src/dmd/dsymbol.d#L258
There was a problem hiding this comment.
Which you are declaring in StorageClassDeclaration.
There was a problem hiding this comment.
I don't understand, why is this needed? Can't you just set the loc in the constructor in attrib.d?
in the constructor in attrib.d
| extern (D) this(const ref Loc loc, StorageClass stc, Dsymbols* decl) | ||
| { | ||
| super(loc, decl); | ||
| this.stc = stc; |
There was a problem hiding this comment.
This is a constructor. You can set the loc here and ditch the dsymbol and attrib constructors.
There was a problem hiding this comment.
That's what I initially tried, but this code
extern (D) this(const ref Loc loc, StorageClass stc, Dsymbols* decl)
{
super(decl);
this.stc = stc;
this.loc = loc;
}
Does not work, it results in the following error: Error: cannot modify constexpressionthis.loc``
There was a problem hiding this comment.
I guess you can use the existing constructor in AttribDeclaration, but pass a null identifier.
|
ping @lucica28 |
Currently there is no possibility of setting the location for a
StorageClassDeclaration. This pr creates a way forStorageClassDeclarationsto be instantiated with a location