Skip to content

[CIR][Transforms][NFC] Use unique_ptr to encapsulate LowerModule #752

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

Merged
merged 2 commits into from
Jul 25, 2024

Conversation

seven-mile
Copy link
Collaborator

Currently LowerModule mimics CodeGenModule and uses many raw references. It cannot be moved or copied. Value semantic does not fit the need. For example, we cannot pass LowerModule around.

A better practice would be to use unique_ptr to encapsulate it. In the future, we hold its ownership in some long-lived contexts (it's CodeGeneratorImpl for CodeGenModule) and pass references to it around safely.

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

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

LGTM with minor change needed

@@ -36,22 +36,22 @@ struct CallConvLoweringPattern : public OpRewritePattern<FuncOp> {
return op.emitError("function has no AST information");

auto modOp = op->getParentOfType<ModuleOp>();
LowerModule lowerModule = createLowerModule(modOp, rewriter);
auto lowerModule = createLowerModule(modOp, rewriter);
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't fit the auto rules from code style (I know, it's annoying =( )

@seven-mile
Copy link
Collaborator Author

Suggestion applied ; )

@bcardosolopes bcardosolopes merged commit 0f94033 into llvm:main Jul 25, 2024
6 checks passed
Hugobros3 pushed a commit to shady-gang/clangir that referenced this pull request Oct 2, 2024
…lvm#752)

Currently `LowerModule` mimics `CodeGenModule` and uses many raw
references. It cannot be moved or copied. Value semantic does not fit
the need. For example, we cannot pass LowerModule around.

A better practice would be to use `unique_ptr` to encapsulate it. In the
future, we hold its ownership in some long-lived contexts (it's
`CodeGeneratorImpl` for `CodeGenModule`) and pass references to it
around safely.
smeenai pushed a commit to smeenai/clangir that referenced this pull request Oct 9, 2024
…lvm#752)

Currently `LowerModule` mimics `CodeGenModule` and uses many raw
references. It cannot be moved or copied. Value semantic does not fit
the need. For example, we cannot pass LowerModule around.

A better practice would be to use `unique_ptr` to encapsulate it. In the
future, we hold its ownership in some long-lived contexts (it's
`CodeGeneratorImpl` for `CodeGenModule`) and pass references to it
around safely.
smeenai pushed a commit to smeenai/clangir that referenced this pull request Oct 9, 2024
…lvm#752)

Currently `LowerModule` mimics `CodeGenModule` and uses many raw
references. It cannot be moved or copied. Value semantic does not fit
the need. For example, we cannot pass LowerModule around.

A better practice would be to use `unique_ptr` to encapsulate it. In the
future, we hold its ownership in some long-lived contexts (it's
`CodeGeneratorImpl` for `CodeGenModule`) and pass references to it
around safely.
keryell pushed a commit to keryell/clangir that referenced this pull request Oct 19, 2024
…lvm#752)

Currently `LowerModule` mimics `CodeGenModule` and uses many raw
references. It cannot be moved or copied. Value semantic does not fit
the need. For example, we cannot pass LowerModule around.

A better practice would be to use `unique_ptr` to encapsulate it. In the
future, we hold its ownership in some long-lived contexts (it's
`CodeGeneratorImpl` for `CodeGenModule`) and pass references to it
around safely.
lanza pushed a commit that referenced this pull request Nov 5, 2024
)

Currently `LowerModule` mimics `CodeGenModule` and uses many raw
references. It cannot be moved or copied. Value semantic does not fit
the need. For example, we cannot pass LowerModule around.

A better practice would be to use `unique_ptr` to encapsulate it. In the
future, we hold its ownership in some long-lived contexts (it's
`CodeGeneratorImpl` for `CodeGenModule`) and pass references to it
around safely.
lanza pushed a commit that referenced this pull request Mar 18, 2025
)

Currently `LowerModule` mimics `CodeGenModule` and uses many raw
references. It cannot be moved or copied. Value semantic does not fit
the need. For example, we cannot pass LowerModule around.

A better practice would be to use `unique_ptr` to encapsulate it. In the
future, we hold its ownership in some long-lived contexts (it's
`CodeGeneratorImpl` for `CodeGenModule`) and pass references to it
around safely.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants