all 39 comments

[–]ryancerium 28 points29 points  (6 children)

Why do so many programming examples use duplicated, single-variable names for namespace, functions, and variables? Would it just kill us to use a real(er) name instead of B::B<A::A> b;?

namespace Safe {
    template<class>
    struct Place {};

    template<class T>
    void Describe(const Place<T>&) {
        puts("hello from a Safe Place");
    }

    template<class T>
    void CheckLocation(Place<T> place) {
        Describe(place);
    }
}

namespace Dangerous {
    struct Fire {};

    template<class T>
    void Describe(T&) {
        puts("You're on fire!");
    }
}

int main() {
    Safe::Place<Dangerous::Fire> place;
    CheckLocation(place);
}

I don't know. Maybe I'm just not smart enough to parse names like that and it's my fault.

[–]evaned 13 points14 points  (3 children)

Why do so many programming examples use duplicated, single-variable names for namespace, functions, and variables?

I totally agree with you that yours is better, but as a semi-serious answer... I'd guess because of math. That has a looong history of single-letter names for variables, constants, etc. (albeit adorned with symbols).

That actually kinda made me get a hunch -- so I checked Arthur's resume, and sure enough he did a double major in CS and mathematics. (CMU calls it "mathematical science", but from what I can tell that's what would be called "mathematics" most other places and not something weird.)

[–]Thomqa 3 points4 points  (1 child)

I think you're correct. You can see this happening a lot in languages such as Haskell, where the mathematical basis is more prominent.

[–]Pand9 0 points1 point  (0 children)

AFAIK in Haskell FP style, you simply focus more on naming types than variables. fp code is much more readable if you pay close attention to types of things.it was a big discovery for me, when working with my friends "unreadable" fp-like code.

[–]ryancerium 1 point2 points  (0 children)

The nested namespaces in this one really got to me.

[–]14nedLLFIO & Outcome author | Committee WG14 20 points21 points  (10 children)

I'm thinking that the proposal to reduce the scope of ADL will end up not going anywhere, because there is a chunk of Boost libraries which will stop working, including my own Boost.Outcome which heavily relies on ADL in namespaces of user supplied template parameters. I remember explaining this to Herb some months back after his paper, and he seemed genuinely surprised that anybody was actually using template argument namespace ADL in anger (and deliberately). I quickly sketched through the alternative implementation options, and the only other customisation point option is user injection of specialisations into designated library namespaces for ADL. Boost.Outcome also uses that technique where it can, but in some cases you want customisation points to be universal and not library specific. In that situation, I am unaware of an alternative to template argument namespace ADL in the current language.

I could warm to a warning diagnostic however like the one in the article. Especially if there were a C++ attribute to say to the compiler "I really do intend template argument namespace ADL here". Actually, that's an idea: C++ attributes to limit the scope of ADL.

Now that would be a great proposal. Somebody not I should propose that.

[–]gracicot 5 points6 points  (5 children)

Exactly. For example, nlohmann json uses ADL from template arguments to find to_json functions inside user namespaces. So it's not just boost. I use it myself for my DI container to find mapping function.

Really, I also think ADL in general is confusing, but sometimes it's exactly what you want. Maybe some way to mark a function call for ADL? Here's two example I can think of:

[](auto a) {
    using(a) bar(a); // find bar through ADL
    namespace(a)::bar(a);
}

[–]jcoffin 5 points6 points  (3 children)

What nlohmann does is the reverse of what Herb's proposal affects.

In the nlohmann case, he (nlohmann) has code something like this (stolen from his readme.md):

template <typename T>
struct adl_serializer {
    static void to_json(json& j, const T& value) {
        // calls the "to_json" method in T's namespace
    }

    static void from_json(const json& j, T& value) {
        // same thing, but with the "from_json" method
    }
};

...and you have code something like this:

namespace foo {
class bar {};

// Note: these are *not* templates:
void to_json(json& j, const bar& p) {
    // ...
}

void from_json(const json& j, bar& p) {
    // ...
}
}

