In D91948#2411058, @whisperity wrote:

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.

Since the analyzer cannot cannot model the size of the containers just yet (or precisely enough), what we are saying with this checker is to "always check the return value of the erase() function before use (for increment/decrement etc.) whether it is not past-end" .

If this is good to go, could you please commit this? Thanks!

In D69308#1727625, @NoQ wrote:

Another interesting problem that we forgot to mention on the open projects page is the modeling of C++17 bindings and decompositions: https://bugs.llvm.org/show_bug.cgi?id=43042

Also, in my opinion, out of all construction context problems mentioned on the open projects page, the NRVO problem is probably the easiest. It might as well be the least rewarding of them, but i think it is the friendliest possible problem to start with, as it doesn't force you to invent large new facilities.

In D69308#1727587, @NoQ wrote:

In D69308#1727108, @Szelethus wrote:

Would love to see this comment in its entirety on the open projects page :^)

I'd rather have a mention that @dkrupp is already working on this project, so that if somebody wanted to help out they could cooperate nicely.

Thanks for your comments @NoQ
I fixed them. Also added your implementation hints to the open projects page.

@aaron.ballman could you please check now? Thanks!

The patch is rebased to the latest master.

Thanks for the reviews!
Could you pls commit this for me?

@aaron.ballman could you please commit?
I don't have commit access. Thx.

Fixing minor capitalization issue and removing an extra newline.

I also analyzed openssl with the baseline and this version, but did not find any new warnings.
See:
http://codechecker-demo.eastus.cloudapp.azure.com/Default/#run=D66049_baseline&newcheck=D66049_improved&review-status=Unreviewed&review-status=Confirmed&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=D66049_baseline_diff_D66049_improved

@Szelethus thanks for your review.
I fixed your suggestions.

Thanks @aaron.ballman and @alexfh for your review.
I fixed your findings.

Thanks for the comments @NoQ , all of them addressed.

Fix comments from @NoQ

@aaron.ballman 's comments are fixed.

Thanks Gabor for writing this.
I suggested some minor changes to the txt. Otherwise LGTM.

I guess this is a placeholder for the subpages of "User Manual" @ https://clang-analyzer.llvm.org, which will be ported in follow-up patches.

In D57858#1500635, @NoQ wrote:

In D57858#1499996, @dkrupp wrote:

Some alpha checkers are considerably more mature than others and are quite usable. In our experience, there are some users who are keen to run these checkers on their code and report back any false positives to us. So in this sense these are not "developer only" checkers. So I think we should let the users list them, read their descriptions and try them out. Some of them will come back with useful feedback as to how to improve them further.

What are such checkers currently? Like, the ones that aren't clearly "missing limbs" and that have somebody happy to address feedback sent against them?

Do you have a chance to call out to your users for testing the checker and actively request feedback, as @Szelethus did on the mailing list?

I feel that we could do some sort of "early access checkers" programme, but i believe this would require a more careful PR than just dumping a list of alpha checkers on our users' heads.

In D57858#1499996, @dkrupp wrote:

Some users would not care if the checker gives some more false positives than the "mature" checkers if they can catch some true positives with them.

Yeah, and these are pretty much the users we're trying to protect from themselves :)

In D57858#1499414, @Szelethus wrote:

In D57858#1498640, @NoQ wrote:

So, like, the global picture is as follows. In our case the Driver (i.e., --analyze) is not much more user facing than frontend flags. It's still fairly unusable directly, as our Static Analyzer is generally not a command-line tool. The real user-facing stuff is the GUIs such as scan-build or CodeChecker. These GUIs decide themselves on what options they want to expose. For instance, you have a right to decide that CodeChecker shouldn't support the aggressive mode of the move-checker and don't expose it as an option in your GUI. In this sense it's not really useful to provide a centralized -help of all user-facing options.

But it sounds as if you want to change this situation and provide a single source of truth on what are the user-facing options. Particular GUIs can still ignore them, but you don't want to hardcode flags in CodeChecker, but instead you want to rely on clang to provide the list of supported options for you and, as a side effect, for scan-build users (if you also add a scan-build help flag). I'm totally in favor of crystallizing such list of user-facing flags, and this patch sounds like a good step in that direction, assuming that non-user-facing options are hidden.

That describes my intention quite well :)

I'm still in favor of hiding alpha checkers (as they are for development only, much like debug flags; i'd recommend hiding them in the CodeChecker UI as well)

Now, why do we care about frontend/driver flags when they're unusable by definition? That's because we have a mental trauma after seeing a few powerusers actively explore those flags, observe that they don't work, and then tell everybody that the Analyzer is broken. So there's a threshold, based on a tiny but painful bit of practical experience, that says that documentation of developer-only features on llvm.org or in code comments is fine, but documentation printed by the released binary itself is not fine.

What you say sounds very reasonable. Still, I am kind of hesitant about hiding all alpha checkers: I initially intended to hide only are developer-only checkers (modeling, debug). I guess if we define alpha checkers (as you stated numerous times) as incomplete, under development, are missing half their limbs and crash if you look at them the wrong way, sure, they belong in the developer-only category. But checkers such as mine (UninitializedObjectChecker), for the longest time were very stable, have been enabled by default for our internal projects, despite only recently moving out of alpha.

Then agaaain, if we're that stubborn about alpha checkers, we could might as well dig them out of -analyzer-checker-help-hidden, and leave the rest there. Untangling what alpha checkers depend on one another could be solved by making yet another frontend flag that would display checker dependencies, which would be super easy since D54438, or create an -analyzer-checker-help-alpha flag that would display alpha, but not developer-only checkers. @dkrupp @o.gyorgy Do you have any feelings on this?

