[cfe-dev] PATCH: Overloaded function declarations in C++

[Doug Gregor]

Hi Argiris,

On Mon, Sep 8, 2008 at 7:14 PM, Argiris Kirtzidis <akyrtzi at gmail.com> wrote:
> I don't quite see why it's necessary to have an "Overload" type and an
> OverloadedFunctionDecl AST node.

It is extremely convenient, because it allows the semantic analysis to
see the result of name lookup as "a set of overloaded functions",
without requiring every client of name lookup to walk through the list
of declarations bound to that name with this boilerplate:

+    DeclContext *DC = New->getDeclContext();
+    IdentifierResolver::iterator
+      I = IdResolver.begin(New->getIdentifier(), DC, false/*LookInParentCtx*/);
+    for (; I != IdResolver.end(); ++I) {
+      if (!IdResolver.isDeclInScope(*I, DC, S))
+        return true;

> It seems to me that these are mainly used for name lookup, and
> IdentifierResolver can be used for such a thing without introducing
> OverloadedFunctionDecl.
> I've attached a patch where I replace the use of OverloadedFunctionDecl by
> using IdentifierResolver (passes the test case), to show you what I mean.

Yes, OverloadedFunctionDecl is mainly used for name lookup, but the
name lookup is often far from the place where the result of name
lookup is consumed. For example, when parsing

  void f(int, float);
  void f(int, int);

  f(1, 2.0);

The identifier "f" refers to both functions "f" declared above, and
will become a DeclRefExpr in the AST. Later, once we've parsed the
call arguments, ActOnCallExpr will need to see that the function "f"
we're referring to is actually a set of overloaded functions. Name
lookup occurred long before, so we're not going to be walking the
IdentifierResolver's results. Something in the DeclRefExpr needs to
have all of the information about all of the functions named "f" in
this scope.

> AST clients may need a set of overloaded functions but I think that this can
> be provided by utility functions (or by chaining the FunctionDecls).

I don't think chaining the FunctionDecls is the way to go, for two reasons:

  1) It's very error-prone: If the DeclRefExpr points to a
FunctionDecl, a naive client could accidentally look at the
FunctionDecl while forgetting to check whether it has any overloads.
I'm guessing that we'll end up with a lot of places where our
accidental overload-resolution strategy is "grab the most recent
FunctionDecl", because we forgot to walk that chain. Having an
OverloadedFunctionDecl node inside that DeclRefExpr forces clients to
pay attention to overloading, and most of the time they'll just go
perform overload resolution to get it down to a single FunctionDecl.

  2) It's going to interact with function redeclarations.

       void f(int); // #1
       void f();  // #2; overload chain points to #1
       void f(int); // #3; redeclaration of #1. Does the overload
chain in #2 need to be updated? Does this have an overload chain?

    This issue can probably be solved, but it's made redeclarations
that much hairier.

With this patch in isolation, it's a bit hard to see the uses of
OverloadedFunctionDecl. I'll finish up the initial overload-resolution
patch this afternoon/evening and send it out to the list. I hope that
will make the design clearer.

  - Doug