search

dlang-community / Pegged

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

Qualify pegged/peg.d as @safe and when possible pure nothrow @nogc #311

Closed nordlow closed 2 years ago

nordlow commented 2 years ago

./ci.sh passes for me on Ubuntu 20.04 now, @John-Colvin.

veelo commented 2 years ago

It seems to me that the CI failures are due to deficiencies in moderately recent (2 years old) druntime.

I am thinking of determining the release at which this starts working a bit more precisely, putting that in a toolchainRequirements section, and releasing this with a mayor version bump. Would that work for Symmetry @John-Colvin? What is the oldest D frontend release that you depend on?

nordlow commented 2 years ago

It seems to me that the CI failures are due to deficiencies in moderately recent (2 years old) druntime. I am thinking of determining the release at which this starts working a bit more precisely, putting that in a toolchainRequirements section, and releasing this with a mayor version bump. Would that work for Symmetry @John-Colvin? What is the oldest D frontend release that you depend on?

Yes, specifically when AA.keys being @system. I've added some versioning to solve some of them at https://github.com/PhilippeSigaud/Pegged/pull/311/commits/9fb4d41be7bb3b7c81dcbfa7658d428a89380eb4.
I doubt we are in need of supporting a pre-2.091 toolchain. Right @John-Colvin? If so I can remove the versioning I just added at https://github.com/PhilippeSigaud/Pegged/pull/311/commits/9fb4d41be7bb3b7c81dcbfa7658d428a89380eb4.

nordlow commented 2 years ago

Hitting 2.091-version compiler bugs aswell at https://ci.appveyor.com/project/PhilippeSigaud/pegged/builds/43483820/job/3503ke4jawrotjej#L83.

John-Colvin commented 2 years ago

We only care about the compilers we actively support (@nordlow the ones our setup scripts install)

nordlow commented 2 years ago

CI is green now. In appveyor.yml, I suggest removing support for dmd 2.087.1 and bump 2.090.1 to 2.091.0 having @safe AA.keys which avoids the need for the versioning added at https://github.com/PhilippeSigaud/Pegged/pull/311/commits/9fb4d41be7bb3b7c81dcbfa7658d428a89380eb4.

nordlow commented 2 years ago

We only care about the compilers we actively support (@nordlow the ones our setup scripts install)

The oldest compiler version "we" need support for is version 2.099.1. So to which compiler version less or equal to <= 2.099.1 do you want to bump matrix-entries in appveyor.yml to, @veelo?

nordlow commented 2 years ago

Can you please react to #311 (comment)? I think it fits into this PR. If you don't want to do it then that's fine, and I'll see if I can do a follow-up PR.

Sorry, missed that. I'll have look.

veelo commented 2 years ago

So to which compiler version less or equal to <= 2.099.1 do you want to bump matrix-entries in appveyor.yml to, @veelo?

I think we should try to support as far back as we reasonably can, so like you suggest:

removing support for dmd 2.087.1 and bump 2.090.1 to 2.091.0

but then also make that explicit with a toolchainRequirements section section in the Dub configuration.

I'll have to think about whether that demands a major version bump, since technically that would brake the build for people that don't update their compiler regularly. I'll ask the forum.

nordlow commented 2 years ago

Correction: AA.keys() became @safe version in 2.098.0 as per https://dlang.org/changelog/2.098.0.html#bugfix-list referencing https://issues.dlang.org/show_bug.cgi?id=14439. Gonna update.

veelo commented 2 years ago

I'll ask the forum.

https://forum.dlang.org/post/pxyqgiyosabquyjzwxex@forum.dlang.org

veelo commented 2 years ago

I cannot release this yet, since parser.d is a generated parser (Pegged generates its own parser). Whenever parser.d is regenerated, the @safe annotations will be reverted from it. Pegged will need to be modified to generate @safe parsers.

I really need to review the CI so that this is covered in the future.

nordlow commented 2 years ago

I cannot release this yet, since parser.d is a generated parser (Pegged generates its own parser). Whenever parser.d is regenerated, the @safe annotations will be reverted from it. Pegged will need to be modified to generate @safe parsers.

Can you give me a sample command line call that does this? Then I can dig into having the generated parser be @safe as well.

veelo commented 2 years ago

It is in https://github.com/PhilippeSigaud/Pegged/tree/master/pegged/dev. But that doesn't work anymore because I have accepted too many PRs that weren't sound :-( The command is currently

cd pegged/dev
rdmd -I../.. -I../../examples/peggedgrammar/src -I../../examples/misc/src regenerate.d

