Page MenuHomePhabricator

[clang-tidy] Don't generate fix for argument constructed from std::initializer_list.
ClosedPublic

Authored by hokein on Jan 9 2018, 1:04 AM.

Details

Summary

A follow-up fix of rL311652.

The previous vector in our test is different with std::vector, so
The check still generates fixes for std::vector (`auto p =
std::unique_ptr<Foo>(new Foo({1,2,3}))`) in real world, the patch makes the
vector behavior in test align with std::vector (both AST nodes are the same now).

Diff Detail

Repository
rL LLVM

Event Timeline

hokein created this revision.Jan 9 2018, 1:04 AM
ilya-biryukov accepted this revision.Jan 17 2018, 8:34 AM

LGTM modulo a change to the 'initializer_list.h'. Do we really need it for this patch?

test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
30 ↗(On Diff #129043)

Why do we need to add this destructor in this patch?

This revision is now accepted and ready to land.Jan 17 2018, 8:34 AM
hokein marked an inline comment as done.Jan 17 2018, 9:13 AM
hokein added inline comments.
test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
30 ↗(On Diff #129043)

Yeah, we do need it to reproduce the issue in real world. The AST is different with/without the destructor.

This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.
ilya-biryukov added inline comments.Jan 18 2018, 7:48 AM
test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
30 ↗(On Diff #129043)

Because without destructor CXXBindTemporaryExpr does not show up?

hokein added inline comments.Jan 19 2018, 1:38 AM
test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
30 ↗(On Diff #129043)

Yes.

ilya-biryukov added inline comments.Jan 19 2018, 2:42 AM
test/clang-tidy/Inputs/modernize-smart-ptr/initializer_list.h
30 ↗(On Diff #129043)

Thanks for explaining clang's AST to me :-)