-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Add type hints to strconv.py, and propagate implications #2162
Conversation
This PR was also "tested" by printing |
Can you clarify? Is this about having them in the same file? If that's so I |
Does this relate to #2109? |
I will address #2109 there, and I agree with the ideal of not putting these classes in the same file, but chose very bad wording; sorry. What I meant is regarding the fact that e.g. NewType has a TypeInfo inside it. (What follows is probably too long and unuseful as usual. Please don't feel obligated to read it) I will try to convey my understanding of core concepts in the analysis; It is not a suggestion as to what should be done (which as you and Jukka explained to me involves complex trade-offs). I use "should" and similar words but please ignore them. I also mix several levels of design and theory since things are not really clear to me. And I am not really sure it's worth your time. I believe that Node subclasses should represent the "fixed" ast, with the added constructs such as NewType, NamedTuple, So the mapping from an ast node to a type feels external to the ast itself - a node is not "built out of" types, and the type of a node depends on lots of context. This is a "shared" pointer, possibly there for convenience or performance. I think that this kind of "incoherence" is part of the reason that Optional is used in so many places. Far too many in my opinion. As you pointed out (if I understood you correctly), SymbolNode should not be involved with all that jazz, and so does TypeInfo (I know it's not easy; I did not succeed making this transformation). The ast comprises the "underlying graph" of the data flow, even if no explicit Graph data structure is used. Semantic analysis builds the type information in "sources" of the data flow graph - the explicitly annotated or trivially inferred parts of the program. It may also prune some unreachable edges in the graph based on this immediately-accessible type information. The result is kept in what is now called SymbolTable - Describing and binding sources, sinks and edges (lvalues, call-signatures matches, etc.). The type information is still incomplete so to be sound there are many "edges" left in the "graph", e.g. from a call to every possible matching signature. Again, most of this is implicit which is probably fine, but it's there. There is yet another distinction which is too fuzzy. A "Type" is just a type. It is not a Context. It should not be mutable. Context is very important for reporting errors, but this is not a "Type". So the solver should work with "TypeOccurrence" which holds both the context and the type - what is now somewhat confusingly called "Type". Separating Context and Type will make it more obvious what a Type mean (again, I don't actually suggest doing that). Assuming the code reflects what I just describe, it should not involve too many dependency cycles. A Type is built from other types. A Node is built from other nodes. TypeOccurrence combines them. SymbolTable binds TypeOccurrences to expressions and TypeInfo to type definitions, etc. |
You probably just need to keep reading and re-reading the code. You didn't One thing though: IIUC there is indeed a confusion about the meaning of The issue with Optional is complicated, but it's not easy to get to a On Tue, Sep 20, 2016 at 8:03 PM, Elazar notifications@github.com wrote:
--Guido van Rossum (python.org/~guido) |
I will. Regarding Context, if the visitor pattern had the side effect of updating the current context of the pass, Contexts need not be passed around explicitly so often - almost exclusively copied and not used. I am not sure that's a better design, since it adds implicitly-mutable state, but it has the benefit of being able to change what Context mean without a cascading effect all over the place. (Incidentally, Context then will be used with a context manager :) ) |
I am not sure that's the best string representation for
NewType
.I strongly feel that composing
TypeInfo
in subclasses ofnodes.Node
make them non-cohesive. This mapping should be external.