Swift: replace or remove assertions in translators

redsun82 · redsun82 · commit 61bb6c912a4a · 2023-04-18T12:16:22.000+02:00
Assertions before fetching a non optional label are not needed as
the dispatcher will replace those with unspecified elements (and
properly log those instances).
diff --git a/swift/extractor/translators/DeclTranslator.cpp b/swift/extractor/translators/DeclTranslator.cpp
@@ -97,7 +97,6 @@ std::optional<codeql::ParamDecl> DeclTranslator::translateParamDecl(const swift:
 codeql::TopLevelCodeDecl DeclTranslator::translateTopLevelCodeDecl(
     const swift::TopLevelCodeDecl& decl) {
   auto entry = createEntry(decl);
-  assert(decl.getBody() && "Expect top level code to have body");
   entry.body = dispatcher.fetchLabel(decl.getBody());
   return entry;
 }
@@ -107,7 +106,6 @@ codeql::PatternBindingDecl DeclTranslator::translatePatternBindingDecl(
   auto entry = createEntry(decl);
   for (unsigned i = 0; i < decl.getNumPatternEntries(); ++i) {
     auto pattern = decl.getPattern(i);
-    assert(pattern && "Expect pattern binding decl to have all patterns");
     entry.patterns.push_back(dispatcher.fetchLabel(pattern));
     entry.inits.push_back(dispatcher.fetchOptionalLabel(decl.getInit(i)));
   }
@@ -309,9 +307,10 @@ std::string DeclTranslator::mangledName(const swift::ValueDecl& decl) {
 
 void DeclTranslator::fillAbstractFunctionDecl(const swift::AbstractFunctionDecl& decl,
                                               codeql::AbstractFunctionDecl& entry) {
-  assert(decl.hasParameterList() && "Expect functions to have a parameter list");
   entry.name = !decl.hasName() ? "(unnamed function decl)" : constructName(decl.getName());
   entry.body = dispatcher.fetchOptionalLabel(decl.getBody());
+  CODEQL_EXPECT_OR(return, decl.hasParameterList(), "Function {} has no parameter list",
+                         entry.name);
   entry.params = dispatcher.fetchRepeatedLabels(*decl.getParameters());
   auto self = const_cast<swift::ParamDecl* const>(decl.getImplicitSelfDecl());
   entry.self_param = dispatcher.fetchOptionalLabel(self);
@@ -378,7 +377,6 @@ void DeclTranslator::fillGenericContext(const swift::GenericContext& decl,
 }
 
 void DeclTranslator::fillValueDecl(const swift::ValueDecl& decl, codeql::ValueDecl& entry) {
-  assert(decl.getInterfaceType() && "Expect ValueDecl to have InterfaceType");
   entry.interface_type = dispatcher.fetchLabel(decl.getInterfaceType());
 }
 
diff --git a/swift/extractor/translators/DeclTranslator.h b/swift/extractor/translators/DeclTranslator.h
@@ -15,6 +15,8 @@ namespace codeql {
 // "forward declarations" while our extraction is incomplete
 class DeclTranslator : public AstTranslatorBase<DeclTranslator> {
  public:
+  static constexpr std::string_view name = "decl";
+
   using AstTranslatorBase<DeclTranslator>::AstTranslatorBase;
 
   std::optional<codeql::ConcreteFuncDecl> translateFuncDecl(const swift::FuncDecl& decl);
diff --git a/swift/extractor/translators/ExprTranslator.cpp b/swift/extractor/translators/ExprTranslator.cpp
@@ -355,11 +355,11 @@ codeql::IsExpr ExprTranslator::translateIsExpr(const swift::IsExpr& expr) {
 codeql::SubscriptExpr ExprTranslator::translateSubscriptExpr(const swift::SubscriptExpr& expr) {
   auto entry = createExprEntry(expr);
   fillAccessorSemantics(expr, entry);
-  assert(expr.getArgs() && "SubscriptExpr has getArgs");
+  fillLookupExpr(expr, entry);
+  CODEQL_EXPECT_OR(return entry, expr.getArgs(), "SubscriptExpr has null getArgs");
   for (const auto& arg : *expr.getArgs()) {
     entry.arguments.push_back(emitArgument(arg));
   }
-  fillLookupExpr(expr, entry);
   return entry;
 }
 
@@ -384,9 +384,10 @@ codeql::KeyPathExpr ExprTranslator::translateKeyPathExpr(const swift::KeyPathExp
     }
     if (auto rootTypeRepr = expr.getRootType()) {
       auto keyPathType = expr.getType()->getAs<swift::BoundGenericClassType>();
-      assert(keyPathType && "KeyPathExpr must have BoundGenericClassType");
+      CODEQL_EXPECT_OR(return entry, keyPathType, "KeyPathExpr must have BoundGenericClassType");
       auto keyPathTypeArgs = keyPathType->getGenericArgs();
-      assert(keyPathTypeArgs.size() != 0 && "KeyPathExpr type must have generic args");
+      CODEQL_EXPECT_OR(return entry, keyPathTypeArgs.size() != 0,
+                              "KeyPathExpr type must have generic args");
       entry.root = dispatcher.fetchLabel(rootTypeRepr, keyPathTypeArgs[0]);
     }
   }
@@ -474,10 +475,10 @@ codeql::ErrorExpr ExprTranslator::translateErrorExpr(const swift::ErrorExpr& exp
 
 void ExprTranslator::fillAbstractClosureExpr(const swift::AbstractClosureExpr& expr,
                                              codeql::AbstractClosureExpr& entry) {
-  assert(expr.getParameters() && "AbstractClosureExpr has getParameters()");
-  entry.params = dispatcher.fetchRepeatedLabels(*expr.getParameters());
   entry.body = dispatcher.fetchLabel(expr.getBody());
   entry.captures = dispatcher.fetchRepeatedLabels(expr.getCaptureInfo().getCaptures());
+  CODEQL_EXPECT_OR(return, expr.getParameters(), "AbstractClosureExpr has null getParameters()");
+  entry.params = dispatcher.fetchRepeatedLabels(*expr.getParameters());
 }
 
 TrapLabel<ArgumentTag> ExprTranslator::emitArgument(const swift::Argument& arg) {
@@ -524,7 +525,7 @@ void ExprTranslator::fillAnyTryExpr(const swift::AnyTryExpr& expr, codeql::AnyTr
 
 void ExprTranslator::fillApplyExpr(const swift::ApplyExpr& expr, codeql::ApplyExpr& entry) {
   entry.function = dispatcher.fetchLabel(expr.getFn());
-  assert(expr.getArgs() && "ApplyExpr has getArgs");
+  CODEQL_EXPECT_OR(return, expr.getArgs(), "ApplyExpr has null getArgs");
   for (const auto& arg : *expr.getArgs()) {
     entry.arguments.push_back(emitArgument(arg));
   }
diff --git a/swift/extractor/translators/ExprTranslator.h b/swift/extractor/translators/ExprTranslator.h
@@ -7,6 +7,8 @@ namespace codeql {
 
 class ExprTranslator : public AstTranslatorBase<ExprTranslator> {
  public:
+  static constexpr std::string_view name = "expr";
+
   using AstTranslatorBase<ExprTranslator>::AstTranslatorBase;
 
   codeql::IntegerLiteralExpr translateIntegerLiteralExpr(const swift::IntegerLiteralExpr& expr);
diff --git a/swift/extractor/translators/PatternTranslator.h b/swift/extractor/translators/PatternTranslator.h
@@ -7,6 +7,8 @@ namespace codeql {
 
 class PatternTranslator : public AstTranslatorBase<PatternTranslator> {
  public:
+  static constexpr std::string_view name = "pattern";
+
   using AstTranslatorBase<PatternTranslator>::AstTranslatorBase;
 
   codeql::NamedPattern translateNamedPattern(const swift::NamedPattern& pattern);
diff --git a/swift/extractor/translators/StmtTranslator.h b/swift/extractor/translators/StmtTranslator.h
@@ -7,6 +7,8 @@ namespace codeql {
 
 class StmtTranslator : public AstTranslatorBase<StmtTranslator> {
  public:
+  static constexpr std::string_view name = "stmt";
+
   using AstTranslatorBase<StmtTranslator>::AstTranslatorBase;
   using AstTranslatorBase<StmtTranslator>::translateAndEmit;
 
diff --git a/swift/extractor/translators/TranslatorBase.h b/swift/extractor/translators/TranslatorBase.h
@@ -11,10 +11,11 @@ namespace detail {
 class TranslatorBase {
  protected:
   SwiftDispatcher& dispatcher;
+  Logger logger;
 
- public:
   // SwiftDispatcher should outlive this instance
-  TranslatorBase(SwiftDispatcher& dispatcher) : dispatcher{dispatcher} {}
+  TranslatorBase(SwiftDispatcher& dispatcher, std::string_view name)
+      : dispatcher{dispatcher}, logger{"translator/" + std::string(name)} {}
 };
 
 // define by macro metaprogramming member checkers
@@ -90,7 +91,7 @@ enum class TranslatorPolicy {
   void visit##CLASS##KIND(swift::CLASS##KIND* e) {                                   \
     constexpr auto policy = getPolicyFor##CLASS##KIND();                             \
     if constexpr (policy == TranslatorPolicy::ignore) {                              \
-      std::cerr << "Unexpected " #CLASS #KIND "\n";                                  \
+      LOG_ERROR("Unexpected " #CLASS #KIND);                                         \
       return;                                                                        \
     } else if constexpr (policy == TranslatorPolicy::translate) {                    \
       dispatcher.emit(static_cast<CrtpSubclass*>(this)->translate##CLASS##KIND(*e)); \
@@ -108,7 +109,7 @@ template <typename CrtpSubclass>
 class AstTranslatorBase : private swift::ASTVisitor<CrtpSubclass>,
                           protected detail::TranslatorBase {
  public:
-  using TranslatorBase::TranslatorBase;
+  AstTranslatorBase(SwiftDispatcher& dispatcher) : TranslatorBase(dispatcher, CrtpSubclass::name) {}
 
   // swift does not provide const visitors. The following const_cast is safe, as we privately
   // route the visit to translateXXX functions only if they take const references to swift
@@ -145,7 +146,8 @@ template <typename CrtpSubclass>
 class TypeTranslatorBase : private swift::TypeVisitor<CrtpSubclass>,
                            protected detail::TranslatorBase {
  public:
-  using TranslatorBase::TranslatorBase;
+  TypeTranslatorBase(SwiftDispatcher& dispatcher)
+      : TranslatorBase(dispatcher, CrtpSubclass::name) {}
 
   // swift does not provide const visitors. The following const_cast is safe, as we privately
   // route the visit to translateXXX functions only if they take const references to swift
diff --git a/swift/extractor/translators/TypeTranslator.h b/swift/extractor/translators/TypeTranslator.h
@@ -7,6 +7,8 @@
 namespace codeql {
 class TypeTranslator : public TypeTranslatorBase<TypeTranslator> {
  public:
+  static constexpr std::string_view name = "type";
+
   using TypeTranslatorBase<TypeTranslator>::TypeTranslatorBase;
   using TypeTranslatorBase<TypeTranslator>::translateAndEmit;
 
