Skip to content
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

clarify interaction with analyzer #4

Open
jmesserly opened this issue Mar 5, 2015 · 9 comments
Open

clarify interaction with analyzer #4

jmesserly opened this issue Mar 5, 2015 · 9 comments

Comments

@jmesserly
Copy link

@bwilkerson wrote dart-archive/dart_enhancement_proposals#6 (comment):

The proposal states:

An analysis tool, like the Dart Analyzer, can see all possible import strings, and either resolve to a single URI for a given platform/configuration, or examine all the combinations.

This is not true (or at the very least is misleading). The analyzer does not require the values of "environment" variables to be specified. Therefore, it will not, in general, be able to determine which import will be selected, and will be forced to analyze every such library against all possible combinations of imports. This will significantly increase the amount of analysis that needs to be performed, which could have serious performance implications. (Three imports with two options each will result in analyzing the library 8 times. Any library that directly or indirectly imports the library containing the imports will (in the worst case scenario) likewise need to be analyzed 8 times.)

@jmesserly jmesserly changed the title clarifiy interaction with analyzer clarify interaction with analyzer Mar 5, 2015
@jmesserly
Copy link
Author

@bwilkerson just brainstorming here, would it be possible for analyzer to have some notion of the configuration? (with a reasonable default state if not provided)

e.g. in C# has (restricted form of) preprocessor directives, which ultimately can affect how C# analysis will interprets the code, and the view of the API that the user of that code sees (via analysis). The "project configuration" controls the set of defines passed to the compiler/analyzer. Then in the editor UI, it's easy to switch between configurations.

@bwilkerson
Copy link

It is possible for users to provide the values of these variables as "-D" flags on the command-line when running the command-line tool. I don't have any data on what percentage of the time these values are provided. But I'm not thinking primarily about the command-line tool; this has implications for other tooling.

I think it is no problem to run the analyzer 8 times if it does 8 different jobs.

It's inconvenient if the user has to run it 8 times, but I agree that it's not a problem for the command-line tool. :-)

The problem, which I should have made more clear, is that the analysis engine isn't just used by the command-line tool. It's also used in the context of editors and IDE's. Those contexts generally, and rightly, don't have any notion of a single "special" configuration because the user is editing all configurations simultaneously. In those cases, we will need to perform all possible analyses in order to be able to provide feedback such as "The type of this argument is not compatible with the type of the parameter when the configuration is ...".

And yes, there are plenty of opportunities for optimizations here. Most of the time the public API of the optionally imported libraries should be identical, and we really only need to verify that that's the case. (In fact, the developer should be told when that's not the case because it likely indicates a problem.) But no matter how much we optimize there will always be more work to do as a result of using this feature.

But that's not the biggest problem. This proposal seriously complicates the tooling story. When a user wants to navigate from the invocation of a method to the implementation of that method there will be multiple possible implementations to consider. Even assuming that the signature of the method is identical in all implementations, there are still multiple implementations to consider. The same can be said of hover text displaying the Dartdoc for the method. There are now multiple docs to display. And code completion needs to take into account any platform specific behavior and decide whether (and how) to display such things. The list goes on.

It will also make it harder for people who want to build their own tools on top of the analysis engine by adding another level of complexity that they have to be aware of. Whether we're talking about framework developers adding plugins to analysis engine and analysis server, or people writing transformers, or contributors of lint rules, or anything else, this will raise the barrier of entry. (Not that it isn't already fairly high.)

But note that all of this is not so much an argument against the proposal. I understand the need for something like this and hope that users will use this feature sparingly and thereby minimize the impact on tooling. My goal is to try to make sure that everyone is aware of the cost of such a feature. When the implementation in the VM and even dart2js is described, it doesn't sound like it will cost very much. But the more significant cost is in providing tooling for such a feature.

@eernstg
Copy link

eernstg commented Mar 5, 2015

