[Lldb-commits] [PATCH] D41086: [lldb] Check that a regex is valid before searching by regex for a symbol in a pdb.

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.


================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:394
+  if (name_str.find_first_of("[]?*.-+\\") != std::string::npos)
+    FindTypesByRegex(RegularExpression(name_str), max_matches, types);
   else
----------------
clayborg wrote:
> You should make the RegularExpression object first and then check for errors. If there are errors, the type lookup should happen by normal name. This is again why I don't like us sniffing for a regular expression. If there is an error, like when trying to lookup "char*" as mentioned by Aaron, would you expect to see an error message saying "char*" isn't a valid regex? Then the user thinks that the type lookup always takes a regular expression which is not the case, just something the PDB plugin added for some reason. I do believe part of this patch should be adding the lldb_private::SymbolFile::FindTypesByRegex(...) to the base class and fixing this issue by removing the regex sniffing code since it is fragile at best (how are we doing to lookup a template type without triggering the regex issues?).
The reason I don't think it's appropriate for this patch is because nothing currently calls that method.  So something much higher level would then have to be updated to call this new method, which might even entail adding a new command line option.  For now, just fixing a crash is sufficient.


Repository:
  rL LLVM

https://reviews.llvm.org/D41086