use variadic macros/templates in IPC message implementation

base/export_template.h

Index: base/export_template.h
diff --git a/base/export_template.h b/base/export_template.h
new file mode 100644
index 0000000000000000000000000000000000000000..11558bb62ab7abaa88579c0c450ef25ba5d145b5
--- /dev/null
+++ b/base/export_template.h
@@ -0,0 +1,104 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BASE_EXPORT_TEMPLATE_H_
+#define BASE_EXPORT_TEMPLATE_H_
+
+// Synopsis

[vmpstr, 2015/12/19 00:26:47]

Does the approach of directly checking if we're on MSVC work here?

ie

#if defined(COMPILER_MSVC)
...
#else
...
#endif

or is there something else going on?

+//
+// This header provides macros for using FOO_EXPORT macros with explicit
+// template instantiation declarations and definitions.
+// Generally, the FOO_EXPORT macros are used at declarations,
+// and GCC requires them to be used at explicit instantiation declarations,
+// but MSVC requires __declspec(dllexport) to be used at the explicit
+// instantiation definition instead.
+
+// Usage
+//
+// In a header file, write:
+//
+//   extern template class EXPORT_TEMPLATE_DECLARE(FOO_EXPORT) foo<bar>;
+//
+// In a source file, write:
+//
+//   template class EXPORT_TEMPLATE_DEFINE(FOO_EXPORT) foo<bar>;
+
+// Implementation notes
+//
+// The implementation of this header uses some subtle macro trickery to
+// detect what the provided FOO_EXPORT value was defined as and then
+// to dispatch to appropriate macro definitions.  Unfortunately,
+// MSVC's C preprocessor is rather non-compliant and requires special
+// care to make it work.
+//
+// Issue 1.
+//
+//   #define F(x)
+//   F()
+//
+// MSVC emits warning C4003 ("not enough actual parameters for macro

[danakj, 2015/12/19 00:15:05]

True in MSVC 2015?

[mdempsky, 2015/12/19 02:14:48]

Version "19.00.23602.0" at http://webcompiler.cloudapp.net/ (which I think is
2015?) at least still emits C4003.

+// 'F'), even though it's a valid macro invocation.  This affects the
+// macros below that take just an "export" parameter, because export
+// may be empty.
+//
+// As a workaround, we can add a dummy parameter and arguments:
+//
+//   #define F(x,_)
+//   F(,)
+//
+// Issue 3.

[danakj, 2015/12/19 00:15:05]

One wonders where Issue 2 is..

[mdempsky, 2015/12/19 02:14:48]

Typo.  Issue 2 had to do with chaining variadic macros, but in this latest patch
set I was able to eliminate needing to do that, so I removed the mention of the
issue.  (I thought I renumbered issue 3 too, but apparently not.)

+//
+//   #define F(x) G##x
+//   #define Gj() ok
+//   F(j())
+//
+// The correct replacement for "F(j())" is "ok", but MSVC replaces it
+// with "Gj()".  As a workaround, we can pass the result to an
+// identity macro to force MSVC to look for replacements again.  (This
+// is why EXPORT_TEMPLATE_STYLE_3 exists.)
+
+#define EXPORT_TEMPLATE_DECLARE(export) \

[danakj, 2015/12/19 00:15:05]

I think this needs a bunch more comments. I've managed to trace through to
EXPORT_TEMPLATE_STYLE_3 and forget most of the stack above me. I think this
could be documented in a way so that the connections between each are described
and one can follow each paragraph of templates and see how it's used from the
comments.

Or maybe more restructuring, I'm not sure.

It would be nice if it didn't require drawing diagrams on paper to read this
file.

[mdempsky, 2015/12/19 02:14:48]

Will do.  What sort of comments would be most useful?  Example inputs and what
they should expand to?

