ymandel 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"),
----------------
xazax.hun wrote:
> 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.
Yes, it is a better approach. :) Done.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:49
+ this);
+ Finder->addMatcher(
+ cxxConstructorDecl(hasAnyConstructorInitializer(
----------------
xazax.hun wrote:
> 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?
Yes, we would. Good catch. Fixed.
================
Comment at:
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp:92
+ for (const SourceLocation &Loc : ExitBlockLattice.getSourceLocations()) {
+ diag(Loc, "unchecked access to optional value");
+ }
----------------
xazax.hun wrote:
> 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 :)
Yes -- we have access to the object expression at the time of detection. So
both are reasonable to include. We just need to extend the
SourceLocationsLattice to accomodate. Added a FIXME in the model.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121120/new/
https://reviews.llvm.org/D121120
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
