This is an archive of the discontinued LLVM Phabricator instance.

Differential D120673

[demangler] Simplify SwapAndRestore
ClosedPublic

Authored by urnathan on Feb 28 2022, 12:18 PM.

Details

Reviewers
iains
ChuanqiXu
Group Reviewers
Restricted Project
Commits
rGe5c98e22fbbe: [demangler] Simplify SwapAndRestore
Summary

The SwapAndRestore class is over engineered. Nothing makes use of the early restoration machinery. Let's just remove that cognitive burden.

Diff Detail

Unit TestsFailed

TimeTest
60,130 msx64 debian > AddressSanitizer-x86_64-linux-dynamic.TestCases::scariness_score_test.cpp
60,090 msx64 debian > AddressSanitizer-x86_64-linux.TestCases::scariness_score_test.cpp
770 msx64 debian > SanitizerCommon-tsan-x86_64-Linux.Linux::decorate_proc_maps.cpp

Event Timeline

urnathan requested review of this revision.Feb 28 2022, 12:18 PM
urnathan created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 28 2022, 12:18 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
Harbormaster completed remote builds in B151794: Diff 411863.Feb 28 2022, 2:40 PM
ChuanqiXu accepted this revision.Feb 28 2022, 5:36 PM

If restoreNow never gets called, the revision should be good.

This revision is now accepted and ready to land.Feb 28 2022, 5:36 PM
urnathan updated this revision to Diff 412036.Mar 1 2022, 3:46 AM

Fix formating

This revision was landed with ongoing or failed builds.Mar 1 2022, 4:37 AM
Closed by commit rGe5c98e22fbbe: [demangler] Simplify SwapAndRestore (authored by urnathan). · Explain Why
This revision was automatically updated to reflect the committed changes.
urnathan added a commit: rGe5c98e22fbbe: [demangler] Simplify SwapAndRestore.
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2022, 4:37 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
Harbormaster completed remote builds in B151915: Diff 412036.Mar 1 2022, 5:15 AM

Revision Contents

PathSize
libcxxabi/
src/
demangle/
12 lines
llvm/
include/
llvm/
Demangle/
12 lines

Diff 411863

libcxxabi/src/demangle/Utility.h

Show First 20 Lines • Show All 157 Lines • ▼ Show 20 Linespublic:
char *getBuffer() { return Buffer; } char *getBuffer() { return Buffer; }
char *getBufferEnd() { return Buffer + CurrentPosition - 1; } char *getBufferEnd() { return Buffer + CurrentPosition - 1; }
size_t getBufferCapacity() const { return BufferCapacity; } size_t getBufferCapacity() const { return BufferCapacity; }
}; };
template <class T> class SwapAndRestore { template <class T> class SwapAndRestore {
T &Restore; T &Restore;
T OriginalValue; T OriginalValue;
bool ShouldRestore = true;
public: public:
SwapAndRestore(T &Restore_) : SwapAndRestore(Restore_, Restore_) {} SwapAndRestore(T &Restore_) : SwapAndRestore(Restore_, Restore_) {}
SwapAndRestore(T &Restore_, T NewVal) SwapAndRestore(T &Restore_, T NewVal)
: Restore(Restore_), OriginalValue(Restore) { : Restore(Restore_), OriginalValue(Restore) {
Restore = std::move(NewVal); Restore = std::move(NewVal);
} }
~SwapAndRestore() { ~SwapAndRestore() {
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-  ~SwapAndRestore() {
-    Restore = std::move(OriginalValue);
-  }
+  ~SwapAndRestore() { Restore = std::move(OriginalValue); }
Lint: Pre-merge checks: clang-format: please reformat the code ``` - ~SwapAndRestore() { - Restore = std::move…
if (ShouldRestore)
Restore = std::move(OriginalValue); Restore = std::move(OriginalValue);
} }
void shouldRestore(bool ShouldRestore_) { ShouldRestore = ShouldRestore_; }
void restoreNow(bool Force) {
if (!Force && !ShouldRestore)
return;
Restore = std::move(OriginalValue);
ShouldRestore = false;
}
SwapAndRestore(const SwapAndRestore &) = delete; SwapAndRestore(const SwapAndRestore &) = delete;
SwapAndRestore &operator=(const SwapAndRestore &) = delete; SwapAndRestore &operator=(const SwapAndRestore &) = delete;
}; };
inline bool initializeOutputBuffer(char *Buf, size_t *N, OutputBuffer &OB, inline bool initializeOutputBuffer(char *Buf, size_t *N, OutputBuffer &OB,
size_t InitSize) { size_t InitSize) {
size_t BufferSize; size_t BufferSize;
if (Buf == nullptr) { if (Buf == nullptr) {
Show All 14 Lines

llvm/include/llvm/Demangle/Utility.h

Show First 20 Lines • Show All 157 Lines • ▼ Show 20 Linespublic:
char *getBuffer() { return Buffer; } char *getBuffer() { return Buffer; }
char *getBufferEnd() { return Buffer + CurrentPosition - 1; } char *getBufferEnd() { return Buffer + CurrentPosition - 1; }
size_t getBufferCapacity() const { return BufferCapacity; } size_t getBufferCapacity() const { return BufferCapacity; }
}; };
template <class T> class SwapAndRestore { template <class T> class SwapAndRestore {
T &Restore; T &Restore;
T OriginalValue; T OriginalValue;
bool ShouldRestore = true;
public: public:
SwapAndRestore(T &Restore_) : SwapAndRestore(Restore_, Restore_) {} SwapAndRestore(T &Restore_) : SwapAndRestore(Restore_, Restore_) {}
SwapAndRestore(T &Restore_, T NewVal) SwapAndRestore(T &Restore_, T NewVal)
: Restore(Restore_), OriginalValue(Restore) { : Restore(Restore_), OriginalValue(Restore) {
Restore = std::move(NewVal); Restore = std::move(NewVal);
} }
~SwapAndRestore() { ~SwapAndRestore() {
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-  ~SwapAndRestore() {
-    Restore = std::move(OriginalValue);
-  }
+  ~SwapAndRestore() { Restore = std::move(OriginalValue); }
Lint: Pre-merge checks: clang-format: please reformat the code ``` - ~SwapAndRestore() { - Restore = std::move…
if (ShouldRestore)
Restore = std::move(OriginalValue); Restore = std::move(OriginalValue);
} }
void shouldRestore(bool ShouldRestore_) { ShouldRestore = ShouldRestore_; }
void restoreNow(bool Force) {
if (!Force && !ShouldRestore)
return;
Restore = std::move(OriginalValue);
ShouldRestore = false;
}
SwapAndRestore(const SwapAndRestore &) = delete; SwapAndRestore(const SwapAndRestore &) = delete;
SwapAndRestore &operator=(const SwapAndRestore &) = delete; SwapAndRestore &operator=(const SwapAndRestore &) = delete;
}; };
inline bool initializeOutputBuffer(char *Buf, size_t *N, OutputBuffer &OB, inline bool initializeOutputBuffer(char *Buf, size_t *N, OutputBuffer &OB,
size_t InitSize) { size_t InitSize) {
size_t BufferSize; size_t BufferSize;
if (Buf == nullptr) { if (Buf == nullptr) {
Show All 14 Lines