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

RFC: Statics in patterns #3305

Closed

Conversation

Jules-Bertholet
Copy link
Contributor

@Jules-Bertholet Jules-Bertholet commented Aug 17, 2022

Rendered

Proposes allowing patterns to refer to statics, including trusted extern statics.

static NUMBER: i32 = 42;

fn foo(scrutinee: i32) {
    match scrutinee {
        NUMBER => println!("yes"),
        _ => println!("no"),
    }
}

Internals thread
Full implementation

@ehuss ehuss added the T-lang Relevant to the language team, which will review and decide on the RFC. label Aug 17, 2022
}
```

Mutable statics are not allowed, however. Patterns can't reference information that can change at runtime, and also can't be `unsafe`.
Copy link
Member

Choose a reason for hiding this comment

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

Ignoring the unsafe issue, is there a reason that the pattern can't reference information that can change at runtime? My understanding of how cosmopolitan is going to use this is that it references information that changes at runtime (but only once, before main), the values of the statics are not available at link time like they normally are. Given that why would it not be possible for a developer to do the same thing using static mut where they set the value once early in main before the pattern is ever tested against.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a reason that the pattern can't reference information that can change at runtime?

The "patterns can't reference information that can change at runtime" rule is not something that I am proposing, it's a rule that Rust already has and that I see no reason to change. Tearing down this Chesterton's Fence may or may not be a good idea, but is out of scope for this RFC.

My understanding of how cosmopolitan is going to use this is that it references information that changes at runtime (but only once, before main)

This is where the exact definition of "at runtime" comes into play. If you define "runtime" as "the time when Rust code runs", then the values of the statics don't change at runtime. I argue that for the purposes of this feature, it doesn't really matter whether the values of the statics are set by the linker, the operating system, the C runtime, etc; all of these things fall into the common category of "rustc doesn't know what the value is, but Rust code can rely on the value being constant." Note that Cosmopolitan sets the values of the statics in assembly (that I don't presume to fully understand); C code written against Cosmopolitan sees them as standard C extern consts, and doesn't need to worry about their value changing, either.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fundamentally this is how all rust static values work. Officially Rust "doesn't have life before main", but it still requires someone (an OS or an assembly boot script) to initialize all the pre-conditions before Rust code actually begins executing.

So this part of the RFC is basically nothing new.

Copy link
Contributor Author

@Jules-Bertholet Jules-Bertholet Aug 17, 2022

Choose a reason for hiding this comment

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

I feel a good analogy is dynamic linking. When you call a dynamically linked function in Rust, the compiler doesn't know the address of that function; that only gets determined when the program runs. But you can still write a safe wrapper for the dynamically linked function, and then treat it pretty much like any other Rust function (though not like a const fn, of course).

Extern statics are the same. Unlike consts, rustc doesn't know their value. But, once the necessary #[trusted_static] ceremonies are performed, it should still be possible (IMO) to treat them"pretty much" like any other constant value.

Copy link
Member

Choose a reason for hiding this comment

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

Fundamentally this is how all rust static values work.

Not normally, the vast majority of Rust code is statically linked which means the values are known at link time (and LTO would then be able to make const and static patterns generate the same code).

Extern statics are the same. Unlike consts, rustc doesn't know their value. But, once the necessary #[trusted_static] ceremonies are performed, it should still be possible (IMO) to treat them"pretty much" like any other constant value.

Would it be unsound to use #[trusted_static] on a dynamically linked extern static that gets initialized early-in-main before any code that reads the static runs? It just seems like it should be possible to unsafely promise that a value is constant once it starts being used in patterns, given that is essentially the same behavior you get with dynamically linked statics (where the dynamic loader has unsafely promised to the executable that all statics have been initialized and will no longer be written to).

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh whoops, I'm the one in charge of trusted statics? That kinda stalled out because of like, long term correctness discussion XD

uhm, don't expect that one soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That kinda stalled out because of like, long term correctness discussion

Is there a list of the unresolved questions somewhere? (Or does one need to be compiled?) Might it be possible to ship an obviously correct MVP sooner, and consider loosening safety requirements later?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just the zulip thread, which the project tracking issue should link to.

The problem is really simple: It's hard to tell people that the unsafe-adjacent code they've been writing the past 6 years is just barely wrong, and that they need to start doing everything the opposite way because of small edge cases. That's a hard sell.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's hard to tell people that the unsafe-adjacent code they've been writing the past 6 years is just barely wrong, and that they need to start doing everything the opposite way because of small edge cases.

Could we figure out the shiny new Right Way of Doing Things ® now, worry about migrating/accommodating old code later once people are used to the new syntax? The longer we wait before offering better tools, the more technically-bad code gets written, and the more painful any necessary changes will be.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it's easy if migration of code and re-training of programmers isn't a concern ;3

Literally we just would have to say one day "hey declaring extern static/fn is unsafe now". And like, a compiler patch to enforce that. That's the entire end goal for Rust 3015. It's just the migration that needs to be designed really.

@ExpHP
Copy link

ExpHP commented Aug 20, 2022

I was skeptical in the internals thread on the basis of complexity of implementation, but the inclusion of a full implementation appears to have proven me wrong. Good job!

I guess there's only a "conceptual burden" now in that a refutable pattern can also be opaque.

Correct me if I'm wrong, but I took a peek at the test cases and didn't notice any where the same static is used twice, e.g.

match x {
    A => 1,
    A => 2,
    _ => 3,
}

(I was looking for this because I was wondering if it would have a lint)

@Jules-Bertholet
Copy link
Contributor Author

I took a peek at the test cases and didn't notice any where the same static is used twice.

I had overlooked that case. Rectified in Jules-Bertholet/rust@46bff83

@clarfonthey
Copy link
Contributor

100% in favour of this change, although I think there's one extra detail that should be addressed: to what extent are nested patterns allowed? I think we'd agree that Some(STATIC) should be allowed, but is STATIC..=OTHER_STATIC allowed? What about STATIC | OTHER_STATIC?

I personally think that all of these are fine, but we probably want to be explicit about it. I'm not sure if the provided implementation allows for all these kinds of patterns, or just plain static patterns.

@Jules-Bertholet
Copy link
Contributor Author

Jules-Bertholet commented Aug 21, 2022

@clarfonthey Some(STATIC), STATIC | OTHER_STATIC, and similar work fine. There is an example of or-patterns in the RFC, but I should probably be more explicit about that. (Edit: fixed)

Statics in range patterns (STATIC..=OTHER_STATIC) currently ICE; I forgot about that case (whoops). One wrinkle is that the compiler currently enforces that range patterns must be non-empty. So we would need to either relax that restriction, allow statics in range patterns only when the other end is the minimum or maximum value for the type, or just not allow statics in range patterns at all. (Edit: added to "unresolved questions")

@clarfonthey
Copy link
Contributor

Wasn't aware that empty range patterns was a hard requirement for the compiler; I would have thought that it would be marked as unused like uninhabited types are, but not be a hard error.

Perhaps statics should have a similar treatment to uninhabited types, where empty ranges simply never match, but are still allowed. But it's fair to leave as an open question at the moment.

I can definitely imagine ranges of the form STATIC_MIN..=STATIC_MAX being useful.

@RalfJung
Copy link
Member

The summary kind of buries the lede by talking about regular static when the main motivation seems to be extern static. Those are very different beasts, so even if we allowed static in patterns, I would not have expected us to ever support extern static.

That's not an objection to supporting them, but the RFC title and summary should state this to make the scope of the proposal clear from the beginning.

@Lokathor
Copy link
Contributor

@RalfJung Could you elaborate a little more on why they're so different in your mind?

@RalfJung
Copy link
Member

The values of (immutable) static are known at compile time. The values of extern static are not.

- not come from an extern block, unless it is marked as safe to use with the [trusted external statics](https://github.com/rust-lang/lang-team/issues/149) feature
- have a type that satisfies the structural match rules, as described in [RFC 1445](1445-restrict-constants-in-patterns.md), but without any allowances for backward compatibility like there are for consts (e.g., floating point numbers in patterns) . These rules exclude all statics with interior mutability.

Static patterns match exactly when a const pattern with a const of the same type and value would match.
Copy link
Member

@RalfJung RalfJung Aug 23, 2022

Choose a reason for hiding this comment

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

This is a very subtle guarantee, since the way these patterns are treated will have to be completely different. Matching consts relies heavily on knowing their value. const will be taken apart to a 'valtree' that is matched on field-by-field, as least if the type permits, whereas extern static will necessarily be compared via PartialEq. This may or may not produce the same result, depending on how PartialEq is implemented.

Above you reference the structural match RFC, but the rules in that RFC have little to do with what the compiler actually does. The real rules are quite the mess and need to be cleaned up, see rust-lang/rust#74446. But it seems inevitable that at least in unsafe code it will be possible to have "structural match" types that actually violate the structural match contract, and we need to say what happens then. This is not an operational requirement, so it probably cannot become UB.

So, I think guaranteeing they behave exactly the same is not feasible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If future relaxations of the structural match requirements are somehow impossible to port to static matches, we can always require the stricter rules for static patterns while loosening them everywhere else. It's impossible to say more without knowing what such relaxations might look like.

Copy link
Member

Choose a reason for hiding this comment

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

"Future relaxation" makes it sound like we currently have some rules that we understand, and we want to change them. That is not the case. As I said, what the compiler does and what the RFC says are already different, and last time this came up, there was little interest in adjusting the compiler to the spec -- rather, the spec will be adjusted to the compiler.

So, you simply cannot use that RFC as a basis for further language evolution.

Copy link
Contributor Author

@Jules-Bertholet Jules-Bertholet Aug 24, 2022

Choose a reason for hiding this comment

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

When I referred to the structural match RFC, I was mostly concerned about the guarantee (as of right now) that, for any local x and const Y of the same type, match x { Y => true, _ => false } evaluates to true iff PartialEq::eq(&x, &Y) evaluates to true. Do you know of a counter-example to this that works in stable Rust?

Copy link
Member

@RalfJung RalfJung Aug 24, 2022

Choose a reason for hiding this comment

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

We have patterns where PartialEq::eq(&x, &Y) doesn't even compile, see e.g. this example from the issue that I linked twice already (rust-lang/rust#74446).

Also your stated equivalence is insufficient to substantiate the claim in the RFC since it fails to take into account UB. There are some fundamental reasoning principles for patterns that are currently true, that are no longer true with this RFC -- as explained here.

Copy link
Contributor Author

@Jules-Bertholet Jules-Bertholet Aug 25, 2022

Choose a reason for hiding this comment

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

We have patterns where PartialEq::eq(&x, &Y) doesn't even compile

OK, I definitely need to account for that then. But are there cases where it does compile, but gives a different answer?

(Also, is "comparing function pointers is weird" just a potentially-fixable LLVM quirk, or is there some deep reason behind it?)

Copy link
Member

Choose a reason for hiding this comment

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

OK, I definitely need to account for that then. But are there cases where it does compile, but gives a different answer?

No, I don't think so. But I already explained why that is not sufficient grounds to claim that static and const matching are equivalent.

(Also, is "comparing function pointers is wierd" just a potentially-fixable LLVM quirk, or is there some deep reason behind it?)

It's because of unnamed_addr, see e.g. rust-lang/rust#70861

}
```

