Bug 51577 - dependent name lookup finds operator in global namespace
Summary: dependent name lookup finds operator in global namespace
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.7.0
: P3 normal
Target Milestone: 12.0
Assignee: Patrick Palka
URL:
Keywords: accepts-invalid, rejects-valid, wrong-code
: 56943 61161 65336 70099 83035 86577 87452 94214 97584 99611 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-12-16 11:59 UTC by Jonathan Wakely
Modified: 2023-09-15 18:04 UTC (History)
17 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-01-13 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Wakely 2011-12-16 11:59:16 UTC
template<typename T, typename U>
bool test( T v1, U v2 )
{
    // g(v1, v2);  // fails
    v1 == v2;
}

namespace A {
    struct X { };
}

namespace B {
    struct Y { };
}

void g(A::X, B::Y) { }

bool operator==(A::X, B::Y) { return true; }


int main()
{
  test( A::X(), B::Y() );
}

This should be rejected, but lookup using associated namespaces finds operator== in the global namespace.  It should only look in namespace A.

If the call uses the function g() instead of an operator, name lookup fails as expected.  If namespace A and the operator== are in an enclosing namespace it also fails, the bug only seems to happen when operator== is in the global namespace.
Comment 1 Jonathan Wakely 2012-01-21 22:53:40 UTC
the problem exists for any operator, it's not specific to ==

here's a reduced form using unary operator+

template<typename T>
void test( T v )
{
    +v;
}

namespace A
{
    struct X { };
}

void operator+(A::X) { }

