Use As and ImplicitAs interfaces for conversions.
#4209
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add Convert method to ImplicitAs for now, given `extend` support is missing.
jonmeow
approved these changes
Aug 9, 2024
There was a problem hiding this comment.
Generally looks good, although there's a lot more reuse of locations, which got me to look at the TODO that'd hamper that. #4211 for the relevant changes.
I'm approving, if you want to merge before heading out and leave the locations for me to clean up, that's fine by me. Though if you object to the location stuff, please comment on #4211 before going. :)
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
jonmeow
reviewed
Aug 30, 2024
| @@ -23,6 +23,8 @@ ExitingStream::~ExitingStream() { | |||
|
|
|||
| auto ExitingStream::Done() -> void { | |||
| buffer_ << "\n"; | |||
| buffer_.flush(); | |||
There was a problem hiding this comment.
How did this come up for the change?
There was a problem hiding this comment.
A lifetime issue with the array of interface arguments caused the tests to fail with opt builds but not with dbg or fastbuild, and the opt builds also seemed to lose the end of the check failure diagnostic if I didn't explicitly flush the stream.
jonmeow
reviewed
Aug 30, 2024
This was referenced Sep 4, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 5, 2024
Conversion can trigger new entities to be imported, which can invalidate the reference it holds to an EntityWithParamBase. Instead of holding such a reference, pull the information we need out of the entity early and only pass that into ConvertCallArgs. I couldn't find a good standalone way to test this, but this fixes the test failure we otherwise see on MacOS after #4209, so it will be tested once that PR lands.
zygoloid
force-pushed
the
toolchain-convert-interfaces
branch
from
b8272f1
to
db8a466
Compare
jonmeow
approved these changes
Sep 5, 2024
dwblaikie
pushed a commit
to dwblaikie/carbon-lang
that referenced
this pull request
Sep 6, 2024
…4277) Conversion can trigger new entities to be imported, which can invalidate the reference it holds to an EntityWithParamBase. Instead of holding such a reference, pull the information we need out of the entity early and only pass that into ConvertCallArgs. I couldn't find a good standalone way to test this, but this fixes the test failure we otherwise see on MacOS after carbon-language#4209, so it will be tested once that PR lands.
dwblaikie
pushed a commit
to dwblaikie/carbon-lang
that referenced
this pull request
Sep 6, 2024
…e#4209) Add these interfaces to the core library. For now, they're two separate interfaces because we don't yet support one interface extending another. This collapses a lot of the layering in check: for example, the call building logic depends on implicit conversions, conversions now depend on the overloaded operator machinery, and that machinery depends on building calls. In passing, improve the diagnostics for failing to find a name required from the prelude. Also convert all the transitively-called code from `NodeId` to `LocId` given the latter is what the conversion machinery has available. --------- Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
dwblaikie
pushed a commit
to dwblaikie/carbon-lang
that referenced
this pull request
Sep 9, 2024
…4277) Conversion can trigger new entities to be imported, which can invalidate the reference it holds to an EntityWithParamBase. Instead of holding such a reference, pull the information we need out of the entity early and only pass that into ConvertCallArgs. I couldn't find a good standalone way to test this, but this fixes the test failure we otherwise see on MacOS after carbon-language#4209, so it will be tested once that PR lands.
dwblaikie
pushed a commit
to dwblaikie/carbon-lang
that referenced
this pull request
Sep 9, 2024
…e#4209) Add these interfaces to the core library. For now, they're two separate interfaces because we don't yet support one interface extending another. This collapses a lot of the layering in check: for example, the call building logic depends on implicit conversions, conversions now depend on the overloaded operator machinery, and that machinery depends on building calls. In passing, improve the diagnostics for failing to find a name required from the prelude. Also convert all the transitively-called code from `NodeId` to `LocId` given the latter is what the conversion machinery has available. --------- Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add these interfaces to the core library. For now, they're two separate interfaces because we don't yet support one interface extending another.
This collapses a lot of the layering in check: for example, the call building logic depends on implicit conversions, conversions now depend on the overloaded operator machinery, and that machinery depends on building calls.
In passing, improve the diagnostics for failing to find a name required from the prelude. Also convert all the transitively-called code from
NodeIdtoLocIdgiven the latter is what the conversion machinery has available.