Skip to content

Commit

Permalink
Address some SString const issues (#92156)
Browse files Browse the repository at this point in the history
* SString const correctness updates

* Remove const from SString::Normalize()

* Remove the SString += operator

* Remove unused operator[]
  • Loading branch information
AaronRobinsonMSFT authored Sep 19, 2023
1 parent 4d948a0 commit 19445f0
Show file tree
Hide file tree
Showing 11 changed files with 48 additions and 63 deletions.
4 changes: 2 additions & 2 deletions src/coreclr/inc/sarray.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ class SArray

COUNT_T GetAllocation() const;

void Preallocate(int count) const;
void Trim() const;
void Preallocate(int count);
void Trim();

void Copy(const Iterator &to, const Iterator &from, COUNT_T size);
void Move(const Iterator &to, const Iterator &from, COUNT_T size);
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/inc/sarray.inl
Original file line number Diff line number Diff line change
Expand Up @@ -165,14 +165,14 @@ inline COUNT_T SArray<ELEMENT, BITWISE_COPY>::GetAllocation() const
}

template <typename ELEMENT, BOOL BITWISE_COPY>
inline void SArray<ELEMENT, BITWISE_COPY>::Preallocate(int count) const
inline void SArray<ELEMENT, BITWISE_COPY>::Preallocate(int count)
{
WRAPPER_NO_CONTRACT;
m_buffer.Preallocate(count * sizeof(ELEMENT));
}

template <typename ELEMENT, BOOL BITWISE_COPY>
inline void SArray<ELEMENT, BITWISE_COPY>::Trim() const
inline void SArray<ELEMENT, BITWISE_COPY>::Trim()
{
WRAPPER_NO_CONTRACT;
m_buffer.Trim();
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/inc/sbuffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,11 @@ class SBuffer
// Preallocate some memory you expect to use. This can prevent
// multiple reallocations. Note this does not change the visible
// size of the buffer.
void Preallocate(COUNT_T allocation) const;
void Preallocate(COUNT_T allocation);

// Shrink memory usage of buffer to minimal amount. Note that
// this does not change the visible size of the buffer.
void Trim() const;
void Trim();

//--------------------------------------------------------------------
// Content manipulation routines
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/inc/sbuffer.inl
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ inline COUNT_T SBuffer::GetAllocation() const
RETURN m_allocation;
}

