REFACTOR: Clean up Buffer and BufferManager.

- Simplify Buffer implementation.
- Fix compiler warnings.
- Add TODOs for future improvements.

Ref #1

Author: nos1dot618

include/lbd/source/Buffer.hpp

@@ -1,24 +1,40 @@
 #pragma once
 
 #include <string>
-#include <string_view>
 #include <vector>
 #include <lbd/source/BufferId.hpp>
 #include <lbd/source/Location.hpp>
 
 namespace lbd::source
 {
-  struct Buffer
+  class Buffer
   {
-    BufferId id;
-    std::string name;
-    std::string contents;
-    std::vector<Offset> lineOffsets;
+  public:
+    Buffer(BufferId id, std::string name, std::string contents);
 
-    [[nodiscard]] size_t getSize() const;
+    [[nodiscard]] BufferId getId() const noexcept { return m_id; }
 
-    [[nodiscard]] char operator[](size_t index) const;
+    [[nodiscard]] std::string_view getName() const noexcept { return m_name; }
 
+    [[nodiscard]] std::string_view getContents() const noexcept { return m_contents; }
+
+    [[nodiscard]] const std::vector<Offset> &getLineOffsets() const noexcept { return m_lineOffsets; }
+
+    [[nodiscard]] size_t getSize() const noexcept { return m_contents.size(); }
+
+    [[nodiscard]] char operator[](const size_t index) const noexcept { return m_contents[index]; }
+
+    /**
+     *Computes the starting offset of each line.
+     *
+     * The first entry is always `0`, representing the start of the first line.
+     */
     void computeLineOffsets() noexcept;
+
+  private:
+    BufferId m_id;
+    std::string m_name;
+    std::string m_contents;
+    std::vector<Offset> m_lineOffsets;
   };
-}
+}

include/lbd/source/BufferManager.hpp

@@ -16,15 +16,18 @@ namespace lbd::source
     [[nodiscard]] BufferId createBuffer(std::string name, std::string contents) noexcept;
 
     // Access Buffers.
-    [[nodiscard]] const Buffer &getBuffer(BufferId id) const noexcept;
+    [[nodiscard]] const Buffer &getBuffer(const BufferId id) const noexcept { return m_buffers[id]; }
 
-    [[nodiscard]] std::string_view getBufferName(BufferId id) const noexcept;
+    [[nodiscard]] std::string_view getBufferName(const BufferId id) const noexcept { return m_buffers[id].getName(); }
 
-    [[nodiscard]] std::string_view getBufferContents(BufferId id) const noexcept;
+    [[nodiscard]] size_t getBufferSize(const BufferId id) const noexcept { return m_buffers[id].getSize(); }
 
-    [[nodiscard]] size_t getBufferSize(BufferId id) const noexcept;
+    [[nodiscard]] char atOffset(const BufferId id, const Offset offset) const noexcept { return m_buffers[id][offset]; }
 
-    [[nodiscard]] char atOffset(BufferId id, Offset offset) const noexcept;
+    [[nodiscard]] std::string_view getBufferContents(const BufferId id) const noexcept
+    {
+      return m_buffers[id].getContents();
+    }
 
     // Location utilities.
     [[nodiscard]] RowNumber getRow(BufferId id, Offset offset) const noexcept;
@@ -39,9 +42,9 @@ namespace lbd::source
   private:
     [[nodiscard]] BufferId addBuffer(std::string name, std::string contents) noexcept;
 
-    std::deque<Buffer> buffers;
-    std::unordered_map<std::string, BufferId> nameToBufferIdMap;
+    std::deque<Buffer> m_buffers;
+    std::unordered_map<std::string, BufferId> m_bufferToIdMap;
   };
 
-  std::string getAbsolutePath(const std::string &path) noexcept;
-}
+  [[nodiscard]] std::string getAbsolutePath(const std::string &path) noexcept;
+}

src/Driver.cpp

@@ -1,8 +1,8 @@
 #include <iostream>
 #include <lbd/Driver.hpp>
+#include <lbd/frontend/Program.hpp>
 #include <lbd/frontend/lexer/Lexer.hpp>
 #include <lbd/frontend/parser/Parser.hpp>
-#include <lbd/frontend/Program.hpp>
 #include <lbd/runtime/Interpreter.hpp>
 
 namespace lbd