but that reverses some changes from some earlier PRs. It's a mess.

If you are waiting for this, I can tag a new release in the mean time while I clean this up. It has been this way since a few releases. Would a beta work for you?

veelo commented 2 years ago

https://github.com/PhilippeSigaud/Pegged/releases/tag/v0.4.6

nordlow commented 2 years ago

https://github.com/PhilippeSigaud/Pegged/releases/tag/v0.4.6

Awesome! Thank you!

Is there anything more you need help with?

nordlow commented 2 years ago

Can you please bump https://code.dlang.org/packages/pegged tag aswell to 0.4.6? It's still at 0.4.5.

veelo commented 2 years ago

Is there anything more you need help with?

Seriously? There is a lot to do actually, but most of it requires a deeper dive. We can talk about those if you are interested.

Otherwise, I'd really like an automated check on PRs to make sure the committed parser.d is the same as one that is generated.

veelo commented 2 years ago

Can you please bump https://code.dlang.org/packages/pegged tag aswell to 0.4.6? It's still at 0.4.5.

To my knowledge, it should update automatically. It probably scans for new tags every so many hours.

nordlow commented 2 years ago

To my knowledge, it should update automatically. It probably scans for new tags every so many hours.

Still no bump to 0.4.6 at https://code.dlang.org/packages/pegged. Can you bump it yourself, please? Are you the only one who has permissions to do so? FYI, @skoppe.

veelo commented 2 years ago

No, I don't even have an account. I think that one is still with @PhilippeSigaud. I am wondering if it is because the other releases have a commit signature icon, but I don't know why the latest doesn't.

nordlow commented 2 years ago

No, I don't even have an account. I think that one is still with @PhilippeSigaud. I am wondering if it is because the other releases have a commit signature icon, but I don't know why the latest doesn't.

Do you have any clues as to why, @John-Colvin @skoppe?

veelo commented 2 years ago

I don't think the signature has anything to do with it, other packages that did update in the Dub registry within the last 16 hours don't have that icon either. I've checked the syntax of the tag multiple times, should be no problems there. I made sure that the pre-release checkbox is unchecked. The Dub docs say that tags are scanned about twice an hour. This is not my first release, and I don't think I did anything different from last time.

nordlow commented 2 years ago

I don't think the signature has anything to do with it, other packages that did update in the Dub registry within the last 16 hours don't have that icon either. I've checked the syntax of the tag multiple times, should be no problems there. I made sure that the pre-release checkbox is unchecked. The Dub docs say that tags are scanned about twice an hour. This is not my first release, and I don't think I did anything different from last time.

Do you have any clues, @CyberShadow?

CyberShadow commented 2 years ago

I remember that the updater used to sometimes get stuck. Sonke would come in and restart it. However, I just tried to update my own package and it succeeded, so I don't think it's that in this case. It seems to have trouble with this specific package.

A package admin could go to https://code.dlang.org/my_packages/pegged and perhaps that page would show an error message.

@s-ludwig Seems that we need your help again; also, if there's anyone else who could help here, perhaps they should be listed on https://wiki.dlang.org/People#Points_of_contact .

nordlow commented 2 years ago

Further note that https://github.com/PhilippeSigaud has been inactive for at least one year.

veelo commented 2 years ago

Maybe it's time to transfer Pegged to https://github.com/dlang-community? I'm fine with continuing as a maintainer of Pegged. At least then there is a group of people that are able to pick up the ball it case it is dropped. Is there a shared dub account for those projects? What is the procedure for such a transfer?

nordlow commented 2 years ago

Maybe it's time to transfer Pegged to https://github.com/dlang-community? I'm fine with continuing as a maintainer of Pegged. At least then there is a group of people that are able to pick up the ball it case it is dropped. Is there a shared dub account for those projects? What is the procedure for such a transfer?

I vote for that. Ping, @John-Colvin @RazvanN7.

skoppe commented 2 years ago

I remember that the updater used to sometimes get stuck

Yes, I have had this happen repeatably the last year. Sometimes it works, sometimes it doesn't. The page says its processing, but nothing happens (for days) unless I manually hit the button, all the while other packages are just updating.

veelo commented 2 years ago

It finally got through now.

nordlow commented 2 years ago

It finally got through now.

Thank you. Was it thanks to somebody manually refreshing the tag on code.dlang.org?

veelo commented 2 years ago

Was it thanks to somebody manually refreshing the tag on code.dlang.org?

Don't know. There was an issue with an expired certificate, but I doubt it is related because other packages had no issue.