This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] fix segfault in cppcore-guidelines-special-member-functions check
ClosedPublic

Authored by jbcoe on Aug 1 2016, 4:09 AM.

Details

Summary

Use a set rather than a vector of defined special member functions so that multiple declarations of the same function are only counted once.

Move some private static member functions into the cpp file.

Run clang-format on header.

Diff Detail

Repository
rL LLVM

Event Timeline

jbcoe updated this revision to Diff 66301.Aug 1 2016, 4:09 AM
jbcoe retitled this revision from to [clang-tidy] fix segfault in cppcore-guidelines-special-member-functions check.
jbcoe updated this object.
jbcoe set the repository for this revision to rL LLVM.
jbcoe added a project: Restricted Project.
aaron.ballman edited edge metadata.Aug 1 2016, 5:23 AM

Please add a test case that would have crashed previously, but this patch corrects. With that test case, this patch will LG.

Eugene.Zelenko added a subscriber: cfe-commits.
jbcoe updated this revision to Diff 66444.Aug 2 2016, 2:32 AM
jbcoe removed rL LLVM as the repository for this revision.

Add test that triggers segfault without fix in place.

aaron.ballman added inline comments.Aug 2 2016, 11:45 AM
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
63–65

I hadn't noticed this before -- why has this been converted to an unrestricted template? If generality is what you were going for, it should have accepted a range (or a pair of iterators) instead?

Prazek accepted this revision.Aug 2 2016, 11:46 AM
Prazek added a reviewer: Prazek.
Prazek added a subscriber: Prazek.

LGTM

This revision is now accepted and ready to land.Aug 2 2016, 11:46 AM
jbcoe added inline comments.Aug 2 2016, 12:58 PM
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
63–65

It needs to handle SmallSetVector which I can't convert to an ArrayRef.

How do I specify a range?

aaron.ballman added inline comments.Aug 2 2016, 1:02 PM
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
63–65
template <typename Iter>
static std::string join(llvm::iterator_range<Iter> R, llvm::StringRef AndOr);

Untested, but you get the idea.

jbcoe updated this revision to Diff 66541.Aug 2 2016, 1:24 PM
jbcoe edited edge metadata.

static join function is no longer a function template.

jbcoe marked an inline comment as done.Aug 2 2016, 1:25 PM
jbcoe added inline comments.
clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
63–64

iterator_range loses me size, empty and index access. There's a function that gets and ArrayRef from a SmallSetVector (I wonder why there's no implicit conversion defined) so I can change the signature to take ArrayRef.

aaron.ballman accepted this revision.Aug 2 2016, 1:30 PM
aaron.ballman edited edge metadata.

LGTM now, thank you for switching back! I was mostly worried about an unrestricted template argument in this case, not the particular form of the change. :-)

This revision was automatically updated to reflect the committed changes.