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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D58653#1410907 <https://reviews.llvm.org/D58653#1410907>, @xiaobai wrote:

> 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.


The "llvm::Triple::normalize" must have been added at some point and I missed it. That will mess up some assumptions in LLDB that have been around for a while.

> 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.

I agree testing is good. The main things I would like to stay consistent are:

- if any part of a triple isn't specified, we need to be able to tell so that MergeFrom can merge in any unspecified parts (arch, env, os, vendor)
- if any part of a triple was specified, including "unknown" (since there is no "none" for OS or vendor), then it should remain as is during a MergeFrom

Target triples are used by many plug-ins to determine if they should load or not. The DynamicLoaderStatic is used for bare board development and relies on someone being able to specify "armv7-unknown-unknown" so that it knows the triple is has a specified "unknown" for OS and vendor. Then it loads all images at the address that they are in the file (load address == file address).


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

https://reviews.llvm.org/D58653