Skip to content

fixed: GToSchema: infinite call: single constructor and recur#37

Merged
maksbotan merged 5 commits into
biocad:masterfrom
ncaq:fix-single-constructor-recur
Mar 13, 2022
Merged

fixed: GToSchema: infinite call: single constructor and recur#37
maksbotan merged 5 commits into
biocad:masterfrom
ncaq:fix-single-constructor-recur

Conversation

@ncaq

@ncaq ncaq commented Feb 2, 2022

Copy link
Copy Markdown
Contributor

We combined Servant with OpenApi3 and found that the calculation does not finish if there is a cycled single field data type.
Since we were creating data types like the one described in our test data, and it was very annoying that we couldn't create OpenApi data, we investigated the cause and fixed it.
It seems that the test does not pass in the new version of GHC, and the same is true for the existing master, so we left it alone.

Thanks for the great library.
It would be great if you could incorporate it and release it.

ncaq added 3 commits February 2, 2022 20:13
why
===

I want to use GHC of supported [haskell/haskell-language-server: Official haskell ide support via language server (LSP). Successor of ghcide & haskell-ide-engine.](https://github.com/haskell/haskell-language-server).
I want timeout for test like below,

~~~hs
-- | create schema of self and ref another.
-- timeout in 1 seconds.
-- because avoid infinite call.
checkToSchemaDeclare :: (HasCallStack, ToSchema a) => Proxy a -> Value -> Spec
checkToSchemaDeclare proxy js = do
  mActuial <- runIO $ timeout (secondsToMicroseconds 1) $ evaluate $ force $ runDeclare (declareSchemaRef proxy) mempty
  case mActuial of
    Nothing -> it "test is timeout" $ mActuial `shouldNotBe` Nothing
    Just actuial -> actuial <=> js

secondsToMicroseconds :: Int -> Int
secondsToMicroseconds seconds = seconds * 10 ^ (6 :: Int)
~~~

However `Schema` is not `NFData`.
So I gave up.
I followed `declareSchemaRef` to make sure that the context of `InsOrdHashMap` disappears.
Since the cause seems to be here, I rewrote it to keep the context and continue the calculation.
@ncaq

ncaq commented Mar 10, 2022

Copy link
Copy Markdown
Contributor Author

I would like to see this MERGE because it fixes a bug that will be important there in using this library.
Is there something wrong with my PR?
Please point it out and I will fix it.
I don't know who to mentions it to, so I will notify as much as I can.

@maksbotan

@maksbotan

Copy link
Copy Markdown
Collaborator

Sorry @ncaq! Looks like I had wrong notifications configuration for this repo and did not notice this PR until you pinged me.

@maksbotan maksbotan merged commit f87e8b6 into biocad:master Mar 13, 2022
@ncaq ncaq deleted the fix-single-constructor-recur branch March 14, 2022 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants