[Lldb-commits] [PATCH] D39387: Invert ArchSpec<->Platform dependency

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.


================
Comment at: source/Target/Platform.cpp:986-991
+    if (normalized_triple.getVendorName().empty())
+      normalized_triple.setVendor(compatible_triple.getVendor());
+    if (normalized_triple.getOSName().empty())
+      normalized_triple.setOS(compatible_triple.getOS());
+    if (normalized_triple.getEnvironmentName().empty())
+      normalized_triple.setEnvironment(compatible_triple.getEnvironment());
----------------
zturner wrote:
> labath wrote:
> > zturner wrote:
> > > Are these cases even possible?  Why would the vendor and os ever be empty?  I thought only the environment could be empty.
> > It is possible, because in some cases, we actually only specify the architecture in calls to this function (see the arch_name comment above). There (I think) its only purpose is to disambiguate which slice in a fat binary are you talking about (and in that case, you don't really need to specify anything other than an architecture).
> > 
> > However, in that case, it is true that what we are passing is not really a triple.
> Right, but you are specifically checking against the //normalized// triple.  I'm pretty sure `Triple::normalize` will never set these to empty strings.  Maybe I'm wrong though (I'm actually about to test)
Huh.  Ok, I guess you're right.  The unit tests have this check:

```
  EXPECT_EQ("i386", Triple::normalize("i386"));
```

I assumed that would return `i386-unknown-unknown` since they are not considered "optional" components.


https://reviews.llvm.org/D39387