Make GPU_DCHECK* keep reference to passed variables

Make GPU_DCHECK* keep reference to passed variables 
to avoid unused variable warnings in release build.


BUG=87490
TEST=None


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=105432

[ph34r, 2011-10-13 14:54:59]

This CL is all about these warnings which appear in release build with gcc 4.6: 
     1 gpu/command_buffer/client/gles2_implementation.cc:2388:46: warning:
variable ‘result’ set but not used [-Wunused-but-set-variable]
      1 gpu/command_buffer/client/gles2_implementation.cc:2457:47: warning:
variable ‘result’ set but not used [-Wunused-but-set-variable]
      1 gpu/command_buffer/client/program_info_manager.cc:376:45: warning:
variable ‘result’ set but not used [-Wunused-but-set-variable]

As an alternative I can suggest mark these 3 variables with ALLOW_UNUSED macro.
So what is more preferable?

[apatrick_chromium, 2011-10-13 18:14:13]

Thanks for looking into this. A few comments.

http://codereview.chromium.org/8271002/diff/1/gpu/command_buffer/common/loggi...
File gpu/command_buffer/common/logging.h (right):

http://codereview.chromium.org/8271002/diff/1/gpu/command_buffer/common/loggi...
gpu/command_buffer/common/logging.h:179: #define GPU_DEBUG_IS_ON() false
This could just be:

#define GPU_DEBUG_IS_ON false

because the macro takes no parameters.

http://codereview.chromium.org/8271002/diff/1/gpu/command_buffer/common/loggi...
gpu/command_buffer/common/logging.h:186: if(GPU_DEBUG_IS_ON()) \
Chromium code style would be:

if (GPU_DEBUG_IS_ON()) \
  ::gpu::Logger::CheckTrue( \
      (X), __FILE__, __LINE__, #X, "GPU_DCHECK")

Note the extra two spaces of indentation in front of "(X)" and the space after
the "if".

But it won't work in all cases. Example:

if (foo)
  GPU_DCHECK(bar > 0);
else
  GPU_DCHECK(bar < 0);

With this change, the "else" would be paired with the "if(GPU_DEBUG_IS_ON())" in
the expanded GPU_DCHECK macro instead of the "if (foo)".

Lastly, have you verified that the compiler does not generate any extra code for
these macros when GPU_DEBUG_IS_ON is false and the optimizer is enabled?

[ph34r, 2011-10-13 20:09:18]

I was inspired by the base/logging.h implementation. It's the same way as
DCHECK_* works, so it should be fine. Compiler output also looks good.

http://google.com/codesearch#OAMlx_jo-ck/src/base/logging.h&exact_package=chr...

[apatrick_chromium, 2011-10-13 20:30:50]

Fair enough. I started a try job.

http://build.chromium.org/p/tryserver.chromium/builders/linux/builds/54466
http://build.chromium.org/p/tryserver.chromium/builders/mac/builds/54201
http://build.chromium.org/p/tryserver.chromium/builders/win/builds/60882

[greggman, 2011-10-13 20:53:25]

if AL's concerns are satisfied then lgtm

Just be aware, anything in src/gpu/common and src/gpu/client is compiled in
Pepper / NaCl as well so you might want to try jobs there too

[apatrick_chromium, 2011-10-13 23:23:43]

Try jobs are good. The test failures are unrelated to this change. The NaCl
integration step was fine. LGTM. I'll add this to the commit queue. Thanks.

[commit-bot: I haz the power, 2011-10-13 23:23:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pph34r@gmail.com/8271002/4001

[commit-bot: I haz the power, 2011-10-14 01:10:06]

Change committed as 105432