As an exception, when all safe values of a type are structurally equal, the compiler is allowed to see that the match will always succeed.
Copy link
Member

Choose a reason for hiding this comment

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

"all safe values" is definitely impossible to implement. I could define

// Safety invariant: the field is always 0.
struct MyWeirdUnit(i32);

MyWeirdUnit has only one safe value, but the compiler has no way of knowing that.

Maybe you meant "all valid values", but even that seems way too subtle to include here. Do we have any precedence for a rule like that?

Copy link
Member

Choose a reason for hiding this comment

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

Also, "is allowed to see" is too weak; I assume you mean that the compiler must accept the code below, not just that it is allowed to accept the code below.

Copy link
Member

@RalfJung RalfJung Aug 23, 2022

Choose a reason for hiding this comment

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

And finally, given that this is a large fraction of the RFC, there also needs to be motivation for all this complexity caused by "1-valued types". The motivating example certainly doesn't need any of this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I basically interpreted this as exclusively applying to ZSTs, but wanting to be more general in case we somehow added the ability to increase the number of cases we could detect this.

Copy link
Contributor

Choose a reason for hiding this comment

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

For example, is isn't possible now, but it eventually could be possible to mark fields as ignored when deriving Eq, which would allow such a type to be non-zero-sized while still having a zero-sized equivalence class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this should be "valid". I had a reason in my head for why this should be "safe", but upon writing it out I realized that my logic made no sense.

