[Bug c++/100465] New: Overloading operator+= and including filesystem causes conflicting overload compilation error

[Bug c++/100465] New: Overloading operator+= and including filesystem causes conflicting overload compilation error

[ramkapte at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100465

            Bug ID: 100465
           Summary: Overloading operator+= and including filesystem causes
                    conflicting overload compilation error
           Product: gcc
           Version: 11.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ramkapte at gmail dot com
  Target Milestone: ---

Created attachment 50771
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50771&action=edit
Minimal example which cau

Adding an templated function overload for operator+= and including <filesystem>
causes a compilation error about conflicting overloads. The code works on GCC
versions <= 9.3 but does not work on GCC versions after that. The code works on
MSVC and Clang, so I believe this is a compiler bug. I'm not sure, but I don't
think the internal code of <filesystem> ought to be seeing the overload in the
global namespace.

See attached error.cpp for minimal example.

(Godbolt for error.cpp: https://godbolt.org/z/ccPvqf9K9 )

[Bug c++/100465] Overloading operator+= and including filesystem causes conflicting overload compilation error

[ramkapte at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100465

--- Comment #1 from Ramchandra Apte <ramkapte at gmail dot com> ---
(In reply to Ramchandra Apte from comment #0)
> Created attachment 50771 [details]
> Minimal example
> 
> Adding an templated function overload for operator+= and including
> <filesystem> causes a compilation error about conflicting overloads. The
> code works on GCC versions <= 9.3 but does not work on GCC versions after
> that. The code works on MSVC and Clang, so I believe this is a compiler bug.
> I'm not sure, but I don't think the internal code of <filesystem> ought to
> be seeing the overload in the global namespace.
> 
> See attached error.cpp for minimal example.
> 
> (Godbolt for error.cpp: https://godbolt.org/z/ccPvqf9K9 )

Note that the code works with Clang with libstdc++, so I believe this is a
frontend bug.

[Bug c++/100465] Overloading operator+= and including filesystem causes conflicting overload compilation error

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100465

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Maybe another case of PR 51577 but I haven't looked into it yet.

I will say that a templated operator in the global namespace with absolutely no
constraints to limit what it accepts is a very bad idea.

[Bug c++/100465] Overloading operator+= and including filesystem causes conflicting overload compilation error

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100465

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-05-07
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The difference is this change from g:e7406c364496dae51ef294b5720923fe4a1dfccb

@@ -1082,7 +1085,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 #endif
              if (__add_slash)
                __str += __slash;
-             __str += __elem._M_pathname;
+             __str += basic_string_view<value_type>(__elem._M_pathname);
              __add_slash = __elem._M_type() == _Type::_Filename;
            }
        }

Previously it was adding a std::string to a basic_string<char,
char_traits<char>, Alloc>, which only worked if the Alloc was
std::allocator<char>.

The change to use string_view is required, but results in this error. I'll try
to reduce the testcase.

[Bug c++/100465] Overloading operator+= and including filesystem causes conflicting overload compilation error

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100465

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Reduced:

namespace N
{
template<typename C>
struct string_view
{
  using char_type = C;
};

template<typename C>
struct string
{
  void operator+=(const string&);

  template<typename T, typename = typename T::char_type>
  void operator+=(const T&);
};

template<typename T>
void f()
{
  string<T> s;
  s += string_view<T>();
}

} // namespace N

template<typename T1, typename T2>
void operator+=(T1&, const T2&){}

int main()
{
  N::f<char>();
}

Clang and EDG compile this, GCC doesn't:

100465.cc: In instantiation of ‘void N::f() [with T = char]’:
100465.cc:32:14:   required from here
100465.cc:22:5: error: ambiguous overload for ‘operator+=’ (operand types are
‘N::string<char>’ and ‘N::string_view<char>’)
   22 |   s += string_view<T>();
      |   ~~^~~~~~~~~~~~~~~~~~~
100465.cc:15:8: note: candidate: ‘void N::string<C>::operator+=(const T&) [with
T = N::string_view<char>; <template-parameter-2-2> = char; C = char]’
   15 |   void operator+=(const T&);
      |        ^~~~~~~~
100465.cc:28:6: note: candidate: ‘void operator+=(T1&, const T2&) [with T1 =
N::string<char>; T2 = N::string_view<char>]’
   28 | void operator+=(T1&, const T2&){}
      |      ^~~~~~~~

[Bug c++/100465] Overloading operator+= and including filesystem causes conflicting overload compilation error

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100465

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
As a workaround the library could use __str.append or __str.operator+=

[Bug c++/100465] Overloading operator+= and including filesystem causes conflicting overload compilation error

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100465

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|2021-05-07 00:00:00         |2021-11-25

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #2)
> Maybe another case of PR 51577 but I haven't looked into it yet.

The testcase in comment 4 was fixed by the patch for that bug, r12-702.

The original testcase using <filesystem> still fails though.

Patrick, do you think this is just a dup of PR 51577? Do I need to reduce this
again to something that still fails, or do we have a matching testcase already?

[Bug c++/100465] Overloading operator+= and including filesystem causes conflicting overload compilation error

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100465

--- Comment #7 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #6)
> (In reply to Jonathan Wakely from comment #2)
> > Maybe another case of PR 51577 but I haven't looked into it yet.
> 
> The testcase in comment 4 was fixed by the patch for that bug, r12-702.
> 
> The original testcase using <filesystem> still fails though.
> 
> Patrick, do you think this is just a dup of PR 51577? Do I need to reduce
> this again to something that still fails, or do we have a matching testcase
> already?

r12-702 should have fixed operator lookup in the block scope case, so I'd
expect the original <filesystem> testcase to work since the problematic name
lookup occurs at block scope.  I think the other unresolved testcases
associated with PR51577 all have to do with name lookup at non-block scope, so
this seems to be a distinct bug.

Here's a reduced rejects-valid testcase:

namespace N
{
  struct string
  {
    template<typename T>
    void operator+=(T);
  };

  struct A {
    void operator+=(char); // #1

    template<typename T>
    void f() {
      string s;
      s += T();
    }

    void g() {
      f<char>();
    }
  };
} // namespace N

template<typename T>
void operator+=(N::string, T);

If we comment out the line #1 then it works.

[Bug c++/100465] Overloading operator+= and including filesystem causes conflicting overload compilation error

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100465

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:bb2a7f80a98de3febefbb32b1e4898062bdb6af8

commit r12-6022-gbb2a7f80a98de3febefbb32b1e4898062bdb6af8
Author: Patrick Palka <ppalka@redhat.com>
Date:   Thu Dec 16 13:40:42 2021 -0500

    c++: two-stage name lookup for overloaded operators [PR51577]

    In order to properly implement two-stage name lookup for dependent
    operator expressions, we need to remember the result of unqualified
    lookup of the operator at template definition time, and reuse that
    result rather than performing another unqualified lookup at
    instantiation time.

    Ideally we could just store the lookup in the expression directly, but
    as pointed out in r9-6405 this isn't really possible since we use the
    standard tree codes to represent most dependent operator expressions.

    We could perhaps create a new tree code to represent dependent operator
    expressions, with enough operands to store the lookup along with
    everything else, but that'd require a lot of careful work to make sure
    we handle this new tree code properly across the frontend.

    But currently type-dependent operator (and call) expressions are given
    an empty TREE_TYPE, which dependent_type_p treats as dependent, so this
    field is effectively unused except to signal that the expression is
    type-dependent.  It'd be convenient if we could store the lookup there
    while preserving the dependent-ness of the expression.

    To that end, this patch creates a new kind of type, called
    DEPENDENT_OPERATOR_TYPE, which we give to dependent operator expressions
    and into which we can store the result of operator lookup at template
    definition time (DEPENDENT_OPERATOR_TYPE_SAVED_LOOKUPS).  Since this
    type is always dependent (by definition), and since the frontend doesn't
    seem to care much about the exact type of a type-dependent expression,
    using this type in place of a NULL_TREE type seems to "just work"; only
    dependent_type_p and WILDCARD_TYPE_P need to be adjusted to return true
    for this new type.

    The rest of the patch mostly consists of adding the necessary plumbing
    to pass DEPENDENT_OPERATOR_TYPE_SAVED_LOOKUPS to add_operator_candidates,
    adjusting all callers of build_x_* appropriately, and removing the now
    unnecessary push_operator_bindings mechanism.

    In passing, this patch simplifies finish_constraint_binary_op to avoid
    using build_x_binary_op for building a binary constraint-expr; we don't
    need to consider operator overloads here, as the &&/|| inside a
    constraint effectively always has the built-in meaning (since atomic
    constraints must have bool type).

    This patch also makes FOLD_EXPR_OP yield a tree_code instead of a raw
    INTEGER_CST.

    Finally, this patch adds the XFAILed test operator-8.C which is about
    broken two-stage name lookup for rewritten non-dependent operator
    expressions, an existing bug that's otherwise only documented in
    build_new_op.

            PR c++/51577
            PR c++/83035
            PR c++/100465

    gcc/cp/ChangeLog:

            * call.c (add_operator_candidates): Add lookups parameter.
            Use it to avoid performing a second unqualified lookup when
            instantiating a dependent operator expression.
            (build_new_op): Add lookups parameter and pass it appropriately.
            * constraint.cc (finish_constraint_binary_op): Use
            build_min_nt_loc instead of build_x_binary_op.
            * coroutines.cc (build_co_await): Adjust call to build_new_op.
            * cp-objcp-common.c (cp_common_init_ts): Mark
            DEPENDENT_OPERATOR_TYPE appropriately.
            * cp-tree.def (DEPENDENT_OPERATOR_TYPE): Define.
            * cp-tree.h (WILDCARD_TYPE_P): Accept DEPENDENT_OPERATOR_TYPE.
            (FOLD_EXPR_OP_RAW): New, renamed from ...
            (FOLD_EXPR_OP): ... this.  Change this to return the tree_code
directly.
            (DEPENDENT_OPERATOR_TYPE_SAVED_LOOKUPS): Define.
            (templated_operator_saved_lookups): Define.
            (build_new_op): Add lookups parameter.
            (build_dependent_operator_type): Declare.
            (build_x_indirect_ref): Add lookups parameter.
            (build_x_binary_op): Likewise.
            (build_x_unary_op): Likewise.
            (build_x_compound_expr): Likewise.
            (build_x_modify_expr): Likewise.
            * cxx-pretty-print.c (get_fold_operator): Adjust after
            FOLD_EXPR_OP change.
            * decl.c (start_preparsed_function): Don't call
            push_operator_bindings.
            * decl2.c (grok_array_decl): Adjust calls to build_new_op.
            * method.c (do_one_comp): Likewise.
            (build_comparison_op): Likewise.
            * module.cc (trees_out::type_node): Handle DEPENDENT_OPERATOR_TYPE.
            (trees_in::tree_node): Likewise.
            * name-lookup.c (lookup_name): Revert r11-2876 change.
            (op_unqualified_lookup): Remove.
            (maybe_save_operator_binding): Remove.
            (discard_operator_bindings): Remove.
            (push_operator_bindings): Remove.
            * name-lookup.h (maybe_save_operator_binding): Remove.
            (push_operator_bindings): Remove.
            (discard_operator_bindings): Remove.
            * parser.c (cp_parser_unary_expression): Adjust calls to build_x_*.
            (cp_parser_binary_expression): Likewise.
            (cp_parser_assignment_expression): Likewise.
            (cp_parser_expression): Likewise.
            (do_range_for_auto_deduction): Likewise.
            (cp_convert_range_for): Likewise.
            (cp_parser_perform_range_for_lookup): Likewise.
            (cp_parser_template_argument): Likewise.
            (cp_parser_omp_for_cond): Likewise.
            (cp_parser_omp_for_incr): Likewise.
            (cp_parser_omp_for_loop_init): Likewise.
            (cp_convert_omp_range_for): Likewise.
            (cp_finish_omp_range_for): Likewise.
            * pt.c (fold_expression): Adjust after FOLD_EXPR_OP change. Pass
            templated_operator_saved_lookups to build_x_*.
            (tsubst_omp_for_iterator): Adjust call to build_x_modify_expr.
            (tsubst_expr) <case COMPOUND_EXPR>: Pass
            templated_operator_saved_lookups to build_x_*.
            (tsubst_copy_and_build) <case INDIRECT_REF>: Likewise.
            <case tcc_unary>: Likewise.
            <case tcc_binary>: Likewise.
            <case MODOP_EXPR>: Likewise.
            <case COMPOUND_EXPR>: Likewise.
            (dependent_type_p_r): Return true for DEPENDENT_OPERATOR_TYPE.
            * ptree.c (cxx_print_type): Handle DEPENDENT_OPERATOR_TYPE.
            * semantics.c (finish_increment_expr): Adjust call to
            build_x_unary_op.
            (finish_unary_op_expr): Likewise.
            (handle_omp_for_class_iterator): Adjust calls to build_x_*.
            (finish_omp_cancel): Likewise.
            (finish_unary_fold_expr): Use build_dependent_operator_type.
            (finish_binary_fold_expr): Likewise.
            * tree.c (cp_free_lang_data): Don't call discard_operator_bindings.
            * typeck.c (rationalize_conditional_expr): Adjust call to
            build_x_binary_op.
            (op_unqualified_lookup): Define.
            (build_dependent_operator_type): Define.
            (build_x_indirect_ref): Add lookups parameter and use
            build_dependent_operator_type.
            (build_x_binary_op): Likewise.
            (build_x_array_ref): Likewise.
            (build_x_unary_op): Likewise.
            (build_x_compound_expr_from_list): Adjust call to
            build_x_compound_expr.
            (build_x_compound_expr_from_vec): Likewise.
            (build_x_compound_expr): Add lookups parameter and use
            build_dependent_operator_type.
            (cp_build_modify_expr): Adjust call to build_new_op.
            (build_x_modify_expr): Add lookups parameter and use
            build_dependent_operator_type.
            * typeck2.c (build_x_arrow): Adjust call to build_new_op.

    libcc1/ChangeLog:

            * libcp1plugin.cc (plugin_build_unary_expr): Adjust call to
            build_x_unary_op.
            (plugin_build_binary_expr): Adjust call to build_x_binary_op.

    gcc/testsuite/ChangeLog:

            * g++.dg/lookup/operator-3.C: Split out operator overload
            declarations into ...
            * g++.dg/lookup/operator-3-ops.h: ... here.
            * g++.dg/lookup/operator-3a.C: New test.
            * g++.dg/lookup/operator-4.C: New test.
            * g++.dg/lookup/operator-4a.C: New test.
            * g++.dg/lookup/operator-5.C: New test.
            * g++.dg/lookup/operator-5a.C: New test.
            * g++.dg/lookup/operator-6.C: New test.
            * g++.dg/lookup/operator-7.C: New test.
            * g++.dg/lookup/operator-8.C: New test.

[Bug c++/100465] Overloading operator+= and including filesystem causes conflicting overload compilation error

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100465

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
   Target Milestone|---                         |12.0
             Status|NEW                         |RESOLVED

--- Comment #9 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Fixed for GCC 12.