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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
