urnathan 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;
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107347/new/

https://reviews.llvm.org/D107347

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to