[Reproducers] Tool to insert SBReproducer macros

[JDevlieghere]

Summary

Simple tool to automate/simplify the workflow of inserting SB_RECORD and SB_REGSITER macros.

Because the tool won't be part of the build process I don't wan't to over-complicate it. Currently it prints a list of SB_REGISTER macros followed by the source file with the SB_RECORD macro's inserted. This means you'd have to copy over the SB_REGISTER macros manually per source file.

See example output here: https://reviews.llvm.org/P8125

[JDevlieghere]

• Update files in place.
• Check for existing macros.

[JDevlieghere]

• Skip unsupported signatures (function pointers and void*)

[JDevlieghere]

Fixed printing of booleans; the tool would show _Bool instead of bool.

I ran this on a few files and it appears to work with the prototype.

[labath]

Is this ready for review now?

tools/sbrepro/SBRepro.cpp

99

MacroMacro?

[JDevlieghere]

Fix MacroMacro and other code cleanups.

[labath]

tools/sbrepro/SBRepro.cpp

23–32

There's a join in ADT/StringExtras.h which should do what you need here.

195–198

What exactly does this skip? Would e.g. a constructor with an empty body still be considered for macro-ization?

[JDevlieghere]

• Use join from StringExtras.
• Rename Skip to ShoupdSkip and document its cases.

[labath]

So, if I understand this correctly, a constructor like SBFoo::SBFoo(){} will not be considered for macroization because it's empty? That doesn't sound right to me. Even an empty constructor has a side effect of constructing the object, which is something that I would expect to see recorded in the trace file. Is there some reason why it is hard/impossible to insert macros into these kinds of functions? If so, it may be worth just warning the user about their existence.

A function with an empty body probably doesn't do anything interesting, so it may be possible to skip recording those, but I think it would make sense to record them nonetheless.

[JDevlieghere]

In D56822#1376770, @labath wrote:

So, if I understand this correctly, a constructor like SBFoo::SBFoo(){} will not be considered for macroization because it's empty? That doesn't sound right to me. Even an empty constructor has a side effect of constructing the object, which is something that I would expect to see recorded in the trace file. Is there some reason why it is hard/impossible to insert macros into these kinds of functions? If so, it may be worth just warning the user about their existence.

A function with an empty body probably doesn't do anything interesting, so it may be possible to skip recording those, but I think it would make sense to record them nonetheless.

Ugh, I was confusing empty bodies with methods without a body (e.g. declarations). Empty bodies are definitely handled! I'll update the comment.

[labath]

ok, looks good then

[JDevlieghere]

• Skip decls that have a canonical decl in the implementation file.

[JDevlieghere]

It doesn't really make sense to land this without the SBReproducer framework, so I'll leave it open until that gets accepted as well.

[JDevlieghere]

• Use new LLDB_ prefix
• Insert two newlines at the end of the macro, so clang-format fixes it up consistently.

tools/CMakeLists.txt

 add_subdirectory(argdumper)
 add_subdirectory(driver)
 add_subdirectory(intel-features)
 add_subdirectory(lldb-mi)
 add_subdirectory(lldb-test)
 add_subdirectory(lldb-vscode)
+add_subdirectory(sbrepro)
 
 if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
   add_subdirectory(darwin-debug)
   add_subdirectory(debugserver)
 endif()
 
 if (LLDB_CAN_USE_LLDB_SERVER AND NOT SKIP_LLDB_SERVER_BUILD)
   add_subdirectory(lldb-server)
 endif()

tools/sbrepro/CMakeLists.txt

+add_lldb_tool(sbrepro
+  SBRepro.cpp
+
+  LINK_LIBS
+    clangAST
+    clangBasic
+    clangFrontend
+    clangTooling
+
+  LINK_COMPONENTS
+    Support
+  )

tools/sbrepro/SBRepro.cpp

