[PATCH] D138658: [DebugInfo] Protect DIFile::Source against empty string

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D138658#3951887 <https://reviews.llvm.org/D138658#3951887>, @Hahnfeld wrote:

> In D138658#3951866 <https://reviews.llvm.org/D138658#3951866>, @dblaikie wrote:
>
>> Where did this issue come up in practice? Change seems reasonable, but wouldn't mind having more context.
>
> Please see https://reviews.llvm.org/D137152 for context. I think what's happening is that we (the Cling interpreter in ROOT) create an empty source file while injecting some code during startup. It may be possible to fix that, but LLVM shouldn't crash from it.

hmm, more context might be nice - like where's the Cling code that's doing something different than the Clang code when it's creating DIFiles?

I think it might be more suitable to say that DIFile shouldn't be given a non-None Optional that refers to an empty string? & it's up to the caller (Cling) to fix that - we could add an assertion to that effect in the getImpl?

Or perhaps there's an intent to support present-but-zero-length source, in which case this change isn't suitable either, I guess? (since this change collapses the empty and None cases together)

If None and empty are meant to be identical, maybe removing the Optional layer entirely and using the empty string to communicate not-present would be suitable? I'm not sure.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138658/new/

https://reviews.llvm.org/D138658