[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-02-13 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment. In D72242#1872827 , @smeenai wrote: > CC @hans for the 10.0 branch. Cherry-picked to 10.x as 808f8a632f8bc12830157c57461ae8f848c566a3 . Please let me know if the

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-02-12 Thread Elizabeth Andrews via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGa58017e5cae5: Fix type-dependency of bitfields in templates (authored by eandrews). Herald added a project: clang. Reposi

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-02-12 Thread Elizabeth Andrews via Phabricator via cfe-commits
eandrews added a comment. In D72242#1873017 , @smeenai wrote: > I'm not sure why Phabricator is still showing Needs Review, but @rnk's > approval should make this count as accepted. Ok. Thank you. I've pushed the patch. CHANGES SINCE LAST ACTION htt

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-02-12 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. I'm not sure why Phabricator is still showing Needs Review, but @rnk's approval should make this count as accepted. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72242/new/ https://reviews.llvm.org/D72242 ___ cfe-c

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-02-12 Thread Elizabeth Andrews via Phabricator via cfe-commits
eandrews added a comment. Actually I just noticed it still says 'Needs review' above. I'm not sure what the protocol is. Can I push the patch? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72242/new/ https://reviews.llvm.org/D72242 ___ cfe

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-02-12 Thread Elizabeth Andrews via Phabricator via cfe-commits
eandrews added a comment. Thanks! I'll push it shortly. Just running a final ninja check-all after updating workspace. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72242/new/ https://reviews.llvm.org/D72242 ___ cfe-commits mailing list cf

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-02-12 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a subscriber: hans. smeenai added a comment. CC @hans for the 10.0 branch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72242/new/ https://reviews.llvm.org/D72242 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-02-12 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. lgtm The fix looks like what Richard asked for, so I feel comfortable approving this. Thanks for the fix! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72242/new/ https://reviews.llvm.org/D72242 __

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-02-12 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. This also fixes https://bugs.llvm.org/show_bug.cgi?id=44886 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72242/new/ https://reviews.llvm.org/D72242 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-02-06 Thread Elizabeth Andrews via Phabricator via cfe-commits
eandrews updated this revision to Diff 243049. eandrews edited the summary of this revision. eandrews added a comment. Thanks for taking a look Richard. This patch adds the required value-dependency check and sets type-dependency accordingly. CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-01-23 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith requested changes to this revision. rsmith added a comment. This revision now requires changes to proceed. This is not an appropriate fix; the code is locally correct. The right way to handle this is exactly what Clang currently does: if the expression is not type-dependent, then contextu

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-01-22 Thread Elizabeth Andrews via Phabricator via cfe-commits
eandrews added a comment. In D72242#1820908 , @rnk wrote: > I think I liked the first version of this patch better. I would say, if the > AST after instantiation remains the same as it was before D69950 > , then the first

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-01-14 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment. I think I liked the first version of this patch better. I would say, if the AST after instantiation remains the same as it was before D69950 , then the first one seems like the right fix. The pattern AST will just be missing a node to repres

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-01-09 Thread Elizabeth Andrews via Phabricator via cfe-commits
eandrews updated this revision to Diff 237132. eandrews edited the summary of this revision. eandrews added a comment. Semantic analysis for value dependent conditions is now skipped as per Erich's comment. Patch adds an argument to CheckBooleanCondition to still do the required analysis for no

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-01-08 Thread Elizabeth Andrews via Phabricator via cfe-commits
eandrews added a comment. Thanks for taking a look Erich! > I'm concerned about just making this fallthrough to 'false'. These ARE > integral promotions, we just don't know the type size. In this case, when integral promotion is skipped, boolean conversion (C++ 4.12) is done instead when parsi

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-01-06 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. I'm concerned about just making this fallthrough to 'false'. These ARE integral promotions, we just don't know the type size. What happens if you instantiate this template? Will it still give the correct answer/type/diagnosis? In the case of the crash, I would sus

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

2020-01-05 Thread Elizabeth Andrews via Phabricator via cfe-commits
eandrews created this revision. eandrews added reviewers: rnk, erichkeane, alexfh, alexfh_. This patch is a follow up to D69950 , to fix a new crash on CXX condition expressions in templates, for value dependent bitfields. Clang currently crashes when integral p