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

C-style cast cleanup VI #7061

Merged
merged 1 commit into from Dec 4, 2019
Merged

C-style cast cleanup VI #7061

merged 1 commit into from Dec 4, 2019

Conversation

Copy link
Member

@Nekotekina Nekotekina commented Dec 3, 2019

No description provided.

}
else if (sct->isDoubleTy())
{
return llvm::ConstantDataVector::get(m_context, llvm::makeArrayRef((const f64*)v._bytes, 2));
return llvm::ConstantDataVector::get(m_context, llvm::makeArrayRef(reinterpret_cast<const f64*>(v._bytes), 2));
Copy link
Contributor

@elad335 elad335 Dec 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return llvm::ConstantDataVector::get(m_context, llvm::makeArrayRef(reinterpret_cast<const f64*>(v._bytes), 2));
return llvm::ConstantDataVector::get(m_context, llvm::makeArrayRef(&std::as_const(v._f64[0]), 2));

Copy link
Member Author

@Nekotekina Nekotekina Dec 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave it as is because v128 normal_array / reversed_array aren't really supposed to be used like plain C arrays

min_index = std::min<u16>(index, static_cast<u16>(min_index));
max_index = std::max<u16>(index, static_cast<u16>(max_index));
Copy link
Contributor

@CookiePLMonster CookiePLMonster Dec 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since index is u16 and you cast min_index and max_index, you shouldn't need to specialize functions with <u16>

Copy link
Member Author

@Nekotekina Nekotekina Dec 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specializing it is preferred way to use min/max though

Copy link
Contributor

@CookiePLMonster CookiePLMonster Dec 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing - but do you then need to cast min_index and max_index? I might be wrong, but I thought it's one or the other.

Copy link
Member Author

@Nekotekina Nekotekina Dec 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need static_cast there because of implicit convertion, but it's still here to silence MSVC warning about truncation

@@ -493,7 +493,7 @@ namespace gl
}
else
{
u64 key = (u64)desc;
u64 key = reinterpret_cast<u64>(desc);
temp_image_cache[key] = std::move(std::make_pair(owner_uid, std::move(tex)));
Copy link
Contributor

@CookiePLMonster CookiePLMonster Dec 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to this PR, but std::move on std::make_pair is redundant AFAIK.

@@ -143,7 +143,7 @@ std::u16string ascii8_to_utf16(const std::string& ascii_string)

for (const auto& code : ascii_string)
{
out.push_back((char16_t)code);
out.push_back(static_cast<char16_t>(code));
Copy link
Contributor

@CookiePLMonster CookiePLMonster Dec 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again unrelated to PR, but you should be able to do exact same pushing directly to std::u16string without an intermediate vector?

@Nekotekina Nekotekina force-pushed the master branch 2 times, most recently from 2b78a2a to bfa8e5a Compare Dec 4, 2019
@@ -138,16 +138,18 @@ std::string utf16_to_ascii8(const std::u16string& utf16_string)

std::u16string ascii8_to_utf16(const std::string& ascii_string)
{
std::vector<char16_t> out;
std::u16string out;
out.reserve(ascii_string.length() + 1);
Copy link
Contributor

@CookiePLMonster CookiePLMonster Dec 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With out changed to string, you don't need to reserve + 1 because you're not appending the null terminator no more.

@Nekotekina Nekotekina merged commit d2fd3c6 into RPCS3:master Dec 4, 2019
0 of 2 checks passed
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.

None yet

3 participants