On 09/12/2012 11:36 AM, Stephen Kelly wrote: > I've force-pushed the generator-expression-refactor branch to my gitorious > clone.
Great progress! One thing that bothers me about the current design is that the cmGeneratorExpression instances now have more states. This pattern appears everywhere they're used now: + ge.Parse(*cli); + ge.Evaluate(this->Makefile, 0, true); If one forgets the first line there will be runtime errors. Can you split the class up so that ge.Parse() returns a "compiled" generator expression as a separate object that provides the Evaluate() interface? This documentation needs clarification: + "This generator expression can only be evaluated if it is used in " \ + "target scope, such as a target property.\n" \ All expressions are evaluated in "target scope" when used to generate build rules. Only expressions used in add_test calls are evaluated without a "this" target. Please refactor the way the documentation of generator expressions is produced so that the copy that appears in the docs of add_test does not mention expressions it can't use. > The unit tests pass for me, and I think it's ready to merge to next. I don't > remember the magic git log command to check the style for text width though? git log origin/master.. --pickaxe-regex -S'.{80}' > The next thing I want to do is to see if I can make full use of > cmGeneratorTarget and use const proactively while doing so, to make the code > more clear. I'm not 100% certain that's possible, but it's the next goal I > have. Any comments on that? That would be a really great example of splitting configure-time constructs from generate-time constructs. -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers