TypeAliasTemplate and PackExpansion importing capability is needed every time when C++11 std lib is used. Based on NoQ's experimental version.
Details
- Reviewers
NoQ a.sidorin xazax.hun szepet spyffe bruno - Commits
- rG7a91c08414b8: [ASTImporter] TypeAliasTemplate and PackExpansion importing capability
rC318147: [ASTImporter] TypeAliasTemplate and PackExpansion importing capability
rL318147: [ASTImporter] TypeAliasTemplate and PackExpansion importing capability
Diff Detail
- Repository
- rL LLVM
Event Timeline
lib/AST/ASTImporter.cpp | ||
---|---|---|
1525 ↗ | (On Diff #121731) | The type should not be repeated here. Use auto. |
1542 ↗ | (On Diff #121731) | The rest of the ASTImporter does not really follow this idiom (having asserts before most of the early returns. |
unittests/AST/ASTImporterTest.cpp | ||
492 ↗ | (On Diff #121731) | The formatting of the tests does not reflect the LLVM style, extra indent before struct. Same problem bellow. |
Hello Zoltan,
this code looks very familiar to me but it still needs some improvements. You can find my comments inline.
lib/AST/ASTImporter.cpp | ||
---|---|---|
775 ↗ | (On Diff #121731) | We should't assert here: in real code there can be conflicts and the return type can be null. |
1542 ↗ | (On Diff #121731) | This looks like a copy-paste from our code on Github. Yes, we have catched tons of errors with them, but these asserts are not for production code. |
1546 ↗ | (On Diff #121731) | cast<> will assert if import fails. You can use cast_or_null instead. |
1547 ↗ | (On Diff #121731) | Another forgotten assert. |
5257 ↗ | (On Diff #121731) | Another two asserts. |
lib/AST/ASTImporter.cpp | ||
---|---|---|
5254 ↗ | (On Diff #122643) | I think in LLVM most of the time we do not use braces for single statements guarded by conditions. |