Xiangling_L marked 3 inline comments as done. Xiangling_L added inline comments.
================ Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466 + + bool preprocessStructorList(const DataLayout &DL, const Constant *List, + SmallVector<Structor, 8> &Structors); ---------------- jasonliu wrote: > Xiangling_L wrote: > > jasonliu wrote: > > > Xiangling_L wrote: > > > > jasonliu wrote: > > > > > A doxygen comment describe what this function does, and what its > > > > > return value means, and mention `Structors` is an output argument. > > > > > By looking at what this function does, it seems `buildStructorList` > > > > > is a better name. > > > > I meant to and should've named this function to > > > > `preprocessXXStructorList`, let me know if you still prefer > > > > `buildStructorList`. And if you do, since the underneath of > > > > `SmallVector` is a variable-sized array, maybe we should try > > > > `buildSortedStructorArray`? > > > `preprocess` sounds like we are already having a XXStructorList and now > > > we try to do something on it. > > > But right now, we are actually passing in an empty StructorList/Array and > > > build it from scratch. So I would still prefer the name of `build` in it. > > > I don't mind changing to a more accurate name as you suggested. > > I think we do have a `XXStructorList` here which is the initializer of > > `llvm.gloal_ctors`or `llvm.gloal_dtors` array? The usage of this term is > > consistent with other spots. > My understanding is that before we enter this `preprocessXXStructorList`, we > do not have a list of XXStructor. We only have a list of `Constant`. This > functions turns a list of `Constant` to a list of `XXStructor`. Just leave a record here, as we discussed offline, we agree that the meaning of term `XXStructorList` is `the initializer of `llvm.gloal_ctors` or `llvm.gloal_dtors` array. So I will keep using `preprocessXXStructorList` as the function name. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84534/new/ https://reviews.llvm.org/D84534 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
