New Xcode 7 const warning with vImageHistogramSpecification_ARGB8888()

[Sean McBride]

Hi all,

With Xcode 7 I have a new warning using vImageHistogramSpecification_ARGB8888().

Reduced:

----------------------------------
#include <Accelerate/Accelerate.h>

int main (void)
{
vImagePixelCount histAlphaBuffer[256];
vImagePixelCount histRedBuffer[256];
vImagePixelCount histGreenBuffer[256];
vImagePixelCount histBlueBuffer[256];

vImagePixelCount* histBuffer[4] = {
histAlphaBuffer, histRedBuffer,
histGreenBuffer, histBlueBuffer};

vImageHistogramCalculation_ARGB8888(
NULL, histBuffer, kvImageNoFlags);

vImageHistogramSpecification_ARGB8888(
NULL, NULL, histBuffer, kvImageNoFlags);

return 0;
}
----------------------------------

test.c:18:16: warning: passing 'vImagePixelCount *[4]' to parameter of type 'const vImagePixelCount **' (aka 'const unsigned long **') discards qualifiers in nested
pointer types [-Wincompatible-pointer-types-discards-qualifiers]
NULL, NULL, histBuffer, kvImageNoFlags);
^~~~~~~~~~

Is it just me or does the declaration of vImageHistogramSpecification_ARGB8888() itself not have its consts right?

Thanks,

--
____________________________________________________________
Sean McBride, B. Eng se...@rogue-research.com
Rogue Research www.rogue-research.com
Mac Software Developer Montréal, Québec, Canada

_______________________________________________
Do not post admin requests to the list. They will be ignored.
PerfOptimization-dev mailing list (PerfOptimi...@lists.apple.com)
Help/Unsubscribe/Update your Subscription:
https://lists.apple.com/mailman/options/perfoptimization-dev/perfoptimization-dev-garchive-8409%40googlegroups.com

This email sent to perfoptimization-...@googlegroups.com

[Eric Postpischil]

On Jul 2, 2015, at 09:43, Sean McBride <se...@rogue-research.com> wrote:

With Xcode 7 I have a new warning using vImageHistogramSpecification_ARGB8888().

…

test.c:18:16: warning: passing 'vImagePixelCount *[4]' to parameter of type 'const vImagePixelCount **' (aka 'const unsigned long **') discards qualifiers in nested
     pointer types [-Wincompatible-pointer-types-discards-qualifiers]
  NULL, NULL, histBuffer, kvImageNoFlags);
              ^~~~~~~~~~

I am unsure why you see this newly in Xcode 7. I see the same error (with a less detailed message) in Snow Leopard, using gcc 4.2.1. Are you sure this code compiled without warning previously?

Since we know vImageHistogramSpecification_ARGB8888 is well behaved, it is safe to ignore this warning, and you may avoid it by casting the argument to “const vImagePixelCount **” in the function call. It is a shortcoming of C that there is not a better way to do this.

The reason this is an error is that a called routine could change one of the pointers, say the pointer in desired_histogram[0], to point to some const data. This is legal within the called routine because desired_histogram is “const vImagePixelCount **”, so desired_histogram[0] is “const vImagePixelCount *”, so it is okay to set it to point at const data. However, when it returns, that pointer is in the caller’s array (called histBuffer in your example). The type of histBuffer is “vImagePixelCount **”, so histBuffer[0] is “vImagePixelCount *”. That means it is legal for the caller to modify the data that histBuffer[0] points to. But it is pointing to const data, so modifying it would have behavior not defined by the C standard.

For example:

void foo(const int **p)

{

static const int Two = 2;

p[0] = &Two; // Legal, assigns const int * to const int *.

}

void bar(void)

{

int *x

foo(&x);

x[0] = 3; // Overtly legal, assigns int to int. But computer attempts to put 3 into constant Two!

}

Is it just me or does the declaration of vImageHistogramSpecification_ARGB8888() itself not have its consts right?

desired_histogram contains input data, so it is reasonable that that data is declared const. I expect the pointers in the array are also unmodified, so they could be declared const as well (making the parameter const vImagePixelCount * const desired_histogram[4]), but that would not help; you would get essentially the same error.

—edp

[Sean McBride]

On Thu, 2 Jul 2015 12:58:41 -0400, Eric Postpischil said:

>On Jul 2, 2015, at 09:43, Sean McBride <se...@rogue-research.com> wrote:
>
>> With Xcode 7 I have a new warning using
>vImageHistogramSpecification_ARGB8888().
>> …
>> test.c:18:16: warning: passing 'vImagePixelCount *[4]' to parameter of
>type 'const vImagePixelCount **' (aka 'const unsigned long **') discards
>qualifiers in nested
>> pointer types [-Wincompatible-pointer-types-discards-qualifiers]
>> NULL, NULL, histBuffer, kvImageNoFlags);
>> ^~~~~~~~~~
>
>I am unsure why you see this newly in Xcode 7. I see the same error
>(with a less detailed message) in Snow Leopard, using gcc 4.2.1. Are you
>sure this code compiled without warning previously?

First, thanks for your reply.

Quite sure. I have 10.6 to 10.10 buildbots that have no such warnings. But...

>Since we know vImageHistogramSpecification_ARGB8888 is well behaved, it
>is safe to ignore this warning

Yeah, I figured, but thanks for confirming.

>, and you may avoid it by casting the
>argument to “const vImagePixelCount **” in the function call. It is a
>shortcoming of C that there is not a better way to do this.

So actually I already have that cast there, which is no doubt why my bots don't show any warnings. New in Xcode 7 is the fact that even *with* that cast I get a warning:

/Users/sean/Desktop/test.c:18:43: warning: cast from 'vImagePixelCount **' (aka 'unsigned long **') to 'const vImagePixelCount **' (aka 'const unsigned long **') must have all intermediate
pointers const qualified to be safe [-Wcast-qual]
NULL, NULL, (const vImagePixelCount **)histBuffer, kvImageNoFlags);
^

>> Is it just me or does the declaration of
>vImageHistogramSpecification_ARGB8888() itself not have its consts right?
>
>desired_histogram contains input data, so it is reasonable that that
>data is declared const. I expect the pointers in the array are also
>unmodified, so they could be declared const as well (making the
>parameter const vImagePixelCount * const desired_histogram[4]), but that
>would not help; you would get essentially the same error.

Actually, it seems it does help, because it allows me to use a cast that gets no warnings, which I can't achieve with its current prototype:

--------------------------------------
#include <Accelerate/Accelerate.h>

static vImage_Error
MY_vImageHistogramSpecification_ARGB8888(
const vImage_Buffer *src,
const vImage_Buffer *dest,
const vImagePixelCount * const desired_histogram[4],
vImage_Flags flags )
{
(void)src; (void)dest;
(void)desired_histogram; (void)flags;
return 0;
}

int main (void)
{
vImagePixelCount histAlphaBuffer[256];
vImagePixelCount histRedBuffer[256];
vImagePixelCount histGreenBuffer[256];
vImagePixelCount histBlueBuffer[256];

vImagePixelCount* histBuffer[4] = {
histAlphaBuffer, histRedBuffer,
histGreenBuffer, histBlueBuffer};

vImageHistogramCalculation_ARGB8888(
NULL, histBuffer, kvImageNoFlags);

MY_vImageHistogramSpecification_ARGB8888(
NULL, NULL, (const vImagePixelCount *const *)histBuffer, kvImageNoFlags);

return 0;
}
--------------------------------------

Try with:

xcrun clang -fsyntax-only -Weverything ~/Desktop/test.c

Cheers,

[Eric Postpischil]

On Jul 2, 2015, at 13:29, Sean McBride <se...@rogue-research.com> wrote:

desired_histogram contains input data, so it is reasonable that that
data is declared const. I expect the pointers in the array are also
unmodified, so they could be declared const as well (making the
parameter const vImagePixelCount * const desired_histogram[4]), but that
would not help; you would get essentially the same error.

