Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Conversation

@JinShil
Copy link
Contributor

@JinShil JinShil commented Jul 16, 2019

About This PR

Followup to #2676 and dlang/phobos#7110

This is the first of several PRs I intend to submit, breaking up #2677 into multiple PRs.

This PR only addresses C language bindings in core.stdc. Since C does not have any concept of scoped pointers, I think it is arguably a mistake for these to be in.

Background

This PR is in support of dlang/dmd#10179

in as a parameter storage class is defined as scope const. However in has not yet
been properly implemented so its current implementation is equivalent to const. Properly
implementing in now will likely break code, so it is recommended to avoid using in, and
explicitly use const or scope const instead, until in is properly implemented.

The use of in as a parameter storage class is already discouraged in the documentation. See https://dlang.org/spec/function.html#parameters

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @JinShil! 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

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 + druntime#2680"

@thewilsonator
Copy link
Contributor

I think it is arguably a mistake for these to be in.

But none of those function leak the pointers given, which is the whole point of scope.

@JinShil
Copy link
Contributor Author

JinShil commented Jul 16, 2019

But none of those function leak the pointers given, which is the whole point of scope.

Hopefully they don't, but C has no way to express such a thing and no way to statically enforce it, so why would we assume that that's the case unless we were to manually audit the C code?

@thewilsonator
Copy link
Contributor

but C has no way to express such a thing

The C standard.

(From memory the only function that is obviously not scope is getopt, as it sets optarg.)

no way to statically enforce it. so why would we assume that that's the case unless we were to manually audit the C code?

Because this isn't some random C library downloaded from some dark and dodgy corner of the internet, this is the C standard library we're talking about. Its only dependance in on the hardware and the OS system calls. If its buggy, malicious, swapped out with LD_PRELOAD, then a) there is nothing we can do to protect against that, and b) you have much more pressing concerns, namely that your C standard lib is dud. This is a pragmatism vs purism issue, and I'm firmly for the pragmatism. The threat model of having a malfunctioning C standard library is not one we should be concerning ourselves with.

@JinShil JinShil changed the title Replace in with const in core.stdc Replace in with const scope in core.stdc Jul 17, 2019
@JinShil
Copy link
Contributor Author

JinShil commented Jul 17, 2019

Updated to use const scope now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants