[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