As stated in the RFC, there's no practical use-case (AFAIK) for the "one valid value" rule. It's just that, on general principle, I feel the compiler should never force you to write useless code that it statically knows to be unreachable, unless there's a good justification for it.

@Lokathor
Copy link
Contributor

I guess the concern is that the match using a static pattern would look like a similar match using a const pattern while acting like something very different.

Could some sort of macro situation solve this within libc and the select other places that need to deal with this? Then it will "look like" a match inside a macro (and we all know "macros can do weird stuff"), and then on select systems that macro can expand to the if-else form instead? And we don't have to disrupt the language for the rest of everyone else.


[reference-level-explanation]: #reference-level-explanation

For a static to be eligible for use in a pattern, it must:
Copy link
Member

@RalfJung RalfJung Aug 23, 2022

Choose a reason for hiding this comment

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

I am not sure if this is enough for soundness. The reason const are not allowed to even point to immutable static is pattern matching soundness: if you have a const of type &i32, we better make sure that the integer it points to is truly immutable. But if you define a static X: &i32 by doing &mut MUTABLE_STATIC and then transmuting that, and then you do const C: &i32 = X, now you have yourself a potentially unsound pattern.

But for extern static we don't know their values, so we can't know if they do shenanigans like that. Also for all we know their type may be a lie, so maybe they actually do get mutated? There is an entire lion's den of issues here that we have so far worked very hard to avoid (e.g. grep the compiler source code for can_access_statics and read all the comments associated with it), and all of this work is for naught if we allow extern static in patterns.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I hope we can fix this in a future edition

