[PATCH] D107347: [Sema] haveSameParameterTypes - fix repeated isNull() test
Simon Pilgrim via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 18 04:12:24 PDT 2021
RKSimon added inline comments.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:9528
QualType T2 = NextParam(F2, I2, I == 0);
- if (!T1.isNull() && !T1.isNull() && !Context.hasSameUnqualifiedType(T1, T2))
+ if (!T1.isNull() && !T2.isNull() && !Context.hasSameUnqualifiedType(T1, T2))
return false;
----------------
urnathan wrote:
> RKSimon wrote:
> > @rsmith Can these isNull checks ever fail? Or would we be better off changing them into an assert?
> > ```
> > QualType T1 = NextParam(F1, I1, I == 0);
> > QualType T2 = NextParam(F2, I2, I == 0);
> > assert(!T1.isNull() && !T2.isNull() && "Unknown types");
> > if (!Context.hasSameUnqualifiedType(T1, T2))
> > ```
> that it's never ICED without checking T2's nullness suggests to me that they're never null. A null type here would seem to be from bad parsing, in which case why are we even checking further? IMHO assert now, there's plenty of time before C14 to revert that.
OK - I'll change this to an assertion
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107347/new/
https://reviews.llvm.org/D107347
More information about the cfe-commits
mailing list