Update gsl-lite dependency to v1.0 (#264)

* Update gsl-lite dependency to v1.0

* `gsl_lite::span<>::iterator` is not a pointer anymore

* `gsl_lite::span<>` does not define comparison operators by default

The least invasive fix would be to re-enable span comparison operators by defining `gsl_CONFIG_ALLOWS_SPAN_COMPARISON=1`. However, comparison operators are no longer provided by default for good reason (unclear semantics), and C++20 `std::span<>` doesn't have comparison operators either, so this appears to be the most forward-looking way to handle it.

* Bump version number of package requirement

* Remove unnecessary build switch

The gsl-lite build script is well-behaved an d does not build tests by default.

* Avoid warning about parameter shadowing

Author: mbeutel

.ci/fetch_system_dependencies.sh

@@ -1,17 +1,17 @@
-#!/bin/bash
+#!/bin/bash
 if [ -z ${EXTERNAL_ROOT+x} ]; then echo EXTERNAL_ROOT not set; exit 1; fi
 
 export EXTERNAL_BUILD_ROOT=$HOME/external_build
 
 mkdir "$EXTERNAL_BUILD_ROOT" || true
 
 # Install gsl-lite dependency
-if [ ! -f "$EXTERNAL_ROOT/include/gsl/gsl-lite.hpp" ]; then
+if [ ! -f "$EXTERNAL_ROOT/include/gsl-lite/gsl-lite.hpp" ]; then
     cd "$EXTERNAL_BUILD_ROOT";
-    wget https://github.com/gsl-lite/gsl-lite/archive/refs/tags/v0.38.1.tar.gz;
-    tar -xzf v0.38.1.tar.gz;
-    cd gsl-lite-0.38.1;
-    cmake -DCMAKE_INSTALL_PREFIX="$EXTERNAL_ROOT" -DGSL_LITE_OPT_BUILD_TESTS=Off .;
+    wget https://github.com/gsl-lite/gsl-lite/archive/refs/tags/v1.0.1.tar.gz;
+    tar -xzf v1.0.1.tar.gz;
+    cd gsl-lite-1.0.1;
+    cmake -DCMAKE_INSTALL_PREFIX="$EXTERNAL_ROOT" .;
     make -j2 && make install;
 fi
 

CMakeLists.txt

@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.12 FATAL_ERROR)
+cmake_minimum_required(VERSION 3.12 FATAL_ERROR)
 cmake_policy(VERSION 3.13)
 
 option(TELNETPP_WITH_ZLIB "Build using ZLib" False)
@@ -51,8 +51,8 @@ endif()
 # Telnet++ requires at least Boost 1.69.
 find_package(Boost 1.69.0 REQUIRED)
 
-# Telnet++ requires at least version 0.38 of gsl-lite.
-find_package(gsl-lite 0.38.0 REQUIRED)
+# Telnet++ requires at least version 1.0.1 of gsl-lite.
+find_package(gsl-lite 1.0.1 REQUIRED)
 
 # If we are building with ZLib, then we require the ZLib library
 if (${TELNETPP_WITH_ZLIB})
