Standalone boost::ignore_unused_variable_warning()
Hi, The ignore_unused_variable_warning() tool is used in many libraries. Its "main" definition (probably the first one) is in concept_check.hpp in the namespace boost but functions of the same name and purpose are also defined in the internals of some libraries (GIL, Interval, Test, uBlas, Unordered), probably to not include the whole concept_check.hpp. So I propose to move this useful tool out of the concept_check.hpp to somewhere else. Since it was already defined in the namespace boost, not boost::detail, the right place would probably be: boost/utility/ignore.hpp or boost/utility/ignore_warning.hpp Furthermore I propose to add a few overloads becasue it's common that the compiler warns about a few variables in the same function, so instead of using N calls: boost::ignore_unused_variable_warning(x) boost::ignore_unused_variable_warning(y) boost::ignore_unused_variable_warning(z) we could use only one: boost::ignore_unused_variable_warning(x, y, z) which would be more convenient to use and read. If you were ok with the change, I'd be glad to hear some tips how to make it in one commit, i.e. to modify two modules at once - modify a file in ConceptCheck and add a file to Utility? And finally I'm considering adding similar tool for unused typedefs because some compilers (GCC4.8) warns about them and there are cases, e.g. when some part of code is shielded with some #ifdef, where they can't be commented out or removed. E.g. something like this would work, though I'm not sure if there is no better way, so I'm open to suggestion. struct empty_type {}; template <typename T1, typename T2 = empty_type, typename T3 = empty_type> struct ignore_unused_typedef_warning {}; int main() { typedef int T1; typedef int T2; ignore_unused_typedef_warning<T1, T2>(); } Regards, Adam
2014-05-18 16:56 GMT+04:00 Adam Wulkiewicz <adam.wulkiewicz@gmail.com>:
Hi,
The ignore_unused_variable_warning() tool is used in many libraries. Its "main" definition (probably the first one) is in concept_check.hpp in the namespace boost but functions of the same name and purpose are also defined in the internals of some libraries (GIL, Interval, Test, uBlas, Unordered), probably to not include the whole concept_check.hpp.
So I propose to move this useful tool out of the concept_check.hpp to somewhere else.
+1
Since it was already defined in the namespace boost, not boost::detail, the right place would probably be:
boost/utility/ignore.hpp
or
boost/utility/ignore_warning.hpp
Maybe boost/utility/unused_variable.hpp and boost/utility/unused_typedef.hpp would be better?
Furthermore I propose to add a few overloads becasue it's common that the compiler warns about a few variables in the same function, so instead of using N calls:
boost::ignore_unused_variable_warning(x) boost::ignore_unused_variable_warning(y) boost::ignore_unused_variable_warning(z)
we could use only one:
boost::ignore_unused_variable_warning(x, y, z)
which would be more convenient to use and read.
+1
If you were ok with the change, I'd be glad to hear some tips how to make it in one commit, i.e. to modify two modules at once - modify a file in ConceptCheck and add a file to Utility?
As far as I know it is impossible to modify two modules in one commit.
And finally I'm considering adding similar tool for unused typedefs because some compilers (GCC4.8) warns about them and there are cases, e.g. when some part of code is shielded with some #ifdef, where they can't be commented out or removed. E.g. something like this would work, though I'm not sure if there is no better way, so I'm open to suggestion.
struct empty_type {};
Just move it into some detail namespace (like boost::detail::utility) and do not define when we have variadic templates. I'd be happy to have such tool, it is much more clear than (void)x; -- Best regards, Antony Polukhin
Hi, 2014-05-18 18:29 GMT+02:00 Antony Polukhin <antoshkka@gmail.com>:
Maybe boost/utility/unused_variable.hpp and boost/utility/unused_typedef.hpp would be better?
Yes, that's better. I requested a pull for unused_variable: https://github.com/boostorg/utility/pull/8 unused_typedef requires more thinking. Regards, Adam
Adam Wulkiewicz wrote:
I requested a pull for unused_variable: https://github.com/boostorg/utility/pull/8
It should be just ignore_unused_variable, IMO. It doesn't ignore the warning, it suppresses or avoids it.
Hi, 2014-05-19 15:00 GMT+02:00 Peter Dimov <lists@pdimov.com>:
Adam Wulkiewicz wrote:
I requested a pull for unused_variable: https://github.com/boostorg/utility/pull/8
It should be just ignore_unused_variable, IMO. It doesn't ignore the warning, it suppresses or avoids it.
To be semantically correct it could be named use_variable() or use_variables(). Or just unused_variable() / unused_variables() meaning "hey! those are unused variables!". And if the name was changed we might as well consider putting it in the different namespace, e.g. boost::utility::unused_variables(). The currently used name is for backward compatibility. We can change it or provide both and deprecate the old one. Or just leave currently defined one in the ConceptCheck but in this case this pull request has less sense than it could. Regards, Adam
On Monday 19 May 2014 16:40:05 Adam Wulkiewicz wrote:
Hi,
2014-05-19 15:00 GMT+02:00 Peter Dimov <lists@pdimov.com>:
Adam Wulkiewicz wrote:
I requested a pull for unused_variable: https://github.com/boostorg/utility/pull/8
It should be just ignore_unused_variable, IMO. It doesn't ignore the warning, it suppresses or avoids it.
To be semantically correct it could be named use_variable() or use_variables(). Or just unused_variable() / unused_variables() meaning "hey! those are unused variables!". And if the name was changed we might as well consider putting it in the different namespace, e.g. boost::utility::unused_variables().
The currently used name is for backward compatibility. We can change it or provide both and deprecate the old one. Or just leave currently defined one in the ConceptCheck but in this case this pull request has less sense than it could.
I'm in favor of renaming. ignore_unused_variable or suppress_unused_variable_warning looks more appealing to me. use_variable means "do something with it" to me, which is not the intention of the component. Is this a documented feature of ConceptCheck? If not, backward compatibility is not an issue.
2014-05-19 16:48 GMT+02:00 Andrey Semashev <andrey.semashev@gmail.com>:
On Monday 19 May 2014 16:40:05 Adam Wulkiewicz wrote:
Hi,
2014-05-19 15:00 GMT+02:00 Peter Dimov <lists@pdimov.com>:
Adam Wulkiewicz wrote:
I requested a pull for unused_variable: https://github.com/boostorg/utility/pull/8
It should be just ignore_unused_variable, IMO. It doesn't ignore the warning, it suppresses or avoids it.
To be semantically correct it could be named use_variable() or use_variables(). Or just unused_variable() / unused_variables() meaning "hey! those are unused variables!". And if the name was changed we might as well consider putting it in the different namespace, e.g. boost::utility::unused_variables().
The currently used name is for backward compatibility. We can change it or provide both and deprecate the old one. Or just leave currently defined one in the ConceptCheck but in this case this pull request has less sense than it could.
I'm in favor of renaming. ignore_unused_variable or suppress_unused_variable_warning looks more appealing to me. use_variable means "do something with it" to me, which is not the intention of the component.
Is this a documented feature of ConceptCheck? If not, backward compatibility is not an issue.
It's only mentioned here: http://www.boost.org/doc/libs/1_55_0/libs/concept_check/implementation.htm but it's used probably in every library which is using ConceptCheck. Plus a few libraries defines it with the same name in their details. So to allow the maintainers to remove their definition and not force them to modify every file where it's used we should probably leave this 1-parameter version of boost::ignore_unused_variable_warning() and provide more flexible replacements. Regards, Adam
On Sun, May 18, 2014 at 5:56 AM, Adam Wulkiewicz <adam.wulkiewicz@gmail.com>wrote:
we could use only one:
boost::ignore_unused_variable_warning(x, y, z)
which would be more convenient to use and read.
If you were ok with the change, I'd be glad to hear some tips how to make it in one commit, i.e. to modify two modules at once - modify a file in ConceptCheck and add a file to Utility?
And finally I'm considering adding similar tool for unused typedefs because some compilers (GCC4.8) warns about them and there are cases, e.g. when some part of code is shielded with some #ifdef, where they can't be commented out or removed. E.g. something like this would work, though I'm not sure if there is no better way, so I'm open to suggestion.
I support these going in, though I have a suggestion for the unused typedef warning -- you may want to be able to do this outside of a function definition (I.E. at global scope or in a class's body). For situations like this, I often find that using a static_assert may be better than an expression, as it can be written almost anywhere. In other words, something along the lines of: ///// static_assert( ignore_unused_typedef< T1, T2 >::value, "This will never fire." ); ///// where the ignore_unused_typedef metafunction is basically just an always_true metafunction. In other words, I suggest you simply have it inherit from std::true_type or boost::mpl::true_. A macro alias that hides the static_assert and unseen message may also be beneficial. -- -Matt Calabrese
Hi, 2014-05-19 3:58 GMT+02:00 Matt Calabrese <rivorus@gmail.com>:
On Sun, May 18, 2014 at 5:56 AM, Adam Wulkiewicz <adam.wulkiewicz@gmail.com>wrote:
And finally I'm considering adding similar tool for unused typedefs because some compilers (GCC4.8) warns about them and there are cases, e.g. when some part of code is shielded with some #ifdef, where they can't be commented out or removed. E.g. something like this would work, though I'm not sure if there is no better way, so I'm open to suggestion.
I support these going in, though I have a suggestion for the unused typedef warning -- you may want to be able to do this outside of a function definition (I.E. at global scope or in a class's body). For situations like this, I often find that using a static_assert may be better than an expression, as it can be written almost anywhere. In other words, something along the lines of:
///// static_assert( ignore_unused_typedef< T1, T2 >::value, "This will never fire." ); /////
where the ignore_unused_typedef metafunction is basically just an always_true metafunction. In other words, I suggest you simply have it inherit from std::true_type or boost::mpl::true_. A macro alias that hides the static_assert and unseen message may also be beneficial.
Thanks for the suggestion! I also like the idea of a macro which could be put anywhere and only take types as parameters. There are some problems with the above example but this gave me an idea. The types could be passed as a parenthized list: BOOST_IGNORE_UNUSED_TYPEDEF_WARNING((T1, T2, T3)); then internally used e.g. to build some function type or just passed to the BOOST_MPL_ASSERT_MSG as the last parameter. So as a reminder the other approach is: ignore_unused_typedef_warning<T1, T2, T3>(); Any other ideas? Regards, Adam
Adam Wulkiewicz wrote:
There are some problems with the above example but this gave me an idea. The types could be passed as a parenthized list:
BOOST_IGNORE_UNUSED_TYPEDEF_WARNING((T1, T2, T3));
then internally used e.g. to build some function type or just passed to the BOOST_MPL_ASSERT_MSG as the last parameter.
So as a reminder the other approach is:
ignore_unused_typedef_warning<T1, T2, T3>();
I'm in favor of BOOST_IGNORE_UNUSED_TYPEDEF( T1 ) BOOST_IGNORE_UNUSED_TYPEDEFS(( T1, T2, T3 )) (Lack of semicolon is intentional and not an omission on my part.) A macro would allow us to vary the implementation in the appropriate compiler-specific way, or to omit it altogether on compilers that do not warn.
2014-05-19 17:07 GMT+02:00 Peter Dimov <lists@pdimov.com>:
Adam Wulkiewicz wrote:
There are some problems with the above example but this gave me an idea. The types could be passed as a parenthized list:
BOOST_IGNORE_UNUSED_TYPEDEF_WARNING((T1, T2, T3));
then internally used e.g. to build some function type or just passed to the BOOST_MPL_ASSERT_MSG as the last parameter.
So as a reminder the other approach is:
ignore_unused_typedef_warning<T1, T2, T3>();
I'm in favor of
BOOST_IGNORE_UNUSED_TYPEDEF( T1 ) BOOST_IGNORE_UNUSED_TYPEDEFS(( T1, T2, T3 ))
(Lack of semicolon is intentional and not an omission on my part.)
A macro would allow us to vary the implementation in the appropriate compiler-specific way, or to omit it altogether on compilers that do not warn.
Then probably also boost::ignore_unused_variable(v1) ; boost::ignore_unused_variables(v1, v2, v3); for consistency. Or would putting it in boost::utility/BOOST_UTILITY be a better idea? Regards, Adam
2014-05-19 19:26 GMT+04:00 Adam Wulkiewicz <adam.wulkiewicz@gmail.com>: <...>
Then probably also
boost::ignore_unused_variable(v1) ; boost::ignore_unused_variables(v1, v2, v3);
for consistency.
I'd prefer to see a single function name, not two: boost::ignore_unused_variables(v1) ; boost::ignore_unused_variables(v1, v2, v3);
Or would putting it in boost::utility/BOOST_UTILITY be a better idea?
boost::utility:: namespace looks reasonable. +1 -- Best regards, Antony Polukhin
On Mon, May 19, 2014 at 10:07 AM, Antony Polukhin wrote:
I'd prefer to see a single function name, not two:
boost::ignore_unused_variables(v1) ; boost::ignore_unused_variables(v1, v2, v3);
[snip]
boost::utility:: namespace looks reasonable. +1
It feels more like a workaround, than a utility, and so I would have thought that: a. The more natural home is Boost.Config b. The macro approaches were preferable I also like the idea of not adding new things to Boost.Utility going forward (even if they only depend on Boost.Config). Best, Glen
Hi, Glen Fernandes wrote:
On Mon, May 19, 2014 at 10:07 AM, Antony Polukhin wrote:
I'd prefer to see a single function name, not two:
boost::ignore_unused_variables(v1) ; boost::ignore_unused_variables(v1, v2, v3);
[snip]
boost::utility:: namespace looks reasonable. +1
It feels more like a workaround, than a utility, and so I would have thought that: a. The more natural home is Boost.Config b. The macro approaches were preferable
I also like the idea of not adding new things to Boost.Utility going forward (even if they only depend on Boost.Config).
I wasn't aware that the Utility is such unconvenient library for adding new things. AFAIU Config provides macros which can be used in conditional compilation. AFAIK there are no "tools" there. Or am I wrong? I thought also about Detail but this tool was originally defined in namespace boost, and btw could be used by the users too. And I remember that some time ago there was a discussion about not adding things in the global namespace/directory. Hence Utility because it seemed to be a "library" for orphaned tools. But I have no preference regarding the placement. Regards, Adam
Antony Polukhin wrote:
2014-05-19 19:26 GMT+04:00 Adam Wulkiewicz <adam.wulkiewicz@gmail.com>:
Or would putting it in boost::utility/BOOST_UTILITY be a better idea?
boost::utility:: namespace looks reasonable. +1
I don't like it very much. There's nothing particularly utility-specific about ignore_unused_variable and if/when we decide to move it somewhere else, we'll have to fix all the uses. A namespace "utility" shouldn't even exist (except for implementation details).
On May 19, 2014 11:26:49 AM EDT, Adam Wulkiewicz <adam.wulkiewicz@gmail.com> wrote:
Then probably also
boost::ignore_unused_variable(v1) ; boost::ignore_unused_variables(v1, v2, v3);
for consistency.
Actually, I'd favor ignore_unused(). I see no reason for "variable" to be in the name. If you use a macro, then you'd need to distinguish between TYPEDEFs and VARIABLEs.
Or would putting it in boost::utility/BOOST_UTILITY be a better idea?
Glen's argument against config makes sense. Utility doesn't seem unreasonable. ___ Rob (Sent from my portable computation engine)
Hi, Rob Stewart wrote:
On May 19, 2014 11:26:49 AM EDT, Adam Wulkiewicz <adam.wulkiewicz@gmail.com> wrote:
Then probably also
boost::ignore_unused_variable(v1) ; boost::ignore_unused_variables(v1, v2, v3);
for consistency. Actually, I'd favor ignore_unused(). I see no reason for "variable" to be in the name. If you use a macro, then you'd need to distinguish between TYPEDEFs and VARIABLEs.
I like those new names but don't forget that function ignore_unused_variable_warning() is already used around Boost. Grep shows ~600 uses in the code of various libraries and ~100 in the docs. And to be honest, I like this long name. When I read it I have an impression that it's not a part of the algorithm. But maybe I got used to it. I'm also curious what the author/maintainer of ConceptCheck thinks about it? Jeremy are you reading this thread? Regards, Adam
Hi, 2014-05-20 12:45 GMT+02:00 Adam Wulkiewicz <adam.wulkiewicz@gmail.com>:
Hi,
Rob Stewart wrote:
On May 19, 2014 11:26:49 AM EDT, Adam Wulkiewicz < adam.wulkiewicz@gmail.com> wrote:
Then probably also
boost::ignore_unused_variable(v1) ; boost::ignore_unused_variables(v1, v2, v3);
for consistency.
Actually, I'd favor ignore_unused(). I see no reason for "variable" to be in the name. If you use a macro, then you'd need to distinguish between TYPEDEFs and VARIABLEs.
I like those new names but don't forget that function ignore_unused_variable_warning() is already used around Boost. Grep shows ~600 uses in the code of various libraries and ~100 in the docs.
And to be honest, I like this long name. When I read it I have an impression that it's not a part of the algorithm. But maybe I got used to it.
Ok, lets finalize this! So, do you think we should: 1. Pick the new names and leave boost::ignore_unused_variable_warning() in Boost.Concept 2. Pick the new names and move the old implementation from Boost.Concept to the same place? 3. Use the old name and remove the old implementation from Boost.Concept? Btw, Rob I think your proposal is very elegant: boost::ignore_unused(v1, v2, v3); boost::ignore_unused<T1, T2, T3>();
I'm also curious what the author/maintainer of ConceptCheck thinks about it? Jeremy are you reading this thread?
I guess not. Regards, Adam
On May 19, 2014 11:07:19 AM EDT, Peter Dimov <lists@pdimov.com> wrote:
Adam Wulkiewicz wrote:
The types could be passed as a parenthized list:
BOOST_IGNORE_UNUSED_TYPEDEF_WARNING((T1, T2, T3)); [snip] So as a reminder the other approach is: ignore_unused_typedef_warning<T1, T2, T3>();
I'm in favor of BOOST_IGNORE_UNUSED_TYPEDEF( T1 ) BOOST_IGNORE_UNUSED_TYPEDEFS(( T1, T2, T3 ))
A macro would allow us to vary the implementation in the appropriate compiler-specific way, or to omit it altogether on compilers that do not warn.
+1 Macros for unused variables would provide consistency and give similar implementation flexibility, though the latter might be unwarranted. ___ Rob (Sent from my portable computation engine)
On Mon, May 19, 2014 at 6:58 AM, Adam Wulkiewicz <adam.wulkiewicz@gmail.com>wrote:
Thanks for the suggestion! I also like the idea of a macro which could be put anywhere and only take types as parameters. There are some problems with the above example but this gave me an idea. The types could be passed as a parenthized list:
BOOST_IGNORE_UNUSED_TYPEDEF_WARNING((T1, T2, T3));
then internally used e.g. to build some function type or just passed to the BOOST_MPL_ASSERT_MSG as the last parameter.
There is a subtle problem with this -- you can't necessarily guarantee that forming a function type is valid. For instance, if one of your typedefs is a void type, forming a function type with it as a parameter will fail. -- -Matt Calabrese
participants (7)
-
Adam Wulkiewicz -
Andrey Semashev -
Antony Polukhin -
Glen Fernandes -
Matt Calabrese -
Peter Dimov -
Rob Stewart