https://codereview.chromium.org/816053002/diff/40001/test/unittests/compiler/graph-unittest.h
File test/unittests/compiler/graph-unittest.h (right):
https://codereview.chromium.org/816053002/diff/40001/test/unittests/compiler/graph-unittest.h#newcode78
test/unittests/compiler/graph-unittest.h:78:
Well this kind of goes against the purpose of GraphTest. GraphTest
should be a base class for Graph tests and as independent and generic as
possible. So please move these to loop-peeling-unittest.cc and only
hoist them into a header when really necessary. And if you only need a
Graph and CommonOperatorBuilder, then pass them as parameters. GraphTest
should not be used as a way to pass around Graph+CommonOperatorBuilder.
https://codereview.chromium.org/816053002/diff/40001/test/unittests/compiler/graph-unittest.h#newcode80
test/unittests/compiler/graph-unittest.h:80: struct While {
Personally I don't like these helper structs at all, esp. not in header
files. IMHO it's easier to read the code without these helpers, even if
the code is twice as long, but then you don't need to lookup/remember
the helper magic. The Diamond class already caused quite some trouble.
But I'm fine with these as long as they are local within the .cc file,
ideally within the class that they are built for.
https://codereview.chromium.org/816053002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
