feat(cxx_indexer): annotate edges we know are writes to vars and ivars (

#4558)

This PR marks a subset of ref edges known to be writes (currently using the primitive operator = with vars or ivars on the lhs) as ref/writes (or ref/writes/implicit).

kythe/cxx/common/indexing/KytheGraphRecorder.cc

@@ -81,6 +81,8 @@ static const std::string* kEdgeKindSpellings[] = {
     new std::string("/kythe/edge/property/writes"),
     new std::string("/clang/usr"),
     new std::string("/kythe/edge/ref/id"),
+    new std::string("/kythe/edge/ref/writes"),
+    new std::string("/kythe/edge/ref/writes/implicit"),
 };
 
 bool of_spelling(absl::string_view str, EdgeKindID* edge_id) {

kythe/cxx/common/indexing/KytheGraphRecorder.h

@@ -122,7 +122,9 @@ enum class EdgeKindID {
   kPropertyReads,
   kPropertyWrites,
   kClangUsr,
-  kRefId
+  kRefId,
+  kRefWrites,
+  kRefWritesImplicit
 };
 
 /// \brief Returns the Kythe spelling of `node_kind_id`

kythe/cxx/indexer/cxx/BUILD

@@ -82,6 +82,7 @@ cc_library(
         "-Wno-implicit-fallthrough",
     ],
     deps = [
+        ":indexed_parent_map",
         "//third_party/llvm/src:clang_builtin_headers",
         "@com_github_google_glog//:glog",
         "@org_llvm//:LLVMSupport",

kythe/cxx/indexer/cxx/GraphObserver.h

@@ -825,6 +825,20 @@ class GraphObserver {
                                    const NodeId& BlameId, Claimability Cl,
                                    Implicit I) {}
 
+  /// \brief Classifies a use site.
+  enum class UseKind {
+    /// No specific determination. Similar to a read.
+    kUnknown,
+    /// This use site is a write.
+    kWrite
+  };
+
+  /// \brief Records a use site for some decl with additional semantic
+  /// information.
+  virtual void recordSemanticDeclUseLocation(const Range& SourceRange,
+                                             const NodeId& DeclId, UseKind K,
+                                             Claimability Cl, Implicit I) {}
+
   /// \brief Records an init site for some decl.
   virtual void recordInitLocation(const Range& SourceRange,
                                   const NodeId& DeclId, Claimability Cl,

kythe/cxx/indexer/cxx/IndexerASTHooks.cc

@@ -2033,9 +2033,12 @@ bool IndexerASTVisitor::VisitDeclRefOrIvarRefExpr(
           }
         }
       }
-      Observer.recordDeclUseLocation(*RCC, DeclId,
-                                     GraphObserver::Claimability::Unclaimable,
-                                     this->IsImplicit(*RCC));
+      auto semantic = IsUsedAsWrite(*getAllParents(), Expr)
+                          ? GraphObserver::UseKind::kWrite
+                          : GraphObserver::UseKind::kUnknown;
+      Observer.recordSemanticDeclUseLocation(
+          *RCC, DeclId, semantic, GraphObserver::Claimability::Unclaimable,
+          this->IsImplicit(*RCC));
       for (const auto& S : Supports) {
         S->InspectDeclRef(*this, SL, *RCC, DeclId, FoundDecl);
       }

kythe/cxx/indexer/cxx/KytheGraphObserver.cc

@@ -64,6 +64,18 @@ absl::string_view ConvertRef(llvm::StringRef ref) {
   return absl::string_view(ref.data(), ref.size());
 }
 
+EdgeKindID EdgeKindForUseKind(GraphObserver::UseKind kind,
+                              GraphObserver::Implicit i) {
+  switch (kind) {
+    case GraphObserver::UseKind::kUnknown:
+      return (i == GraphObserver::Implicit::Yes ? EdgeKindID::kRefImplicit
+                                                : EdgeKindID::kRef);
+    case GraphObserver::UseKind::kWrite:
+      return (i == GraphObserver::Implicit::Yes ? EdgeKindID::kRefWritesImplicit
+                                                : EdgeKindID::kRefWrites);
+  }
+}
+
 }  // anonymous namespace
 
 using clang::SourceLocation;
@@ -1071,6 +1083,12 @@ void KytheGraphObserver::recordBlameLocation(
                Claimability::Claimable);
 }
 
+void KytheGraphObserver::recordSemanticDeclUseLocation(
+    const GraphObserver::Range& source_range, const NodeId& node, UseKind kind,
+    Claimability claimability, Implicit i) {
+  RecordAnchor(source_range, node, EdgeKindForUseKind(kind, i), claimability);
+}
+
 void KytheGraphObserver::recordInitLocation(
     const GraphObserver::Range& source_range, const NodeId& node,
     Claimability claimability, Implicit i) {

kythe/cxx/indexer/cxx/KytheGraphObserver.h

@@ -243,6 +243,10 @@ class KytheGraphObserver : public GraphObserver {
                            GraphObserver::Claimability cl,
                            GraphObserver::Implicit i) override;
 
+  void recordSemanticDeclUseLocation(const Range& SourceRange,
+                                     const NodeId& DeclId, UseKind K,
+                                     Claimability Cl, Implicit I) override;
+
   void recordInitLocation(const Range& source_range, const NodeId& node,
                           GraphObserver::Claimability cl,
                           GraphObserver::Implicit i) override;

kythe/cxx/indexer/cxx/clang_utils.cc

@@ -93,4 +93,25 @@ bool ShouldHaveBlameContext(const clang::Decl* decl) {
       return false;
   }
 }