I agree entirely that it's much more difficult to provide good tools for any kind of software product line (e.g., a set of configurations using configurable imports) than it is for a single program, and also that it would be a big software engineering loss if the tools were to insist that you must choose one particular configuration and then work on the software from that point of view only. I guess my only point was that this extra difficulty is to be expected, which also means that nobody could be surprised that the good tooling for this more difficult problem doesn't exist already, and it will take a substantial effort to create it. Supporting configurable imports with a commitment to just one configuration (the -D trick) at a time might be a useful way to get started, though.

@munificent
Copy link

But note that all of this is not so much an argument against the proposal.

Personally, I think it is an argument against this proposal. Whether or not it's an overwhelming one is a separate question. But your concerns about tooling, which I share, do, I think make this proposal less compelling compared to alternatives.

I understand the need for something like this and hope that users will use this feature sparingly and thereby minimize the impact on tooling.

Even if used sparingly, the tooling still has to take the fixed cost of implementing the support for configurations in general. Likewise, users have to take the mental hit to load this concept into their brains.

@BillDStrong
Copy link

The only reason I could see for this proposal would be to allow a package to provide tools for the client and the server. In such and implementation, I envision a library would import a client version of say mongoDB and a server version of mongoDB. This only makes sense to me if they are providing one interface on both platforms, and handling all the syncing issues in the library, as an example.

What other use cases do all of you see for this?

As for tooling, this could be very complex, but would we reduce the complexity by implicitly forcing this type of use case? Or am I being to narrow minded?

@bwilkerson
Copy link

As for tooling, this could be very complex, but would we reduce the complexity by implicitly forcing this type of use case?

It would reduce some of the complexity. For example, I had suggested earlier that we could analyze the importing library once if the public interface was the same across all of the potentially imported libraries. That would eliminate the multiplicative effect of having multiple configured imports in the same library (either directly or indirectly). It would also address the code completion issues.

But it doesn't help with the navigation issues (for example, when you want to go to the declaration of an imported type, which declaration do you want to see?), nor even questions like which Dartdoc to display when hovering over an imported name.

It does raise the question of how to enforce the constraint. If we made it a compile time error for the public API's to be different (and note that the "public API" is not the same as the public namespace) then every platform has to be able to access and fully resolve every import. Given that one purpose of this proposal is to handle cases where an import is not appropriate for some platforms, that would be problematic. If we only made it a warning then we'd have to define how analysis should proceed when the API's are different.

@BillDStrong
Copy link

Just an off the cuff thought, could we make one of the implementations canonical, and use it as a sort of litmus test?

Also, more tooling I know, but could the Dartdoc be constructed by combining the two implementations of exposed api? This would solve that particular headache, and make the developer aware of the dual nature of the package.

Navigation wise, wouldn't you only want the package author aware of the duality of the package? For error messages propagated to the consumers, maybe denoted by those that have the package in their package list, warning errors would only show for whatever expected type.

Tooling for this issue becomes harder the more I think of it.

And my own suggestion was mostly tooling based to boot, located here.

#9

@zoechi
Copy link

zoechi commented Mar 7, 2015

There might be situations where it would be nice when the tools consider all platform implementations but when I build a client application it would annoy me a lot when the editor asks me whether it should navigate to the standalone or browser implementation (same the other way around).

When I write a cross-platform package I guess I might concentrate on one platform implementation and then on the next.
I would want to be able to run every unit tests once for each platform automatically though.
The main situation where every platform specific implementation should be considered is when I write a platform-independent package which itself depends on platform-independent packages.

I guess most of the time I would program against a common interface (abstract class) which will be implemented by the platform specific implementations to abstract API differences away.
In this case the navigation would just lead to the interface anyway. "Find implementations" should find implementations across each platform though.

@lrhn
Copy link
Owner

lrhn commented Mar 19, 2015

The interaction with static analysis is why this proposal was rejected by the DEP committee as needing more work.
The dynamic behavior is generally trivial, no matter what syntax or features we decide on, so for that we just need to pick something usable.
The big question is which questions we want to be able to answer statically, and which features we are willing to give up to achieve that. There is a very large range of options, and it's not clear where the best tradeoff lies.

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

No branches or pull requests

7 participants