- User Since
- Jul 19 2017, 6:59 AM (79 w, 10 h)
And if we find out that this is far too painful, we can always put it back anyways. Doubt that'll happen though.
Hmmm, came across this one in the not too distant future, and always wondered how painful that performance hit would be that even Release+Asserts should be spared from it. I think 5% performance hit, if asserts are enabled, is an acceptable tradeoff, if the assert is crucial.
If you don't mind, I'd prefer to finally get over this patch. I'll commit on the 31st, but will wait for any potential feedback 'til then.
Poor wording on my end, sorry about that. Let me clarify.
Mon, Jan 21
Yup, I'm sold on that.
Cheers, this is as good as it gets! IRL we also mentioned making a unipage for each checker which would be awesome (and would require a lot of tedious work), but as a start, I'd much prefer browsing through this doc than the current one. Very much appreciated!
Sun, Jan 20
- Resolve the issue mentioned above by not enabling checkers that has any of their dependencies explicitly disabled
- Introduce osx.RetainCountBase to "solve" the issue mentioned in D55400#1364683, but the test files are still untouched. This leads to 9 lit test failures, but since this patch is ancient, I'd rather update it now and follow up with another, final version later.
Remove mutable specifiers from all fields of CheckerRegistry.
I'm working on rebasing my dependency branches on top of trunk, and I'm somewhat stuck on this patch, could you lend a hand please?
Thu, Jan 17
Overall I think this looks great, thanks! I left some inlines that would be nice to fix before commiting, but all of them are minor nits.
Wed, Jan 16
Tue, Jan 15
Mon, Jan 14
Awesome detective work! I glanced over the code, it looks great. I'd love to dedicate more time to your liveness-related patches, but university is a thing, so finding typos and the like is the best I can do for a while.
Mon, Jan 7
Always bothered me! Is there a way to squeeze this into %diff_plist?
Fri, Dec 28
Missed out on the developments. Thank you so much for handling this!
Dec 20 2018
Thanks! Sorry about being a little slow with this, I'm sadly busier than I expected, but I'll definitely think about a nicer solution.
Sure! I'm on my phone, and will be for a little while, can you commit on my behalf?
Dec 19 2018
I'm still looking for a sensible solution, but I'll at least share a patch that actually works.
Yea, should've used hasArg anyways. I'll look into this tonight.
Dec 18 2018
Global nit: I guess having both DESC and DOCS as variable/macro arg names is confusing. Can we instead use DOCSURI/DocsUri?
Thank you so much for working on this! The lack of a proper documentation is indeed a weak points of the project.
Dec 17 2018
I still expect some skeletons to fall out of the closet
This patch doesn't handle -analyzer-disable-checker, which leads to assertation failures in later pathes. Since the way which checker should/shouldnt be enabled is implemented rather poorly, I'll probably try to find a long-term solution.
Lit test failures are gone for Windows after reverting this. Will probably deal with this revision after 8.0.0.
Reverted in rC349344.
Reverted in rC349340. With the wrong revision name... oops...
A little late to the party, but I don't see anything against this.
@boga95 Do you have commit access, or need this to be commited on your behalf?
Interesting, I've been watching the bots closely, but got no mail after a while. I'm not sure what the cause is, so I'll revert one-by-one.
Dec 16 2018
Dec 15 2018
Hmm, I find the revision title and summary a little vague -- I'd expect a revision called "Refactoring" not to feature any funcitonal change, yet you changed how variadic functions are handled. Please
- Keep purely formatting changes to your earlier revision, and rebase this patch on that
- Edit the revision title and/or summary about what your patch does,
- If it features any change in the checker's behavior, include tests.
Commited in rC349274.
Hmm, actually, if you're doing changes all over the file in the followup patches, it shouldn't matter much. Let's just wait a couple days for ppl to object, now that they are subscribed.
That I think sounds great! Thanks!
I guess we used to have something that would result in an event duplication (which was the motivation for this patch), but it's long forgotten.
I don't really use scan-build, so I can't barrage you with suggestions, but the patch looks great.
Please don't mind me intruding.
If you create a patch for the Clang Static Analyzer, please add "[analyzer]" in the revision title, because many of us are automatically subscribed to those patches. Also, upload patches with full context.