I agree that's better long term, but this code was already there, and the patch makes things strictly better than before (literally just fixes a crash in existing code). So I think we can agree that this should be fixed, but as it's a bigger change I don't think it should hold up fixing a crash.
On Mon, Dec 18, 2017 at 9:44 AM Greg Clayton via Phabricator < revi...@reviews.llvm.org> wrote: > clayborg added inline comments. > > > ================ > Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:393 > // try really hard not to use a regex match. > - bool is_regex = false; > - if (name_str.find_first_of("[]?*.-+\\") != std::string::npos) { > - // Trying to compile an invalid regex could throw an exception. > - // Only search by regex when it's valid. > - lldb_private::RegularExpression name_regex(name_str); > - is_regex = name_regex.IsValid(); > - } > - if (is_regex) > - FindTypesByRegex(name_str, max_matches, types); > + if (name_str.find_first_of("[]?*.-+\\") != std::string::npos) > + FindTypesByRegex(RegularExpression(name_str), max_matches, types); > ---------------- > I would rather not sniff the string passed in to see if this could be a > regex and add a new FindTypesByRegex() call to lldb_private::SymbolFile > that anyone can use. Do we have PDB tests that are using this functionality? > > > Repository: > rL LLVM > > https://reviews.llvm.org/D41086 > > > >
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits