Annotate SK_COMPILE_ASSERT's typedef as permissibly unused, to fix GCC 4.8 build warning. Patch ori…

Annotate SK_COMPILE_ASSERT's typedef as permissibly unused, to fix GCC 4.8 build warning. Patch originally by dholbert.

R=bsalomon@google.com

Committed: https://code.google.com/p/skia/source/detail?r=10575

[bungeman-skia, 2013-08-05 21:56:28]

https://chromiumcodereview.appspot.com/22285002/diff/1/include/core/SkTypes.h
File include/core/SkTypes.h (right):

https://chromiumcodereview.appspot.com/22285002/diff/1/include/core/SkTypes.h...
include/core/SkTypes.h:137: #    define SK_COMPILE_ASSERT_UNUSED_ATTRIBUTE /*
nothing */
The comment here seems a bit odd. The comment will actually be removed by the
Tokenization phase of translation, before the Preprocessing phase. As such,
there will be no comment present (at the use site) from the compiler's point of
view. This is probably a good thing as it won't magically interfere with
commented out blocks of code with static asserts in them. Skia doesn't do this
anywhere else.

Also, this sort of thing is mostly in SkPostConfig.h (like SK_OVERRIDE) is it
not?

[bungeman-skia, 2013-08-05 22:20:57]

So I'm going to go all unicorns and rainbows and state where we'd like this to
end up. We should have a SK_ATTRIBUTE(attr) macro which can expand to nothing,
[[attr]], or __attribute__((attr)) and then this could be SK_ATTRIBUTE_UNUSED
which would just be defined as SK_ATTRIBUTE(unused). Then (later, totally
outside the scope of this CL) all the other uses of __attribute__((...)) would
be replaced with SK_ATTRIBUTE. And SK_ATTRIBUTE and SK_ATTRIBUTE_UNUSED should
go in SkPostConfig.h with the others.

[wrightg, 2013-08-06 14:54:22]

https://chromiumcodereview.appspot.com/22285002/diff/1/include/core/SkTypes.h
File include/core/SkTypes.h (right):

https://chromiumcodereview.appspot.com/22285002/diff/1/include/core/SkTypes.h...
include/core/SkTypes.h:137: #    define SK_COMPILE_ASSERT_UNUSED_ATTRIBUTE /*
nothing */
On 2013/08/05 21:56:28, bungeman1 wrote:
> The comment here seems a bit odd. The comment will actually be removed by the
> Tokenization phase of translation, before the Preprocessing phase. As such,
> there will be no comment present (at the use site) from the compiler's point
of
> view.

I don't think that the comment was intended to be present anywhere other than
right here to clarify that we're deliberately defining it to nothing.

> Also, this sort of thing is mostly in SkPostConfig.h (like SK_OVERRIDE) is it
> not?

We need it for the SK_COMPILE_ASSERT definition which occurs in this file.

[bsalomon, 2013-08-06 14:56:59]

I feel better with checking it in as is if the macro were just renamed SK_UNUSED
or something more generic since this may have other use cases than compile
asserts. Also, should be moved to post config and put in an #ifndef
SK_COMPILE_ASSERT_UNUSED_ATTRIBUTE block.

[bsalomon, 2013-08-06 14:57:39]

> > Also, this sort of thing is mostly in SkPostConfig.h (like SK_OVERRIDE) is
it
> > not?
> 
> We need it for the SK_COMPILE_ASSERT definition which occurs in this file.

Oh right.

[bungeman-skia, 2013-08-06 17:30:25]

I sneakily inserted Patch Set 2 for a more general alternate reflecting my
previous suggestions.

[bsalomon, 2013-08-06 17:35:53]

On 2013/08/06 17:30:25, bungeman1 wrote:
> I sneakily inserted Patch Set 2 for a more general alternate reflecting my
> previous suggestions.

lgtm

[bungeman-chromium, 2013-08-06 18:32:40]

Committed patchset #2 manually as r10575 (presubmit successful).