-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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][Support] Eliminate ',' at end of MemoryEffects print #106545
Conversation
jurahul
commented
Aug 29, 2024
- Eliminate comma at end of a MemoryEffects print.
- Added basic unit test to validate that.
- Eliminate comma at end of a MemoryEffects print. - Added basic unit test to validate that.
ea80950
to
179395d
Compare
@llvm/pr-subscribers-llvm-support Author: Rahul Joshi (jurahul) Changes
Full diff: https://github.com/llvm/llvm-project/pull/106545.diff 3 Files Affected:
diff --git a/llvm/lib/Support/ModRef.cpp b/llvm/lib/Support/ModRef.cpp
index c5978296e97f0c..b57ea30b93832f 100644
--- a/llvm/lib/Support/ModRef.cpp
+++ b/llvm/lib/Support/ModRef.cpp
@@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/Support/ModRef.h"
+#include "llvm/ADT/STLExtras.h"
using namespace llvm;
@@ -33,7 +34,7 @@ raw_ostream &llvm::operator<<(raw_ostream &OS, ModRefInfo MR) {
}
raw_ostream &llvm::operator<<(raw_ostream &OS, MemoryEffects ME) {
- for (IRMemLocation Loc : MemoryEffects::locations()) {
+ interleaveComma(MemoryEffects::locations(), OS, [&](IRMemLocation Loc) {
switch (Loc) {
case IRMemLocation::ArgMem:
OS << "ArgMem: ";
@@ -45,7 +46,7 @@ raw_ostream &llvm::operator<<(raw_ostream &OS, MemoryEffects ME) {
OS << "Other: ";
break;
}
- OS << ME.getModRef(Loc) << ", ";
- }
+ OS << ME.getModRef(Loc);
+ });
return OS;
}
diff --git a/llvm/unittests/Support/CMakeLists.txt b/llvm/unittests/Support/CMakeLists.txt
index db47a170e814a6..d64f89847aa8e7 100644
--- a/llvm/unittests/Support/CMakeLists.txt
+++ b/llvm/unittests/Support/CMakeLists.txt
@@ -61,6 +61,7 @@ add_llvm_unittest(SupportTests
MemoryBufferRefTest.cpp
MemoryBufferTest.cpp
MemoryTest.cpp
+ ModRefTest.cpp
NativeFormatTests.cpp
OptimizedStructLayoutTest.cpp
ParallelTest.cpp
diff --git a/llvm/unittests/Support/ModRefTest.cpp b/llvm/unittests/Support/ModRefTest.cpp
new file mode 100644
index 00000000000000..5ebb5f6a41a586
--- /dev/null
+++ b/llvm/unittests/Support/ModRefTest.cpp
@@ -0,0 +1,28 @@
+//===- llvm/unittest/Support/ModRefTest.cpp - ModRef tests ----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Support/ModRef.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/raw_ostream.h"
+#include "gtest/gtest.h"
+#include <string>
+
+using namespace llvm;
+
+namespace {
+
+// Verify that printing a MemoryEffects does not end with a ,.
+TEST(ModRefTest, PrintMemoryEffects) {
+ std::string S;
+ raw_string_ostream OS(S);
+ OS << MemoryEffects::none();
+ OS.flush();
+ EXPECT_EQ(S, "ArgMem: NoModRef, InaccessibleMem: NoModRef, Other: NoModRef");
+}
+
+} // namespace
|
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.
LGTM
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/146/builds/997 Here is the relevant piece of the build log for the reference
|