Empty strings vs Maybe Strings in DIFile

[langston-barrett]

Currently, llvm-pretty requires that the Directory and File arguments of DIFile be non-null:

data DIFile = DIFile
  { difFilename  :: FilePath
  , difDirectory :: FilePath
  } deriving (Show,Generic)

However, in several LLVM test cases, one or both of these are "", which leads the parser to be unable to parse these when they are assembled with llvm-as.

• https://github.com/llvm-mirror/llvm/blob/release_38/test/DebugInfo/AArch64/cfi-eof-prologue.ll
• https://github.com/llvm-mirror/llvm/blob/release_38/test/Linker/type-unique-simple2-b.ll#L58
• https://github.com/llvm-mirror/llvm/blob/release_38/test/Assembler/debug-info.ll#L38

Should we consider making these both Maybes? Or is the correct behavior of the parser to do something like this:

        <*> (fromMaybe "" <$> mdStringOrNull ctx mt <$> parseField r 2 numeric) -- difDirectory

here: https://github.com/GaloisInc/llvm-pretty-bc-parser/blob/master/src/Data/LLVM/BitCode/IR/Metadata.hs#L442?

Let me know what you think @elliottt

[elliottt]

I think I like the latter approach a little better, as it keeps the value you would be working with (DIFile) a bit closer to what you'd see in the LLVM textual source.

[langston-barrett]

@elliottt Thanks for the feedback!

It seems the current behavior of both llvm-pretty and llvm-pretty-bc-parser is correct; since posting this, the parser moved to the fromMaybe solution: https://github.com/GaloisInc/llvm-pretty-bc-parser/blob/master/src/Data/LLVM/BitCode/IR/Metadata.hs#L175