[PATCH] Add clang-tidy check to remove redundant .get() calls on smart pointers.

[Samuel Benzaquen]

================
Comment at: clang-tidy/misc/RedundantSmartptrGet.cpp:81
@@ +80,3 @@
+      *Result.SourceManager, LangOptions());
+  // Replace *foo->get() with **foo, and foo.get() with foo
+  const std::string Replacement =
----------------
Alexander Kornienko wrote:
> nit: Please add "." in the end of the comment.
Done.

================
Comment at: test/clang-tidy/redundant-smartptr-get.cpp:76
@@ +75,2 @@
+
+// CHECK-NOT: warning
----------------
Alexander Kornienko wrote:
> nit: This one is also redundant.
Done.

================
Comment at: test/clang-tidy/redundant-smartptr-get.cpp:44
@@ +43,3 @@
+  std::unique_ptr<Bar>().get()->Do();
+  // CHECK: warning: Redundant get() call on smart pointer. [misc-redundant-smartptr-get]
+
----------------
Alexander Kornienko wrote:
> Samuel Benzaquen wrote:
> > Alexander Kornienko wrote:
> > > Samuel Benzaquen wrote:
> > > > Alexander Kornienko wrote:
> > > > > Maybe add line and column numbers to make the tests more precise?
> > > > > // CHECK: :[[@LINE-1]]:<column number>: warning: ...
> > > > > 
> > > > > I'd also leave the whole error message only once, and abbreviate all other occasions so that they don't exceed the column limit and thus are easier to read.
> > > > Done.
> > > I meant adding the line numbers for all checks. This way we can ensure the warnings are issued on correct lines. Currently, for example, a warning may be issued on the line 66 instead of line 60, but the test will still pass (or even all 5 warning checks without line numbers may match to warnings generated from your negative tests, and the test will still pass).
> > > 
> > > The lit+FileCheck system is rather primitive. CHECK lines are processed in order, but without any reference to the surrounding text, so it's important to make CHECK lines unique (e.g. by specifying line numbers).
> > > 
> > > Also, specifying many CHECK-NOT lines one after another makes little sense. It may seem to be good for documentation purposes, but it's not how FileCheck works, so it's misleading. If you have all your negative test cases in the end of the file, it's better to use one "// CHECK-NOT: warning" to assert that there are no warnings expected in this part (the part of the output after the line matched by the last CHECK: that is). You can put CHECK-NOT before this section for documentation purposes, but it doesn't matter for FileCheck.
> > > 
> > > I know, it's rather confusing, so there are plans to implement an analogue of clang's -verify option for clang-tidy. But while it's not here, we should deal with FileCheck's quirks.
> > Done.
> > Would it make sense to add a little more of the output in the check?
> > Each warning also contains a few more lines that print the before and after of the fix.
> Good idea. It would overlap with your fix-it tests, but that doesn't harm, I guess.
Done. Adding those lines will make it hard for errors to match in the wrong place now.


http://llvm-reviews.chandlerc.com/D3186

BRANCH
  master

ARCANIST PROJECT
  clang-tools-extra