common: add STATIC_IF and STATIC_IF_NOT macros

A common pattern to use with IS_ENABLED is like this:

/*
 * This var should only be used if CONFIG_FOO. The linker errors if
 * CONFIG_FOO is not defined is intentional.
 */
 #ifdef CONFIG_FOO
 static
 #else
 extern
 #endif
 int some_var;

The issue with this is that it leads to an over-verbose and
potentially hard to read pattern, and does not have the check that
CONFIG_FOO was only defined to blank.

Suppose a macro like this existed:

STATIC_IF(CONFIG_FOO) int some_var;

... which expands to "static" when CONFIG_FOO is defined to empty,
"extern" when CONFIG_FOO is not defined, and errors when CONFIG_FOO is
defined to non-empty.

This CL implements that, as well as the inverse (STATIC_IF_NOT).

BUG=chromium:989786
BRANCH=none
TEST=provided unit tests, buildall

Change-Id: Ib57aaba62bc184fda9aa782a780d5f13ba44ae88
Signed-off-by: Jack Rosenthal <jrosenth@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1731859
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>

7 files changed, 219 insertions, 20 deletions

diff --git a/include/common.h b/include/common.h
index 73e4a19abc..fab97f17b4 100644
--- a/include/common.h
+++ b/include/common.h
@@ -9,6 +9,7 @@
 #define __CROS_EC_COMMON_H
 
 #include <stdint.h>
