THRIFT-4846. Topologically sort structs before generation

[toddlipcon]

This adds a toposort step prior to emitting generating code for structs.
For many languages (eg Java) this isn't necessary as there is no
"ordering" in the generated code. However, with C and C++, it's
important to define structs prior to their inclusion in another struct.

This also makes Thrift fail when generating C/C++ code with cyclic
references as the toposort is impossible.

A new test includes some structs defined in the "wrong" order to ensure
that the sort produces working code.

I manually tested a file with a true cycle in it, and verified the error
output.

[jeking3]

The CI issues have been resolved so I would suggest rebasing on master to get a clean run.

[Jens-G]

What's blocking this?

[toddlipcon]

Not sure. I took a look at the failed test runs for a few minutes and couldn't find anything that indicated issues with this patch, but it's a wall of text and I'm not familiar enough with the test suite to be able to diagnose what went wrong.

[Jens-G]

I would suggest to start with the conflict :-)

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[kainjow]

@toddlipcon I took a look at one of the test outputs and saw this:

org.apache.thrift.TestUnsafeBinaries > testSubStructValidation FAILED
4046    junit.framework.AssertionFailedError
4047WARNING: An illegal reflective access operation has occurred
4048WARNING: Illegal reflective access by org.mockito.cglib.core.ReflectUtils$2 (file:/root/.gradle/caches/modules-2/files-2.1/org.mockito/mockito-all/1.9.5/79a8984096fc6591c1e3690e07d41be506356fa5/mockito-all-1.9.5.jar) to method java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int,java.security.ProtectionDomain)
4049WARNING: Please consider reporting this to the maintainers of org.mockito.cglib.core.ReflectUtils$2
4050WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
4051WARNING: All illegal access operations will be denied in a future release
4052
4053org.apache.thrift.TestStruct > testSubStructValidation FAILED
4054    junit.framework.AssertionFailedError at TestStruct.java:383
4055
4056org.apache.thrift.TestReuse > testSubStructValidation FAILED
4057    junit.framework.AssertionFailedError
4058
4059191 tests completed, 3 failed
4060
4061> Task :test FAILED
4062
4063FAILURE: Build failed with an exception.

testSubStructValidation failed for Java and this tests StructA and StructB which have been changed in this PR.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically closed due to inactivity. Thank you for your contributions.

[stale]

This issue is no longer stale. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically closed due to inactivity. Thank you for your contributions.