Thanks!  Will fix<br><div class="gmail_quote"><div dir="ltr">On Mon, Sep 18, 2017 at 10:20 PM David L. Jones via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">dlj added inline comments.<br>
<br>
<br>
================<br>
Comment at: llvm/test/<a href="http://lit.cfg.py:153" rel="noreferrer" target="_blank">lit.cfg.py:153</a><br>
+    ToolFilter('llvm-symbolizer', pre=JUNKCHARS),<br>
+    ToolFilter('opt', JUNKCHARS),<br>
+    ToolFilter('sancov', pre=JUNKCHARS),<br>
----------------<br>
pre=JUNKCHARS for consistency?<br>
<br>
<br>
================<br>
Comment at: llvm/utils/lit/lit/llvm/__init__.py:27<br>
+<br>
+            literal: If True, `name` is an exact regex that is passed to the<br>
+            underlying substitution<br>
----------------<br>
(Minor nit) I really hate to bikeshed ... but I would expect this to do something like check for 'name' literally, without any regex matching. So I won't suggest changing the name to "is_regex" ;-) , but are there any other names that might make it more clear that the first argument will be interpreted as a well-formed regex?<br>
<br>
<br>
================<br>
Comment at: llvm/utils/lit/lit/llvm/__init__.py:30<br>
+        """<br>
+        def any_char_in(chars):<br>
+            def escape_one(c):<br>
----------------<br>
I think re.escape will do what you need:<br>
<a href="https://docs.python.org/2/library/re.html#re.escape" rel="noreferrer" target="_blank">https://docs.python.org/2/library/re.html#re.escape</a><br>
<br>
<br>
================<br>
Comment at: llvm/utils/lit/lit/llvm/__init__.py:44<br>
+            pre = any_char_in(pre)<br>
+            regex = '(?<!(%s))' % pre<br>
+            name = regex + name<br>
----------------<br>
It might make more sense to factor out the regex construction, then you can assign self.regex all at once:<br>
<br>
<br>
```<br>
def not_in(patterns, where=''):<br>
  if not patterns:<br>
    return ''<br>
  pattern_str = '|'.join(re.escape(x) for x in patterns)<br>
  return r'(?{where}!({pattern_str}))'.format(<br>
      where=where, pattern_str=pattern_str)<br>
<br>
self.regex = not_in(pre, where='<') + '\b' + name + '\b' + not_in(post)<br>
```<br>
<br>
(Not sure if that's actually clearer, but there might be some useful tidbits...)<br>
<br>
<br>
================<br>
Comment at: llvm/utils/lit/lit/llvm/config.py:174<br>
+        # out its resource dir here in an easy to scrape form.<br>
+        dir, _ = self.get_process_output([clang, '-print-file-name=include'], encoding='ascii')<br>
+<br>
----------------<br>
Nit: 'dir' is a Python builtin, this shadows it.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D37944" rel="noreferrer" target="_blank">https://reviews.llvm.org/D37944</a><br>
<br>
<br>
<br>
</blockquote></div>