C++ Interop: import const methods as non-mutating

[egorzhdan]

This change makes ClangImporter import some C++ member functions as non-mutating, given that they satisfy two requirements:

• the function itself is declared as const
• the parent struct doesn't contain any mutable members

get accessors of subscript operators are now also imported as non-mutating if the C++ operator[] satisfies the requirements above.

In the future we should allow annotating a C++ method as mutating explicitly, to make it possible to preserve the correct behavior for methods that are declared const, but mutate the object anyway, for example via const_cast.

Fixes SR-12795.

[egorzhdan]

@swift-ci please smoke test

[zoecarver]

It would be cool to see some performance improvements here. Unfortunately, we don't have enough C++ interop performance tests yet.

Anyway, thanks for the patch, as always!

[egorzhdan]

@swift-ci please smoke test macOS

[egorzhdan]

Yeah, looks like the C++ interop performance tests currently verify object instantiation, but not method calls.

[LucianoPAlmeida]

Hey @egorzhdan :)
It seems like this caused a failure on tests https://ci.swift.org/job/swift-PR-macos/28502/testReport/junit/Swift(watchsimulator-i386)/ClangImporter/cxx_interop_ir_swift/ when running targeting i386 watch simulator, noted when running tests for #38464. By the logs of why is failing, my guess would be since i386 is a 32-bit this alloca %TSo7MethodsV, align 8 expects 4 bytes align 4. Is that right?

[shahmishal]

To unblock CI, I created a PR to disable the test and filed a issue:
#38690
https://bugs.swift.org/browse/SR-14986

[egorzhdan]

Sorry for breaking the test!
@LucianoPAlmeida yes, you're right, the test should accept align 4 on a 32-bit arch.
I submitted a patch with a fix: #38692.

[LucianoPAlmeida]

Sorry for breaking the test!
@LucianoPAlmeida yes, you're right, the test should accept align 4 on a 32-bit arch.
I submitted a patch with a fix: #38692.

Thank you for the fix :)

[varungandhi-apple]

Also, for the future, if you're changing code in the ClangImporter, IRGen or the runtime, it is better to err on the side of caution and run full testing instead of smoke testing as there are often platform differences which aren't caught by smoke testing.

[egorzhdan]

Also, for the future, if you're changing code in the ClangImporter, IRGen or the runtime, it is better to err on the side of caution and run full testing instead of smoke testing as there are often platform differences which aren't caught by smoke testing.

Yes, that's reasonable, thanks!