[PATCH] D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check

[Piotr Dziwinski via cfe-commits]

piotrdz added a comment.

@alexfh: here we go again. Any comments?


================
Comment at: clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp:289
@@ +288,3 @@
+
+    formatDifferingParamsDiagnostic(FormatParamsDiagnosticContext{
+        InconsistentDeclaration.DeclarationLocation, "other declaration",
----------------
alexfh wrote:
> I really don't like this idea of introducing a structure for the sole purpose of passing three parameters to a function. I don't see what we gain by doing so, and I would prefer the usual way to pass parameters. The same argument is valid for `formatDiagnosticsInCaseOfOnlyFunctionDeclarationsPresent` and `formatDiagnosticsInOtherCases`.
There is something to gain by it. Although in the end this proved to be a false problem, and I was able to get rid of these structures, I'll explain my reasoning.

The problem I was trying to solve here by introducing these structures, is actually the problem ofpassing reference to the results in `InconsistentDeclarationsContainer` and `DifferingParamsContainer`.

C++ does not allow for forward declarations of typedefs, so I cannot just say `class InconsistentDeclarationsContainer;` in header file. I have to provide full typedef involving `SmallVector` and its magic number in second template argument. This, I believe, is unnecessary introduction of implementation detail in header file.

So you see, I created such problem for myself, and this is the solution that I came up with.

However, now that I looked again at this, I saw a simpler solution.

All these functions formatting diagnostics, use only one member function of the check class: `diag()`. I was working under the assumption that it was a protected member of `ClangTidyCheck`. But I was wrong. It's public. So these don't have to be member functions.*Poof* and all our problems are gone :-)


http://reviews.llvm.org/D12462