Added test and fixed bugs with release dependency

Fixed 'visited' bug where sibling were poluting the visit set, preventing dependency expantion.
Fixed release list generation where a node's typeref releases were not constrained by the node's min/max.
Added generator unit tests to PRESUBMIT when changes to generator are detected.
Added test for release dependency when a child uses different releases than the parent.




Fix release dependencies and tests.
R=sehr@chromium.org
BUG=157017


Review URL: https://chromiumcodereview.appspot.com/11419173

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@171499 0039d316-1c4b-4281-b951-d872f2087c98

ppapi/PRESUBMIT.py

@@ -31,10 +31,10 @@ def RunUnittests(input_api, output_api):
     if name_parts[0:2] == ['ppapi', 'generators']:
       generator_files.append(filename)
   if generator_files != []:
-    cmd = [ sys.executable, 'idl_gen_pnacl.py', '--wnone', '--test']
+    cmd = [ sys.executable, 'idl_tests.py']
     ppapi_dir = input_api.PresubmitLocalPath()
     results.extend(RunCmdAndCheck(cmd,
-                                  'PPAPI IDL Pnacl unittest failed.',
+                                  'PPAPI IDL unittests failed.',
                                   output_api,
                                   os.path.join(ppapi_dir, 'generators')))
   return results

ppapi/generators/idl_c_header.py

@@ -23,15 +23,19 @@
 Option('guard', 'Include guard prefix', default=os.path.join('ppapi', 'c'))
 
 
-def GetOutFileName(filenode, relpath=None, prefix=None):
+def GetPathFromNode(filenode, relpath=None, ext=None):
   path, name = os.path.split(filenode.GetProperty('NAME'))
-  name = os.path.splitext(name)[0] + '.h'
-  if prefix: name = '%s%s' % (prefix, name)
+  if ext: name = os.path.splitext(name)[0] + ext
   if path: name = os.path.join(path, name)
   if relpath: name = os.path.join(relpath, name)
+  name = os.path.normpath(name)
   return name
 
 
+def GetHeaderFromNode(filenode, relpath=None):
+  return GetPathFromNode(filenode, relpath, ext='.h')
+
+
 def WriteGroupMarker(out, node, last_group):
   # If we are part of a group comment marker...
   if last_group and last_group != node.cls:
@@ -108,7 +112,7 @@ def __init__(self):
     Generator.__init__(self, 'C Header', 'cgen', 'Generate the C headers.')
 
   def GenerateFile(self, filenode, releases, options):
-    savename = GetOutFileName(filenode, GetOption('dstroot'))
+    savename = GetHeaderFromNode(filenode, GetOption('dstroot'))
     my_min, my_max = filenode.GetMinMax(releases)
     if my_min > releases[-1] or my_max < releases[0]:
       if os.path.isfile(savename):
@@ -126,7 +130,7 @@ def GenerateHead(self, out, filenode, releases, options):
     __pychecker__ = 'unusednames=options'
     cgen = CGen()
     gpath = GetOption('guard')
-    def_guard = GetOutFileName(filenode, relpath=gpath)
+    def_guard = GetHeaderFromNode(filenode, relpath=gpath)
     def_guard = def_guard.replace(os.sep,'_').replace('.','_').upper() + '_'
 
     cright_node = filenode.GetChildren()[0]
@@ -137,8 +141,7 @@ def GenerateHead(self, out, filenode, releases, options):
     out.Write('%s\n' % cgen.Copyright(cright_node))
 
     # Wrap the From ... modified ... comment if it would be >80 characters.
-    from_text = 'From %s' % (
-        filenode.GetProperty('NAME').replace(os.sep,'/'))
+    from_text = 'From %s' % GetPathFromNode(filenode)
     modified_text = 'modified %s.' % (
         filenode.GetProperty('DATETIME'))
     if len(from_text) + len(modified_text) < 74:
@@ -158,7 +161,7 @@ def GenerateHead(self, out, filenode, releases, options):
       depfile = dep.GetProperty('FILE')
       if depfile:
         includes.add(depfile)
