[workspace] Make vendor_cxx smarter (#17334)

Reduce the flapping of open/close namespaces.
No need to wrap blank lines nor preprocessor directives.

tools/workspace/vendor_cxx.py

@@ -8,17 +8,138 @@
 """
 
 import argparse
+from enum import Enum
+import re
+
+
+def _designate_wrapped_lines(lines):
+    """Given a list[str] of the lines in a C++ source file, returns a
+    list[bool] that is True iff the corresponding line should be part
+    of the inline hidden namespace.
+
+    We MUST wrap all C/C++ code. We must NOT wrap #include statements.
+
+    Blank lines and other non-C++ lines such as comments or non-#include
+    preprocessor directives can go either way; we'll group them into the
+    wrapping status of their neighbors in order to minimize the number of
+    added lines.
+    """
+    class Flag(Enum):
+        WRAP = 1
+        NO_WRAP = -1
+        DONT_CARE = 0
+
+    # Regexs to match various kinds of code patterns.
+    is_include = re.compile(r'^\s*#\s*include\s*["<].*$')
+    is_preprocessor = re.compile(r'^\s*#\s*[a-zA-Z].*$')
+    is_blank = re.compile(r'^\s*$')
+    is_blank_cpp_comment = re.compile(r'^\s*//.*$')
+    is_blank_c_comment_begin = re.compile(r'^\s*/\*.*$')
+    is_c_comment_end = re.compile(r'^.*\*/\s*(.*)$')
+
+    # Loop over all lines and determine each one's flag.
+    flags = [None] * len(lines)
+    i = 0
+    while i < len(lines):
+        line = lines[i]
+        # When the prior line has continuation, this line inherits its Flag.
+        if i > 0 and lines[i - 1].endswith('\\'):
+            flags[i] = flags[i - 1]
+            i += 1
+            continue
+        # We must NOT wrap #include statements.
+        if is_include.match(line):
+            flags[i] = Flag.NO_WRAP
+            i += 1
+            continue
+        # Other preprocessor directives can go either way.
+        if is_preprocessor.match(line):
+            flags[i] = Flag.DONT_CARE
+            i += 1
+            continue
+        # Blank lines (or lines that are blank other than their comments)
+        # can go either way.
+        if is_blank.match(line) or is_blank_cpp_comment.match(line):
+            flags[i] = Flag.DONT_CARE
+            i += 1
+            continue
+        # For C-style comments, consume the entire comment block immediately.
+        if is_blank_c_comment_begin.match(line):
+            first_c_comment_line = i
+            while True:
+                line = lines[i]
+                match = is_c_comment_end.match(line)
+                flags[i] = Flag.DONT_CARE
+                i += 1
+                if match:
+                    break
+            # If the close-comment marker had code after it, we need to go back
+            # and set the entire C-style comment to WRAP.
+            (trailing,) = match.groups()
+            if trailing:
+                for fixup in range(first_c_comment_line, i):
+                    flags[fixup] = Flag.WRAP
+            continue
+        # We MUST wrap all C/C++ code.
+        flags[i] = Flag.WRAP
+        i += 1
+
+    # We want to insert inline namespaces such that:
+    #
+    # - all WRAP lines are enclosed;
+    # - no NO_WRAP lines are enclosed;
+    # - the only DONT_CARE lines enclosed are surrouneded by WRAP.
+    #
+    # We'll do that by growing the NO_WRAP spans as large as possible.
+
+    # Grow the start-of-file run of NO_WRAP:
+    for i in range(len(flags)):
+        if flags[i] == Flag.DONT_CARE:
+            flags[i] = Flag.NO_WRAP
+        else:
+            break
+
+    # Grow the end-of-file run of NO_WRAP:
+    for i in range(len(flags) - 1, -1, -1):
+        if flags[i] == Flag.DONT_CARE:
+            flags[i] = Flag.NO_WRAP
+        else:
+            break
+
+    # Grow any interior regions of NO_WRAP:
+    for i in range(len(flags)):
+        if flags[i] == Flag.NO_WRAP:
+            # Change all of the immediately prior and subsequent homogeneous
+            # runs of DONT_CARE to NO_WRAP.
+            for j in range(i - 1, -1, -1):
+                if flags[j] == Flag.DONT_CARE:
+                    flags[j] = Flag.NO_WRAP
+                else:
+                    break
+            for j in range(i + 1, len(flags)):
+                if flags[j] == Flag.DONT_CARE:
+                    flags[j] = Flag.NO_WRAP
+                else:
+                    break
+
+    # Anything remaining is DONT_CARE bookended by WRAP, so we'll WRAP it.
+    for i in range(len(flags)):
+        if flags[i] == Flag.DONT_CARE:
+            flags[i] = Flag.WRAP
+
+    # Return True only for the wrapped lines.
+    return [x == Flag.WRAP for x in flags]
 
 
 def _rewrite_one_text(*, text, edit_include):
     """Rewrites the C++ file contents in `text` with specific alterations:
 
     - The paths in #include statements are replaced per the (old, new) pairs in
-    the include_edit list. Only includes that use quotation marks will be
-    changed. The pairs are literal strings that must be a prefix of the path.
+    the include_edit list. The pairs are literal strings that must be a prefix
+    of the path.
 
     - Wraps an inline namespace "drake_vendor" with hidden symbol visibility
-    around the entire file; it is withdrawn prior to any include statement.
+    around all of the code in file (but not any #include statements).
 
     Returns the new C++ contents.
 
@@ -29,30 +150,31 @@ def _rewrite_one_text(*, text, edit_include):
     # Re-spell the project's own include statements.
     for old_inc, new_inc in edit_include:
         text = text.replace(f'#include "{old_inc}', f'#include "{new_inc}')
+        text = text.replace(f'#include <{old_inc}', f'#include <{new_inc}')
 
-    # Add an inline namespace around the whole file, but disable it around
-    # include statements.
+    # We'll add an inline namespace around the C++ code in this file.
+    # Designate each line of the file for whether it should be wrapped.
+    lines = text.split('\n')
+    if lines[-1] == '':
+        lines.pop()
+    should_wrap = _designate_wrapped_lines(lines)
+
+    # Anytime the sense of wrapping switches, we'll insert a line.
+    # Do this in reverse order so that the indices into lines[] are stable.
     open_inline = ' '.join([
         'inline namespace drake_vendor',
         '__attribute__ ((visibility ("hidden")))',
-        '{\n'])
-    close_inline = '}  /* inline namespace drake_vendor */\n'
-    text = open_inline + text + close_inline
-    search_start = 0
-    while True:
-        i = text.find('\n#include ', search_start)
-        if i < 0:
-            break
-        first = i + 1
-        last = text.index('\n', first) + 1
-        text = (
-            text[:first]
-            + close_inline
-            + text[first:last]
-            + open_inline
-            + text[last:])
-        search_start = last + len(open_inline) + len(close_inline) - 1
-
+        '{'])
+    close_inline = '}  /* inline namespace drake_vendor */'
+    for i in range(len(lines), -1, -1):
+        this_wrap = should_wrap[i] if i < len(lines) else False
+        prior_wrap = should_wrap[i - 1] if i > 1 else False
+        if this_wrap == prior_wrap:
+            continue
+        insertion = open_inline if this_wrap else close_inline
+        lines.insert(i, insertion)
+
+    text = '\n'.join(lines) + '\n'
     return text
 
 

