lit: Remove the binary_feature function and inline it everywhere.

[pcc]

Summary

This is not only less code but makes the feature names easier to grep for.

[pcc]

• Fix typo

[hctim]

I see four instances of nozlib. Could we canonicalise the prefix to not_zlib and update the following?

test/MC/ELF/nocompression.s
test/tools/llvm-dwp/X86/nocompress.test
test/tools/llvm-profdata/nocompress.test
tools/clang/test/Driver/nozlibcompress.c

[zturner]

For some context, the reason I added this in the first place is because there used to be a lot of inconsistency in the feature names, so I was trying to give consistency. For example, we would have not-<feature> in some places and <not>_feature in other places. You can even see an example of this here, where it's zlib and nozlib instead of zlib and not_zlib. This inconsistency makes it hard for people to guess the correct negation for the feature they're trying to specify.

Or, someone would define the feature but not the converse of it, so we'd get x but there would be no corresponding feature for not_x.

[pcc]

Hmm, I remembered that we can just do e.g. REQUIRES: !asan. So maybe we should make that change globally and only define the positive features here.

[zturner]

In D59160#1423435, @pcc wrote:

Hmm, I remembered that we can just do e.g. REQUIRES: !asan. So maybe we should make that change globally and only define the positive features here.

I wasn't aware that was even a thing. If you can confirm that works, then yes defining only the positive featuers would be an improvement. You might want to announce to the list for visibility though.

[hctim]

In D59160#1423439, @zturner wrote:

I wasn't aware that was even a thing. If you can confirm that works, then yes defining only the positive featuers would be an improvement. You might want to announce to the list for visibility though.

Me neither. By my quick grep, it looks like there are 13 places where we use a negation-based feature in REQUIRES (i.e. the requirement starts with no). I would be happy to do the cleanup next week if this is what we want.

llvm/utils/lit/lit/llvm/config.py

 import os
 import platform
 import re
 import subprocess
 import sys
 
 import lit.util
 from lit.llvm.subst import FindTool
 from lit.llvm.subst import ToolSubst
 
 
-def binary_feature(on, feature, off_prefix):
-    return feature if on else off_prefix + feature
-
-
 class LLVMConfig(object):
 
     def __init__(self, lit_config, config):
         self.lit_config = lit_config
         self.config = config
 
         features = config.available_features
 
def __init__(self, lit_config, config):
         host_triple = getattr(config, 'host_triple', None)
         target_triple = getattr(config, 'target_triple', None)
         if host_triple and host_triple == target_triple:
             features.add('native')
 
         # Sanitizers.
         sanitizers = getattr(config, 'llvm_use_sanitizer', '')
         sanitizers = frozenset(x.lower() for x in sanitizers.split(';'))
-        features.add(binary_feature('address' in sanitizers, 'asan', 'not_'))
-        features.add(binary_feature('memory' in sanitizers, 'msan', 'not_'))
-        features.add(binary_feature(
-            'undefined' in sanitizers, 'ubsan', 'not_'))
+        features.add('asan' if 'address' in sanitizers else 'not_asan')
+        features.add('msan' if 'memory' in sanitizers else 'not_msan')
+        features.add('ubsan' if 'undefined' in sanitizers else 'not_ubsan')
 
         have_zlib = getattr(config, 'have_zlib', None)
-        features.add(binary_feature(have_zlib, 'zlib', 'no'))
+        features.add('zlib' if have_zlib else 'nozlib')
 
         # Check if we should run long running tests.
         long_tests = lit_config.params.get('run_long_tests', None)
         if lit.util.pythonize_bool(long_tests):
             features.add('long_tests')
 
         if target_triple:
             if re.match(r'^x86_64.*-apple', target_triple):