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

Fix issue 23142 - Scope should not apply to unittests #14177

Merged
merged 1 commit into from
Jun 9, 2022

Conversation

dkorpel
Copy link
Contributor

@dkorpel dkorpel commented May 31, 2022

Ignore scope when it doesn't apply instead of raising an error, similar to other attributes.

@dkorpel dkorpel added the dip1000 memory safety with scope, ref, return label May 31, 2022
@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @dkorpel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

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 references

Auto-close Bugzilla Severity Description
23142 enhancement Scope should not apply to unittests

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#14177"

@dkorpel dkorpel force-pushed the scope-static-ignore branch from cba6c70 to 00d4d31 Compare May 31, 2022 11:58
@Geod24
Copy link
Member

Geod24 commented May 31, 2022

similar to other attributes.

Which ones ? const and immutable both error out. Yes they are STC and not type ctor, but they apply to the implicit this, and IMO that makes them closer to scope.

@dkorpel
Copy link
Contributor Author

dkorpel commented May 31, 2022

Which ones ?

pragma(inline, true) int x;
@nogc int y;
scope int z;
alias x = __gshared int;

@Geod24
Copy link
Member

Geod24 commented May 31, 2022

But not for functions?

@dkorpel
Copy link
Contributor Author

dkorpel commented May 31, 2022

But not for functions?

__gshared void f() {}
static void g() {}
final void h() {}

@rikkimax
Copy link
Contributor

Okay, experiment time:

Only one of these error out.

struct Thing1 {
@safe scope:
    static void func() { // onlineapp.d(7): Error: function `onlineapp.Thing1.func` functions cannot be `scope`
    }
    
    unittest {
    }
}

struct Thing2 {
@safe const:
    static void func() {
    }
    
    unittest {
    }
}

struct Thing3 {
@safe immutable:
    static void func() {
    }
    
    unittest {
    }
}

struct Thing4 {
@safe shared:
    static void func() {
    }
    
    unittest {
    }
}

@dlang-bot dlang-bot merged commit b6587ba into dlang:master Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dip1000 memory safety with scope, ref, return Merge:auto-merge Severity:Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants