MosheBerman added a comment. In D123352#3458530 <https://reviews.llvm.org/D123352#3458530>, @steakhal wrote:
> In D123352#3439649 <https://reviews.llvm.org/D123352#3439649>, @MosheBerman > wrote: > >> In D123352#3439390 <https://reviews.llvm.org/D123352#3439390>, @steakhal >> wrote: >> >>> tldr; static-analyzer fixits are not completely implemented. >> >> Where can I learn more about this? > > Grep through the code and look for references to the variable. It's not used > widely. > In fact, there are no checkers facilitating fixits. There's a DeadStore checker that does have some fixit code, which is where I found the `FixItHint` class. I did notice it isn't used in too many other places. >> Would it be possible and idiomatically/architecturally sounds to write a >> clang-tidy that processes output from this checker? > > I don't believe that `clang-tidy` should be involved in this. It would be > better to keep it in-house (in the analyzer). > >>> When I passed the `apply-fixits`, it modified the input source file - as I >>> expected. >> >> Did you test this diff, or an existing checker? Would you please share the >> command you used to test? > > Sort of. I tried to apply some parts of it by hand, and check the output > difference in my command. > > I cannot immediately recall, but I cannot remember any major obstacles, which > suggests that I had to set the `apply-fixits=true` analyzer config option and > that's it. > Since there were no checkers emitting fixit hints, I modified an existing one > to emit a dummy fixithint. Nothing fancy there. > >>> Then I tried the `-analyzer-output=text` and suddenly it inserted the fixit >>> 2 times xD, which is less than ideal and we should fix this. >>> And I'm expecting many more bugs with this feature. >> >> This is why it's gated. xD. > > What do you mean by 'gated'? It seems to be a bug, which we should fix prior > to this. The `-analyzer-output=text` comes from a frontend. (I misplaced the source right now, but I did see it recently on the LLVM github.) The reasons I'm not too concerned are: 1. It looks like a pre-existing behavior. (The text frontend consumer appeared to be printing fixits independently of whatever other diagnostic flags) 2. When I say gated, I mean that the default behavior of the checker hasn't changed. These fixits are opt-in. If you're opting in, you can also choose to not use the text output. > PS: I'm also inviting @whisperity since he is more experienced with fixits > and that sort of stuff. Thanks! More reviewers means I learn more and can be more accurate. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123352/new/ https://reviews.llvm.org/D123352 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
