[PATCH] D39212: Check special-case-list regex before insertion.

[Vlad Tsyrklevich via Phabricator via llvm-commits]

vlad.tsyrklevich added a comment.

Please add a test.



================
Comment at: lib/Support/SpecialCaseList.cpp:40
   }
   Trigrams.insert(Regexp);
 
----------------
vsk wrote:
> CC'ing @vlad.tsyrklevich 
> 
> You might prevent more bugs by having TrigramIndex::insert check for empty strings and return an Error. Moving the check there should also make it easier to add a unit test (see TrigramIndexTest.cpp).
> 
In this case the exception is actually hit before we even get to the TrigramIndex--it's on insertion into Strings since the empty string is a literal. I'm not sure if TrigramIndex should try to check regexp validity. That seems like a complicated thing to do and TrigramIndex would only be able to look for the empty string. If this were actually to hit the regexp case we would have caught it with the isValid() check below.


https://reviews.llvm.org/D39212