[Lldb-commits] [PATCH] D122912: Simplify ArchSpec::IsFullySpecifiedTriple() (NFC)

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
Comment at: lldb/source/Utility/ArchSpec.cpp:1399
 bool ArchSpec::IsFullySpecifiedTriple() const {
-  const auto &user_specified_triple = GetTriple();
-
-  bool user_triple_fully_specified = false;
-
-  if ((user_specified_triple.getOS() != llvm::Triple::UnknownOS) ||
-      TripleOSWasSpecified()) {
-    if ((user_specified_triple.getVendor() != llvm::Triple::UnknownVendor) ||
-        TripleVendorWasSpecified()) {
-      const unsigned unspecified = 0;
-      if (!user_specified_triple.isOSDarwin() ||
-          user_specified_triple.getOSMajorVersion() != unspecified) {
-        user_triple_fully_specified = true;
-      }
-    }
-  }
+  if (!TripleOSWasSpecified())
+    return false;
----------------
aprantl wrote:
> JDevlieghere wrote:
> > Are you sure that `TripleOSWasSpecified() == user_specified_triple.getOS() != llvm::Triple::UnknownOS`. Was the check in the original code redundant? Looking at the implementation
> > 
> > ```
> > bool TripleOSWasSpecified() const { return !m_triple.getOSName().empty(); }
> > ```
> > 
> > I guess this is identical? But when dealing with triples it's probably worth double checking. 
> Great question, yes I checked that. The code for both
> 
> https://llvm.org/doxygen/Triple_8cpp_source.html#l00524
> 
> is a StringSwitch that uses Unknown as the Default case.
> 
> 
Thanks for checking!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122912