[Lldb-commits] [PATCH] D58653: [Utility] Allow the value 'unknown' when checking if triple components are unknown

Tue Feb 26 10:40:01 PST 2019

xiaobai added a comment.

In D58653#1410593 <https://reviews.llvm.org/D58653#1410593>, @clayborg wrote:

> A "specified unknown" is when the user actually typed "unknown" in their triple. IIRC there is no "none" to specify that there is no OS, so we use "specified unknowns" for this case. In this case we expect the enum _and_ the string accessor value for vendor and/or OS to be the string "unknown".

This will work when the user types "armv7" as their triple, but when they type "armv7--linux" the vendor gets implicitly set to "unknown" even though the user didn't specify it.

In D58653#1410184 <https://reviews.llvm.org/D58653#1410184>, @labath wrote:

> I think this basically defeats the purpose of the `IsUnspecifiedUnknown` functions, which was to differentiate between foo-unknown-bar and foo--bar triples. That in itself was a pretty big hack, but it seems that there is functionality which depends on this. There was a discussion about this back in december http://lists.llvm.org/pipermail/lldb-dev/2018-December/014437.html, and IIRC it converged to some sort of a conclusion, but I don't think there hasn't been any effort to try to implement it. I'm not sure what exactly it means about the future of this patch, but I'd be cautious about it.

Part of what you said is inconsistent with my understanding. Specifically, if you specify a Triple with the string "foo-unknown-bar" and another Triple with the string "foo--bar" you end up with the same triple. We rely on the `llvm::Triple::normalize` function to create triples from strings, and there is no way to indicate using this method that a triple has unspecified non-optional parts. We are relying on ArchSpec being created with a Triple object and not a StringRef for this behavior to work.

Yeah it does kind of defeat the purpose of some of these functions. I would like to change it to just `IsUnknown` (or just delete the functions entirely) because `IsUnspecifiedUnknown`should always be the opposite of the `WasSpecified` functions. My understanding is that the llvm Triple class tries its best to make sure things are marked as the Unknown enum value if they are unspecified or marked as unknown, but there could be some strange edge case I'm not aware of.

Regardless, I would like to add these tests (or similar ones) to document the understanding of when vendors, OSes, and environments are considered unspecified or not.

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

https://reviews.llvm.org/D58653