[Insight-developers] itkSetClampMacro -- warnings for unsigned int values?

[Bill Lorensen]

I would say, in that case,use itkSetMacro.

On Mon, Jul 18, 2011 at 1:44 PM, Williams, Norman K
<norman-k-williams at uiowa.edu> wrote:
> itkSetClampUnsignedMacro is an OK idea but it isn't simple to implement
> effectively.
>
>
> The problem in this case is this: When you call itkSetClampMacro one of
> the parameters is the lower limit.  This is a numeric constant, but not
> necessarily zero.
>
> This problem only occurs if you use itkSetClampMacro thusly -- as happens
> in the FEMRegistrationFilter
>
>  itkSetClampMacro(MaxLevel, unsigned int, 0, SomeUnsignedMaximum );
>
> In this particular case the usage of itkSetClampMacro is, not to put too
> fine a point on it, stupid:
>
>
>  itkSetClampMacro(MaxLevel, unsigned int, 0, NumericTraits<unsigned
> int>::max() );
>
> What does it mean to clamp a value to the entire numeric range of the type
> being used?  It's meaningless.  Especially since when the macro is
> expanded, you get
>
>
> virtual void SetMaxLevel (unsigned int _arg)
>    {
>  itkDebugMacro("setting " << #name " to " << _arg);
>  if ( this->m_MaxLevel != ( _arg < 0 ? 0 : ( _arg >
> NumericTraits<unsigned int>::max() ? NumericTraits<unsigned int>::max() :
> _arg ) ) )
>    {
>    this->m_MaxLevel = ( _arg < 0 ? 0 : ( _arg > NumericTraits<unsigned
> int>::max() ? NumericTraits<unsigned int>::max() : _arg ) );
>    this->Modified();
>    }
>    }
>
> The problem with this is evident if you do something like this:
>
>
> unsigned long bogus = NumericTraits<unsigned int>::max();
> bogus *= 2;
> object->SetMaxLevel(bogus);
>
> Trying to clamp the value in the body of the function is closing the barn
> door after the piggie have escaped; the compiler will use its built-in
> unsigned-long-to-unsigned-int conversion, which if I recall correctly
> amounts to truncation.
>
> On 7/18/11 11:51 AM, "Bill Lorensen" <bill.lorensen at gmail.com> wrote:
>
>>Perhaps a new macro:
>>itkSetClampUnsignedMacro
>>
>>On Mon, Jul 18, 2011 at 12:43 PM, David Cole <david.cole at kitware.com>
>>wrote:
>>> On Mon, Jul 18, 2011 at 12:30 PM, Williams, Norman K
>>> <norman-k-williams at uiowa.edu> wrote:
>>>> I'm trying the change out, to see what happens. It looks like it does
>>>> silence that warning, as it's valid to ask if an unsigned value is
>>>>greater
>>>> than or equal than zero.
>>>
>>> Sure it's valid, but it's still always true. In other words: may as
>>> well not have an "if" for such an expression... I'm surprised the
>>> compiler doesn't give a "conditional expression always true" warning.
>>>
>>>>
>>>>
>>>> I'm more concerned that itkSetClampMacro is used on types where that
>>>> comparison causes problem. It should not cause a problem with any POD
>>>> type, but I'm not sure whether its use is limited to POD.
>>>>
>>>> On 7/18/11 11:03 AM, "David Cole" <david.cole at kitware.com> wrote:
>>>>
>>>>>On Mon, Jul 18, 2011 at 11:43 AM, Williams, Norman K
>>>>><norman-k-williams at uiowa.edu> wrote:
>>>>>> This throws compiler warnings:
>>>>>>
>>>>>>  itkSetClampMacro(MaxLevel, unsigned int, 0, NumericTraits<unsigned
>>>>>> int>::max() );
>>>>>>
>>>>>> Specifically, it uses the expression _arg < 0, which is always false.
>>>>>>
>>>>>>
>>>>>> This is only seen in the '-Wall -Wextra' world.
>>>>>>
>>>>>> Possible solution -- change itkMacro.h and replace
>>>>>>
>>>>>> _arg < min
>>>>>>
>>>>>> with
>>>>>>
>>>>>> !(_arg >= min)
>>>>>>
>>>>>
>>>>>Won't this just change the warning to "this condition is always true"
>>>>>..... ?
>>>>>
>>>>>
>>>>>> This is a niggling point but eliminating specious and spurious
>>>>>>warnings
>>>>>> will make it easier to see the real warnings.
>>>>>>
>>>>>> I imagine that once a compiler optimizes the code in that macro, the
>>>>>>end
>>>>>> result will be nearly the same.
>>>>>> --
>>>>>> Kent Williams norman-k-williams at uiowa.edu
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> ________________________________
>>>>>> Notice: This UI Health Care e-mail (including attachments) is covered
>>>>>>by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is
>>>>>>confidential and may be legally privileged.  If you are not the
>>>>>>intended
>>>>>>recipient, you are hereby notified that any retention, dissemination,
>>>>>>distribution, or copying of this communication is strictly prohibited.
>>>>>>Please reply to the sender that you have received the message in
>>>>>>error,
>>>>>>then delete it.  Thank you.
>>>>>> ________________________________
>>>>>> _______________________________________________
>>>>>> Powered by www.kitware.com
>>>>>>
>>>>>> Visit other Kitware open-source projects at
>>>>>> http://www.kitware.com/opensource/opensource.html
>>>>>>
>>>>>> Kitware offers ITK Training Courses, for more information visit:
>>>>>> http://kitware.com/products/protraining.html
>>>>>>
>>>>>> Please keep messages on-topic and check the ITK FAQ at:
>>>>>> http://www.itk.org/Wiki/ITK_FAQ
>>>>>>
>>>>>> Follow this link to subscribe/unsubscribe:
>>>>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>>>>
>>>>
>>>>
>>>>
>>>> ________________________________
>>>> Notice: This UI Health Care e-mail (including attachments) is covered
>>>>by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is
>>>>confidential and may be legally privileged.  If you are not the
>>>>intended recipient, you are hereby notified that any retention,
>>>>dissemination, distribution, or copying of this communication is
>>>>strictly prohibited.  Please reply to the sender that you have received
>>>>the message in error, then delete it.  Thank you.
>>>> ________________________________
>>>> _______________________________________________
>>>> Powered by www.kitware.com
>>>>
>>>> Visit other Kitware open-source projects at
>>>> http://www.kitware.com/opensource/opensource.html
>>>>
>>>> Kitware offers ITK Training Courses, for more information visit:
>>>> http://kitware.com/products/protraining.html
>>>>
>>>> Please keep messages on-topic and check the ITK FAQ at:
>>>> http://www.itk.org/Wiki/ITK_FAQ
>>>>
>>>> Follow this link to subscribe/unsubscribe:
>>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>>
>>> _______________________________________________
>>> Powered by www.kitware.com
>>>
>>> Visit other Kitware open-source projects at
>>> http://www.kitware.com/opensource/opensource.html
>>>
>>> Kitware offers ITK Training Courses, for more information visit:
>>> http://kitware.com/products/protraining.html
>>>
>>> Please keep messages on-topic and check the ITK FAQ at:
>>> http://www.itk.org/Wiki/ITK_FAQ
>>>
>>> Follow this link to subscribe/unsubscribe:
>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>
>
>
>
> ________________________________
> Notice: This UI Health Care e-mail (including attachments) is covered by the Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential and may be legally privileged.  If you are not the intended recipient, you are hereby notified that any retention, dissemination, distribution, or copying of this communication is strictly prohibited.  Please reply to the sender that you have received the message in error, then delete it.  Thank you.
> ________________________________
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Kitware offers ITK Training Courses, for more information visit:
> http://kitware.com/products/protraining.html
>
> Please keep messages on-topic and check the ITK FAQ at:
> http://www.itk.org/Wiki/ITK_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.itk.org/mailman/listinfo/insight-developers
>