TypeVar and virtual support in Symbol Resolver #99
No reviewers
Labels
No Milestone
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: M-Labs/nac3#99
Loading…
Reference in New Issue
No description provided.
Delete Branch "symbol_resolver_typevar"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Add
TypeVar
andvirtual
support for Symbol Resolver in nac3artiq and nac3standaloneRegarding
_eval_type
:A
in module X with a classA
which is not imported in our current module.Otherwise LGTM. (if tested, I did not look into details)
90b7657a72
todab06bdb58
Yes I also have some conern about this when writing the code. I tried to find a better python API but could not find one so far... This PEP contains some information about this, currently the public API seems to be only
typing.get_type_hints
, which can only get type hints for function, method, module or class object, but does not seem to work on contraints inTypeVar
s.There is an
from __future__ import annotations
which could also solve this problem, but this should require users to write it in their code.I am not sure if I fully got your question. But in this case, if class
A
is defined in moduleY
, and moduleY
is imported to moduleX
, type annotation"X.A"
would work. Otherwise there is no way for python to know that isA
?Can we just remove anything that has to do with
ForwardRef
for now, and deal with the forward type annotation situation later? #73Ok, I will do that and update this PR.
Edit: removed in the new commit
@ -17,2 +18,4 @@
pass
# place the `virtual` class infront of the construct of NAC3 object to ensure the
# virtual class is known during the initializing of NAC3 object
Unclear - do you have an example?
Sorry for the unclear comment. Here I mean that before this line in
min_artiq.py
:compiler = nac3artiq.NAC3(core_arguments["target"])
, the classvirtual
should already be known to CPython (because the construction ofcompiler
needs to read get the id of it)if we move this class definition below:
and run the compiler, we will have:
By the way, why is this different than the KernelInvariant class ?
As I understand it, here during the initialization of the nac3 python object it needs to know the id of class
virtual
becuasevirtual[..]
is actually a type that can exist in the range of a typevar, and when resolving typevars, nac3artiq needs the id of it to know that it is handling avirtual
.While KernelInvariant in
min_artiq.py
is just for cpython to not complain about unknown name and we do not need to know its id because so far it only occurs in class fields definition and nac3 do not need to know any cpython related information about it since we are directly looking into the string in the ast.@ -114,0 +111,4 @@
// do not handle type var param and concrete check here
Ok(Ok((unifier.add_ty(TypeEnum::TTuple { ty: vec![] }), false)))
} else if let Some(def_id) = self.pyid_to_def.read().get(&ty_id).cloned() {
// println!("getting def");
remove println
@ -114,1 +113,4 @@
} else if let Some(def_id) = self.pyid_to_def.read().get(&ty_id).cloned() {
// println!("getting def");
let def = defs[def_id.0].read();
// println!("got def");
same
@ -69,0 +71,4 @@
assert_eq!(targets.len(), 1, "only support single assignment for now, at {}", targets[0].location);
if let ExprKind::Call { func, args, .. } = &value.node {
if matches!(&func.node, ExprKind::Name { id, .. } if id == &"TypeVar".into()) {
print!("registering typevar {:?}", targets[0].node);
Remove print. If we want this sort of debug output we should use the logger crate.
@ -69,0 +87,4 @@
})
.collect::<Vec<_>>();
let res_ty = composer.unifier.get_fresh_var_with_range(&constraints).0;
println!(
same
@ -68,1 +68,4 @@
for stmt in parser_result.into_iter() {
// handle type vars in toplevel
if let StmtKind::Assign { value, targets, .. } = &stmt.node {
assert_eq!(targets.len(), 1, "only support single assignment for now, at {}", targets[0].location);
Again, is it difficult to iterate?
@ -69,0 +88,4 @@
let res_ty = composer.unifier.get_fresh_var_with_range(&constraints).0;
internal_resolver.add_id_type(
if let ExprKind::Name { id, .. } = &targets[0].node { *id } else {
panic!("must assign simple name variable as type variable")
Do not call panic from errors in user code. Proper error message and Python exceptions are required here.
Thanks for pointing this out, yes this and the previous issue about iterating over assignment should be resolved. Sorry that I forgot to handle these earlier..
Since this is basically the same place in the code as the default default parameter support (in
nac3standalone/src/main.rs
), maybe I will wait for that one to be mereged first and then modify this PR?Other PR merged.
Hi, the new commits resolves the conflict, and support iteration over multiple typevar assignments in the same line. I will also see if my previous PRs introduce the bug newly found..
dada7d6688
has nothing to do with this PR. Please keep PRs contained to one topic.Also panic in nac3standalone is acceptable, this is just a demonstration program.
Ok I see, I will remove the commits about removing panics.
dada7d6688
to49240a80ad
TypeVar, virtual and ForwardRef support in Symbol Resolverto TypeVar and virtual support in Symbol Resolver