-
Notifications
You must be signed in to change notification settings - Fork 11.4k
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
[clang-repl] Fix the process return code if diagnostics occurred. #89879
Conversation
@llvm/pr-subscribers-clang Author: Vassil Vassilev (vgvassilev) ChangesShould fix the failure seen in the pre-merge infrastructure of #89804. Full diff: https://github.com/llvm/llvm-project/pull/89879.diff 2 Files Affected:
diff --git a/clang/test/Interpreter/fail.cpp b/clang/test/Interpreter/fail.cpp
index 4e301f37548f1f..b9035aca3cb7b1 100644
--- a/clang/test/Interpreter/fail.cpp
+++ b/clang/test/Interpreter/fail.cpp
@@ -1,12 +1,9 @@
-// FIXME: There're some inconsistencies between interactive and non-interactive
-// modes. For example, when clang-repl runs in the interactive mode, issues an
-// error, and then successfully recovers if we decide it's a success then for
-// the non-interactive mode the exit code should be a failure.
-// RUN: clang-repl "int x = 10;" "int y=7; err;" "int y = 10;"
// REQUIRES: host-supports-jit
// UNSUPPORTED: system-aix
-// RUN: cat %s | not clang-repl | FileCheck %s
-BOOM!
+// RUN: not clang-repl "int x = 10;" "int y=7; err;" "int y = 10;"
+// RUN: cat %s | clang-repl | FileCheck %s
+// RUN: cat %s | not clang-repl -Xcc -Xclang -Xcc -verify | FileCheck %s
+BOOM! // expected-error {{intended to fail the -verify test}}
extern "C" int printf(const char *, ...);
int i = 42;
auto r1 = printf("i = %d\n", i);
diff --git a/clang/tools/clang-repl/ClangRepl.cpp b/clang/tools/clang-repl/ClangRepl.cpp
index aecf61b97fc719..bdc740c33a8f72 100644
--- a/clang/tools/clang-repl/ClangRepl.cpp
+++ b/clang/tools/clang-repl/ClangRepl.cpp
@@ -215,12 +215,15 @@ int main(int argc, const char **argv) {
} else
Interp = ExitOnErr(clang::Interpreter::create(std::move(CI)));
+ bool HasError = false;
+
for (const std::string &input : OptInputs) {
- if (auto Err = Interp->ParseAndExecute(input))
+ if (auto Err = Interp->ParseAndExecute(input)) {
llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
+ HasError = true;
+ }
}
- bool HasError = false;
if (OptInputs.empty()) {
llvm::LineEditor LE("clang-repl");
@@ -241,18 +244,13 @@ int main(int argc, const char **argv) {
break;
}
if (Input == R"(%undo)") {
- if (auto Err = Interp->Undo()) {
+ if (auto Err = Interp->Undo())
llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
- HasError = true;
- }
} else if (Input.rfind("%lib ", 0) == 0) {
- if (auto Err = Interp->LoadDynamicLibrary(Input.data() + 5)) {
+ if (auto Err = Interp->LoadDynamicLibrary(Input.data() + 5))
llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
- HasError = true;
- }
} else if (auto Err = Interp->ParseAndExecute(Input)) {
llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
- HasError = true;
}
Input = "";
|
0660299
to
d6b3d2a
Compare
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.
I assume the return code is supposed to reflect only frontend errors? (See my note inline.) From reading the test it's not obvious what is interactive mode and what is non-interactive/batch mode. Might be worth a note. Otherwise, this LGTM.
llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: "); | ||
HasError = true; |
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.
This did include errors like "file not found" in interactive mode
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.
The idea is that when you use interactive mode and it recovers from errors properly to return success as exit code. However, for the when run it in non interactive mode people might use clang-repl as a calculator eg clang-repl "printf(...)"
and can be put in scripts. There we want to get exit code 1 if it miscompiled something.
d6b3d2a
to
49b4798
Compare
The Unix pre-merge seems okay, however the windows pre-merge check is doing nothing for more than 12h. I will move forward. |
Yeah same here. I recognized that Windows PR checks are running Flang regression tests now. I guess that adds a huge load on the builders and causes the delays. Mine was cancelled after 15h.. #89734 @philnik777 Brought it up in Discord on Wednesday https://discord.com/channels/636084430946959380/645949235295944724/1232232568196173846 |
Should fix the failure seen in the pre-merge infrastructure of #89804.