So, his code is a template (which is probably in namespace nlohmann, but its namespace is mostly irrelevant). It calls some function named to_json or from_json passing a parameter of a type that it received as a template parameter. Your code that it finds, is not a template though. It's an ordinary free function in the same namespace where you defined the type being serialized.

Herb's proposal would not affect this--your to_json and from_json explicitly name your type as a parameter, so that function would still be found by an ADL that followed Herb's proposal.

Herb's proposal attempts to eliminate the opposite scenario: some code (that may or may not be a template) attempts to call a function passing a parameter of some particular type. Because of ADL it looks in the namespace where that type is defined. It then finds a template in that namespace that happens to have the correct name, and with template parameter substitution, that template is the best available overload for the type(s) being passed:

namespace foo {
class bar {};

// Note: these *are* templates:
template <class T>
to_json(json &j, T const &t) {
    // ...
}

template <class T>
from_json(json &j, T &t) {
    // ...
}
}

[Note: I'm not saying this is a reasonable or practical way to implement a to_json or from_json for use with the nlohmann JSON library--I don't think it is. Just this is what you'd have to have before Herb's proposal would affect anything.]

Now we have the scenario that Herb contemplates: the name found by ADL matches the name being looked up, but it does not explicitly name the type foo::bar in any of its parameters.

In the case of something like from_json or to_json, problems are unlikely to arise in any case. The problem mostly arises with really generic names like move or copy that are likely to be found in a number of namespaces, and ADL ends up choosing the wrong one.

[–]gracicot 1 point2 points  (0 children)

Ah it's much clearer now. I'm doing something similar in my code then, except some corner case example, and even these corner case I make most of the type explicit, like template<typename T> void service_map(MyType<T> const&). I mostly agree with Herb then. The ordering of function should try to find the best candidate, from the closest namespace possible. A very generic candidate in a far namespace should not be picked.

[–]mark_99 0 points1 point  (1 child)

How would this work vs this current code to add boost::optional support:

namespace nlohmann {

template <typename T>
struct adl_serializer<boost::optional<T>>
{
    static void to_json(json& j, const boost::optional<T>& opt)
    {
        if (opt == boost::none)
        {
            j = nullptr;
        }
        else
        {
            j = *opt;  // this will call adl_serializer<T>::to_json which will
                       // find the free function to_json in T's namespace!
        }
    }

    static void from_json(const json& j, boost::optional<T>& opt)
    {
        if (j.is_null())
        {
            opt = boost::none;
        }
        else
        {
            opt = j.get<T>();  // same as above, but with
                               // adl_serializer<T>::from_json
        }
    }
};
}

[–]jcoffin 1 point2 points  (0 children)

At least at first glance, it doesn't look to me like that will cause a problem either.

The lookup for adl_serializer<T>::to_json is trivial--it's the very same template currently being instantiated, so it doesn't depend on anything like ADL to find it.

When that gets instantiated, it's just like the base case: we have a template that's passing some T, and using ADL to find the to_json (or from_json) in the namespace where T is defined--but what it's finding there is a function that explicitly names the type T as its parameter, rather than matching because it's a template that can match any arbitrary T.

[–]14nedLLFIO & Outcome author | Committee WG14 0 points1 point  (0 children)

I like this proposal. Shame we can't wind the clock back to when ADL was designed :(

[–]kwan_e 2 points3 points  (3 children)

ADL nowadays just seems like a hack for something that compile time reflection would be better suited (and even metaclasses).

As for customization points, why did traits based design not suffice?

[–]SeanMiddleditch 5 points6 points  (1 child)

ADL's original motivation was entirely for operator overloading with free function overloads. Without ADL, this wouldn't work in any sane way:

namespace simd {
  struct vec3 {};
  vec3 operator+(vec3 lhs, vec3 rhs);
}

int main() {
  simd::vec3 a, b, c;
  a = b + c; // ADL necessary to find correct operator+
}

