ilya-biryukov created this revision.
ilya-biryukov added reviewers: rnk, sammccall.
Herald added a subscriber: kadircet.
Herald added a project: clang.
Instead of asserting all typos are corrected in the sema destructor.
The sema destructor is not run in the common case of running the compiler
with the -disable-free cc1 flag (which is the default in the driver).
Having this assertion led to crashes in libclang and clangd, which are not
reproducible when running the compiler.
Asserting at the end of the TU could be an option, but finding all
missing typo correction cases is hard and having worse diagnostics instead
of a failing assertion is a better trade-off.
For more discussion on this, see:
https://lists.llvm.org/pipermail/cfe-dev/2019-July/062872.html
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D64799
Files:
clang/lib/Sema/Sema.cpp
clang/test/SemaObjC/typo-correction-subscript.m
Index: clang/test/SemaObjC/typo-correction-subscript.m
===================================================================
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -9,6 +9,7 @@
- (void)rdar47403222:(Dictionary *)opts {
[self undeclaredMethod:undeclaredArg];
// expected-error@-1{{no visible @interface for 'Test' declares the selector
'undeclaredMethod:'}}
+ // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
opts[(__bridge id)undeclaredKey] = 0;
// expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
}
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -37,6 +37,7 @@
#include "clang/Sema/SemaInternal.h"
#include "clang/Sema/TemplateDeduction.h"
#include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/Support/TimeProfiler.h"
@@ -376,8 +377,6 @@
// Detach from the PP callback handler which outlives Sema since it's owned
// by the preprocessor.
SemaPPCallbackHandler->reset();
-
- assert(DelayedTypos.empty() && "Uncorrected typos!");
}
/// makeUnavailableInSystemHeader - There is an error in the current
@@ -910,6 +909,15 @@
assert(LateParsedInstantiations.empty() &&
"end of TU template instantiation should not create more "
"late-parsed templates");
+
+ // Report diagnostics for uncorrected delayed typos. Ideally all of them
+ // should have been corrected by that point, but it is very hard to cover all
+ // cases in practice.
+ for (const auto &Typo : DelayedTypos) {
+ // We pass an empty TypoCorrection to indicate no correction was performed.
+ Typo.second.DiagHandler(TypoCorrection());
+ }
+ DelayedTypos.clear();
}
/// ActOnEndOfTranslationUnit - This is called at the very end of the
Index: clang/test/SemaObjC/typo-correction-subscript.m
===================================================================
--- clang/test/SemaObjC/typo-correction-subscript.m
+++ clang/test/SemaObjC/typo-correction-subscript.m
@@ -9,6 +9,7 @@
- (void)rdar47403222:(Dictionary *)opts {
[self undeclaredMethod:undeclaredArg];
// expected-error@-1{{no visible @interface for 'Test' declares the selector 'undeclaredMethod:'}}
+ // expected-error@-2{{use of undeclared identifier 'undeclaredArg}}
opts[(__bridge id)undeclaredKey] = 0;
// expected-error@-1{{use of undeclared identifier 'undeclaredKey'}}
}
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -37,6 +37,7 @@
#include "clang/Sema/SemaInternal.h"
#include "clang/Sema/TemplateDeduction.h"
#include "clang/Sema/TemplateInstCallback.h"
+#include "clang/Sema/TypoCorrection.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/Support/TimeProfiler.h"
@@ -376,8 +377,6 @@
// Detach from the PP callback handler which outlives Sema since it's owned
// by the preprocessor.
SemaPPCallbackHandler->reset();
-
- assert(DelayedTypos.empty() && "Uncorrected typos!");
}
/// makeUnavailableInSystemHeader - There is an error in the current
@@ -910,6 +909,15 @@
assert(LateParsedInstantiations.empty() &&
"end of TU template instantiation should not create more "
"late-parsed templates");
+
+ // Report diagnostics for uncorrected delayed typos. Ideally all of them
+ // should have been corrected by that point, but it is very hard to cover all
+ // cases in practice.
+ for (const auto &Typo : DelayedTypos) {
+ // We pass an empty TypoCorrection to indicate no correction was performed.
+ Typo.second.DiagHandler(TypoCorrection());
+ }
+ DelayedTypos.clear();
}
/// ActOnEndOfTranslationUnit - This is called at the very end of the
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
