dmd
dmd copied to clipboard
dlang
Fix Issue 22987 - Make traits getLocation have an option to return an…
… absolute path
The feature is required for an application of metaprogramming within Symmetry.
A flag is added rather than changing the default in case it breaks something.
Thanks for your pull request, @maxhaton!
Bugzilla references
| Auto-close | Bugzilla | Severity | Description |
|---|---|---|---|
| ✓ | 22987 | normal | __traits(getLocation) needs a way to get an absolute path. |
Testing this PR locally
If you don't have a local development environment setup, you can use Digger to test this PR:
dub run digger -- build "master + dmd#13947"
The feature is required for an application of metaprogramming within Symmetry.
Why isn't a relative path sufficient?
The feature is required for an application of metaprogramming within Symmetry.
Why isn't a relative path sufficient?
Because we need the absolute path at compile time, without this patch we have to inject some crap into our build system which is quite irritating.
Because we need the absolute path at compile time, without this patch we have to inject some crap into our build system which is quite irritating.
That's a better place than injecting some crap into the D language.
Because we need the absolute path at compile time, without this patch we have to inject some crap into our build system which is quite irritating.
That's a better place than injecting some crap into the D language.
I strongly contend this is not crap, traits is supposed to be for getting info out of the compiler, this is info we need. We generate interpreter hooks at compile time, we need to know where the hooks come from, this doesn't really work with a relative path, hence this patch. On top of that the cost of adding it is zero.
In fact, dmd actually already outputs absolute paths sometimes
/home/mhh/.dub/packages/intel-intrinsics-1.8.0/intel-intrinsics/source/inteli/math.d:39
asdasd/test.d:3
I strongly contend this is not crap
Agreed, I just mirrored your wording. I would call it an instance of feature creep.
this is info we need.
This does not provide new info, you already have the working directory and a relative path, you're just letting dmd do the conversion work.
this doesn't really work with a relative path
Why not?
On top of that the cost of adding it is zero.
Features add maintenance cost. This one in particular adds a dependency on dmd.root.filename to traits.d and makes semanticTraits more dependent on global state, making refactoring that giant function harder than it already is.
In fact, dmd actually already outputs absolute paths sometimes
Is that a bug? If I want a relative path always, are we going to add __traits(getLocation, symbol, false, true)?
@ibuclaw The CircleCI thing strikes again!
It's just you, and it only happens for pipelines under /app.circleci.com/pipelines/github/maxhaton/ namespace, so can't do much to help. Delete your dmd repo and recreate? :shrug:
The point about global state is pretty dumb. It's a couple of branches, if you were refactoring you could copy the whole thing because it's self-contained. So again, no cost. If I'd implemented a new reflection API as a trait and dumped it in there I'd agree, but I haven't.
If we had a function to convert to a relative path then I'd have made the default a relative path but as far as I can tell there isn't one in dmd.root. the point about refactoring is misguided, the point about adding another boolean parameter is positively stupid.
On Tue, 5 Apr 2022, 09:37 Dennis, @.***> wrote:
I strongly contend this is not crap
Agreed, I just mirrored your wording. I would call it an instance of feature creep.
this is info we need.
This does not provide new info, you already have the working directory and a relative path, you're just letting dmd do the conversion work.
this doesn't really work with a relative path
Why not?
On top of that the cost of adding it is zero.
Features add maintenance cost. This one in particular adds a dependency on dmd.root.filename to traits.d and makes semanticTraits more dependent on global state, making refactoring that giant function harder than it already is.
In fact, dmd actually already outputs absolute paths sometimes
Is that a bug? If I want a relative path always, are we going to add __traits(getLocation, symbol, false, true)?
— Reply to this email directly, view it on GitHub https://github.com/dlang/dmd/pull/13947#issuecomment-1088425985, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLI75GSQI5NISWAMFNLAFDVDP3TNANCNFSM5SQARBDQ . You are receiving this because you authored the thread.Message ID: @.***>
@ibuclaw The CircleCI thing strikes again!
It's just you, and it only happens for pipelines under
/app.circleci.com/pipelines/github/maxhaton/namespace, so can't do much to help. Delete your dmd repo and recreate? 🤷
That's probably what I'm going to do although if it's stateful on CircleCI then I have no idea
I suggest wrapping the __traits(getLocation, symbol) with:
https://dlang.org/phobos/std_path.html#absolutePath
in the application (not the compiler).