+#include "clang/AST/AST.h"
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/ASTConsumers.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Tooling.h"
+
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/raw_ostream.h"
+
+#include <sstream>
+#include <string>
+
+using namespace clang;
+using namespace clang::driver;
+using namespace clang::tooling;
+
+static llvm::cl::OptionCategory ReproCategory("SB Reproducer");
+
+/// Get the macro name for recording method calls.
+///
+/// SB_RECORD_METHOD
+/// SB_RECORD_METHOD_CONST
+/// SB_RECORD_METHOD_NO_ARGS
+/// SB_RECORD_METHOD_CONST_NO_ARGS
+/// SB_RECORD_STATIC_METHOD
+/// SB_RECORD_STATIC_METHOD_NO_ARGS
+static std::string GetRecordMethodMacroName(bool Static, bool Const,
+                                            bool NoArgs) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+
+  OS << "SB_RECORD";
+  if (Static)
+    OS << "_STATIC";
+  OS << "_METHOD";
+  if (Const)
+    OS << "_CONST";
+  if (NoArgs)
+    OS << "_NO_ARGS";
+
+  return OS.str();
+}
+
+/// Get the macro name for register methods.
+///
+/// SB_REGISTER_CONSTRUCTOR
+/// SB_REGISTER_METHOD
+/// SB_REGISTER_METHOD_CONST
+/// SB_REGISTER_STATIC_METHOD
+static std::string GetRegisterMethodMacroName(bool Static, bool Const) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+
+  OS << "SB_REGISTER";
+  if (Static)
+    OS << "_STATIC";
+  OS << "_METHOD";
+  if (Const)
+    OS << "_CONST";
+
+  return OS.str();
+}
+
+static std::string GetRecordMethodMacro(StringRef Result, StringRef Class,
+                                        StringRef Method, StringRef Signature,
+                                        StringRef Values, bool Static,
+                                        bool Const) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+
+  OS << GetRecordMethodMacroName(Static, Const, Values.empty());
+  OS << "(" << Result << ", " << Class << ", " << Method;
+
+  if (!Values.empty()) {
+    OS << ", (" << Signature << "), " << Values << ");\n";
+  } else {
+    OS << ");\n";
+  }
+
+  return OS.str();
+}
+
+static std::string GetRecordConstructorMacro(StringRef Class,
+                                             StringRef Signature,
+                                             StringRef Values) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+  if (!Values.empty()) {
+    OS << "SB_RECORD_CONSTRUCTOR(" << Class << ", (" << Signature << "), "
+       << Values << ");\n";
+  } else {
+    OS << "SB_RECORD_CONSTRUCTOR_NO_ARGS(" << Class << ");\n";
+  }
+  return OS.str();
+}
+
+static std::string GetRegisterConstructorMacro(StringRef Class,
+                                               StringRef Signature) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+  OS << "SB_REGISTER_CONSTRUCTOR(" << Class << ", (" << Signature << "));\n";
+  return OS.str();
+}
+
+static std::string GetRegisterMethodMacro(StringRef Result, StringRef Class,
+                                          StringRef Method, StringRef Signature,
+                                          bool Static, bool Const) {
+  std::string Macro;
+  llvm::raw_string_ostream OS(Macro);
+  OS << GetRegisterMethodMacroName(Static, Const);
+  OS << "(" << Result << ", " << Class << ", " << Method << ", (" << Signature
+     << "));\n";
+  return OS.str();
+}
+
+class SBVisitor : public RecursiveASTVisitor<SBVisitor> {
+public:
+  SBVisitor(Rewriter &R, ASTContext &Context)
+      : MyRewriter(R), Context(Context) {}
+
+  bool VisitCXXMethodDecl(CXXMethodDecl *Decl) {
+    // Not all decls should be registered for the reproducer. Please refer to
+    // that method's comment for details.
+    if (ShouldSkip(Decl))
+      return false;
+
+    // Print 'bool' instead of '_Bool'.
+    PrintingPolicy Policy(Context.getLangOpts());
+    Policy.Bool = true;
+
+    // Collect the functions parameter types and names.
+    std::vector<std::string> ParamTypes;
+    std::vector<std::string> ParamNames;
+    for (auto *P : Decl->parameters()) {
+      QualType T = P->getType();
+
+      // Currently we don't support functions that have function pointers as an
+      // argument.
+      if (T->isFunctionPointerType())
+        return false;
+
+      // Currently we don't support functions that have void pointers as an
+      // argument.
+      if (T->isVoidPointerType())
+        return false;
+
+      ParamTypes.push_back(T.getAsString(Policy));
+      ParamNames.push_back(P->getNameAsString());
+    }
+
+    // Convert the two lists to string for the macros.
+    std::string ParamTypesStr = llvm::join(ParamTypes, ", ");
+    std::string ParamNamesStr = llvm::join(ParamNames, ", ");
+
+    CXXRecordDecl *Record = Decl->getParent();
+    QualType ReturnType = Decl->getReturnType();
+
+    // Construct the macros.
+    std::string Macro;
+    if (isa<CXXConstructorDecl>(Decl)) {
+      llvm::outs() << GetRegisterConstructorMacro(Record->getNameAsString(),
+                                                  ParamTypesStr);
+
+      Macro = GetRecordConstructorMacro(Record->getNameAsString(),
+                                        ParamTypesStr, ParamNamesStr);
+    } else {
+      llvm::outs() << GetRegisterMethodMacro(
+          ReturnType.getAsString(Policy), Record->getNameAsString(),
+          Decl->getNameAsString(), ParamTypesStr, Decl->isStatic(),
+          Decl->isConst());
+
+      Macro = GetRecordMethodMacro(
+          ReturnType.getAsString(Policy), Record->getNameAsString(),
+          Decl->getNameAsString(), ParamTypesStr, ParamNamesStr,
+          Decl->isStatic(), Decl->isConst());
+    }
+
+    // If this CXXMethodDecl already starts with a macro we're done.
+    Stmt *Body = Decl->getBody();
+    for (auto &C : Body->children()) {
+      if (C->getBeginLoc().isMacroID())
+        return false;
+      break;
+    }
+
+    // Insert the macro at the beginning of the function.
+    SourceLocation InsertLoc = Lexer::getLocForEndOfToken(
+        Body->getBeginLoc(), 0, MyRewriter.getSourceMgr(),
+        MyRewriter.getLangOpts());
+    MyRewriter.InsertTextAfter(InsertLoc, Macro);
+
+    return true;
+  }
+
+private:
+  /// Determine whether we need to consider the given CXXMethodDecl.
+  ///
+  /// Currently we skip the following cases:
+  ///  1. Decls outside the main source file,
+  ///  2. Decls that are only present in the source file,
+  ///  3. Decls that are not definitions,
+  ///  4. Non-public decls,
+  ///  5. Destructors.
+  bool ShouldSkip(CXXMethodDecl *Decl) {
+    // Skip anything outside the main file.
+    if (!MyRewriter.getSourceMgr().isInMainFile(Decl->getBeginLoc()))
+      return true;
+
+    // Skip if the canonical decl in the current decl. It means that the method
+    // is declared in the implementation and is therefore not exposed as part
+    // of the API.
+    if (Decl == Decl->getCanonicalDecl())
+      return true;
+
+    // Skip decls that have no body, i.e. are just declarations.
+    Stmt *Body = Decl->getBody();
+    if (!Body)
+      return true;
+
+    // Skip non-public decls.
+    AccessSpecifier AS = Decl->getAccess();
+    if (AS != AccessSpecifier::AS_public)
+      return true;
+
+    // Skip destructors.
+    if (isa<CXXDestructorDecl>(Decl))
+      return true;
+
+    return false;
+  }
+
+  Rewriter &MyRewriter;
+  ASTContext &Context;
+};
+
+class SBConsumer : public ASTConsumer {
+public:
+  SBConsumer(Rewriter &R, ASTContext &Context) : Visitor(R, Context) {}
+
+  // Override the method that gets called for each parsed top-level
+  // declaration.
+  bool HandleTopLevelDecl(DeclGroupRef DR) override {
+    for (DeclGroupRef::iterator b = DR.begin(), e = DR.end(); b != e; ++b) {
+      Visitor.TraverseDecl(*b);
+    }
+    return true;
+  }
+
+private:
+  SBVisitor Visitor;
+};
+
+class SBAction : public ASTFrontendAction {
+public:
+  SBAction() = default;
+
+  void EndSourceFileAction() override { MyRewriter.overwriteChangedFiles(); }
+
+  std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI,
+                                                 StringRef file) override {
+    MyRewriter.setSourceMgr(CI.getSourceManager(), CI.getLangOpts());
+    return llvm::make_unique<SBConsumer>(MyRewriter, CI.getASTContext());
+  }
+
+private:
+  Rewriter MyRewriter;
+};
+
+/// Utility to generate the macros for the SBReproducer framework.
+///
+/// It inserts the SB_RECORD macro in place for the given input file, and print
+/// the SB_REGISTER macros to stdout.
+int main(int argc, const char **argv) {
+  CommonOptionsParser OP(argc, argv, ReproCategory);
+  ClangTool T(OP.getCompilations(), OP.getSourcePathList());
+  return T.run(newFrontendActionFactory<SBAction>().get());
+}