and we should probably automatically hide options of checker that are hidden.

Checker options are a different kind of animal entirely. I think we should definitely let the user fine-tune some modeling checkers, for instance, unix.DynamicMemoryModeling:Optimistic, despite us not really wanting to let anyone (even developers really) mess around whether unix.DynamicMemoryModeling should be enabled. While that specific option is, to put it nicely, a little esoteric, making some decisions the analyzer makes less conservative, or limiting state splits to help performance may be desirable in the future.

Let's move the rest of the discussion directly related to hiding checker options to D61839!

Thanks for your reviews!
I have fixed all your comments and rebased the patch to the latest master.

@dcoughlin I don't necessarily agree with you.
Let me explain why we think this feature is important.

Thanks @NoQ .
So I created a very simple main page with the table of contents only http://cc.elte.hu/clang-docs/docs/html/ClangStaticAnalyzer.html

Thanks for your comments. I fixed them all. I also added the handling of redundant sizeof() and alignof() operators on the way. Please check if OK now...

All comments fixed.

@dcoughlin @NoQ ping...

In D55255#1319784, @JonasToth wrote:

Committed, Thank you for the patch! Was there a bug-report for this issue? If yes can you please close it/reference?

Comments addressed. Please commit if looks good, I don't have commit rights.
Thanks.

new undef/defined testcase added

In D55125#1315335, @Szelethus wrote:

I see your point, but here's why I think it isn't a bug: I like to see macros as constexpr variables, and if I used those instead, I personally wouldn't like to get a warning just because they have the same value. In C, silencing such a warning isn't even really possible.

Another interesting thought, @donat.nagy's check works by comparing actual nodes of the AST, while this one would work with Lexer, but they almost want to do the same thing, the only difference is that the first checks whether two pieces of code are equivalent, and the second checks whether they are the same. Maybe it'd be worth to extract the logic into a simple areStmtsEqual(const Stmt *S1, const Stmt *S2, bool ShouldCompareLexically = false) function, that would do AST based comparison if the last param is set to false, and would use Lexer if set to true. After that, we could just add command line options to both of these checks, to leave it up to the user to choose in between them. Maybe folks who suffer from heavily macro-infested code would rather bear the obvious performance hit Lexer causes for little more precise warnings, and (for example) users of C++11 (because there are few excuses not to prefer constexpr there) could run in AST mode.

edit: I'm not actually all that sure about the performance hit. Just a guess.

But I'm just thinking aloud really.

In D55125#1314788, @JonasToth wrote:

In D55125#1314741, @Szelethus wrote:

@JonasToth this is the Lexer based expression equality check I talked about in D54757#1311516. The point of this patch is that the definition is a macro sure could be build dependent, but the macro name is not, so it wouldn't warn on the case I showed. What do you think?

Yes, this approach is possible.
IMHO it is still a bug/redudant if you do the same thing on both paths and warning on it makes the programmer aware of the fact. E.g. the macros might have been something different before, but a refactoring made them equal and resulted in this situation.

-clang-format applied
-clang:: namespace qualifiers removed

@dcoughlin could you please look into this?

-scanbuild and xcode pictures are included now
-intro text ("What is Static Analysis?" etc.) are put under the Introduction section
-Download section is created, but I am not sure how well was the this Mac OSX binary release section was maintained. Should users really download from this site or through a package manager instead?

making the diff full context.

Which means that for some calls we aren't even trying to make a CTU lookup.

Thanks @NoQ, we will take a look at it!

@NoQ do we need any more update to this patch? Thanks.

The patch has been updated.
Changes:

Would be interesting to extend this checker (maybe in an upcoming patch) to report on uninitialized members not only in constructors, but also copy constructors and move constructors.

Please fix the incompatibility between analyze-build and lib/CrossTU in the format of externalFnMap.txt mappfing file.

The creation of this library (libCrossTU) is approved for importing function definitions. @zaks.anna, @NoQ , @klimek could you please help us reviewing the code itself?

It would be best to just add the scan-build-py support to the tree, especially, since the new scrips are not tested.

OK. We will update this patch with the scan-build-py changes and remove the ctu-build.py and ctu-analyze.py scripts.

Thanks for the reviews so far.
I think we have addressed all major concerns regarding this patch:

Thanks. Gabor, could you please merge this? I don't have commit right.

issues fixed

I tried to address all your comments.

its a good idea to include in LLVM/Clang i will propose it

In D12906#272243, @zaks.anna wrote:

In D10305#224956, @zaks.anna wrote:
For example, you could keep the information about the reports in the plist files and use those to
render the reports in HTML.

If you're okay with adding HTML file name in plist for each bug, I will prepare a new patch for that.
Thanks for the review!

I think you misunderstood my comment. I am not talking about using the existing HTML files here but rather having an HTML viewer, which you could use to browse source code. This viewer would be extended to read the bug reports from the plist files and display them. Currently, we create an html file with source code + report info for each bug report. This does not scale when you have a lot of reports on a single large file (ex: sqlite).

What I describe above is a larger project. What workflow are you trying to support? I think adding the issue hash to the HTML file is fine if you find it to be useful for your workflow...

Regarding testing:
I think we should create a RecursiveASTvistor based "test checker" that matches every statement and declaration and reports a bug there.
Then we could create a test file similar to what we have in /tools/clang/test/Analysis/diagnostics/report-issues-within-main-file.cpp
where the expected plist output can be written at the end of the file.

when calculating the has for the following fields