labath wrote: > Why is that a bad thing? Can we do math operations later without types? Plus > it's a unified interface, node evaluation returns a `ValueObject`. > > > ... > > I mean... this works and is a basis for future patches, why remove something > that we will have to bring back shortly afterwards? After replacing frame > var, DIL will just have a little bit of extra capabilities, like using > another variable as an index.
Because I wanted to avoid this getting bogged down in the discussion about the types of numbers. I don't really mind the "extra capability" of indexing an array with a another variable. The part I have problem with is the precedent it sets about the representation of numbers. You're right that we can't do math operations without (implicitly or explicitly) assigning them some type, but that's exactly the part I think needs more discussion. Right now, you're assigning the type based on the first type system you find (which is likely going to be C), but I think that's not a good choice. If you're debugging some swift code, I think you'd be surprised if `47` resolves to a C type. Using the type system of the currently selected frame might be better, but I'm not convinced that the right choice either. Since you support looking up global variables you can easily end up with a global variable from a different language, and then we have the question of what to do with expressions like `global_c_variable+(__swift int)1`. Normally, I would think we can just say we don't support arithmetic on values from different type systems, but if constants can end up with a foreign type system because of the context, then we'd probably want to support working with those.. somehow. Which brings me to another idea which might work (but again, I'm not saying it's the right one), which is to give constants some neutral type -- for example, the one (implicitly) defined by the operations on the `Scalar` class, and only convert it to a specific type system once it starts interacting with one. Or maybe do it the other way around, and convert everything into a Scalar once we start doing arithmetic. My point is: the space of possible options is very big here, but none of this is necessary to make array indexing (with a constant) work. I agree with your "it works" assertion (but I think that a much simpler implementation would also "work"), but not with the "it's a basis for future patches" part. I think that part needs more discussion. I don't think you need to change the evaluation interface for this -- yet. It's true that some of the ideas above would require that, but that's very much in the air. For now, it should be sufficient to make the array index a raw integer (instead of an AST node). Then there's nothing to evaluate, and you can just directly take the number and index with it. > We still need to implement bit extraction that current `frame var` allows, > which looks like this: `integer[4-8]`, another node we will have to > re-implement later if we redo how numbers are stored now. Oh wow, I didn't even know that existed. I guess that means we should implement that as well, but I don't think it changes the equation fundamentally. Right now, the two numbers can be stored as... numbers, and later that could be changed to a recursive `Evaluate` call. It should be a local change, if we end up going with the approach you have here. And if we end up with something completely different then it will be less code to change, as this implementation will be simpler. https://github.com/llvm/llvm-project/pull/138551 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
