> On Jan 10, 2017, at 10:17 PM, Filip Pizlo <[email protected]> wrote:
>
> while (Arg src = worklist.pop()) {
> HashMap<Arg, Vector<ShufflePair>>::iterator iter =
> mapping.find(src);
> if (iter == mapping.end()) {
> // With a shift it's possible that we previously built
> the tail of this shift.
> // See if that's the case now.
> if (verbose)
> dataLog("Trying to append shift at ", src, "\n");
> currentPairs.appendVector(shifts.take(src));
> continue;
> }
> Vector<ShufflePair> pairs = WTFMove(iter->value);
> mapping.remove(iter);
>
> for (const ShufflePair& pair : pairs) {
> currentPairs.append(pair);
> ASSERT(pair.src() == src);
> worklist.push(pair.dst());
> }
> }
Here is the version I would write in my favored coding style:
while (auto source = workList.pop()) {
auto foundSource = mapping.find(source);
if (foundSource == mapping.end()) {
// With a shift it's possible that we previously built the tail of
this shift.
// See if that's the case now.
if (verbose)
dataLog("Trying to append shift at ", source, "\n");
currentPairs.appendVector(shifts.take(source));
continue;
}
auto pairs = WTFMove(foundSource->value);
mapping.remove(foundSource);
for (auto& pair : pairs) {
currentPairs.append(pair);
ASSERT(pair.source() == source);
workList.push(pair.destination());
}
}
You argued that specifying the type for both source and for the iterator helps
reassure you that the types match. I am not convinced that is an important
interesting property unless the code has types that can be converted, but with
conversions that we must be careful not to do. If there was some type that
could be converted to Arg or that Arg could be converted to that was a
dangerous possibility, then I grant you that, although I still prefer my
version despite that.
You also said that it’s important to know that the type of foundSource->value
matches the type of pairs. I would argue that’s even clearer in my favored
style, because we know that auto guarantees that are using the same type for
both, although we don’t state explicitly what that type is.
Here’s one thing to consider: Why is it important to see the type of
foundSource->value, but not important to see the type of shifts.take(source)?
In both cases they need to be Vector<ShufflePair>.
— Darin
_______________________________________________
webkit-dev mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-dev
