<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    Hi Richard,<br>
    <br>
    In our use (Persistency of C++ objects, including the
    (meta)description of their data content as well as use the name as
    key to access the class' reflection information at run-time), the
    'context' is indeed always the "at the end of the translation unit"
    (or global scope).<br>
    Thus (for us), <br>
    <br>
    > What is the fully-qualified name of `B::t`'s type? Is it
    `B::TX` or `std::tuple<A::X>` or `std::tuple<::A::X>`?<br>
    <br>
    It indeed either `B::TX` or `std::tuple<A::X>` depending on
    whether the typedefs are being desugared or not.<br>
    <br>
    > from within namespace `B`<br>
    <br>
    Within our API there is no way to actually express this (and I can't
    think of a semantic where it would make sense). Thus by construction
    it is always the "end of the translation unit" case.<br>
    <br>
    > The "ideal interface" idea is a good--and very cool--one, but
    my use case doesn't call for it. <br>
    <br>
    This also our situation. In addition, I do think that supporting the
    general case would increase the implementation complexity.  One
    'simple' adaption is to support also the case with the leading ::
    (i.e. ::std::tuple<::A::X> which would always be safe no
    matter which context you use it).<br>
    <br>
    Cheers,<br>
    Philippe.<br>
    <br>
    <br>
    <blockquote cite="mid:56902327.1060306@cern.ch" type="cite">
      <div class="moz-forward-container"><br>
        <br>
        -------- Forwarded Message --------
        <table class="moz-email-headers-table" border="0"
          cellpadding="0" cellspacing="0">
          <tbody>
            <tr>
              <th align="RIGHT" nowrap="nowrap" valign="BASELINE">Subject:


              </th>
              <td>Re: [PATCH] D15861: Support fully-qualified names for
                all QualTypes</td>
            </tr>
            <tr>
              <th align="RIGHT" nowrap="nowrap" valign="BASELINE">Date:
              </th>
              <td>Fri, 8 Jan 2016 19:50:16 +0000</td>
            </tr>
            <tr>
              <th align="RIGHT" nowrap="nowrap" valign="BASELINE">From:
              </th>
              <td>Sterling Augustine via cfe-commits <a
                  class="moz-txt-link-rfc2396E"
                  href="mailto:cfe-commits@lists.llvm.org"><a class="moz-txt-link-rfc2396E" href="mailto:cfe-commits@lists.llvm.org"><cfe-commits@lists.llvm.org></a></a></td>
            </tr>
            <tr>
              <th align="RIGHT" nowrap="nowrap" valign="BASELINE">Reply-To:


              </th>
              <td><a moz-do-not-send="true"
                  class="moz-txt-link-abbreviated"
                  href="mailto:reviews+D15861+public+e412d660379b225f@reviews.llvm.org">reviews+D15861+public+e412d660379b225f@reviews.llvm.org</a>,
                Sterling Augustine <a moz-do-not-send="true"
                  class="moz-txt-link-rfc2396E"
                  href="mailto:saugustine@google.com"><saugustine@google.com></a></td>
            </tr>
            <tr>
              <th align="RIGHT" nowrap="nowrap" valign="BASELINE">To: </th>
              <td><a moz-do-not-send="true"
                  class="moz-txt-link-abbreviated"
                  href="mailto:saugustine@google.com">saugustine@google.com</a>,
                <a moz-do-not-send="true"
                  class="moz-txt-link-abbreviated"
                  href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a></td>
            </tr>
            <tr>
              <th align="RIGHT" nowrap="nowrap" valign="BASELINE">CC: </th>
              <td><a moz-do-not-send="true"
                  class="moz-txt-link-abbreviated"
                  href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a></td>
            </tr>
          </tbody>
        </table>
        <br>
        <br>
        <pre>saugustine added a comment.

Thanks for the reviews. Please take another look when you get a chance.


================
Comment at: include/clang/Tooling/Core/QualTypeNames.h:32-33
@@ +31,4 @@
+namespace TypeName {
+///\brief Convert the type into one with fully qualified template
+/// arguments.
+///\param[in] QT - the type for which the fully qualified type will be
----------------
rsmith wrote:
> rsmith wrote:
> > Please ensure there's a space between each `/// ` and the content.
> What do you mean by "fully qualified template arguments" here? Let me give you some examples:

>     namespace A {
>       struct X {};
>     }
>     using A::X;
>     namespace B {
>       using std::tuple;
>       typedef typle<X> TX;
>       TX t;
>       struct A { typedef int X; };
>     }

> What is the fully-qualified name of `B::t`'s type? Is it `B::TX` or `std::tuple<A::X>` or `std::tuple<::A::X>`? Note that if you want to redeclare `t` from within namespace `B`, `std::tuple<A::X>` will name the wrong type.


> Why does this only affect template arguments? Its name suggests it should affect the type as a whole (for instance, in the above case it should produce `std::tuple<...>`, not `tuple<...>`).


> Generally, I think this interface needs to specify where the produced names can be used as a name for the specified type, otherwise I don't see how it can ever be reliable. For instance:

> > "Generates a name for a type that can be used to name the same type if used at the end of the current translation unit." (eg, `B::TX` or `std::tuple<X>`)

> or:

> > "Generates a maximally-explicit name for a type that can be used in any context where all the relevant components have been declared. In such a context, this name will either name the intended type or produce an ambiguity error." (eg, `::std::tuple<::A::X>`)

> You should also specify what happens when it's not possible to generate such a name. For instance, given:

>     void f() {
>       struct X {} x;
>     }

> ... there's no way to name the type of `x` from outside `f` (which makes certain refactoring operations impossible unless you also move the definition of `struct X`).


> I think the ideal interface here would allow you to specify a location where you wish to insert the name, and would produce a "best possible" name for that type for that location, avoiding adding explicit qualification / desugaring wherever possible, but the interface should at least take the context-sensitivity of names into account.
My use case is to take a function signature, and communicate to a developer one way to declare the variables they need to call the function.

It does expand entire qualtypes, not just template parameters. (I've updated that description.)

Given the use case, "at the end of the translation unit" is the closest description of where these names would be valid, with the exception that this code avoids relying on any "using" declaration. "using foo::bar; void bat(bar b);", this code would describe foo's parameter as type foo::bar, rather than plain "bar", even though plain "bar" would work at the end of the translation unit.

I have updated the file header's comment to reflect all this, and added a couple of test cases to prove to myself that it does what I have documented. Along the way I have found a couple of places to explicitly mark where one would do things differently if one wanted to change this behavior.

The "ideal interface" idea is a good--and very cool--one, but my use case doesn't call for it.

================
Comment at: include/clang/Tooling/Core/QualTypeNames.h:49-79
@@ +48,33 @@
+
+///\brief Create a NestedNameSpecifier for Namesp and its enclosing
+/// scopes.
+///
+///\param[in] Ctx - the AST Context to be used.
+///\param[in] Namesp - the NamespaceDecl for which a NestedNameSpecifier
+/// is requested.
+clang::NestedNameSpecifier *createNestedNameSpecifier(
+    const clang::ASTContext &Ctx, const clang::NamespaceDecl *Namesp);
+
+///\brief Create a NestedNameSpecifier for TagDecl and its enclosing
+/// scopes.
+///
+///\param[in] Ctx - the AST Context to be used.
+///\param[in] TD - the TagDecl for which a NestedNameSpecifier is
+/// requested.
+///\param[in] FullyQualify - Convert all template arguments into fully
+/// qualified names.
+clang::NestedNameSpecifier *createNestedNameSpecifier(
+    const clang::ASTContext &Ctx, const clang::TagDecl *TD, bool FullyQualify);
+
+///\brief Create a NestedNameSpecifier for TypedefDecl and its enclosing
+/// scopes.
+///
+///\param[in] Ctx - the AST Context to be used.
+///\param[in] TD - the TypedefDecl for which a NestedNameSpecifier is
+/// requested.
+///\param[in] FullyQualify - Convert all template arguments (of possible
+/// parent scopes) into fully qualified names.
+clang::NestedNameSpecifier *createNestedNameSpecifier(
+    const clang::ASTContext &Ctx, const clang::TypedefNameDecl *TD,
+    bool FullyQualify);
+
----------------
rsmith wrote:
> Is it useful to expose the intermediary functionality to generate fully-qualified `QualType`s and `NestedNameSpecifier`s rather than only exposing functionality to generate `string`s? What are the intended uses of these functions?
I believe the Cling project (from which this code is adapted) uses all those functions internally. So I expect at least one client for those functions.

Let me know if you think hiding this interface is still the right thing.

================
Comment at: lib/Tooling/Core/QualTypeNames.cpp:341-343
@@ +340,5 @@
+          llvm::dyn_cast<ElaboratedType>(QT.getTypePtr())) {
+    // Intentionally, we do not care about the other compononent of
+    // the elaborated type (the keyword) as part of the name
+    // normalization is to remove it.
+    Prefix = ETypeInput->getQualifier();
----------------
rsmith wrote:
> This will not work in C, or if the type name is normally shadowed by a variable or function. You sometimes need to keep the keyword.
I've rewritten this code a bit, and this problem is solved near the end of this function. (This section itself is now mostly gone.)


<a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://reviews.llvm.org/D15861">http://reviews.llvm.org/D15861</a>



_______________________________________________
cfe-commits mailing list
<a moz-do-not-send="true" class="moz-txt-link-abbreviated" href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>
<a moz-do-not-send="true" class="moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a>
</pre>
        <br>
      </div>
      <br>
    </blockquote>
    <br>
  </body>
</html>