ymandel marked an inline comment as done.
ymandel added inline comments.
================
Comment at:
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:262
/// Returns true if and only if `Type` is an optional type.
bool isOptionalType(QualType Type) {
if (!Type->isRecordType())
----------------
ymandel wrote:
> ymandel wrote:
> > xazax.hun wrote:
> > > Why do we need two places to define what is an optional type? Would it be
> > > possible to somehow reuse the Declaration matcher on the RecordDecel
> > > here? Or would that be bad for performance?
> > Good point. I dismissed reusing the matcher here because of performance
> > concerns -- setting up the match is expensive (in the constant-time sense
> > but still a lot of overhead). But, we're already using matching quite a lot
> > so I suspect it doesn't matter. Still, what about the reverse -- use this
> > predicate in the matcher, which is faster while still ensuring consistent
> > results?
> >
> > That said, what I really want is for the matcher logic to be made available
> > outside of the matcher framework -- I think `NamedDecl` should have the
> > core logic (parallel to `getQualifiedNameAsString`) and that could be used
> > elsewhere, including the matcher.
> regardless, the matcher is wrong (if we keep it): it should be `hasAnyName`
> rather than `anyOf` and lots of `hasName`. not sure how I never caught that
> earlier...
I've updated the matcher but otherwise left the code alone. I will send a
followup patch that shares the code now that I understand how to avoid the
mention of the libc++ internal names.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148344/new/
https://reviews.llvm.org/D148344
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
