carbon-lang icon indicating copy to clipboard operation
carbon-lang copied to clipboard

Consider using "pure" or "const" to tag methods as non-mutating instead of the "addr" keyword

Open sor opened this issue 2 years ago • 6 comments

Hey Carbon 👋, thanks for being open before everything is finalized 👍

C++ did place the cv qualifiers in a rather unfortunate location, since it could not put it anywhere else thanks to C's baggage. There are more than one names which people would assume to denote a method to not change this, but addr does not come to my mind. fn Offset[addr me: Self*](dx: i32, dy: i32); You really need someone explain to you why it was named that way, the addr in it makes no inherent sense.

I think any of the following ways would make it easier to understand right away that the method is const/pure/non-mutating

pure fn Offset[me: Self*](dx: i32, dy: i32);
const fn Offset[me: Self*](dx: i32, dy: i32);
fn Offset[me: const Self*](dx: i32, dy: i32);
fn Offset[me: pure Self*](dx: i32, dy: i32);
fn Offset[const me: Self*](dx: i32, dy: i32);
fn Offset[pure me: Self*](dx: i32, dy: i32);

sor avatar Jul 26 '22 18:07 sor

Experience has shown that non-mutating should generally be the default:

  • Having fewer side effects means less need to alert readers that something dangerous or unexpected could be happening.
  • Non-mutating methods should be encouraged, and so making them shorter and therefore easier is desirable.

We think this is an area where C++ gets the default wrong.

josh11b avatar Jul 26 '22 20:07 josh11b

I agree @josh11b, rather immutable by default. But in case they want to keep it mutating by default, at least use a recognizable keyword for non-mutability

sor avatar Jul 27 '22 19:07 sor

If we were to do this (and had to pick a keyword), I would expect pure to mean that the me receiver as well as all of the arguments are not mutated, but const would only affect me.

nigeltao avatar Jul 29 '22 02:07 nigeltao

I'm not saying you ought to do this (especially as familiarity for C++ programmers is a goal) but if we're considering syntax, here's some ideas...

In Wuffs, functions and methods are pure by default and purity needs no explicit syntactic annotation. Impure functions are explicitly marked with the ! symbol (and the not operator is, like Carbon and unlike C++, spelled not). Impure coroutines are explicitly marked with the ? symbol (and there is no x ? y : z syntax) and ?ness implies !ness.

Subtly, the ! symbol (indicating mutation and side effects) is present at the call site too, not just at the function declaration. It is a compile-time error if you omit the ! at the call site. Conversely, if I see a Wuffs function call without an ! then I know (at the call site) that the function call has no side effects.

As a programming language, Wuffs' focus is on proving memory safety by static analysis, so knowing (at the AST level, without requiring further significant computation, crossing multiple source files) that a line of code has no side effects (other than being part of an assignment statement) is very useful. Carbon of course has different goals, so it can obviously make different syntax choices. But you still might want to consider the idea.

nigeltao avatar Jul 29 '22 02:07 nigeltao

const fn might be used to mean something else (similar to constexpr functions in C++ and like const functions in Rust). Obviously, you could choose another keyword (like the rather long constexpr keyword), but it's just something to keep in mind that this syntax might be useful to express something else.

h3har avatar Jul 29 '22 04:07 h3har

I think that C++ is mutable by default because it wants to be compatible with C (and that may come further down from ancient languages).

If this is already not C++, then it could do the right thing and be const by default with explicit mutation. Stealing Rust's mut keyword would be nice to avoid coming up with a different one just to be special.

Compile-time constness should have an explicit keyword akin to constexpr, but hopefully better (expr is redundant there right?)

Dietr1ch avatar Sep 04 '22 20:09 Dietr1ch

Alternative approach:

fn should always be pure (in classes and otherwise). pr (or similar) is introduced for any impure functions, and only this reserved word allows using addr (or even makes addr default for methods).

TheKashe avatar Sep 26 '22 11:09 TheKashe

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please comment or remove the inactive label. The long term label can also be added for issues which are expected to take time. This issue is labeled inactive because the last activity was over 90 days ago.

github-actions[bot] avatar Dec 26 '22 01:12 github-actions[bot]

While pass-by-immutable-value does seem like a good default, we might want to consider other options for pass-by-mutable-reference. One option I think we should seriously consider is to add inout parameters that carry the semantics of either copying/moving/borrowing the source object, and then copying/moving/unborrowing back when the call finishes. In particular, it would be incorrect to access the source object during a call where it is used as an inout parameter, and we should be able to provide some compiler enforcement of that. (This is an idea that Val has explored in some detail, and can provide some safety and correctness guarantees that are hard to achieve with raw pointers.)

zygoloid avatar Jan 26 '23 20:01 zygoloid

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please comment or remove the inactive label. The long term label can also be added for issues which are expected to take time. \n\n\n This issue is labeled inactive because the last activity was over 90 days ago.

github-actions[bot] avatar Apr 28 '23 01:04 github-actions[bot]

We triage inactive PRs and issues in order to make it easier to find active work. If this issue should remain active or becomes active again, please comment or remove the inactive label. The long term label can also be added for issues which are expected to take time. \n\n\n This issue is labeled inactive because the last activity was over 90 days ago.

github-actions[bot] avatar Jul 29 '23 01:07 github-actions[bot]

I think the core question here is whether we should move away from addr right now. I think this is something we should explore, and there are lots of comments in this issue with ideas that should be explored. But I don't think now is the time to do that exploration, and I don't think this issue is really helping us get there.

My suggestion is that we decide "not right now" on this question. That doesn't preclude us revisiting this, especially holistically, at a later point.

chandlerc avatar Aug 06 '23 00:08 chandlerc

Marking this as decided with "not right now" -- all the leads were happy with that direction as I outline.

chandlerc avatar Aug 25 '23 22:08 chandlerc