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

[Brock Wyma via Phabricator via llvm-commits]

bwyma marked 5 inline comments as done.
bwyma added inline comments.


================
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>()});
----------------
rnk wrote:
> 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();
> ```
Fixed in the next patch upload.


================
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.
----------------
rnk wrote:
> 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.
Yes, gv should appear in the debug symbols inside the comdat section for f.


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

https://reviews.llvm.org/D55336