+  EXPORT_TEMPLATE_INVOKE(DECLARE, EXPORT_TEMPLATE_STYLE(export, ), export)
+#define EXPORT_TEMPLATE_DEFINE(export) \
+  EXPORT_TEMPLATE_INVOKE(DEFINE, EXPORT_TEMPLATE_STYLE(export, ), export)
+
+#define EXPORT_TEMPLATE_INVOKE(which, style, export) \
+  EXPORT_TEMPLATE_INVOKE_2(which, style, export)
+#define EXPORT_TEMPLATE_INVOKE_2(which, style, export) \
+  EXPORT_TEMPLATE_##which##_##style(export, )
+
+#define EXPORT_TEMPLATE_DECLARE_DEFAULT(export, _) export
+#define EXPORT_TEMPLATE_DEFINE_DEFAULT(export, _)
+
+#define EXPORT_TEMPLATE_DECLARE_MSVC_HACK(export, _)
+#define EXPORT_TEMPLATE_DEFINE_MSVC_HACK(export, _) export
+
+#define EXPORT_TEMPLATE_STYLE(export, _) EXPORT_TEMPLATE_STYLE_2(export, )
+#define EXPORT_TEMPLATE_STYLE_2(export, _) \
+  EXPORT_TEMPLATE_STYLE_3(EXPORT_TEMPLATE_STYLE_MATCH##export)
+#define EXPORT_TEMPLATE_STYLE_3(style) style
+
+#define EXPORT_TEMPLATE_STYLE_MATCH DEFAULT
+#define EXPORT_TEMPLATE_STYLE_MATCH__attribute__(...) DEFAULT

[dcheng, 2015/12/21 18:32:37]

Any name that contains a double-underscore is reserved. The Google style header
guard used to use double-underscores, and we went to some effort to migrate away
from that.

[mdempsky, 2015/12/23 23:17:45]

Hm, indeed.  (I didn't realize C++ reserved double-underscore anywhere in an
identifier; C only reserves it at the beginning.)

How problematic is that?  I don't think there's a way to implement this header's
API without defining macro names containing double-underscores.  I could change
the EXPORT_TEMPLATE_STYLE_MATCH prefix to be arbitrarily complex (e.g., contain
a random 256-bit base64-encoded value) to minimize collisions, but they'll still
technically be reserved.

(We can do without EXPORT_TEMPLATE_{DECLARE,DEFINE}, but it requires changing
every "#define IPC_MESSAGE_EXPORT FOO_EXPORT" throughout the source tree.  Not
impossible, but I was hoping to keep the user-facing API untouched for now.)

+#define EXPORT_TEMPLATE_STYLE_MATCH__declspec(arg) \
+  EXPORT_TEMPLATE_STYLE_MATCH_DECLSPEC_##arg
+
+#define EXPORT_TEMPLATE_STYLE_MATCH_DECLSPEC_dllexport MSVC_HACK
+#define EXPORT_TEMPLATE_STYLE_MATCH_DECLSPEC_dllimport DEFAULT
+
+// Sanity checks.
+
+#define EXPORT_TEMPLATE_TEST(want, export)                                 \
+  static_assert(EXPORT_TEMPLATE_INVOKE(                                    \
+                    TEST_##want, EXPORT_TEMPLATE_STYLE(export, ), export), \

[danakj, 2015/12/19 00:15:05]

Same even for the tests. This ends up as TEST_DEFAULT, which is passed to the
INVOKE.. 

[mdempsky, 2015/12/19 02:14:48]

FWIW, what happens here is that when the macros work as expected,
EXPORT_TEMPLATE_INVOKE(...) expands to

    static_assert(EXPORT_TEMPLATE_TEST_##want##_##got(,), #export)

since I define EXPORT_TEMPLATE_TEST_DEFAULT_DEFAULT(...) and
EXPORT_TEMPLATE_TEST_MSVC_HACK_MSVC_HACK(...) to true, that makes the
static_assert valid.

When the macros fail to work as intended, then it'll produce some other crazy
bogus line that causes a compiler failure.

(I'll put this into comments.)

+                #export)
+#define EXPORT_TEMPLATE_TEST_DEFAULT_DEFAULT(...) true
+#define EXPORT_TEMPLATE_TEST_MSVC_HACK_MSVC_HACK(...) true
+
+EXPORT_TEMPLATE_TEST(DEFAULT, );
+EXPORT_TEMPLATE_TEST(DEFAULT, __attribute__((visibility("default"))));
+EXPORT_TEMPLATE_TEST(MSVC_HACK, __declspec(dllexport));
+EXPORT_TEMPLATE_TEST(DEFAULT, __declspec(dllimport));
+
+#endif  // BASE_EXPORT_TEMPLATE_H_