This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Detect dependent initializer_lists in google-explicit-constructor.
ClosedPublic

Authored by alexfh on Feb 5 2015, 4:11 AM.

Diff Detail

Event Timeline

alexfh updated this revision to Diff 19395.Feb 5 2015, 4:11 AM
alexfh retitled this revision from to [clang-tidy] Detect dependent initializer_lists in google-explicit-constructor..
alexfh updated this object.
alexfh edited the test plan for this revision. (Show Details)
alexfh added a reviewer: djasper.
alexfh added a subscriber: Unknown Object (MLST).
djasper accepted this revision.Feb 5 2015, 4:18 AM
djasper edited edge metadata.

Looks good.

clang-tidy/google/ExplicitConstructorCheck.cpp
58–59

I am fine with removing the extra-check here as I don't think it'll lead to a significant amount of false negatives, but please mention that in the patch description.

test/clang-tidy/google-explicit-constructor.cpp
66

I recommend not checking the full error message every time. It is just too much churn if the warning gets changed.

This revision is now accepted and ready to land.Feb 5 2015, 4:18 AM
alexfh updated this revision to Diff 19397.Feb 5 2015, 4:48 AM
alexfh edited edge metadata.

Addressed review comments.

clang-tidy/google/ExplicitConstructorCheck.cpp
58–59

The main reason to remove the qualified name comparison was to make the check consistent with the newly added handling of instantiation-dependent argument types (where I was lazy to check qualified name).

After thinking a bit, I'll better make both consistent in a different way. PTAL.

test/clang-tidy/google-explicit-constructor.cpp
66

You're right. I removed the check name, but I can make the patterns even shorter.

alexfh closed this revision.Feb 5 2015, 4:50 AM

Still looks good.