tools/workspace/vendor_cxx_test.py

@@ -26,23 +26,88 @@ def _check(self, old_lines, expected_new_lines):
         expected_new_text = '\n'.join(expected_new_lines) + '\n'
         self.assertMultiLineEqual(expected_new_text, new_text)
 
-    def test_simple(self):
+    def test_comments(self):
         self._check([
-            '#include "somelib/somefile.h"',
-            '#include "unrelated/thing.h"',
+            ' // file comment',
+            ' /* block comment',
+            '    continues */',
+            'int foo();  // eol comment',
+            'int bar();  /* eol comment */',
+            ' /* intro comment */ class Baz;',
+            ' /* single line c style */',
+            '#include <something.h>',
+            ' /* intro comment that spans multiple lines',
+            '    but continues with code after */ class Quux;',
+        ], [
+            ' // file comment',
+            ' /* block comment',
+            '    continues */',
+            self._open,
+            'int foo();  // eol comment',
+            'int bar();  /* eol comment */',
+            ' /* intro comment */ class Baz;',
+            self._close,
+            ' /* single line c style */',
+            '#include <something.h>',
+            self._open,
+            ' /* intro comment that spans multiple lines',
+            '    but continues with code after */ class Quux;',
+            self._close,
+        ])
+
+    def test_preprocessor(self):
+        self._check([
+            '#define FOO \\',
+            '   foo',          # This line looks like code, but it isn't.
             'int foo();',
         ], [
-            # Adjacent pairs of open/close are useless; ideally, we teach
-            # vendor_cxx to skip these.
-            self._open, self._close,  # TODO(jwnimmer-tri) Useless filler.
+            '#define FOO \\',
+            '   foo',
+            self._open,
+            'int foo();',
+            self._close,
+        ])
 
-            # All include statements must NOT be within an open-namespace.
-            '#include "drake_vendor/somelib/somefile.h"',
+    def test_include_guard(self):
+        self._check([
+            '#ifndef FOO_HH',
+            '#define FOO_HH',
+            '#include <something.h>',
+            'int foo();',
+            '#endif FOO_HH',
+        ], [
+            '#ifndef FOO_HH',
+            '#define FOO_HH',
+            '#include <something.h>',
+            self._open,
+            'int foo();',
+            self._close,
+            '#endif FOO_HH',
+        ])
 
-            self._open, self._close,  # TODO(jwnimmer-tri) Useless filler.
-            '#include "unrelated/thing.h"',
+    def test_pragma_once(self):
+        self._check([
+            '#pragma once',
+            '#include <something.h>',
+            'int foo();',
+        ], [
+            '#pragma once',
+            '#include <something.h>',
+            self._open,
+            'int foo();',
+            self._close,
+        ])
 
-            # All code MUST be within an open-namespace.
+    def test_edit_include(self):
+        self._check([
+            '#include "somelib/somefile.h"',
+            '#include <somelib/otherfile.h>',
+            '#include <unrelated/thing.h>',
+            'int foo();',
+        ], [
+            '#include "drake_vendor/somelib/somefile.h"',
+            '#include <drake_vendor/somelib/otherfile.h>',
+            '#include <unrelated/thing.h>',
             self._open,
             'int foo();',
             self._close,