Fix stack overflow containsTensorFlowValue for recursive data structures.

[bgogul]

The utility function containsTensorFlowValue fails with a stack overflow error when it is invoked on a recursive data structure such as PairedStructure struct in Prototypes/PatternMatching.swift test.

This PR simply breaks cycles by keeping track of the parent decls.

[bgogul]

@swift-ci please test tensorflow

[mhong]

Interesting finding! Some thoughts / suggestions:

1. can you create a stand-alone unit test (which deals with tensors) that exposes this problem?

From the cited example:

struct PairedStructure<I: Comparable> {
  let bounds: Range<I>
  let subStructure: [PairedStructure<I>]
}

subStructure is not itself a struct, and given https://stackoverflow.com/questions/36080491/swift-struct-type-recursion, i wonder if we should tweak the impl to avoid descending into a non-struct type, such as array?

2. i'm also wondering if we could avoid calling structContainsTensorFlowValue() on structs like PairedStructure, which are not related to tensors / TF. As a general principle, can we avoid triggering any of our S4TF related code, when user code involves no tensors (the "pay only for what you use" principle in language/system design and impl).

[bgogul]

can you create a stand-alone unit test (which deals with tensors) that exposes this problem?

I will try to create one. I think I will have to create the Struct explicitly in the test.

i wonder if we should tweak the impl to avoid descending into a non-struct type, such as array?

Shouldn't we return true if the field is an array of tensors?

on structs like PairedStructure, which are not related to tensors / TF.

Don't we want to know if a struct has fields of Tensors? e.g. Parameters?

[mhong]

I played with this issue and understood it better now. The test I used is:

struct S {
  let sub: [S] = []
}

func foo() -> S {
  let s = S()
  return s
}

public func bar() {
  _ = Tensor<Float>(1.0)
  _ = foo()
}

In this case the sub field has type:

(array_slice_type
  (struct_type decl=tmp4.(file).S@test/TensorFlow/tmp4.swift:41:8))

So I agree this is a real issue. The fix looks fine. Another option that might be worth considering is extending TypeContainsTensorFlowValue::declContainsTensorFlowValue to also remember which decls / types do not contain tensorflow value. This way hopefully we can hide the info in parentDecls from the function interfaces like containsTensorFlowValueImpl(). Please assess and you can decide.

Following up on another point that was raised earlier -- I'm still curious how this issue got triggered for PairedStructure. I can understand how it's triggered in foo() / bar() above since they have tensor code.

Can you confirm that we will not be calling containsTensorFlowValue() unnecessarily (i.e., when we can decide from the code context that there is no tensor code)? In particular, the existing Swift unit tests (that also run in apple master branch) should hopefully not involve containsTensorFlowValue()?

[bgogul]

@swift-ci please test tensorflow