src/frontend/lexer/Lexer.cpp

@@ -1,5 +1,3 @@
-#include <cctype>
-#include <fstream>
 #include <lbd/diagnostics/ContextGuard.hpp>
 #include <lbd/frontend/lexer/Lexer.hpp>
 #include <lbd/source/Range.hpp>
@@ -50,13 +48,13 @@ namespace lbd::frontend::lexer
   {
     while (std::isspace(static_cast<unsigned char>(getCurrentCharacter()))) advanceCursor();
 
-    const source::Location beginLocation{buffer.id, cursor};
+    const source::Location beginLocation{buffer.getId(), cursor};
 
     // Adding Context Guard for diagnostics.
     diagnostics::ContextGuard contextGuard(
       context.getDiagnosticEmitter(),
-      source::Range(beginLocation, {buffer.id, static_cast<source::Offset>(buffer.getSize())}),
-      fmt::format("While lexing `{}`.", buffer.name));
+      source::Range(beginLocation, {buffer.getId(), static_cast<source::Offset>(buffer.getSize())}),
+      fmt::format("While lexing `{}`.", buffer.getName()));
 
     if (isEof()) return {token::TokenKind::END_OF_FILE, "", {beginLocation, beginLocation}};
 
@@ -65,8 +63,9 @@ namespace lbd::frontend::lexer
     {
       const source::Offset beginOffset = cursor;
       while (!isEof() && isIdentifier(getCurrentCharacter())) advanceCursor();
-      const auto lexeme = buffer.contents.substr(beginOffset, cursor - beginOffset);
-      return {token::TokenKind::IDENTIFIER, lexeme, {beginLocation, {buffer.id, cursor}}};
+      // TODO: Relook whether Lexer should create a copy of lexeme or refer to buffer.
+      const auto lexeme = std::string(buffer.getContents().substr(beginOffset, cursor - beginOffset));
+      return {token::TokenKind::IDENTIFIER, lexeme, {beginLocation, {buffer.getId(), cursor}}};
     }
 
     // Number.
@@ -79,7 +78,7 @@ namespace lbd::frontend::lexer
       if (getCurrentCharacter() == '-' && !std::isdigit(peekNextCurrentCharacter()))
       {
         context.getDiagnosticEmitter().error(
-          {beginLocation, {buffer.id, cursor}},
+          {beginLocation, {buffer.getId(), cursor}},
           diagnostics::DiagnosticId::LEXER_UNEXPECTED_CHARACTER, '-'
         );
       }
@@ -91,8 +90,8 @@ namespace lbd::frontend::lexer
         advanceCursor(); // Consume '.'
         while (!isEof() && std::isdigit(getCurrentCharacter())) advanceCursor();
       }
-      const auto lexeme = buffer.contents.substr(beginOffset, cursor - beginOffset);
-      return {token::TokenKind::NUMBER, lexeme, {beginLocation, {buffer.id, cursor}}};
+      const auto lexeme = std::string(buffer.getContents().substr(beginOffset, cursor - beginOffset));
+      return {token::TokenKind::NUMBER, lexeme, {beginLocation, {buffer.getId(), cursor}}};
     }
 
     // String literal.
@@ -104,14 +103,14 @@ namespace lbd::frontend::lexer
       if (getCurrentCharacter() != '"')
       {
         context.getDiagnosticEmitter().error(
-          {beginLocation, {buffer.id, cursor}},
+          {beginLocation, {buffer.getId(), cursor}},
           diagnostics::DiagnosticId::LEXER_UNBALANCED_QUOTE
         );
       }
-      advanceCursor(); // Consume '"'
+      advanceCursor(); // Consume double-quote.
       // +1 and -1 for excluding the quotes.
-      const auto lexeme = buffer.contents.substr(beginOffset + 1, cursor - beginOffset - 2);
-      return {token::TokenKind::STRING, lexeme, {beginLocation, {buffer.id, cursor}}};
+      const auto lexeme = std::string(buffer.getContents().substr(beginOffset + 1, cursor - beginOffset - 2));
+      return {token::TokenKind::STRING, lexeme, {beginLocation, {buffer.getId(), cursor}}};
     }
 
   LexSymbol:
