[PATCH] D62184: [AST] Add RecoveryExpr; produce it on overload resolution failure and missing member.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
Herald added subscribers: cfe-commits, kadircet, arphaman, ilya-biryukov.
Herald added a project: clang.

In many places, Sema refuses to create an Expr if it encounters an error.
This means that broken code has no representation in the AST. As well as
the broken node itself, child Exprs and parent Exprs are usually dropped.

This is terrible for tools that rely on the AST and see a lot of broken
code (e.g. libclang-based IDEs and clangd).
In the expression `foo(a, takes2args(b))`, "go to definition" doesn't
work on any of the subexpressions. And after `takes2args(x).`, member
completion doesn't work, although in practice we almost always know the type.

This patch introduces RecoveryExpr. This AST node represents broken
code, and has very weak semantics. It's part of the final AST (unlike TypoExpr)
but can't produce code, as it's always accompanied by an error.
It captures valid subexpressions without checking or assigning them meaning.
Some RecoveryExprs have a known type (e.g. a broken non-overloaded call).
Where the type is unknown, it is modeled as a new type ErrorTy.

In this patch, ErrorTy is not dependent. See D61722 <https://reviews.llvm.org/D61722> for that approach.
Here ErrorTy must be explicitly handled on many paths, similar to dependent
types (though also for C). I've certainly missed some that tests didn't flag.

Initiallly, RecoveryExpr is emitted in two common cases:

- a function call where overload resolution failed (most typically: wrong args) Callee is captured as an UnresolvedLookupExpr, and args are captured. Type is available if the "best" candidates have the same return type.
- access of a nonexistent member Base expression is captured, type is never available.

Test changes:

- SemaCXX/enable_if.cpp: we now emit more detailed diagnostics (the non-constexpr subexpression is inside a function call), which breaks the heuristic that tries to move the whole diagnostic to the subexpression location. This case (IMO) doesn't matter much: the subexpression is invalid, flagging it as non-constexpr isn't very useful to start with.
- SemaTemplate/instantiate-function-params.cpp: it looks like the testcase was minimized in an unrealistic way: the only version of if_ doesn't have `type` and the only version of `requirement_` doesn't have `failed`, and it seems reasonable to diagnose this. If the precise invalid form is required, I can add another 17 expect-*s to the test, or remove -verify so we only fail on crashing. Original patch adding this test is very terse: https://github.com/llvm/llvm-project/commit/5112157958438005095aff805853f9b14ba974eb Testcase subsequently modified to test for a crash: https://github.com/llvm/llvm-project/commit/a02bb341552b2b91c93e708645c32d11cc4133d2
- CodeGen/builtins-systemz-zvector[2]-error.c: these tests now generate extra noisy (but correct) diagnostics: vecintrin.h uses a cast to an invalid type to suppress them, and the cast now succeeds. The right fix is probably to rewrite the builtin header somehow, I haven't investigated fully yet.


Repository:
  rC Clang

https://reviews.llvm.org/D62184

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/BuiltinTypes.def
  include/clang/AST/Expr.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/Stmt.h
  include/clang/AST/TextNodeDumper.h
  include/clang/AST/Type.h
  include/clang/Basic/StmtNodes.td
  include/clang/Sema/Initialization.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/AST/NSAPI.cpp
  lib/AST/Stmt.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/AST/TextNodeDumper.cpp
  lib/AST/Type.cpp
  lib/AST/TypeLoc.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Index/USRGeneration.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaCoroutine.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  lib/Sema/SemaExprMember.cpp
  lib/Sema/SemaInit.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaPseudoObject.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/CXX/temp/temp.decls/temp.variadic/fixed-expansion.cpp
  test/CodeCompletion/member-access.cpp
  test/Index/getcursor-recovery.cpp
  test/Parser/recovery.c
  test/SemaCXX/constructor-initializer.cpp
  test/SemaCXX/enable_if.cpp
  test/SemaTemplate/dependent-names.cpp
  test/SemaTemplate/instantiate-function-params.cpp
  test/SemaTemplate/instantiate-init.cpp
  tools/libclang/CIndex.cpp
  tools/libclang/CXCursor.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D62184.200451.patch
Type: text/x-patch
Size: 55875 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190521/d472e3f0/attachment-0001.bin>