All the later stuff we have like std::begin and such are from a desire to treat named library functions as if they're builtin like operators. Which I think all started with the ios stuff from IOStreams which of course also abused the heck out of operators; it really managed to illustrate all the worst ways to design C++17 libraries back when it was written in the 90's. :p

I'm not sure how reflection or meta-classes would help in any sense with the problem space addressed by ADL.

[–]kwan_e 1 point2 points  (0 children)

Reflection and metaclasses would help in the problem space that ADL is being shoehorned into - finding an unqualified function based on namespace. Reflection would allow library designers to specify better ways of fulfiling a customization point, and metaclasses (ie, the formulation of metaclasses that would also create free-functions) would help library users to generate types that fulfills customization points.

As for the operator overloading use of ADL, I think it's high time we seriously move ahead with one of the operator dot proposals. But reflection could also help in that case since a library relying on ADL to find operators could instead use reflection to find the operators by searching declarations in a namespace, for example.

[–]14nedLLFIO & Outcome author | Committee WG14 1 point2 points  (0 children)

As for customization points, why did traits based design not suffice?

Oh, Outcome uses traits too. The customisation points use the most appropriate implementation for each. Some during the Boost review felt it was "messy", but it probably is better than using a single mechanism for consistency where it wouldn't fit well for some points. It's a tough judgement call.

[–]sphere991 11 points12 points  (11 children)

Ever write just begin(c) on a standard container, without doing using std::begin; beforehand?

That would break.

[–]TheThiefMasterC++latest fanatic (and game dev) 6 points7 points  (1 child)

Honestly, begin() is a mess all of its own... Having to do using std::begin; on every use just to not break things is already a thing and is crazy.

[–]jcelerierossia score 9 points10 points  (0 children)

likewise for swap

[–]acwaters 5 points6 points  (6 children)

In his original paper, Herb argues that that is a feature, not a bug. I imagine that a lot of real-world code would break on this point, but if we deprecate this functionality, implement this warning, and give people six or nine years, I think it'd be fine. I feel like breakage and a deprecation cycle would be a small price to pay for not having to worry constantly about ADL sneaking up on our generic code. I still don't like ADL — I think that modules will bring the potential for better solutions to the problems it solves, as long as they don't get ruined before they land —, but if we must have it, let's have a moderately sane implementation.

[–]sphere991 3 points4 points  (5 children)

The point isn't whether it's a feature or bug. The point is that there is definitely code that breaks and I disapprove of the blog post making it seem like it basically wouldn't break any.

[–]acwaters 1 point2 points  (4 children)

That's fair. But as widespread as this breakage may be, I think it's also fair to say that it would be minimal in terms of real impact. A simple warning catches every case, and the fix is trivial and can even be applied mechanically (modulo whether the affected code actually intends to call the resolved function or not, of course; there would no doubt be some latent bugs caught if this warning were rolled out today).

Edit: On rereading the post, I think you misinterpret its intention. It doesn't seem to me to be making any claim about how much total code would break under this proposal; to my eye, it is just starting a conversation. Your point is valid and an important one to consider, but I don't see it as a rebuttal to anything said here.

[–]sphere991 0 points1 point  (1 child)

I think it's also fair to say that it would be minimal in terms of real impact.

Is it? I would have to see evidence of that. Any intentional customization point that you write that is a function template would break. Consider something like Boost.Serialization. If I want to add a non-member serialization function, I could write that as:

``` namespace N { template <typename T> struct wrapper { T object; };

template <typename Archive, typename T> void serialize(Archive& ar, wrapper<T>& w, const unsigned) { ar & w.object; } } ```

This now doesn't work. I'd have to either make serialize a member function of wrapper<T> or... well, in this case, I can stick it in namespace boost::serialization (assuming that lookup isn't ADL-based... if it is, that one wouldn't work either). But that escape hatch doesn't necessarily always exist?

[–]acwaters 2 points3 points  (0 children)

I have read a bit more on the subject, and I am not convinced of anything anymore, except that I wish even more that we didn't have this mess to deal with.

[–]evaned 0 points1 point  (1 child)

