8263135: unique_ptr should not be used for types that are not pointers

[YaSuenag]

I saw error during jpackage compilation with VS 2019 (16.9.0) as following (on Japanese locale):

c:\progra~2\micros~2\2019\commun~1\vc\tools\msvc\1428~1.299\include\utility(604): error C2440: '=': '_Other' から '_Ty' に変換できません。
        with
        [
            _Other=nullptr
        ]
        and
        [
            _Ty=unsigned long
        ]
c:\progra~2\micros~2\2019\commun~1\vc\tools\msvc\1428~1.299\include\utility(604): note: ネイティブの nullptr はブールに変換するか、または reinterpret_cast を使用して整数型に変換することのみが可能です
c:\progra~2\micros~2\2019\commun~1\vc\tools\msvc\1428~1.299\include\memory(3423): note: コンパイル対象の関数 テンプレート インスタンス化 '_Ty std::exchange<_Ty2,nullptr>(_Ty &,_Other &&) noexcept(false)' のリファレンスを確認してください
        with
        [
            _Ty=unsigned long,
            _Ty2=unsigned long,
            _Other=nullptr
        ]
c:\progra~2\micros~2\2019\commun~1\vc\tools\msvc\1428~1.299\include\memory(3422): note: クラス テンプ レート メンバー関数 'unsigned long std::unique_ptr<MSIHANDLE,msi::MsiHandleDeleter>::release(void) noexcept' のコンパイル中
d:\github-forked\jdk\src\jdk.jpackage\windows\native\common\MsiDb.cpp(237): note: コンパイル対象の関数 テンプレート インスタンス化 'unsigned long std::unique_ptr<MSIHANDLE,msi::MsiHandleDeleter>::release(void) noexcept' のリファレンスを確認してください
d:\github-forked\jdk\src\jdk.jpackage\windows\native\common\MsiDb.h(119): note: コンパイル対象の クラ ス テンプレート インスタンス化 'std::unique_ptr<MSIHANDLE,msi::MsiHandleDeleter>' のリファレンスを確認してください

UniqueMSIHANDLE is declared in MsiUtils.h as unique_ptr for MSIHANDLE. MSIHANDLE seems to be declared as synonym for unsigned long, not a pointer type.
I think MSIHANDLE does not need to handle as unique_ptr if it releases at d'tor of the class which holds it (Database, DatabaseRecord).

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8263135: unique_ptr should not be used for types that are not pointers

Reviewers

• Alexey Semenyuk (@alexeysemenyukoracle - Committer)
• Andy Herrick (@andyherrick - Reviewer)

Download

$ git fetch https://git.openjdk.java.net/jdk pull/2858/head:pull/2858
$ git checkout pull/2858

[bridgekeeper]

👋 Welcome back ysuenaga! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@YaSuenag The following label will be automatically applied to this pull request:

• core-libs

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (7035f68)

[openjdk]

@YaSuenag This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8263135: unique_ptr should not be used for types that are not pointers

Reviewed-by: asemenyuk, herrick

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 22 new commits pushed to the master branch:

• 17853ee: 8263200: Add -XX:StressCCP to CTW
• a2b8858: 8263041: Shenandoah: Cleanup C1 keep alive barrier check
• 1f9ed90: 8219555: compiler/jvmci/compilerToVM/IsMatureTest.java fails with Unexpected isMature state for multiple times invoked method: expected false to equal true
• bf9b74d: 8262446: DragAndDrop hangs on Windows
• b1cc864: 8251210: Link JDK api docs to other versions
• 0da889e: 8210100: ParallelGC should use parallel WeakProcessor
• 679faa6: 8263097: Update JMH devkit to 1.28
• ab2cfcc: 8252831: Correct "no comment" warnings in jdk.net module
• 51cca57: 8259577: Dangling reference to temp_path in Java_sun_tools_attach_VirtualMachineImpl_getTempDir
• a0c3f24: 8263091: Remove CharacterData.isOtherUppercase/-Lowercase
• ... and 12 more: https://git.openjdk.java.net/jdk/compare/e1cad97049642ab201d53ff608937f7e7ef3ff3e...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[YaSuenag]

/integrate

[openjdk]

@YaSuenag Since your change was applied there have been 27 commits pushed to the master branch:

• f71b21b: 8263038: Optimize String.format for simple specifiers
• 14cfbda: 8261366: Add discussion of IEEE 754 to BigDecimal
• 414ee95: 8261462: GCM ByteBuffer decryption problems
• eb4a8af: 8260664: Phaser.arrive() memory consistency effects
• 9221540: 8213269: convert test/hotspot/jtreg/runtime/memory/RunUnitTestsConcurrently to gtest
• 17853ee: 8263200: Add -XX:StressCCP to CTW
• a2b8858: 8263041: Shenandoah: Cleanup C1 keep alive barrier check
• 1f9ed90: 8219555: compiler/jvmci/compilerToVM/IsMatureTest.java fails with Unexpected isMature state for multiple times invoked method: expected false to equal true
• bf9b74d: 8262446: DragAndDrop hangs on Windows
• b1cc864: 8251210: Link JDK api docs to other versions
• ... and 17 more: https://git.openjdk.java.net/jdk/compare/e1cad97049642ab201d53ff608937f7e7ef3ff3e...master

Your commit was automatically rebased without conflicts.

Pushed as commit 4e94760.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

[TheShermanTanker]

I'm wondering how MSVC is compiling without the header, when there are several other unique_ptr declarations inside MsiUtils.h. Sorry for bringing up an old commit, but this is now causing issues for JDK-8288293, and I'm a little curious to know why MSVC still strangely accepts this as valid

[alexeysemenyukoracle]

I'm wondering how MSVC is compiling without the header

For older MSVC it picks unique_ptr from
https://github.com/openjdk/jdk/blob/2624324ac216aadc938a870075f6fad287fedf05/src/jdk.jpackage/share/native/common/kludge_c%2B%2B11.h

[TheShermanTanker]

I'm wondering how MSVC is compiling without the header

For older MSVC it picks unique_ptr from https://github.com/openjdk/jdk/blob/2624324ac216aadc938a870075f6fad287fedf05/src/jdk.jpackage/share/native/common/kludge_c%2B%2B11.h

Hmm, I just checked in that file, and it seems like the kludge unique_ptr is only defined for gcc, with no case that enables it for MSVC? (On top of that the gcc conditional is also not spelled correctly)

[alexeysemenyukoracle]

The intention was to make jpackage native code work with compilers without native std::unique_ptr.

[TheShermanTanker]

The intention was to make jpackage native code work with compilers without native std::unique_ptr.

I understand, however it appears MSVC does not use the code inside that file