tools: API boosting support for using decls and enum constants. (#9418)

Via AST node matchers on UsingDecl and DeclRefExpr. This commit also
refactors the clang::tooling::SourceFileCallbacks run() routine, by
moving to a per AST node dispatch model. This will support scaling to
additional AST node types in the future without having run() grow to
an unreadable length.

Risk level: Low
Testing: Additional golden tests.

Part of #8082.

Signed-off-by: Harvey Tuch <htuch@google.com>

tools/api_boost/api_boost.py

@@ -82,7 +82,8 @@ def RewriteIncludes(args):
 def ApiBoostTree(target_paths,
                  generate_compilation_database=False,
                  build_api_booster=False,
-                 debug_log=False):
+                 debug_log=False,
+                 sequential=False):
   dep_build_targets = ['//%s/...' % PrefixDirectory(prefix) for prefix in target_paths]
 
   # Optional setup of state. We need the compilation database and api_booster
@@ -139,12 +140,15 @@ def ApiBoostTree(target_paths,
     file_path = entry['file']
     if any(file_path.startswith(prefix) for prefix in target_paths):
       file_paths.add(file_path)
+  # Ensure a determinstic ordering if we are going to process sequentially.
+  if sequential:
+    file_paths = sorted(file_paths)
 
   # The API boosting is file local, so this is trivially parallelizable, use
   # multiprocessing pool with default worker pool sized to cpu_count(), since
   # this is CPU bound.
   try:
-    with mp.Pool() as p:
+    with mp.Pool(processes=1 if sequential else None) as p:
       # We need multiple phases, to ensure that any dependency on files being modified
       # in one thread on consumed transitive headers on the other thread isn't an
       # issue. This also ensures that we complete all analysis error free before
@@ -173,7 +177,11 @@ def ApiBoostTree(target_paths,
   parser.add_argument('--generate_compilation_database', action='store_true')
   parser.add_argument('--build_api_booster', action='store_true')
   parser.add_argument('--debug_log', action='store_true')
+  parser.add_argument('--sequential', action='store_true')
   parser.add_argument('paths', nargs='*', default=['source', 'test', 'include'])
   args = parser.parse_args()
-  ApiBoostTree(args.paths, args.generate_compilation_database, args.build_api_booster,
-               args.debug_log)
+  ApiBoostTree(args.paths,
+               generate_compilation_database=args.generate_compilation_database,
+               build_api_booster=args.build_api_booster,
+               debug_log=args.debug_log,
+               sequential=args.sequential)

tools/api_boost/api_boost_test.py

@@ -5,6 +5,8 @@
 # tools/clang_tools/api_booster, as well as the type whisperer and API type
 # database.
 
+import argparse
+from collections import namedtuple
 import logging
 import os
 import pathlib
@@ -15,10 +17,15 @@
 
 import api_boost
 
+TestCase = namedtuple('TestCase', ['name', 'description'])
+
 # List of test in the form [(file_name, explanation)]
-TESTS = [
-    ('elaborated_type.cc', 'ElaboratedTypeLoc type upgrades'),
-]
+TESTS = list(
+    map(lambda x: TestCase(*x), [
+        ('elaborated_type', 'ElaboratedTypeLoc type upgrades'),
+        ('using_decl', 'UsingDecl upgrades for named types'),
+        ('decl_ref_expr', 'DeclRefExpr upgrades for named constants'),
+    ]))
 
 TESTDATA_PATH = 'tools/api_boost/testdata'
 
@@ -31,29 +38,45 @@ def Diff(some_path, other_path):
 
 
 if __name__ == '__main__':
+  parser = argparse.ArgumentParser(description='Golden C++ source tests for api_boost.py')
+  parser.add_argument('tests', nargs='*')
+  args = parser.parse_args()
+
   # Accumulated error messages.
   logging.basicConfig(format='%(message)s')
   messages = []
 
+  def ShouldRunTest(test_name):
+    return len(args.tests) == 0 or test_name in args.tests
+
   # Run API booster against test artifacts in a directory relative to workspace.
   # We use a temporary copy as the API booster does in-place rewriting.
   with tempfile.TemporaryDirectory(dir=pathlib.Path.cwd()) as path:
     # Setup temporary tree.
     shutil.copy(os.path.join(TESTDATA_PATH, 'BUILD'), path)
-    for filename, _ in TESTS:
-      shutil.copy(os.path.join(TESTDATA_PATH, filename), path)
+    for test in TESTS:
+      if ShouldRunTest(test.name):
+        shutil.copy(os.path.join(TESTDATA_PATH, test.name + '.cc'), path)
+      else:
+        # Place an empty file to make Bazel happy.
+        pathlib.Path(path, test.name + '.cc').write_text('')
 
     # Run API booster.
-    api_boost.ApiBoostTree([str(pathlib.Path(path).relative_to(pathlib.Path.cwd()))],
+    relpath_to_testdata = str(pathlib.Path(path).relative_to(pathlib.Path.cwd()))
+    api_boost.ApiBoostTree([relpath_to_testdata],
                            generate_compilation_database=True,
                            build_api_booster=True,
-                           debug_log=True)
+                           debug_log=True,
+                           sequential=True)
 
     # Validate output against golden files.
-    for filename, description in TESTS:
-      delta = Diff(os.path.join(TESTDATA_PATH, filename + '.gold'), os.path.join(path, filename))
-      if delta is not None:
-        messages.append('Non-empty diff for %s (%s):\n%s\n' % (filename, description, delta))
+    for test in TESTS:
+      if ShouldRunTest(test.name):
+        delta = Diff(os.path.join(TESTDATA_PATH, test.name + '.cc.gold'),
+                     os.path.join(path, test.name + '.cc'))
+        if delta is not None:
+          messages.append('Non-empty diff for %s (%s):\n%s\n' %
+                          (test.name, test.description, delta))
 
   if len(messages) > 0:
     logging.error('FAILED:\n{}'.format('\n'.join(messages)))

tools/api_boost/testdata/BUILD

@@ -8,8 +8,20 @@ load(
 
 envoy_package()
 
+envoy_cc_library(
+    name = "decl_ref_expr",
+    srcs = ["decl_ref_expr.cc"],
+    deps = ["@envoy_api//envoy/config/overload/v2alpha:pkg_cc_proto"],
+)
+
 envoy_cc_library(
     name = "elaborated_type",
     srcs = ["elaborated_type.cc"],
     deps = ["@envoy_api//envoy/config/overload/v2alpha:pkg_cc_proto"],
 )
+
+envoy_cc_library(
+    name = "using_decl",
+    srcs = ["using_decl.cc"],
+    deps = ["@envoy_api//envoy/config/overload/v2alpha:pkg_cc_proto"],
+)

tools/api_boost/testdata/decl_ref_expr.cc

@@ -0,0 +1,21 @@
+#include "envoy/config/overload/v2alpha/overload.pb.h"
+
+using envoy::config::overload::v2alpha::Trigger;
+
+class ThresholdTriggerImpl {
+public:
+  ThresholdTriggerImpl(const envoy::config::overload::v2alpha::Trigger& config) {
+    switch (config.trigger_oneof_case()) {
+    case envoy::config::overload::v2alpha::Trigger::kThreshold:
+      break;
+    default:
+      break;
+    }
+    switch (config.trigger_oneof_case()) {
+    case Trigger::kThreshold:
+      break;
+    default:
+      break;
+    }
+  }
+};

tools/api_boost/testdata/decl_ref_expr.cc.gold

@@ -0,0 +1,21 @@
+#include "envoy/config/overload/v3alpha/overload.pb.h"
+
+using envoy::config::overload::v3alpha::Trigger;
+
+class ThresholdTriggerImpl {
+public:
+  ThresholdTriggerImpl(const envoy::config::overload::v3alpha::Trigger& config) {
+    switch (config.trigger_oneof_case()) {
+    case envoy::config::overload::v3alpha::Trigger::kThreshold:
+      break;
+    default:
+      break;
+    }
+    switch (config.trigger_oneof_case()) {
+    case Trigger::kThreshold:
+      break;
+    default:
+      break;
+    }
+  }
+};

tools/api_boost/testdata/using_decl.cc

@@ -0,0 +1,10 @@
+#include "envoy/config/overload/v2alpha/overload.pb.h"
+
+using envoy::config::overload::v2alpha::ThresholdTrigger;
+using SomePtrAlias = std::unique_ptr<envoy::config::overload::v2alpha::ThresholdTrigger>;
+
+class ThresholdTriggerImpl {
+public:
+  ThresholdTriggerImpl(const ThresholdTrigger& /*config*/) {}
+  ThresholdTriggerImpl(SomePtrAlias /*config*/) {}
+};

tools/api_boost/testdata/using_decl.cc.gold

@@ -0,0 +1,10 @@
+#include "envoy/config/overload/v3alpha/overload.pb.h"
+
+using envoy::config::overload::v3alpha::ThresholdTrigger;
+using SomePtrAlias = std::unique_ptr<envoy::config::overload::v3alpha::ThresholdTrigger>;
+
+class ThresholdTriggerImpl {
+public:
+  ThresholdTriggerImpl(const ThresholdTrigger& /*config*/) {}
+  ThresholdTriggerImpl(SomePtrAlias /*config*/) {}
+};