search

dlang-community / Pegged

A Parsing Expression Grammar (PEG) module, using the D programming language.
534 stars 66 forks source link

chore(peg): make it compilable under -dip1000 flag #314

Closed ljmf00-wekaio closed 1 year ago

ljmf00-wekaio commented 1 year ago

Signed-off-by: Luís Ferreira luis.ferreira@weka.io

I believe this is either a phobos or a compiler issue on scope inference. Documentation/source code guarantees it copies the data, so no need to write an extra .dup/.idup, just trust it doesn't escape a reference.

ljmf00-wekaio commented 1 year ago

@dkorpel since you are more into dip1000, do you know any compiler issue on scope inference, that might affect these functions?

dkorpel commented 1 year ago

@dkorpel since you are more into dip1000, do you know any compiler issue on scope inference, that might affect these functions?

Issue 23300 - std.array : array wrongly propagates scopeness of source Issue 20674 - [DIP1000] inference of scope is easily confused

I don't think the workaround of returning scope pointers in a @trusted lambda will stay working when https://github.com/dlang/dmd/pull/14236 gets merged. I think it's better to mark stringified and getUpto as @trusted.

ljmf00-wekaio commented 1 year ago

@dkorpel since you are more into dip1000, do you know any compiler issue on scope inference, that might affect these functions?

Issue 23300 - std.array : array wrongly propagates scopeness of source Issue 20674 - [DIP1000] inference of scope is easily confused

Thanks!

I don't think the workaround of returning scope pointers in a @trusted lambda will stay working when dlang/dmd#14236 gets merged. I think it's better to mark stringified and getUpto as @trusted.

Yeah, my intention was not to trust the function call, but only the result.

veelo commented 1 year ago

I don't think the workaround of returning scope pointers in a @trusted lambda will stay working when dlang/dmd#14236 gets merged. I think it's better to mark stringified and getUpto as @trusted.

Yeah, my intention was not to trust the function call, but only the result.

I'll merge this then -- I "trust" you'll be back with another fix if that becomes necessary ;-)

Do you need me to tag a new release?

Geod24 commented 1 year ago

@veelo : Would be great if you could tag a new release as v2.101.0 is about to go out.

ljmf00-wekaio commented 1 year ago

@veelo : Would be great if you could tag a new release as v2.101.0 is about to go out.

Maybe we can make the change suggested by @dkorpel instead, as my solution turns out to be "hackier" and not correct on casting away the scope (https://github.com/dlang/dmd/pull/14236#issuecomment-1256628770). I can make a PR fixing that and tag it afterwards?

TL;'DR; this hack will stop working on next compiler releases.

veelo commented 1 year ago

I can make a PR fixing that and tag it afterwards?

Sounds like a good plan.

ljmf00-wekaio commented 1 year ago

CC @Geod24 See the new tag :)