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 an option to ConstantsScraper to import constants from another namespace #1890

Merged
merged 1 commit into from
May 6, 2024

Conversation

dpaoliello
Copy link
Contributor

Although ConstantsScraper can reference named constants from other header files, if that other header file's constants were placed into a different namespace (e.g., in a different WinMD file) then the C# generated is invalid.

This change adds a new option --useConstantsFrom that can be used to generate using static declarations for other namespaces that the current C# file may reference.

@mikebattista
Copy link
Contributor

We have ConstantsScraper.header.txt in the build. It's not clear what the relationship is between that and this.

@dpaoliello
Copy link
Contributor Author

It means that you can get rid of ConstantsScraper.header.txt and replace it with a bunch of --useConstantsFrom options. Better yet, you could dynamically change the set of using declarations instead of having a static list.

Having a static list like that doesn't work well when you are converting a sequence of header files that you want to keep in their own namespaces/assemblies. What I'm wanting to solve is a problem like this:

In one header file, let's call it Base.h, you have a constant:

#define BASE_CONSTANT 42

Now, in another header file, let call it Derived.h, you have a constant that uses the value from Base.h:

#define DERIVED_CONSTANT BASE_CONSTANT 

If you run ConstantsScraper on each of these header files individually, then each of these header files becomes a different C# file with a different namespace. So, for Derived.h you end up with Derived.cs which has:

public const int DERIVED_CONSTANT = BASE_CONSTANT;

Which doesn't compile because there is no using declaration that pulls in BASE_CONSTANT.

Instead, if we had --useConstantsFrom then I could add --useConstantsFrom=Base.Apis when generating Derived.cs which would produce:

using static Base.Apis;

public const int DERIVED_CONSTANT = BASE_CONSTANT;

Which will compile (if I'm building both C# files at the same time or set up the correct reference assemblies).

It is possible to do this with --headerText, but it means that you must create a temporary text file with your static header text and then append all the using declarations that you need:

type normalHeaderFile.txt > tempHeaderFile.txt
echo using static Base.Apis; >> tempHeaderFile.txt
dotnet ConstantsScrapter.dll .... --headerText=tempHeaderFile.txt

@mikebattista
Copy link
Contributor

You still have to manually list the dependent namespaces whether it's a parameter or a line in ConstantsScraper.header.txt.

It means that you can get rid of ConstantsScraper.header.txt and replace it with a bunch of --useConstantsFrom options. Better yet, you could dynamically change the set of using declarations instead of having a static list.

Sure we could, but I don't see a reason to do this. I need a better pitch.

@riverar
Copy link
Collaborator

riverar commented May 6, 2024

It's my understanding that Daniel is not proposing we change any workflows in this repository. I believe the example above was put together to demonstrate how the proposed parameter is useful in his scenario.

@dpaoliello
Copy link
Contributor Author

Sure we could, but I don't see a reason to do this. I need a better pitch.

It's my understanding that Daniel is not proposing we change any workflows in this repository. I believe the example above was put together to demonstrate how the proposed parameter is useful in his scenario.

100% this: I described my scenario in #1890 (comment), specifically the ability to dynamically generate using declarations as you are building a chain of C# files for headers.

@mikebattista
Copy link
Contributor

The part I'm stuck on is can't you have your own ConstantsScraper.header.txt in your project that lists the additional using statements you need to resolve references?

Or is your point that you have your own build process where you're calling tools like ConstantsScraper directly without using our MSBUILD SDK project, and this extra parameter is helpful in that scenario?

@dpaoliello
Copy link
Contributor Author

The part I'm stuck on is can't you have your own ConstantsScraper.header.txt in your project that lists the additional using statements you need to resolve references?

Or is your point that you have your own build process where you're calling tools like ConstantsScraper directly without using our MSBUILD SDK project, and this extra parameter is helpful in that scenario?

The second: we have our own build system that calls ConstantsScraper (and other tools) to generate WinMD files for our header files. We're building a sequence of WinMD files and so the set of using declarations we need to generate are dynamic depending on which header file we're currently reading, so a single static header.txt doesn't work.

@mikebattista mikebattista merged commit 602e5ba into microsoft:main May 6, 2024
1 check passed
@mikebattista
Copy link
Contributor

Ok. Thanks for clarifying. Makes sense.

@dpaoliello dpaoliello deleted the useconstfrom branch May 6, 2024 20:50
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

Successfully merging this pull request may close these issues.

None yet

3 participants