a_sidorin added inline comments.
================ Comment at: lib/AST/ASTImporter.cpp:6160 + // condition-dependent. + bool CondIsTrue = false; + if (!E->isConditionDependent()) ---------------- aaron.ballman wrote: > tmroeder wrote: > > aaron.ballman wrote: > > > a_sidorin wrote: > > > > aaron.ballman wrote: > > > > > `bool CondIsTrue = E->isConditionDependent() ? false : > > > > > E->isConditionTrue();` > > > > `bool CondIsTrue = E->isConditionDependent() && E->isConditionTrue();`? > > > I like that even better than my suggestion. :-) > > Wait, this doesn't have the same truth table as my original code. > > > > let `CD = E->isConditionDependent()` > > let `CT = E->isConditionTrue()` > > > > in > > > > ``` > > bool CondIsTrue = false; > > if (!CD) > > CondIsTrue = CT; > > ``` > > > > has the table for `CondIsTrue`: > > > > | `CD` | `CT` | `CondIsTrue` | > > | T | T | F | > > | T | F | F | > > | F | T | T | > > | F | F | F | > > i.e., if CD is true, then CondIsTrue is always false. Otherwise, it's the > > value of CT. > > > > The suggested line has the truth table > > > > | `CD` | `CT` | `CondIsTrue` | > > | T | T | T | > > | T | F | F | > > | F | T | F | > > | F | F | F | > > > > That is, the effect of CD is swapped. > > > > Aaron's suggestion matches my original table. I based my code on > > include/clang/AST/Expr.h line 4032, which asserts !isConditionDependent() > > in the implementation of isConditionTrue. > > > > I realized this after I "fixed" my comment to match the implementation > > change. Am I missing something? Or is the assertion in Expr.h wrong? I > > think this should be > > > > ``` > > bool CondIsTrue = !E->isConditionDependent() && E->isConditionTrue(); > > ``` > > > > I've changed my code to that and reverted the comment change. > Good catch -- I think my eyes just missed the change in logic. Perhaps we > should add a test case that exercises this? Wow, that's a nice catch. Sorry for the misleading. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58292/new/ https://reviews.llvm.org/D58292 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
