rsmith added inline comments.

================
Comment at: clang/lib/Sema/SemaChecking.cpp:10164
     const BuiltinType *BT = cast<BuiltinType>(T);
-    assert(BT->isInteger());
+    if (!BT->isInteger()) {
+      // This can happen in a conditional expression with a throw statement
----------------
Mordante wrote:
> rsmith wrote:
> > Can we handle this in code that's specific to conditional expressions 
> > instead? Presumably somewhere higher up in the call graph, some code is 
> > assuming that it can recurse from a conditional expression to its 
> > subexpressions, and that assumption is wrong.
> I can take  a look at it if you want. However I feel this fix is better. If 
> the conditional doesn't throw it can properly evaluate the required IntRange. 
> If it throws the range doesn't matter, therefore I didn't want to increment 
> the required range.
> Do you agree?
> Should I add more comment to clarify the design decission?
In order to get here, we need to have called functions that are documented as 
taking only integer types but passing them a `void` type. So the bug has 
already occurred before we got here.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:10317-10320
     IntRange L =
         GetExprRange(C, CO->getTrueExpr(), MaxWidth, InConstantContext);
     IntRange R =
         GetExprRange(C, CO->getFalseExpr(), MaxWidth, InConstantContext);
----------------
It seems to me that the bug is here. `GetExprRange` is documented as 
"Pseudo-evaluate the given **integer** expression", but the true and false 
expressions here might not be integer expressions -- specifically, one of them 
could be of type `void` if it's a //throw-expression//. In that case, we should 
only call `GetExprRange` on the other expression. The expression range of the 
`void`-typed expression is irrelevant in this case, because that expression 
never returns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85601

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

Reply via email to