Use the fallible llvm::Regex constructor in Clang.

[dlj]

Summary

This change updates Clang to check for an llvm::Expected<llvm::Regex>, instead
of constructing an invalid Regex and checking .isValid().

Since most of the uses already (silently) ignore invalid llvm::Regex objects,
I've not tried too hard to propagate construction-time errors.

[vsk]

lib/Driver/Multilib.cpp

220–221

I have the same comments about this patch as I did re: D27465, but I thought I'd chime in here with an extra warning.

I don't think this will compile. The errs() object doesn't take ownership of the Error or check it. Better to write: llvm::toString(R.takeError()).

[dlj]

• Added comments about dropped assertion checks, and ensured checks in other places.
• (Probably?) Fixed a unit test. The intent isn't clear, it's an un-handleable erroneous input case.

[dlj]

Ping?

include/clang/ASTMatchers/ASTMatchers.h

AST_POLYMORPHIC_MATCHER_P(isExpansionInFileMatching,
   }
   auto FileEntry =
       SourceManager.getFileEntryForID(SourceManager.getFileID(ExpansionLoc));
   if (!FileEntry) {
     return false;
   }
 
   auto Filename = FileEntry->getName();
-  llvm::Regex RE(RegExp);
-  return RE.match(Filename);
+  auto RE = llvm::Regex::compile(RegExp);
+  // Note that this does not handle any errors from regex compilation, so
+  // it will crash on broken patterns when asserts are enabled.
+  return RE && RE->match(Filename);
 }
 
 /// \brief Matches declarations.
 ///
 /// Examples matches \c X, \c C, and the friend declaration inside \c C;
 /// \code
 ///   void X();
 ///   class C {

 ///
 /// Example matches X (regexp is one of "::X", "^foo::.*X", among others)
 /// \code
 ///   namespace foo { namespace bar { class X; } }
 /// \endcode
 AST_MATCHER_P(NamedDecl, matchesName, std::string, RegExp) {
   assert(!RegExp.empty());
   std::string FullNameString = "::" + Node.getQualifiedNameAsString();
-  llvm::Regex RE(RegExp);
-  return RE.match(FullNameString);
+  auto RE = llvm::Regex::compile(RegExp);
+  // Note that this does not handle any errors from regex compilation, so
+  // it will crash on broken patterns when asserts are enabled.
+  return RE && RE->match(FullNameString);
 }
 
 /// \brief Matches overloaded operator names.
 ///
 /// Matches overloaded operator names specified in strings without the
 /// "operator" prefix: e.g. "<<".
 ///
 /// Given:

 ///  matches the outer message expr in the code below, but NOT the message
 ///  invocation for self.bodyView.
 /// \code
 ///     [self.bodyView loadHTMLString:html baseURL:NULL];
 /// \endcode
 AST_MATCHER_P(ObjCMessageExpr, matchesSelector, std::string, RegExp) {
   assert(!RegExp.empty());
   std::string SelectorString = Node.getSelector().getAsString();
-  llvm::Regex RE(RegExp);
-  return RE.match(SelectorString);
+  auto RE = llvm::Regex::compile(RegExp);
+  // Note that this does not handle any errors from regex compilation, so
+  // it will crash on broken patterns when asserts are enabled.
+  return RE && RE->match(SelectorString);
 }
 
 /// \brief Matches when the selector is the empty selector
 ///
 /// Matches only when the selector of the objCMessageExpr is NULL. This may
 /// represent an error condition in the tree!
 AST_MATCHER(ObjCMessageExpr, hasNullSelector) {
   return Node.getSelector().isNull();

include/clang/Frontend/VerifyDiagnosticConsumer.h

public:
     const std::string Text;
     unsigned Min, Max;
     bool MatchAnyLine;
 
     virtual ~Directive() { }
 
     // Returns true if directive text is valid.
     // Otherwise returns false and populates E.
+    //
+    // This should only be called once: after calling, the object's
+    // internal representation for isValid may be in an indeterminate
+    // state, where it is only safe to destruct (although the Directive
+    // object itself must be safe to use if isValid returns true).
     virtual bool isValid(std::string &Error) = 0;
 
     // Returns true on match.
     virtual bool match(StringRef S) = 0;
 
   protected:
     Directive(SourceLocation DirectiveLoc, SourceLocation DiagnosticLoc,
               bool MatchAnyLine, StringRef Text, unsigned Min, unsigned Max)

lib/Driver/Multilib.cpp

 }
 
 MultilibSet &MultilibSet::FilterOut(FilterCallback F) {
   filterInPlace(F, Multilibs);
   return *this;
 }
 
 MultilibSet &MultilibSet::FilterOut(const char *Regex) {
-  llvm::Regex R(Regex);
+  auto R = llvm::Regex::compile(Regex);
+  if (!R) {
 #ifndef NDEBUG
-  std::string Error;
-  if (!R.isValid(Error)) {
-    llvm::errs() << Error;
+    llvm::errs() << toString(R.takeError());
+#endif
     llvm_unreachable("Invalid regex!");
   }
-#endif
 
-  filterInPlace([&R](const Multilib &M) { return R.match(M.gccSuffix()); },
+  filterInPlace([&R](const Multilib &M) { return R->match(M.gccSuffix()); },
                 Multilibs);
   return *this;
 }
 
 void MultilibSet::push_back(const Multilib &M) { Multilibs.push_back(M); }
 
 void MultilibSet::combineWith(const MultilibSet &Other) {
   Multilibs.insert(Multilibs.end(), Other.begin(), Other.end());

lib/Format/ContinuationIndenter.cpp

 ContinuationIndenter::ContinuationIndenter(const FormatStyle &Style,
                                            const AdditionalKeywords &Keywords,
                                            const SourceManager &SourceMgr,
                                            WhitespaceManager &Whitespaces,
                                            encoding::Encoding Encoding,
                                            bool BinPackInconclusiveFunctions)
     : Style(Style), Keywords(Keywords), SourceMgr(SourceMgr),
       Whitespaces(Whitespaces), Encoding(Encoding),
-      BinPackInconclusiveFunctions(BinPackInconclusiveFunctions),
-      CommentPragmasRegex(Style.CommentPragmas) {}
+      BinPackInconclusiveFunctions(BinPackInconclusiveFunctions) {
+  if (auto RegexOrError = llvm::Regex::compile(Style.CommentPragmas)) {
+    CommentPragmasRegex = std::move(*RegexOrError);
+  }
+}
 
 LineState ContinuationIndenter::getInitialState(unsigned FirstIndent,
                                                 const AnnotatedLine *Line,
                                                 bool DryRun) {
   LineState State;
   State.FirstIndent = FirstIndent;
   State.Column = FirstIndent;
   State.Line = Line;

lib/Format/Format.cpp

 
 // This class manages priorities of #include categories and calculates
 // priorities for headers.
 class IncludeCategoryManager {
 public:
   IncludeCategoryManager(const FormatStyle &Style, StringRef FileName)
       : Style(Style), FileName(FileName) {
     FileStem = llvm::sys::path::stem(FileName);
-    for (const auto &Category : Style.IncludeCategories)
-      CategoryRegexs.emplace_back(Category.Regex);
+    for (const auto &Category : Style.IncludeCategories) {
+      if (auto RegexOrError = llvm::Regex::compile(Category.Regex)) {
+        CategoryRegexs.emplace_back(std::move(*RegexOrError));
+      }
+    }
     IsMainFile = FileName.endswith(".c") || FileName.endswith(".cc") ||
                  FileName.endswith(".cpp") || FileName.endswith(".c++") ||
                  FileName.endswith(".cxx") || FileName.endswith(".m") ||
                  FileName.endswith(".mm");
   }
 
   // Returns the priority of the category which \p IncludeName belongs to.
   // If \p CheckMainHeader is true and \p IncludeName is a main header, returns

 
 private:
   bool isMainHeader(StringRef IncludeName) const {
     if (!IncludeName.startswith("\""))
       return false;
     StringRef HeaderStem =
         llvm::sys::path::stem(IncludeName.drop_front(1).drop_back(1));
     if (FileStem.startswith(HeaderStem)) {
-      llvm::Regex MainIncludeRegex(
-          (HeaderStem + Style.IncludeIsMainRegex).str());
-      if (MainIncludeRegex.match(FileStem))
+      auto MainIncludeRegex =
+          llvm::Regex::compile((HeaderStem + Style.IncludeIsMainRegex).str());
+      // Note that this does not handle the error case. In a build with
+      // asserts enabled, an invalid pattern will cause a crash.
+      if (MainIncludeRegex && MainIncludeRegex->match(FileStem))
         return true;
     }
     return false;
   }
 
   const FormatStyle &Style;
   bool IsMainFile;
   StringRef FileName;
   StringRef FileStem;
   SmallVector<llvm::Regex, 4> CategoryRegexs;
 };
 
 const char IncludeRegexPattern[] =
     R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
 
 } // anonymous namespace
 
 tooling::Replacements sortCppIncludes(const FormatStyle &Style, StringRef Code,
                                       ArrayRef<tooling::Range> Ranges,
                                       StringRef FileName,
                                       tooling::Replacements &Replaces,
                                       unsigned *Cursor) {
   unsigned Prev = 0;
   unsigned SearchFrom = 0;
-  llvm::Regex IncludeRegex(IncludeRegexPattern);
+  llvm::Regex IncludeRegex;
+  // Note that this does not handle the error case. In a build with asserts
+  // enabled, an invalid IncludeRegexPattern will cause a crash.
+  if (auto RegexOrError = llvm::Regex::compile(IncludeRegexPattern))
+    IncludeRegex = std::move(*RegexOrError);
   SmallVector<StringRef, 4> Matches;
   SmallVector<IncludeDirective, 16> IncludesInBlock;
 
   // In compiled files, consider the first #include to be the main #include of
   // the file if it is not a system #include. This ensures that the header
   // doesn't have hidden dependencies
   // (http://llvm.org/docs/CodingStandards.html#include-style).
   //
auto Reformat = [](const FormatStyle &Style, StringRef Code,
     return reformat(Style, Code, Ranges, FileName);
   };
   return processReplacements(Reformat, Code, *SortedReplaces, Style);
 }
 
 namespace {
 
 inline bool isHeaderInsertion(const tooling::Replacement &Replace) {
+  auto RegexOrError = llvm::Regex::compile(IncludeRegexPattern);
+  // Note that this does not handle the error case. In a build with asserts
+  // enabled, an invalid IncludeRegexPattern will cause a crash.
+  if (!RegexOrError) return false;
   return Replace.getOffset() == UINT_MAX && Replace.getLength() == 0 &&
-         llvm::Regex(IncludeRegexPattern).match(Replace.getReplacementText());
+      RegexOrError->match(Replace.getReplacementText());
 }
 
 inline bool isHeaderDeletion(const tooling::Replacement &Replace) {
   return Replace.getOffset() == UINT_MAX && Replace.getLength() == 1;
 }
 
 // Returns the offset after skipping a sequence of tokens, matched by \p
 // GetOffsetAfterSequence, from the start of the code.
if (isHeaderInsertion(R)) {
                    << R.getReplacementText() << "\n";
     } else {
       llvm::consumeError(Result.add(R));
     }
   }
   if (HeaderInsertions.empty() && HeadersToDelete.empty())
     return Replaces;
 
-  llvm::Regex IncludeRegex(IncludeRegexPattern);
-  llvm::Regex DefineRegex(R"(^[\t\ ]*#[\t\ ]*define[\t\ ]*[^\\]*$)");
+  llvm::Regex IncludeRegex;
+  // Note that this does not handle the error case. In a build with asserts
+  // enabled, an invalid IncludeRegexPattern will cause a crash.
+  if (auto RegexOrError = llvm::Regex::compile(IncludeRegexPattern))
+    IncludeRegex = std::move(*RegexOrError);
   SmallVector<StringRef, 4> Matches;
 
   StringRef FileName = Replaces.begin()->getFilePath();
   IncludeCategoryManager Categories(Style, FileName);
 
   // Record the offset of the end of the last include in each category.
   std::map<int, int> CategoryEndOffsets;
   // All possible priorities.

lib/Format/FormatTokenLexer.cpp

 
 FormatTokenLexer::FormatTokenLexer(const SourceManager &SourceMgr, FileID ID,
                                    const FormatStyle &Style,
                                    encoding::Encoding Encoding)
     : FormatTok(nullptr), IsFirstToken(true), StateStack({LexerState::NORMAL}),
       Column(0), TrailingWhitespace(0), SourceMgr(SourceMgr), ID(ID),
       Style(Style), IdentTable(getFormattingLangOpts(Style)),
       Keywords(IdentTable), Encoding(Encoding), FirstInLineIndex(0),
-      FormattingDisabled(false), MacroBlockBeginRegex(Style.MacroBlockBegin),
-      MacroBlockEndRegex(Style.MacroBlockEnd) {
+      FormattingDisabled(false) {
+  if (auto RegexOrError = llvm::Regex::compile(Style.MacroBlockBegin)) {
+    MacroBlockBeginRegex = std::move(*RegexOrError);
+  }
+  if (auto RegexOrError = llvm::Regex::compile(Style.MacroBlockEnd)) {
+    MacroBlockEndRegex = std::move(*RegexOrError);
+  }
   Lex.reset(new Lexer(ID, SourceMgr.getBuffer(ID), SourceMgr,
                       getFormattingLangOpts(Style)));
   Lex->SetKeepWhitespaceMode(true);
 
   for (const std::string &ForEachMacro : Style.ForEachMacros)
     ForEachMacros.push_back(&IdentTable.get(ForEachMacro));
   std::sort(ForEachMacros.begin(), ForEachMacros.end());
 }

lib/Frontend/CompilerInvocation.cpp

 
 /// \brief Create a new Regex instance out of the string value in \p RpassArg.
 /// It returns a pointer to the newly generated Regex instance.
 static std::shared_ptr<llvm::Regex>
 GenerateOptimizationRemarkRegex(DiagnosticsEngine &Diags, ArgList &Args,
                                 Arg *RpassArg) {
   StringRef Val = RpassArg->getValue();
   std::string RegexError;
-  std::shared_ptr<llvm::Regex> Pattern = std::make_shared<llvm::Regex>(Val);
-  if (!Pattern->isValid(RegexError)) {
+  auto RegexOrError = llvm::Regex::compile(Val);
+  if (!RegexOrError) {
     Diags.Report(diag::err_drv_optimization_remark_pattern)
-        << RegexError << RpassArg->getAsString(Args);
-    Pattern.reset();
+        << toString(RegexOrError.takeError()) << RpassArg->getAsString(Args);
+    return nullptr;
   }
-  return Pattern;
+  return std::make_shared<llvm::Regex>(std::move(*RegexOrError));
 }
 
 static bool parseDiagnosticLevelMask(StringRef FlagName,
                                      const std::vector<std::string> &Levels,
                                      DiagnosticsEngine *Diags,
                                      DiagnosticLevelMask &M) {
   bool Success = true;
   for (const auto &Level : Levels) {

lib/Frontend/VerifyDiagnosticConsumer.cpp

 /// RegexDirective - Directive with regular-expression matching.
 ///
 class RegexDirective : public Directive {
 public:
   RegexDirective(SourceLocation DirectiveLoc, SourceLocation DiagnosticLoc,
                  bool MatchAnyLine, StringRef Text, unsigned Min, unsigned Max,
                  StringRef RegexStr)
     : Directive(DirectiveLoc, DiagnosticLoc, MatchAnyLine, Text, Min, Max),
-      Regex(RegexStr) { }
+      Err(llvm::Error::success()) {
+    if (auto RegexOrError = llvm::Regex::compile(RegexStr)) {
+      Regex = std::move(*RegexOrError);
+    } else {
+      Err = RegexOrError.takeError();
+    }
+  }
 
   bool isValid(std::string &Error) override {
-    return Regex.isValid(Error);
+    if (!Err)
+      return true;
+    Error = toString(std::move(Err));
+    return false;
   }
 
   bool match(StringRef S) override {
     return Regex.match(S);
   }
 
 private:
   llvm::Regex Regex;
+  llvm::Error Err;
 };
 
 class ParseHelper
 {
 public:
   ParseHelper(StringRef S)
     : Begin(S.begin()), End(S.end()), C(Begin), P(Begin), PEnd(nullptr) {}
 

tools/driver/driver.cpp

if (Edit[0] == '^') {
     StringRef MatchPattern = Edit.substr(2).split('/').first;
     StringRef ReplPattern = Edit.substr(2).split('/').second;
     ReplPattern = ReplPattern.slice(0, ReplPattern.size()-1);
 
     for (unsigned i = 1, e = Args.size(); i != e; ++i) {
       // Ignore end-of-line response file markers
       if (Args[i] == nullptr)
         continue;
-      std::string Repl = llvm::Regex(MatchPattern).sub(ReplPattern, Args[i]);
+      auto RE = llvm::Regex::compile(MatchPattern);
+      std::string Repl = RE ? RE->sub(ReplPattern, Args[i]) : Args[i];
 
       if (Repl != Args[i]) {
         OS << "### Replacing '" << Args[i] << "' with '" << Repl << "'\n";
         Args[i] = GetStableCStr(SavedStrings, Repl);
       }
     }
   } else if (Edit[0] == 'x' || Edit[0] == 'X') {
     auto Option = Edit.substr(1);

unittests/ASTMatchers/ASTMatchersNodeTest.cpp

EXPECT_TRUE(matchesObjC(
     Objc1String,
     objcMessageExpr(anything())));
   EXPECT_TRUE(matchesObjC(
     Objc1String,
     objcMessageExpr(hasSelector("contents"))));
   EXPECT_TRUE(matchesObjC(
     Objc1String,
     objcMessageExpr(matchesSelector("cont*"))));
-  EXPECT_FALSE(matchesObjC(
+  EXPECT_TRUE(matchesObjC(
     Objc1String,
-    objcMessageExpr(matchesSelector("?cont*"))));
+    objcMessageExpr(matchesSelector("-?cont*"))));
   EXPECT_TRUE(notMatchesObjC(
     Objc1String,
     objcMessageExpr(hasSelector("contents"), hasNullSelector())));
   EXPECT_TRUE(matchesObjC(
     Objc1String,
     objcMessageExpr(hasSelector("contents"), hasUnarySelector())));
   EXPECT_TRUE(matchesObjC(
     Objc1String,