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

Add support for combining conflicting exported names. #3954

Merged
merged 4 commits into from
May 20, 2024

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented May 16, 2024

I think this is a key remaining bit of polish for intra-package exports. Next I'm going to be dealing with cross-package exports, though I think the fundamentals here will remain intact.

Extern declarations could cause issues for the current approach, but I think that can be addressed separately. I don't think it's a fundamental problem, more just incremental.

@jonmeow
Copy link
Contributor Author

jonmeow commented May 16, 2024

Depends on #3950 (dependency merged)

@jonmeow jonmeow force-pushed the export-conflict branch 2 times, most recently from 23ad66c to 3f37a25 Compare May 17, 2024 22:38
toolchain/check/import.cpp Outdated Show resolved Hide resolved
Comment on lines +33 to +35
auto operator==(const ImportIRInst& rhs) const -> bool {
return ir_id == rhs.ir_id && inst_id == rhs.inst_id;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
auto operator==(const ImportIRInst& rhs) const -> bool {
return ir_id == rhs.ir_id && inst_id == rhs.inst_id;
}
auto operator==(const ImportIRInst& rhs) const -> bool = default;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In file included from toolchain/sem_ir/name.cpp:8:
In file included from ./toolchain/sem_ir/file.h:20:
./toolchain/sem_ir/import_ir.h:33:8: error: explicitly defaulted equality comparison operator is implicitly deleted [-Werror,-Wdefaulted-function-deleted]
  auto operator==(const ImportIRInst& rhs) const -> bool = default;
       ^
./toolchain/sem_ir/import_ir.h:28:30: note: defaulted 'operator==' is implicitly deleted because there is no viable three-way comparison function for base class 'Printable<Carbon::SemIR::ImportIRInst>'
struct ImportIRInst : public Printable<ImportIRInst> {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding an explicit operator== to Printable fixes this, but makes all other children comparable too. I'm not sure that's what we want. Will follow up separately.

jonmeow and others added 3 commits May 20, 2024 08:28
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
@jonmeow jonmeow enabled auto-merge May 20, 2024 15:50
@jonmeow jonmeow added this pull request to the merge queue May 20, 2024
Merged via the queue into carbon-language:trunk with commit 69c7012 May 20, 2024
7 checks passed
@jonmeow jonmeow deleted the export-conflict branch May 20, 2024 16:09
CJ-Johnson pushed a commit to CJ-Johnson/carbon-lang that referenced this pull request May 23, 2024
…e#3954)

I think this is a key remaining bit of polish for intra-package exports.
Next I'm going to be dealing with cross-package exports, though I think
the fundamentals here will remain intact.

Extern declarations could cause issues for the current approach, but I
think that can be addressed separately. I don't think it's a fundamental
problem, more just incremental.

---------

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
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.

None yet

2 participants