Brad King wrote: > 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?
Good idea. Pushed to next with this modification. > > > 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. For now I've removed the 'new API' parts of the branch. I'll keep this in mind when I re-add them when I add the first generator expression capability to target properties. > >> 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}' Thanks. Added an alias. > >> 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. Right. I investigated a little bit already and it seems there's at least some low-hanging fruit. Thanks, Steve. -- 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