Skip to content

Conversation

@geoffromer
Copy link
Contributor

@geoffromer geoffromer commented Oct 27, 2025

Support for bound, and for the ref tag on arguments, is left as future work.

@geoffromer geoffromer requested a review from a team as a code owner October 27, 2025 23:49
@geoffromer geoffromer requested review from chandlerc and removed request for a team October 27, 2025 23:49
base class Base {
var a: i32;

fn F[addr self: Self*]();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it really worth preserving all the addr tests?

I had thought we fully planned to remove addr at this stage....

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, removing addr is the plan, but I can't do that until I've migrated the prelude and C++ interop to use ref instead. Those changes aren't entirely trivial, so I'm trying to make incremental progress. As long as we still have addr, it seems preferable to keep the tests for it.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, it jsut seems like a lot of churn to duplicate the test and then remove it.

But if it helps in the interim, sure. =]

I've not tried especially hard to review the addr tests though as they don't appear as branched from the original ones and so have no useful diff. Probably ok though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the addr tests are just renames of the original tests.

Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

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

Seems like a reasonable increment, LG!

base class Base {
var a: i32;

fn F[addr self: Self*]();
Copy link
Contributor

Choose a reason for hiding this comment

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

OK, it jsut seems like a lot of churn to duplicate the test and then remove it.

But if it helps in the interim, sure. =]

I've not tried especially hard to review the addr tests though as they don't appear as branched from the original ones and so have no useful diff. Probably ok though.

@geoffromer geoffromer enabled auto-merge October 29, 2025 15:49
@geoffromer geoffromer added this pull request to the merge queue Oct 29, 2025
Merged via the queue into carbon-language:trunk with commit 4821eec Oct 29, 2025
8 checks passed
@geoffromer geoffromer deleted the ref-pattern branch October 29, 2025 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants