Swift: implement review suggestions

Author: redsun82

swift/extractor/infra/SwiftLocationExtractor.cpp

@@ -25,8 +25,8 @@ void SwiftLocationExtractor::attachLocationImpl(const swift::SourceManager& sour
   entry.file = fetchFileLabel(file);
   std::tie(entry.start_line, entry.start_column) = sourceManager.getLineAndColumnInBuffer(start);
   std::tie(entry.end_line, entry.end_column) = sourceManager.getLineAndColumnInBuffer(end);
-  SwiftMangledName locName{{"loc", entry.file, ":", entry.start_line, ":", entry.start_column, ":",
-                            entry.end_line, ":", entry.end_column}};
+  SwiftMangledName locName{"loc", entry.file,     ':', entry.start_line, ':', entry.start_column,
+                           ':',   entry.end_line, ':', entry.end_column};
   entry.id = trap.createTypedLabel<DbLocationTag>(locName);
   trap.emit(entry);
   trap.emit(LocatableLocationsTrap{locatableLabel, entry.id});
@@ -99,7 +99,7 @@ TrapLabel<FileTag> SwiftLocationExtractor::fetchFileLabel(const std::filesystem:
   }
 
   DbFile entry({});
-  entry.id = trap.createTypedLabel<DbFileTag>({{"file_", file.string()}});
+  entry.id = trap.createTypedLabel<DbFileTag>({"file_", file.string()});
   entry.name = file.string();
   trap.emit(entry);
   store[file] = entry.id;

swift/extractor/infra/SwiftMangledName.cpp

@@ -3,52 +3,13 @@
 
 namespace codeql {
 
-namespace {
-void appendPart(std::string& out, const std::string& prefix) {
-  out += prefix;
-}
-
-void appendPart(std::string& out, UntypedTrapLabel label) {
-  absl::StrAppend(&out, "{", label.str(), "}");
-}
-
-void appendPart(std::string& out, unsigned index) {
-  absl::StrAppend(&out, index);
-}
-
-}  // namespace
-
-std::string SwiftMangledName::str() const {
-  std::string out;
-  for (const auto& part : parts) {
-    std::visit([&](const auto& contents) { appendPart(out, contents); }, part);
-  }
-  return out;
-}
-
 SwiftMangledName& SwiftMangledName::operator<<(UntypedTrapLabel label) & {
-  assert(label && "using undefined label in mangled name");
-  parts.emplace_back(label);
+  absl::StrAppend(&value, "{", label.str(), "}");
   return *this;
 }
 
 SwiftMangledName& SwiftMangledName::operator<<(unsigned int i) & {
-  parts.emplace_back(i);
-  return *this;
-}
-
-SwiftMangledName& SwiftMangledName::operator<<(SwiftMangledName&& other) & {
-  parts.reserve(parts.size() + other.parts.size());
-  for (auto& p : other.parts) {
-    parts.emplace_back(std::move(p));
-  }
-  other.parts.clear();
-  return *this;
-}
-
-SwiftMangledName& SwiftMangledName::operator<<(const SwiftMangledName& other) & {
-  parts.reserve(parts.size() + other.parts.size());
-  parts.insert(parts.end(), other.parts.begin(), other.parts.end());
+  absl::StrAppend(&value, i);
   return *this;
 }
 

swift/extractor/infra/SwiftMangledName.h

@@ -9,14 +9,13 @@
 
 namespace codeql {
 
-struct SwiftMangledName {
+class SwiftMangledName {
+ public:
   using Part = std::variant<UntypedTrapLabel, std::string, unsigned>;
 
-  std::vector<Part> parts;
+  explicit operator bool() const { return !value.empty(); }
 
-  explicit operator bool() const { return !parts.empty(); }
-
-  std::string str() const;
+  const std::string& str() const { return value; }
 
   // let's avoid copying as long as we don't need it
   SwiftMangledName() = default;
@@ -25,28 +24,40 @@ struct SwiftMangledName {
   SwiftMangledName(SwiftMangledName&&) = default;
   SwiftMangledName& operator=(SwiftMangledName&&) = default;
 
+  template <typename... Args>
+  SwiftMangledName(Args&&... args) {
+    (operator<<(std::forward<Args>(args)), ...);
+  }
+
   // streaming labels or ints into a SwiftMangledName just appends them
   SwiftMangledName& operator<<(UntypedTrapLabel label) &;
   SwiftMangledName& operator<<(unsigned i) &;
 
+  template <typename Tag>
+  SwiftMangledName& operator<<(TrapLabel<Tag> label) & {
+    return operator<<(static_cast<UntypedTrapLabel>(label));
+  }
+
   // streaming string-like stuff will add a new part it only if strictly required, otherwise it will
   // append to the last part if it is a string
   template <typename T>
   SwiftMangledName& operator<<(T&& arg) & {
-    if (parts.empty() || !std::holds_alternative<std::string>(parts.back())) {
-      parts.emplace_back("");
-    }
-    std::get<std::string>(parts.back()) += std::forward<T>(arg);
+    value += arg;
     return *this;
   }
 
-  SwiftMangledName& operator<<(SwiftMangledName&& other) &;
-  SwiftMangledName& operator<<(const SwiftMangledName& other) &;
+  SwiftMangledName& operator<<(const SwiftMangledName& other) {
+    value += other.value;
+    return *this;
+  }
 
   template <typename T>
   SwiftMangledName&& operator<<(T&& arg) && {
     return std::move(operator<<(std::forward<T>(arg)));
   }
+
+ private:
+  std::string value;
 };
 
 }  // namespace codeql

swift/extractor/mangler/SwiftMangler.cpp

@@ -40,20 +40,22 @@ std::string_view getTypeKindStr(const swift::TypeBase* type) {
 
 template <typename E>
 UntypedTrapLabel SwiftMangler::fetch(E&& e) {
-  return dispatcher.fetchLabel(std::forward<E>(e));
+  auto ret = dispatcher.fetchLabel(std::forward<E>(e));
+  // TODO use a generic logging handle for Swift entities here, once it's available
+  CODEQL_ASSERT(ret.valid(), "using an undefined label in mangling");
+  return ret;
 }
 
 SwiftMangledName SwiftMangler::initMangled(const swift::TypeBase* type) {
-  return SwiftMangledName() << getTypeKindStr(type) << '_';
+  return {getTypeKindStr(type), '_'};
 }
 
 SwiftMangledName SwiftMangler::initMangled(const swift::Decl* decl) {
-  return SwiftMangledName() << swift::Decl::getKindName(decl->getKind()) << "Decl_"
-                            << fetch(getParent(decl));
+  return {swift::Decl::getKindName(decl->getKind()), "Decl_", fetch(getParent(decl))};
 }
 
 SwiftMangledName SwiftMangler::mangleModuleName(std::string_view name) {
-  return SwiftMangledName() << "ModuleDecl_" << name;
+  return {"ModuleDecl_", name};
 }
 
 SwiftMangledName SwiftMangler::visitModuleDecl(const swift::ModuleDecl* decl) {
@@ -124,10 +126,12 @@ unsigned SwiftMangler::getExtensionIndex(const swift::ExtensionDecl* decl,
   } else if (auto iterableParent = llvm::dyn_cast<swift::IterableDeclContext>(parent)) {
     indexExtensions(iterableParent->getAllMembers());
   } else {
-    assert(false && "non-local context must be module or iterable decl context");
+    // TODO use a generic logging handle for Swift entities here, once it's available
+    CODEQL_ASSERT(false, "non-local context must be module or iterable decl context");
   }
   auto found = preloadedExtensionIndexes.extract(decl);
-  assert(found && "extension not found within parent");
+  // TODO use a generic logging handle for Swift entities here, once it's available
+  CODEQL_ASSERT(found, "extension not found within parent");
   return found.mapped();
 }
 
@@ -141,32 +145,17 @@ void SwiftMangler::indexExtensions(llvm::ArrayRef<swift::Decl*> siblings) {
   }
 }
 
-SwiftMangledName SwiftMangler::visitGenericTypeDecl(const swift::GenericTypeDecl* decl) {
-  auto ret = visitValueDecl(decl);
-  if (!ret) {
-    return {};
-  }
-  if (auto genericParams = decl->getParsedGenericParams()) {
-    ret << '<' << genericParams->size() << '>';
-  }
-  return ret;
-}
-
 SwiftMangledName SwiftMangler::visitAbstractTypeParamDecl(
     const swift::AbstractTypeParamDecl* decl) {
   return visitValueDecl(decl, /* force */ true);
 }
 
 SwiftMangledName SwiftMangler::visitGenericTypeParamDecl(const swift::GenericTypeParamDecl* decl) {
-  auto ret = visitAbstractTypeParamDecl(decl);
-  ret << '_' << decl->getDepth() << '_' << decl->getIndex();
-  return ret;
+  return visitAbstractTypeParamDecl(decl) << '_' << decl->getDepth() << '_' << decl->getIndex();
 }
 
 SwiftMangledName SwiftMangler::visitModuleType(const swift::ModuleType* type) {
-  auto ret = initMangled(type);
-  ret << fetch(type->getModule());
-  return ret;
+  return initMangled(type) << fetch(type->getModule());
 }
 
 SwiftMangledName SwiftMangler::visitTupleType(const swift::TupleType* type) {

swift/extractor/mangler/SwiftMangler.h

@@ -63,7 +63,6 @@ class SwiftMangler : private swift::TypeVisitor<SwiftMangler, SwiftMangledName>,
   SwiftMangledName visitAbstractFunctionDecl(const swift::AbstractFunctionDecl* decl);
   SwiftMangledName visitSubscriptDecl(const swift::SubscriptDecl* decl);
   SwiftMangledName visitVarDecl(const swift::VarDecl* decl);
-  SwiftMangledName visitGenericTypeDecl(const swift::GenericTypeDecl* decl);
   SwiftMangledName visitAbstractTypeParamDecl(const swift::AbstractTypeParamDecl* decl);
   SwiftMangledName visitGenericTypeParamDecl(const swift::GenericTypeParamDecl* decl);