Remove unused typemap includes from generated C++ mojo bindings.

mojo/public/tools/bindings/generators/mojom_cpp_generator.py

Index: mojo/public/tools/bindings/generators/mojom_cpp_generator.py
diff --git a/mojo/public/tools/bindings/generators/mojom_cpp_generator.py b/mojo/public/tools/bindings/generators/mojom_cpp_generator.py
index e66bec38bd32749d604981de050d1c91755535ec..a06e928747278ba6ad84224e1ac14dbf5ebf72f0 100644
--- a/mojo/public/tools/bindings/generators/mojom_cpp_generator.py
+++ b/mojo/public/tools/bindings/generators/mojom_cpp_generator.py
@@ -187,14 +187,12 @@ def IsHashableKind(kind):
       return False
     elif mojom.IsStructKind(kind):
       if (IsTypemappedKind(kind) and
-          not _current_typemap[GetFullMojomNameForKind(kind)]["hashable"]):
+          not _current_typemap[ GetFullMojomNameForKind(kind)]["hashable"]):

[yzshen1, 2017/02/03 00:31:45]

nit: the empty space is not needed.

[Sam McNally, 2017/02/03 02:28:32]

Done.

         return False
       return all(Check(field.kind) for field in kind.fields)
     elif mojom.IsEnumKind(kind):
-      if (IsTypemappedKind(kind) and
-          not _current_typemap[GetFullMojomNameForKind(kind)]["hashable"]):
-        return False
-      return True
+      return not (IsTypemappedKind(kind) and not _current_typemap[

[yzshen1, 2017/02/03 00:31:45]

It seems a little easier to read with
"return not XXX or YYY"

[Sam McNally, 2017/02/03 02:28:32]

Done.

+          GetFullMojomNameForKind(kind)]["hashable"])
     elif mojom.IsUnionKind(kind):
       return all(Check(field.kind) for field in kind.fields)
     elif mojom.IsAnyHandleKind(kind):
@@ -419,8 +417,7 @@ def GetUnmappedTypeForSerializer(kind):
 
 def TranslateConstants(token, kind):
   if isinstance(token, mojom.NamedValue):
-    return _NameFormatter(token, _variant).FormatForCpp(
-        flatten_nested_kind=True)
+    return GetNameForKind(token, flatten_nested_kind=True)
 
   if isinstance(token, mojom.BuiltinValue):
     if token.value == "double.INFINITY":
@@ -530,6 +527,91 @@ def GetNewContainerValidateParams(kind):
   return "new mojo::internal::ContainerValidateParams(%s)" % (
       GetContainerValidateParamsCtorArgs(kind))
 
