New build flag system, convert Google Now flag

build/build_header.h

+// Copyright 2015 The Chromium Authors. All rights reserved.

[Mark Mentovai, 2015/11/19 21:52:55]

If this defines BUILDFLAG() as the primary macro, the file should be named
build_flag.h or similar.

[Mark Mentovai, 2015/11/19 21:53:26]

In the CL description, capitalize Google Now, because otherwise, it sounds like
you’re converting Google, and you’re doing it now.

[brettw, 2015/11/20 00:02:37]

It defines BUILDFLAG and BUILDVAR, and I was generalizing this class of thing as
build headers. This will normally be included only by the generated "build
headers" (I added a comment to this effect).

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BASE_BUILD_HEADER_H_

[Mark Mentovai, 2015/11/19 21:52:55]

BUILD_BUILD_HEADER_H_ here and in two other places.

+#define BASE_BUILD_HEADER_H_
+
+// These macros un-mangle the names of the build flags in a way that looks
+// natural, and gives errors if the flag is not defined. Normally in the
+// preprocessor it's easy to make mistakes that interpret "you haven't done
+// the setup to know what the flag is" as "flag is off".
+//
+// This is for use with headers generated by write_build_header.py.
+
+#define BUILDFLAG_CAT(a, b) a ## b
+#define BUILDFLAG_CAT2(a, b) BUILDFLAG_CAT(a, b)

[Mark Mentovai, 2015/11/19 21:52:55]

I recognize this pattern (although I doubt most developers would), but I don’t
actually think it’s necessary here.

[brettw, 2015/11/20 00:02:37]

This stuff is all removed now. Yay!

+
+// These allow the code below to have an expression evaluate to 1 if the value
+// is 0 or 1, and 0 if it's undefined or a different value.
+#define BUILDFLAG_DEFINE_CHECK_0 1
+#define BUILDFLAG_DEFINE_CHECK_1 1
+
+// Accessor for boolean values. Throws an error if undefined. Since the

[Mark Mentovai, 2015/11/19 21:52:55]

“if flag is undefined”

+// invocation of a #define can't normally throw an error, emulate that by

[Mark Mentovai, 2015/11/19 21:52:55]

of a macro

+// forcing a divide-by-0 in the preprocessor.
+#define BUILDFLAG(flag) (BUILDFLAG_CAT2(BUILDFLAG_FLAG_VALUE_, flag) / BUILDFLAG_CAT2(BUILDFLAG_DEFINE_CHECK_, BUILDFLAG_CAT2(BUILDFLAG_FLAG_VALUE_, flag)))  // If you see a division by 0 it means your flag is undefined and you're missing the header.

[Mark Mentovai, 2015/11/19 21:52:55]

I believe that this can be simplified by using defined() for the denominator:

#define BUILDFLAG(flag) ((BUILDFLAG_FLAG_VALUE_ ## flag) /
defined(BUILDFLAG_FLAG_VALUE_ ## flag))

You shouldn’t need BUILDFLAG_DEFINE_CHECK_0/1. This will make BUILDFLAG() work
equally well for BUILDFLAG_FLAG_VALUE_* values of 0 and 1 (as you have here) and
2 and -1 and everything else.

This won’t work if you intended to use BUILDFLAG() in C++ expressions and not
just preprocessor expressions, because defined() is meaningless to C++. If the
comments here had some example usage, your intentions for how these are to be
used would be clearer.

[Mark Mentovai, 2015/11/19 21:52:55]

Can we name this CR_BUILDFLAG to provide some semblance of namespacing?

[brettw, 2015/11/20 00:02:37]

I thought about CR_* and decided against it. I can add it if you feel strongly.
My reasoning was that one of the goals of this is to make the build directory
reusable by v8 and nacl, etc. and currently the main impediment is the global
defines we add. So this is more "build" than "chrome". Also, I think CR_ looks
ugly and BUILDFLAG is hopefully unique enough (I thought of some simpler ones
like ENABLE or USE like WebKit uses, but they obviously fail this test).

[brettw, 2015/11/20 00:02:37]

This works. I like your way.

[brettw, 2015/11/20 04:10:39]

Scratch that, Visual Studio doesn't like this. I spent like 30 minutes on this
and I couldn't simplify this express *at all* in a way that makes Visual Studio
happy.

For example, this passes:
  #define FOO
  #if (1 / defined(FOO))
but this throws a divide by 0:
  #define FOO
  #define DOTEST(a) (1 / defined(FOO))
  #if DOTEST(a)

[spang, 2015/11/20 19:00:24]

Was

#if BUILDFLAG_ENABLE_FOO()

considered? It's a preprocessor syntax error if BUILDFLAG_ENABLE_FOO() was not
defined.

+
+// For flags where you need to know the value.
+#define BUILDVAR(var) (BUILDFLAG_VAR_VALUE_##var)

[Mark Mentovai, 2015/11/19 21:52:55]

Having both BUILDFLAG_FLAG_VALUE_* and BUILDFLAG_VAR_VALUE_* is a little
confusing. I think we can unify behind one of them.

If we do have cases where you need to get at a value for use in C++ expressions,
we should be able to get at flags. For cases where you need to get a value in a
preprocessor expression, use CR_BUILDFLAG(). For cases where you need to get a
value for use in C++, use CR_BUILDFLAG_VALUE(). The comments here should explain
this and give some example usage.

+
+#endif  // BASE_BUILD_HEADER_H_

[Mark Mentovai, 2015/11/19 21:52:55]

After I read this file, I saw that you had more documentation-like stuff in the
gni file. OK, but this file should point to that then, or the more C++-oriented
stuff should live in this file. People having a hard time with BUILDFLAG() won’t
think to look at that gn file first, they’ll probably look for the file that
defines BUILDFLAG().