Skip to content

Conversation

@kazutakahirata
Copy link
Contributor

With std::make_unique, we don't have to mention the type twice.

Identified with modernize-make-unique.

With std::make_unique, we don't have to mention the type twice.

Identified with modernize-make-unique.
vporpo pushed a commit to vporpo/llvm-project that referenced this pull request Dec 13, 2024
This patch changes the visibility of the constructors of CatchSwitchInst
ResumeInst and SwitchInst to private instead of public.
This is similar to all other Sandbox IR instructions. The constructor
is private to force the user go through the Context create* API.

The issue was exposed by: llvm#119824
@vporpo
Copy link
Contributor

vporpo commented Dec 13, 2024

The reason we were using std::uniuqe_ptr with new is because the CatchSwitchInst is supposed to be private such that creation of object is always done through the API in Context. But I just checked and ithe constructor is public, not private. This PR should fix it: #119901

@kazutakahirata
Copy link
Contributor Author

The reason we were using std::uniuqe_ptr with new is because the CatchSwitchInst is supposed to be private such that creation of object is always done through the API in Context. But I just checked and ithe constructor is public, not private. This PR should fix it: #119901

Thank you for the explanation! I just LGTMed #119901.

@kazutakahirata kazutakahirata deleted the cleanup_001_tidy_readability_smartptr_get branch December 13, 2024 18:18
vporpo pushed a commit that referenced this pull request Dec 13, 2024
This patch changes the visibility of the constructors of CatchSwitchInst
ResumeInst and SwitchInst to private instead of public. This is similar
to all other Sandbox IR instructions. The constructor is private to
force the user go through the Context create* API.

The issue was exposed by:
#119824
@vporpo
Copy link
Contributor

vporpo commented Dec 13, 2024

Thanks! I just pushed the patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants