]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Fix GCC v12 build errors related to ADL in "printing" code (#1215)
authorAlex Rousskov <rousskov@measurement-factory.com>
Thu, 22 Dec 2022 02:12:12 +0000 (02:12 +0000)
committerSquid Anubis <squid-anubis@squid-cache.org>
Fri, 23 Dec 2022 14:58:50 +0000 (14:58 +0000)
This change contains three fixes/adjustments described below.

### Fix Store::SwapMetaView printing

    src/sbuf/Stream.h:66:20: error: no match for 'operator<<' (operand
    types are 'std::basic_ostream<char>' and 'const
    Store::SwapMetaView'): (out << ... << args);

Disclaimer: This explanation omits many details that become important in
some other contexts. Most Argument-Dependent Lookup details are
documented at https://en.cppreference.com/w/cpp/language/adl

    // In some printer.h, we define a printing function for any type A.
    // This is similar to an ToSBuf() declaration in sbuf/Stream.h.
    template <typename A>
    void print(ostream &os, const A &argument) { os << argument; }

    // In some n/t.h, we define how to print objects of type N::T.
    // This is similar to store/SwapMetaView.h.
    operator <<(ostream &os, const N::T &argument);

    // In some caller1.cc, we include both headers and call print():
    #include "printer.h"
    #include "n/t.h"
    void caller1(const N::T &argument) { print(std::cout, argument); }

    // In some caller2.cc, we do the same but change #include order:
    #include "n/t.h"
    #include "printer.h"
    void caller2(const N::T &argument) { print(std::cout, argument); }

When looking at "os << argument", the compiler considers two sets of
argument printing operators, formed by the following two sources:

* The usual unqualified name lookup. This set includes N::T printing
  operator if that operator is declared in global namespace somewhere
  above the print() _template declaration_. In the example above, only
  caller2() will have that printing operator in this set, provided that
  operator is declared in global namespace (as it used to be). None of
  the callers will have that printing operator in this set otherwise.

* ADL. This set is computed from the caller point of view. It includes
  N::T printing operator if that operator is declared inside namespace N
  somewhere above the print() _caller_. In the example above, both
  caller1() and caller2() will have that printing operator in this set,
  provided that operator is declared in namespace N (as it is now). None
  of the callers will have that printing operator in this set otherwise.

For code to compile, one of the sets must contain the printing operator.
Given the above outcomes, there is only one sane solution that allows
any caller to instantiate print() with an N::T argument: The argument
printing operator must be declared inside namaspace N! Declaring in
global namespace would require maintaining certain #include order (that
will cause headaches and, eventually, circular dependencies).

In other words, we must rely on ADL, and that means declaring operators
in the namespace of one of their argument types.

### Fix std::optional<Ip::Address> printing (in src/icap/Xaction.cc)

    src/base/AsyncJobCalls.h:115:61: error: no match for 'operator<<'
    (operand types are 'std::basic_ostream<char>' and 'const
    std::optional<Ip::Address>'):
    void print(std::ostream &os) const { os << '(' << arg1 << ')'; }

In this context, both printing operator argument types are in std, but
ADL also looks at template parameters of argument types (if an argument
type is a template). That recursion adds the Ip namespace to the search.

This is a minimal fix. We should move both Ip::Address printers into
ip/print.h or similar, away from Ip::Address users that print nothing.

### Do not declare an overly general std::optional printer

The removed declaration:

* evidently became unused (after the other changes described above);
* places std::optional<N::T> printers in the wrong namespace (global
  instead of N), where ADL cannot find them;
* exposes all I/O manipulators to a, technically, unrelated
  std::optional interface.

Co-authored-by: Amos Jeffries <squid3@treenet.co.nz>
src/adaptation/icap/Xaction.cc
src/base/IoManip.h
src/store/SwapMetaView.cc
src/store/SwapMetaView.h

index 2fdabcb34dc90d5a35f7b43739edfabcd7199ace..2b7103added9b83e20f98a09e9cd2d6b38f49c63 100644 (file)
@@ -134,6 +134,22 @@ void Adaptation::Icap::Xaction::start()
     Adaptation::Initiate::start();
 }
 
+// TODO: Make reusable by moving this (and the printing operator from
+// ip/Address.h that this code is calling) into ip/print.h or similar.
+namespace Ip {
+
+inline std::ostream &
+operator <<(std::ostream &os, const std::optional<Address> &optional)
+{
+    if (optional.has_value())
+        os << optional.value();
+    else
+        os << "[no IP]";
+    return os;
+}
+
+}
+
 static void
 icapLookupDnsResults(const ipcache_addrs *ia, const Dns::LookupDetails &, void *data)
 {
index 241d01bb1fe2331fe6163099befe1107ca726b41..bb60a8b67323a9821d3a08bd094afa5156662733 100644 (file)
@@ -13,7 +13,6 @@
 
 #include <iostream>
 #include <iomanip>
-#include <optional>
 
 /// Safely prints an object pointed to by the given pointer: [label]<object>
 /// Prints nothing at all if the pointer is nil.
@@ -102,17 +101,5 @@ inline AsHex<Integer> asHex(const Integer n) { return AsHex<Integer>(n); }
 /// Prints the first n data bytes using hex notation. Does nothing if n is 0.
 void PrintHex(std::ostream &, const char *data, size_t n);
 
-/// prints the value stored inside std::optional (if any)
-template <typename Value>
-inline std::ostream &
-operator <<(std::ostream &os, const std::optional<Value> &optional)
-{
-    if (optional.has_value())
-        os << optional.value();
-    else
-        os << "[no value]";
-    return os;
-}
-
 #endif /* SQUID_SRC_BASE_IO_MANIP_H */
 
index 42ca27156950c0ea82ced705b116ef6bbe555038..d9f6954fbc6f8169f554b1fdb47bb54e08134b2b 100644 (file)
@@ -88,7 +88,7 @@ Store::SwapMetaView::checkExpectedLength(const size_t expectedLength) const
 }
 
 std::ostream &
-operator <<(std::ostream &os, const Store::SwapMetaView &meta)
+Store::operator <<(std::ostream &os, const SwapMetaView &meta)
 {
     os << "type=" << int(meta.rawType);
     // XXX: Change Raw constructor to take void* data instead of casting here.
index 8d066a00b3f716480dc63bb8b6f5625b8767df8a..c0afa4a3b20ba769472c9d393d2a3fba991fd916 100644 (file)
@@ -72,10 +72,10 @@ SwapMetaExtract(Item &item, const char * &input, const void *end)
     input += sizeof(item);
 }
 
-} // namespace Store
-
 /// writes a short human-readable summary of the given SwapMetaView object
-std::ostream &operator <<(std::ostream &, const Store::SwapMetaView &);
+std::ostream &operator <<(std::ostream &, const SwapMetaView &);
+
+} // namespace Store
 
 #endif /* SQUID_SRC_STORE_SWAPMETAVIEW_H */