Copy link
Member

Choose a reason for hiding this comment

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

Not sure this needs a new edition, we just need to be okay with semver hazards and post-monomorphization errors.

But anyway, that won't help for extern static. I can see some ways of resolving the structural match situation that would probably also make other kinds of "opaque consts" like extern static be fine (*), but as of now I don't know where the structural match stuff will go so I'd rather not rely on it going any particular way.

(*) Well, there are also the issues around "evading match arms" by changing the pattern mid-evaluation, but that can probably be avoided by not doing any exhaustiveness checking and clearly documenting when opaque patterns vs valtree patterns are used (because they will probably not always be equivalent).

Copy link
Contributor

@Lokathor Lokathor Aug 23, 2022

Choose a reason for hiding this comment

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

Sorry, I meant specifically that with extern static (non-mut) I would like declaring them to be the unsafe part, and it would require precise type and truly immutable memory and such, then the accesses themselves can be the safe part and people can worry a lot less.

Doesn't help the actual valtree matching (or not) part of it though.

Copy link
Member

Choose a reason for hiding this comment

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

The condition we need here is that any pointer we reach through this memory points to immutable memory, recursively. This is required for matching on references. It is why I keep saying this is a bigger soundness issue than the RFC lets on.

I doubt trusted statics want to require that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but if that value can be valtree'd, then those two things are not UB-equivalent.

How? The current stuctural match rules oly allow constants (and, with this feature, statics) if PartialEq::eq is guaranteed to give the same results as a valtree-based match.

Copy link
Member

Choose a reason for hiding this comment

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

Mutating the value referenced by a &&&i32 is itself UB imho,

That's probably not correct. You certainly shouldn't rely on it. The rules for UB on references will probably not recursively follow references to arbitrary depth, but instead work on a 'shallow' level -- mutation is only UB one level down.

As a concrete example, Miri sees no UB here.

How? The current stuctural match rules oly allow constants (and, with this feature, statics) if PartialEq::eq is guaranteed to give the same results as a valtree-based match.

The current rules are a mess and before anyone relies on them for anything, we need to re-do those rules. See rust-lang/rust#74446.

The future rules will, hopefully, give users the ability to unsafe impl StructuralEq for MyType. (And in fact they can already do that, even without unsafe, by using unstable features.)

Users of course cannot expect things to behave completely sanely when they do that, but we still need to specify precisely what happens. Note in particular that saying "it is UB to have such an instance" does not work, that's simply not how we are using UB in Rust -- UB can only arise as a consequence of the program executing a statement in the wrong way, and only under conditions that can actually be decided automatically. "This type has a PartialEq that behavesd equivalently to valtrees" is not a decidable question (halting problem, yada yada), and hence cannot be grounds for UB.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"This type has a PartialEq that behaves equivalently to valtrees" is not a decidable question

But "at the point this specific match statement was executed, this specific call to PartialEq::eq completed and returned a value without panicking, and that return value was different from what a valtree-based match would have returned" is decidable, no?

(This would mean loop {} is a valid PartialEq::eq impl, but I don't think that's a problem).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mutating the value referenced by a &&&i32 is itself UB imho,
That's probably not correct. You certainly shouldn't rely on it.

