StrangeNoises commented on pull request #1601: URL: https://github.com/apache/groovy/pull/1601#issuecomment-874852409
I'll look at the TypeChooser thing, next time I get back to it (probably this weekend). The code there that digs for the type of method calls and properties is a partial hangover from trying to get it to work in a completely dynamic context, where it certainly wasn't working if I just left it to TypeChooser. I'll check what they do in typechecked. (I did already remove the special code for variables as I found that was unnecessary.) I still think casting to `Object` should be caught and interpreted as "not an array", in this context, as otherwise `OBJECT_TYPE = DYNAMIC_TYPE` (in ClassHelper) which gets us nowhere if the coder's weeping at his keyboard crying "No, you moron, I'm telling you it's *not* an array!" As for just making it emit compiler warnings, no, it wasn't discussed. To discuss it now I would say: What would the coder be expected to do as a result of seeing such a warning? Maybe to put in a cast or coerce, or make sure a property/variable etc. was typed so they didn't need to... But this change is the change needed so that doing that would even work. At the moment, casting to a non-array type is completely ignored. Declared types are completely ignored. A null is always a null array no matter what you do to try to indicate otherwise. Your only recourse is to put it in an array yourself, like varargs was never invented. And of course if you, writing your code, knew for an absolute fact that a given expression would never return a null, and therefore you'd never hit this problem, those compiler warnings would get annoying real fast. The expressions in question are known non-arrays. If they happen to evaluate to non-null values they will currently certainly be wrapped in an array at runtime (`ParameterTypes::fitToVargs` which is also where the "null is always an array" decision is enshrined). They'd have to or you'd be getting missing method exceptions when it tries to invoke the method with the bare non-array type. How is it not a bug that something else happens if *those expressions* happen to evaluate to null? I can't seem to stress or repeat enough that we're only doing *anything* to expressions whose types are known to be non-arrays to a high degree of confidence, where it simply cannot be right that they ever get passed straight to an array-typed parameter. Dynamically typed expressions, where it's *not* obvious whether it should be wrapped or not, are left well alone. If anything there should be compiler warnings if they try to apply a dynamic-typed expression to a varargs method. But again, what are they going to do about it? Put a cast in to say "it's an Object" or "It's an array". Again: This is the change that means that doing that actually accomplishes anything at all. (And I still think this should be fixed for dynamic as well 😉 - It's still a crime that you can't push it the right way with an explicit cast at *least*. It's just harder to be sure you're even in a varargs context there.) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
