Refactor wasm/WriterUtil.{cpp,h}.

[ruiu]

Summary

• Makes code more in line with LLVM style (e.g. const char * -> StringRef)
• Do not use formatv since we can construct message just by +
• Replace some odd types such as const StringRef with more common type
• Do not use default arguments if they are not necessary

[ruiu]

• removed if (!Msg.empty()) check. When we use these functions, we should always print out debug message.

[sbc100]

Thanks!

lld/wasm/WriterUtils.cpp

83 ↗

(On Diff #134686)

I sometimes find the format string more readable than lots of small strings fragments +'d together. But I don't feel strongly enough about to to break local convention.

[ruiu]

I sometimes find the format string more readable than lots of small strings fragments +'d together. But I don't feel strongly enough about to to break local convention.

Not everybody memorizes this Python-style format string, and lld doesn't have a lot of code that needs formatting strings (so even if there's something better, investing in it isn't worth the cost), so we generally avoid using that in lld.

wasm/OutputSections.h

protected:
   size_t Offset = 0;
 };
 
 class SyntheticSection : public OutputSection {
 public:
   SyntheticSection(uint32_t Type, std::string Name = "")
       : OutputSection(Type, Name), BodyOutputStream(Body) {
     if (!Name.empty())
-      writeStr(BodyOutputStream, Name);
+      writeStr(BodyOutputStream, Name, "section name");
   }
 
   void writeTo(uint8_t *Buf) override {
     assert(Offset);
     log("writing " + toString(*this));
     memcpy(Buf + Offset, Header.data(), Header.size());
     memcpy(Buf + Offset + Header.size(), Body.data(), Body.size());
   }

 // class is used to create subsections and then write them into the stream
 // of the parent section.
 class SubSection : public SyntheticSection {
 public:
   explicit SubSection(uint32_t Type) : SyntheticSection(Type) {}
 
   std::string getSectionName() const;
   void writeToStream(raw_ostream &OS) {
-    writeBytes(OS, Header.data(), Header.size());
-    writeBytes(OS, Body.data(), Body.size());
+    OS.write(Header.data(), Header.size());
+    OS.write(Body.data(), Body.size());
   }
 };
 
 class CodeSection : public OutputSection {
 public:
   explicit CodeSection(ArrayRef<InputFunction *> Functions);
   size_t getSize() const override { return Header.size() + BodySize; }
   void writeTo(uint8_t *Buf) override;

wasm/OutputSections.cpp

 #include "OutputSections.h"
 
 #include "InputChunks.h"
 #include "InputFiles.h"
 #include "OutputSegment.h"
 #include "lld/Common/ErrorHandler.h"
 #include "lld/Common/Threads.h"
 #include "llvm/ADT/Twine.h"
+#include "llvm/Support/LEB128.h"
 
 #define DEBUG_TYPE "lld"
 
 using namespace llvm;
 using namespace llvm::wasm;
 using namespace lld;
 using namespace lld::wasm;
 

 
 std::string SubSection::getSectionName() const {
   return std::string("subsection <type=") + std::to_string(Type) + ">";
 }
 
 void OutputSection::createHeader(size_t BodySize) {
   raw_string_ostream OS(Header);
   debugWrite(OS.tell(), "section type [" + Twine(getSectionName()) + "]");
-  writeUleb128(OS, Type, nullptr);
+  encodeULEB128(Type, OS);
   writeUleb128(OS, BodySize, "section size");
   OS.flush();
   log("createHeader: " + toString(*this) + " body=" + Twine(BodySize) +
       " total=" + Twine(getSize()));
 }
 
 CodeSection::CodeSection(ArrayRef<InputFunction *> Functions)
     : OutputSection(WASM_SEC_CODE), Functions(Functions) {

wasm/WriterUtils.h

 //===- WriterUtils.h --------------------------------------------*- C++ -*-===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLD_WASM_WRITERUTILS_H
 #define LLD_WASM_WRITERUTILS_H
 
+#include "lld/Common/LLVM.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Object/Wasm.h"
 #include "llvm/Support/raw_ostream.h"
 
 using llvm::raw_ostream;
 
 // Needed for WasmSignatureDenseMapInfo
 inline bool operator==(const llvm::wasm::WasmSignature &LHS,

 namespace wasm {
 
 struct OutputRelocation {
   llvm::wasm::WasmRelocation Reloc;
   uint32_t NewIndex;
   uint32_t Value;
 };
 
-void debugWrite(uint64_t Offset, llvm::Twine Msg);
+void debugWrite(uint64_t Offset, const Twine &Msg);
 
-void writeUleb128(raw_ostream &OS, uint32_t Number, const char *Msg);
+void writeUleb128(raw_ostream &OS, uint32_t Number, StringRef Msg);
 
-void writeSleb128(raw_ostream &OS, int32_t Number, const char *Msg);
+void writeSleb128(raw_ostream &OS, int32_t Number, StringRef Msg);
 
-void writeBytes(raw_ostream &OS, const char *Bytes, size_t count,
-                const char *Msg = nullptr);
+void writeBytes(raw_ostream &OS, const char *Bytes, size_t count, StringRef Msg);
 
-void writeStr(raw_ostream &OS, const llvm::StringRef String,
-              const char *Msg = nullptr);
+void writeStr(raw_ostream &OS, StringRef String, StringRef Msg);
 
-void writeU8(raw_ostream &OS, uint8_t byte, const char *Msg);
+void writeU8(raw_ostream &OS, uint8_t byte, StringRef Msg);
 
-void writeU32(raw_ostream &OS, uint32_t Number, const char *Msg);
+void writeU32(raw_ostream &OS, uint32_t Number, StringRef Msg);
 
-void writeValueType(raw_ostream &OS, int32_t Type, const char *Msg);
+void writeValueType(raw_ostream &OS, int32_t Type, StringRef Msg);
 
 void writeSig(raw_ostream &OS, const llvm::wasm::WasmSignature &Sig);
 
 void writeInitExpr(raw_ostream &OS, const llvm::wasm::WasmInitExpr &InitExpr);
 
 void writeLimits(raw_ostream &OS, const llvm::wasm::WasmLimits &Limits);
 
 void writeGlobalType(raw_ostream &OS, const llvm::wasm::WasmGlobalType &Type);

wasm/WriterUtils.cpp

 //===- WriterUtils.cpp ----------------------------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "WriterUtils.h"
 
 #include "lld/Common/ErrorHandler.h"
 
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/EndianStream.h"
-#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/LEB128.h"
 
 #define DEBUG_TYPE "lld"
 
 using namespace llvm;
 using namespace llvm::wasm;
 using namespace lld::wasm;
 
case WASM_TYPE_F64:
     return "f64";
   default:
     llvm_unreachable("invalid value type");
   }
 }
 
 namespace lld {
 
-void wasm::debugWrite(uint64_t Offset, Twine Msg) {
-  DEBUG(dbgs() << format("  | %08" PRIx64 ": ", Offset) << Msg << "\n");
+void wasm::debugWrite(uint64_t Offset, const Twine &Msg) {
+  DEBUG(dbgs() << format("  | %08lld: ", Offset) << Msg << "\n");
 }
 
-void wasm::writeUleb128(raw_ostream &OS, uint32_t Number, const char *Msg) {
-  if (Msg)
-    debugWrite(OS.tell(), Msg + formatv(" [{0:x}]", Number));
+void wasm::writeUleb128(raw_ostream &OS, uint32_t Number, StringRef Msg) {
+  debugWrite(OS.tell(), Msg + "[" + utohexstr(Number) + "]");
   encodeULEB128(Number, OS);
 }
 
-void wasm::writeSleb128(raw_ostream &OS, int32_t Number, const char *Msg) {
-  if (Msg)
-    debugWrite(OS.tell(), Msg + formatv(" [{0:x}]", Number));
+void wasm::writeSleb128(raw_ostream &OS, int32_t Number, StringRef Msg) {
+  debugWrite(OS.tell(), Msg + "[" + utohexstr(Number) + "]");
   encodeSLEB128(Number, OS);
 }
 
 void wasm::writeBytes(raw_ostream &OS, const char *Bytes, size_t Count,
-                      const char *Msg) {
-  if (Msg)
-    debugWrite(OS.tell(), Msg + formatv(" [data[{0}]]", Count));
+                      StringRef Msg) {
+  debugWrite(OS.tell(), Msg + " [data[" + Twine(Count) + "]]");
   OS.write(Bytes, Count);
 }
 
-void wasm::writeStr(raw_ostream &OS, const StringRef String, const char *Msg) {
-  if (Msg)
-    debugWrite(OS.tell(),
-               Msg + formatv(" [str[{0}]: {1}]", String.size(), String));
-  writeUleb128(OS, String.size(), nullptr);
-  writeBytes(OS, String.data(), String.size());
+void wasm::writeStr(raw_ostream &OS, StringRef String, StringRef Msg) {
+             Msg + " [str[" + Twine(String.size()) + "]: " + String + "]");
+  encodeULEB128(String.size(), OS);
+  OS.write(String.data(), String.size());
 }
 
-void wasm::writeU8(raw_ostream &OS, uint8_t byte, const char *Msg) {
-  OS << byte;
-}
+void wasm::writeU8(raw_ostream &OS, uint8_t byte, StringRef Msg) { OS << byte; }
 
-void wasm::writeU32(raw_ostream &OS, uint32_t Number, const char *Msg) {
-  debugWrite(OS.tell(), Msg + formatv("[{0:x}]", Number));
+void wasm::writeU32(raw_ostream &OS, uint32_t Number, StringRef Msg) {
+  debugWrite(OS.tell(), Msg + "[" + utohexstr(Number) + "]");
   support::endian::Writer<support::little>(OS).write(Number);
 }
 
-void wasm::writeValueType(raw_ostream &OS, int32_t Type, const char *Msg) {
-  debugWrite(OS.tell(), Msg + formatv("[type: {0}]", valueTypeToString(Type)));
-  writeSleb128(OS, Type, nullptr);
+void wasm::writeValueType(raw_ostream &OS, int32_t Type, StringRef Msg) {
+  debugWrite(OS.tell(), Msg + "[type: " + valueTypeToString(Type) + "]");
+  encodeSLEB128(Type, OS);
 }
 
 void wasm::writeSig(raw_ostream &OS, const WasmSignature &Sig) {
   writeSleb128(OS, WASM_TYPE_FUNC, "signature type");
   writeUleb128(OS, Sig.ParamTypes.size(), "param Count");
   for (int32_t ParamType : Sig.ParamTypes) {
     writeValueType(OS, ParamType, "param type");
   }