inline void SBuffer::Preallocate(COUNT_T allocation) const
inline void SBuffer::Preallocate(COUNT_T allocation)
{
CONTRACT_VOID
{
Expand All @@ -396,12 +396,12 @@ inline void SBuffer::Preallocate(COUNT_T allocation) const
CONTRACT_END;

if (allocation > m_allocation)
const_cast<SBuffer *>(this)->ReallocateBuffer(allocation, PRESERVE);
ReallocateBuffer(allocation, PRESERVE);

RETURN;
}

inline void SBuffer::Trim() const
inline void SBuffer::Trim()
{
CONTRACT_VOID
{
Expand All @@ -412,7 +412,7 @@ inline void SBuffer::Trim() const
CONTRACT_END;

if (!IsImmutable())
const_cast<SBuffer *>(this)->ReallocateBuffer(m_size, PRESERVE);
ReallocateBuffer(m_size, PRESERVE);

RETURN;
}
Expand Down
8 changes: 3 additions & 5 deletions src/coreclr/inc/sstring.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ class EMPTY_BASES_DECL SString : private SBuffer

// Normalizes the string representation to unicode. This can be used to
// make basic read-only operations non-failing.
void Normalize() const;
void Normalize();

// Return the number of characters in the string (excluding the terminating NULL).
COUNT_T GetCount() const;
Expand Down Expand Up @@ -302,10 +302,10 @@ class EMPTY_BASES_DECL SString : private SBuffer
void Replace(const Iterator &i, COUNT_T length, const SString &s);

// Make sure that string buffer has room to grow
void Preallocate(COUNT_T characters) const;
void Preallocate(COUNT_T characters);

// Shrink buffer size as much as possible (reallocate if necessary.)
void Trim() const;
void Trim();

// ------------------------------------------------------------------
// Iterators:
Expand Down Expand Up @@ -593,11 +593,9 @@ class EMPTY_BASES_DECL SString : private SBuffer

operator const WCHAR * () const { WRAPPER_NO_CONTRACT; return GetUnicode(); }

WCHAR operator[](int index) { WRAPPER_NO_CONTRACT; return Begin()[index]; }
WCHAR operator[](int index) const { WRAPPER_NO_CONTRACT; return Begin()[index]; }

SString &operator= (const SString &s) { WRAPPER_NO_CONTRACT; Set(s); return *this; }
SString &operator+= (const SString &s) { WRAPPER_NO_CONTRACT; Append(s); return *this; }

// -------------------------------------------------------------------
// Check functions
Expand Down
8 changes: 4 additions & 4 deletions src/coreclr/inc/sstring.inl
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ inline const UTF8 *SString::GetUTF8() const
}

// Normalize the string to unicode. This will make many operations nonfailing.
inline void SString::Normalize() const
inline void SString::Normalize()
{
SS_CONTRACT_VOID
{
Expand Down Expand Up @@ -1097,7 +1097,7 @@ inline void SString::Delete(const Iterator &i, COUNT_T length)
}

// Preallocate some space for the string buffer
inline void SString::Preallocate(COUNT_T characters) const
inline void SString::Preallocate(COUNT_T characters)
{
WRAPPER_NO_CONTRACT;

Expand All @@ -1106,14 +1106,14 @@ inline void SString::Preallocate(COUNT_T characters) const
}

// Trim unused space from the buffer
inline void SString::Trim() const
inline void SString::Trim()
{
WRAPPER_NO_CONTRACT;

if (GetRawCount() == 0)
{
// Share the global empty string buffer.
const_cast<SString *>(this)->SBuffer::SetImmutable(s_EmptyBuffer, sizeof(s_EmptyBuffer));
SBuffer::SetImmutable(s_EmptyBuffer, sizeof(s_EmptyBuffer));
}
else
{
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/vm/excep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11121,7 +11121,7 @@ VOID ThrowBadFormatWorker(UINT resID, LPCWSTR imageName DEBUGARG(_In_z_ const ch
{
resStr.LoadResource(CCompRC::Error, BFA_BAD_IL); // "Bad IL format."
}
msgStr += resStr;
msgStr.Append(resStr);

if ((imageName != NULL) && (imageName[0] != 0))
{
Expand All @@ -11130,8 +11130,8 @@ VOID ThrowBadFormatWorker(UINT resID, LPCWSTR imageName DEBUGARG(_In_z_ const ch
{
SString suffixMsgStr;
suffixMsgStr.FormatMessage(FORMAT_MESSAGE_FROM_STRING, (LPCWSTR)suffixResStr, 0, 0, SString{ SString::Literal, imageName });
msgStr.AppendASCII(" ");
msgStr += suffixMsgStr;
msgStr.Append(W(" "));
msgStr.Append(suffixMsgStr);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/nativeimage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ NativeImage *NativeImage::Open(
SString compositeImageFileName(SString::Utf8, nativeImageFileName);
SString fullPath;
fullPath.Set(path, path.Begin(), (COUNT_T)pathDirLength);
fullPath += compositeImageFileName;
fullPath.Append(compositeImageFileName);
LPWSTR searchPathsConfig;
IfFailThrow(CLRConfig::GetConfigValue(CLRConfig::INTERNAL_NativeImageSearchPaths, &searchPathsConfig));

Expand Down Expand Up @@ -194,7 +194,7 @@ NativeImage *NativeImage::Open(
}

fullPath.Append(DIRECTORY_SEPARATOR_CHAR_W);
fullPath += compositeImageFileName;
fullPath.Append(compositeImageFileName);

EX_TRY
{
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/vm/nativelibrary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ namespace
SString::Iterator i = m_message.Begin();
if (!m_message.Find(i, new_message))
{
m_message += new_message;
m_message += SString(SString::Utf8, "\n");
m_message.Append(new_message);
m_message.AppendUTF8("\n");
}
#else
m_message = SString(SString::Utf8, message);
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/peimage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ CHECK PEImage::CheckCanonicalFullPath(const SString &path)
{
// Drive path
i++;
SString sDrivePath(SString::Literal, ":\\");
SString sDrivePath(SString::Literal, W(":\\"));
CCHECK(path.Skip(i, sDrivePath));
}
else
Expand Down
59 changes: 23 additions & 36 deletions src/coreclr/vm/typestring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -801,36 +801,33 @@ void TypeString::AppendType(TypeNameBuilder& tnb, TypeHandle ty, Instantiation t
StackSString ss;
AppendType(ss, retAndArgTypes[0], format);

SString ssOpening(SString::Literal, "(");
ss += ssOpening;

ss.Append(W("("));

SString ssComma(SString::Literal, ", ");
DWORD cArgs = fnPtr->GetNumArgs();
for (DWORD i = 1; i <= cArgs; i++)
{
if (i != 1)
ss += ssComma;
ss.Append(ssComma);

AppendType(ss, retAndArgTypes[i], format);
}

if ((fnPtr->GetCallConv() & IMAGE_CEE_CS_CALLCONV_MASK) == IMAGE_CEE_CS_CALLCONV_VARARG)
{
if (cArgs)
ss += ssComma;
ss.Append(ssComma);

ss.Append(W("..."));
}

SString ssEllipsis(SString::Literal, "...");
ss += ssEllipsis;
}
ss.Append(W(")"));

SString ssClosing(SString::Literal, ")");
ss += ssClosing;

tnb.AddNameNoEscaping(ss);
}
else
{
tnb.AddNameNoEscaping(W(""));
{
tnb.AddNameNoEscaping(W(""));
}
}

Expand Down Expand Up @@ -941,13 +938,11 @@ void TypeString::AppendMethodImpl(SString& ss, MethodDesc *pMD, Instantiation ty
{
if (pMD->IsLCGMethod())
{
SString sss(SString::Literal, "DynamicClass");
ss += sss;
ss.AppendUTF8("DynamicClass");
}
else if (pMD->IsILStub())
{
SString sss(SString::Literal, ILStubResolver::GetStubClassName(pMD));
ss += sss;
ss.AppendUTF8(ILStubResolver::GetStubClassName(pMD));
}
}
else
Expand All @@ -956,10 +951,8 @@ void TypeString::AppendMethodImpl(SString& ss, MethodDesc *pMD, Instantiation ty
AppendType(ss, th, typeInstantiation, format);
}

SString sss1(SString::Literal, NAMESPACE_SEPARATOR_STR);
ss += sss1;
SString sss2(SString::Utf8, pMD->GetName());
ss += sss2;
ss.AppendUTF8(NAMESPACE_SEPARATOR_STR);
ss.AppendUTF8(pMD->GetName());

if (pMD->HasMethodInstantiation() && !pMD->IsGenericMethodDefinition())
{
Expand All @@ -972,40 +965,34 @@ void TypeString::AppendMethodImpl(SString& ss, MethodDesc *pMD, Instantiation ty

SigFormat sigFormatter(pMD, th);
const char* sigStr = sigFormatter.GetCStringParmsOnly();
SString sss(SString::Utf8, sigStr);
ss += sss;
ss.AppendUTF8(sigStr);
}

if (format & FormatStubInfo) {
if (format & FormatStubInfo)
{
if (pMD->IsInstantiatingStub())
{
SString sss(SString::Literal, "{inst-stub}");
ss += sss;
ss.AppendUTF8("{inst-stub}");
}
if (pMD->IsUnboxingStub())
{
SString sss(SString::Literal, "{unbox-stub}");
ss += sss;
ss.AppendUTF8("{unbox-stub}");
}
if (pMD->IsSharedByGenericMethodInstantiations())
{
SString sss(SString::Literal, "{method-shared}");
ss += sss;
ss.AppendUTF8("{method-shared}");
}
else if (pMD->IsSharedByGenericInstantiations())
{
SString sss(SString::Literal, "{shared}");
ss += sss;
ss.AppendUTF8("{shared}");
}
if (pMD->RequiresInstMethodTableArg())
{
SString sss(SString::Literal, "{requires-mt-arg}");
ss += sss;
ss.AppendUTF8("{requires-mt-arg}");
}
if (pMD->RequiresInstMethodDescArg())
{
SString sss(SString::Literal, "{requires-mdesc-arg}");
ss += sss;
ss.AppendUTF8("{requires-mdesc-arg}");
}
}
}
Expand Down

0 comments on commit 19445f0

Please sign in to comment.