[sil] Teach VariableNameInferrer how to look through /all/ accessors.

Author: gottesmm

include/swift/SILOptimizer/Utils/VariableNameUtils.h

@@ -17,6 +17,7 @@
 #ifndef SWIFT_SILOPTIMIZER_UTILS_VARIABLENAMEUTILS_H
 #define SWIFT_SILOPTIMIZER_UTILS_VARIABLENAMEUTILS_H
 
+#include "swift/Basic/OptionSet.h"
 #include "swift/SIL/ApplySite.h"
 #include "swift/SIL/DebugUtils.h"
 #include "swift/SIL/MemAccessUtils.h"
@@ -26,6 +27,19 @@
 namespace swift {
 
 class VariableNameInferrer {
+public:
+  enum class Flag {
+    /// If set then we should look through get and set accessors and infer their
+    /// name from self.
+    ///
+    /// DISCUSSION: This may not be the correct semantics for all name inference
+    /// since we may want to consider computed properties to be tied to self.
+    InferSelfThroughAllAccessors = 0x1,
+  };
+
+  using Options = OptionSet<Flag>;
+
+private:
   /// The stacklist that we use to process from use->
   StackList<PointerUnion<SILInstruction *, SILValue>> variableNamePath;
 
@@ -37,10 +51,21 @@ class VariableNameInferrer {
   /// The final string we computed.
   SmallString<64> &resultingString;
 
+  /// Options that control how we do our walk.
+  ///
+  /// Example: In certain cases we may want to impute self as a name for
+  /// computed getters/setters and in other cases we may not want to.
+  Options options;
+
 public:
   VariableNameInferrer(SILFunction *fn, SmallString<64> &resultingString)
       : variableNamePath(fn), resultingString(resultingString) {}
 
+  VariableNameInferrer(SILFunction *fn, Options options,
+                       SmallString<64> &resultingString)
+      : variableNamePath(fn), resultingString(resultingString),
+        options(options) {}
+
   /// Attempts to infer a name from just uses of \p searchValue.
   ///
   /// Returns true if we found a name.

lib/SILOptimizer/Utils/VariableNameUtils.cpp

@@ -124,30 +124,40 @@ VariableNameInferrer::findDebugInfoProvidingValue(SILValue searchValue) {
     }
 
     auto getNamePathComponentFromCallee =
-        [&](FullApplySite call) -> std::optional<SILValue> {
+        [&](FullApplySite call) -> SILValue {
       // Use the name of the property being accessed if we can get to it.
       if (isa<FunctionRefBaseInst>(call.getCallee()) ||
           isa<MethodInst>(call.getCallee())) {
-        variableNamePath.push_back(call.getCallee()->getDefiningInstruction());
-        // Try to name the base of the property if this is a method.
         if (call.getSubstCalleeType()->hasSelfParam()) {
+          variableNamePath.push_back(call.getCallee()->getDefiningInstruction());
           return call.getSelfArgument();
         }
 
         return SILValue();
       }
-      return {};
+
+      return SILValue();
     };
 
     // Read or modify accessor.
     if (auto bai = dyn_cast_or_null<BeginApplyInst>(
             searchValue->getDefiningInstruction())) {
       if (auto selfParam = getNamePathComponentFromCallee(bai)) {
-        searchValue = *selfParam;
+        searchValue = selfParam;
         continue;
       }
     }
 
+    if (options.contains(Flag::InferSelfThroughAllAccessors)) {
+      if (auto fas =
+              FullApplySite::isa(searchValue->getDefiningInstruction())) {
+        if (auto selfParam = getNamePathComponentFromCallee(fas)) {
+          searchValue = selfParam;
+          continue;
+        }
+      }
+    }
+
     // Addressor accessor.
     if (auto ptrToAddr =
             dyn_cast<PointerToAddressInst>(stripAccessMarkers(searchValue))) {
@@ -164,7 +174,7 @@ VariableNameInferrer::findDebugInfoProvidingValue(SILValue searchValue) {
       if (addressorInvocation) {
         if (auto selfParam =
                 getNamePathComponentFromCallee(addressorInvocation)) {
-          searchValue = *selfParam;
+          searchValue = selfParam;
           continue;
         }
       }
@@ -263,7 +273,9 @@ static FunctionTest VariableNameInferrerTests(
     "variable-name-inference", [](auto &function, auto &arguments, auto &test) {
       auto value = arguments.takeValue();
       SmallString<64> finalString;
-      VariableNameInferrer inferrer(&function, finalString);
+      VariableNameInferrer::Options options;
+      options |= VariableNameInferrer::Flag::InferSelfThroughAllAccessors;
+      VariableNameInferrer inferrer(&function, options, finalString);
       SILValue rootValue =
           inferrer.inferByWalkingUsesToDefsReturningRoot(value);
       llvm::outs() << "Input Value: " << *value;

test/SILOptimizer/variable_name_inference.sil

@@ -1,12 +1,29 @@
-// RUN: %target-sil-opt -test-runner %s 2>&1 | %FileCheck %s
+// RUN: %target-sil-opt -module-name infer -test-runner %s 2>&1 | %FileCheck %s
 
 import Builtin
 
-class Klass {}
+//===----------------------------------------------------------------------===//
+//                             MARK: Declarations
+//===----------------------------------------------------------------------===//
+
+class Klass {
+}
+
+class ContainsKlass {
+  var computedKlass: Klass { get }
+  final var klass: Klass
+
+  init()
+}
 
 sil @getKlass : $@convention(thin) () -> @owned Klass
+sil @getContainsKlass : $@convention(thin) () -> @owned ContainsKlass
 sil @useIndirect : $@convention(thin) <T> (@in_guaranteed T) -> ()
 
+//===----------------------------------------------------------------------===//
+//                                MARK: Tests
+//===----------------------------------------------------------------------===//
+
 // CHECK-LABEL: begin running test {{[0-9]+}} of {{[0-9]+}} on simple_test_case: variable-name-inference with: @trace[0]
 // CHECK: Input Value:   %1 = apply %0() : $@convention(thin) () -> @owned Klass
 // CHECK: Name: 'MyName'
@@ -74,3 +91,25 @@ bb0:
   %9999 = tuple ()
   return %9999 : $()
 }
+
+// CHECK-LABEL: begin running test {{.*}} of {{.*}} on look_through_accessors_get: variable-name-inference with: @trace[0]
+// CHECK: Name: 'myName.computedKlass'
+// CHECK: Root: %2 = move_value [lexical] [var_decl]
+// CHECK: end running test {{.*}} of {{.*}} on look_through_accessors_get: variable-name-inference with: @trace[0]
+sil [ossa] @look_through_accessors_get : $@convention(thin) () -> () {
+bb0:
+  specify_test "variable-name-inference @trace[0]"
+  %0 = function_ref @getContainsKlass : $@convention(thin) () -> @owned ContainsKlass
+  %1 = apply %0() : $@convention(thin) () -> @owned ContainsKlass
+  %3 = move_value [lexical] [var_decl] %1 : $ContainsKlass
+  debug_value %3 : $ContainsKlass, let, name "myName"
+  %5 = begin_borrow %3 : $ContainsKlass
+  %6 = class_method %5 : $ContainsKlass, #ContainsKlass.computedKlass!getter : (ContainsKlass) -> () -> Klass, $@convention(method) (@guaranteed ContainsKlass) -> @owned Klass
+  %7 = apply %6(%5) : $@convention(method) (@guaranteed ContainsKlass) -> @owned Klass
+  debug_value [trace] %7 : $Klass
+  end_borrow %5 : $ContainsKlass
+  destroy_value %7 : $Klass
+  destroy_value %3 : $ContainsKlass
+  %13 = tuple ()
+  return %13 : $()
+}