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

[Pavel Samolysov via Phabricator via llvm-commits]

psamolysov added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVScope.h:304
+  // Names (files and directories) used by the Compile Unit.
+  std::vector<size_t> Filenames;
+
----------------
CarlosAlbertoEnciso wrote:
> psamolysov wrote:
> > Is this a vector of indices?
> Yes. The vector contains indices into the String Pool table.
> `Filenames.push_back(getStringPool().getIndex(Name));`
Well, thank you for the answer.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVElement.cpp:431
+void LVElement::printFileIndex(raw_ostream &OS, bool Full) const {
+  if (options().getPrintFormatting() && options().getAttributeAnySource() &&
+      getFilenameIndex()) {
----------------
CarlosAlbertoEnciso wrote:
> psamolysov wrote:
> > Early return if the inverted condition takes place is more preferable by the coding style.
> I see your point.
> 
> ```
> if (!(options().getPrintFormatting() && options().getAttributeAnySource() &&
>       getFilenameIndex()))
>   return;
> ```
> But with the inverted condition, it seems the statetment gets obscure.
I see such long `if`s are actively used though the code. So, I think the code could be left as it was initially written for consistency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125778