steinwurf / cpuid

C++ library for detecting CPU capabilities
https://cpuid.steinwurf.com
Other
105 stars 21 forks source link

Test parameters #4

Closed nestorjhernandezm closed 10 years ago

nestorjhernandezm commented 10 years ago

Hi @mortenvp and @jpihl !

I decided to include the commits for you to be able to see it :)

Tomorrow I will add more functionalities :+1:

mortenvp commented 10 years ago

Cool did you try if the argument parsing works?

nestorjhernandezm commented 10 years ago

Hi @mortenvp and @jpihl , please take a look. I had some issues with the extern variables but Jeppe gave me a hand to do the trick. Thanks Jeppe! :+1:

Besides other reviews that we may find here, Jeppe was pointing out the fact of using the extern variables instead of Google Test Framework's TEST_P or testing::AddGlobalTestEnvironment(); but I think at the very end it is the same.

Another suggestion is to not define commandline_arguments class but instead calling directly from boost::program_options because the cpuid repo should consist only of cpu*.hpp files from a design point of view. What do you think @petya2164 and @janusheide ?

mortenvp commented 10 years ago

@nestorjhernandezm looks I would say merge after removing the help message (if you agree?)

nestorjhernandezm commented 10 years ago

Great guys! Thanks for your feedback. Will do the changes now then!

nestorjhernandezm commented 10 years ago

@mortenvp and @jpihl please take a look. The only thing remaining is how to remove the warnings and notes of the boost libraries if it is possible.

They are something like:

/home/nestor/steinwurf_projects/bundle_dependencies/boost-11f274/1.1.0/boost/range/algorithm/equal.hpp:185:13: note: in expansion of macro ‘BOOST_RANGE_CONCEPT_ASSERT’
             BOOST_RANGE_CONCEPT_ASSERT(( SinglePassRangeConcept<const SinglePassRange1> ));
             ^
/home/nestor/steinwurf_projects/bundle_dependencies/boost-11f274/1.1.0/boost/concept/detail/general.hpp:71:20: warning: typedef ‘boost_concept_check186’ locally defined but not used [-Wunused-local-typedefs]
       BOOST_PP_CAT(boost_concept_check,__LINE__)
                    ^
/home/nestor/steinwurf_projects/bundle_dependencies/boost-11f274/1.1.0/boost/preprocessor/cat.hpp:29:34: note: in definition of macro ‘BOOST_PP_CAT_I’
 #    define BOOST_PP_CAT_I(a, b) a ## b
                                  ^
In file included from /home/nestor/steinwurf_projects/bundle_dependencies/boost-11f274/1.1.0/boost/program_options/value_semantic.hpp:14:0,
                 from /home/nestor/steinwurf_projects/bundle_dependencies/boost-11f274/1.1.0/boost/program_options/options_description.hpp:13,
                 from /home/nestor/steinwurf_projects/bundle_dependencies/boost-11f274/1.1.0/boost/program_options.hpp:15,
                 from ../../test/commandline_arguments.hpp:3,
                 from ../../test/commandline_arguments.cpp:4:
/home/nestor/steinwurf_projects/bundle_dependencies/boost-11f274/1.1.0/boost/lexical_cast.hpp: In function ‘bool boost::detail::lcast_ret_unsigned(T&, const CharT*, const CharT*)’:
/home/nestor/steinwurf_projects/bundle_dependencies/boost-11f274/1.1.0/boost/lexical_cast.hpp:868:47: warning: typedef ‘int_type’ locally defined but not used [-Wunused-local-typedefs]
             typedef typename Traits::int_type int_type;
                                               ^
mortenvp commented 10 years ago

It looks good - what we could do is to add an example which prints what capabilities your cpu has so that it is easy to copy-past when trying to run the unit tests on your own machine :)

mortenvp commented 10 years ago

Btw unfortunately there is not much we can do about those warnings - besides trying to upgrade Boost I think.

nestorjhernandezm commented 10 years ago

@mortenvp how do i create the binary example? Should I just modify the wscript to include new *.cpp files or should just add different tests in the test path?

mortenvp commented 10 years ago

Look how we do it in the kodo project - there we have an examples folder. The examples are included in the main wscript file so that they are also compiled when compiling the project. Simply do the same and then make one called print_cpuinfo or something similar. Let me know if you need help with it.

nestorjhernandezm commented 10 years ago

@mortenvp Done! Let me know if you are ok with this to merge it

mortenvp commented 10 years ago

I added a few minor comments otherwise I think we can merge this one and start the next :)

nestorjhernandezm commented 10 years ago

Excellent @mortenvp ! Then we can do the source-file splitting. I will make the changes and merge

mortenvp commented 10 years ago

Ok, do you mean the splitting into cpuinfo.hpp and cpuinfo_x86.cpp etc.?

nestorjhernandezm commented 10 years ago

Yep

mortenvp commented 10 years ago

Ok :+1: