rsmith added inline comments.
================
Comment at: include/clang/Tooling/Core/QualTypeNames.h:32
@@ +31,3 @@
+namespace TypeName {
+///\brief Convert the type into one with fully qualified template
+/// arguments.
----------------
Please ensure there's a space between each `/// ` and the content.
================
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:
> 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.
================
Comment at: include/clang/Tooling/Core/QualTypeNames.h:37
@@ +36,3 @@
+///\param[in] Ctx - the ASTContext to be used.
+clang::QualType getFullyQualifiedType(clang::QualType QT,
+ const clang::ASTContext &Ctx);
----------------
Remove the `clang::`s; you're already in namespace `clang`.
================
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);
+
----------------
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?
================
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();
----------------
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.
http://reviews.llvm.org/D15861
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