All the issues that arise with "a static of type &&&i32 could change during a pattern match" already occur with "a scrutinee of type &&&i32 could change during a pattern match", no?

Copy link
Member

Choose a reason for hiding this comment

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

But "at the point this specific match statement was executed, this specific call to PartialEq::eq completed and returned a value without panicking, and that return value was different from what a valtree-based match would have returned" is decidable, no?

So now we need a way to express that in MIR, and carefully specify exactly what that does and when.

That might be possible, but it'll be a lot of work, and it'll also make the core language spec more complicated for a niche feature.

Instead I think we should just specify that statics are treated as opaque for pattern matching and always use PartialEq no matter their type. We don't have any other "UB due to valtree inconsistencies", and IMO there is no good reason to introduce such UB here.

All the issues that arise with "a static of type &&&i32 could change during a pattern match" already occur with "a scrutinee of type &&&i32 could change during a pattern match", no?

No, patterns and scrutinees are treated very differently in pattern matching, because the pattern is inherently known at compile-time and the scrutinee isn't. This is not a symmetric situation. This RFC is proposing to make it more symmetric, but that is exactly why this is a bug fundamental change to pattern matching.

See here for a long discussion about how to ensure that we have UB when the scrutinee changes during pattern matching. The same approach does not obviously work for patterns, since it relies heavily on the fact that we compare the same value X against a number of things -- Rust has some ingredients that make it possible to ensure that X doesn't change when it is used multiple times. But with patterns that just happen to use the same extern static in several places across several patterns, we have none of that structure, so it'd be even more complicated.

So, again, we should just specify that extern statics are treated as opaque, and we should make no claims that this is equivalent to a const of the same type and value.

@Jules-Bertholet
Copy link
Contributor Author

Jules-Bertholet commented Aug 24, 2022

I guess the concern is that the match using a static pattern would look like a similar match using a const pattern while acting like something very different.

The intention is that, while under the hood the implementation may be different, the two types of patterns should behave exactly the same (except for exhaustiveness/reachability checking and usage in const contexts).

@Jules-Bertholet
Copy link
Contributor Author

The summary kind of buries the lede by talking about regular static when the main motivation seems to be extern static. Those are very different beasts

This RFC takes it as a given that regular and trusted extern statics should not be different; in fact, they should be 100% interchangeable in all cases. Maybe I should be more explicit about justifying this.

@RalfJung
Copy link
Member

This RFC takes it as a given that regular and trusted extern statics should not be different; in fact, they should be 100% interchangeable in all cases.

That assumption is just wrong though. For regular statics we know their value at compile-time, for trusted extern statics we don't.

@RalfJung
Copy link
Member

RalfJung commented Aug 24, 2022

The intention is that, while under the hood the implementation may be different, the two types of patterns should behave exactly the same (except for exhaustiveness/reachability checking and usage in const contexts).

Yeah but that's already not the case in your implementation. A const of certain types will be turned into a structured, tree-like pattern representation (in the future, a valtree); that is inherently impossible for a static, so it uses the 'fallback' path of being treated opaquely. At least in my plans, and also from what I have understood of @lcnr's plans, there is no constraint that these two ways of matching are 100% equivalent -- rather, we have a trait that lets users select which of these approaches to use for their type, and the spec always says we use this way or that way.

So this RFC attempts to put a constraint on the pattern matching machinery that at least I explicitly intended not to have. Not having this constraint means users can unsafely declare their own types to be structural-match, which is broadly useful. (And we can do so without having super complicated machinery whose only purpose it is to declare code that violates certain expectations to be UB. And to what benefit? UB should be avoided whenever possible, basically the only acceptable justification is that we can get optimizations out of it, or that allowing such code would make the spec enormously complicated and hard to understand. Neither of these apply here.) Having this constraint enables a niche RFC to have a slightly simpler spec. I don't think that's a hard choice to make...

Also note that I am not saying we should encourage anyone to have strange sturctural eq types like that! We should certainly establish a general expectation that structural eq types have a PartialEq that behaves like comparing their valtrees. And for such well-behaved types, the equivalence you claim is correct. But the key point is that violating this expectation should not be UB! It's just a logic bug, similar to having a PartialEq that is not transitive. And therefore, when specifying pattern matching, we have to assume that the two ways of matching are not equivalent, but when teaching pattern matching, we can still say that they generally are equivalent (and if not, someone wrote a bad unsafe impl StructuralEq for MyType).

