[PATCH] D106431: [clang-tidy] Fix cppcoreguidelines-init-variables with enum judgement

Tue Jul 27 09:17:37 PDT 2021

whisperity added a comment.

In D106431#2907002 <https://reviews.llvm.org/D106431#2907002>, @Sockke wrote:

> Any thoughts?  : )

First, let's first fix that we should still warn for the uninitialised `enum` case, without a FixIt. That's the issue at hand, right now, Clang-Tidy generates, as you identified, broken output. We can discuss the later steps after this is fixed. Please implement this logic, and update the patch, so we have a snapshot of how that would look like and the thing working.

Afterwards, as Aaron suggested:

In D106431#2896441 <https://reviews.llvm.org/D106431#2896441>, @aaron.ballman wrote:

> for enumerations, we could issue up to two fix-its on a note, one for the first and one for the last enumerator in an enumeration (and if the enum only contains one enumerator, there's only one fix-it to generate which suggests it could be on the warning rather than a note, but that seems like a lot of trouble for an unlikely scenario). However, I don't recall how clang-tidy interacts with fix-its on notes off the top of my head, so I'm making an assumption that clang-tidy's automatic fixit applying mode handles notes the same way as clang and we should double-check that assumption.

This might require nontrivial changes to the check's code, and investigating the potential problem with automated fix-it application when multiple conflicting fix-its are given on a **note** (not a //warning// line).

I think we all would be fine with only doing the first step (reintroduce the warning, without a fixit) in this patch, so it can be merged and hit the the mainline code quickly. The next step can be its own patch. 🙂

----

In addition, I wager that adding a line about this fix to the release notes at `clang-tools-extra/docs/ReleaseNotes.rst` is a good option, I can imagine people having turned this check off due to it being broken on enums.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106431/new/

https://reviews.llvm.org/D106431