+
+bool IsUsedAsWrite(const IndexedParentMap& map, const clang::Stmt* stmt) {
+  // TODO(zarko): Improve coverage (or get rid of this entirely and maintain
+  // traversal state in the AST walker; this would be more of a maintenance
+  // and correctness burden, but may be required for richer representations.)
+  if (stmt == nullptr) return false;
+  const auto* indexed_parent = map.GetIndexedParent(*stmt);
+  if (indexed_parent == nullptr) return false;
+  const auto* parent_stmt = indexed_parent->parent.get<clang::Stmt>();
+  if (parent_stmt == nullptr) return false;
+  switch (parent_stmt->getStmtClass()) {
+    case clang::Stmt::StmtClass::BinaryOperatorClass: {
+      const auto* binop = clang::dyn_cast<clang::BinaryOperator>(parent_stmt);
+      if (binop == nullptr) return false;
+      return binop->getOpcode() == clang::BinaryOperator::Opcode::BO_Assign &&
+             binop->getLHS() == stmt;
+    }
+    default:
+      return false;
+  }
+}
 }  // namespace kythe

kythe/cxx/indexer/cxx/clang_utils.h

@@ -21,6 +21,7 @@
 #include "clang/AST/DeclarationName.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceManager.h"
+#include "kythe/cxx/indexer/cxx/indexed_parent_map.h"
 
 namespace kythe {
 /// \return true if `DN` is an Objective-C selector.
@@ -34,6 +35,10 @@ const clang::Decl* FindSpecializedTemplate(const clang::Decl* decl);
 /// \return true if a reference to `decl` should be given blame context.
 bool ShouldHaveBlameContext(const clang::Decl* decl);
 
+/// \return true if `stmt` is being used in a write position according to
+/// `map`.
+bool IsUsedAsWrite(const IndexedParentMap& map, const clang::Stmt* stmt);
+
 }  // namespace kythe
 
 #endif  // KYTHE_CXX_INDEXER_CXX_CLANG_UTILS_H_

kythe/cxx/indexer/cxx/testdata/BUILD

@@ -3240,6 +3240,12 @@ cc_indexer_test(
     tags = ["df"],
 )
 
+cc_indexer_test(
+    name = "df_var_rw",
+    srcs = ["df/df_var_rw.cc"],
+    tags = ["df"],
+)
+
 test_suite(
     name = "indexer_df",
     tags = ["df"],

kythe/cxx/indexer/cxx/testdata/df/df_var_ref_blame.cc

@@ -4,7 +4,7 @@
 void f() {
   //- @x defines/binding VarX
 	int x;
-  //- @x ref VarX
+  //- @x ref/writes VarX
   //- @x childof FnF
 	x = 3;
 }

kythe/cxx/indexer/cxx/testdata/df/df_var_rw.cc

@@ -0,0 +1,10 @@
+// Variable writes are distinguished from reads.
+
+int f() {
+  //- @x defines/binding VarX
+  int x = 3;  // Definition site.
+  //- @x ref/writes VarX
+  x = 4;  // Write site.
+  //- @x ref VarX
+  return x;  // Read site.
+}

kythe/cxx/indexer/cxx/testdata/objc/categories/extension_property.m

@@ -29,7 +29,7 @@ @implementation Box
 @synthesize val = data;
 
 -(int) foo {
-  //- @data ref Data
+  //- @data ref/writes Data
   self->data = 300;
 
   //- @x defines/binding XVar

kythe/cxx/indexer/cxx/testdata/objc/ivar_decl.m

@@ -15,7 +15,7 @@ @interface Box {
 //- @Box defines/binding BoxImpl
 @implementation Box
 -(int)foo {
-  //- @data ref Data
+  //- @data ref/writes Data
   self->data = 300;
   return self->data;
 }

kythe/cxx/indexer/cxx/testdata/objc/property_decl_defn_synth.m

@@ -45,25 +45,25 @@ @implementation Box
 @synthesize d = newvar;
 
 -(void) foo {
-  //- @width ref WidthIVarDecl
+  //- @width ref/writes WidthIVarDecl
   self->width = 100;
 
   //- @a ref ADecl
   self.a = 200;
 
   //- @b ref BDecl
   self.b = 300;
-  //- @"_b" ref BIvarDecl
+  //- @"_b" ref/writes BIvarDecl
   self->_b = 301;
 
   //- @c ref CDecl
   self.c = 400;
-  //- @c ref CIvarDecl
+  //- @c ref/writes CIvarDecl
   self->c = 401;
 
   //- @d ref DDecl
   self.d = 500;
-  //- @newvar ref NewvarIVarDecl
+  //- @newvar ref/writes NewvarIVarDecl
   self->newvar = 501;
 }
 @end

kythe/cxx/indexer/cxx/testdata/rec/rec_class_member_ptr.cc

@@ -15,6 +15,6 @@ void fn() {
   int C::* mptr;
   //- @C ref CDecl
   //- @member ref MemberDecl
-  //- @mptr ref PtrDecl
+  //- @mptr ref/writes PtrDecl
   mptr = &C::member;
 }