Daniel Pfeifer wrote: > On Wed, Feb 10, 2016 at 12:12 AM, Stephen Kelly > <steve...@gmail.com> wrote: >> 3) Compute cmGeneratorTarget state non-lazily in its constructor. >> * Historically target state for generators was computed lazily because it >> might need to be cleared and re-computed. That is no-longer true. > > SourceFilesMap is cleared in a call to AddSource. It is then > recomputed the next time GetSourceFiles is called.
Hmm, maybe there is a way to reduce the amount of times it is cleared, once the computation sequence is better defined by being extracted to cmComputeTarget. cmGeneratorTarget::AddSource is currently called from several generate-time locations, some of which could become compute-time locations. The uses in * cmQtAutoGeneratorInitializer::InitializeAutogenSources * cmGlobalVisualStudio8Generator::AddCheckTarget * cmLocalVisualStudio7Generator::CreateVCProjBuildRule * cmGlobalXCodeGenerator seem that way to me, because the file names don't depend on anything from the cmGeneratorTarget which is only known at build time. Unless I'm missing something, they could be added to the list of files for the cmComputeTarget at compute time before the SourceFilesMap is populated at generate time. The tricky case is when AddSource is called with something computed from the result of cmGeneratorTarget::GetSources. The only case of that seems to be SetupSourceFiles in cmQtAutoGeneratorInitializer. That calls AddSource in a loop. If cmComputeTarget gains an AddSource and doesn't have a SourceFilesMap, then that would be the only use of cmGeneratorTarget::AddSource. That could then be replaced with a new AddSources or AddSourcesForRCC, or something better might be possible. This is just a brain-dump which I haven't investigated. You might find some facts which make the source addition refactoring to compute time to be impossible. Then maybe the entries of the ComputedTargets map might not have to be recomputed lazily. > Please review: > https://github.com/purpleKarrot/CMake/commits/computed-target Cool, thanks for working on this! I am a bit confused by the first commit in the branch. It removes some c++ template code while adding cmComputeTarget. I think that's because you make the working class inherit from cmComputeTarget, and you do that just to re- use the data members, and then later in the branch, to re-use the cmComputeTarget itself. Is it possible to split what is happening in that commit? Is it possible to remove the template code first without introducing cmComputeTarget? Even if doing so would mean introducing a struct with the same data members early in the branch and perhaps removing that struct later, I think it might make the commit more clear. Currently I don't understand it. I also generally recommend to put the most obvious/cleanup commits at the start of the branch (or even in a separate minor-cleanups branch). The 'don't clear container in destructor' commit and the 'use erase-unique instead of reinitialization' commit and the 'factor out common part of AddSources commands' all seem to fit that description. I was able to rebase them at least, but I didn't try building the result. Getting those kinds of commits out of the way first makes the rest of the branch smaller and focused on extending the code in only one specific way. Thanks, Steve. -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers