haoNoQ wrote:

Hi! Thank you for digging into this! Sorry for the delay.

> The new UnsafeBufferUsageCtorAttrGadget gadget explicitly avoids matching 
> against the std::span(ptr, size) constructor because that is handled by 
> SpanTwoParamConstructorGadget and we never want two gadgets to match the same 
> thing (and this is guarded by asserts).

Hmm at a glance I'm not sure this should really be illegal. It's a big problem 
when fixable gadgets overlap, because this means that they'll try to fix the 
same code in two incompatible ways. I don't immediately see why this would be a 
problem for warning gadgets that don't try to fix anything. This just means 
that the code is non-compliant for two different reasons, which is 
theoretically fine. I also tried to reproduce your assert and I didn't hit it; 
which one are you running into?

> To handle this we allow the gadget to control if the warning is general (it 
> calls handleUnsafeBufferUsage()) or is a std-container-specific warning (it 
> calls handleUnsafeOperationInContainer()).

Ooo I love this.

My initial thinking was, just make the base Gadget class "public" (move it into 
`UnsafeBufferUsage.h` so that the handler could see it) and make the handler 
switch over the gadget's `getKind()` to produce specialized messages for each 
gadget. This would help us unscrew the rest of the `Reporter` code so that it 
also didn't have to guess by statement kind.

Your design can grow into that too - make a separate handler method for each 
gadget kind (or group of kinds), and it preserves even more encapsulation. 
Additionally it throws away `getBaseStmt()` in favor of a more "integrated" 
solution. I'm a big fan, let's keep this.

https://github.com/llvm/llvm-project/pull/91777
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to