[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check
Roman Lebedev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 2 13:28:27 PDT 2020
lebedev.ri updated this revision to Diff 295902.
lebedev.ri marked 10 inline comments as done.
lebedev.ri added a comment.
In D36836#2309243 <https://reviews.llvm.org/D36836#2309243>, @aaron.ballman wrote:
> In D36836#2308981 <https://reviews.llvm.org/D36836#2308981>, @lattner wrote:
>
>> Hi all,
>>
>> The LLVM foundation board discussed this offline -- it looks like the contributor is a bit confused about the LLVM project policies towards copyright, license, patent stuff etc. I really appreciate the dilligence here to capture the ideas that went into this code.
>>
>> My understanding is that the code and documentation are not derived from anyone else's code or documentation -- they are fresh implementations based on a paper written in english prose. If that is the case, please remove the LICENSE.TXT file -- it isn't necessary and it is confusing for people. I would also recommend replacing all the references to sonar source with a functional description of what this patch is doing - we aren't talking about adding a feature because of sonar source, my understanding is that this is adding a cyclomatic complexity checker. If and when the feature evolves over time, it would be misleading for it to be called sonar source.
>>
>> Please note that I didn't do a full code review here, others should do that.
>
> Thank you to you and the board for looking into this and helping us reach the right decision!
+1.
> In D36836#2309103 <https://reviews.llvm.org/D36836#2309103>, @lebedev.ri wrote:
>
>>> please remove the LICENSE.TXT file -- it isn't necessary and it is confusing for people. I would also recommend replacing all the references to sonar source with a functional description of what this patch is doing - we aren't talking about adding a feature because of sonar source,
>>
>> Done, as much as possible. I'd still prefer to maintain a reference to the spec on which it is based.
>> Note that it was done because that is what was requested previously by reviewers.
>
> I agree that we should still document that this is based off the Sonar Source paper because it's not a generic implementation but a specific one. I think you've struck a good balance with the current patch.
Good!
I've addressed most of the review comments, excluding the one about Microsoft SEH support - i'd prefer to leave that for a follow-up :)
There may be other straddlers anyway.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D36836/new/
https://reviews.llvm.org/D36836
Files:
clang-tools-extra/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.h
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/readability-function-cognitive-complexity.rst
clang-tools-extra/test/clang-tidy/checkers/readability-function-cognitive-complexity.cpp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D36836.295902.patch
Type: text/x-patch
Size: 70632 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201002/6791fd6b/attachment-0001.bin>
More information about the cfe-commits
mailing list