================
@@ -1710,15 +1710,17 @@ VendorSignatures getVendorSignature(unsigned *MaxLeaf) {
 
 #if defined(__i386__) || defined(_M_IX86) || \
     defined(__x86_64__) || defined(_M_X64)
-bool sys::getHostCPUFeatures(StringMap<bool> &Features) {
+std::optional<StringMap<bool>> sys::getHostCPUFeatures() {
   unsigned EAX = 0, EBX = 0, ECX = 0, EDX = 0;
   unsigned MaxLevel;
 
   if (getX86CpuIDAndInfo(0, &MaxLevel, &EBX, &ECX, &EDX) || MaxLevel < 1)
-    return false;
+    return {};
 
   getX86CpuIDAndInfo(1, &EAX, &EBX, &ECX, &EDX);
 
+  StringMap<bool> Features;
----------------
DavidSpickett wrote:

The caller creating a blank map to pass to this function is the symptom of what 
I'm looking to fix here. The implementations are still going to create a map if 
they're going to return features.

The motivation for the change is to allow:
```
if (auto features = sys::getCPUHostFeatures())
```
Instead of:
```
map features;
if (sys::getCPUHostFeatures(map)
```
As the optional better describes the situation. You either get a feature map or 
you don't. Versus bool+mutable ref which could allow any combination of states. 
The docstring helps but might as well use the typesystem to say the same thing 
as well.

If the callers were building a map as they went, I'd understand the mutable ref 
parameter. Like:
```
map features
if (add_some_features(features))
  if (add_some_more_features(features))
    filter_features(features)
```
But all uses create an empty map specifically to pass to the function.

...which is to say yeah, I should include the motivation in the commit message, 
let me do that :)

https://github.com/llvm/llvm-project/pull/97824
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to