Merge pull request #6168 from trufflesuite/factor-nothing

Fix modifier argument detection by factoring out no-op handling

packages/debugger/lib/data/sagas/index.js

@@ -7,7 +7,8 @@ import {
  prefixName,
  stableKeccak256,
  makeAssignment,
- makePath
+ makePath,
+ peelAwayPotentialEVMNoOp
 } from "lib/helpers";
 
 import { TICK } from "lib/trace/actions";
@@ -776,6 +777,8 @@ function* decodeMappingKeyCore(indexDefinition, keyDefinition) {
  const modifierDepth = yield select(data.current.modifierDepth);
  const inModifier = yield select(data.current.inModifier);
 
+ let peeledIndexDefinition;
+
  //why the loop? see the end of the block it heads for an explanatory
  //comment
  while (true) {
@@ -864,41 +867,19 @@ function* decodeMappingKeyCore(indexDefinition, keyDefinition) {
  }
  }
  //there's still one more reason we might have failed to decode it:
- //certain (silent) type conversions aren't sourcemapped either.
+ //certain operations aren't sourcemapped because they're no-ops on
+ //the EVM side. This includes, for instance, certain type conversions.
  //(thankfully, any type conversion that actually *does* something seems
  //to be sourcemapped.) So if we've failed to decode it, we try again
- //with the argument of the type conversion, if it is one; we leave
+ //with the argument of the no-op operation; we leave
  //indexValue undefined so the loop will continue
  //(note that this case is last for a reason; if this were earlier, it
  //would catch *non*-silent type conversions, which we want to just read
  //off the stack)
  else if (
- indexDefinition.nodeType === "FunctionCall" &&
- indexDefinition.kind === "typeConversion"
- ) {
- debug("type conversion case");
- indexDefinition = indexDefinition.arguments[0];
- }
- //...also prior to 0.5.0, unary + was legal, which needs to be accounted
- //for for the same reason
- else if (
- indexDefinition.nodeType === "UnaryOperation" &&
- indexDefinition.operator === "+"
- ) {
- debug("unary + case");
- indexDefinition = indexDefinition.subExpression;
- }
- //...and starting in 0.8.8, we'd better handle wrap and unwrap as well for
- //the same reason!
- else if (
- indexDefinition.nodeType === "FunctionCall" &&
- indexDefinition.kind === "functionCall" &&
- ["wrap", "unwrap"].includes(
- Codec.Ast.Utils.functionClass(indexDefinition.expression)
- )
+ (peeledIndexDefinition = peelAwayPotentialEVMNoOp(indexDefinition))
  ) {
- debug("wrap/unwrap case");
- indexDefinition = indexDefinition.arguments[0];
+ indexDefinition = peeledIndexDefinition;
  }
  //otherwise, we've just totally failed to decode it, so we mark
  //indexValue as null (as distinct from undefined) to indicate this. In

packages/debugger/lib/data/selectors/index.js

@@ -10,7 +10,8 @@ import {
  stableKeccak256,
  makePath,
  topLevelNodeTypes,
- isTopLevelNode
+ isTopLevelNode,
+ peelAwayPotentialEVMNoOp
 } from "lib/helpers";
 
 import trace from "lib/trace/selectors";
@@ -1336,18 +1337,17 @@ const data = createSelectorTree({
  }
 
  //now: are we on the node corresponding to an argument, or, if
- //it's a type conversion, its nested argument?
+ //it's a potential EVM no-op, its nested argument?
  if (index === undefined) {
  return false;
  }
  let argument = invocation.arguments[index];
- while (argument.kind === "typeConversion") {
+ do {
  if (node.id === argument.id) {
  return true;
  }
- argument = argument.arguments[0];
- }
- return node.id === argument.id;
+ } while ((argument = peelAwayPotentialEVMNoOp(argument)));
+ return false;
  }
  ),
 

packages/debugger/lib/helpers/index.js

@@ -1,6 +1,27 @@
 import * as Codec from "@truffle/codec";
 import stringify from "json-stable-stringify";
 
+export function peelAwayPotentialEVMNoOp(node) {
+ if (node.nodeType === "FunctionCall" && node.kind === "typeConversion") {
+ //some type conversions are no-ops;
+ //not all are, but it's not worth the trouble to detect which ones,
+ //the places where this is used no it's only checking for *potential* no-ops
+ return node.arguments[0];
+ } else if (node.nodeType === "UnaryOperation" && node.operator === "+") {
+ //prior to 0.5.0, unary + was legal, which was a no-op
+ return node.subExpression;
+ } else if (
+ node.nodeType === "FunctionCall" &&
+ node.kind === "functionCall" &&
+ ["wrap", "unwrap"].includes(Codec.Ast.Utils.functionClass(node.expression))
+ ) {
+ //starting in 0.8.8, there are wrap and unwrap which are no-ops
+ return node.arguments[0];
+ } else {
+ return null;
+ }
+}
+
 /** AST node types that are skipped by stepNext() to filter out some noise */
 export function isDeliberatelySkippedNodeType(node) {
  const skippedTypes = [