Skip to content

Conversation

andykaylor
Copy link
Collaborator

This changes the alias prefix for record types to make it less general.

This changes the alias prefix for record types to make it less general.
@andykaylor
Copy link
Collaborator Author

As discussed on llvm/llvm-project#136387

Copy link
Collaborator

@erichkeane erichkeane 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 good change, I'm happy with the rec_ prefix.

@@ -63,9 +63,9 @@ struct CIROpAsmDialectInterface : public OpAsmDialectInterface {
if (auto recordType = dyn_cast<cir::RecordType>(type)) {
StringAttr nameAttr = recordType.getName();
if (!nameAttr)
os << "ty_anon_" << recordType.getKindAsStr();
os << "rec_anon_" << recordType.getKindAsStr();
Copy link
Collaborator

Choose a reason for hiding this comment

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

not to do in this patch, but we discussed making this an 'assert' at one point, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah. I was going to do that separately.

Copy link
Collaborator

@xlauko xlauko left a comment

Choose a reason for hiding this comment

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

lgtm

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.

Awesome!

@bcardosolopes bcardosolopes merged commit 395999a into llvm:main Apr 22, 2025
10 checks passed
terapines-osc-cir pushed a commit to Terapines/clangir that referenced this pull request Sep 2, 2025
This changes the alias prefix for record types to make it less general.
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.

4 participants