clang/win: Let SK_TRACEHR not produce -Wunused-value warnings in release builds.

clang/win: Let SK_TRACEHR not produce -Wunused-value warnings in release builds.

Fixes warnings like:
..\..\third_party\skia\include\utils\win\SkHRESULT.h(51,23) :  note: expanded from macro 'HRNM'
                      ^~~~~~~~~~~~~~~~~~~~~~~~~
..\..\third_party\skia\include\utils\win\SkHRESULT.h(26,20) :  note: expanded from macro 'HR_GENERAL'
        SK_TRACEHR(_hr, _msg);\
                   ^~~
..\..\third_party\skia\include\utils\win\SkHRESULT.h(20,31) :  note: expanded from macro 'SK_TRACEHR'
                              ^~~

BUG=chromium:505318
TBR=reed
This is a trivial implementation change.

Committed: https://skia.googlesource.com/skia/+/573ce20f4298e0a83c36a106cde78f5442801f50

[Nico, 2015-12-05 02:55:41]

The CQ bit was checked by thakis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-12-05 02:55:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1503463004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1503463004/1

[commit-bot: I haz the power, 2015-12-05 02:55:53]

Note for Reviewers:
The CQ is waiting for an approval.
If you believe that the CL is not ready yet, or if you would like to L-G-T-M
with comments then please uncheck the CQ checkbox.

Waiting for LGTM from valid reviewer(s) till 2015-12-05 08:55 UTC

[Nico, 2015-12-05 02:56:00]

thakis@chromium.org changed reviewers:
+ bungeman@chromium.org

[commit-bot: I haz the power, 2015-12-05 08:55:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-05 08:55:06]

Dry run: No LGTM from a valid reviewer yet. Please ask for an LGTM from a
full Skia committer

[bungeman-skia, 2015-12-06 21:03:46]

bungeman@google.com changed reviewers:
+ bungeman@google.com
- bungeman@chromium.org

[bungeman-skia, 2015-12-06 21:03:47]

https://codereview.chromium.org/1503463004/diff/1/include/utils/win/SkHRESULT.h
File include/utils/win/SkHRESULT.h (left):

https://codereview.chromium.org/1503463004/diff/1/include/utils/win/SkHRESULT...
include/utils/win/SkHRESULT.h:19: #define SK_TRACEHR(_hr, _msg) _hr
I will say that this existing code is odd, in that SkTraceHR is void, but this
isn't (I suppose I wrote it, but that was a while ago). I think maybe the reason
why is to prevent other (real) unused warnings, in instances where SK_TRACEHR is
used independently of HR_GENERAL and the hresult will be reported (used) in
debug, but isn't actually used in release at all. This might happen when the
actual failure mode doesn't make a difference but one of the out parameters
already lets us know better if there was success. I think SkFontMgr_win_dw.cpp
does something like this.

Perhaps this should be 'sk_ignore_unused_variable(_hr)' instead.

[Nico, 2015-12-07 17:42:28]

https://codereview.chromium.org/1503463004/diff/1/include/utils/win/SkHRESULT.h
File include/utils/win/SkHRESULT.h (left):

https://codereview.chromium.org/1503463004/diff/1/include/utils/win/SkHRESULT...
include/utils/win/SkHRESULT.h:19: #define SK_TRACEHR(_hr, _msg) _hr
On 2015/12/06 21:03:47, bungeman1 wrote:
> I will say that this existing code is odd, in that SkTraceHR is void, but this
> isn't (I suppose I wrote it, but that was a while ago). I think maybe the
reason
> why is to prevent other (real) unused warnings, in instances where SK_TRACEHR
is
> used independently of HR_GENERAL and the hresult will be reported (used) in
> debug, but isn't actually used in release at all. This might happen when the
> actual failure mode doesn't make a difference but one of the out parameters
> already lets us know better if there was success. I think SkFontMgr_win_dw.cpp
> does something like this.
> 
> Perhaps this should be 'sk_ignore_unused_variable(_hr)' instead.

Done. (It looks like skia isn't built with unused variable warnings enabled when
building it as part of Chromium; it built fine with PS1 for me. It still built
fine when I added "int a = 4;" to some random function in skia.)

[Nico, 2015-12-07 17:42:54]

Description was changed from

==========
clang/win: Let SK_TRACEHR not produce -Wunused-value warnings in release builds.

Fixes warnings like:
..\..\third_party\skia\include\utils\win\SkHRESULT.h(51,23) :  note: expanded
from macro 'HRNM'
                      ^~~~~~~~~~~~~~~~~~~~~~~~~
..\..\third_party\skia\include\utils\win\SkHRESULT.h(26,20) :  note: expanded
from macro 'HR_GENERAL'
        SK_TRACEHR(_hr, _msg);\
                   ^~~
..\..\third_party\skia\include\utils\win\SkHRESULT.h(20,31) :  note: expanded
from macro 'SK_TRACEHR'
                              ^~~

Since SK_TRACEHR() is used only in places where its result isn't used, just let
SK_TRACEHR() expand to nothing in release builds.

BUG=chromium:505318
==========

to

==========
clang/win: Let SK_TRACEHR not produce -Wunused-value warnings in release builds.

