Re: [cmake-developers] cmLocalGenerator::GetTargetFlags

2016-06-09 Thread Brad King
On 06/09/2016 11:28 AM, Ben Boeckel wrote:
> On Wed, Jun 08, 2016 at 10:12:13 -0400, Brad King wrote:
>> Side note: This is another thing that should be cleaned up in the Ninja
>> generator.  It should hold target-wide flags in the per-target rules.ninja
>> entries instead of duplicating them for every object file in build.ninja.
>> That would make the build.ninja files smaller.
> 
> Don't we already do this with target-specific build rules in
> rules.ninja?

We converted to target-specific rules but IIRC have not actually
moved all the flags there.  The build.ninja file still uses the
rules but spells out all the flags on every object.

-Brad

-- 

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


Re: [cmake-developers] cmLocalGenerator::GetTargetFlags

2016-06-09 Thread Ben Boeckel
On Wed, Jun 08, 2016 at 10:12:13 -0400, Brad King wrote:
> Side note: This is another thing that should be cleaned up in the Ninja
> generator.  It should hold target-wide flags in the per-target rules.ninja
> entries instead of duplicating them for every object file in build.ninja.
> That would make the build.ninja files smaller.

Don't we already do this with target-specific build rules in
rules.ninja?

--Ben
-- 

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


Re: [cmake-developers] cmLocalGenerator::GetTargetFlags

2016-06-08 Thread Brad King
Hi Tobias,

So it looks like I was completely wrong in pointing you at GetTargetFlags
to get compiler flags.  It is indeed for linker flags.

On 06/08/2016 04:23 AM, Tobias Hunger wrote:
> what use is the "flags" out-parameter in cmLocalGenerator::GetTargetFlags? It 
> is
> only ever set for targets of type EXECUTABLE and is filled based on the 
> *linker
> language* of the target.
> 
> I really do not see any use for this information, and indeed the flag seems
> unused in the cmake code base, except for the Ninja generator, where that may 
> or
> may not make sense (probably not).

That is because when we invoke the C compiler to drive linking we pass
the compiler flags from CMAKE_C_FLAGS (and similarly for CXX).  For
non-executables the same thing is done in a different code path here:

  
https://cmake.org/gitweb?p=cmake.git;a=blob;f=Source/cmNinjaNormalTargetGenerator.cxx;hb=v3.6.0-rc1#l471

I think the comment about a mistake in the Ninja generator is telling.
The AddLanguageFlags is done there for non-executables and is done in
GetTargetFlags for executables.  This should be refactored and made
consistent.

> Can the flag get removed altogether and the method get renamed to
> GetTargetLinkerFlags?

+1 for renaming.  The best place for AddLanguageFlags to be called
needs to be determined.  Note that the Makefile generators use
cmCommonTargetGenerator::AddFeatureFlags to call AddLanguageFlags,
and that follows up with some additional flags for IPO.  That may
be another case that is broken in the Ninja generator.

> Maybe it could return the linker language instead, that is
> a piece of data that is more important in its context IMHO.

We already have cmGeneratorTarget::GetLinkerLanguage for that.

> While discussing this method: How about moving the method to 
> cmTargetGenerator?
> That would get rid of the target parameter. That is where I would actually 
> look
> for information on targets.

Yes, perhaps.  What may be more important is to consolidate this
with what the Makefile generator is doing.  There is no reason the
Ninja and Makefile generators to not share more code (and perhaps
other generators too, but VS and Xcode do not necessarily generate
the full command lines).  Once there is a single clear place for
this stuff to be computed then the daemon can use it with confidence.

> Which brings me back to my original question: How can I find the compiler 
> flags
> in use for a cmSourceFile/cmGeneratorTarget? The only place that I found that
> seems to do what I want is cmCommonTargetGenerator::GetFlags.

Yes, GetFlags it the correct place to get target-wide flags.  The Ninja
generator has cmNinjaTargetGenerator::ComputeFlagsForObject to also add
per-source flags.  The Makefile generator factors out the target-wide
flags into a make variable and then adds the same per-source flags
elsewhere.  The code that adds per-source flags should be factored
into a common helper that the daemon can then use along with GetFlags.

Side note: This is another thing that should be cleaned up in the Ninja
generator.  It should hold target-wide flags in the per-target rules.ninja
entries instead of duplicating them for every object file in build.ninja.
That would make the build.ninja files smaller.

> cmCommonTargetGenerator does caching of results. Should this be also done for
> cmLocalGenerator::GetTargetFlags, etc?

Not sure.  That is an optimization so may not be worth doing yet until
the needed refactoring is done first.

-Brad

-- 

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


[cmake-developers] cmLocalGenerator::GetTargetFlags

2016-06-08 Thread Tobias Hunger
Hi everybody,

what use is the "flags" out-parameter in cmLocalGenerator::GetTargetFlags? It is
only ever set for targets of type EXECUTABLE and is filled based on the *linker
language* of the target.

I really do not see any use for this information, and indeed the flag seems
unused in the cmake code base, except for the Ninja generator, where that may or
may not make sense (probably not).

Can the flag get removed altogether and the method get renamed to
GetTargetLinkerFlags? Maybe it could return the linker language instead, that is
a piece of data that is more important in its context IMHO.

While discussing this method: How about moving the method to cmTargetGenerator?
That would get rid of the target parameter. That is where I would actually look
for information on targets.



Which brings me back to my original question: How can I find the compiler flags
in use for a cmSourceFile/cmGeneratorTarget? The only place that I found that
seems to do what I want is cmCommonTargetGenerator::GetFlags. Or is that
concerned about cmake internal flags that need to get mapped to compiler flags
later? Is there another copy of that code in a place more convenient to use or
do I need to create that?

cmCommonTargetGenerator does caching of results. Should this be also done for
cmLocalGenerator::GetTargetFlags, etc?

Best Regards,
Tobias

-- 
Tobias Hunger, Senior Software Engineer | The Qt Company
The Qt Company GmbH, Rudower Chaussee 13, D-12489 Berlin
Geschäftsführer: Mika Pälsi, Juha Varelius, Mika Harjuaho. Sitz der
Gesellschaft: Berlin, Registergericht: Amtsgericht Charlottenburg, HRB 144331 B
-- 

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