-    includes = [GetOutFileName(
+    includes = [GetHeaderFromNode(
         include, relpath=gpath).replace(os.sep, '/') for include in includes]
     includes.append('ppapi/c/pp_macros.h')
 
@@ -167,7 +170,8 @@ def GenerateHead(self, out, filenode, releases, options):
       includes.append('ppapi/c/pp_stdint.h')
 
     includes = sorted(set(includes))
-    cur_include = GetOutFileName(filenode, relpath=gpath).replace(os.sep, '/')
+    cur_include = GetHeaderFromNode(filenode,
+                                    relpath=gpath).replace(os.sep, '/')
     for include in includes:
       if include == cur_include: continue
       out.Write('#include "%s"\n' % include)
@@ -209,14 +213,14 @@ def GenerateBody(self, out, filenode, releases, options):
   def GenerateTail(self, out, filenode, releases, options):
     __pychecker__ = 'unusednames=options,releases'
     gpath = GetOption('guard')
-    def_guard = GetOutFileName(filenode, relpath=gpath)
+    def_guard = GetPathFromNode(filenode, relpath=gpath, ext='.h')
     def_guard = def_guard.replace(os.sep,'_').replace('.','_').upper() + '_'
     out.Write('#endif  /* %s */\n\n' % def_guard)
 
 
 hgen = HGen()
 
-def Main(args):
+def main(args):
   # Default invocation will verify the golden files are unchanged.
   failed = 0
   if not args:
@@ -249,5 +253,5 @@ def Main(args):
   return failed
 
 if __name__ == '__main__':
-  sys.exit(Main(sys.argv[1:]))
+  sys.exit(main(sys.argv[1:]))
 

ppapi/generators/idl_c_proto.py

@@ -449,7 +449,7 @@ def DefineTypedef(self, node, releases, prefix='', comment=False):
 
     # TODO(noelallen) : Bug 157017 finish multiversion support
     if len(build_list) != 1:
-      node.Error('Can not support multiple versions of node.')
+      node.Error('Can not support multiple versions of node: %s' % build_list)
     assert len(build_list) == 1
 
     out = 'typedef %s;\n' % self.GetSignature(node, build_list[0], 'return',
@@ -671,7 +671,8 @@ def TestFile(filenode):
     outstr = CleanString(outstr)
 
     if instr != outstr:
-      ErrOut.Log('Failed match of\n>>%s<<\n>>%s<<\nto:' % (instr, outstr))
+      ErrOut.Log('Failed match of\n>>%s<<\nto:\n>>%s<<\nFor:\n' %
+                 (instr, outstr))
       node.Dump(1, comments=True)
       errors += 1
   return errors
@@ -701,7 +702,7 @@ def TestFiles(filenames):
     InfoOut.Log('Passed generator test.')
   return total_errs
 
-def Main(args):
+def main(args):
   filenames = ParseOptions(args)
   if GetOption('test'):
     return TestFiles(filenames)
@@ -716,5 +717,5 @@ def Main(args):
 
 
 if __name__ == '__main__':
-  sys.exit(Main(sys.argv[1:]))
+  sys.exit(main(sys.argv[1:]))
 

ppapi/generators/idl_log.py

@@ -12,6 +12,7 @@
 #
 import sys
 
+
 class IDLLog(object):
   def __init__(self, name, out):
     if name:

ppapi/generators/idl_node.py

@@ -322,7 +322,8 @@ def _GetReleases(self, releases):
     return self.releases
 
 
-  def _GetReleaseList(self, releases, visited=set()):
+  def _GetReleaseList(self, releases, visited=None):
+    visited = visited or set()
     if not self.releases:
       # If we are unversionable, then return first available release
       if self.IsA('Comment', 'Copyright', 'Label'):
@@ -344,36 +345,38 @@ def _GetReleaseList(self, releases, visited=set()):
 
       visited |= set([self])
 
-      # Files inherit all thier releases from items in the file
+      # Files inherit all their releases from items in the file
       if self.IsA('AST', 'File'):
         my_releases = set()
 
       # Visit all children
       child_releases = set()
+
+      # Exclude sibling results from parent visited set
+      cur_visits = visited
+
       for child in self.children:
-        child_releases |= set(child._GetReleaseList(releases, visited))
+        child_releases |= set(child._GetReleaseList(releases, cur_visits))
         visited |= set(child_releases)
 
       # Visit my type
       type_releases = set()
       if self.typelist:
         type_list = self.typelist.GetReleases()
         for typenode in type_list:
-          type_releases |= set(typenode._GetReleaseList(releases, visited))
-          visited |= set(type_releases)
+          type_releases |= set(typenode._GetReleaseList(releases, cur_visits))
 
         type_release_list = sorted(type_releases)
         if my_min < type_release_list[0]:
           type_node = type_list[0]
           self.Error('requires %s in %s which is undefined at %s.' % (
               type_node, type_node.filename, my_min))
 
-      for rel in child_releases:
+      for rel in child_releases | type_releases:
         if rel >= my_min and rel <= my_max:
           my_releases |= set([rel])
 
       self.releases = sorted(my_releases)
-
     return self.releases
 
   def GetReleaseList(self):

ppapi/generators/idl_parser.py

@@ -1130,22 +1130,15 @@ def FindVersionError(releases, node):
   if node.IsA('Interface', 'Struct'):
     comment_list = []
     comment = node.GetOneOf('Comment')
-    if comment:
-      print comment.GetName()
     if comment and comment.GetName()[:4] == 'REL:':
       comment_list = comment.GetName()[5:].strip().split(' ')
-      print comment_list
 
-    if len(comment_list) != len(releases):
-      node.Error("Mismatch size of releases: %s vs %s." % (
-          comment_list, releases))
+    first_list = [node.first_release[rel] for rel in releases]
+    first_list = sorted(set(first_list))
+    if first_list != comment_list:
+      node.Error("Mismatch in releases: %s vs %s." % (
+          comment_list, first_list))
       err_cnt += 1
-    else:
-      first_list = [node.first_release[rel] for rel in releases]
-      if first_list != comment_list:
-        node.Error("Mismatch in releases: %s vs %s." % (
-            comment_list, first_list))
-        err_cnt += 1
 
   for child in node.GetChildren():
     err_cnt += FindVersionError(releases, child)

ppapi/generators/idl_tests.py

@@ -0,0 +1,41 @@
+#!/usr/bin/env python
+# Copyright (c) 2012 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.
+
+""" Test runner for IDL Generator changes """
+
+import subprocess
+import sys
+
+def TestIDL(testname, args):
+  print '\nRunning unit tests for %s.' % testname
+  try:
+    args = [sys.executable, testname] + args
+    subprocess.check_call(args)
+    return 0
+  except subprocess.CalledProcessError as err:
+    print 'Failed with %s.' % str(err)
+    return 1
+
+def main(args):
+  errors = 0
+  errors += TestIDL('idl_lexer.py', ['--test'])
+  assert errors == 0
+  errors += TestIDL('idl_parser.py', ['--test'])
+  assert errors == 0
+  errors += TestIDL('idl_c_header.py', [])
+  assert errors == 0
+  errors += TestIDL('idl_c_proto.py', ['--wnone', '--test'])
+  assert errors == 0
+  errors += TestIDL('idl_gen_pnacl.py', ['--wnone', '--test'])
+  assert errors == 0
+
+  if errors:
+    print '\nFailed tests.'
+  return errors
+
+
+if __name__ == '__main__':
+  sys.exit(main(sys.argv[1:]))
+

ppapi/generators/test_cgen/enum_typedef.h

@@ -4,7 +4,7 @@
  * found in the LICENSE file.
  */
 
-/* From test_cgen/enum_typedef.idl modified Mon Aug 22 15:15:49 2011. */
+/* From test_cgen/enum_typedef.idl modified Wed Dec  5 13:08:05 2012. */
 
 #ifndef PPAPI_C_TEST_CGEN_ENUM_TYPEDEF_H_
 #define PPAPI_C_TEST_CGEN_ENUM_TYPEDEF_H_
@@ -41,11 +41,11 @@ typedef enum {
 /* typedef int32_t i; */
 typedef int32_t i;
 
-/* typedef int32_t i2[2]; */
-typedef int32_t i2[2];
+/* typedef int32_t i2[3]; */
+typedef int32_t i2[3];
 
-/* typedef int32_t (*i_func)(); */
-typedef int32_t (*i_func)();
+/* typedef int32_t (*i_func)(void); */
+typedef int32_t (*i_func)(void);
 
 /* typedef int32_t (*i_func_i)(int32_t i); */
 typedef int32_t (*i_func_i)(int32_t i);

ppapi/generators/test_cgen/enum_typedef.idl

@@ -14,10 +14,10 @@ enum et1 { A=1, B=2, C=3, D=A+B, E=~D };
 /* typedef int32_t i; */
 typedef int32_t i;
 
-/* typedef int32_t i2[2]; */
-typedef int32_t[2] i2;
+/* typedef int32_t i2[3]; */
+typedef int32_t[3] i2;
 
-/* typedef int32_t (*i_func)(); */
+/* typedef int32_t (*i_func)(void); */
 typedef int32_t i_func();
 
 /* typedef int32_t (*i_func_i)(int32_t i); */

ppapi/generators/test_cgen/interface.h

@@ -4,14 +4,19 @@
  * found in the LICENSE file.
  */
 
-/* From test_cgen/interface.idl modified Mon Aug 22 15:15:43 2011. */
+/* From test_cgen/interface.idl modified Wed Nov 21 14:22:50 2012. */
 
 #ifndef PPAPI_C_TEST_CGEN_INTERFACE_H_
 #define PPAPI_C_TEST_CGEN_INTERFACE_H_
 
 #include "ppapi/c/pp_macros.h"
 #include "ppapi/c/test_cgen/stdint.h"
 
+#define IFACEFOO_INTERFACE_1_0 "ifaceFoo;1.0"
+#define IFACEFOO_INTERFACE IFACEFOO_INTERFACE_1_0
+
+#define IFACEBAR_INTERFACE_1_0 "ifaceBar;1.0"
+#define IFACEBAR_INTERFACE IFACEBAR_INTERFACE_1_0
 
 /**
  * @file
@@ -36,16 +41,41 @@ struct ist {
  * @{
  */
 /*
- * struct iface1 {
+ * struct ifaceFoo_1_0 {
  * int8_t (*mem1)(int16_t x, int32_t y);
  * int32_t (*mem2)(const struct ist* a);
  * int32_t (*mem3)(struct ist* b);
+ * int32_t (*mem4)(const void** ptr);
+ * int32_t (*mem5)(void** ptr);
  * };
+ * typedef struct ifaceFoo_1_0 ifaceFoo;
  */
-struct iface1 {
+struct ifaceFoo_1_0 {
   int8_t (*mem1)(int16_t x, int32_t y);
   int32_t (*mem2)(const struct ist* a);
   int32_t (*mem3)(struct ist* b);
+  int32_t (*mem4)(const void** ptr);
+  int32_t (*mem5)(void** ptr);
+};
+
+typedef struct ifaceFoo_1_0 ifaceFoo;
+
+struct ifaceBar_1_0 {
+  int8_t (*testIface)(const struct ifaceFoo_1_0* foo, int32_t y);
+  struct ifaceFoo_1_0* (*createIface)(const char* name);
+};
+
+typedef struct ifaceBar_1_0 ifaceBar;
+/**
+ * @}
+ */
+
+/**
+ * @addtogroup Structs
+ * @{
+ */
+struct struct2 {
+  struct ifaceBar_1_0* bar;
 };
 /**
  * @}

ppapi/generators/test_cgen/interface.idl

@@ -18,14 +18,14 @@ struct ist {
 };
 
 /*
- * struct iface1 {
+ * struct ifaceFoo_1_0 {
  * int8_t (*mem1)(int16_t x, int32_t y);
  * int32_t (*mem2)(const struct ist* a);
  * int32_t (*mem3)(struct ist* b);
  * int32_t (*mem4)(const void** ptr);
  * int32_t (*mem5)(void** ptr);
- * int32_t (*mem6)(void** ptr);
  * };
+ * typedef struct ifaceFoo_1_0 ifaceFoo;
  */
 interface ifaceFoo {
   int8_t mem1([in] int16_t x, [in] int32_t y);