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

Add Driver.Context setter #89081

Merged

Conversation

mrvoorhe
Copy link
Contributor

We implement our own Driver.SetupContext. Our command line parsing logic is different and we have additional command line options.

In our very old revision of the upstream linker there was

protected LinkContext context;

Which we would set. This would ensure that if one of the helper methods that uses the context were called that things would all work. The most common scenario seems to be for error logging. There are many helper methods that will call Context.LogError and if we haven't set the context field by then we have a problem.

We are syncing up and now the context field is private and there is a protected property getter, but no setter.

Adding a setter was the easiest way to get our context stored in the context field.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Jul 18, 2023
@ghost ghost added linkable-framework Issues associated with delivering a linker friendly framework community-contribution Indicates that the PR has been added by a community member labels Jul 18, 2023
@ghost
Copy link

ghost commented Jul 18, 2023

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details

We implement our own Driver.SetupContext. Our command line parsing logic is different and we have additional command line options.

In our very old revision of the upstream linker there was

protected LinkContext context;

Which we would set. This would ensure that if one of the helper methods that uses the context were called that things would all work. The most common scenario seems to be for error logging. There are many helper methods that will call Context.LogError and if we haven't set the context field by then we have a problem.

We are syncing up and now the context field is private and there is a protected property getter, but no setter.

Adding a setter was the easiest way to get our context stored in the context field.

Author: mrvoorhe
Assignees: -
Labels:

linkable-framework, community-contribution, area-Tools-ILLink

Milestone: -

We implement our own `Driver.SetupContext`.  Our command line parsing logic is different and we have additional command line options.

In our very old revision of the upstream linker there was
```
protected LinkContext context;
```

Which we would set.  This would ensure that if one of the helper methods that uses the context were called that things would all work.
The most common scenario seems to be for error logging.  There are many helper methods that will call `Context.LogError` and if we haven't set the `context` field by then we have a problem.

We are syncing up and now the `context` field is private and there is a protected property getter, but no setter.

Adding a setter was the easiest way to get our context stored in the `context` field.
@mrvoorhe mrvoorhe force-pushed the linker-expose-context-for-setting branch from 5fbca84 to 9c9016c Compare July 19, 2023 13:05
@vitek-karas vitek-karas merged commit 50ba405 into dotnet:main Jul 19, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers community-contribution Indicates that the PR has been added by a community member linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants