- User Since
- Dec 6 2016, 10:52 AM (174 w, 5 h)
Mon, Mar 23
Sat, Mar 21
- Renamed check to experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.
- s/argument/parameter/g in the code and output where appropriate.
ClangTidyForceLinker.h also contained the entries without alphabetic order.
@aaron.ballman I've gone over LLVM (and a few other projects). Some general observations:
Tue, Mar 17
Feb 26 2020
Also, errors should conceptually break the operation at hand, so this thing should be a warning instead?
Do you have access to the Driver somehow? Instead of a cerr (-ish) output, something that is formatted like a "true" error diagnostic (or warning), such as "no such file or directory" would be much better, I fear this [ERROR] will simply be flooded away with the usual diagnostic output on screen, especially if multiple files are concerned.
Feb 25 2020
WIP because there's a lot of debug stuff that should be cleared out from here. This is a crude patch. It works fancy, but the code is crude.
@aaron.ballman @vingeldal I'll go over the findings (as I've run this on LLVM and various other projects, a few examples are uploaded in my first comment as HTML renders of the bug report!) soon, but I want to wait until a research paper I have based on this topic gets a final decision. (It should happen soon.) I also plan to refurbish D20689 and have it upstreamed as a "co-checker" of this (one for the interface rule, one for the call sites, they kinda want to solve different aspects of the same issue, let it be up for the project how much they wish to enforce).
Feb 24 2020
I'd rather not call them false positive because the definition of false positive is ugly and mushy with regards to this check. This check attempts to enforce an interface rule, whether you(r project) wants to adhere the rule is a concise decision. A type equivalence (or convertibility in case of the follow-up patch D75041 that considers implicit conversions) should be a "true positive" in every case, as an indicator of potentially bad design.
Jan 1 2020
There are a few really minor bug fixes, test additions, documentation update, etc. coming along soon, but I've some more pressing matters. However, please feel free to review the patch as-is!
Dec 31 2019
@vingeldal Apologies, in that case. However, I would still claim that style (as a potential severity) has its purpose for Tidy checkers, not just for clang-format.
Dec 30 2019
Dec 24 2019
The checks.rst has recently been updated to show "normal" (main entry) checks and "aliases" in different tables in D36051. Please register the alias-ness accordingly.
Dec 22 2019
Dec 20 2019
I am a little bit conflicted about the Severity column. While I know our people put a great deal of effort into keeping this classification sane, what was put into CodeChecker is, at the end of the day, a pretty arbitrary classification.
Dec 17 2019
Are buffers otherwise modelled by the Analyser, such as results of the malloc family and alloca intentionally not matched, or are there some tests missing regarding this?
Dec 12 2019
I have developed a related check in D69560. That one considers types, but is an interface rule checker, and does not consider (any) potential call sites. Moreover, it does not consider "swaps" that happen across a function call, only, as the name implies, adjacent similar-type ranges.
Dec 9 2019
Can you refresh my memory on whether a rule for "if init expr is constant, initialise in class body instead" exists for init list members? If so, this will be a funny "two pass needed to fix" kind of check.
Dec 5 2019
While I can't pitch in with actual findings of this check, @MyDeveloperDay, you're right in many aspects, including those specific examples not firing. But an example that actually fires this check indicates a very specific undefined behaviour case. So if such bogus code (that would trigger on this check) did not cause run-time issues so far, it's because they never free() their memory allocations (really bad?), or their platform is particular enough that it allows calling free into the buffer, not only for the start of the buffer.
Nov 18 2019
Oct 31 2019
- Removed This check from the documentation comment of the check's class too.
- Fix some comments, formatting and documentation
- Organised test files to be in the same directory as others on the Monorepo structure
- Helper functions moved from namespace (anonymous) to static.
- Added test for C and enabled check for C code.
Oct 29 2019
@Szelethus and @baloghadamsoftware are colleagues to me whom are far more knowledgeable about check development and I want them to see that I want a review from them. I specifically didn't do an "internal with colleagues" downstream review with regards to this code.
A few interesting "true positive" findings:
Aug 30 2019
Aug 16 2019
Apr 29 2019
@dcoughlin How would removing the USAGE part of the dump and keeping only the list of options and their formatted help sound? That way, this option will not invite the user to directly call the analyzer.
I think this is good. Patch still marked as Needs review for some reason. 😦 Can we look up this blocking review thing? Perhaps this could be marked ready to roll once the dependency patch is ironed out.
@Szelethus I know the dependent patch D59464 will move examples/analyzer-plugin to test/Analysis/plugins/..., but this patch still seems to affect examples/. Are you sure this is the right diff? Because you are adding brand new files and brand new "folders", I don't think that'll apply cleanly.
Apr 26 2019
Mar 12 2019
In case no bug reports against the checker are reported on the mailing list or Bugzilla, I wholeheartedly agree with kicking the ball here.
Feb 18 2019
Yeah, it seems upstream has moved away due to @Szelethus' implementation of a much more manageable "checker dependency" system. You most likely will have to rebase your patch first, then check what you missed which got added to other merged, existing checkers.
Jan 23 2019
Jan 18 2019
Dec 14 2018
Dec 3 2018
Nov 21 2018
Bar the previous comments, I really like this. The test suite is massive and well-constructed. Do we know of any real-world findings, maybe even from LLVM?
Nov 19 2018
Let's register the ID...
Nov 12 2018
Nov 11 2018
Nov 8 2018
Yes, down the line I realised the function is not needed. (It emits a diagnostic because the diagnostic comes from comparing the AST file's config blocks to the current (at the time of import) compilation.)
Nov 7 2018
Test was added.
Nov 6 2018
Nov 2 2018
Oct 30 2018
Oct 24 2018
Updating the diff just in case so that I don't lose the test code.
@dblaikie I have created a test, but unfortunately %clang_cpp in LIT invokes clang --driver-mode=cpp which is not the same as if clang++ is called. I'm trying to construct the following command-line
Oct 17 2018
Oct 16 2018
With regards to D53352: you can change the diff related to a patch whilst keeping discuccion and metadata of the diff.
Remove the custom file paths and URLs but other than that this is pretty trivial.