xazax.hun added inline comments.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:38
+
+ auto HasOptionalCallDescendant = hasDescendant(callExpr(callee(cxxMethodDecl(
+ ofClass(anyOf(hasName("std::optional"), hasName("absl::optional"),
----------------
I am a bit afraid that we have two places to update the list of supported
optional types and they can get out of sync. I wonder whether creating a
function in `clang/Analysis/FlowSensitive/Models/UncheckedOptionalUseModel.h`
to return a matcher that matches the supported optional types is a better
approach.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:49
+ this);
+ Finder->addMatcher(
+ cxxConstructorDecl(hasAnyConstructorInitializer(
----------------
If we have a `CXXConstructorDecl` where both the initializer and the body has
calls to optional types, i.e. both matchers would match, would we process the
body of that twice?
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:60
+
+ if (Result.SourceManager->getDiagnostics().hasUncompilableErrorOccurred()) {
+ return;
----------------
Nit: Redundant braces.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:92
+ for (const SourceLocation &Loc : ExitBlockLattice.getSourceLocations()) {
+ diag(Loc, "unchecked access to optional value");
+ }
----------------
Is there a way in the future to include either the name of the optional or a
source range of the access? This can be confusing when we have multiple
optionals in the same line. The of course, the column information helps. But
the more visual guides we have the better :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121120/new/
https://reviews.llvm.org/D121120
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