@@ -168,7 +168,7 @@ target_sources(telnetpp
 target_link_libraries(telnetpp
     PUBLIC
         Boost::boost
-        gsl::gsl-lite
+        gsl-lite::gsl-lite
 )
 
 # The zlib compressors for MCCP should only be compiled into the library

README.md

@@ -17,13 +17,13 @@ Telnet++ is an implementation of the Telnet Session Layer protocol that is used
 
 Telnet++ requires a C++17 compiler and the following libraries:
   * Boost (At least version 1.69.0)
-  * GSL-lite (At least version 1.38)
+  * gsl-lite (At least version 1.0)
   * (Optionally) ZLib
   * (For testing only) Google Test
 
 # Installation - CMake
 
-Telnet++ can be installed from source using CMake.  This requires Boost, GSL-Lite and any other dependencies to have been installed beforehand, using their own instructions, or for the call to `cmake --configure` to be adjusted appropriately (e.g. `-DBOOST_ROOT=...` or `-Dgsl-lite_DIR=...`).  If you do not wish to install into a system directory, and thus avoid the use of sudo, you can also pass `-DCMAKE_INSTALL_PREFIX=...` into the `cmake --configure` call.
+Telnet++ can be installed from source using CMake.  This requires Boost, gsl-lite and any other dependencies to have been installed beforehand, using their own instructions, or for the call to `cmake --configure` to be adjusted appropriately (e.g. `-DBOOST_ROOT=...` or `-Dgsl-lite_DIR=...`).  If you do not wish to install into a system directory, and thus avoid the use of sudo, you can also pass `-DCMAKE_INSTALL_PREFIX=...` into the `cmake --configure` call.
 
     git clone https://github.com/KazDragon/telnetpp.git && cd telnetpp
     mkdir build && cd build

include/telnetpp/core.hpp

@@ -1,14 +1,17 @@
-#pragma once
+#pragma once
 
 #include "telnetpp/detail/export.hpp"  // IWYU pragma: export
 
-#include <gsl/gsl-lite.hpp>
+#include <gsl-lite/gsl-lite.hpp>
 
 #include <string>
 #include <cstdint>
+#include <algorithm>
 
 namespace telnetpp {
 
+namespace gsl = ::gsl_lite;
+
 using byte = std::uint8_t;
 using option_type = std::uint8_t;
 using command_type = std::uint8_t;
@@ -44,6 +47,17 @@ using bytes = gsl::span<byte const>;
 // string optimization, meaning that most cases will not cause an allocation.
 using byte_storage = std::basic_string<byte>;
 
+// Comparison function for bytes.
+// gsl::span<>, like std::span<> in C++20, does not define comparison
+// operators by default because the semantics are unclear (deep or shallow?).
+// This named comparison function is provided because we cannot define a
+// useful operator== because ADL would not find it in our namespace.
+constexpr inline auto bytes_equal = [](
+    bytes const &lhs, bytes const &rhs) noexcept
+{
+    return std::equal(lhs.begin(), lhs.end(), rhs.begin(), rhs.end());
+};
+
 namespace literals {
 
 // A simple function to convert from string literals to stored bytes.

include/telnetpp/element.hpp

@@ -1,4 +1,4 @@
-#pragma once
+#pragma once
 
 #include "telnetpp/command.hpp"  // IWYU pragma: export
 #include "telnetpp/core.hpp"
@@ -18,6 +18,28 @@ namespace telnetpp {
 //* =========================================================================
 using element = std::variant<bytes, negotiation, subnegotiation, command>;
 
+//* =========================================================================
+/// \brief Equality comparison operator for a telnetpp::element.
+//* =========================================================================
+gsl_constexpr20 inline bool operator==(
+    element const &lhs, element const &rhs) noexcept
+{
+    auto visitor = [&rhs](auto const& _lhs) noexcept
+    {
+        using T = gsl::std20::remove_cvref_t<decltype(_lhs)>;
+        if constexpr (std::is_same_v<T, bytes>)
+        {
+            return telnetpp::bytes_equal(_lhs, std::get<bytes>(rhs));
+        }
+        else
+        {
+            return _lhs == std::get<T>(rhs);
+        }
+    };
+    if (lhs.index() != rhs.index()) return false;
+    return std::visit(visitor, lhs);
+}
+
 //* =========================================================================
 /// \brief A contiguous range of elements.
 //* =========================================================================

include/telnetpp/options/new_environ/detail/for_each_request.hpp

@@ -1,4 +1,4 @@
-#pragma once
+#pragma once
 
 #include "telnetpp/core.hpp"
 #include "telnetpp/options/new_environ/detail/request_parser_helper.hpp"
@@ -10,8 +10,8 @@ void for_each_request(telnetpp::bytes requests, Continuation &&cont)
 {
     request_parsing_state state;
 
-    auto const *current = requests.begin();
-    auto const *end = requests.end();
+    auto current = requests.begin();
+    auto end = requests.end();
 
     while (current != end)
     {

include/telnetpp/options/new_environ/detail/for_each_response.hpp

@@ -1,4 +1,4 @@
-#pragma once
+#pragma once
 
 #include "telnetpp/options/new_environ/detail/response_parser_helper.hpp"
 
@@ -9,8 +9,8 @@ void for_each_response(telnetpp::bytes content, Continuation &&cont)
 {
     parsing_state state;
 
-    auto const *current = content.begin();
-    auto const *end = content.end();
+    auto current = content.begin();
+    auto end = content.end();
 
     while (current != end)
     {

include/telnetpp/subnegotiation.hpp

@@ -1,4 +1,4 @@
-#pragma once
+#pragma once
 
 #include "telnetpp/core.hpp"
 
@@ -45,11 +45,11 @@ class TELNETPP_EXPORT subnegotiation
 //* =========================================================================
 /// \brief Comparison function for subnegotiations
 //* =========================================================================
-TELNETPP_EXPORT
-constexpr bool operator==(
+gsl_constexpr20 inline bool operator==(
     subnegotiation const &lhs, subnegotiation const &rhs) noexcept
 {
-    return lhs.option() == rhs.option() && lhs.content() == rhs.content();
+    return lhs.option() == rhs.option() &&
+        telnetpp::bytes_equal(lhs.content(), rhs.content());
 }
 
 //* =========================================================================

test/mccp_zlib_compressor_test.cpp

@@ -1,4 +1,4 @@
-#include <boost/range/algorithm/generate.hpp>
+#include <boost/range/algorithm/generate.hpp>
 #include <gtest/gtest.h>
 #include <telnetpp/options/mccp/zlib/compressor.hpp>
 #include <zlib.h>
@@ -110,7 +110,7 @@ TEST_F(a_started_zlib_compressor, compresses_received_data)
 
     auto const expected_data = telnetpp::bytes{test_data};
 
-    ASSERT_EQ(expected_data, output_data);
+    ASSERT_TRUE(telnetpp::bytes_equal(expected_data, output_data));
     ASSERT_FALSE(compression_ended_);
 
     inflateEnd(&stream);
@@ -141,7 +141,7 @@ TEST_F(a_started_zlib_compressor, sends_a_stream_end_when_compression_is_ended)
 
     auto const expected_data = telnetpp::bytes{test_data};
 
-    ASSERT_EQ(expected_data, output_data);
+    ASSERT_TRUE(telnetpp::bytes_equal(expected_data, output_data));
     ASSERT_TRUE(compression_ended_);
 
     inflateEnd(&stream);

test/parser_test.cpp

@@ -1,4 +1,4 @@
-#include <gtest/gtest.h>
+#include <gtest/gtest.h>
 #include <telnetpp/element.hpp>
 #include <telnetpp/parser.hpp>
 
@@ -71,7 +71,7 @@ TEST_F(parser_test, two_normal_characters_parse_to_string)
         static constexpr telnetpp::bytes const expected{expected_values};
 
         auto const actual = std::get<telnetpp::bytes>(elem);
-        ASSERT_EQ(expected, actual);
+        ASSERT_TRUE(telnetpp::bytes_equal(expected, actual));
     });
 
     ASSERT_EQ(size_t{1}, result_.size());
@@ -95,7 +95,7 @@ TEST_F(parser_test, byte_then_iac_emits_byte_only)
         static constexpr telnetpp::bytes const expected{expected_values};
 
         auto const actual = std::get<telnetpp::bytes>(elem);
-        ASSERT_EQ(expected, actual);
+        ASSERT_TRUE(telnetpp::bytes_equal(expected, actual));
     });
 
     ASSERT_EQ(size_t{1}, result_.size());
@@ -110,7 +110,7 @@ TEST_F(parser_test, double_iac_parses_to_iac)
         static constexpr telnetpp::bytes const expected{expected_values};
 
         auto const actual = std::get<telnetpp::bytes>(elem);
-        ASSERT_EQ(expected, actual);
+        ASSERT_TRUE(telnetpp::bytes_equal(expected, actual));
     });
 
     ASSERT_EQ(size_t{1}, result_.size());