discussions-and-proposals
discussions-and-proposals copied to clipboard
Adding HTTPS connection to bundle for Android
Introduction
We're building a React Native hosting service that allow external developers to access the pre-built engine to make their own stuff.
This requires that the native app connect to custom hostname and ports
Details
In iOS, we're able to connect to a React Native hosting via HTTPS and WSS. Problem is with Android, there is no way i can figure out to do that for now.
The only way in Android at the moment was to set the "debug_http_host", which can change the "hostname" and the "port".
The problem is in Android it will always try to point to "HTTP" in the DevServerHelper.
Discussion points
- Support Android for being able to connect to HTTPS, instead of only HTTP.
Just my 2¢, but I think that the shorthand for derive(..., unsafe(Trait), ...) is a bit unprecedented.
You have to separate out derive traits any time there's some different requirement, e.g. cfg_attr(feature = "serde", derive(Serialize, Deserialize)), and by keeping unsafe at the top level you can easily grep for #!?\[unsafe\( whereas unsafe( will catch any call to method_unsafe(.
Sure, it's likely it won't make a difference, but I think that only having to check attributes at the top level for unsafe to verify safety is best.
The greppability is already broken by things like #[cfg_attr(all(), unsafe(no_mangle)], and can be restored by just grepping unsafe( I think putting the unsafe with the trait makes syntactic sense as it discharges the safety obligation of each trait derive.
Any reason the syntax for defining an unsafe derive/attribute isn't unsafe fn?
#[proc_macro_attribute]
pub unsafe fn dangerous(_attr: TokenStream, item: TokenStream) -> TokenStream {
item
}
because that makes no sense, the unsafety is at a meta-level and there are no safety requirements on the macro function itself
Is there not? I thought the point of an unsafe derive/attribute was that it was unsafe to trigger and had safety requirements?
The greppability is already broken by things like
#[cfg_attr(all(), unsafe(no_mangle)], and can be restored by just greppingunsafe(I think putting the unsafe with the trait makes syntactic sense as it discharges the safety obligation of each trait derive.
I feel kind of silly for literally alluding to this point in my post and missing it somehow. You're right and I retract my original claim.
We had a @rust-lang/lang design meeting today on the set of macro RFCs. I've updated the RFC to incorporate the feedback from that design meeting.
Per the feedback in that meeting, I'm starting an FCP to start letting people register consensus for this RFC.
@rfcbot merge
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:
- [x] @joshtriplett
- [ ] @nikomatsakis
- [ ] @pnkfelix
- [ ] @scottmcm
- [ ] @tmandry
- [ ] @traviscross
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. See this document for info about what commands tagged team members can give me.
@rust-lang/lang I've updated this to match the consensus syntax of derive(unsafe(UnsafeTrait)), and provided all the points of rationale that I remember from the discussion. Please feel free to post further suggestions if I missed any points of rationale.
One thing I thought of was that it might make sense to make a derive maybe-unsafe and branch between whether unsafe was provided, although it might be better to just spell that out as 2 separate derives... The use case for that would be something where the derive can check the preconditions using static_asserts or similar, and one where you can promise the preconditions hold if they aren't checkable.
One thing I thought of was that it might make sense to make a derive maybe-unsafe and branch between whether unsafe was provided
I think that would be a big mistake. The presence or absence of unsafe should never affect the resulting generated code, it should only affect whether the code is accepted or not.
@rust-lang/lang I've updated this to match the consensus syntax of
derive(unsafe(UnsafeTrait)), and provided all the points of rationale that I remember from the discussion. Please feel free to post further suggestions if I missed any points of rationale.
In terms of the mental model I find derive(unsafe(Trait)) a bit odd -- this looks like the trait is called unsafe(Trait). In the grammar of attributes (well, in its simplified form in my head), unsafe($attr) is also an $attr, and derive($trait) is an $attr, but to fit the proposed scheme into that model we have to do an ad-hoc extension of the grammar to allow unsafe inside derive.
OTOH, the RFC does make some good points in favor of "inner unsafe", so I am a bit torn here.
In terms of the mental model I find
derive(unsafe(Trait))a bit odd -- this looks like the trait is calledunsafe(Trait).
The argument we landed on, on this point (setting aside the ergonomic arguments in favor of the inner unsafe), is that the derive word is always safe. It's just signaling that we're going to invoke a code transformation. It's the derive macro that is unsafe. That's what has the proof obligation. And that's what the path represents.
Sometimes those derive macro names correspond directly to a trait, but sometimes they don't (e.g. CoercePointee). So in derive(unsafe(MyDerive)), it's the derive macro that's unsafe, not necessarily any trait that shares the same name.
@traviscross Thanks for the reminder. IIRC that was one of many possible rationales that came up in the meeting, but it's one that I hadn't captured. I've now done so.
The argument we landed on, on this point (setting aside the ergonomic arguments in favor of the inner unsafe), is that the derive word is always safe. It's just signaling that we're going to invoke a code transformation. It's the derive macro that is unsafe. That's what has the proof obligation. And that's what the path represents.
I don't understand that distinction. no_mangle also just invokes a particular code generation. But that generated code can lead to unsoundness and that's what makes in unsafe. This applies equally to derive.
Or put differently, a function invoking an unsafe function without checking its preconditions needs to be unsafe.
But good point that the name in the derive is a macro name, not a trait name. That had temporarily escaped my memory.
I don't understand that distinction...
That distinction is tied exactly to:
But good point that the name in the
deriveis a macro name, not a trait name.
That is, I agree that if derive were itself doing a code transformation producing obligations for the user to uphold, that it should be unsafe. But it's actually the derive macro that's doing that. So derive(..) is kind of just a context marker that allows us to invoke one or more derive macros.
Putting that all together, we could say that, within attributes, we have to be in a derive(..) context to invoke derive macros, some of which might be unsafe, and those have to be so marked.
Let's cancel the stale FCP and restart...
@rfcbot fcp cancel
@traviscross proposal cancelled.
This all looks correct and desirable to me with one amendment, which I'm including in this proposed FCP, which is that we change the def site for derive macros to match the use site, and write the unsafe(..) around the derive macro name in the proc_macro_derive attribute, e.g.:
#[proc_macro_derive(unsafe(DangerousDeriveMacro))]
pub fn derive_helper_attr(_item: TokenStream) -> TokenStream {
TokenStream::new()
}
(For me, that the use site syntax we chose allows for this symmetry is yet another reason to prefer it.)
@rfcbot fcp merge
Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:
- [x] @joshtriplett
- [x] @nikomatsakis
- [ ] @scottmcm
- [ ] @tmandry
- [x] @traviscross
Concerns:
- needs fleshed out use case (https://github.com/rust-lang/rfcs/pull/3715#issuecomment-2815798055)
- ~~pick-def-site-name~~ resolved by https://github.com/rust-lang/rfcs/pull/3715#issuecomment-2810241277
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. See this document for info about what commands tagged team members can give me.
@traviscross As mentioned by @carbotaniuman and myself in response to that suggestion: that makes it look like something that's unsafe to define rather than unsafe to use. For that reason, I don't think that syntax is an improvement.
We've already set the precedent, with unsafe attributes, that unsafe(...) is what an unsafe block looks like in an attribute context.
I could imagine making the case that this should be written proc_macro_derive(unsafe DangerousDeriveMacro) (by analogy with unsafe fn), but not proc_macro_derive(unsafe(DangerousDeriveMacro)).
I've listed an alternative for this, along with rationale for why we shouldn't go with that alternative.
Let's use a concern to mark the discussion we need to have on def-site syntax.
@rfcbot concern pick-def-site-name
(Edit: I meant to type pick-def-site-syntax, but it's OK.)
We'll need to discuss it on a lang call regardless, and I'm rather curious to hear what others on the team think. While I'm thinking about it, though, here are my notes on the case for symmetry between the def site and the use site. (@joshtriplett and I talked these points over, and our interesting and fruitful discussion helped to sharpen them for this quick write-up. These are my views only though.)
First off, I agree that it's a bit of a pun, but I think it's a kind of attractive pun in this case. What pushes me over the line is that I just can't imagine that, having picked unsafe(Macro) at the use site, that we'd ever use unsafe(Macro) at the def site to mean something else. If I thought we were seriously considering some alternative future meaning for that, I'd raise a concern about using unsafe(Macro) at the use site.
Broadly, I think it's fine, given how things are today, if define(unsafe(Thing)) creates an obligation and use(unsafe(Thing)) discharges it. If we're unhappy with that, probably the problem is the reuse of the keyword, not the reuse of how that keyword is placed. One thing that makes me happy with it here is that I just don't see any other plausible meaning for define(unsafe(Thing)). The Thing doesn't exist yet -- we're defining it. There's no way we could be discharging an obligation for it yet. So we must be creating an obligation, just like we are in other places that we're defining something using unsafe, e.g. in unsafe fn.
I'm reminded here too about our recent syntax discussions for const trait impls. We may not end up marking them at all -- that's what the current RFC draft says -- but when we were thinking of doing that, we had to pick between const impl Trait for .. and impl const Trait for ... There are a lot of good reasons to like const impl Trait, e.g. in terms of syntactic and conceptual parity with const fn and other uses. But people really liked impl const Trait for .. because it matched the T: const Trait bound syntax. We could say here too that this is just a pun as these are different. One "requires a const impl" and the other "provides a const impl". And yet, it seemed attractive to lean into this pun. That's roughly similar to how I feel here, mapped to "discharging a proof obligation" and "creating a proof obligation".
(The parity we try to maintain between our type syntax and constructor expression syntax is maybe also a place where we could say that it's a bit of a pun between two distinct things, but that we like it.)
Another angle is to imagine what we'd do if we didn't have this feature. Probably we'd still mark these unsafe derive macros by naming them things like UnsafeThing. Then the def site would look like define(UnsafeThing) and the use site like use(UnsafeThing). So in that way, define(unsafe(Thing)) and use(unsafe(Thing)) feel equally fine to me. It's like we're just binding the unsafety to the identity of the derive macro.
(Here and throughout, I'm using define(..) and use(..) to abstract the question a bit, and to avoid having to repeat proc_macro_derive(..) everywhere.)
Regarding the fact that unsafe(..) in attributes today only discharges obligations, what I'd point out is that we only very recently shipped unsafe attributes at all, so I don't assign much in the way of precedent value. It doesn't surprise me one bit that we started by only using the syntax one way, and I don't think that precludes us from using it in another way, in a different context, if it's appropriate, well motivated, and sufficiently unambiguous.
One reason that I proposed this change is that putting the unsafe keyword somewhere arbitrary in an argument list -- as with define(Thing, attributes(..), unsafe) -- just doesn't speak to me strongly enough that we're creating an unsafe obligation. Everywhere else that we do this, it comes in front and is paired closely with the name of what we're defining that carries that obligation. So probably I'd like to see us address that somehow. I had thought too about define(unsafe Thing), and that'd be OK if others don't like the proposed pun, but the arguments above cause me to still lean more toward the symmetric answer.
We're already using unsafe both to mark the point of introducing unsafe obligations and of discharging them. This is a point that often trips people up (it is on the short list of things I would change about Rust 1.0 if I could). For the existing cases at least there's fairly short list of you have to learn to know which is which. The proposal to use unsafe(Trait) for both cases means that in the future one will have to know exactly how all attributes use unsafe to understand which of them introduces vs discharges obligations -- that does not seem great.
it just occurred to me that we may want to mark derive macro helper attributes unsafe instead of (or in addition to) the derive macro:
#[proc_macro_derive(MyDebug, attributes(unsafe(debug_deref)))]
fn my_debug ...
// no `unsafe(MyDebug)` here since it's only unsafe
// when you need to deref pointers, which not all structs have.
#[derive(MyDebug)]
pub struct FooPtr<T> {
// SAFETY: ptr points to a valid memory chunk allocated with `malloc`
#[unsafe(debug_deref)]
ptr: NonNull<T>,
bar: usize,
}
Excellent point. We'll take that up with the nomination as well.
The motivation section is really short. Even though this seems like an "obvious" extension, I would appreciate it if @joshtriplett and others could add concrete examples of derives that would use this. Until then further bikeshedding on the spelling seems premature.
@programmerjake That exact proposal is in the future possibilities section:
We could provide a syntax to declare specific helper attributes of a derive as unsafe, without declaring the entire derive unsafe.
it just occurred to me that we may want to mark derive macro helper attributes
unsafeinstead of (or in addition to) the derive macro:[...snip]
I'd go even further: the same attribute could be either safe or unsafe, depending on other attributes of the same macro, so there should be a way to accept both #[unsafe(debug_deref)] and #[debug_deref].
@moulins
it just occurred to me that we may want to mark derive macro helper attributes
unsafeinstead of (or in addition to) the derive macro: [...snip]I'd go even further: the same attribute could be either safe or
unsafe, depending on other attributes of the same macro, so there should be a way to accept both#[unsafe(debug_deref)]and#[debug_deref].
We could certainly do that, but I think it's reasonable for that to be future work. This proposal introduces always-unsafe derives and always-unsafe attributes. In the future, we could have an RFC for sometimes-unsafe derives and sometimes-unsafe attributes, which would require additional definition and documentation.