Skip to content

[mlir] Make classof substitution in interface use an instance #65492

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 3 commits into from
Sep 7, 2023

Conversation

zero9178
Copy link
Member

@zero9178 zero9178 commented Sep 6, 2023

The substitution supported by extraClassOf is currently limited to only the base instance, i.e. Operation*, Type or Attribute, which limits the kind of checks you can perform in the classof implementation.

Since prior to the user code, the interface concept is fetched, we can use it to construct an instance of the interface, allowing use of its methods in the classof check.

Since an instance of the interface allows access to the base class methods through the -> operator, I've gone ahead and replaced the substitution of $_op/$_type/$_attr with an interface instance. This is also consistent with extraSharedClassDeclaration and other methods created in the interface class which do the same.

@zero9178 zero9178 requested review from a team as code owners September 6, 2023 15:05
@github-actions github-actions bot added mlir:core MLIR Core Infrastructure mlir labels Sep 6, 2023
Copy link
Collaborator

@joker-eph joker-eph left a comment

Choose a reason for hiding this comment

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

Can there be a test for this?

The substitution supported by `extraClassOf` is currently limited to only the base instance, i.e. `Operation*`, `Type` or `Attribute`, which limits the kind of checks you can perform in the `classof` implementation.

Since prior to the user code, the interface concept is fetched, we can use it to construct an instance of the interface, allowing use of its methods in the `classof` check.

Since an instance of the interface allows access to the base class methods through the `->` operator, I've gone ahead and replaced the substitution of `$_op/$_type/$_attr` with an interface instance. This is also consistent with `extraSharedClassDeclaration` and other methods created in the interface class which do the same.
@zero9178 zero9178 force-pushed the classof-interface-substitution branch from 29158fb to 2f40679 Compare September 7, 2023 07:16
@zero9178 zero9178 merged commit edae8f6 into llvm:main Sep 7, 2023
@zero9178 zero9178 deleted the classof-interface-substitution branch September 8, 2023 11:53
avillega pushed a commit to avillega/llvm-project that referenced this pull request Sep 11, 2023
…#65492)

The substitution supported by `extraClassOf` is currently limited to
only the base instance, i.e. `Operation*`, `Type` or `Attribute`, which
limits the kind of checks you can perform in the `classof`
implementation.

Since prior to the user code, the interface concept is fetched, we can
use it to construct an instance of the interface, allowing use of its
methods in the `classof` check.

Since an instance of the interface allows access to the base class
methods through the `->` operator, I've gone ahead and replaced the
substitution of `$_op/$_type/$_attr` with an interface instance. This is
also consistent with `extraSharedClassDeclaration` and other methods
created in the interface class which do the same.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:core MLIR Core Infrastructure mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants