Details
- Reviewers
shchenz jsji DiggerLin jhenderson sfertile hubert.reinterpretcast - Group Reviewers
Restricted Project - Commits
- rG6143ec2961d1: [NFC][XCOFF] Refactor and format XCOFFObjectWriter.cpp.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| llvm/lib/MC/XCOFFObjectWriter.cpp | ||
|---|---|---|
| 566–567 | do we still need is64Bit() ? | |
| 567–569 | " in 32-bit mode" will be deleted ? | |
| 662–669 | change writeSymbolBody to writeSymbolEntry ? | |
| 664 | do we want to set a default AuxCnt=1 ? | |
| 676–677 | delete the comment. | |
| 680 | change Name to writeSymbolAuxCsectEntry ? | |
| 682 | I am not sure which ( "x_scnlen" or "SectionOrLength")one is better for comment ? maybe the SectionOrLength from XCOFFObjectFile.h is better. | |
| 691 | function name change to writeSymbolAuxDwarfEntry ? | |
| 743–744 | since the header is for 32bit. W.write<uint32_t>(SymbolTableOffset); // f_symptr W.write<int32_t>(SymbolTableEntryCount); // f_nsyms we should modify the code | |
| 769–770 | I do not think we need the s_size here , Sec->Size can self-explain. | |
| llvm/lib/MC/XCOFFObjectWriter.cpp | ||
|---|---|---|
| 823–829 | we can use ".file" directly instead of StringRef(".file") | |
Addressed @DiggerLin 's comments. Thanks
| llvm/lib/MC/XCOFFObjectWriter.cpp | ||
|---|---|---|
| 682 |
| |
| 682 | I would prefer the x_scnlen one because it is easier for the developer to refer to the documentation, and this comment style references ELFObjectWriter.cpp. | |
| 743–744 | Thanks! | |
| 769–770 | Removed // s_size and // s_flags, and kept other comments. | |
| llvm/lib/MC/XCOFFObjectWriter.cpp | ||
|---|---|---|
| 201 | change to | |
| llvm/lib/MC/XCOFFObjectWriter.cpp | ||
|---|---|---|
| 201 | sorry should be change to const uint64_t MaxRawDataSize = | |
| llvm/lib/MC/XCOFFObjectWriter.cpp | ||
|---|---|---|
| 278 | using writeSymbolAuxCsectEntry(uint64_t SecLen, uint8_t Type, uint8_t MapClass) is easy to understand the interface than writeSymbolAuxCsectEntry(uint64_t, uint8_t, uint8_t); | |
| 662–669 | uint64_t Value--> uint32_t Value | |
| 665 | maybe better to change the AuxCnt to NumberOfAuxEntries ? same as function | |
| 682 | the function looks only support 32bit AuxCsect now . "uint64_t SecLen" should be "uint32_t SecLen"? if we change to "uint32_t SecLen" W.write<uint32_t>(Lo_32(SecLen)); // x_scnlen also need to be changed. | |
| 693 | "uint64_t SecLen" should be "uint32_t SecLen" ? and "uint64_t RelCnt" should be "uint32_t RelCnt" | |
| 720–723 | change "DwarfSectionRef.getSymbolTableName(), 0, SectionIndex, 0," adding comment will make the value "0" easy to understand. | |
| 730 | add a comment for value 0, ditto in the following value zero or 1 | |
| llvm/lib/MC/XCOFFObjectWriter.cpp | ||
|---|---|---|
| 276–279 | Please provide the parameter names here. | |
| 694 | Default arguments are usually specified in the declaration, not definition. Same elsewhere. | |
| 701 | For consistency with other names. | |
| 743–744 | I'd have the assertion: in the future, you may not call this function from the same place. | |
| 822–823 | Ditto the line below. | |
| llvm/lib/MC/XCOFFObjectWriter.cpp | ||
|---|---|---|
| 710 | Here and similar comments elsewhere, the format is as suggested inline. | |
| llvm/lib/MC/XCOFFObjectWriter.cpp | ||
|---|---|---|
| 667 | the writeSymbolName(SymbolName) can self explain . | |
change to
constexpr uint64_t MaxRawDataSize =