On Wed, Nov 7, 2012 at 6:02 PM, Jin Gu Kang <[email protected]> wrote: > Hi Eli, > > Thank you for your reviewing. > >> Your patch is missing a testcase. >> Please submit patches based on trunk > > I attach testcase file "recursive_struct.c". Before making patch file based > on trunk, I would like to explain the reason why I modified ConvertType not > FuctionType code. After reading the Clang code, I thought current Clang > generates struct type as type holder to cut the cycle of types on struct type > and function type. Type holder of struct type can be changed to correct > struct type later but one of function type can't be changed to correct > function type becase type holder is struct type. So I removed type holder > generation on function type and made only struct type cut the cycle of types. > There is one example to explain this situation. > > struct foo1 { > struct foo2* a; > struct foo3* b; > }; > struct foo2 { > void (*func1)(struct foo1*); > }; > struct foo3 { > void (*func1)(struct foo1*); > }; > struct foo2 e; > > Cycle of types can be generated by struct type, function type and pointer > type. Type dependence graph of above code is as following. > > ---> struct foo2 > | | > | V > | void (*func1)(struct foo1*) <---- > 1 | | | > | V | > | void (func1)(struct foo1 *) | > | | | > | V | > | struct foo1* | > | | | 2 > | V | > | struct foo1 | > |____ _| | > | | > struct foo3* | > | | > struct foo | > |___________________| > > Current Clang generates type holder at "struct foo2" to cut cycle 1 and at > "void (func1)(struct foo1 *)" to cut cycle 2. My patch is to use type holder > of struct type to remove cycle 2 not type holder of function type. In other > words, function type and pointer type return null when they have cycle and > only struct type generates type holder. For example, on above graph, "void > (func1)(struct foo1)" know that it has cycle so returns null and defers its > type translation, and "void (func1)(struct foo1)" also returns null and > finally "struct foo" generates type holder. To implement this concept on > current Clang code, I modifed ConvertType function. What do you think about > this? I couldn't find other solutions without modifying llvm vmcore to > support type holder for function type. If modifying of ConvertType function > is not problem, I will make patch base on trunk.
By "testcase", I mean a patch to clang/test/. Modifying ConvertType is fine. I'm still not happy with making it return null in some cases, particularly since there are obvious places where ConvertType itself doesn't handle a null return from ConvertType correctly. There is another way to cut cycles: ConvertType already has a mechanism to delay the conversion of a struct type (look for "DeferredRecords" in CodeGenTypes.cpp). It just isn't triggering for your testcase. -Eli _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
