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 support for constants #5

Open
PathogenDavid opened this issue Aug 30, 2020 · 4 comments
Open

Add support for constants #5

PathogenDavid opened this issue Aug 30, 2020 · 4 comments
Labels
Area-Translation Issues concerning the translation from libclang into Biohazrd Concept-CppFeatures Issues concerning unsupported C++ features

Comments

@PathogenDavid
Copy link
Member

IE:

const int SomeConst = 100;

One consideration to be made for these types: I believe C++ guarantees that &SomeConst is the same across all translation units. (If not, I bet most compilers do this anyway.) Making this guarantee means we can't let the constant be a C# const since C# does not allow taking the address of a constant. (And even if it did, the address would be different.)

There are essentially three ways to represent C++-style constants in C#:

  • A) Export a pointer to the const and use NativeLibrary.GetExport
    • This is how non-const globals work today.
    • ❌ This isn't ideal because it requires a stub.
    • ❌ This has negative performance implications (neither Roslyn nor the JIT would recognize this as a constant.)
    • ❌ The const-ness of the variable will not be exposed in C# land.
      • It's basically not a const anymore, although we could use an analyzer.
      • This means it could be modified by mistake, which would cause a runtime exception on some platforms.
    • ✔ This satisfies the previously mentioned pointer guarantee.
    • ✔ Supports types that C# doesn't support as const.
  • B) Expose the const as an actual C# constant
    • ❌ Only works with the few types C# allows as a constant
    • ❌ Can't take a pointer to them (for libraries that care.)
    • ✔ Is a compile-time constant
    • ❌ Would be problematic if the value of the constant is platform-dependent
  • C) Expose the const as a static readonly field
    • ✔ Works with all types that we can translate
    • ✔ Is a JIT-time constant
    • ❌ Has negative performance implications
    • ❌ There are some situations where compile-time constants are needed.
    • ❌ You can take the address of a static readonly field, but we can't provide pointer same-ness guarantees.
      • Could restrict this with an analyzer, but realistically this rarely matters. Maybe make it opt-in for libraries that care.

I think the right solution here is C, but will revisit when I go to implement.

@PathogenDavid PathogenDavid added Area-Translation Issues concerning the translation from libclang into Biohazrd Concept-CppFeatures Issues concerning unsupported C++ features labels Sep 21, 2020
@PathogenDavid
Copy link
Member Author

PathogenDavid commented Oct 27, 2020

B is actually much more viable as of e4c8c7e (which was implemented for #75.)

This will probably be the preference with one of the other options as a fallback.

@PathogenDavid
Copy link
Member Author

Whenever we do this, make sure to add relevant support to MoveLooseDeclarationsIntoTypesTransformation for C#.

@PathogenDavid
Copy link
Member Author

We might want to use InlineExportHelper to expose constants which cannot be converted to C# constants.

PathogenDavid added a commit that referenced this issue Dec 6, 2021
Note that Biohazrd doesn't currently emit these declarations on its own, it's currently only for use in transformations.
Additionally the C# generator cannot handle emitting constants which cannot be constant in C#. (Not that Biohazrd's ConstantValue infrastructure supports such constants in the first place.
Contributes to #5
@PathogenDavid
Copy link
Member Author

TranslatedConstant was added in 9a2e781, but it's currently not emitted by Biohazrd its self. (It can however be used in transformations.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Translation Issues concerning the translation from libclang into Biohazrd Concept-CppFeatures Issues concerning unsupported C++ features
Projects
None yet
Development

No branches or pull requests

1 participant