[Sema] Don't issue analysis-based warnings when a fatal error has occurred

[ahatanak]

Summary

This patch makes AnalysisBasedWarnings::IssueWarnings return without issuing warnings based on CFG analysis when a fatal error has occurred. This is needed to avoid a crash that occurs when compiling the test case (instantiate-template-fatal-error.cpp).

My first patch made changes in InstantiatingTemplate::InstantiatingTemplate to call hasUncompilableErrorOccurred instead of hasFatalErrorOccurred at SemaTemplateInstantiate.cpp:214. That fixed the crash for the test case but introduced a large number of regression test failures, so I fixed AnalysisBasedWarnings::IssueWarnings instead.

[rsmith]

It looks like the bug is that if we hit an uncompilable error after hitting a non-uncompilable fatal error (promoted from a warning), we fail to notice that we've hit an uncompileable error. Our AST should be complete if hasUncompilableErrorOccurred() is not set, and it looks like we're violating that invariant here.

[ahatanak]

I think the non-uncompilable error you mentioned is the one on line 7 of the test case and the uncompilable error is the one on line 15. Is that correct? The modified test case has only one error (on line 7, which is promoted from a warning) and it compiles without any errors if the #pragmas are removed.

It seems to me that the root of the problem is that the template instantiation S2<int> is left in an incomplete state and VarDecl s2 in the AST, which is of type S2<int>, is marked invalid. As I mentioned in my previous comment, I tried to fix this by calling hasUncompilableErrorOccurred instead of hasFatalErrorOccurred in InstantiatingTemplate::InstantiatingTemplate, but that didn't seem correct as a large number of regression tests failed. What is the right way to fix this?

`-FunctionDecl 0x7f889602fa90 <line:19:1, line:22:1> line:19:6 foo1 '_Bool (const long long *, int *)'

|-ParmVarDecl 0x7f889602f918 <col:11, col:28> col:28 used a 'const long long *'
|-ParmVarDecl 0x7f889602f9c0 <col:31, col:36> col:36 used b 'int *'
`-CompoundStmt 0x7f889602fde0 <col:39, line:22:1>
  `-DeclStmt 0x7f889602fd78 <line:20:3, col:13>
    `-VarDecl 0x7f889602fd18 <col:3, col:11> col:11 invalid s2 'S2<int>':'struct S2<int>'

[rsmith]

In D26166#584285, @ahatanak wrote:

It seems to me that the root of the problem is that the template instantiation S2<int> is left in an incomplete state and VarDecl s2 in the AST, which is of type S2<int>, is marked invalid. As I mentioned in my previous comment, I tried to fix this by calling hasUncompilableErrorOccurred instead of hasFatalErrorOccurred in InstantiatingTemplate::InstantiatingTemplate, but that didn't seem correct as a large number of regression tests failed. What is the right way to fix this?

Ah, I see. Try changing that to hasFatalErrorOccurred() && hasUncompilableErrorOccurred(). (The former indicates we don't need more diagnostics, and the latter indicates we don't need a complete AST; if both flags are set, then we don't need to instantiate any more templates.)

[ahatanak]

Thanks, that fixed the failing regression tests.

[rsmith]

lib/Sema/SemaTemplateInstantiate.cpp

213–214 ↗

(On Diff #76521)

Maybe "Any diagnostics we might have raised will not be visible, and we do not need to construct a correct AST." to justify the two checks?

[malcolm.parsons]

cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp

213

s/occcured/occurred/

cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp

 
 Sema::InstantiatingTemplate::InstantiatingTemplate(
     Sema &SemaRef, ActiveTemplateInstantiation::InstantiationKind Kind,
     SourceLocation PointOfInstantiation, SourceRange InstantiationRange,
     Decl *Entity, NamedDecl *Template, ArrayRef<TemplateArgument> TemplateArgs,
     sema::TemplateDeductionInfo *DeductionInfo)
     : SemaRef(SemaRef), SavedInNonInstantiationSFINAEContext(
                             SemaRef.InNonInstantiationSFINAEContext) {
-  // Don't allow further instantiation if a fatal error has occcured.  Any
-  // diagnostics we might have raised will not be visible.
-  if (SemaRef.Diags.hasFatalErrorOccurred()) {
+  // Don't allow further instantiation if a fatal error and an uncompilable
+  // error have occcured. Any diagnostics we might have raised will not be
+  // visible, and we do not need to construct a correct AST.
+  if (SemaRef.Diags.hasFatalErrorOccurred() &&
+      SemaRef.Diags.hasUncompilableErrorOccurred()) {
     Invalid = true;
     return;
   }
   Invalid = CheckInstantiationDepth(PointOfInstantiation, InstantiationRange);
   if (!Invalid) {
     ActiveTemplateInstantiation Inst;
     Inst.Kind = Kind;
     Inst.PointOfInstantiation = PointOfInstantiation;

cfe/trunk/test/SemaCXX/instantiate-template-fatal-error.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++14 %s
+
+#pragma clang diagnostic fatal "-Wall"
+#pragma clang diagnostic fatal "-Wold-style-cast"
+
+template <class T> bool foo0(const long long *a, T* b) {
+  return a == (const long long*)b; // expected-error {{use of old-style cast}}
+}
+
+template<class T>
+struct S1 {
+};
+
+template<class T>
+struct S2 : S1<T> {
+  bool m1(const long long *a, T *b) const { return foo0(a, b); }
+};
+
+bool foo1(const long long *a, int *b) {
+  S2<int> s2;
+  return s2.m1(a, b);
+}