- Make sure values are "used" as part of ASSERT even when compiling optimized.

- Make sure values are "used" as part of ASSERT even when compiling optimized.

R=floitsch@google.com

Committed: https://code.google.com/p/dart/source/detail?r=28528

[Ivan Posva, 2013-10-11 05:09:36]

clang 3.4 is complaining unless we make the inputs to ASSERT used:

  runtime/third_party/double-conversion/src/cached-powers.cc:134:18: error:
unused variable 'kCachedPowersLength'
        [-Werror,-Wunused-const-variable]

Florian, do you mind integrating this or a similar fix into double-conversion?

Thanks,
-Ivan

[floitsch, 2013-10-11 08:00:47]

LGTM, although I would be afraid that GCC will eventually warn about
dead-code...
I googled a little bit and found:
===
#ifdef NDEBUG
#define ASSERT(expression) assert(expression)
#else
#define ASSERT(expression) ((void) expression)
#endif
===

I slightly prefer this version, but don't change the CL just for me.

I will look into doing something similar in the double-conversion library.
Note, that we also have some other warning-problem with newer GCCs:
https://code.google.com/p/double-conversion/issues/detail?id=37
I believe this is not just the double-conversion library but also triggers in
the Dart VM.

[Ivan Posva, 2013-10-11 16:59:58]

On 2013/10/11 08:00:47, floitsch wrote:
> LGTM, although I would be afraid that GCC will eventually warn about
> dead-code...
> I googled a little bit and found:
> ===
> #ifdef NDEBUG
> #define ASSERT(expression) assert(expression)
> #else
> #define ASSERT(expression) ((void) expression)
> #endif
> ===
> 
> I slightly prefer this version, but don't change the CL just for me.
> 
> I will look into doing something similar in the double-conversion library.
> Note, that we also have some other warning-problem with newer GCCs:
> https://code.google.com/p/double-conversion/issues/detail?id=37
> I believe this is not just the double-conversion library but also triggers in
> the Dart VM.

If I understand your suggestion, it will execute the expression.

-Ivan

[Ivan Posva, 2013-10-11 17:02:47]

Committed patchset #1 manually as r28528.

[floitsch, 2013-10-11 17:11:08]

On 2013/10/11 16:59:58, Ivan Posva wrote:
> On 2013/10/11 08:00:47, floitsch wrote:
> > LGTM, although I would be afraid that GCC will eventually warn about
> > dead-code...
> > I googled a little bit and found:
> > ===
> > #ifdef NDEBUG
> > #define ASSERT(expression) assert(expression)
> > #else
> > #define ASSERT(expression) ((void) expression)
> > #endif
> > ===
> > 
> > I slightly prefer this version, but don't change the CL just for me.
> > 
> > I will look into doing something similar in the double-conversion library.
> > Note, that we also have some other warning-problem with newer GCCs:
> > https://code.google.com/p/double-conversion/issues/detail?id=37
> > I believe this is not just the double-conversion library but also triggers
in
> > the Dart VM.
> 
> If I understand your suggestion, it will execute the expression.
Yes. In double-conversion that would have been ok. But I must admit that this is
counter-intuitive for ASSERTs.
> 
> -Ivan