[PATCH] Fix UseAuto not transforming iterators when non-fully qualified names are used and libc++.
Guillaume Papin
guillaume.papin at epitech.eu
Tue Jul 9 10:11:55 PDT 2013
Some errors in the test cases themselves have been fixed but no test cases have been added to really test the `inline namespace`, I would suggest adding one (see comments).
================
Comment at: test/cpp11-migrate/UseAuto/basic_iterator_tests.cpp:23
@@ -22,3 +22,3 @@
// RUN: cpp11-migrate -use-auto %t.cpp -- -DCONTAINER=array \
// RUN: -DUSE_BASE_CLASS_ITERATORS -I %S/Inputs
// RUN: FileCheck -input-file=%t.cpp %s
----------------
Since the `-DUSE_INLINE_NAMESPACE` has been fixed I would fix this one as well `-DUSE_BASE_CLASS_ITERATORS=1` .
================
Comment at: test/cpp11-migrate/UseAuto/basic_iterator_tests.cpp:28
@@ -27,3 +27,3 @@
// RUN: cpp11-migrate -use-auto %t.cpp -- -DCONTAINER=array \
// RUN: -DUSE_INNER_CLASS_ITERATORS -I %S/Inputs
// RUN: FileCheck -input-file=%t.cpp %s
----------------
`-DUSE_INNER_CLASS_ITERATORS=1` as well here.
================
Comment at: test/cpp11-migrate/UseAuto/basic_iterator_tests.cpp:121
@@ -120,3 +120,3 @@
}
return 0;
----------------
I suggest to add this test here:
```
{
using std::CONTAINER;
CONTAINER<int> C;
CONTAINER<int>::iterator I = C.begin();
// CHECK: auto I = C.begin();
}
```
I may be wrong but I don't think these the tests in this file tested the inline namespace bug (when there is no nested name specifier with 'std').
================
Comment at: test/cpp11-migrate/UseAuto/Inputs/test_std_container.h:117
@@ -116,3 +116,3 @@
} // namespace _1
using _1::CONTAINER;
#endif
----------------
This line should be removed. The declarations in an inline namespace are available in its 'outer scope'.
================
Comment at: cpp11-migrate/UseAuto/UseAutoMatchers.cpp:171
@@ +170,3 @@
+/// \c q.
+AST_MATCHER(NestedNameSpecifier, hasCanonicalTypeFromStd) {
+ if (const Type *T = Node.getAsType()) {
----------------
I thought the matcher from ReplaceAutoPtr that deals with std declarations could solve this issue as well, any reason not to use it?
For information here is the matcher:
```
/// \brief Matches declarations whose declaration context is the C++ standard
/// library namespace \c std.
///
/// Note that inline namespaces are silently ignored during the lookup since
/// both libstdc++ and libc++ are known to use them for versioning purposes.
///
/// Given
/// \code
/// namespace ns {
/// struct my_type {};
/// using namespace std;
/// }
///
/// using std::vector;
/// using ns::my_type;
/// using ns::list;
/// \endcode
/// usingDecl(hasAnyUsingShadowDecl(hasTargetDecl(isFromStdNamespace())))
/// matches "using std::vector" and "using ns::list".
AST_MATCHER(Decl, isFromStdNamespace) {
const DeclContext *D = Node.getDeclContext();
while (D->isInlineNamespace())
D = D->getParent();
if (!D->isNamespace() || !D->getParent()->isTranslationUnit())
return false;
const IdentifierInfo *Info = cast<NamespaceDecl>(D)->getIdentifier();
return Info && Info->isStr("std");
}
```
Not tested but I think the matcher `typedefIterator()` could be rewritten for example that way:
```
// \brief Returns a TypeMatcher that matches typedefs for standard iterators
// inside records with a standard container name.
TypeMatcher typedefIterator() {
return typedefType(
hasDeclaration(
allOf(
namedDecl(hasStdIteratorName()),
hasDeclContext(
- recordDecl(hasStdContainerName(true))
+ recordDecl(hasStdContainerName(), isFromStdNamespace())
)
)
)
);
}
```
If for some reason it can't be used as-is nor improved I think the logic can be re-used by the matcher `hasStdContainerName()`, the bool argument `WithStd` can be removed (and then the UseAuto matchers can be refactored a bit).
http://llvm-reviews.chandlerc.com/D1116
More information about the cfe-commits
mailing list