Changeset 106315 in webkit

Timestamp:

Jan 30, 2012 6:40:24 PM (12 years ago)

Author:

haraken@chromium.org

Message:

REGRESSION(r105797): prepare-ChangeLog for a .cpp file can
output an empty method name (i.e. "()")
​https://bugs.webkit.org/show_bug.cgi?id=77336

Reviewed by Darin Adler.

r105797 tried to detect a change outside methods, but it causes a bug that
prepare-ChangeLog can output an empty method name, like this:

• foo/bar/baz.cpp: (method1): (): (method2):

This is because the cpp parser in prepare-ChangeLog cannot distinguish
'{' as the beginning of a method with '{' as the beginning of an array definition
at the top level.

int a[] = { 1, 2, 3 }; This '{' is the beginning of an array definition.

void func() { This '{' is the beginning of a method.

...;

}

This patch fixes prepare-ChangeLog so that it skips an array definition at the top level.

• Scripts/prepare-ChangeLog:

(get_function_line_ranges_for_cpp): Modified as described above.
(generateFunctionLists): As a hack, modified so that prepare-ChangeLog does not output
an empty method name. Ideally this should not happen but may happen, since the
parsers are not perfect.

• Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp: Added test cases.

(NameSpace7):
(NameSpace8):
(Class109):

• Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests-expected.txt:

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/prepare-ChangeLog (modified) (4 diffs)
• Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests-expected.txt (modified) (1 diff)
• Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+2012-01-30  Kentaro Hara  <haraken@chromium.org>
+
+        REGRESSION(r105797): prepare-ChangeLog for a .cpp file can
+        output an empty method name (i.e. "()")
+        https://bugs.webkit.org/show_bug.cgi?id=77336
+
+        Reviewed by Darin Adler.
+
+        r105797 tried to detect a change outside methods, but it causes a bug that
+        prepare-ChangeLog can output an empty method name, like this:
+
+            * foo/bar/baz.cpp:
+            (method1):
+            ():
+            (method2):
+
+        This is because the cpp parser in prepare-ChangeLog cannot distinguish
+        '{' as the beginning of a method with '{' as the beginning of an array definition
+        at the top level.
+
+            int a[] = { 1, 2, 3 };  // This '{' is the beginning of an array definition.
+
+            void func() { // This '{' is the beginning of a method.
+                ...;
+            }
+
+        This patch fixes prepare-ChangeLog so that it skips an array definition at the top level.
+
+        * Scripts/prepare-ChangeLog:
+        (get_function_line_ranges_for_cpp): Modified as described above.
+        (generateFunctionLists): As a hack, modified so that prepare-ChangeLog does not output
+        an empty method name. Ideally this should not happen but may happen, since the
+        parsers are not perfect.
+        * Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp: Added test cases.
+        (NameSpace7):
+        (NameSpace8):
+        (Class109):
+        * Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests-expected.txt:
+
 2012-01-30  Ojan Vafai  <ojan@chromium.org>
 

trunk/Tools/Scripts/prepare-ChangeLog

@@ -271 +271 @@
                 my @function_range = @$function_range_ref;
 
