[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS
erikjv closed this revision. erikjv added a comment. Committed as r318142. https://reviews.llvm.org/D37554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG. https://reviews.llvm.org/D37554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS
nik added a comment. I don't get a review and I also don't know who should I add further to this change. What now? https://reviews.llvm.org/D37554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS
nik added a comment. Ping... https://reviews.llvm.org/D37554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS
nik added reviewers: ddunbar, krememek. nik added a comment. ...added some more reviewers that I've found with git blame. Ping to the new ones :) https://reviews.llvm.org/D37554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS
ilya-biryukov added a comment. In https://reviews.llvm.org/D37554#903401, @nik wrote: > Ilya, I hope it's OK if I take your description :) Sure, no problem. https://reviews.llvm.org/D37554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS
nik added a comment. Hmm, apparently "arc diff --update https://reviews.llvm.org/D37554; did not take the new commit message into account. Changed it manually with the web interface. Ilya, I hope it's OK if I take your description :) > That said, I am not familiar with the code you're changing, so can't really > LGTM this. Hopefully, someone else can do that. So who would be the appropriate reviewer here? Manuel, any idea? https://reviews.llvm.org/D37554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS
nik updated this revision to Diff 119823. nik added a comment. Rebased and took over better wording/description from Ilya. https://reviews.llvm.org/D37554 Files: tools/libclang/CIndex.cpp tools/libclang/CIndexCodeCompletion.cpp tools/libclang/Indexing.cpp Index: tools/libclang/Indexing.cpp === --- tools/libclang/Indexing.cpp +++ tools/libclang/Indexing.cpp @@ -880,11 +880,6 @@ TU_options); }; - if (getenv("LIBCLANG_NOTHREADS")) { -IndexSourceFileImpl(); -return result; - } - llvm::CrashRecoveryContext CRC; if (!RunSafely(CRC, IndexSourceFileImpl)) { @@ -934,11 +929,6 @@ index_options, TU); }; - if (getenv("LIBCLANG_NOTHREADS")) { -IndexTranslationUnitImpl(); -return result; - } - llvm::CrashRecoveryContext CRC; if (!RunSafely(CRC, IndexTranslationUnitImpl)) { Index: tools/libclang/CIndexCodeCompletion.cpp === --- tools/libclang/CIndexCodeCompletion.cpp +++ tools/libclang/CIndexCodeCompletion.cpp @@ -806,11 +806,6 @@ llvm::makeArrayRef(unsaved_files, num_unsaved_files), options); }; - if (getenv("LIBCLANG_NOTHREADS")) { -CodeCompleteAtImpl(); -return result; - } - llvm::CrashRecoveryContext CRC; if (!RunSafely(CRC, CodeCompleteAtImpl)) { Index: tools/libclang/CIndex.cpp === --- tools/libclang/CIndex.cpp +++ tools/libclang/CIndex.cpp @@ -3508,11 +3508,6 @@ llvm::makeArrayRef(unsaved_files, num_unsaved_files), options, out_TU); }; - if (getenv("LIBCLANG_NOTHREADS")) { -ParseTranslationUnitImpl(); -return result; - } - llvm::CrashRecoveryContext CRC; if (!RunSafely(CRC, ParseTranslationUnitImpl)) { @@ -3921,8 +3916,7 @@ result = clang_saveTranslationUnit_Impl(TU, FileName, options); }; - if (!CXXUnit->getDiagnostics().hasUnrecoverableErrorOccurred() || - getenv("LIBCLANG_NOTHREADS")) { + if (!CXXUnit->getDiagnostics().hasUnrecoverableErrorOccurred()) { SaveTranslationUnitImpl(); if (getenv("LIBCLANG_RESOURCE_USAGE")) @@ -4045,11 +4039,6 @@ TU, llvm::makeArrayRef(unsaved_files, num_unsaved_files), options); }; - if (getenv("LIBCLANG_NOTHREADS")) { -ReparseTranslationUnitImpl(); -return result; - } - llvm::CrashRecoveryContext CRC; if (!RunSafely(CRC, ReparseTranslationUnitImpl)) { @@ -8164,7 +8153,7 @@ unsigned Size) { if (!Size) Size = GetSafetyThreadStackSize(); - if (Size) + if (Size && !getenv("LIBCLANG_NOTHREADS")) return CRC.RunSafelyOnThread(Fn, Size); return CRC.RunSafely(Fn); } Index: tools/libclang/Indexing.cpp === --- tools/libclang/Indexing.cpp +++ tools/libclang/Indexing.cpp @@ -880,11 +880,6 @@ TU_options); }; - if (getenv("LIBCLANG_NOTHREADS")) { -IndexSourceFileImpl(); -return result; - } - llvm::CrashRecoveryContext CRC; if (!RunSafely(CRC, IndexSourceFileImpl)) { @@ -934,11 +929,6 @@ index_options, TU); }; - if (getenv("LIBCLANG_NOTHREADS")) { -IndexTranslationUnitImpl(); -return result; - } - llvm::CrashRecoveryContext CRC; if (!RunSafely(CRC, IndexTranslationUnitImpl)) { Index: tools/libclang/CIndexCodeCompletion.cpp === --- tools/libclang/CIndexCodeCompletion.cpp +++ tools/libclang/CIndexCodeCompletion.cpp @@ -806,11 +806,6 @@ llvm::makeArrayRef(unsaved_files, num_unsaved_files), options); }; - if (getenv("LIBCLANG_NOTHREADS")) { -CodeCompleteAtImpl(); -return result; - } - llvm::CrashRecoveryContext CRC; if (!RunSafely(CRC, CodeCompleteAtImpl)) { Index: tools/libclang/CIndex.cpp === --- tools/libclang/CIndex.cpp +++ tools/libclang/CIndex.cpp @@ -3508,11 +3508,6 @@ llvm::makeArrayRef(unsaved_files, num_unsaved_files), options, out_TU); }; - if (getenv("LIBCLANG_NOTHREADS")) { -ParseTranslationUnitImpl(); -return result; - } - llvm::CrashRecoveryContext CRC; if (!RunSafely(CRC, ParseTranslationUnitImpl)) { @@ -3921,8 +3916,7 @@ result = clang_saveTranslationUnit_Impl(TU, FileName, options); }; - if (!CXXUnit->getDiagnostics().hasUnrecoverableErrorOccurred() || - getenv("LIBCLANG_NOTHREADS")) { + if (!CXXUnit->getDiagnostics().hasUnrecoverableErrorOccurred()) { SaveTranslationUnitImpl(); if (getenv("LIBCLANG_RESOURCE_USAGE")) @@ -4045,11 +4039,6 @@ TU, llvm::makeArrayRef(unsaved_files, num_unsaved_files), options); }; - if (getenv("LIBCLANG_NOTHREADS")) { -ReparseTranslationUnitImpl(); -return result; - } - llvm::CrashRecoveryContext CRC; if (!RunSafely(CRC, ReparseTranslationUnitImpl)) { @@
[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS
ilya-biryukov added a comment. I think your change makes sense, but maybe asking for a better description. It appears you: - Enabled crash recovery for some libclang operations on a calling thread even when `LIBCLANG_NOTHREAD` is specified. Previously it would only run under crash recovery if `LIBCLANG_NOTHREAD` is not set. - Moved handling of `LIBCLANG_NOTHREAD` env variable into `RunSafely` from its call sites. That said, I am not familiar with the code you're changing, so can't really LGTM this. Hopefully, someone else can do that. https://reviews.llvm.org/D37554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS
nik added a comment. Ping 4 https://reviews.llvm.org/D37554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS
klimek added a reviewer: akyrtzi. klimek added a comment. Adding Argyrios, who might have insight on how this is used. I think this had the wrong list of reviewers so far :( https://reviews.llvm.org/D37554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS
yvvan added a comment. ping 3 https://reviews.llvm.org/D37554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS
nik added a comment. Ping 2 https://reviews.llvm.org/D37554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS
nik added a comment. Ping https://reviews.llvm.org/D37554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37554: [libclang] Allow crash recovery with LIBCLANG_NOTHREADS
yvvan added a comment. This actually fixes the ability to run safely without threads. This happens because by default this solution leads to try/catch block instead of the direct function call which is implemented in deleted if blocks. https://reviews.llvm.org/D37554 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits