[Lldb-commits] [PATCH] D43912: [Symbol] Add InvalidType, a force-checked recoverable error

[Vedant Kumar via Phabricator via lldb-commits]

vsk created this revision.
vsk added reviewers: labath, zturner, davide, aprantl, lhames.

LLDB CompilerTypes may be invalid, i.e they should be checked for
validity before use.

Compared to a more typical model in which only valid types exist [1],
this has a few disadvantages:

- The debugger is guaranteed to operate on invalid types as if they were valid, leading to unexpected or unpredictable results. Because type validity checks are not mandatory, they can and will be skipped.

  E.g in this patch I chose a random function which returns a type. Its callers checked for invalid results inconsistently, hiding a bug.
- Operations on types have high complexity, because each operation has fallback paths to deal with invalid types [2]. Beyond making code hard to read, this results in redundant double-checking of type validity.

Going forward, we should transition to a model in which CompilerTypes
are either valid or do not exist. This is a first, incremental step
towards that goal. Ultimately we should delete "Type::IsValid()".

[1] See llvm::Type, clang::Type, swift::Type, etc. All of these either
exist and are valid, or do not exist.

[2] See the methods in CompilerType. Each of these branch on the type's
validity.

[3] For an overview of the principles and advantages of llvm::Error, see
Lang Hames's talk: https://www.youtube.com/watch?v=Wq8fNK98WGw.


https://reviews.llvm.org/D43912

Files:
  include/lldb/Symbol/CompilerType.h
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
  source/Symbol/CompilerType.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43912.136399.patch
Type: text/x-patch
Size: 5845 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20180228/25b0fc76/attachment-0001.bin>