-
Notifications
You must be signed in to change notification settings - Fork 66
Cabal flag to control auto-labelling of threads #164
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
6035549
to
5a7bd63
Compare
I'm open to the idea of labelling the internal threads - we now have two PRs for that (#162 and #163, I slightly prefer #162). But I'm not keen on adding auto-labelling functionality via cabal flags and CPP. Could you do this using a wrapper package |
That won't be ergonomic. If there's some other pakcage using Compared to adding |
I agree with @phadej. I think the functionality of auto labelling the threads is a very big win. In terms of comparing #162 and #163, I think mine is more minimal as it labels internal threads. The other one does also label the threads the user spawns. That should usually be a task for the user who presumably would want different names for his actions. |
Control/Concurrent/Async/Internal.hs
Outdated
#ifdef DEBUG_AUTO_LABEL | ||
HasCallStack => | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would an alias work there? Like the following:
#ifdef DEBUG_AUTO_LABEL
import qualified GHC.Stack
#else
import qualified GHC.Exts
#endif
...
#ifdef DEBUG_AUTO_LABEL
type HasCallStack = GHC.Stack.HasCallStack
#else
type HasCallStack = () :: GHC.Exts.Constraint
#endif
That way one won't need to put ifdef
in every single type signature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing. I prioritized being explicit everywhere but what you suggest sounds good to me too.
I'm not sure if the suggested code works for as far back as 7.0.1 which is the one with base 4.3, and I don't know how I could test it or even if I can install such old versions still. In particular it seems I need |
Having this cabal flag allows the user to, on-demand, auto-label the async threads, pointing to the place where the async task is spawned. The idea is that the user then can go to the code/dependency and add a meaningful label to that thread.
Using CPP makes it transparent for users that don't want to opt-in to this behavior.
My idea is for this PR to be merged after #163. The first commit in this branch is the same as in #163.