int main()
{
  test( A::X() );
}
Comment 2 Nathan Ridge 2013-04-13 00:44:58 UTC
*** Bug 56943 has been marked as a duplicate of this bug. ***
Comment 3 Jonathan Wakely 2013-04-13 11:17:55 UTC
The example in PR 56943 gives a wrong-code example
Comment 4 Daniel Krügler 2013-04-13 11:26:25 UTC
(In reply to comment #3)
> The example in PR 56943 gives a wrong-code example

Could you explain why? It looks valid to me. According to my understanding, the free operator+ overload in global namespace is no valid candidate, but the member operator+ of N::A is one.
Comment 5 Jonathan Wakely 2013-04-13 11:52:51 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > The example in PR 56943 gives a wrong-code example
> 
> Could you explain why? It looks valid to me. According to my understanding, the
> free operator+ overload in global namespace is no valid candidate, but the
> member operator+ of N::A is one.

and G++ calls the global one, returning the wrong result, so it's wrong-code
Comment 6 Daniel Krügler 2013-04-13 11:55:36 UTC
(In reply to comment #5)
> and G++ calls the global one, returning the wrong result, so it's wrong-code
OK, I misunderstood the meaning of "wrong-code": I thought that was intended to mean a code that should not compile. I see now that wrong-code is a specific keyword here (never used it before).
Comment 7 Paolo Carlini 2013-04-13 12:10:58 UTC
Essentially in gcc-english at least, wrong code means wrong assembly code, what the back end emits. Normally for wrong C++ code we say invalid code, hard to confuse.
Comment 8 Richard Smith 2014-05-12 23:45:22 UTC
*** Bug 61161 has been marked as a duplicate of this bug. ***
Comment 9 Jonathan Wakely 2015-03-09 10:10:15 UTC
*** Bug 65336 has been marked as a duplicate of this bug. ***
Comment 10 Jonathan Wakely 2015-03-09 10:11:18 UTC
Testcase from PR65336:


extern "C" int puts(const char*);

struct ostream {} cout; 
template<typename T> struct A{ T t; };

struct B{};
struct C : public B{};

ostream& operator<< (ostream& out, const B&) 
  { puts("right"); return out; }

namespace {
template<typename T>
ostream& operator<< (ostream& out, const A<T>&v) 
  { return out << v.t; }

ostream& operator<< (ostream& out, const C&) 
  { puts("wrong"); return out; }
}

int main(){
  A<C> a;
  cout << a;
}
Comment 11 Jonathan Wakely 2015-03-09 10:11:47 UTC
And the testcase from PR 61161:

struct T {
  template<typename T> void f(const T &v) {
    0 << v;
  }
};

namespace N {
  struct X {};
  struct Y : X {};
  void operator<<(int, const X&) {}
}

void operator<<(int, const N::Y&) = delete;

int main() {
  N::Y d;
  T().f(d);
}
Comment 12 Jonathan Wakely 2020-03-18 17:20:34 UTC
*** Bug 94214 has been marked as a duplicate of this bug. ***
Comment 13 Jonathan Wakely 2020-10-27 13:55:55 UTC
*** Bug 97584 has been marked as a duplicate of this bug. ***
Comment 14 Jonathan Wakely 2020-10-27 13:56:25 UTC
*** Bug 86577 has been marked as a duplicate of this bug. ***
Comment 15 Jonathan Wakely 2020-10-27 13:56:59 UTC
*** Bug 70099 has been marked as a duplicate of this bug. ***
Comment 16 Jonathan Wakely 2020-10-27 13:57:31 UTC
*** Bug 83035 has been marked as a duplicate of this bug. ***
Comment 17 Nathan Sidwell 2020-10-27 15:02:22 UTC
ah, the logic to squirrel away lookups on a magic attribute list, records that nothing is found.  But we don't preserve that negative lookup when injecting these lookups into the parameter binding.  So we'll end up finding later-declared namespace-scope entities.
Comment 18 Patrick Palka 2021-05-06 14:12:51 UTC
If the dependent operator call is inside a lambda, e.g. when adjusting the testcase in comment #1 to:

template<typename T>
void test( T v )
{
    [&] { +v; }
}

namespace A
{
    struct X { };
}

void operator+(A::X) { }

int main()
{
  test( A::X() );
}

then GCC 11 correctly rejects the testcase:

51577.C: In instantiation of ‘bool test(T, U) [with T = A::X; U = B::Y]’:
51577.C:23:7:   required from here
51577.C:5:14: error: no match for ‘operator==’ (operand types are ‘A::X’ and ‘B::Y’)
    5 |     [&] { v1 == v2; };
      |           ~~~^~~~~

ever since Nathan's r11-2876.  As this commit mentions, should we enable the maybe_save_operator_binding / push_operator_binding mechanism for all templates?  Something like

--- a/gcc/cp/name-lookup.c                                                                                                                                                                                         
+++ b/gcc/cp/name-lookup.c                                                                                                                                                                                         
@@ -9116,7 +9116,7 @@ static const char *const op_bind_attrname = "operator bindings";                                                                                                                             
 void                                                                                                                                                                                                              
 maybe_save_operator_binding (tree e)                                                                                                                                                                              
 {                                                                                                                                                                                                                 
-  /* This is only useful in a generic lambda.  */                                                                                                                                                                 
+  /* This is only useful in a template.  */                                                                                                                                                                       
   if (!processing_template_decl)                                                                                                                                                                                  
     return;                                                                                                                                                                                                       
                                                                                                                                                                                                                   
@@ -9124,11 +9124,6 @@ maybe_save_operator_binding (tree e)                                                                                                                                                        
   if (!cfn)                                                                                                                                                                                                       
     return;                                                                                                                                                                                                       
                                                                                                                                                                                                                   
-  /* Do this for lambdas and code that will emit a CMI.  In a module's                                                                                                                                            
-     GMF we don't yet know whether there will be a CMI.  */                                                                                                                                                       
-  if (!module_has_cmi_p () && !global_purview_p () && !current_lambda_expr())                                                                                                                                     
-     return;                                                                                                                                                                                                      
-                                                                                                                                                                                                                  
   tree fnname = ovl_op_identifier (false, TREE_CODE (e));                                                                                                                                                         
   if (!fnname)                                                                                                                                                                                                    
     return;                                                                                                                                                                                                       

seems to fix all the related testcases I tried.
Comment 19 Jason Merrill 2021-05-06 18:10:56 UTC
(In reply to Patrick Palka from comment #18)
> ever since Nathan's r11-2876.  As this commit mentions, should we enable the
> maybe_save_operator_binding / push_operator_binding mechanism for all
> templates?

Let's.
Comment 20 CVS Commits 2021-05-11 02:39:44 UTC
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:6ab1176667734bd6de20833f8d263c03a418c452

commit r12-702-g6ab1176667734bd6de20833f8d263c03a418c452
Author: Patrick Palka <ppalka@redhat.com>
Date:   Mon May 10 22:38:34 2021 -0400

    c++: dependent operator expression lookup [PR51577]
    
    This unconditionally enables the maybe_save_operator_binding mechanism
    for all function templates, so that when resolving a dependent operator
    expression from a function template we ignore later-declared
    namespace-scope bindings that weren't visible at template definition
    time.  This patch additionally makes the mechanism apply to dependent
    comma and compound-assignment operator expressions.
    
    Note that this doesn't fix the testcases in PR83035 or PR99692 because
    there the dependent operator expressions aren't at function scope.  I'm
    not sure how adapt this mechanism for these testcases, since although
    we'll in both testcases have a TEMPLATE_DECL to associate the lookup
    result with, at instantiation time we won't have an appropriate binding
    level to push to.
    
    gcc/cp/ChangeLog:
    
            PR c++/51577
            * name-lookup.c (maybe_save_operator_binding): Unconditionally
            enable for all function templates, not just generic lambdas.
            Handle compound-assignment operator expressions.
            * typeck.c (build_x_compound_expr): Call maybe_save_operator_binding
            in the type-dependent case.
            (build_x_modify_expr): Likewise.  Move declaration of 'op' closer
            to its first use.
    
    gcc/testsuite/ChangeLog:
    
            PR c++/51577
            * g++.dg/lookup/operator-3.C: New test.
Comment 21 Patrick Palka 2021-05-20 20:13:50 UTC
*** Bug 99611 has been marked as a duplicate of this bug. ***
Comment 22 CVS Commits 2021-12-16 18:41:02 UTC
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.
Comment 23 Patrick Palka 2021-12-16 18:44:54 UTC
Fixed for GCC 12.
Comment 24 Patrick Palka 2023-09-15 18:04:02 UTC
*** Bug 87452 has been marked as a duplicate of this bug. ***