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

Fix #1251 three parameter equality operator #1776

Merged
merged 3 commits into from
Oct 19, 2023
Merged

Conversation

Saalvage
Copy link
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
Collaborator

tritao commented Oct 19, 2023

Fixes #1251

@tritao
Copy link
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
Collaborator

tritao commented Oct 19, 2023

You may be able to simplify it further with GetFinalPointee.

@Saalvage
Copy link
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
5 checks passed
@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