[Mlir-commits] [mlir] [mlir-lsp] Abstract input and output of the `JSONTransport` (PR #129320)

[Nikolay Panchenko]

================
@@ -43,14 +43,86 @@ enum JSONStreamStyle {
   Delimited
 };
 
+/// An abstract class used by the JSONTransport to read JSON message.
+class JSONTransportInput {
+public:
+  explicit JSONTransportInput(JSONStreamStyle style = JSONStreamStyle::Standard)
+      : style(style) {}
+  virtual ~JSONTransportInput() = default;
+
+  virtual bool getError() const = 0;
+  virtual bool isEndOfInput() const = 0;
+
+  /// Read in a message from the input stream.
+  virtual LogicalResult readMessage(std::string &json) {
+    return style == JSONStreamStyle::Delimited ? readDelimitedMessage(json)
+                                               : readStandardMessage(json);
+  }
+  virtual LogicalResult readDelimitedMessage(std::string &json) = 0;
+  virtual LogicalResult readStandardMessage(std::string &json) = 0;
+
+private:
+  /// The JSON stream style to use.
+  JSONStreamStyle style;
+};
+
+/// An abstract class used by the JSONTransport to write JSON messages.
+class JSONTransportOutput {
+public:
+  explicit JSONTransportOutput() = default;
+  virtual ~JSONTransportOutput() = default;
+
+  virtual void sendMessage(const llvm::json::Value &msg) = 0;
+};
+
+/// Concrete implementation of the JSONTransportInput that reads from a file.
+class JSONTransportInputOverFile : public JSONTransportInput {
+public:
+  explicit JSONTransportInputOverFile(
+      std::FILE *in, JSONStreamStyle style = JSONStreamStyle::Standard)
+      : JSONTransportInput(style), in(in) {}
+
+  bool getError() const final { return ferror(in); }
+  bool isEndOfInput() const final { return feof(in); }
+
+  LogicalResult readDelimitedMessage(std::string &json) final;
+  LogicalResult readStandardMessage(std::string &json) final;
+
+private:
+  std::FILE *in;
+};
+
+/// Concrete implementation of the JSONTransportOutput that writes to a stream.
+class JSONTransportOutputOverStream : public JSONTransportOutput {
----------------
npanchen wrote:

I do agree that raw_ostream seems sufficient even for extreme case, such as network communication, but it does provide unnecessary functionality that isn't needed. That is, it's like to use a cannon to kill a fly.

Shouldn't output be abstracted with minimal methods ? For example, I see that [MCStreamer](https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/MC/MCStreamer.h#L215) is quite similar to our discussion (with exception of many methods inside), which is not inherited from raw_ostream, [MCAsmStreamer](https://github.com/llvm/llvm-project/blob/main/llvm/lib/MC/MCAsmStreamer.cpp#L47), a concrete implementation, does use raw_ostream to output.

That said, I do understand your point about "not being necessary", at the same time I think it's better to hide footgun (unneeded and untested methods of raw_ostream) away. API-wise, user still can construct JSONTransport using raw_ostream for convenience.

https://github.com/llvm/llvm-project/pull/129320