[PATCH] D44295: [clang-tidy] Detects and fixes calls to grand-...parent virtual methods instead of calls to parent's virtual methods

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis marked 5 inline comments as done.
zinovy.nis added inline comments.


================
Comment at: test/clang-tidy/bugprone-parent-virtual-call.cpp:113
+  int virt_1() override { return A::virt_1(); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: qualified function name A::virt_1 refers to a function not from a direct base class; did you mean 'BI'? [bugprone-parent-virtual-call]
+  // CHECK-FIXES:  int virt_1() override { return BI::virt_1(); }
----------------
aaron.ballman wrote:
> zinovy.nis wrote:
> > aaron.ballman wrote:
> > > zinovy.nis wrote:
> > > > aaron.ballman wrote:
> > > > > This seems like a false positive to me. Yes, the virtual function is technically exposed in `BI`, but why is the programmer obligated to call that one rather than the one from `A`, which is written in the source?
> > > > IMHO it's a matter of safety. Today virt_1() is not overridden in BI, but tomorrow someone will implement BI::virt_1() and it will silently lead to bugs or whatever.
> > > If tomorrow someone implements `BI::virt_1()`, then the check will start diagnosing at that point.
> > Correct, but anyway I don't think it's a problem.
> I'd prefer to see this changed to not diagnose. I don't relish the idea of explaining why that code diagnoses as it stands.
Done. Now the check looks for a most recent class with the overridden method. Please review this change.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44295