Biohazrd icon indicating copy to clipboard operation
Biohazrd copied to clipboard

Add support for constants

Open PathogenDavid opened this issue 3 years ago • 4 comments

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 avatar Aug 30 '20 19:08 PathogenDavid

B is actually much more viable as of https://github.com/InfectedLibraries/Biohazrd/commit/e4c8c7e6c467d84dc062380d7c299bf8958628dc (which was implemented for #75.)

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

PathogenDavid avatar Oct 27 '20 23:10 PathogenDavid

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

PathogenDavid avatar Mar 30 '21 08:03 PathogenDavid

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

PathogenDavid avatar Sep 30 '21 21:09 PathogenDavid

TranslatedConstant was added in https://github.com/MochiLibraries/Biohazrd/commit/9a2e781a44f0b201d6c57687c5fed3f86c6c2651, but it's currently not emitted by Biohazrd its self. (It can however be used in transformations.)

PathogenDavid avatar Dec 06 '21 20:12 PathogenDavid