+                # FIXME: This is a hack. If the function name is empty, skip it.
+                # The cpp, python, javascript, perl, css and java parsers
+                # are not perfectly implemented and sometimes function names cannot be retrieved
+                # correctly. As you can see in get_function_line_ranges_XXXX(), those parsers
+                # are not intended to implement real parsers but intended to just retrieve function names
+                # for most practical syntaxes.
+                next unless $function_range[2];
+
                 # Advance to successive change ranges.
                 for (;; @change_range = @{shift @change_ranges}) {
@@ -619 +627 @@
     my $in_parentheses = 0;
     my $in_braces = 0;
+    my $in_toplevel_array_brace = 0;
     my $brace_start = 0;
     my $brace_end = 0;
     my $namespace_start = -1;
     my $skip_til_brace_or_semicolon = 0;
+    my $equal_observed = 0;
 
     my $word = "";
@@ -750 +760 @@
 
         # Find function, interface and method names.
-        while (m&((?:[[:word:]]+::)*operator(?:[ \t]*\(\)|[^()]*)|[[:word:]:~]+|[(){}:;])|\@(?:implementation|interface|protocol)\s+(\w+)[^{]*&g) {
+        while (m&((?:[[:word:]]+::)*operator(?:[ \t]*\(\)|[^()]*)|[[:word:]:~]+|[(){}:;=])|\@(?:implementation|interface|protocol)\s+(\w+)[^{]*&g) {
+            # Skip an array definition at the top level.
+            # e.g. static int arr[] = { 1, 2, 3 };
+            if ($1) {
+                if ($1 eq "=" and !$in_parentheses and !$in_braces) {
+                    $equal_observed = 1;
+                } elsif ($1 eq "{" and $equal_observed) {
+                    # This '{' is the beginning of an array definition, not the beginning of a method.
+                    $in_toplevel_array_brace = 1;
+                    $in_braces++;
+                    $equal_observed = 0;
+                    next;
+                } elsif ($1 !~ /[ \t]/) {
+                    $equal_observed = 0;
+                }
+            }
+
             # interface name
             if ($2) {
@@ -834 +860 @@
                 # This could be a function body.
                 if (!$in_braces and $name) {
+                    # This is the end of an array definition at the top level, not the end of a method.
+                    if ($in_toplevel_array_brace) {
+                        $in_toplevel_array_brace = 0;
+                        next;
+                    }
+
                     push @ranges, [ $start, $., $name ];
                     if (@namespaces) {

trunk/Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests-expected.txt

@@ -327 +327 @@
       '321',
       'Class108'
+    ],
+    [
+      '340',
+      '354',
+      'NameSpace7'
+    ],
+    [
+      '356',
+      '369',
+      'NameSpace8'
+    ],
+    [
+      '371',
+      '371',
+      'NameSpace7'
+    ],
+    [
+      '373',
+      '386',
+      'Class109'
+    ],
+    [
+      '388',
+      '388',
+      'NameSpace7'
     ]
   ]

trunk/Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/resources/cpp_unittests.cpp

@@ -321 +321 @@
     int h;
 };
+
+int a[] = { };
+int a[] = {
+};
+int a[] = { 1, 2, 3 };
+int a[] = {
+    1,
+    2,
+    3
+};
+int a[3] = { 1, 2, 3 };
+int a[][3] = { {1, 2, 3}, {4, 5, 6} };
+int a[2][3] = { {1, 2, 3}, {4, 5, 6} };
+extern int a[];
+char a[4] = "test";
+
+namespace NameSpace7 {
+int a[] = { };
+int a[] = {
+};
+int a[] = { 1, 2, 3 };
+int a[] = {
+    1,
+    2,
+    3
+};
+int a[3] = { 1, 2, 3 };
+int a[][3] = { {1, 2, 3}, {4, 5, 6} };
+int a[2][3] = { {1, 2, 3}, {4, 5, 6} };
+extern int a[];
+char a[4] = "test";
+
+namespace NameSpace8 {
+int a[] = { };
+int a[] = {
+};
+int a[] = { 1, 2, 3 };
+int a[] = {
+    1,
+    2,
+    3
+};
+int a[3] = { 1, 2, 3 };
+int a[][3] = { {1, 2, 3}, {4, 5, 6} };
+int a[2][3] = { {1, 2, 3}, {4, 5, 6} };
+extern int a[];
+char a[4] = "test";
+};
+
+class Class109 {
+    int a[] = { };
+    int a[] = {
+    };
+    int a[] = { 1, 2, 3 };
+    int a[] = {
+        1,
+        2,
+        3
+    };
+    int a[3] = { 1, 2, 3 };
+    int a[][3] = { {1, 2, 3}, {4, 5, 6} };
+    int a[2][3] = { {1, 2, 3}, {4, 5, 6} };
+    extern int a[];
+    char a[4] = "test";
+};
+
+};