Skip to content

Fix #1251 three parameter equality operator#1776

Merged
tritao merged 3 commits intomono:mainfrom
Saalvage:fix/3-arg-eq-op
Oct 19, 2023
Merged

Fix #1251 three parameter equality operator#1776
tritao merged 3 commits intomono:mainfrom
Saalvage:fix/3-arg-eq-op

Conversation

@Saalvage
Copy link
Copy Markdown
Contributor

  • Operators in generic classes do not attempt to generate as extension methods anymore
  • Empty ...Extensions classes are no longer generated
  • string as a template argument is correctly cast
  • MarshalCharAsManagedChar option also generates correct casts
  • Suppress warning regarding returning struct field by ref
  • Eliminate some tabs that snuck into the test C++ header

Considerations:

  • Is there a better way to force template instantation than using them in a method? I tried explicit template instantiation, but it didn't seem to have an effect (new feature/bug?)
  • I'm not entirely sure if the IsTemplate extension method is entirely correct or if the same functionality might exist someplace else.

- Operators in generic classes do not attempt to generate as extension methods anymore
- Empty `...Extensions` classes are no longer generated
- `string` as a template argument is correctly cast
- `MarshalCharAsManagedChar` option also generates correct casts
- Suppress warning regarding returning struct field by ref
- Eliminate some tabs that snuck into the test C++ header
@tritao
Copy link
Copy Markdown
Collaborator

tritao commented Oct 19, 2023

Fixes #1251

@tritao
Copy link
Copy Markdown
Collaborator

tritao commented Oct 19, 2023

  • Is there a better way to force template instantation than using them in a method? I tried explicit template instantiation, but it didn't seem to have an effect (new feature/bug?)

Do you mean force the symbols to get exported in the shared library? If so, you can see how we do it in https://github.com/mono/CppSharp/blob/main/src/CppParser/Bindings/CSharp/i686-apple-darwin12.4.0/Std-symbols.cpp. There are some other techniques we use for forcing constructors to get exported for instance.

There is a flag to do it at the compiler level but Clang (and maybe GCC) for instance just ignores it and does nothing with it, so nothing reliable we can use.

  • I'm not entirely sure if the IsTemplate extension method is entirely correct or if the same functionality might exist someplace else.

Dunno, I guess it's fine to add a new helper.

Just wondering about the name, should it be IsTemplateParameterType instead?

The only other thing that comes to mind is maybe use it recursively:

        public static bool IsTemplateParameterType(this Type type)
        {
            if (type is TemplateParameterType or TemplateParameterSubstitutionType)
                return true;

            var ptr = type;
            while (ptr is PointerType pType)
            {
                ptr = pType.Pointee;
                if (ptr.IsTemplateParameterType(ptr))
                    return true;
            }

            return false;
        }

@tritao
Copy link
Copy Markdown
Collaborator

tritao commented Oct 19, 2023

You may be able to simplify it further with GetFinalPointee.

@Saalvage
Copy link
Copy Markdown
Contributor Author

Do you mean force the symbols to get exported in the shared library? If so, you can see how we do it in https://github.com/mono/CppSharp/blob/main/src/CppParser/Bindings/CSharp/i686-apple-darwin12.4.0/Std-symbols.cpp. There are some other techniques we use for forcing constructors to get exported for instance.

I don't think that is what I mean. If I try to do explicit template instantiation (template class Optional<unsigned int>;) CppSharp does not generate any C# code for the type. However, explicit template instantiations marked with DLL_API are included in the shared library from the C++ side. So it seems to be a case of CppSharp not accurately reflecting the shared libraries contents from what I can tell. If my understanding is correct in order to fix this it just needs to handle explicit template instantiations in the same way parameters are handled to generate the corresponding template types in the C# code.

Either way, if this turns out to be a bug/missing feature I think it's preferable to just open an issue and deal with it later, the method should be fine for now.

Just wondering about the name, should it be IsTemplateParameterType instead?

Yep! I wasn't happy with the name either, this fits much better.

You may be able to simplify it further with GetFinalPointee.

Very helpful! The code is much cleaner now!

@tritao tritao merged commit adffc99 into mono:main Oct 19, 2023
@Saalvage Saalvage deleted the fix/3-arg-eq-op branch October 19, 2023 13:21
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.

2 participants