@@ -128,7 +127,7 @@ namespace lbd::frontend::lexer
       {
         advanceCursor();
         const auto lexeme = std::string(1, symbol);
-        return {symbolToTokenKind(symbol), lexeme, {beginLocation, {buffer.id, cursor}}};
+        return {symbolToTokenKind(symbol), lexeme, {beginLocation, {buffer.getId(), cursor}}};
       }
       case '=':
       {
@@ -137,9 +136,9 @@ namespace lbd::frontend::lexer
         {
           // FAT_ARROW
           advanceCursor(); // Consume '>'
-          return {token::TokenKind::FAT_ARROW, "=>", {beginLocation, {buffer.id, cursor}}};
+          return {token::TokenKind::FAT_ARROW, "=>", {beginLocation, {buffer.getId(), cursor}}};
         }
-        return {token::TokenKind::ASSIGNMENT, "=", {beginLocation, {buffer.id, cursor}}};
+        return {token::TokenKind::ASSIGNMENT, "=", {beginLocation, {buffer.getId(), cursor}}};
       }
       case '-':
       {
@@ -154,7 +153,7 @@ namespace lbd::frontend::lexer
           {
             // Arrow.
             advanceCursor(); // Consume '>'.
-            return {token::TokenKind::ARROW, "->", {beginLocation, {buffer.id, cursor}}};
+            return {token::TokenKind::ARROW, "->", {beginLocation, {buffer.getId(), cursor}}};
           }
           default:
             break;
@@ -164,7 +163,7 @@ namespace lbd::frontend::lexer
         break;
     }
     context.getDiagnosticEmitter().error(
-      {beginLocation, {buffer.id, cursor}},
+      {beginLocation, {buffer.getId(), cursor}},
       diagnostics::DiagnosticId::LEXER_UNEXPECTED_CHARACTER, getCurrentCharacter()
     );
   }
@@ -205,11 +204,11 @@ namespace lbd::frontend::lexer
       case '(': return token::TokenKind::OPEN_PARENTHESIS;
       case ')': return token::TokenKind::CLOSE_PARENTHESIS;
       default:
-        const source::Location location{buffer.id, cursor};
+        const source::Location location{buffer.getId(), cursor};
         context.getDiagnosticEmitter().error(
           {location, location},
           diagnostics::DiagnosticId::LEXER_UNEXPECTED_CHARACTER, symbol
         );
     }
   }
-}
+}

src/frontend/parser/Parser.cpp

@@ -1,4 +1,5 @@
 #include <filesystem>
+// ReSharper disable once CppUnusedIncludeDirective
 #include <fmt/core.h>
 #include <lbd/diagnostics/ContextGuard.hpp>
 #include <lbd/frontend/ast/expression/FunctionApplicationExpression.hpp>
@@ -23,7 +24,7 @@ namespace lbd::frontend::parser
     diagnostics::ContextGuard contextGuard(
       context.getDiagnosticEmitter(),
       lexer.peek().range,
-      fmt::format("While parsing `{}`.", lexer.getBuffer().name));
+      fmt::format("While parsing `{}`.", lexer.getBuffer().getName()));
 
     while (lexer.hasNext())
     {

src/source/Buffer.cpp

@@ -2,16 +2,19 @@
 
 namespace lbd::source
 {
-  size_t Buffer::getSize() const { return contents.size(); }
-  char Buffer::operator[](const size_t index) const { return contents[index]; }
+  Buffer::Buffer(const BufferId id, std::string name, std::string contents)
+    : m_id(id), m_name(std::move(name)), m_contents(std::move(contents))
+  {
+    computeLineOffsets();
+  }
 
   void Buffer::computeLineOffsets() noexcept
   {
-    lineOffsets.clear();
-    lineOffsets.push_back(0);
-    for (uint32_t i = 0; i < contents.size(); ++i)
+    m_lineOffsets.clear();
+    m_lineOffsets.push_back(0);
+    for (uint32_t i = 0; i < m_contents.size(); ++i)
     {
-      if (contents[i] == '\n') lineOffsets.push_back(i + 1);
+      if (m_contents[i] == '\n') m_lineOffsets.push_back(i + 1);
     }
   }
-}
+}

src/source/BufferManager.cpp

