Minor changes to improve the type annotations

[michel-steuwer]

These two changes improve the usage of xDSL in PyCharm.

1. Making RegionDef a subclass of Region allows op and ops (and other properties and methods of Regions) to show up in the autocomplete menu.
   Before:

After:

2. Removing the TypeVar in the irdl_op_definition significantly improves type checking experience in PyCharm. (I don't understand why!).

Before, the IDE doesn't see that func_def is of type FuncDef:

After, it now sees this correctly:

[math-fehr]

The region change, and the builder change makes sense.
Though I'm not understanding at all why the irdl_op_definition makes it work in your PyCharm. My PyCharm seems to work before your change though.
What version of PyCharm do you have? And which version of Python do you use?

[michel-steuwer]

Latest version of PyCharm (2021.3.2 Professional Edition) and Python 3.9.

I will try to reimport the project and see if it makes a difference.

[math-fehr]

Oh actually, nevermind, my version seems to also have this bug, but I think it is not always there for some reasons.

This works without issues

But this fails:

Do you have the same behavior?

[math-fehr]

I think it might be related to the isinstance

[michel-steuwer]

That the first of your examples works is expected, as you have an explicit type annotation in the first line.

The "type narrowing" that isinstance is doing (https://www.python.org/dev/peps/pep-0647/) should work. The type checker should understand that in the nested branch, the type of op is now narrower than before.
I think this might be a bug of the PyCharm editor.

Does this PR fixes this in your IDE as well?

[math-fehr]

For me, it seems to break for other reasons now.
Essentially, it doesn't show any errors, but this is because PyCharm doesn't understand what FuncOp is

Here, op.foo is correctly shown a warning, while the op.foo after the if has no warnings

[michel-steuwer]

... hmm

maybe it is just disabling all checking and reverting to the Any type?

[math-fehr]

I think it's the case, since we don't provide any information about the resulting type of irdl_op_definition.
I'm trying to find if there is a way to make the isinstance work better though, I'm not having much luck for now

[math-fehr]

Otherwise, this is indeed a problem with isinstance:

[michel-steuwer]

I think the type var that we currently have (before this PR) is modelling the type correctly.

[michel-steuwer]

I am trying to see what mypy thinks about this ...

[math-fehr]

Yeah, what we had before should be correct according to PEP. Though PyCharm seems to be buggy because of the decorator/TypeVar I think. I'm trying to find a minimum reproducer.

[math-fehr]

I've made some experiments, and I'm realizing that in fact, nothing was working in the first place.
So even before, the type checker would not show an error on wrong attributes, because it knew the type could have arbitrary attributes in addition to the FuncDef ones. Though the IDE recommendations were still correct.
The problem we are having here is that in addition to that, the isinstance is bugged, thus giving errors instead of casting to "Any".
I'm not sure how to fix that bug, but what I found is that moving the function to the same file as the dialect definition makes it work, so maybe there is an import issue somewhere. I'll try a bit to fix that if there is an easy solution.

[math-fehr]

Well, I abandon, I have no idea on how to fix this bug.
I guess that we will have to wait that this is fixed somehow, though I still don't have a simple reproducer to fix it.

I would suggest to add your changes, since they at least seem to not give annoying warnings, and that they seem to provide the right suggestions somehow!
We can always fix that later, though I doubt that this will be fixed soon!