So it's a trivial change for this RFC -- just say that statics are always treated opaquely and compared with PartialEq, even if a const of the same type would be treated differently.

@Jules-Bertholet
Copy link
Contributor Author

Jules-Bertholet commented Aug 24, 2022

That assumption is just wrong though. For regular statics we know their value at compile-time, for trusted extern statics we don't.

We may know the value, but currently we don't take advantage of that fact to allow non-extern statics to do special things no other static can. I believe we should continue not doing that. We should take our special compile-time knowledge and forget about it. At most it should be used to inform lints.

Edit: I was wrong. The following works today:

static IN: i32 = 5;
static OUT: i32 = IN + 3;

fn main() {
    println!("{OUT}")
}

@Jules-Bertholet
Copy link
Contributor Author

So it's a trivial change for this RFC -- just say that statics are always treated opaquely and compared with PartialEq, even if a const of the same type would be treated differently.

As of right now, these two comparison methods are always 100% equivalent for all eligible types. Maybe some future hypothetical might-never-happen still-completely-unspecified language extension will change that. But if I have to fully specify how this feature might interact with every imaginable hypothetical future language extension, this RFC will end up with infinite length. So I would rather say "this feature does not prevent future extensions to structural match/match in patterns" and leave it at that, rather than slide down endless rabbit holes.

@nikomatsakis
Copy link
Contributor

I skimmed the RFC and the discussion. As it stands, my inclination is to close it (though I thank the author for the submission).

To start, the motivation as presented feels a bit weak. Matching against constants feels like it could be well-solved with a macro as well, where the same code is compiled to a match on one operating system but a if-else-if chain on others. I'd be more convinced if there were a lot of examples of where this would be useful that included more mainstream scenarios.

Secondly, I don't think we're ready to make this decision yet. As @RalfJung noted, this question feels highly entangled with valtree and how match semantics shake out. I don't feel we can accept this RFC until we've settled that story definitively (side note that I would really love to see progress there). Also, I may be misremembering, but my sense is that we are moving in the direction of more compilation time reasoning in matches and not less, and thus I think this RFC would wind up being a bad fit overall.

@Jules-Bertholet
Copy link
Contributor Author

Jules-Bertholet commented Aug 31, 2022

Secondly, I don't think we're ready to make this decision yet. As @RalfJung noted, this question feels highly entangled with valtree and how match semantics shake out. I don't feel we can accept this RFC until we've settled that story definitively

That's fair. I agree that the use-cases for this feature are relatively niche. With the current compiler, the spec and implementation are also relatively simple, which is why I felt this RFC to still be worth the trouble. But if future changes to the match code are going to potentially affect that, then the motivation for this RFC is no longer as compelling. Maybe once those issues are settled, this can be revistied?

@Jules-Bertholet
Copy link
Contributor Author

That being said, I don't agree that a macro is a sufficient general solution to the problem that motivated this RFC. Within the standard library it might be good enough, but if crates like nix and rustix want to support Cosmopolitan, they will have to adopt the macro as well. That doesn't scale.

@comex
Copy link

comex commented Sep 2, 2022

I do wonder why Cosmopolitan doesn't just pick arbitrary error numbers and translate from the native ones in its system call wrappers. It's not as if it needs to interoperate with native userspace libraries that may have errno and/or error numbers as part of their API. Is it just because this way it can dead-code-strip the error numbers that aren't used? I know Cosmopolitan likes small binaries. But if you ever use strerror in the program, you need to reference all the error numbers anyway, and at that point you're probably left with a larger code size due to referencing variables instead of compile-time constants...

@pnkfelix
Copy link
Member

pnkfelix commented Sep 6, 2022

based on comments so far, and discussion at the lang team meeting, I'm opting to close this proposed RFC.

Like @oli-obk above, I do not think the proposed features fits the character of the Rust language. Separate from that personal opinion, I also think @nikomatsakis and @RalfJung have established that even if it did fit the character of the language, the feature is premature.

(With respect to the question of whether the topic could be revisited in the future, my advice would be to identify a champion on the language design team, or the libs team, who does think this feature is worth fighting for. But I do not think there is such a champion amongst the current members of the language design team.)

@rfcbot fcp close

@rfcbot
Copy link
Collaborator

rfcbot commented Sep 6, 2022

