-
Couldn't load subscription status.
- Fork 15k
[clang] Don't fail ExecuteCompilerInvocation() due to caller errors
#158695
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
Conversation
|
@llvm/pr-subscribers-clang Author: Jan Svoboda (jansvoboda11) ChangesThis PR changes the behavior of Full diff: https://github.com/llvm/llvm-project/pull/158695.diff 1 Files Affected:
diff --git a/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp b/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
index 9a6844d5f7d40..f1965a4e27e07 100644
--- a/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ b/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -210,6 +210,8 @@ CreateFrontendAction(CompilerInstance &CI) {
}
bool ExecuteCompilerInvocation(CompilerInstance *Clang) {
+ unsigned NumErrorsBefore = Clang->getDiagnostics().getNumErrors();
+
// Honor -help.
if (Clang->getFrontendOpts().ShowHelp) {
driver::getDriverOptTable().printHelp(
@@ -293,7 +295,7 @@ bool ExecuteCompilerInvocation(CompilerInstance *Clang) {
#endif
// If there were errors in processing arguments, don't do anything else.
- if (Clang->getDiagnostics().hasErrorOccurred())
+ if (Clang->getDiagnostics().getNumErrors() != NumErrorsBefore)
return false;
// Create and execute the frontend action.
std::unique_ptr<FrontendAction> Act(CreateFrontendAction(*Clang));
|
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.
Can you add more comment to explain when do we actually call this function when Diagnostics Engine already has error? This will help to evaluate what is a right thing to do.
|
Currently we don't, but with #158381 we'll try to initialize the VFS overlays before calling |
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.
Can you summarize the reason and leave a comment in the code?
clang::ExecuteCompilerInvocation() for unrelated errorsclang::ExecuteCompilerInvocation() for caller errors
clang::ExecuteCompilerInvocation() for caller errorsExecuteCompilerInvocation() due to caller errors
…llvm#158695) This PR changes the behavior of `clang::ExecuteCompilerInvocation()` so that it only returns early when the `DiagnosticsEngine` emitted errors **within** the function. Handling errors emitted before the function got called is a responsibility of the caller. Necessary for llvm#158381. (cherry picked from commit f33fb0d)
…llvm#158695) This PR changes the behavior of `clang::ExecuteCompilerInvocation()` so that it only returns early when the `DiagnosticsEngine` emitted errors **within** the function. Handling errors emitted before the function got called is a responsibility of the caller. Necessary for llvm#158381. (cherry picked from commit f33fb0d)
…llvm#158695) This PR changes the behavior of `clang::ExecuteCompilerInvocation()` so that it only returns early when the `DiagnosticsEngine` emitted errors **within** the function. Handling errors emitted before the function got called is a responsibility of the caller. Necessary for llvm#158381. (cherry picked from commit f33fb0d)
…llvm#158695) This PR changes the behavior of `clang::ExecuteCompilerInvocation()` so that it only returns early when the `DiagnosticsEngine` emitted errors **within** the function. Handling errors emitted before the function got called is a responsibility of the caller. Necessary for llvm#158381. (cherry picked from commit f33fb0d)
…Instance Sharing (#11631) * [clang][deps] Remove dependency on `tooling::ToolAction` (llvm#149904) The dependency scanner was initially using a fair amount of infrastructure provided by the `clangTooling` library. Over time, the needs for bespoke handling of command lines grew and the overlap with the tooling library kept shrinking. I don't think the library provides any value anymore. I decided to remove the dependency and only reimplement the small bits required by the scanner. This allowed for a nice simplification, where we no longer need to create temporary dummy `FileManager` instances (mis-named as `DriverFileMgr` in some parts) and `SourceManager` instances to attach to the `DiagnosticsEngine`. That code was copied from the tooling library to support `DiagnosticConsumers` that expect these to exist. The scanner uses a closed set of consumers and none need these objects to exist. The motivation for this (hopefully NFC) patch are some new restrictions to how VFS's can be propagated in Clang that I'm working on. (cherry picked from commit aa1b416) * Reland "[clang] Delay normalization of `-fmodules-cache-path` (llvm#150123)" This reverts commit 613caa9, essentially reapplying 4a4bdde after moving `normalizeModuleCachePath` from clangFrontend to clangLex. This PR is part of an effort to remove file system usage from the command line parsing code. The reason for that is that it's impossible to do file system access correctly without a configured VFS, and the VFS can only be configured after the command line is parsed. I don't want to intertwine command line parsing and VFS configuration, so I decided to perform the file system access after the command line is parsed and the VFS is configured - ideally right before the file system entity is used for the first time. This patch delays normalization of the module cache path until `CompilerInstance` is asked for the cache path in the current compilation context. (cherry picked from commit 55bef46) * NFC: Clean up of IntrusiveRefCntPtr construction from raw pointers. (llvm#151545) Handles clang::DiagnosticsEngine and clang::DiagnosticIDs. For DiagnosticIDs, this mostly migrates from `new DiagnosticIDs` to convenience method `DiagnosticIDs::create()`. Part of cleanup llvm#151026 (cherry picked from commit c7f3437) Conflicts: clang/tools/driver/cc1_main.cpp clang/unittests/Driver/DXCModeTest.cpp clang/unittests/Driver/SimpleDiagnosticConsumer.h clang/unittests/Frontend/SearchPathTest.cpp clang/unittests/Lex/HeaderSearchTest.cpp clang/unittests/Tooling/RewriterTestContext.h * NFC: Clean up of IntrusiveRefCntPtr construction from raw pointers. (llvm#151782) This commit handles the following types: - clang::ExternalASTSource - clang::TargetInfo - clang::ASTContext - clang::SourceManager - clang::FileManager Part of cleanup llvm#151026 (cherry picked from commit 4205da0) Conflicts: clang/lib/Frontend/ASTUnit.cpp clang/lib/Frontend/ChainedIncludesSource.cpp clang/lib/Frontend/CompilerInstance.cpp * Merge commit '30633f308941' from llvm.org/main into next (cherry picked from commit 95ea104) Conflicts: clang/include/clang/Frontend/CompilerInstance.h clang/lib/Frontend/CompilerInstance.cpp * Merge pull request #11450 from swiftlang/jan_svoboda/cas-fix-early-vfs [clang] Fix CAS initialization after upstream llvm#158381 (cherry picked from commit 6d73002) * [clang] Avoid reparsing VFS overlay files for module dep collector (llvm#158372) This PR uses the new-ish `llvm::vfs::FileSystem::visit()` interface to collect VFS overlay entries from an existing `FileSystem` instance rather than parsing the VFS YAML file anew. This prevents duplicate diagnostics as observed by `clang/test/VFS/broken-vfs-module-dep.c`. (cherry picked from commit 4957c47) * [clang] Don't fail `ExecuteCompilerInvocation()` due to caller errors (llvm#158695) This PR changes the behavior of `clang::ExecuteCompilerInvocation()` so that it only returns early when the `DiagnosticsEngine` emitted errors **within** the function. Handling errors emitted before the function got called is a responsibility of the caller. Necessary for llvm#158381. (cherry picked from commit f33fb0d) * [clang] Only set non-empty bypass to scan VFS (llvm#159605) Normalizing an empty modules cache path results in an incorrect non-empty path (the working directory). This PR conditionalizes more code to avoid this. Tested downstream by swift/llvm-project and the `DependencyScanningCAPITests.DependencyScanningFSCacheOutOfDate` unit test. (cherry picked from commit 5a339b0) * Merge commit '0e35f56d40d3' from llvm.org/main into next (cherry picked from commit 3efcc0f) Conflicts: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp * [clang] NFCI: Clean up `CompilerInstance::create{File,Source}Manager()` (llvm#160748) The `CompilerInstance::createSourceManager()` function currently accepts the `FileManager` to be used. However, all clients call `CompilerInstance::createFileManager()` prior to creating the `SourceManager`, and it never makes sense to use a `FileManager` in the `SourceManager` that's different from the rest of the compiler. Passing the `FileManager` explicitly is redundant, error-prone, and deviates from the style of other `CompilerInstance` initialization APIs. This PR therefore removes the `FileManager` parameter from `createSourceManager()` and also stops returning the `FileManager` pointer from `createFileManager()`, since that was its primary use. Now, `createSourceManager()` internally calls `getFileManager()` instead. (cherry picked from commit b86ddae) Conflicts: clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp * Merge commit '436861645247' from llvm.org/main into next (cherry picked from commit 286ea7d) Conflicts: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp * [clang] Pass VFS into `ASTUnit::LoadFromASTFile()` (llvm#159166) This PR makes the `VFS` parameter to `ASTUnit::LoadFromASTFile()` required and explicit, rather than silently defaulting to the real file system. This makes it easy to correctly propagate the fully-configured VFS and load any input files like the rest of the compiler does. (cherry picked from commit cda542d) * Fix a line missing when merging 30633f3 * [clang][deps] Fix a use-after-free from expanding response files (llvm#164676) In 4368616 we accidentally moved uses of command-line args saved into a bump pointer allocator during response file expansion out of scope of the allocator. Also, the test that should have caught this (at least with asan) was not working correctly because clang-scan-deps was expanding response files itself during argument adjustment rather than the underlying scanner library. rdar://162720059 (cherry picked from commit 3e6f696) --------- Co-authored-by: Jan Svoboda <jan_svoboda@apple.com> Co-authored-by: James Y Knight <jyknight@google.com> Co-authored-by: git apple-llvm automerger <am@git-apple-llvm> Co-authored-by: Ben Langmuir <blangmuir@apple.com>
This PR changes the behavior of
clang::ExecuteCompilerInvocation()so that it only returns early when theDiagnosticsEngineemitted errors within the function. Handling errors emitted before the function got called is a responsibility of the caller. Necessary for #158381.