@@ -8,13 +8,15 @@ namespace lbd::source
   BufferId BufferManager::loadFile(const std::string &path) noexcept
   {
     const std::string absolutePath = getAbsolutePath(path);
-    if (nameToBufferIdMap.contains(absolutePath)) return nameToBufferIdMap[absolutePath];
+    if (const auto it = m_bufferToIdMap.find(absolutePath); it != m_bufferToIdMap.end()) return it->second;
 
+    // TODO: Read file in binary mode, as it may translate the line-endings to LF, making offset incorrect.
     std::ifstream file(absolutePath);
+    // TODO: emit diagnostic / return invalid BufferId, upon failure to open a file.
     std::string contents((std::istreambuf_iterator(file)), std::istreambuf_iterator<char>());
 
     const BufferId id = addBuffer(absolutePath, std::move(contents));
-    nameToBufferIdMap[absolutePath] = id;
+    m_bufferToIdMap.emplace(absolutePath, id);
     return id;
   }
 
@@ -23,53 +25,51 @@ namespace lbd::source
     return addBuffer(std::move(name), std::move(contents));
   }
 
-  const Buffer &BufferManager::getBuffer(const BufferId id) const noexcept { return buffers[id]; }
-  std::string_view BufferManager::getBufferName(const BufferId id) const noexcept { return buffers[id].name; }
-  std::string_view BufferManager::getBufferContents(const BufferId id) const noexcept { return buffers[id].contents; }
-  size_t BufferManager::getBufferSize(const BufferId id) const noexcept { return buffers[id].getSize(); }
-  char BufferManager::atOffset(const BufferId id, const Offset offset) const noexcept { return buffers[id][offset]; }
-
   RowNumber BufferManager::getRow(const BufferId id, const Offset offset) const noexcept
   {
-    const auto &lines = buffers[id].lineOffsets;
+    const auto &lines = m_buffers[id].getLineOffsets();
     const auto it = std::ranges::upper_bound(lines, offset);
     return static_cast<RowNumber>(it - lines.begin());
   }
 
   ColumnNumber BufferManager::getColumn(const BufferId id, const Offset offset) const noexcept
   {
-    const auto &lines = buffers[id].lineOffsets;
+    const auto &lines = m_buffers[id].getLineOffsets();
     const auto it = std::ranges::upper_bound(lines, offset);
     const RowNumber row = static_cast<RowNumber>(it - lines.begin());
     return offset - lines[row - 1] + 1;
   }
 
   std::string_view BufferManager::getLine(const BufferId id, const Offset offset) const noexcept
   {
-    const auto &buffer = buffers[id];
-    const auto &lines = buffer.lineOffsets;
+    const auto &buffer = getBuffer(id);
+    const auto &lines = buffer.getLineOffsets();
 
     const auto it = std::ranges::upper_bound(lines, offset);
-    const size_t row = it - lines.begin();
+    const size_t row = static_cast<size_t>(it - lines.begin());
     const size_t lineStart = lines[row - 1];
-    const size_t lineEnd = row < lines.size() ? lines[row] - 1 : buffer.contents.size();
+    // TODO: Relook whether this works for Windows CRLF (may need to subtract 2 in that case.)
+    const size_t lineEnd = row < lines.size() ? lines[row] - 1 : buffer.getSize();
 
-    return {buffer.contents.data() + lineStart, lineEnd - lineStart};
+    return buffer.getContents().substr(lineStart, lineEnd - lineStart);
   }
 
   bool BufferManager::isBufferLoaded(const std::string &name) const noexcept
   {
     const std::string absolutePath = getAbsolutePath(name);
-    return nameToBufferIdMap.contains(absolutePath);
+    return m_bufferToIdMap.contains(absolutePath);
   }
 
   BufferId BufferManager::addBuffer(std::string name, std::string contents) noexcept
   {
-    const auto id = static_cast<BufferId>(buffers.size());
-    buffers.push_back(Buffer{id, std::move(name), std::move(contents)});
-    buffers.back().computeLineOffsets();
+    const auto id = static_cast<BufferId>(m_buffers.size());
+    m_buffers.emplace_back(id, std::move(name), std::move(contents));
     return id;
   }
 
-  std::string getAbsolutePath(const std::string &path) noexcept { return std::filesystem::absolute(path).string(); }
-}
+  std::string getAbsolutePath(const std::string &path) noexcept
+  {
+    // TODO: Handle thrown exception.
+    return std::filesystem::weakly_canonical(path).string();
+  }
+}