Address Feedback Nr.6

Author: Alex B

llvm/include/llvm/DebugInfo/GSYM/CallSiteInfo.h

@@ -86,7 +86,8 @@ class CallSiteInfoLoader {
   /// structures.
   ///
   /// \param GCreator A reference to the GsymCreator.
-  CallSiteInfoLoader(GsymCreator &GCreator) : GCreator(GCreator) {}
+  CallSiteInfoLoader(GsymCreator &GCreator, std::vector<FunctionInfo> &Funcs)
+      : GCreator(GCreator), Funcs(Funcs) {}
 
   /// This method reads the specified YAML file, parses its content, and updates
   /// the `Funcs` vector with call site information based on the YAML data.
@@ -97,7 +98,7 @@ class CallSiteInfoLoader {
   /// file to be loaded.
   /// \returns An `llvm::Error` indicating success or describing any issues
   /// encountered during the loading process.
-  llvm::Error loadYAML(std::vector<FunctionInfo> &Funcs, StringRef YAMLFile);
+  llvm::Error loadYAML(StringRef YAMLFile);
 
 private:
   /// Builds a map from function names to FunctionInfo pointers based on the
@@ -106,7 +107,7 @@ class CallSiteInfoLoader {
   /// \param Funcs A reference to a vector of FunctionInfo objects.
   /// \returns A StringMap mapping function names (StringRef) to their
   /// corresponding FunctionInfo pointers.
-  StringMap<FunctionInfo *> buildFunctionMap(std::vector<FunctionInfo> &Funcs);
+  StringMap<FunctionInfo *> buildFunctionMap();
 
   /// Processes the parsed YAML functions and updates the `FuncMap` accordingly.
   ///
@@ -121,6 +122,9 @@ class CallSiteInfoLoader {
 
   /// Reference to the parent Gsym Creator object.
   GsymCreator &GCreator;
+
+  /// Reference to the vector of FunctionInfo objects to be populated.
+  std::vector<FunctionInfo> &Funcs;
 };
 
 raw_ostream &operator<<(raw_ostream &OS, const CallSiteInfo &CSI);

llvm/include/llvm/DebugInfo/GSYM/GsymCreator.h

@@ -329,14 +329,15 @@ class GsymCreator {
   /// \returns The unique 32 bit offset into the string table.
   uint32_t insertString(StringRef S, bool Copy = true);
 
-  /// Retrieve a string fromt he GSYM string table given its offset.
+  /// Retrieve a string from the GSYM string table given its offset.
   ///
   /// The offset is assumed to be a valid offset into the string table.
   /// otherwise an assert will be triggered.
   ///
-  /// \param offset The offset of the string to retrieve, previously returned by
-  /// insertString. \returns The string at the given offset in the string table.
-  StringRef getString(uint32_t offset);
+  /// \param Offset The offset of the string to retrieve, previously returned by
+  /// insertString.
+  /// \returns The string at the given offset in the string table.
+  StringRef getString(uint32_t Offset);
 
   /// Insert a file into this GSYM creator.
   ///

llvm/lib/DebugInfo/GSYM/CallSiteInfo.cpp

@@ -149,8 +149,7 @@ template <> struct MappingTraits<FunctionsYAML> {
 LLVM_YAML_IS_SEQUENCE_VECTOR(CallSiteYAML)
 LLVM_YAML_IS_SEQUENCE_VECTOR(FunctionYAML)
 
-Error CallSiteInfoLoader::loadYAML(std::vector<FunctionInfo> &Funcs,
-                                   StringRef YAMLFile) {
+Error CallSiteInfoLoader::loadYAML(StringRef YAMLFile) {
   // Step 1: Read YAML file
   auto BufferOrError = MemoryBuffer::getFile(YAMLFile);
   if (!BufferOrError)
@@ -167,14 +166,13 @@ Error CallSiteInfoLoader::loadYAML(std::vector<FunctionInfo> &Funcs,
                              Buffer->getBufferIdentifier().str().c_str());
 
   // Step 3: Build function map from Funcs
-  auto FuncMap = buildFunctionMap(Funcs);
+  auto FuncMap = buildFunctionMap();
 
   // Step 4: Process parsed YAML functions and update FuncMap
   return processYAMLFunctions(FuncsYAML, FuncMap);
 }
 
-StringMap<FunctionInfo *>
-CallSiteInfoLoader::buildFunctionMap(std::vector<FunctionInfo> &Funcs) {
+StringMap<FunctionInfo *> CallSiteInfoLoader::buildFunctionMap() {
   // If the function name is already in the map, don't update it. This way we
   // preferentially use the first encountered function. Since symbols are
   // loaded from dSYM first, we end up preferring keeping track of symbols

llvm/lib/DebugInfo/GSYM/GsymCreator.cpp

@@ -191,8 +191,8 @@ llvm::Error GsymCreator::encode(FileWriter &O) const {
 
 llvm::Error GsymCreator::loadCallSitesFromYAML(StringRef YAMLFile) {
   // Use the loader to load call site information from the YAML file.
-  CallSiteInfoLoader Loader(*this);
-  return Loader.loadYAML(Funcs, YAMLFile);
+  CallSiteInfoLoader Loader(*this, Funcs);
+  return Loader.loadYAML(YAMLFile);
 }
 
 void GsymCreator::prepareMergedFunctions(OutputAggregator &Out) {

llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-gsym-callsite-info-dsym.yaml

@@ -4,7 +4,7 @@
 # RUN: split-file %s %t
 # RUN: yaml2obj %t/call_sites.dSYM.yaml -o %t/call_sites.dSYM
 
-# RUN: llvm-gsymutil --convert=%t/call_sites.dSYM --callsites-from-yaml=%t/callsites.yaml -o %t/call_sites_dSYM.gsym
+# RUN: llvm-gsymutil --convert=%t/call_sites.dSYM --callsites-yaml-file=%t/callsites.yaml -o %t/call_sites_dSYM.gsym
 
 # Dump the GSYM file and check the output for callsite information
 # RUN: llvm-gsymutil %t/call_sites_dSYM.gsym | FileCheck --check-prefix=CHECK-GSYM %s

llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-gsym-callsite-info-exe.yaml

@@ -4,7 +4,7 @@
 # RUN: split-file %s %t
 # RUN: yaml2obj %t/call_sites.exe.yaml -o %t/call_sites.exe
 
-# RUN: llvm-gsymutil --convert=%t/call_sites.exe --callsites-from-yaml=%t/callsites.yaml -o %t/call_sites_exe.gsym
+# RUN: llvm-gsymutil --convert=%t/call_sites.exe --callsites-yaml-file=%t/callsites.yaml -o %t/call_sites_exe.gsym
 
 # Dump the GSYM file and check the output for callsite information
 # RUN: llvm-gsymutil %t/call_sites_exe.gsym | FileCheck --check-prefix=CHECK-GSYM %s

llvm/test/tools/llvm-gsymutil/ARM_AArch64/macho-gsym-callsite-info-obj.test

@@ -2,7 +2,7 @@
 
 // Assemble the input assembly code into an object file
 // RUN: llc -enable-machine-outliner=never -mtriple arm64-apple-darwin -filetype=obj %t/call_sites.ll -o %t/call_sites.o
-// RUN: llvm-gsymutil --convert=%t/call_sites.o --callsites-from-yaml=%t/callsites.yaml -o %t/call_sites_obj.gsym
+// RUN: llvm-gsymutil --convert=%t/call_sites.o --callsites-yaml-file=%t/callsites.yaml -o %t/call_sites_obj.gsym
 
 // Dump the GSYM file and check the output for callsite information
 // RUN: llvm-gsymutil %t/call_sites_obj.gsym | FileCheck --check-prefix=CHECK-GSYM %s

llvm/tools/llvm-gsymutil/Opts.td

@@ -18,8 +18,8 @@ defm convert :
      "Convert the specified file to the GSYM format.\nSupported files include ELF and mach-o files that will have their debug info (DWARF) and symbol table converted">;
 def merged_functions :
   FF<"merged-functions", "Encode merged function information for functions in debug info that have matching address ranges.\nWithout this option one function per unique address range will be emitted.">;
-defm callsites_from_yaml :
-  Eq<"callsites-from-yaml", "Load call site info from YAML file.">;
+defm callsites_yaml_file :
+  Eq<"callsites-yaml-file", "Load call site info from YAML file.">;
 defm arch :
   Eq<"arch",
      "Process debug information for the specified CPU architecture only.\nArchitectures may be specified by name or by number.\nThis option can be specified multiple times, once for each desired architecture">;

llvm/tools/llvm-gsymutil/llvm-gsymutil.cpp

@@ -180,12 +180,12 @@ static void parseArgs(int argc, char **argv) {
   LookupAddressesFromStdin = Args.hasArg(OPT_addresses_from_stdin);
   StoreMergedFunctionInfo = Args.hasArg(OPT_merged_functions);
 
-  if (Args.hasArg(OPT_callsites_from_yaml_EQ)) {
-    CallSiteYamlPath = Args.getLastArgValue(OPT_callsites_from_yaml_EQ);
+  if (Args.hasArg(OPT_callsites_yaml_file_EQ)) {
+    CallSiteYamlPath = Args.getLastArgValue(OPT_callsites_yaml_file_EQ);
     if (CallSiteYamlPath.empty()) {
       llvm::errs()
           << ToolName
-          << ": --callsites-from-yaml option requires a non-empty argument.\n";
+          << ": --callsites-yaml-file option requires a non-empty argument.\n";
       std::exit(1);
     }
   }