[PATCH] D36907: [codeview] support more DW_OPs for more complete debug info

[Bob Haarman via Phabricator via llvm-commits]

inglorion added a comment.

Thanks for taking a look!



================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:1009
+      auto Op = DIExpr->expr_op_begin();
+      while (Supported && Op != DIExpr->expr_op_end()) {
+        switch (Op->getOp()) {
----------------
rnk wrote:
> Please factor out the DIExpression parsing into a helper that extracts the fragment info, Deref, Supported, and Offset.
> 
> In the long run, we might want to remodel DIExpression so that it encodes this information directly. Right now DIExpressions feel very "stringly-typed": they're just a sequence of integer opcodes and arguments that don't have any clear relationship to each other. Every bit of code that touches them rolls its own parsing.
> In the long run, we might want to remodel DIExpression so that it encodes this information directly.

I was thinking that, too. It's great to be able to express arbitrarily complex expressions, but if we generate only simple ones and backends handle only a few specific forms anyway, we might as well simplify our lives and avoid the extra parsing.

For now, I'll factor this out. I was actually planning to do that today anyway.


https://reviews.llvm.org/D36907