Looking at match01 --- flakey test #6673
Conversation
KevinRansom
force-pushed
the
opttest
branch
4 times, most recently
from
b2fb111 to
f3eb3ac
Compare
dsyme
left a comment
dsyme
left a comment
There was a problem hiding this comment.
The PR basically looks good apart from the addition of the val_global_state field - we must be able to avoid that. And if we're going to add a field we would add TcGlobals to all the primary objects
src/fsharp/xlf/FSComp.txt.es.xlf
Outdated
src/fsharp/tast.fs
Outdated
There was a problem hiding this comment.
This is a problem - it is adding a word to the critical Val data structure, which we have worked very hard to remove every word from.
I will search around for what makes this neccessary
src/fsharp/tast.fs
Outdated
There was a problem hiding this comment.
I guess we should just pass g into CompiledName
There was a problem hiding this comment.
I would love to have passed around TcGlobals, however, it uses types from tast.fs, and so I can't pass tcGlobals that far back
|
|
|
@dsyme, I will give that a look. |
| member x.DebugText = x.ToString() | ||
|
|
||
| override x.ToString() = sprintf "TBind(%s, ...)" x.Var.CompiledName | ||
| override x.ToString() = sprintf "TBind(%s, ...)" (x.Var.CompiledName None) |
There was a problem hiding this comment.
@ dsyme … This is why CompilerGlobalSources is an option.
There is no way to get tcglobals to here,
There was a problem hiding this comment.
Nit: whitespace before sprintf.
|
@dsyme redone without the extra field. I think there may be even more plumbing in this one, sigh !!!! but it doesn't bloat the val record. Let me know what you think |
|
passed |
|
passed |
|
passed |
|
passed |
|
passed |
|
@KevinRansom It's passed for quite some time now... |
|
@cartermp, I know, but it was intermittent failing, I wanted to be sure … the fix really fixed it. |
4 participants
Okay … what was happening is … the StableNameGenerator is global. The fsharpqa tests use a compiler server to reduce build time. Since the fsharpqa test framework parallelizes the tests and they can run in different orders, causing the stablenamegenerator to create different names, the
This change attaches the stablenamegenerator to TcGlobals and plumb in tcGlobals, to a number of low level ast generation apis.
Because tcGlobals is recreated for every compilation, the Match01 optimization test case uses a bsl file, and so we noticed the name change.
I made a new type CompilerGlobalState to hang this type of state, because tcGlobals needs to appear after tast.fs.
Fixes #6657