+
+def QuoteHeader(header):
+  if header[0] == '<':
+    return header
+  return '"%s"' % header
+
+
+def QuoteAndSortHeaders(headers):
+  return (QuoteHeader(header) for header in sorted(headers))
+
+
+CHROMIUM_HEADERS = {
+    "array": ["<vector>"],

[yzshen1, 2017/02/02 22:33:58]

(Not saying that I have a strong preference, just curious) Does it cost much if
we always include these headers, and only selectively include those files for
typemapping?

[Sam McNally, 2017/02/03 02:28:32]

Perf-wise, probably not too much. Including a .mojom.h file does pull in a lot
of other headers though, which makes IWYU harder. That's probably a lost cause
until modules solve everything.

I'll split it out from this CL at least.

+    "interface": [
+          "base/callback.h",
+          "mojo/public/cpp/bindings/associated_interface_ptr.h",
+          "mojo/public/cpp/bindings/associated_interface_ptr_info.h",
+          "mojo/public/cpp/bindings/associated_interface_request.h",
+          "mojo/public/cpp/bindings/interface_ptr.h",
+          "mojo/public/cpp/bindings/interface_request.h",
+          "mojo/public/cpp/bindings/raw_ptr_impl_ref_traits.h",
+          "mojo/public/cpp/bindings/thread_safe_interface_ptr.h",
+        ],
+    "map": ["<unordered_map>"],
+    "native_enum": ["mojo/public/cpp/bindings/native_enum.h"],
+    "native_struct": ["mojo/public/cpp/bindings/native_struct.h"],
+    "optional": ["base/optional.h"],
+    "string": ["<string>"],
+    "struct": [
+        "<vector>",
+        "mojo/public/cpp/bindings/lib/clone_equals_util.h",
+        "mojo/public/cpp/bindings/lib/hash_util.h",
+        "mojo/public/cpp/bindings/struct_ptr.h",
+        "mojo/public/cpp/bindings/struct_traits.h",
+    ],
+    "union": [
+        "<vector>",
+        "mojo/public/cpp/bindings/lib/union_accessor.h",
+        "mojo/public/cpp/bindings/union_traits.h",
+    ],
+}
+
+BLINK_HEADERS = {
+    "array": ["third_party/WebKit/Source/wtf/Vector.h"],
+    "interface": [
+          "base/callback.h",
+          "mojo/public/cpp/bindings/associated_interface_ptr.h",
+          "mojo/public/cpp/bindings/associated_interface_ptr_info.h",
+          "mojo/public/cpp/bindings/associated_interface_request.h",
+          "mojo/public/cpp/bindings/interface_ptr.h",
+          "mojo/public/cpp/bindings/interface_request.h",
+          "mojo/public/cpp/bindings/raw_ptr_impl_ref_traits.h",
+          "mojo/public/cpp/bindings/thread_safe_interface_ptr.h",
+        ],
+    "map": ["third_party/WebKit/Source/wtf/HashMap.h"],
+    "native_enum": ["mojo/public/cpp/bindings/native_enum.h"],
+    "native_struct": ["mojo/public/cpp/bindings/native_struct.h"],
+    "optional": ["third_party/WebKit/Source/wtf/Optional.h"],
+    "string": ["third_party/WebKit/Source/wtf/text/WTFString.h"],
+    "struct": [
+        "third_party/WebKit/Source/wtf/Vector.h",
+        "mojo/public/cpp/bindings/struct_ptr.h",
+        "mojo/public/cpp/bindings/struct_traits.h",
+        "mojo/public/cpp/bindings/lib/wtf_clone_equals_util.h",
+        "mojo/public/cpp/bindings/lib/wtf_hash_util.h",
+    ],
+    "union": [
+        "mojo/public/cpp/bindings/lib/union_accessor.h",
+        "mojo/public/cpp/bindings/union_traits.h",
+        "third_party/WebKit/Source/wtf/Vector.h",
+    ],
+}
+
+SHARED_HEADERS = {
+    "array": ["mojo/public/cpp/bindings/array_data_view.h"],
+    "enum": ["mojo/public/cpp/bindings/enum_traits.h"],
+    "interface": ["mojo/public/cpp/bindings/interface_data_view.h"],
+    "map": ["mojo/public/cpp/bindings/map_data_view.h"],
+    "native_enum": ["mojo/public/cpp/bindings/native_enum.h"],
+    "native_struct": ["mojo/public/cpp/bindings/native_struct_data_view.h"],
+    "non_native_enum": ["<ostream>"],
+    "string": ["mojo/public/cpp/bindings/string_data_view.h"],
+}
+
+
 class Generator(generator.Generator):
 
   cpp_filters = {
@@ -575,7 +657,6 @@ class Generator(generator.Generator):
     "is_typemapped_kind": IsTypemappedKind,
     "is_union_kind": mojom.IsUnionKind,
     "passes_associated_kinds": mojom.PassesAssociatedKinds,
-    "struct_size": lambda ps: ps.GetTotalSize() + _HEADER_SIZE,
     "stylize_method": generator.StudlyCapsToCamel,
     "under_to_camel": generator.UnderToCamel,
     "unmapped_type_for_serializer": GetUnmappedTypeForSerializer,
@@ -583,15 +664,112 @@ class Generator(generator.Generator):
 
   def GetExtraTraitsHeaders(self):
     extra_headers = set()
-    for entry in self.typemap.itervalues():
-      extra_headers.update(entry.get("traits_headers", []))
-    return list(extra_headers)
+    for typemap in self._GetAllUsedTypemaps():
+      extra_headers.update(typemap.get("traits_headers", []))
+    return QuoteAndSortHeaders(extra_headers)
+
+  def _GetAllUsedTypemaps(self):
+    used_typemaps = []
+    seen_types = set()
+    def AddKind(kind):
+      if (mojom.IsIntegralKind(kind) or mojom.IsStringKind(kind) or
+          mojom.IsDoubleKind(kind) or mojom.IsFloatKind(kind) or
+          mojom.IsAnyHandleKind(kind) or
+          mojom.IsInterfaceKind(kind) or
+          mojom.IsInterfaceRequestKind(kind) or
+          mojom.IsAssociatedKind(kind)):
+        pass
+      elif mojom.IsArrayKind(kind):
+        AddKind(kind.kind)
+      elif mojom.IsMapKind(kind):
+        AddKind(kind.key_kind)
+        AddKind(kind.value_kind)
+      else:
+        name = GetFullMojomNameForKind(kind)
+        if name in seen_types:
+          return
+        seen_types.add(name)
+
+        typemap = _current_typemap.get(name, None)
+        if typemap:
+          used_typemaps.append(typemap)
+        if mojom.IsStructKind(kind) or mojom.IsUnionKind(kind):
+          for field in kind.fields:
+            AddKind(field.kind)
+
+    for kind in self.module.structs + self.module.unions:

[yzshen1, 2017/02/03 00:31:45]

We should also consider enum typemapping, right? Enums could be standalone,
inside structs, insides interfaces.

[Sam McNally, 2017/02/03 02:28:32]

This is used to generate the list of traits headers needed by a .mojom.cc file.
The traits header is only required if a type is serialised within that file,
which is only when a struct, union or interface within that mojom file contains
the type. This is why the structs and unions defined within the file are
skipped. Added a comment explaining this.

+      for field in kind.fields:
+        AddKind(field.kind)
+
+    for interface in self.module.interfaces:
+      for method in interface.methods:
+        for parameter in method.parameters + (method.response_parameters or []):
+          AddKind(parameter.kind)
+
+    return used_typemaps
+
+  def _GetPublicHeadersImpl(self, header_type_to_headers):
+    header_types = set()
+    headers = set()
+
+    if self.module.structs:
+      header_types.add('struct')
+    if self.module.unions:
+      header_types.add('union')
+    if self.module.interfaces:
+      header_types.add('interface')
 
-  def GetExtraPublicHeaders(self):
-    extra_headers = set()
-    for entry in self.typemap.itervalues():
-      extra_headers.update(entry.get("public_headers", []))
-    return list(extra_headers)
+    all_enums = list(self.module.enums)
+    for struct in self.module.structs:
+      all_enums.extend(struct.enums)
+    for interface in self.module.interfaces:
+      all_enums.extend(interface.enums)
+
+    potential_types = set(GetFullMojomNameForKind(typename)
+                          for typename in
+                          self.module.structs + all_enums + self.module.unions)
+    for key, entry in self.typemap.iteritems():
+      if key in potential_types:
+        headers.update(entry.get("public_headers", []))
+    if all_enums:
+      header_types.add('enum')
+    native_enums = [enum.native_only for enum in all_enums]
+    if native_enums:
+      header_types.add('native_enum')
+    if len(native_enums) != len(all_enums):
+      header_types.add('non_native_enum')
+
+    if any(struct.native_only for struct in self.module.structs):
+      header_types.add('native_struct')
+    for kind in self._GetDirectlyUsedKinds():
+      if hasattr(kind, 'is_nullable') and kind.is_nullable:
+        header_types.add('optional')
+      if mojom.IsStringKind(kind):
+        header_types.add('string')
+      elif mojom.IsArrayKind(kind):
+        header_types.add('array')
+      elif mojom.IsMapKind(kind):
+        header_types.add('map')
+    for header_type in header_types:
+      headers.update(header_type_to_headers.get(header_type, []))
+    return QuoteAndSortHeaders(headers)
+
+  def GetPublicHeaders(self):
+    return self._GetPublicHeadersImpl(
+        BLINK_HEADERS if self.for_blink else CHROMIUM_HEADERS)
+
+  def GetPublicSharedHeaders(self):
+    return self._GetPublicHeadersImpl(SHARED_HEADERS)
+
+  def _GetDirectlyUsedKinds(self):
+    for struct in self.module.structs + self.module.unions:
+      for field in struct.fields:
+        yield field.kind
+
+    for interface in self.module.interfaces:
+      for method in interface.methods:
+        for param in method.parameters + (method.response_parameters or []):
+          yield param.kind
 
   def GetJinjaExports(self):
     structs = self.GetStructs()
@@ -615,7 +793,8 @@ class Generator(generator.Generator):
       "interfaces": interfaces,
       "variant": self.variant,
       "extra_traits_headers": self.GetExtraTraitsHeaders(),
-      "extra_public_headers": self.GetExtraPublicHeaders(),
+      "public_headers": self.GetPublicHeaders(),
+      "public_shared_headers": self.GetPublicSharedHeaders(),
       "for_blink": self.for_blink,
       "use_once_callback": self.use_once_callback,
       "export_attribute": self.export_attribute,