[Experimental Dependencies] Add -enable-experimental-dependencies and push it through. #20078
Conversation
davidungar
force-pushed
the
manual-rebasing-exp-deps
branch
3 times, most recently
from
0e83ce8 to
8b92a6b
Compare
|
@swift-ci please smoke test os x platform |
davidungar
force-pushed
the
manual-rebasing-exp-deps
branch
from
8b92a6b to
1f4dfce
Compare
|
@swift-ci please smoke test os x platform |
include/swift/AST/Decl.h
Outdated
|
|
||
| /// Scaffolding to permit experimentation with finer-grained dependencies and | ||
| /// faster rebuilds. | ||
| bool getEnableExperimentalDependencies() const; |
There was a problem hiding this comment.
Are you expecting this to be a per-Decl or per-DeclContext setting? If not, please don't do this; just stick to the longer getASTContext().LangOpts. EnableExperimentalDependencies.
| @@ -234,7 +238,8 @@ class Compilation { | |||
| Optional<unsigned> BatchSizeLimit = None, | |||
| bool SaveTemps = false, | |||
| bool ShowDriverTimeCompilation = false, | |||
| std::unique_ptr<UnifiedStatsReporter> Stats = nullptr); | |||
| std::unique_ptr<UnifiedStatsReporter> Stats = nullptr, | |||
| bool EnableExperimentalDependencies = false); | |||
There was a problem hiding this comment.
Getting convinced we need a CompilationOptions struct, but that doesn't have to be part of this patch.
There was a problem hiding this comment.
Yes, good points.
include/swift/Option/Options.td
Outdated
| @@ -129,6 +129,10 @@ def driver_always_rebuild_dependents : | |||
| Flag<["-"], "driver-always-rebuild-dependents">, InternalDebugOpt, | |||
| HelpText<"Always rebuild dependents of files that have been modified">; | |||
|
|
|||
| def enable_experimental_dependencies : | |||
| Flag<["-"], "enable-experimental-dependencies">, Flags<[FrontendOption, HelpHidden]>, | |||
| HelpText<"When interface changes, be more selective">; | |||
There was a problem hiding this comment.
Nitpick: I don't understand this help text.
There was a problem hiding this comment.
It is a bit terse, but I'm not sure exactly what would be needed to make it understandable--I'm too close. Could you suggest something?
There was a problem hiding this comment.
I don't think there's context in what the "interface" is, but even knowing that this means "a file's semantic and binary interface as presented to the rest of the module", it doesn't say what the compiler is going to be more selective about.
There was a problem hiding this comment.
Thanks! Will fix.
There was a problem hiding this comment.
Just pushed a fix. Hope it's better.
| return DependencyGraphImpl::loadFromPath(Traits::getAsVoidPointer(node), | ||
| path); | ||
| LoadResult loadFromPath(T node, StringRef path, | ||
| const bool EnableExperimentalDependencies) { |
There was a problem hiding this comment.
Are you even going to use the same dependency graph structure? It might be better to not bother with this flag at all.
(Then again, you can always take it back out.)
There was a problem hiding this comment.
Yes, you are right! Taking it out.
davidungar
force-pushed
the
manual-rebasing-exp-deps
branch
from
19cd39b to
19caf59
Compare
| @@ -288,6 +288,10 @@ namespace swift { | |||
| /// Whether to verify the parsed syntax tree and emit related diagnostics. | |||
| bool VerifySyntaxTree = false; | |||
|
|
|||
| /// Scaffolding to permit experimentation with finer-grained dependencies | |||
| /// and faster rebuilds. | |||
| bool EnableExperimentalDependencies = false; | |||
There was a problem hiding this comment.
Another option is to put this on a SourceFile's ReferencedNameTracker. I'm not sure if that's better since it means putting this flag in FrontendOptions just to pass it through, but it might be easier for whatever implementation you end up with to have the flag right there. Since that's inside the frontend, though, you can always change it later.
There was a problem hiding this comment.
Good thought, will keep it in mind.
|
@swift-ci please smoke test and merge |
|
@jrose-apple Thanks for the review. It helps to keep the quality and consistency up. |
|
@swift-ci please smoke test |
|
@swift-ci please smoke test os x platform |
|
@swift-ci please smoke test linux platform |
Adds -enable-experimental-dependencies option and pushing it through to the places where it will be needed in the frontend and driver. Getting this in now will reduce future rebasing work.