This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Do not show incorrect fix in modernize-make-unique
ClosedPublic

Authored by ilya-biryukov on May 7 2019, 8:02 AM.

Event Timeline

ilya-biryukov created this revision.May 7 2019, 8:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 7 2019, 8:02 AM
Herald added a subscriber: xazax.hun. · View Herald Transcript
ilya-biryukov marked an inline comment as done.May 7 2019, 8:23 AM

BTW, for a common use-case we can do the same trick that's being done for aggregate init:

new X({1,2,3}, 123, {a});

into

make_unique<X>(X({1,2,3}, 123, {a}));

I can try fixing this, but would want to land this first to fix the issue at hand.

clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
278

I'm wondering how to properly check that no fixes were produced here.
Do we have a common way to express this?
Would CHECK-FIXES-NOT: ... mentioning the line numbers somehow work?

aaron.ballman marked an inline comment as done.May 7 2019, 9:03 AM
aaron.ballman added inline comments.
clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
301

We can now remove the IgnoreImplicit() call here, can't we?

clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
278

I believe we typically use CHECK-FIXES: <the original code blurb> to ensure that the fix does not change any code.

ilya-biryukov marked 3 inline comments as done.
  • Remove redundant check.
  • Actually check the code stays the same in tests.
clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
301

Right. Thanks for spotting that!

clang-tools-extra/test/clang-tidy/modernize-make-unique.cpp
278

That worked. Thanks!

This revision is now accepted and ready to land.May 7 2019, 11:30 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMay 8 2019, 1:50 AM