[PATCH] D138658: [DebugInfo] Store optional DIFile::Source as pointer

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

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

>> I think we can't use `getCanonicalMDString` here, because it collapses the empty case into the null case, when they were intended to be differentiated. So this'll need to be manually expanded to the `MDString::get(Context, S)` without the empty->nullptr part. Then empty -> empty MDString, None -> null, non-empty -> non-empty MDString.
>
> Ok, I understood @scott.linder's comment to say that we can collapse the two cases.

@scott.linder mind clarifying what you had in mind? Did you mean to suggest that we should remove support for zero length source files as distinct from absent source files (eg: MDString * is null for empty or zero-length source, and non-null+non-empty otherwise?) or to drop the Optional but still render 3 cases distinctly (null for not present, non-null but zero-length for zero-length source, and non-null/non-zero-length otherwise)?

> Maybe I got this wrong, could you clarify if we need to differentiate the two cases? If so, I'm not actually sure it makes sense to have a non-`nullptr` for the empty string, this would be a first as well, all other cases in this file are using `getCanonicalMDString`...

Because, I think, all the other cases don't need to differentiate not-present from empty, but this case does, maybe. So I think that it could be different in that way.


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

https://reviews.llvm.org/D138658