[PATCH] D56383: [XRay][tools] Use symbols instead of function id
Kristina Brooks via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 7 21:41:04 PST 2019
kristina added a comment.
Quite a big style nit. I'd say get rid of the switch completely.
================
Comment at: llvm/tools/llvm-xray/xray-graph-diff.cpp:353
+ .str());
default:
if (containsNullptr(VertexAttr.CorrVertexPtr))
----------------
- Default is generally the first case in a switch.
- Switch case, followed by if, followed by return. Somewhat difficult to parse.
- No extra indentation for case statements.
- //Is there a case for using a switch statement here at all?// I think just an if statement will do, plus you can make use of early return here more easily. Switch is generally hard to parse (by a human) construct so I think it's generally a bad idea to use 2 case switches.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56383/new/
https://reviews.llvm.org/D56383
More information about the llvm-commits
mailing list