[PATCH] D29349: Fix the bitcode upgrade for DIGlobalVariable in a DIImportedEntity context.

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: lib/Bitcode/Reader/MetadataLoader.cpp:793
       upgradeCUSubprograms();
+      upgradeCUVariables();
       return Error::success();
----------------
Should we have a single entry point "performDebugInfoUpgrade" instead of duplicating the calls at every exit point?


================
Comment at: lib/Bitcode/Reader/MetadataLoader.cpp:1463
 
-      auto *DGVE = DIGlobalVariableExpression::getDistinct(Context, DGV, Expr);
-      MetadataList.assignValue(DGVE, NextMetadataNo);
-      NextMetadataNo++;
+      NeedUpgradeToDIGlobalVariableExpression = true;
+      DIGlobalVariableExpression *DGVE = nullptr;
----------------
Does this need to be set to true when DGVE is correctly set below?


================
Comment at: lib/Bitcode/Reader/MetadataLoader.cpp:1470
+
+      Metadata *MDNode = Expr ? (Metadata *)DGVE : (Metadata *)DGV;
+      MetadataList.assignValue(MDNode, NextMetadataNo);
----------------
Are we using C-style cast elsewhere in similar situation?

Also the logic seems quite complex to me (it was already, but getting worse) around the invariant that link `Attach` and `Expr` and what we end up assigning to NextMetadataNo, or attaching.



https://reviews.llvm.org/D29349