[PATCH] D125778: [llvm-dva] 03 - Logical elements

[Paul Robinson via Phabricator via llvm-commits]

probinson added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVReader.h:109
+  StringRef getFilename() const { return InputFilename; }
+  void setFilename(std::string Name) { InputFilename = Name; }
+  StringRef getFileFormatName() const { return FileFormatName; }
----------------
psamolysov wrote:
> 
Should Name be a reference parameter? Passing std::string by value could result in a creating/destroying a temp copy.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h:221
+
+  virtual void resolve() override;
+  virtual void resolveName() override;
----------------
psamolysov wrote:
> Either `virtual` or `override` is enough.
When a declaration overrides an inherited `virtual` declaration, it really should use `override` to guarantee that the prior `virtual` declaration exactly matches.  `override` implies `virtual` and so we omit `virtual` to reduce clutter.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125778/new/

https://reviews.llvm.org/D125778