CallMethod, CallComputedMethod and Construct now support Spread Elements #238
Conversation
amarekano
force-pushed
the
extend-spread-element-support
branch
3 times, most recently
from
d091a59
to
07be3e9
Compare
amarekano
force-pushed
the
extend-spread-element-support
branch
from
07be3e9
to
6619a93
Compare
amarekano
force-pushed
the
extend-spread-element-support
branch
from
e8b1196
to
1ff978c
Compare
|
I've removed the Call*WithSpread operations but left the generators and ProgramBuilder API unchanged. I think it's probably best to have multiple generators that use fewer ops in a variety of ways. This imo gives us more flexibilty with code gen and also concise operation set for the AI, mutators, analysers, reducers, etc to work on. |
|
@saelo any thoughts on this one? |
| guard b.mode != .conservative else { return } | ||
| methodName = b.genMethodName() | ||
| } | ||
| guard let arguments = b.randCallArguments(forMethod: methodName!, on: obj) else { return } |
There was a problem hiding this comment.
I think we don't need b.randCallArguments(forMethod:,on:) here and in the other XWithSpread generators since we will (hopefully) be spreading some of the arguments, in which case the following arguments are anyway at the wrong position (instead, probably just pick 3-5 random variables). Due to that, I think we might anyway only want to run these XWithSpread CodeGenerators if we are not in conservative mode, since we are currently unable to track element types of arrays. So lets put the guard b.mode != .conservative else { return } at the start (and add a short comment in front of it). That will also simplify the first 5 lines of this function, and we can then also drop the b.type(of: arg).Is(.iterable) requirement I think (but then decrease the spread probability, maybe to 0.25?).
amarekano
force-pushed
the
extend-spread-element-support
branch
from
7c19256
to
b03767c
Compare
| @@ -255,6 +255,27 @@ public let CodeGenerators: [CodeGenerator] = [ | |||
| b.callMethod(methodName!, on: obj, withArgs: arguments) | |||
| }, | |||
|
|
|||
| CodeGenerator("MethodCallWithSpreadGenerator", input: .object()) { b, obj in | |||
| guard b.mode != .conservative else { return } | |||
There was a problem hiding this comment.
Please add a comment here and in the other functions like We cannot currently track element types of Arrays and other Iterable objects and so cannot properly determine argument types when spreading. For that reason, we don't run this CodeGenerator in conservative mode
| CodeGenerator("MethodCallWithSpreadGenerator", input: .object()) { b, obj in | ||
| guard b.mode != .conservative else { return } | ||
|
|
||
| var methodName = b.type(of: obj).randomMethod() |
There was a problem hiding this comment.
Let's simplify this here and below to just let methodName = b.type(of: obj).randomMethod() ?? b.genMethodName()
| methodName = b.genMethodName() | ||
| } | ||
|
|
||
| var arguments: [Variable] = [] |
There was a problem hiding this comment.
Thinking about it again, I think we should still try to prefer spreading iterable objects, since attempting to spread most values will lead to exceptions (e.g. TypeError: Found non-callable @@iterator for a simple object). So how about turning lines 267-274, and the same code below, into
var arguments: [Variable] = []
var spreads: [Bool] = []
for _ in 0...Int.random(in: 3...5) {
let val = b.randVar()
arguments.append(val)
// Prefer to spread values that we know are iterable, as non-iterable values will lead to exceptions ("TypeError: Found non-callable @@iterator")
if b.type(of: val).Is(.iterable) {
spreads.append(probability(0.9))
} else {
spreads.append(probability(0.1))
}
}And, since this is I think required 4 times now, WDYT about moving it into a new ProgramBuilder.randCallArgumentsWithSpreading(n: Int) -> (arguments: [Variable], spreads: [Bool]) method?
There was a problem hiding this comment.
Nice! I like this approach
|
Nice, thanks! |
No description provided.