Fixes warnings like:
..\..\third_party\skia\include\utils\win\SkHRESULT.h(51,23) :  note: expanded
from macro 'HRNM'
                      ^~~~~~~~~~~~~~~~~~~~~~~~~
..\..\third_party\skia\include\utils\win\SkHRESULT.h(26,20) :  note: expanded
from macro 'HR_GENERAL'
        SK_TRACEHR(_hr, _msg);\
                   ^~~
..\..\third_party\skia\include\utils\win\SkHRESULT.h(20,31) :  note: expanded
from macro 'SK_TRACEHR'
                              ^~~

BUG=chromium:505318
==========

[bungeman-skia, 2015-12-07 18:14:45]

The CQ bit was checked by bungeman@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-12-07 18:14:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1503463004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1503463004/20001

[commit-bot: I haz the power, 2015-12-07 18:33:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-07 18:33:31]

Dry run: This issue passed the CQ dry run.

[bungeman-skia, 2015-12-07 18:34:01]

The CQ bit was checked by bungeman@google.com

[bungeman-skia, 2015-12-07 18:34:01]

lgtm

[commit-bot: I haz the power, 2015-12-07 18:34:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1503463004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1503463004/20001

[commit-bot: I haz the power, 2015-12-07 18:35:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-12-07 18:35:07]

Try jobs failed on following builders:
  skia_presubmit-Trybot on client.skia.fyi (JOB_FAILED,
http://build.chromium.org/p/client.skia.fyi/builders/skia_presubmit-Trybot/bu...)

[bungeman-skia, 2015-12-07 18:39:06]

Description was changed from

==========
clang/win: Let SK_TRACEHR not produce -Wunused-value warnings in release builds.

Fixes warnings like:
..\..\third_party\skia\include\utils\win\SkHRESULT.h(51,23) :  note: expanded
from macro 'HRNM'
                      ^~~~~~~~~~~~~~~~~~~~~~~~~
..\..\third_party\skia\include\utils\win\SkHRESULT.h(26,20) :  note: expanded
from macro 'HR_GENERAL'
        SK_TRACEHR(_hr, _msg);\
                   ^~~
..\..\third_party\skia\include\utils\win\SkHRESULT.h(20,31) :  note: expanded
from macro 'SK_TRACEHR'
                              ^~~

BUG=chromium:505318
==========

to

==========
clang/win: Let SK_TRACEHR not produce -Wunused-value warnings in release builds.

Fixes warnings like:
..\..\third_party\skia\include\utils\win\SkHRESULT.h(51,23) :  note: expanded
from macro 'HRNM'
                      ^~~~~~~~~~~~~~~~~~~~~~~~~
..\..\third_party\skia\include\utils\win\SkHRESULT.h(26,20) :  note: expanded
from macro 'HR_GENERAL'
        SK_TRACEHR(_hr, _msg);\
                   ^~~
..\..\third_party\skia\include\utils\win\SkHRESULT.h(20,31) :  note: expanded
from macro 'SK_TRACEHR'
                              ^~~

BUG=chromium:505318
TBR=reed
This is a trivial implementation change.
==========

[bungeman-skia, 2015-12-07 18:39:14]

The CQ bit was checked by bungeman@google.com

[commit-bot: I haz the power, 2015-12-07 18:39:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1503463004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1503463004/20001

[commit-bot: I haz the power, 2015-12-07 18:41:38]

Description was changed from

==========
clang/win: Let SK_TRACEHR not produce -Wunused-value warnings in release builds.

Fixes warnings like:
..\..\third_party\skia\include\utils\win\SkHRESULT.h(51,23) :  note: expanded
from macro 'HRNM'
                      ^~~~~~~~~~~~~~~~~~~~~~~~~
..\..\third_party\skia\include\utils\win\SkHRESULT.h(26,20) :  note: expanded
from macro 'HR_GENERAL'
        SK_TRACEHR(_hr, _msg);\
                   ^~~
..\..\third_party\skia\include\utils\win\SkHRESULT.h(20,31) :  note: expanded
from macro 'SK_TRACEHR'
                              ^~~

BUG=chromium:505318
TBR=reed
This is a trivial implementation change.
==========

to

==========
clang/win: Let SK_TRACEHR not produce -Wunused-value warnings in release builds.

Fixes warnings like:
..\..\third_party\skia\include\utils\win\SkHRESULT.h(51,23) :  note: expanded
from macro 'HRNM'
                      ^~~~~~~~~~~~~~~~~~~~~~~~~
..\..\third_party\skia\include\utils\win\SkHRESULT.h(26,20) :  note: expanded
from macro 'HR_GENERAL'
        SK_TRACEHR(_hr, _msg);\
                   ^~~
..\..\third_party\skia\include\utils\win\SkHRESULT.h(20,31) :  note: expanded
from macro 'SK_TRACEHR'
                              ^~~

BUG=chromium:505318
TBR=reed
This is a trivial implementation change.

Committed:
https://skia.googlesource.com/skia/+/573ce20f4298e0a83c36a106cde78f5442801f50
==========

[commit-bot: I haz the power, 2015-12-07 18:41:39]

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/573ce20f4298e0a83c36a106cde78f5442801f50