rsmith added inline comments.
================ Comment at: clang/lib/Sema/SemaOverload.cpp:9438 // FIXME: Pass in the explicit template arguments? ArgumentDependentLookup(Name, Loc, Args, Fns); ---------------- It would seem preferable to me to do the filtering in `ArgumentDependentLookup` itself rather than here (but I don't feel strongly about it). ================ Comment at: clang/lib/Sema/SemaOverload.cpp:9455-9459 + // Functions in 'std::ranges' are hidden from ADL per [range.iter.ops]/2 and + // [algorithms.requirements]/2. + if ((*I)->isInStdRangesNamespace() && + Name.getNameKind() == DeclarationName::NameKind::Identifier) + continue; ---------------- I worry that this might break some stdlib implementation that finds helper functions via ADL in `std::ranges` somehow. Also, it seems desirable to make this opt-in for the stdlib implementation and indeed for end-user functions not in `std::ranges`. Have you considered enabling this behavior with an attribute instead of by detecting whether the function is in `std::ranges`? ================ Comment at: clang/lib/Sema/SemaOverload.cpp:12837-12841 + // Functions in 'std::ranges' inhibit ADL per [range.iter.ops]/2 and + // [algorithms.requirements]/2. + if (!ULE->decls().empty() && ULE->decls_begin()->isInStdRangesNamespace() && + ULE->getName().getNameKind() == DeclarationName::NameKind::Identifier) + return; ---------------- What should happen if a `using` declaration names one of these things? Should we care about the properties of the underlying declaration (eg, whether the target of the using declaration is in this namespace / has the attribute), or about the found decl (eg, whether the `using` declaration itself is in the namespace / has the attribute)? Depending on the answer, we may need to check all declarations in the `UnresolvedLookupExpr`, not only the first one. For example, we could have an overload set that contains both a user-declared function and a `using` declaration that names a function in `std::ranges` here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129951/new/ https://reviews.llvm.org/D129951 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
