https://github.com/NeKon69 commented:

Thanks!

One thing I noticed is that you haven’t reused the logic for getting the 
insertion point yet. I’m thinking it might make sense to create a helper 
function that takes either a `ParmVarDecl *` or a `CXXMethodDecl *` and returns 
both the fix-it text and the location. That way, the same logic could be reused 
in both your new code and the existing code.

Not something that needs to be addressed in this PR, but another thing I’m a 
bit unsure about is whether we should add these warnings to 
`LifetimeSafetySuggestions` now that they have fix-its. My concern is that this 
would also cause these warnings to show up in `warn-lifetime-safety-fixits.cpp` 
and `warn-lifetime-safety-suggestions.cpp`, which probably wouldn’t be ideal.

We could maybe refactor these tests later, for example by splitting 
`-Wlifetime-safety-suggestions` into multiple subgroups and creating separate 
test files for each subgroup. However, I wouldn’t frame this as a final 
decision yet. We may want to wait for others’ opinions on that.

https://github.com/llvm/llvm-project/pull/199149
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to