Redecl chains of function template specializations are not handled well
currently. We want to handle them similarly to functions, i.e. try to
keep the structure of the original AST as much as possible. The aim is
to not squash a prototype with a definition, rather we create both and
put them in a redecl chain.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Hi Gabor,
The patch looks almost good bu I have some comments inline.
lib/AST/ASTImporter.cpp | ||
---|---|---|
3002 ↗ | (On Diff #188341) | I like this lambda. To make the code even better, we can move this lambda outside of VisitFunctionDecl because this method is already pretty big. |
unittests/AST/ASTImporterTest.cpp | ||
3862 ↗ | (On Diff #188341) | I don't like numbers. Maybe To0 and To1 are LastDecl and ImportedDecl, correspondingly? |
3881 ↗ | (On Diff #188341) | To0 and From0F actually have the same value, and To0F is unused. |
3884 ↗ | (On Diff #188341) | If I don't miss something, this assumption does not depend on To0 and To1 kind and can be moved out of the condition, to the function scope. |
Alexei, thanks for the review!
lib/AST/ASTImporter.cpp | ||
---|---|---|
3002 ↗ | (On Diff #188341) | Ok, I have created a member function in ASTNodeImporter instead of the lambda. |
unittests/AST/ASTImporterTest.cpp | ||
3862 ↗ | (On Diff #188341) | Ok, I have renamed To0 to Prev and To1 to Current as they should form a redecl chain this way: Prev->Current. |
3881 ↗ | (On Diff #188341) | Good catch. I removed the second variable. |
3884 ↗ | (On Diff #188341) | Yes, thanks for catching this. I have moved this upward right at the beginning of the function. |