-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[NFC][SpecialCaseList] Store SectionStr as StringRef #167278
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
[NFC][SpecialCaseList] Store SectionStr as StringRef #167278
Conversation
Created using spr 1.3.7 [skip ci]
Created using spr 1.3.7
|
@llvm/pr-subscribers-llvm-support Author: Vitaly Buka (vitalybuka) ChangesThe SectionStr is already copied in addSection, so Full diff: https://github.com/llvm/llvm-project/pull/167278.diff 2 Files Affected:
<html>
<head>
<meta content="origin" name="referrer">
<title>Rate limit · GitHub</title>
<meta name="viewport" content="width=device-width">
<style type="text/css" media="screen">
body {
background-color: #f6f8fa;
color: rgba(0, 0, 0, 0.5);
font-family: -apple-system,BlinkMacSystemFont,Segoe UI,Helvetica,Arial,sans-serif,Apple Color Emoji,Segoe UI Emoji,Segoe UI Symbol;
font-size: 14px;
line-height: 1.5;
}
.c { margin: 50px auto; max-width: 600px; text-align: center; padding: 0 24px; }
a { text-decoration: none; }
a:hover { text-decoration: underline; }
h1 { color: #24292e; line-height: 60px; font-size: 48px; font-weight: 300; margin: 0px; }
p { margin: 20px 0 40px; }
#s { margin-top: 35px; }
#s a {
color: #666666;
font-weight: 200;
font-size: 14px;
margin: 0 10px;
}
</style>
</head>
<body>
<div class="c">
<h1>Access has been restricted</h1>
<p>You have triggered a rate limit.<br><br>
Please wait a few minutes before you try again;<br>
in some cases this may take up to an hour.
</p>
<div id="s">
<a href="https://support.github.com">Contact Support</a> —
<a href="https://githubstatus.com">GitHub Status</a> —
<a href="https://twitter.com/githubstatus">@githubstatus</a>
</div>
</div>
</body>
</html>
|
The SectionStr is already copied in addSection, so there is no need to copy it again in the Section constructor. Pull Request: llvm#167278
Created using spr 1.3.7 [skip ci]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the SpecialCaseList::Section class to store SectionStr as a StringRef instead of std::string. The change eliminates redundant copying since the string is already copied into the StrAlloc BumpPtrAllocator before the Section is constructed.
- Moved the
SectionStr.copy(StrAlloc)call to execute beforeSections.emplace_back()instead of after - Changed
Section::SectionStrmember type fromstd::stringtoStringRef
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| llvm/lib/Support/SpecialCaseList.cpp | Reordered the copy operation to occur before Section construction |
| llvm/include/llvm/Support/SpecialCaseList.h | Changed Section::SectionStr from std::string to StringRef |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/65/builds/25299 Here is the relevant piece of the build log for the reference |
The SectionStr is already copied in addSection, so
there is no need to copy it again in the Section
constructor.