Actually, it seems it does help, because it allows me to use a cast that gets no warnings, which I can't achieve with its current prototype:

…

xcrun clang -fsyntax-only -Weverything ~/Desktop/test.c

Unfortunately, changing the declaration raises issues of compatibility with existing source code.

The -Weverything switch is quite broad. I find -Wmost serves well. But you can also cast first to “void *” and then to “const vImagePixelCount **”, and then the warning does not appear.

—edp

[Sean McBride]

On Thu, 2 Jul 2015 17:31:48 -0400, Eric Postpischil said:

>On Jul 2, 2015, at 13:29, Sean McBride <se...@rogue-research.com> wrote:
>
>>> desired_histogram contains input data, so it is reasonable that that
>>> data is declared const. I expect the pointers in the array are also
>>> unmodified, so they could be declared const as well (making the
>>> parameter const vImagePixelCount * const desired_histogram[4]), but that
>>> would not help; you would get essentially the same error.
>>
>> Actually, it seems it does help, because it allows me to use a cast
>that gets no warnings, which I can't achieve with its current prototype:
>> …
>> xcrun clang -fsyntax-only -Weverything ~/Desktop/test.c
>
>Unfortunately, changing the declaration raises issues of compatibility
>with existing source code.

True, but there's precedent: the 10.9 SDK added a lot of consts to various Accelerate APIs (ex: rdar://13054722).

I've filed <rdar://21670389> in hope. :) It seems to me that as your compiler gets smarter/pickier, the SDKs/APIs have to keep up...

>The -Weverything switch is quite broad. I find -Wmost serves well.

I was only showing -Weverything because it's great for little snippets, I don't actually use it in my real project, it's impossibly noisy.

>you can also cast first to “void *” and then to “const vImagePixelCount
>**”, and then the warning does not appear.

Sneaky! Thanks!!

[Eric Postpischil]

On Jul 3, 2015, at 11:41, Sean McBride wrote:

True, but there's precedent: the 10.9 SDK added a lot of consts to various Accelerate APIs (ex: rdar://13054722).

Adding const to the innermost pointer of a parameter type¹, as was done in 10.9, does not break compatibility. This is because a function call converts its arguments to the types of the pointers as if by assignment (C 2011 6.5.2.2 7), and assignment may assign pointers to non-const to pointers to const (6.5.16.1 1, third list item). However, if you insert const elsewhere in a type, the new type is incompatible with the old type (6.7.3 10). Unlike the earlier change, this would cause some existing code that compiles to fail.

I've filed <rdar://21670389> in hope. :)  It seems to me that as your compiler gets smarter/pickier, the SDKs/APIs have to keep up...

Compiling the code without warning is supported by using “-Wno-incomptible-pointer-types-discards-qualifiers”, even after “-Weverything”. We can support compiling programs that make sense and work even though they do not conform to the C standard, which is what this switch allows you to do.

However, if you compile with “-Weverything” (and do not turn off any warnings after that), you are asking for all warnings, including warnings about things that do not conform to the C standard. Since this code does not conform to the standard, a warning must be issued.

This is a shortcoming of C. There are some conversions which logically could be safe but which the language does not support. Since they are safe, a compiler can support them. But there is nothing the compiler can do about the fact that they are not strictly conforming C code. If you ask for all warnings, the compiler must give you a warning. Changing the SDK or API cannot alter this.

you can also cast first to “void *” and then to “const vImagePixelCount
**”, and then the warning does not appear.

Actually, you can just convert to “void *”, and the compiler will do the remaining conversion to parameter type automatically. Of course, either of these discards the error-checking that the type system provides.

— edp (Eric Postpischil)
http://edp.org

¹ Specifically: If a parameter type is a pointer to X, changing it to a pointer to a const X does not break compatibility, regardless of whether X is a simple char or is a compound type such as a pointer to a pointer to a struct. However, if a parameter type is a pointer to a pointer to Y, changing it to a pointer to a pointer to const Y does break compatibility.