[PATCH] D55336: [CodeView] Emit global variables within lexical scopes to limit visibility

[Reid Kleckner via Phabricator via llvm-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm, thanks!



================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:2959-2963
+        auto I = ScopeGlobals.find(Scope);
+        if (I == ScopeGlobals.end()) {
+          bool inserted;
+          std::tie(I, inserted) = ScopeGlobals.insert(
+              {Scope, llvm::make_unique<GlobalVariableList>()});
----------------
Doing find then insert seems unnecessary, but I guess it's done to avoid creating a temporary. I think you can write this as:
```
auto Insertion = ScopeGlobals.insert({Scope, std::unique_ptr<GlobalVariableList>()});
if (Insertion.second)
  Insertion.first.second = llvm::make_unique<GlobalVariableList>();
VariableList = Insertion.first.second.get();
```


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:2967
+        VariableList = I->second.get();
+      } else if (GV->hasComdat())
+        // Emit this global variable into a COMDAT section.
----------------
Hm, so this raises an interesting point. What happens for comdat local statics, like these:
```
inline int f() {
  static int gv;
  return ++gv;
}
```
Presumably it should be handled as any regular local static, so gv should appear in the symbols for f, which is how you have it.


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

https://reviews.llvm.org/D55336