clayborg added a comment. In D138259#3943739 <https://reviews.llvm.org/D138259#3943739>, @labath wrote:
> In D138259#3941859 <https://reviews.llvm.org/D138259#3941859>, @clayborg > wrote: > >> In D138259#3941465 <https://reviews.llvm.org/D138259#3941465>, @labath wrote: >> >>> In D138259#3941431 <https://reviews.llvm.org/D138259#3941431>, @clayborg >>> wrote: >>> >>>> "a type should be complete but isn't and you are losing information that >>>> should have been available for you to debug". >>> >>> I agree, but there are still two (or more) ways to communicate that >>> information. >>> >>> 1. "this type is complete" + "actually, I'm just missing the debug info and >>> pretending it's complete" >>> 2. "this type is incomplete" + "it is incomplete because I am missing its >>> debug info" >>> >>> My question is which method would be more useful to the user. >> >> Gotcha. We could change "bool SBType::IsTypeComplete()" to return false, and >> then add a new: >> >> bool SBType::ShouldBeComplete(); >> >> That would return true if IsTypeComplete() returned false because it was >> forcefully completed. > > That is the expected behavior, but we wouldn't need to implement it that way. > We could still implement it by calling the "is forcefully completed" metadata > function, and so it wouldn't force the completion of the type. So a `true` > return value would mean that the type has been forcefully completed, while a > return value of `false` would mean that the type is complete (not just > pretend-complete) **or** the completion of the type hasn't been attempted yet. The current SBType::IsTypeComplete() will complete the type in order to figure out if it is actually complete. So although we can shortcut this for forcefully completed types, it wouldn't work for other types. > Note that I'm not saying that this is how it needs to be implemented. > However, I think its worth thinking about this, given that we're adding a new > public interface and all. I think it comes down to the question of which > situation would be more/less confusing to the (SB) user > > - querying a member/base class of an object and finding that it's incomplete > (even though that is not possible in regular c++) > - querying a member/base class of an object and finding that it's empty (even > though the truth is that we just don't know what it contains.) Why don't I remove this API change from this patch to allow the patch to get into open source and we can revisit if we need to since we don't have any clients for this right now. But I do think it is a good idea to change the SBTType::IsTypeComplete() to return false for forcefully completed types, so I will make that change and test it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138259/new/ https://reviews.llvm.org/D138259 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
