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

Redesign implicit import implementation #1447

Closed
rainersigwald opened this issue Dec 6, 2016 · 2 comments
Closed

Redesign implicit import implementation #1447

rainersigwald opened this issue Dec 6, 2016 · 2 comments
Assignees
Labels
Milestone

Comments

@rainersigwald
Copy link
Member

rainersigwald commented Dec 6, 2016

As implemented (in the parser), implicit XML elements have caused a fair amount of surprise and bug tail (#1416, #1431, #1445) related to having differences between the ProjectElement tree exposed through the construction APIs and the Xml tree. We should redesign to avoid this.

I spoke to @jeffkl about this and he thought moving the transformation to be just before evaluation/preprocessing begins would be cleaner: Construction API consumers would see only the project file as it exists on disk, but the evaluator would get the complete tree as it expects. That sounds like a good plan to me.

We could also potentially go with @dsplaisted's preferred approach of implementing implicits entirely in the evaluator, but my first attempts to do so were stymied by assumptions about how the evaluation tree and the XML tree were intertwined.

@rainersigwald rainersigwald added this to the Feature: Sdks milestone Dec 6, 2016
@jeffkl
Copy link
Contributor

jeffkl commented Dec 6, 2016

@AndyGerlicher had two questions:

  1. What the implications to CPS
  2. Would properties be evaluated correctly from the implicit imports?

The property evaluation was something I didn't consider, if you load the project and ask for a property value that comes from the implicit import, you would expect to get it. Could the implicit imports only be evaluated in PerformDepthFirstPass?

@jeffkl
Copy link
Contributor

jeffkl commented Dec 6, 2016

jeffkl added a commit to jeffkl/msbuild that referenced this issue Dec 16, 2016
…g parsing of projects.

Had to do some hacky stuff in the preprocessor but all of that code is private and we can make it better in the future.

Closes dotnet#1447
jeffkl added a commit to jeffkl/msbuild that referenced this issue Dec 16, 2016
…g parsing of projects.

Had to do some hacky stuff in the preprocessor but all of that code is private and we can make it better in the future.

Closes dotnet#1447
jeffkl added a commit to jeffkl/msbuild that referenced this issue Jan 5, 2017
…g parsing of projects.

Had to do some hacky stuff in the preprocessor but all of that code is private and we can make it better in the future.

Closes dotnet#1447
AndyGerlicher pushed a commit that referenced this issue Jan 10, 2017
…ng (#1492)

* Implementation of implicit imports during evaluation instead of during parsing of projects.

Had to do some hacky stuff in the preprocessor but all of that code is private and we can make it better in the future.

Closes #1447
jeffkl added a commit to jeffkl/msbuild that referenced this issue Mar 28, 2017
…g parsing of projects.

Had to do some hacky stuff in the preprocessor but all of that code is private and we can make it better in the future.

Closes dotnet#1447
@AR-May AR-May added the triaged label Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants