This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] Ignore substituted template types in modernize-use-nullptr check.
ClosedPublic

Authored by hokein on Mar 6 2017, 3:38 AM.

Event Timeline

hokein created this revision.Mar 6 2017, 3:38 AM
malcolm.parsons added inline comments.
test/clang-tidy/modernize-use-nullptr.cpp
252

vale -> value

hokein updated this revision to Diff 90677.Mar 6 2017, 4:24 AM
hokein marked an inline comment as done.

typo: vale => value.

xazax.hun added inline comments.
test/clang-tidy/modernize-use-nullptr.cpp
252

It might be great to have a test case for:

template<typename T>
class TemplateClass {
 public:
  explicit TemplateClass(int a, T *default_value = 0) {}
This revision is now accepted and ready to land.Mar 6 2017, 6:29 AM
hokein updated this revision to Diff 90691.Mar 6 2017, 6:43 AM
hokein marked an inline comment as done.

Add more tests.

xazax.hun added inline comments.Mar 6 2017, 6:44 AM
test/clang-tidy/modernize-use-nullptr.cpp
254

Great! Thanks for adding this test. I have the impression we do want to warn and fix this case however. What do you think?

hokein added inline comments.Mar 6 2017, 6:44 AM
test/clang-tidy/modernize-use-nullptr.cpp
252

Good catch. Done.

This revision was automatically updated to reflect the committed changes.
xazax.hun added inline comments.Mar 6 2017, 7:24 AM
clang-tools-extra/trunk/test/clang-tidy/modernize-use-nullptr.cpp
254 ↗(On Diff #90694)

Maybe as a separate patch, but I think it might be worth to warn here. WDYT? (Sorry for the double post, the previous one is disappeared after the commit.)

alexfh added inline comments.Mar 6 2017, 8:21 AM
clang-tools-extra/trunk/test/clang-tidy/modernize-use-nullptr.cpp
254 ↗(On Diff #90694)

Seems reasonable, but probably not an extremely frequent case. Deserves a FIXME at least.

hokein added inline comments.Mar 6 2017, 8:34 AM
clang-tools-extra/trunk/test/clang-tidy/modernize-use-nullptr.cpp
254 ↗(On Diff #90694)

+1, patch are welcome.