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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2f0630f8bc91: [clang-tidy] Add folly::Optional to unchecked-optional-access (authored by Anton Dukeman antonduke...@fb.com, committed by PiotrZSL).

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

PiotrZSL accepted this revision. PiotrZSL added a comment. +- LGTM, I will correct those nits and push it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155890/new/ https://reviews.llvm.org/D155890 ___

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

PiotrZSL added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone/unchecked-optional-access.rst:11 This check identifies unsafe accesses to values contained in -``std::optional``, ``absl::optional``, or ``base::Optional`` +``std::optional``,

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

adukeman updated this revision to Diff 543659. adukeman added a comment. Update docs and run clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155890/new/ https://reviews.llvm.org/D155890 Files:

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

PiotrZSL added a comment. Some entry in release notes or/and check documentation would be welcome. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155890/new/ https://reviews.llvm.org/D155890 ___

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

adukeman marked 3 inline comments as done. adukeman added a comment. Thanks for all the comments! I think I've got them all addressed and tests are passing locally. In D155890#4528560 , @carlosgalvezp wrote: > The review is marked as accepted, should

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

adukeman updated this revision to Diff 543619. adukeman added a comment. Update calls to OptionalMemberCall to take a name matcher Supports treating calls to optional::has_value and optional::hasValue identically Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

ymandel added a comment. In D155890#4528560 , @carlosgalvezp wrote: > The review is marked as accepted, should we land it? Let me know if you need > help with that :) We're still waiting on some small changes:

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

carlosgalvezp added a comment. The review is marked as accepted, should we land it? Let me know if you need help with that :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155890/new/ https://reviews.llvm.org/D155890

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

ymandel added a comment. In D155890#4525144 , @carlosgalvezp wrote: > I have opened a refactoring ticket here: > https://github.com/llvm/llvm-project/issues/64037 Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

carlosgalvezp added a comment. I have opened a refactoring ticket here: https://github.com/llvm/llvm-project/issues/64037 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155890/new/ https://reviews.llvm.org/D155890

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

carlosgalvezp added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:62 +return N != nullptr && (isTopLevelNamespaceWithName(*N, "base") || +isTopLevelNamespaceWithName(*N, "folly")); }

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

gribozavr2 added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:62 +return N != nullptr && (isTopLevelNamespaceWithName(*N, "base") || +isTopLevelNamespaceWithName(*N, "folly")); }

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

carlosgalvezp resigned from this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. > I think it'd be good to add reviewers there I realize the CodeOwners for Analysis are already in the list of reviewers, I won't interfere then :) Repository: rG LLVM

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

carlosgalvezp added a comment. In D155890#4523262 , @gribozavr2 wrote: > In D155890#4523243 , @adukeman > wrote: > >> In D155890#4522266 , @ymandel >> wrote: >> >>> In

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

gribozavr2 added a comment. In D155890#4523243 , @adukeman wrote: > In D155890#4522266 , @ymandel wrote: > >> In D155890#4521266 , >> @carlosgalvezp wrote: >> >>> This

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

ymandel added a comment. In D155890#4523243 , @adukeman wrote: > In D155890#4522266 , @ymandel wrote: > >> In D155890#4521266 , >> @carlosgalvezp wrote: >> >>> This

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

adukeman added a comment. In D155890#4522266 , @ymandel wrote: > In D155890#4521266 , @carlosgalvezp > wrote: > >> This should be a configuration option, we should not hardcore >> project-specific things in the

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

ymandel added a comment. In 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

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

carlosgalvezp requested changes to this revision. carlosgalvezp added a comment. This revision now requires changes to proceed. This should be a configuration option, we should not hardcore project-specific things in the source code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

gribozavr2 accepted this revision. gribozavr2 added a comment. This revision is now accepted and ready to land. Thank you! Do you have commit access or do you need me to commit this patch for you? Comment at:

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

PiotrZSL added a comment. To be honest, all those things just should be configurable instead of hardcoding them. For example, a project that I work for got 4 different custom optional implementations, and this check is useless for all of them. And still no boost::optional support. Repository:

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

adukeman created this revision. adukeman added reviewers: njames93, ymandel, sgatev. Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All. adukeman requested review of this revision. Herald added projects: clang, clang-tools-extra.