[llvm] [nfc] Allow forwarding `Error` returns from `Expected` callers (PR #92208)
Mircea Trofin via llvm-commits
llvm-commits at lists.llvm.org
Wed May 15 10:34:47 PDT 2024
https://github.com/mtrofin updated https://github.com/llvm/llvm-project/pull/92208
>From 6cedc34faa8b6a35a8c426ac2b7ba921ef03d58b Mon Sep 17 00:00:00 2001
From: Mircea Trofin <mtrofin at google.com>
Date: Tue, 14 May 2024 19:33:30 -0700
Subject: [PATCH 1/3] [nfc] Allow forwarding `Error` returns from `Expected`
callers
On a few compilers (clang 11/12 for example [1]), the following does not
result in a copy elision, and since `Error`'s copy dtor is elided, results
in a compile error:
```
Expect<Something> foobar() {
...
if (Error E = aCallReturningError())
return E;
...
}
```
Moving `E` would, conversely, result in the pessimizing-move warning on
more recent clangs warning ("moving a local object in a return statement
prevents copy elision")
We just need to make the `Expected` ctor taking an `Error` take it as a
r-value reference.
[1] https://lab.llvm.org/buildbot/#/builders/54/builds/10505
---
llvm/include/llvm/Support/Error.h | 2 +-
llvm/lib/Bitstream/Reader/BitstreamReader.cpp | 12 ++++++------
llvm/lib/Object/COFFObjectFile.cpp | 6 +++---
llvm/lib/Object/WindowsResource.cpp | 4 ++--
4 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/llvm/include/llvm/Support/Error.h b/llvm/include/llvm/Support/Error.h
index 894b6484336ae..217130ce293af 100644
--- a/llvm/include/llvm/Support/Error.h
+++ b/llvm/include/llvm/Support/Error.h
@@ -493,7 +493,7 @@ template <class T> class [[nodiscard]] Expected {
public:
/// Create an Expected<T> error value from the given Error.
- Expected(Error Err)
+ Expected(Error &&Err)
: HasError(true)
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
// Expected is unchecked upon construction in Debug builds.
diff --git a/llvm/lib/Bitstream/Reader/BitstreamReader.cpp b/llvm/lib/Bitstream/Reader/BitstreamReader.cpp
index 3cc9dfdf7b858..5b2c76350029b 100644
--- a/llvm/lib/Bitstream/Reader/BitstreamReader.cpp
+++ b/llvm/lib/Bitstream/Reader/BitstreamReader.cpp
@@ -167,7 +167,7 @@ Expected<unsigned> BitstreamCursor::skipRecord(unsigned AbbrevID) {
if (Error Err =
JumpToBit(GetCurrentBitNo() + static_cast<uint64_t>(NumElts) *
EltEnc.getEncodingData()))
- return std::move(Err);
+ return Err;
break;
case BitCodeAbbrevOp::VBR:
assert((unsigned)EltEnc.getEncodingData() <= MaxChunkSize);
@@ -180,7 +180,7 @@ Expected<unsigned> BitstreamCursor::skipRecord(unsigned AbbrevID) {
break;
case BitCodeAbbrevOp::Char6:
if (Error Err = JumpToBit(GetCurrentBitNo() + NumElts * 6))
- return std::move(Err);
+ return Err;
break;
}
continue;
@@ -206,7 +206,7 @@ Expected<unsigned> BitstreamCursor::skipRecord(unsigned AbbrevID) {
// Skip over the blob.
if (Error Err = JumpToBit(NewEnd))
- return std::move(Err);
+ return Err;
}
return Code;
}
@@ -344,7 +344,7 @@ Expected<unsigned> BitstreamCursor::readRecord(unsigned AbbrevID,
// over tail padding first, in case jumping to NewEnd invalidates the Blob
// pointer.
if (Error Err = JumpToBit(NewEnd))
- return std::move(Err);
+ return Err;
const char *Ptr = (const char *)getPointerToBit(CurBitPos, NumElts);
// If we can return a reference to the data, do so to avoid copying it.
@@ -421,7 +421,7 @@ Error BitstreamCursor::ReadAbbrevRecord() {
Expected<std::optional<BitstreamBlockInfo>>
BitstreamCursor::ReadBlockInfoBlock(bool ReadBlockInfoNames) {
if (llvm::Error Err = EnterSubBlock(bitc::BLOCKINFO_BLOCK_ID))
- return std::move(Err);
+ return Err;
BitstreamBlockInfo NewBlockInfo;
@@ -452,7 +452,7 @@ BitstreamCursor::ReadBlockInfoBlock(bool ReadBlockInfoNames) {
if (!CurBlockInfo)
return std::nullopt;
if (Error Err = ReadAbbrevRecord())
- return std::move(Err);
+ return Err;
// ReadAbbrevRecord installs the abbrev in CurAbbrevs. Move it to the
// appropriate BlockInfo.
diff --git a/llvm/lib/Object/COFFObjectFile.cpp b/llvm/lib/Object/COFFObjectFile.cpp
index 18506f39f6b57..5a85b8e00c633 100644
--- a/llvm/lib/Object/COFFObjectFile.cpp
+++ b/llvm/lib/Object/COFFObjectFile.cpp
@@ -294,7 +294,7 @@ COFFObjectFile::getSectionContents(DataRefImpl Ref) const {
const coff_section *Sec = toSec(Ref);
ArrayRef<uint8_t> Res;
if (Error E = getSectionContents(Sec, Res))
- return std::move(E);
+ return E;
return Res;
}
@@ -807,7 +807,7 @@ Expected<std::unique_ptr<COFFObjectFile>>
COFFObjectFile::create(MemoryBufferRef Object) {
std::unique_ptr<COFFObjectFile> Obj(new COFFObjectFile(std::move(Object)));
if (Error E = Obj->initialize())
- return std::move(E);
+ return E;
return std::move(Obj);
}
@@ -1959,7 +1959,7 @@ ResourceSectionRef::getContents(const coff_resource_data_entry &Entry) {
uint64_t Offset = Entry.DataRVA + Sym->getValue();
ArrayRef<uint8_t> Contents;
if (Error E = Obj->getSectionContents(*Section, Contents))
- return std::move(E);
+ return E;
if (Offset + Entry.DataSize > Contents.size())
return createStringError(object_error::parse_failed,
"data outside of section");
diff --git a/llvm/lib/Object/WindowsResource.cpp b/llvm/lib/Object/WindowsResource.cpp
index 983c8e30a9420..306e8ec542068 100644
--- a/llvm/lib/Object/WindowsResource.cpp
+++ b/llvm/lib/Object/WindowsResource.cpp
@@ -80,7 +80,7 @@ Expected<ResourceEntryRef>
ResourceEntryRef::create(BinaryStreamRef BSR, const WindowsResource *Owner) {
auto Ref = ResourceEntryRef(BSR, Owner);
if (auto E = Ref.loadNext())
- return std::move(E);
+ return E;
return Ref;
}
@@ -1006,7 +1006,7 @@ writeWindowsResourceCOFF(COFF::MachineTypes MachineType,
Error E = Error::success();
WindowsResourceCOFFWriter Writer(MachineType, Parser, E);
if (E)
- return std::move(E);
+ return E;
return Writer.write(TimeDateStamp);
}
>From 7199b7349d5a9a59ca11d9ae93d348250f307019 Mon Sep 17 00:00:00 2001
From: Mircea Trofin <mtrofin at google.com>
Date: Tue, 14 May 2024 21:59:41 -0700
Subject: [PATCH 2/3] clang-format
---
llvm/include/llvm/Support/Error.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/llvm/include/llvm/Support/Error.h b/llvm/include/llvm/Support/Error.h
index 217130ce293af..6fb7ee97ff9c4 100644
--- a/llvm/include/llvm/Support/Error.h
+++ b/llvm/include/llvm/Support/Error.h
@@ -497,7 +497,8 @@ template <class T> class [[nodiscard]] Expected {
: HasError(true)
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
// Expected is unchecked upon construction in Debug builds.
- , Unchecked(true)
+ ,
+ Unchecked(true)
#endif
{
assert(Err && "Cannot create Expected<T> from Error success value.");
>From bb83c04722263625437ea67931d2eeacbb7289eb Mon Sep 17 00:00:00 2001
From: Mircea Trofin <mtrofin at google.com>
Date: Wed, 15 May 2024 10:33:27 -0700
Subject: [PATCH 3/3] undo clang-format
---
llvm/include/llvm/Support/Error.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/llvm/include/llvm/Support/Error.h b/llvm/include/llvm/Support/Error.h
index 6fb7ee97ff9c4..217130ce293af 100644
--- a/llvm/include/llvm/Support/Error.h
+++ b/llvm/include/llvm/Support/Error.h
@@ -497,8 +497,7 @@ template <class T> class [[nodiscard]] Expected {
: HasError(true)
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
// Expected is unchecked upon construction in Debug builds.
- ,
- Unchecked(true)
+ , Unchecked(true)
#endif
{
assert(Err && "Cannot create Expected<T> from Error success value.");
More information about the llvm-commits
mailing list