Relax the Symbol requirement to ConstBlockAddr

[Ptival]

As part of an attempt to fix GaloisInc/llvm-pretty-bc-parser#149 , I believe we may need some version of ConstBlockAddr that is more lazy in the symbol.

That is, the value of the symbol field is a forward reference, and eagerly trying to evaluate it leads to an infinite loop.

I am not very familiar with either code bases, so I don't know whether this is the recommended way of solving such issues, but I can at least confirm that such a constructor helps with that problem.

I'd also like to make sure that this:

https://github.com/elliottt/llvm-pretty/blob/5a821ad50edc33047c75311b7d7b31bd5fc52c21/src/Text/LLVM/Labels.hs#L118

does the right generic thing corresponding to what happens here:

https://github.com/elliottt/llvm-pretty/blob/5a821ad50edc33047c75311b7d7b31bd5fc52c21/src/Text/LLVM/Labels.hs#L140

[elliottt]

The Symbol on ConstBlockAddr is already not strict, why does adding this new constructor help? Is the value represented different from ValConstExpr (ConstBlockAddr sym lab)?

[Ptival]

Sorry, my use of the word lazy was misleading.

The problem as I understand it is that in order to put a Symbol-typed expression there, the parser has to run elimValSymbol (typedValue (forwardRef ctx valref t)), a computation whose spine depends on the result of its sub-expression:

https://github.com/elliottt/llvm-pretty/blob/641ae933652369cf0d7e1e47c8928f631fb71858/src/Text/LLVM/AST.hs#L1041-L1043

So we would either need a way of having a Symbol-typed value that has delayed its forward reference evaluation, or we need some way to store something other than a Symbol giving us evaluation flexibility. Does this make sense?

[elliottt]

Yep, this makes sense to me. What do you think about changing ConstBlockAddr to no longer take a Symbol, and instead take a Typed (Value' lab) for the symbol part? That would mean that you could remove the use of elimValSymbol when parsing, and validate that the argument is a symbol at a later stage once you're no longer in a context where the symbol might be defined lazily.

[elliottt]

An alternative approach would be to make a partial version of elimValSymbol, which would mean that you could decouple the check that it's a symbol from the sequencing of the Parse monad. Up to you about which seems like the better approach here, debugging lazy values with errors can be a bit of a trial, but I think there are other cases where that's done already in llvm-pretty-bc-parser.

[Ptival]

I like the first approach. I think I was worried that Const meant I shouldn't put expressions in there, but looking back, there are already many Typed (Value' lab) in there. Will update!

[Ptival]

This trick no longer works:

https://github.com/elliottt/llvm-pretty/blob/641ae933652369cf0d7e1e47c8928f631fb71858/src/Text/LLVM/Labels.hs#L140

Is the generic instance good enough, or ought this really evaluate the symbol to be able to call f on l?

[elliottt]

You could match on the typed value to call out the special case when a symbol is present, and otherwise fall back on the generic instance.

[elliottt]

However this might introduce additional strictness that could break what you're going for here. The difficulty is that the relabeling needs to know that the label comes from a different function, but the only mechanism to signal that is when the symbol value is present. I don't remember when the relabeling occurs in llvm-pretty-bc-parser, but that would be a good thing to check to determine if this is a change that would make sense.

[Ptival]

The updated version seems to solve my problems and not blow up in any of my examples!