[PATCH] D119363: [clang] Add `ObjCProtocolLoc` to represent protocol references

[Sam McCall via Phabricator via cfe-commits]

sammccall added a subscriber: akyrtzi.
sammccall added a comment.

Thanks @akyrtzi @egorzhdan I was going to suggest that I think someone at Apple should probably look at changes here first.

The goal here is to ensure that any tokens in the code that reference a decl have some AST node (which can be stored in DynTypedNode).
This is provides a nice simple model which clangd's go-to-definition etc rely on: token -> ast node -> target.
Clang almost follows this rule but references to protocols have no dedicated AST node.



================
Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:331
+  /// \returns false if the visitation was terminated early, true otherwise.
+  bool TraverseObjCProtocolLoc(ObjCProtocolLoc ProtocolLoc);
+
----------------
RAV changes should have tests


================
Comment at: clang/include/clang/AST/TypeLoc.h:2612
+public:
+  const ObjCProtocolDecl *Protocol = nullptr;
+  SourceLocation Loc = SourceLocation();
----------------
These fields shouldn't be public in AST nodes, provide accessors instead (I think they can be read-only).


================
Comment at: clang/include/clang/AST/TypeLoc.h:2621
+  /// Evaluates true when this protocol loc is valid/non-empty.
+  explicit operator bool() const { return Protocol; }
+};
----------------
will we ever have invalid instances?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119363/new/

https://reviews.llvm.org/D119363