Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error for implicit catch statement #8241

Merged
merged 1 commit into from
May 29, 2018
Merged

Error for implicit catch statement #8241

merged 1 commit into from
May 29, 2018

Conversation

JinShil
Copy link
Contributor

@JinShil JinShil commented May 12, 2018

See https://dlang.org/deprecate.html#Implicit%20catch%20statement

This PR moves a deprecation forward, further narrowing the gap between specification and implementation.

Update to deprecated features table: dlang/dlang.org#2365

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @JinShil!

Bugzilla references

Your 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 locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#8241"

@JinShil JinShil added WIP Work In Progress - not ready for review or pulling and removed Easy Review labels May 12, 2018
@JinShil JinShil removed the WIP Work In Progress - not ready for review or pulling label May 12, 2018
@@ -4105,8 +4105,9 @@ void catchSemantic(Catch c, Scope* sc)

if (!c.type)
{
deprecation(c.loc, "`catch` statement without an exception " ~
error(c.loc, "`catch` statement without an exception " ~
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errorSupplemental might be a good idea here for the second line.

@wilzbach
Copy link
Member

Seems like two D projects need upgrades first :/

@JinShil
Copy link
Contributor Author

JinShil commented May 12, 2018

Seems like two D projects need upgrades first :/

I'm not sure how Jenkins selects the branch to test, but the error messages don't seem to correspond to the code in the repositories' master branch. Maybe the repositories just need an updated tag.

@wilzbach
Copy link
Member

Yes, Jenkins checks out the latest git tag.

@wilzbach
Copy link
Member

Regarding the Jenkins failure - the Ocean failure is unrelated.
See dlang/ci#208 (after the next push, Jenkins should be green again).

@wilzbach
Copy link
Member

Interestingly this triggers an ICE when building dlang.org:

DMD v2.080.0-138-g97876ca
predefs   MARS CoreDdoc StdDdoc DigitalMars Posix linux ELFv1 LittleEndian D_Version2 all D_SIMD D_InlineAsm_X86_64 X86_64 CRuntime_Glibc D_LP64 D_Ddoc D_PIC assert D_HardFloatbinary    ../dmd/generated/linux/release/64/dmd
version   v2.080.0-138-g97876ca
config    ../dmd/generated/linux/release/64/dmd.conf
DFLAGS    -I../dmd/generated/linux/release/64/../../../../../druntime/import -I../dmd/generated/linux/release/64/../../../../../phobos -L-L../dmd/generated/linux/release/64/../../../../../phobos/generated/linux/release/64 -L--export-dynamic -fPIC
---
core.exception.AssertError@dmd/cond.d(378): Assertion failure
----------------
??:? _d_assertp [0x75987d]
dmd/cond.d:378 _D3dmd4cond13StaticForeach7prepareMFPS3dmd6dscope5ScopeZ9__requireMFNaNbNiNfZv [0x51de3f]
dmd/cond.d:377 void dmd.cond.StaticForeach.prepare(dmd.dscope.Scope*) [0x51dcec]
dmd/attrib.d:1083 _ZN24StaticForeachDeclaration7includeEP5Scope [0x5159ab]
dmd/json.d:587 _ZN13ToJsonVisitor5visitEP17AttribDeclaration [0x5ed543]
dmd/parsetimevisitor.d:75 _ZN16ParseTimeVisitorI10ASTCodegenE5visitEP24StaticForeachDeclaration [0x657c26]
dmd/attrib.d:1143 _ZN24StaticForeachDeclaration6acceptEP7Visitor [0x515b59]
dmd/json.d:592 _ZN13ToJsonVisitor5visitEP17AttribDeclaration [0x5ed58d]
dmd/parsetimevisitor.d:71 _ZN16ParseTimeVisitorI10ASTCodegenE5visitEP15ProtDeclaration [0x657b96]
dmd/attrib.d:605 _ZN15ProtDeclaration6acceptEP7Visitor [0x514a15]
dmd/json.d:592 _ZN13ToJsonVisitor5visitEP17AttribDeclaration [0x5ed58d]
dmd/parsetimevisitor.d:71 _ZN16ParseTimeVisitorI10ASTCodegenE5visitEP15ProtDeclaration [0x657b96]
dmd/attrib.d:605 _ZN15ProtDeclaration6acceptEP7Visitor [0x514a15]
dmd/json.d:656 _ZN13ToJsonVisitor5visitEP20AggregateDeclaration [0x5ed81a]
dmd/parsetimevisitor.d:83 _ZN16ParseTimeVisitorI10ASTCodegenE5visitEP17StructDeclaration [0x657cdd]
dmd/dstruct.d:595 _ZN17StructDeclaration6acceptEP7Visitor [0x56a0d9]
dmd/json.d:756 _ZN13ToJsonVisitor5visitEP19TemplateDeclaration [0x5edd41]
dmd/dtemplate.d:2276 _ZN19TemplateDeclaration6acceptEP7Visitor [0x5854dd]
dmd/json.d:592 _ZN13ToJsonVisitor5visitEP17AttribDeclaration [0x5ed58d]
dmd/parsetimevisitor.d:73 _ZN16ParseTimeVisitorI10ASTCodegenE5visitEP23StorageClassDeclaration [0x657bde]
dmd/attrib.d:376 _ZN23StorageClassDeclaration6acceptEP7Visitor [0x51419d]
dmd/json.d:511 _ZN13ToJsonVisitor5visitEP6Module [0x5ed129]
dmd/dmodule.d:1332 _ZN6Module6acceptEP7Visitor [0x55f689]
dmd/json.d:840 _ZN13ToJsonVisitor15generateModulesEP5ArrayIP6ModuleE [0x5ee095]
dmd/json.d:1014 _Z13json_generateP9OutBufferP5ArrayIP6ModuleE [0x5ee88c]
dmd/mars.d:1003 void dmd.mars.generateJson(dmd.root.array.Array!(dmd.dmodule.Module).Array*) [0x60c360]
dmd/mars.d:894 int dmd.mars.tryMain(ulong, const(char)**) [0x60b7f7]
dmd/mars.d:1089 _Dmain [0x60c546]
??:? _D2rt6dmain211_d_run_mainUiPPaPUAAaZiZ6runAllMFZ9__lambda1MFNlZv [0x75bb2f]

@JinShil
Copy link
Contributor Author

JinShil commented May 18, 2018

Interestingly this triggers an ICE when building dlang.org:

I've seen that before in DAutoTest. I simply initiated a new build and it went away. The same issue was also noticed in DAutoTest for dlang/druntime#2176. It appears @TurkeyMan solved that also by pushing again and initiating a new build. I don't know what the core problem is, but it's not related to this PR. cc @CyberShadow

Edit: You can currently see this same error manifesting itself at #8234. See the DAutoTest output
Edit: Also at dlang/druntime#2179. See the DAutoTest Output

@JinShil
Copy link
Contributor Author

JinShil commented May 21, 2018

This is still waiting on msgpack-d, so I'm removing the auto-merge label to de-prioritize it in the CI.

@RazvanN7
Copy link
Contributor

How long are we supposed to wait on jenkins projects to update themselves? It's been a fair amount of time since this PR was open not to mention the deprecation period. I suggest that we move on with this.

@wilzbach
Copy link
Member

How long are we supposed to wait on jenkins projects to update themselves?

Three days. If a PR was needed to fix the project and hasn't been merged yet before we remove the project due to inactivity.
In this case, msgpack-d looks generally un-maintained and I just made a PR to remove it from the project tester (-> dlang/ci#211) which should be auto-merged in a few minutes. Then this PR can be repushed and it should finally pass on Jenkins.
The motivation behind removing inactive projects is quite simple: there's only so much we can expect from a contributor to worry about and if a project hasn't managed remove the deprecation warnings which in this case have existed for a long time, that's already a bad sign. If PRs to fix the deprecations aren't merged quickly, it just blocks the general flow of PRs here and costs us much more than we gain by having it as part of the Project Tester.
(@ people using msgpack-d): If the project ever gets active again, it's super easy to add it back again. Just let us know or open the PR yourself.

@jacob-carlborg
Copy link
Contributor

(@ people using msgpack-d): If the project ever gets active again, it's super easy to add it back again. Just let us know or open the PR yourself.

FYI, msgpack-d is used by DCD.

@wilzbach
Copy link
Member

Then we should better try to move it to dlang-community :O

@RazvanN7 RazvanN7 merged commit 6d27555 into dlang:master May 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants