[PATCH] D51976: [DebugInfo][Dexter] Speculated BB presents illegal variable value to debugger

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: lib/Transforms/Utils/Local.cpp:2525
 
+void llvm::dropDebugMetadata(Instruction &I) {
+  SmallVector<DbgVariableIntrinsic *, 1> DbgUsers;
----------------
probinson wrote:
> The function name and the comment in the .h file made me think this is removing the `!dbg` metadata from the specified instruction.  But that's not at all what it does.
> 
> People generally don't use the term 'metadata' to refer to the debug intrinsic instructions.  Maybe `dropDebugUsers` would be a better name.
> 
Good observation. Changing the name to `dropDebugUsers`.


================
Comment at: lib/Transforms/Utils/Local.cpp:2528
+  findDbgUsers(DbgUsers, &I);
+  for (DbgVariableIntrinsic *DII : DbgUsers)
+    DII->eraseFromParent();
----------------
TWeaver wrote:
> TWeaver wrote:
> > I'd use for (auto &DII : DbgUsers) here. The type is obvious from the declaration of DbgUsers. This should make this line a little easier on the eyes.
> is DII the correct name for the iterator here? should this be DVI inline with the name of the type used in the declaration of DbgUsers?
> 
> That is : DVI for DbgVariableIntrinsic.
Changing the loop to `for (auto &DII : DbgUsers)`.


================
Comment at: lib/Transforms/Utils/Local.cpp:2528
+  findDbgUsers(DbgUsers, &I);
+  for (DbgVariableIntrinsic *DII : DbgUsers)
+    DII->eraseFromParent();
----------------
CarlosAlbertoEnciso wrote:
> TWeaver wrote:
> > TWeaver wrote:
> > > I'd use for (auto &DII : DbgUsers) here. The type is obvious from the declaration of DbgUsers. This should make this line a little easier on the eyes.
> > is DII the correct name for the iterator here? should this be DVI inline with the name of the type used in the declaration of DbgUsers?
> > 
> > That is : DVI for DbgVariableIntrinsic.
> Changing the loop to `for (auto &DII : DbgUsers)`.
I can see your point.

The `SmallVector<DbgVariableIntrinsic *, 1>` is used in quite few other places and it seems the `DII` is the standard name used for the iterator.

I would prefer to follow that name convention.


Repository:
  rL LLVM

https://reviews.llvm.org/D51976