User Details
- User Since
- Dec 6 2016, 10:52 AM (215 w, 2 d)
Today
Obsolete. There was no community consensus, or even a large enough discussion about this. What's more, the only dependee of this patch has superseded in ellegance, dropping the need for this group.
This patch will be replaced with the refactoring of the base check.
This patch will be replaced with the refactoring of the base check.
Refactored the check and rebased it against "current" master. This version now has the tests rewritten and fixed. In addition, this patch only introduces the very basic frame of the check (i.e. only strict type equality), nothing more. This is so that review can be done in a more digestible way.
Mon, Jan 18
I have re-analysed Bitcoin and Xerces with the new version (not yet published to Phab), and the results are looking really good! There is an insignificant change in the number of reports, 1 or 2 new ones compared to the previous, all explained by the fact that "unnamed parameters" are marked as ignored, i.e. the previous report (which reported 3 swappable parameters, one unnamed) is marked "Resolved" (1), but there is the (2 swappable one without the unnamed at the start/end) as "New". Due to this, I will spare you from showing the essentially same table from above again: the total number of results are unchanged, only the details of each (and they are changed in a positive (towards more sensible) direction).
Fri, Jan 8
I was just about to write an issue to the CoreGuidelines project, but I was @vingeldal has posted about "relatedness" before me: https://github.com/isocpp/CppCoreGuidelines/issues/1579. It was in the list of closed issues.
Ignore one-way implicit conversions
Tue, Jan 5
Dec 16 2020
But of course, after having written all of that, I forgot to upload the results themselves... 😲
Right, let's bump. I've checked the output of the checker on a set of test projects (our usual testbed, I should say) which contains around 15 projects, half of which was C and the other half C++. All projects were analysed independently in a virtual machine. All projects were analysed after checking out the latest release tag that was closest to Jan 2019. (I know this sounds dated, but the whole reproduction image was originally constructed for another checker I'm working on, and I did not wish to re-do the whole "What is needed to install this particular version?" pipeline.) The more important thing is that releases were analysed, which should, in theory, under-approximate the findings because releases are generally more polished than in-the-works code. The check was run as-is (sans a minor rebase issue that does not affect functionality) currently in the patch, namely, Diff 259508.
Nov 23 2020
Actually, while the explanation is understandable for me with additional knowledge about the representation... I think it would be useful to add the most simple example from the iterator checkers to the end of the document, how this whole thing ties together and how it is useful in a checker.
In general, perhaps you should go over the rendered text once again and make use of emph and bold some more.
Explanation looks okay from my part, although I'm really not knowledgeable about CSA internals. But there are plenty of "typesetting" issues.
Sep 29 2020
Sep 25 2020
Sep 24 2020
Ping. Can this go in? Still using this on a local fork, and still feels nice to be able to specify just a single tool.
I'd like to resurrect the discussion about this. Unfortunately, the concept of experimental- group (in D76545) has fallen silent, too. We will present the results of this analysis soon at IEEE SCAM (Source Code Analysis and Manipulation) 2020. While a previous submission about this topic was rejected on the grounds of not being in line with the conference's usual, hyper-formalised nature, with one reviewer especially praising the paper for its nice touch with the empirical world and the fact that neither argument swaps (another checker worked on by a colleague) nor the interactions of this issue with the type system (this checker) was really investigated in the literature for C++ before, we've tailored both the paper and the implementation based on reviewer comments from the scientific world, and the comments here.
The reviews were mostly favourable, excl. comments about the lack of formalisation and the unfortunate lack of modelling for template instantiations. Such a cold cut, however, only introduces false negatives into the result set. Which is fine, as the users will never see those non-existent reports!
Sep 14 2020
Minor inline comments.
Sep 11 2020
Sep 7 2020
One minor "typo", otherwise I think we are saying everything that could be and needs to be said. Also, @steakhal, that must have been one massive blame.
Aug 26 2020
Aug 25 2020
Some grammatical fixes and suggestions, inline. I might have absolutely butchered 80-col in the suggestions (thanks Phab for not showing any indication of line length...), so make sure you manually reformat the document before going forward!
Aug 24 2020
Aug 14 2020
Aug 13 2020
Aug 12 2020
@baloghadamsoftware Maybe there is a typo in the summary of the patch
Aug 11 2020
Aug 10 2020
Aug 7 2020
Aug 5 2020
What will happen with the ability to analyse a translation unit whose target was not part of LLVM_TARGETS_TO_BUILD of the current clang binary? Will it break, because the binary lacks the information on how to generate for the given target?
Aug 3 2020
Jul 30 2020
After using this for a while, it feels more natural and more obvious that these are the CTE targets, not the "clang/tools" test targets.
Jul 29 2020
Jul 28 2020
Jul 21 2020
@lebedev.ri If you have an idea on who's competent in accepting this change, please update the reviewers field, I'm in the dark here.
Jul 20 2020
Jul 14 2020
Jul 9 2020
In general, the test files should be cleaned up.
Jul 2 2020
Jun 26 2020
Jun 25 2020
As a future work, when something support ifs, it should also support ?:, and eliminate redundant conditionals from there, too.
Jun 9 2020
May 28 2020
May 15 2020
May 12 2020
(Maybe this will make Phab not show "✅ Accepted"...)
Please check the summary of the patch, it seems to contain old information as well.
May 6 2020
Apr 24 2020
@steakhal You might want to update the patch summary before committing this to the upstream (it still mentions "not needing a visitor" 😄)
Apr 23 2020
First things first, we were 50 thousand (!) patches behind reality. Rebased to master. Fixed it to compile, too. Otherwise, NFC so far.
Assuming direct control. Previous colleagues and university mates departed for snowier pastures, time to try to do something with this check.
Apr 22 2020
- Re-organised code, removed debug prints, rebased, the usual tidy-up.
- Bug fixes on certain crashes like incomplete types, conversion operator templates, etc.
- Even more bug fixes against crashes, hopefully... sorry I lost the individual commits in a squash I think
- Better organisation of code, cleanup of debug prints, fix nomenclature of functions
- Explicitly mention the first and last param of the range for clarity
- Downlift the logic of joining and breaking ranges to main patch (this isn't terribly useful at this point, but both subsequent improvements to the check depend on this change)
- Basically no longer assume that if param N can be swapped with param 1, then it can also be swapped with 2, 3, etc. without checking.
- Made the list of ignored parameter and type names configurable as a checker option
- Changed the default value of MinimumLength to 2, as prescribed in the guideline's text.
Apr 21 2020
Apr 14 2020
Adding @martong, he might have knowledge about the whole summary thing as he's tinkering the(?) STL checker.
Mar 23 2020
@aaron.ballman @njames93 Should I write up a pitch mail to cfe-dev about this?
Mar 21 2020
- Renamed check to experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.
- s/argument/parameter/g in the code and output where appropriate.