[PATCH] D85601: Fixes an assertion when IntRange processes a throw expression

Mark de Wever via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 10 12:54:20 PDT 2020


Mordante marked an inline comment as done.
Mordante added a comment.

I added `void g()` since that's valid code which also caused an assertion failure. So the issue isn't in the error recovery but in determining the required IntRange. It seems the code doesn't take http://eel.is/c++draft/expr.cond#2.1 into account.



================
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
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85601



More information about the cfe-commits mailing list