Swift: address review comments

Author: redsun82

swift/extractor/infra/SwiftDispatcher.h

@@ -78,7 +78,7 @@ class SwiftDispatcher {
     waitingForNewLabel = e;
     visit(e);
     if (auto l = store.get(e)) {
-      if constexpr (std::is_base_of_v<LocatableTag, TrapTagOf<E>>) {
+      if constexpr (!std::is_base_of_v<swift::TypeBase, E>) {
         attachLocation(e, *l);
       }
       return *l;
@@ -285,8 +285,9 @@ class SwiftDispatcher {
     return realPath.str().str();
   }
 
-  // TODO: The following methods are supposed to redirect TRAP emission to correpsonding visitors,
-  // which are to be introduced in follow-up PRs
+  // TODO: for const correctness these should consistently be `const` (and maybe const references
+  // as we don't expect `nullptr` here. However `swift::ASTVisitor` and `swift::TypeVisitor` do not
+  // accept const pointers
   virtual void visit(swift::Decl* decl) = 0;
   virtual void visit(const swift::IfConfigClause* clause) = 0;
   virtual void visit(swift::Stmt* stmt) = 0;

swift/extractor/visitors/ExprVisitor.cpp

@@ -667,11 +667,11 @@ void ExprVisitor::emitLookupExpr(const swift::LookupExpr* expr, TrapLabel<Lookup
     dispatcher_.emit(LookupExprMembersTrap{label, declLabel});
   }
 }
+
 codeql::UnresolvedPatternExpr ExprVisitor::translateUnresolvedPatternExpr(
     swift::UnresolvedPatternExpr& expr) {
   auto entry = dispatcher_.createEntry(expr);
   entry.sub_pattern = dispatcher_.fetchLabel(expr.getSubPattern());
   return entry;
 }
-
 }  // namespace codeql

swift/extractor/visitors/ExprVisitor.h

@@ -84,10 +84,12 @@ class ExprVisitor : public AstVisitorBase<ExprVisitor> {
   codeql::BridgeFromObjCExpr translateBridgeFromObjCExpr(const swift::BridgeFromObjCExpr& expr);
   codeql::DotSelfExpr translateDotSelfExpr(const swift::DotSelfExpr& expr);
   codeql::ErrorExpr translateErrorExpr(const swift::ErrorExpr& expr);
-  // following requires non-const because:
-  // * `swift::UnresolvedPatternExpr::getSubPattern` gives `const swift::Pattern*` on const refs
+  // The following function requires a non-const parameter because:
+  // * `swift::UnresolvedPatternExpr::getSubPattern` has a `const`-qualified overload returning
+  //   `const swift::Pattern*`
   // * `swift::ASTVisitor` only visits non-const pointers
-  // either we accept this, or we fix constness by providing our own const visiting in VisitorBase
+  // either we accept this, or we fix constness, e.g. by providing `visit` on `const` pointers
+  // in `VisitorBase`, or by doing a `const_cast` in `SwifDispatcher::fetchLabel`
   codeql::UnresolvedPatternExpr translateUnresolvedPatternExpr(swift::UnresolvedPatternExpr& expr);
 
  private:

swift/extractor/visitors/TypeVisitor.cpp

@@ -1,4 +1,5 @@
 #include "swift/extractor/visitors/TypeVisitor.h"
+
 namespace codeql {
 void TypeVisitor::visit(swift::TypeBase* type) {
   TypeVisitorBase<TypeVisitor>::visit(type);
@@ -367,6 +368,7 @@ codeql::BuiltinVectorType TypeVisitor::translateBuiltinVectorType(
     const swift::BuiltinVectorType& type) {
   return createTypeEntry(type);
 }
+
 codeql::OpenedArchetypeType TypeVisitor::translateOpenedArchetypeType(
     const swift::OpenedArchetypeType& type) {
   auto entry = createTypeEntry(type);