[llvm] [XCOFF] Add compiler version to an auxiliary symbol table entry (PR #80162)

David Tenty via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 2 14:37:49 PST 2024


================
@@ -486,6 +488,15 @@ class MCAssembler {
     FileNames.emplace_back(std::string(FileName), Symbols.size());
   }
 
+  bool setCompilerVersion(std::string CompilerVers) {
+    if (CompilerVersion.empty()) {
+      CompilerVersion = CompilerVers;
+      return true;
+    }
+    return false;
+  }
----------------
daltenty wrote:

1. Is there are reason to have a return value here? We don't check it elsewhere
2. I think there's an extra copy here? Probably either you should: take the parameter as string ref and do the conversion here; or you should use std::move
3. I'm not sure I follow why we need the empty check to prevent modifying the string?

```suggestion
  void setCompilerVersion(std::string CompilerVers) {
      CompilerVersion = std::move(CompilerVers);
  }
```

https://github.com/llvm/llvm-project/pull/80162


More information about the llvm-commits mailing list