a.sidorin added a comment.
Hello Gabor,
The patch is fine, I have found only some style issues.
================
Comment at: lib/AST/ASTImporter.cpp:4073
+// Returns the definition for a (forwad) declaration of a ClassTemplateDecl, if
+// it has any definition in the redecl chain.
----------------
forward
================
Comment at: lib/AST/ASTImporter.cpp:4121
- // We found a forward declaration but the class to be imported has a
- // definition.
- // FIXME Add this forward declaration to the redeclaration chain.
- if (D->isThisDeclarationADefinition() &&
- !FoundTemplate->isThisDeclarationADefinition())
+ // The class to be imported has a definition.
+ if (D->isThisDeclarationADefinition()) {
----------------
is a definition?
================
Comment at: lib/AST/ASTImporter.cpp:4124
+ // Lookup will find the fwd decl only if that is more recent than the
+ // definition. So, lets try to get the definition if that is
available
+ // in the redecl chain.
----------------
let's; if it is available
================
Comment at: lib/AST/ASTImporter.cpp:4126
+ // in the redecl chain.
+ ClassTemplateDecl* TemplateWithDef = getDefinition(FoundTemplate);
+ if (!TemplateWithDef)
----------------
* is misplaced here.
================
Comment at: unittests/AST/ASTImporterTest.cpp:1550
+ ASTImporterTestBase,
+
ImportDefinitionOfClassTemplateSpecializationIfThereIsAnExistingFwdDeclAndDefinition)
{
+ Decl *ToTU = getToTuDecl(
----------------
Although long names make me happy, this one exceeds 80-char limit. We can
abbreviate "Specialization" to "Spec".
================
Comment at: unittests/AST/DeclMatcher.h:49
+ using UnaryPredicate = std::function<bool(const NodeType *)>;
+ UnaryPredicate predicate;
+ unsigned count = 0;
----------------
Predicate, Count (member names should start with capital).
Repository:
rC Clang
https://reviews.llvm.org/D46950
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
