[llvm] Add test for iterating over MDNode operands when they are empty (PR #80737)

Felipe de Azevedo Piovezan via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 6 14:50:54 PST 2024


felipepiovezan wrote:

> If anything, maybe some place on the forums to post "does anyone see anything wrong with this general class of tidy/fixit/change (not each specific instance), if not, I'm going to apply this across the LLVM repo" for each type of change, perhaps.

This is the key here IMO. If we had taken this to the forums, I suspect some of these classes of changes would have seen pushback, either in a more general form or from subprojects.
Historically, we have been pretty averse to widespread changes like those, we favor doing these changes at the same time as the same code lines are being moved or changed for other reasons. To cite some examples: we've tried to change the camel case convention in one sweep, we've tried to perform clang-format in a similar fashion, these were all shut off.

> I think these patches do qualify as not requiring precommit review according to policy

That's another issue here, it's pretty difficult to tell. A handful of those are probably fine, but if we count how many such changes were pushed last year, the volume is a bit high: 623 commits with the "NFC" tag since Jan 1 2023. Considering this volume of commits is coming from a single author, it's pretty difficult as a group to evaluate what's going on, and the cost of bugs like this can be high.

> These aren't formatting or whitespace-only changes, though. So I don't think this applies.

I understand your point if take the policy literally, but IMO the spirit of that guideline is to avoid polluting the git log and avoid some downstream churn. Maybe we should try to clarify if this is the intent or not.

https://github.com/llvm/llvm-project/pull/80737


More information about the llvm-commits mailing list