+#include "compile_time_macros.h"
 
 /*
  * Macros to concatenate 2 - 4 tokens together to form a single token.
@@ -273,6 +274,41 @@ enum ec_error_list {
 #define __fls(n) (31 - __builtin_clz(n))
 
 /*
+ * __cfg_select(CONFIG_NAME, EMPTY, OTHERWISE) is a macro used for
+ * defining other macros which conditionally select code based on a
+ * config option. It will generate the argument passed as EMPTY
+ * when CONFIG_NAME was defined to the empty string, and OTHERWISE
+ * when the argument was not defined or defined to something
+ * non-empty.
+ *
+ * Generally speaking, macros which use this should make some sort of
+ * context-dependent assertion in OTHERWISE that CONFIG_NAME is
+ * undefined, rather than defined to something else. This usually
+ * involves tricks with __builtin_strcmp.
+ */
+#define __cfg_select(cfg, empty, otherwise)     \
+	__cfg_select_1(cfg, empty, otherwise)
+#define __cfg_select_placeholder_ _,
+#define __cfg_select_1(value, empty, otherwise)			  \
+	__cfg_select_2(__cfg_select_placeholder_##value, empty, otherwise)
+#define __cfg_select_2(arg1_or_junk, empty, otherwise)		\
+	__cfg_select_3(arg1_or_junk _, empty, otherwise)
+#define __cfg_select_3(_ignore1, _ignore2, select, ...) select
+
+/*
+ * This version concatenates a BUILD_ASSERT(...); before OTHERWISE,
+ * handling the __builtin_strcmp trickery where a BUILD_ASSERT is
+ * appropriate in the context.
+ */
+#define __cfg_select_build_assert(cfg, value, empty, undef)	\
+	__cfg_select(						\
+		value,						\
+		empty,						\
+		BUILD_ASSERT(					\
+			__builtin_strcmp(cfg, #value) == 0);	\
+		undef)
+
+/*
  * Attribute for generating an error if a function is used.
  *
  * Clang does not have a function attribute to do this. Rely on linker
@@ -286,26 +322,18 @@ enum ec_error_list {
 
 /*
  * Getting something that works in C and CPP for an arg that may or may
- * not be defined is tricky.  Here, if we have "#define CONFIG_FOO"
- * we match on the placeholder define, insert the "_, 1," for arg1 and generate
- * the triplet (_, 1, _, (...)).  Then the last step cherry picks the 2nd arg
- * (a one).
- * When CONFIG_FOO is not defined, we generate a (_, (...)) pair, and when
- * the last step cherry picks the 2nd arg, we get a code block that verifies
- * the value of the option. Since the preprocessor won't replace an unknown
- * token, we compare the option name with the value string. If they are
- * identical we assume that the value was undefined and return 0. If the value
- * happens to be anything else we call an undefined method that will raise
- * a compiler error. This technique requires that the optimizer be enabled so it
- * can remove the undefined function call.
+ * not be defined is tricky.
  *
+ * Compare the option name with the value string in the OTHERWISE to
+ * __cfg_select. If they are identical we assume that the value was
+ * undefined and return 0. If the value happens to be anything else we
+ * call an undefined method that will raise a compiler error. This
+ * technique requires that the optimizer be enabled so it can remove
+ * the undefined function call.
  */
-#define __ARG_PLACEHOLDER_ _, 1,
-#define _config_enabled(cfg, value) \
-	__config_enabled(__ARG_PLACEHOLDER_##value, cfg, value)
-#define __config_enabled(arg1_or_junk, cfg, value)			      \
-	___config_enabled(						      \
-		arg1_or_junk _, ({					      \
+#define __config_enabled(cfg, value)					      \
+	__cfg_select(							      \
+		value, 1, ({						      \
 			int __undefined = __builtin_strcmp(cfg, #value) == 0; \
 			extern int IS_ENABLED_BAD_ARGS(void) __error(	      \
 				cfg " must be <blank>, or not defined.");     \
@@ -313,7 +341,6 @@ enum ec_error_list {
 				IS_ENABLED_BAD_ARGS();			      \
 			0;						      \
 		}))
-#define ___config_enabled(__ignored, val, ...) val
 
 /**
  * Checks if a config option is enabled or disabled
@@ -329,6 +356,28 @@ enum ec_error_list {
  * Note: This macro will only function inside a code block due to the way
  * it checks for unknown values.
  */
-#define IS_ENABLED(option) _config_enabled(#option, option)
+#define IS_ENABLED(option) __config_enabled(#option, option)
+
+/**
+ * Makes a global variable static when a config option is enabled,
+ * extern otherwise (with the intention to cause linker errors if the
+ * variable is used outside of a config context, for example thru
+ * IS_ENABLED, that it should be).
+ *
+ * This follows the same constraints as IS_ENABLED, the config option
+ * should be defined to nothing or undefined.
+ */
+#define STATIC_IF(option)						\
+	__cfg_select_build_assert(#option, option, static, extern)
+
+/**
+ * STATIC_IF_NOT is just like STATIC_IF, but makes the variable static
+ * only if the config option is *not* defined, extern if it is.
+ *
+ * This is to assert that a variable will go unused with a certain
+ * config option.
+ */
+#define STATIC_IF_NOT(option)						\
+	__cfg_select_build_assert(#option, option, extern, static)
 
 #endif  /* __CROS_EC_COMMON_H */
diff --git a/test/build.mk b/test/build.mk
index 8d68da07f5..5868513962 100644
--- a/test/build.mk
+++ b/test/build.mk
@@ -61,6 +61,8 @@ test-list-host += sbs_charging_v2
 test-list-host += sha256
 test-list-host += sha256_unrolled
 test-list-host += shmalloc
+test-list-host += static_if
+test-list-host += static_if_error
 test-list-host += system
 test-list-host += thermal
 test-list-host += timer_dos
@@ -132,6 +134,7 @@ sbs_charging_v2-y=sbs_charging_v2.o
 sha256-y=sha256.o
 sha256_unrolled-y=sha256.o
 shmalloc-y=shmalloc.o
+static_if-y=static_if.o
 stress-y=stress.o
 system-y=system.o
 thermal-y=thermal.o
@@ -163,3 +166,6 @@ $(out)/RO/test/nvmem_tpm2_mock.o: CFLAGS += -I$(TPM2_ROOT)
 
 host-is_enabled_error: TEST_SCRIPT=is_enabled_error.sh
 is_enabled_error-y=is_enabled_error.o.cmd
+
+host-static_if_error: TEST_SCRIPT=static_if_error.sh
+static_if_error-y=static_if_error.o.cmd
diff --git a/test/static_if.c b/test/static_if.c
new file mode 100644
index 0000000000..46e8057d7f
--- /dev/null
+++ b/test/static_if.c
@@ -0,0 +1,50 @@
+/* Copyright 2019 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+/* Test the STATIC_IF and STATIC_IF_NOT macros. */
+
+#include "common.h"
+#include "test_util.h"
+
+#undef CONFIG_UNDEFINED
+#define CONFIG_BLANK
+
+STATIC_IF(CONFIG_UNDEFINED) int this_var_is_extern;
+STATIC_IF_NOT(CONFIG_BLANK) int this_var_is_extern_too;
+STATIC_IF(CONFIG_BLANK) int this_var_is_static;
+STATIC_IF_NOT(CONFIG_UNDEFINED) int this_var_is_static_too;
+
+static int test_static_if_blank(void)
+{
+	TEST_ASSERT(this_var_is_static == 0);
+	TEST_ASSERT(this_var_is_static_too == 0);
+
+	return EC_SUCCESS;
+}
+
+static int test_static_if_unused_no_fail(void)
+{
+	/*
+	 * This should not cause linker errors because the variables
+	 * go unused (usage is optimized away).
+	 */
+	if (IS_ENABLED(CONFIG_UNDEFINED))
+		this_var_is_extern = 1;
+
+	if (!IS_ENABLED(CONFIG_BLANK))
+		this_var_is_extern_too = 1;
+
+	return EC_SUCCESS;
+}
+
+void run_test(void)
+{
+	test_reset();
+
+	RUN_TEST(test_static_if_blank);
+	RUN_TEST(test_static_if_unused_no_fail);
+
+	test_print_result();
+}
diff --git a/test/static_if.tasklist b/test/static_if.tasklist
new file mode 100644
index 0000000000..5ffe662d01
--- /dev/null
+++ b/test/static_if.tasklist
@@ -0,0 +1,9 @@
+/* Copyright 2019 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+/**
+ * See CONFIG_TASK_LIST in config.h for details.
+ */
+#define CONFIG_TEST_TASK_LIST  /* No test task */
diff --git a/test/static_if_error.c b/test/static_if_error.c
new file mode 100644
index 0000000000..bbe4839981
--- /dev/null
+++ b/test/static_if_error.c
@@ -0,0 +1,34 @@
+/* Copyright 2019 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+/*
+ * Test the STATIC_IF and STATIC_IF_NOT macros fail on unexpected
+ * input.
+ */
+
+#include "common.h"
+#include "test_util.h"
+
+#define CONFIG_FOO TEST_VALUE
+
+/*
+ * At compiler invocation, define TEST_MACRO to STATIC_IF or
+ * STATIC_IF_NOT.
+ */
+#ifndef TEST_MACRO
+#error "This error should not be seen in the compiler output!"
+#endif
+
+/* This is intended to cause a compilation error. */
+TEST_MACRO(CONFIG_FOO) __maybe_unused int foo;
+
+void run_test(void)
+{
+	test_reset();
+
+	/* Nothing to do, observe compilation error */
+
+	test_print_result();
+}
diff --git a/test/static_if_error.sh b/test/static_if_error.sh
new file mode 100644
index 0000000000..efc7cd3e1e
--- /dev/null
+++ b/test/static_if_error.sh
@@ -0,0 +1,42 @@
+#!/bin/bash -e
+# Copyright 2019 The Chromium OS Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+# This file is implemented similar to is_enabled_error.sh
+
+TEST_DIR="$(dirname "${BASH_SOURCE[0]}")"
+TEST_CMD="$(cat "${TEST_DIR}/RO/test/static_if_error.o.cmd")"
+TEST_ERROR_COUNT=0
+BAD_ERROR_MSG="This error should not be seen in the compiler output!"
+
+fail() {
+  echo "Fail"
+  echo "$1"
+  echo "$BUILD_OUTPUT"
+  TEST_ERROR_COUNT=$((TEST_ERROR_COUNT+1))
+}
+
+for test_macro in STATIC_IF STATIC_IF_NOT; do
+  for test_value in 0 1 2 A "5 + 5"; do
+    echo -n "Running TEST_MACRO=${test_macro} TEST_VALUE=${test_value}..."
+    TEST_CMD_COMPLETE="
+      ${TEST_CMD} \"-DTEST_MACRO=${test_macro}\" \"-DTEST_VALUE=${test_value}\""
+    echo "$TEST_CMD_COMPLETE"
+    if BUILD_OUTPUT="$(sh -c "$TEST_CMD_COMPLETE" 2>&1)"; then
+      fail "Compilation should not have succeeded."
+      continue
+    fi
+
+    if grep -q "$BAD_ERROR_MSG" <<<"$BUILD_OUTPUT"; then
+      fail "TEST_MACRO was not defined."
+      continue
+    fi
+  done
+done
+
+if [[ $TEST_ERROR_COUNT -eq 0 ]]; then
+  echo "Pass!"
+else
+  echo "Fail! (${TEST_ERROR_COUNT} tests)"
+fi
diff --git a/test/static_if_error.tasklist b/test/static_if_error.tasklist
new file mode 100644
index 0000000000..5ffe662d01
--- /dev/null
+++ b/test/static_if_error.tasklist
@@ -0,0 +1,9 @@
+/* Copyright 2019 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+/**
+ * See CONFIG_TASK_LIST in config.h for details.
+ */
+#define CONFIG_TEST_TASK_LIST  /* No test task */