[ModuleTrace] Handle cycle detection edge case in module trace emission (ep2). #33640
Conversation
…on (ep2). Context: rdar://67704000.
|
@swift-ci please smoke test |
lib/FrontendTool/FrontendTool.cpp
Outdated
| /// | ||
| /// This happens when we have a dependency like: | ||
| /// \code | ||
| /// Overlay (Swift) -> sandwichedModule -> Underlying (Clang) | ||
| /// Overlay (Swift) -> sandwichedModule -> .. -> Underlying (Clang) |
There was a problem hiding this comment.
@varungandhi-apple With both more distance from the original PR and the description of the cycles it detects becoming more abstract, I find I'm not really clear about what isFakeCycleThroughOverlay() should do. I think this is down to the doc comment requiring too much context to understand. It ought to describe:
- When these cycles occur
- What should be done about them (I believe this is "ignore them because they're benign", but that's not clear from the comment)
- What the relationship should be between the various modules involved in them (e.g. is
Overlayalways the overlay ofUnderlying? Can there be other overlays or other underlying modules between them? How issandwichedModulerelated to them? How are the other modules in between related to them?)
I think I'll have more success reviewing the actual implementation once I can compare it to a clearer description of what it ought to do.
There was a problem hiding this comment.
I've tried to explain things with a concrete example and point out what can happen in the most general case, instead of focusing on the most general thing in the comment. Is it better now?
varungandhi-apple
force-pushed
the
vg-fix-module-trace-cycle
branch
from
534a14d to
daaf609
Compare
|
@swift-ci please smoke test |
varungandhi-apple
force-pushed
the
vg-fix-module-trace-cycle
branch
from
daaf609 to
500d505
Compare
|
@swift-ci please smoke test |
…on (ep2). Fixes rdar://67704000.
varungandhi-apple
force-pushed
the
vg-fix-module-trace-cycle
branch
from
500d505 to
3e7f339
Compare
|
@swift-ci please smoke test and merge |
|
It doesn't seem to have started. Strange. @swift-ci please smoke test and merge |
|
@swift-ci please smoke test macOS |
Fixes rdar://67704000.