[PATCH] D155890: [clang-tidy] Add folly::Optional to unchecked-optional-access

Fri Jul 21 10:30:16 PDT 2023

ymandel added a comment.

In D155890#4523243 <https://reviews.llvm.org/D155890#4523243>, @adukeman wrote:

> In D155890#4522266 <https://reviews.llvm.org/D155890#4522266>, @ymandel wrote:
>
>> In D155890#4521266 <https://reviews.llvm.org/D155890#4521266>, @carlosgalvezp wrote:
>>
>>> This should be a configuration option, we should not hardcore project-specific things in the source code.
>>
>> I agree, but we already are hardcoding specific types -- I think this is a separate (and valid) critique of the design. I'd propose filing an issue on the github tracker and we can follow up there.  I, for one, would love to review such a change but don't have the time to write it.
>
> Is moving these values to config an appropriate task for somebody like me new to working on clang-tidy? I'd be happy to merge this and then try the transition to a config assuming there's some similar examples I can borrow from elsewhere in the codebase.

This is one of the most complex clang-tidy checks. So, if you're looking for a CT starter task, I wouldn't recommend this particular challenge. That said, I think the clang-tidy side will be relatively easy -- CT has a mature config system/API.  The harder part (and not CT relevant) is refactoring this code to consume that config. It's not terribly complicated but will require a bunch of changes and probably some design questions.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155890