Interestingly, this many year old (when I last looked I remember 2010ish) checker was committed without any tests, so I thought I'd implement them, but I was shocked to see how I barely managed to get it working. The code is severely outdated, I'm not even sure it has ever been used, so I'd propose to move it back into alpha, and possibly even remove it.
Details
Diff Detail
- Repository
- rC Clang
Event Timeline
lib/StaticAnalyzer/Checkers/LLVMConventionsChecker.cpp | ||
---|---|---|
193–195 | The StringRefpart of this checker doesn't do a thing for me at all (even after a good amount of trying to make it work), but this part of the checker suffers a lot too. I'm sure we can some up with better ways for this function (and every single one in this file to be honest) now that almost a decade has passed. |
I've actually never noticed this checker until you started updating the website :)
This might be also covered by @rnkovacs's string buffer escape checker - either already or eventually, it'll become just yet another string view API that the checker supports.
test/Analysis/llvm-conventions.cpp | ||
---|---|---|
9 | I think we should try to re-use (and possibly extend) test/Analysis/inputs/system-header-simulator-cxx.h. |
In addition, clang/lib/StaticAnalyzer/README.txt and other related docs in clang/lib/docs/analyzer are also out of date.
www/analyzer/alpha_checks.html | ||
---|---|---|
599 | Like StringRef, ArrayRef may have similar issues. After this patch landed, maybe we can enhance this checker step by step. By that time, we may need the ideas of the heavy users of the LLVM library. |
I thought about that too, adding some StringRef specific information to that checker makes more sense then essentially duplicating the logic. However, what @MTC mentioned about ArrayRef<T> would be a neat addition too, and maybe it isn't worth making InnerPointerChecker that general.
@rnkovacs, any thoughts?
- Moved std::string implementation from InnerPointerChecker's testfile to the system header simulator header file.
I agree, adding StringRef support to the buffer escape checker around the time we add string_view will be great. Also, we named it InnerPointerChecker to allow for covering more general structures in the future, so I guess ArrayRefs may also be added eventually. Unfortunately, I have some other tasks to finish before getting to these, so I think this patch is fine now as-is.
I also agree that we should generally try harder to determine common logic behind checkers. Eg., MallocChecker and StreamChecker should be the same checker, or at least re-use this fairly large chunk of common code that implements the overall idea of "one function produces a symbol, then another function consumes it, and the consuming function always needs to be called on that symbol, while the symbol should not be used in certain (almost all) manners after it is consumed".
The StringRefpart of this checker doesn't do a thing for me at all (even after a good amount of trying to make it work), but this part of the checker suffers a lot too.
I'm sure we can some up with better ways for this function (and every single one in this file to be honest) now that almost a decade has passed.