Address Feedback nr.1

Author: Alex B

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

@@ -34,7 +34,6 @@ namespace gsym {
 class FileWriter;
 struct FunctionInfo;
 struct CallSiteInfo {
-public:
   enum Flags : uint8_t {
     None = 0,
     // This flag specifies that the call site can only call a function within
@@ -52,7 +51,7 @@ struct CallSiteInfo {
   std::vector<uint32_t> MatchRegex;
 
   /// Bitwise OR of CallSiteInfo::Flags values
-  uint8_t Flags;
+  uint8_t Flags = CallSiteInfo::Flags::None;
 
   /// Decode a CallSiteInfo object from a binary data stream.
   ///
@@ -73,16 +72,8 @@ struct CallSiteInfo {
 };
 
 struct CallSiteInfoCollection {
-public:
   std::vector<CallSiteInfo> CallSites;
 
-  void clear() { CallSites.clear(); }
-
-  /// Query if a CallSiteInfoCollection object is valid.
-  ///
-  /// \returns True if the collection is not empty.
-  bool isValid() const { return !CallSites.empty(); }
-
   /// Decode a CallSiteInfoCollection object from a binary data stream.
   ///
   /// \param Data The binary stream to read the data from.
@@ -156,54 +147,27 @@ class CallSiteInfoLoader {
   /// \returns A 32-bit unsigned integer representing the offset of the string.
   uint32_t offsetFromString(StringRef str);
 
-  /// Reads the content of the YAML file specified by `YAMLFile` into
-  /// `yamlContent`.
-  ///
-  /// \param YAMLFile A StringRef representing the path to the YAML file.
-  /// \param Buffer The memory buffer containing the YAML content.
-  ///
-  /// \returns An `llvm::Error` indicating success or describing any issues
-  /// encountered while reading the file.
-  llvm::Error readYAMLFile(StringRef YAMLFile,
-                           std::unique_ptr<llvm::MemoryBuffer> &Buffer);
-
-  /// Parses the YAML content and populates `functionsYAML` with the parsed
-  /// data.
-  ///
-  /// \param Buffer The memory buffer containing the YAML content.
-  /// \param functionsYAML A reference to an llvm::yaml::FunctionsYAML object to
-  /// be populated.
-  ///
-  /// \returns An `llvm::Error` indicating success or describing any issues
-  /// encountered during parsing.
-  llvm::Error parseYAML(llvm::MemoryBuffer &Buffer,
-                        llvm::yaml::FunctionsYAML &functionsYAML);
-
   /// Builds a map from function names to FunctionInfo pointers based on the
   /// provided `Funcs` vector.
   ///
   /// \param Funcs A reference to a vector of FunctionInfo objects.
   ///
-  /// \returns An unordered_map mapping function names (std::string) to their
+  /// \returns A StringMap mapping function names (StringRef) to their
   /// corresponding FunctionInfo pointers.
-  std::unordered_map<std::string, FunctionInfo *>
-  buildFunctionMap(std::vector<FunctionInfo> &Funcs);
+  StringMap<FunctionInfo *> buildFunctionMap(std::vector<FunctionInfo> &Funcs);
 
   /// Processes the parsed YAML functions and updates the `FuncMap` accordingly.
   ///
   /// \param functionsYAML A constant reference to an llvm::yaml::FunctionsYAML
   /// object containing parsed YAML data.
-  /// \param FuncMap A reference to an unordered_map mapping function names to
+  /// \param FuncMap A reference to a StringMap mapping function names to
   /// FunctionInfo pointers.
-  /// \param YAMLFile A StringRef representing the name of the YAML file (used
-  /// for error messages).
   ///
   /// \returns An `llvm::Error` indicating success or describing any issues
   /// encountered during processing.
   llvm::Error
   processYAMLFunctions(const llvm::yaml::FunctionsYAML &functionsYAML,
-                       std::unordered_map<std::string, FunctionInfo *> &FuncMap,
-                       StringRef YAMLFile);
+                       StringMap<FunctionInfo *> &FuncMap);
 
   /// Map of existing string offsets to CachedHashStringRef.
   DenseMap<uint64_t, CachedHashStringRef> &StringOffsetMap;

llvm/lib/DebugInfo/GSYM/CallSiteInfo.cpp

@@ -162,49 +162,32 @@ uint32_t CallSiteInfoLoader::offsetFromString(StringRef str) {
 
 llvm::Error CallSiteInfoLoader::loadYAML(std::vector<FunctionInfo> &Funcs,
                                          StringRef YAMLFile) {
-  std::unique_ptr<llvm::MemoryBuffer> Buffer;
   // Step 1: Read YAML file
-  if (auto Err = readYAMLFile(YAMLFile, Buffer))
-    return Err;
-
-  // Step 2: Parse YAML content
-  llvm::yaml::FunctionsYAML functionsYAML;
-  if (auto Err = parseYAML(*Buffer, functionsYAML))
-    return Err;
-
-  // Step 3: Build function map from Funcs
-  auto FuncMap = buildFunctionMap(Funcs);
-
-  // Step 4: Process parsed YAML functions and update FuncMap
-  return processYAMLFunctions(functionsYAML, FuncMap, YAMLFile);
-}
-
-llvm::Error
-CallSiteInfoLoader::readYAMLFile(StringRef YAMLFile,
-                                 std::unique_ptr<llvm::MemoryBuffer> &Buffer) {
   auto BufferOrError = llvm::MemoryBuffer::getFile(YAMLFile);
   if (!BufferOrError)
     return errorCodeToError(BufferOrError.getError());
-  Buffer = std::move(*BufferOrError);
-  return llvm::Error::success();
-}
 
-llvm::Error
-CallSiteInfoLoader::parseYAML(llvm::MemoryBuffer &Buffer,
-                              llvm::yaml::FunctionsYAML &functionsYAML) {
-  // Use the MemoryBufferRef constructor
-  llvm::yaml::Input yin(Buffer.getMemBufferRef());
+  std::unique_ptr<llvm::MemoryBuffer> Buffer = std::move(*BufferOrError);
+
+  // Step 2: Parse YAML content
+  llvm::yaml::FunctionsYAML functionsYAML;
+  llvm::yaml::Input yin(Buffer->getMemBufferRef());
   yin >> functionsYAML;
   if (yin.error()) {
     return llvm::createStringError(yin.error(), "Error parsing YAML file: %s\n",
-                                   Buffer.getBufferIdentifier().str().c_str());
+                                   Buffer->getBufferIdentifier().str().c_str());
   }
-  return llvm::Error::success();
+
+  // Step 3: Build function map from Funcs
+  auto FuncMap = buildFunctionMap(Funcs);
+
+  // Step 4: Process parsed YAML functions and update FuncMap
+  return processYAMLFunctions(functionsYAML, FuncMap);
 }
 
-std::unordered_map<std::string, FunctionInfo *>
+StringMap<FunctionInfo *>
 CallSiteInfoLoader::buildFunctionMap(std::vector<FunctionInfo> &Funcs) {
-  std::unordered_map<std::string, FunctionInfo *> FuncMap;
+  StringMap<FunctionInfo *> FuncMap;
   auto insertFunc = [&](auto &Function) {
     std::string FuncName = stringFromOffset(Function.Name).str();
     // If the function name is already in the map, don't update it. This way we
@@ -227,8 +210,7 @@ CallSiteInfoLoader::buildFunctionMap(std::vector<FunctionInfo> &Funcs) {
 
 llvm::Error CallSiteInfoLoader::processYAMLFunctions(
     const llvm::yaml::FunctionsYAML &functionsYAML,
-    std::unordered_map<std::string, FunctionInfo *> &FuncMap,
-    StringRef YAMLFile) {
+    StringMap<FunctionInfo *> &FuncMap) {
   // For each function in the YAML file
   for (const auto &FuncYAML : functionsYAML.functions) {
     auto it = FuncMap.find(FuncYAML.name);
@@ -247,9 +229,9 @@ llvm::Error CallSiteInfoLoader::processYAMLFunctions(
       // Since YAML has specifies relative return offsets, add the function
       // start address to make the offset absolute.
       CSI.ReturnAddress = FuncInfo->Range.start() + CallSiteYAML.return_offset;
-      for (const auto &regex : CallSiteYAML.match_regex) {
-        CSI.MatchRegex.push_back(offsetFromString(regex));
-      }
+      for (const auto &Regex : CallSiteYAML.match_regex)
+        CSI.MatchRegex.push_back(offsetFromString(Regex));
+
       // Initialize flags to None
       CSI.Flags = CallSiteInfo::None;
       // Parse flags and combine them

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

@@ -180,7 +180,7 @@ static void parseArgs(int argc, char **argv) {
   LookupAddressesFromStdin = Args.hasArg(OPT_addresses_from_stdin);
   StoreMergedFunctionInfo = Args.hasArg(OPT_merged_functions);
 
-  for (const llvm::opt::Arg *A : Args.filtered(OPT_callsites_from_yaml_EQ))
+  if (const llvm::opt::Arg *A = Args.getLastArg(OPT_callsites_from_yaml_EQ)) {
     if (A->getValue() && A->getValue()[0] != '\0')
       CallSiteYamlPaths.emplace_back(A->getValue());
     else {
@@ -189,6 +189,7 @@ static void parseArgs(int argc, char **argv) {
           << ": --callsites-from-yaml option requires a non-empty argument.\n";
       std::exit(1);
     }
+  }
 }
 
 /// @}