-
Notifications
You must be signed in to change notification settings - Fork 212
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
FR: Use StringDict-like return value for Thread.Load #463
Comments
That's not true in general. If you intend to produce the documentation for all bindings in a module as a dynamic analysis--i.e. by loading the module, causing the execution of all its top-level statements including possible side effects on the host application, or even nontermination--then the exact definitions of everything loaded do potentially matter. (I think the Bazel documentation tools made a design mistake by taking the dynamic route.) Why not produce documentation statically, that is, by parsing the source file and looking for the statements that define top-level variables? This route is guaranteed to produce its answer deterministically, efficiently, and robustly. Loosely related: |
Correct, but good enough in terms of doc generation.
Although Starlark is designed to replace the way-too-flexible python used in the early versions of Bazel, the core concepts like Static analysis won't work for those dynamically defined constructors (and they are not rare!) That's why Stardoc is considering building the doc gen into bazel (https://github.com/bazelbuild/stardoc/blob/master/docs/future_plans.md). But I don't think that will come any time soon. Until then, I want to have a zero-config replacement for Stardoc which requires lots of instruments to get even the most basic docs. |
Fair point about the dynamism of the language being a limit on what you can achieve using the static approach. (I mistakenly thought you were making an analogy with .bzl as opposed to meaning it literally.)
I'm glad you linked the Starlark future plans doc, because I think it is evidence that the "dynamic fake" approach is actually not good enough for doc generation, and that really it's better to get the build tool to execute the initialization of the .bzl files using 100% accurate semantics and then dump an easily parseable representation of the dynamic effects (e.g. rules, providers, aspects) from which you can generate documentation. (I argued for this approach while I was working on the Java implementation of Starlark used by Bazel, as Stardoc was the reason for a large number of hacky intrusions into the intepreter that would not have been necessary had Stardoc called "bazel query" as a subprocess.) |
I'd love to have this built into the bazel as a subprocess. But I'm a little skeptical about when it will be ready (almost 2 years overdue already). My somewhat production-ready experiment with the patched starlark-go is about 1000-loc, and already delivering better results compared to stardoc, moreover, it doesn't need From this perspective, I really want to have this patch to be merged (after some revising on naming and doc). |
Thread.Load cannot be changed without breaking API compatibility. We could add a "Thread.Load2" hook that takes precedence, but I still don't really understand how StringDictLike is materially different from StringDict. |
Here is how this trick plays out. Instead of loading top-level variables, I can pretend that they are loaded -- just unknown. In Bazel context, this won't hurt much -- as a rule rarely need a symbol to complete rule definition -- they might need a foreign symbol to complete its implementation. func (ld *Loader) load(t *starlark.Thread, module string) (starlark.StringDictLike, error) {
l, err := label.Parse(module)
if err != nil {
return nil, err
}
fsys, ok := ld.moduleRoots[l.Repo]
if !ok {
return unknownModule{l.String()}, nil
}
...
}
type unknownModule struct{ name string }
func (m unknownModule) Keys() []string { return nil }
func (m unknownModule) Has(name string) bool {
glog.Warningf("Unresolved symbol %q from %q", name, m.name)
return true
}
func (m unknownModule) Get(name string) starlark.Value {
return UnknownSymbol{}
} I'm open to the idea of introducing Load2, but I'd argue that starlark-go is not hitting v1.0 yet, and perhaps a cleaner API is preferable? |
I still don't understand. If the point of the trick is to allow you to execute the file containing the load statement without knowing anything about the module that it loads, then what will happen when execution encounters a statement that causes Get to be called? It will create an UnknownSymbol value and then execution will go wrong. How is that useful? If you don't care about the actual values in the module it's easy to fake one by creating a real StringDict whose keys are the names mentioned in the load statement and whose values are all UnknownSymbol. |
Frist of all the UnknownSymbol is both callable and acts as a dict and will always return another UnknownSymbol for further invocations. type UnknownSymbol struct{}
var (
_ starlark.HasAttrs = UnknownSymbol{}
_ starlark.Callable = UnknownSymbol{}
) Second, generating docs only requires executing the part of code that declares This statement holds against a dozen of top rulesets.
Well, this is not easy to pull off with the existing high-level APIs:
The alternative is to use:
However, this requires no API change, and I'm willing to give it a shot. |
Right, that's what I had in mind. I don't think you need Walk, BTW, since LoadStmts are all at top level. |
I'm implementing a stardoc-like program (extracting docs from bzl files). In such a program, we don't really need to know the concrete types of foreign symbols. For now, the
Thread.Load
returns aStringDict
, which means I need to resolve the loading bzl path, and exec that file just to make theExec
work.I'm proposing that let
Thread.Load
returns aStringDict
-like interface withGet
andKeys
methods instead. Which will allow users to mock a load.The
Thread.Load
is used as follow:starlark-go/starlark/interp.go
Lines 566 to 587 in 4b1e35f
The text was updated successfully, but these errors were encountered: