[PATCH] D52680: Add comments explaning variables in action table generation (NFC)

[Derek Schuff via Phabricator via llvm-commits]

dschuff added a comment.

I think it could be clearer than it is. The problem is that there are 2 overlapping naming conventions in use simultaneously:

- `SizeFoo` referring to the size of a foo object or objects, (`SizeAction`, `SizeActions`)
- `BarAction` referring to the index of a particular action (`FirstAction`, `PrevAction`).

Both of those are `unsigned` but mean very different things. So it's good that there's a convention but `SizeAction` fits into both and is ambiguous.
I think renaming `SizeAction` to `SizeActionEntry` would fix that. `SizeActions` is probably ok. I think a brief comment explaining the usage is fine but putting it inline with the declaration minimizes risk of it getting outdated.

So, how about the suggestions below:



================
Comment at: lib/CodeGen/AsmPrinter/EHStreamer.cpp:105
   int FirstAction = 0;
   unsigned SizeActions = 0;
   const LandingPadInfo *PrevLPI = nullptr;
----------------
`unsigned SizeActions = 0; // Total size of all action entries for a function`


================
Comment at: lib/CodeGen/AsmPrinter/EHStreamer.cpp:113
     unsigned NumShared = PrevLPI ? sharedTypeIDs(LPI, PrevLPI) : 0;
     unsigned SizeSiteActions = 0;
 
----------------
`unsigned SizeSiteActions = 0; // Total size of all entries for one lnadingpad`


================
Comment at: lib/CodeGen/AsmPrinter/EHStreamer.cpp:116
     if (NumShared < TypeIds.size()) {
       unsigned SizeAction = 0;
       unsigned PrevAction = (unsigned)-1;
----------------
`unsigned SizeActionEntry = 0; // Size of one action entry (typeid + next action)`


Repository:
  rL LLVM

https://reviews.llvm.org/D52680