Per desccription, patch attached. Mostly small changes: a few trivial
bugfixes, and some refactoring. This patch fixes all the test
failures except for one minor warning change.
The one non-obvious change is the change to CheckImplicitInitList,
doing the typechecking before constructing the implicit init list. As
I state in a comment, the reason is that we can't know how many
elements we need to add to the implicit init list until we've
typechecked the children. I think this is the most reasonable way for
the code to work correctly in its current form.
I won't commit any patches to SemaInit.cpp without snaroff's approval,
since I don't want to step on his toes. I can also split up the
patch, if that would make it easier to review.
-Eli
ttt.txt (12.5 KB)
Per desccription, patch attached. Mostly small changes: a few trivial
bugfixes, and some refactoring. This patch fixes all the test
failures except for one minor warning change.
Excellent.
The one non-obvious change is the change to CheckImplicitInitList,
doing the typechecking before constructing the implicit init list. As
I state in a comment, the reason is that we can't know how many
elements we need to add to the implicit init list until we've
typechecked the children. I think this is the most reasonable way for
the code to work correctly in its current form.
Agreed. I had a feeling we'd need to separate type checking from implicit init list construction.
I won't commit any patches to SemaInit.cpp without snaroff's approval,
since I don't want to step on his toes. I can also split up the
patch, if that would make it easier to review.
No worries. Since you/I worked on CheckInitializerListTypes, it's really good to have you review/enhance CheckInitList. I find this part of the C language very tricky. fyi...I'm in the process of moving my family and Apple's World Wide Developers conference is coming up soon (as a result, CheckInitList hasn't received my attention for a couple weeks now).
I noticed this patch doesn't enable any of these changes yet (i.e. you didn't change Sema::CheckInitializerTypes()). Was this intentional?
I haven't reviewed the patch closely, however I think it moves the ball forward. Please commit (and we can iterate if necessary...).
Thanks again - I greatly appreciate the work you are doing!
snaroff
No worries. Since you/I worked on CheckInitializerListTypes, it's really
good to have you review/enhance CheckInitList. I find this part of the C
language very tricky. fyi...I'm in the process of moving my family and
Apple's World Wide Developers conference is coming up soon (as a result,
CheckInitList hasn't received my attention for a couple weeks now).
Okay.
I noticed this patch doesn't enable any of these changes yet (i.e. you
didn't change Sema::CheckInitializerTypes()). Was this intentional?
The enabling patch is trivial; I'll do it in a separate commit.
I haven't reviewed the patch closely, however I think it moves the ball
forward. Please commit (and we can iterate if necessary...).
Okay, cool.
Thanks again - I greatly appreciate the work you are doing!
Thanks.
-Eli