Team member @pnkfelix has proposed to close this. The next step is review by the rest of the tagged team members:

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!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-close This RFC is in PFCP or FCP with a disposition to close it. labels Sep 6, 2022
@jart
Copy link

jart commented Sep 7, 2022

@comex Cosmopolitan uses symbolic system numbers because errno values are just the tip of the iceberg. Consider for example all the magic numbers that exist for functions like setsockopt() and ioctl(). If we used compile-time constants for magnums, then every system call would have to link in the means to translate all possible numbers. That's bloaty. Furthermore consider many system numbers (e.g. termios) are sparse. Would you want your C library to perform a bunch of binary searches and table lookups every time you interact with your kernel? Of course not. With Cosmopolitan's solution, system call wrappers do not need to do any translation except shuffling around registers to conform to ABIs. It's very fast. The only exception is we DO need to translate data structures like struct stat. That's unavoidable, but we make it fast!

@joshtriplett
Copy link
Member

As far as I can tell, this solution would only ever work for non-exhaustive matches; it could never support exhaustive matches. Given that, it seems like if or similar would work in almost every way, except for making existing code (written to assume const values) work unmodified.

I appreciate the value of trying to reuse existing code without modifying it, but I think there's a meaningful difference between "compile-time constant" and "runtime value", and I don't think we should support match or other patterns on the latter.

@nikomatsakis
Copy link
Contributor

@rfcbot reviewed

@Jules-Bertholet
Copy link
Contributor Author

Jules-Bertholet commented Sep 7, 2022

I appreciate the value of trying to reuse existing code without modifying it, but I think there's a meaningful difference between "compile-time constant" and "runtime value", and I don't think we should support match or other patterns on the latter.

I would argue that the distinction between "compile time constant" and "runtime value" has a different meaning from the compiler's perspective versus the programmer's perspective.

  • What the compiler mostly cares about is "can I reason about this value at compile time?" From this perspective, anything that's not a const is a runtime value, and supporting such values means a completely different implementation of exhaustiveness checking, codegen, specification, etc. Any non-const value brings additional complexity that may not be warranted. From this point of view, the prohibition on statics in patterns makes perfect sense.

  • What the programmer mostly cares about is "is this mutable state? Can I rely on the meaning of this symbol not changing?" From this perspective, a non-mut static without interior mutability is a constant value that won't change at runtime. So, from this point of view, the prohibition on such statics in patterns seems like a strange omission, because they are constant values just like consts.

@cuviper
Copy link
Member

cuviper commented Sep 7, 2022

Would you want your C library to perform a bunch of binary searches and table lookups every time you interact with your kernel?

If I may nitpick this a little -- using symbols for magic numbers is still a table lookup (by string!), but either a static linker or the RTLD (ld.so) is doing it on your behalf. Only thereafter, each use becomes an O(1) load from the linked/relocated symbol.

What the programmer mostly cares about is "is this mutable state? Can I rely on the meaning of this symbol not changing?" From this perspective, a non-mut static without interior mutability is a constant value that won't change at runtime. So, from this point of view, the prohibition on such statics in patterns seems like a strange omission, because they are constant values just like consts.

Does the programmer care that such values may change from one run to the next, especially on different machines? Maybe, maybe not, but they are only constant for a particular combination of executable+libraries.

@rfcbot rfcbot added final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. and removed proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. labels Sep 8, 2022
@rfcbot
Copy link
Collaborator

rfcbot commented Sep 8, 2022

🔔 This is now entering its final comment period, as per the review above. 🔔

@nikomatsakis nikomatsakis removed the I-lang-nominated Indicates that an issue has been nominated for prioritizing at the next lang team meeting. label Sep 13, 2022
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. to-announce and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Sep 18, 2022
@rfcbot
Copy link
Collaborator

rfcbot commented Sep 18, 2022

The final comment period, with a disposition to close, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This is now closed.

@rfcbot rfcbot added closed This FCP has been closed (as opposed to postponed) and removed disposition-close This RFC is in PFCP or FCP with a disposition to close it. labels Sep 18, 2022
@rfcbot rfcbot closed this Sep 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-impls-libstd Standard library implementations related proposals. closed This FCP has been closed (as opposed to postponed) finished-final-comment-period The final comment period is finished for this RFC. T-lang Relevant to the language team, which will review and decide on the RFC. T-libs Relevant to the library team, which will review and decide on the RFC. to-announce
Projects
None yet
Development

Successfully merging this pull request may close these issues.