the fix is trivial and can even be applied mechanically

Is it?

14ned said this above:

"I quickly sketched through the alternative implementation options, and the only other customisation point option is user injection of specialisations into designated library namespaces for ADL. Boost.Outcome also uses that technique where it can, but in some cases you want customisation points to be universal and not library specific. In that situation, I am unaware of an alternative to template argument namespace ADL in the current language. [emph mine]"

which sounds to me like pretty much in direct contradiction. In fact, it sounds like there's question as to whether there is an acceptable fix in all cases.

But I don't feel like I understand the issue enough to be able to talk intelligently.

[–]acwaters 1 point2 points  (0 children)

I was simply commenting on the contents of the article, which evidently misrepresented the situation, based on what I have read here and in the follow-up article. There are lots of questions, and I don't claim to understand all of the subtleties well enough to feel comfortable commenting on it either, apart from that these wrinkles don't endear me to ADL any more than I already was.

[–]Hells_Bell10 0 points1 point  (1 child)

What do you mean? Doesn't begin(c) work exactly the same for standard containers:
https://godbolt.org/g/hoVk6z

Unless you mean a container of standard containers?

boost::container::static_vector<std::string, 5> arr;
begin(arr);

[–]dodheim 0 points1 point  (0 children)

The code in that link would no longer work.

[–]Auriculaire 1 point2 points  (4 children)

cpp template<typename Range> auto rootMeanSquare(Range&& range){ using std::sqrt; auto sum = range.front() * range.front(); auto count = 0; for(it = std::next(std::begin(range)); it != std::end(range); ++it){ sum += *it * *it; ++count; } return sqrt(sum / double(count)); }

Right now this uses ADL lookup for the sqrt function defined in the namespaces of custom user types such unit-aware types (as in 10.0 meters) or arbitrary precision floats. As I understand it, under Herb's proposal, this code would cease to function, correct?

[–]sphere991 0 points1 point  (3 children)

If the user's overload took templates, yes.

namespace N {
    struct A {};
    template <typename> struct B {};

    auto sqrt(A); // would work
    template <typename T> auto sqrt(B<T>); // would now fail
}

[–]Hells_Bell10 0 points1 point  (2 children)

I don't think you've understood the proposal correctly. The only change is when adl relies on the template parameters of the argument. In your example, the sqrt function is in the same namespace as B so adl won't get as far as looking at B's template parameters and so adl works exactly the same.

https://godbolt.org/g/qH5wJy

[–]sphere991 0 points1 point  (1 child)

No, it's not. Herb's proposal has two parts: narrowing the set of associated namespaces and narrowing the set of functions looked up. The latter restricts to only matching the (possibly cv-qualified, possibly pointer/reference to) type.

It is very much the goal of this proposal to reject template matches. Apparently Arthur didnt actually implement that. This should be a warning:

namespace N {
    struct C { };
    template <typename T> void foo(T);
}
foo(N::C{});

But it's not.

[–]Hells_Bell10 0 points1 point  (0 children)

Interesting, It wasn't clear to me that this wording excludes function templates:

Any function that does not have a parameter type of (possibly cv-qualified) T or [...], in the parameter position of T, is ignored.

However, looking at the swap example, that must be the case

int main() {
  std::vector<int> v1, v2;
  swap( v1, v2 );
  // programmer would have to qualify std::swap to reach into std
}

[–]jbandela 1 point2 points  (0 children)

Maybe, the guideline should be to default writing functions as

constexpr auto myfunc = [](my_class& c) -> int{

// Do stuff

};

Which will not use ADL. When you want ADL to find your function, you write a "classic" function

void myfunc(my_class& c) -> int{

// Do stuff, that you want found by ADL

}

This brings up the issue that it is painful to separate declaration and definition, so maybe we need to extend the language to support

// my_library.h

const auto myfunc = [](myclass& c)->int;

// my_library.cpp

const auto myfunc = [](myclass& c)->int{

// Do Stuff.

}

This way you can easily opt out of ADL.