<div dir="ltr">(or remove the side-channel error handling, and have the APIs return errors explicitly instead - the side channel's likely to be accidentally unchecked by many callers, which is probably a mistake)</div><br><div class="gmail_quote"><div dir="ltr">On Tue, Aug 30, 2016 at 8:43 AM David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Sounds like the error handling is part of the identity of the object - I'm not sure it'd be a good idea to make that mutable & make the object logically const when it's actually changing that error field. (it seems it'd be easy to introduce misuse there - where one user does something to the Regex, then passes a const ref somewhere, then examines the error field and finds it changed - let alone multithreading)<br><br>Perhaps the right fix is for you to use MutableArrayRef instead?</div><br><div class="gmail_quote"><div dir="ltr">On Tue, Aug 30, 2016 at 6:52 AM George Rimar <<a href="mailto:grimar@accesssoftek.com" target="_blank">grimar@accesssoftek.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">grimar created this revision.<br>
grimar added reviewers: rafael, davide, dblaikie, ruiu.<br>
grimar added subscribers: llvm-commits, grimar.<br>
<br>
This can help to D23829.<br>
I have a std::vector of Regex and use ArrayRefs to pass it.<br>
The fact of match() is not const forces me to use const_cast for<br>
removing it. Also there is no way to call match() on<br>
Regex const reference, at the same time the only thing match()<br>
can change is error field and method name and job it do does not sounds<br>
like something not-const.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D24027" rel="noreferrer" target="_blank">https://reviews.llvm.org/D24027</a><br>
<br>
Files:<br>
  include/llvm/Support/Regex.h<br>
  lib/Support/Regex.cpp<br>
<br>
Index: lib/Support/Regex.cpp<br>
===================================================================<br>
--- lib/Support/Regex.cpp<br>
+++ lib/Support/Regex.cpp<br>
@@ -56,7 +56,7 @@<br>
   return preg->re_nsub;<br>
 }<br>
<br>
-bool Regex::match(StringRef String, SmallVectorImpl<StringRef> *Matches){<br>
+bool Regex::match(StringRef String, SmallVectorImpl<StringRef> *Matches) const {<br>
   unsigned nmatch = Matches ? preg->re_nsub+1 : 0;<br>
<br>
   // pmatch needs to have at least one element.<br>
Index: include/llvm/Support/Regex.h<br>
===================================================================<br>
--- include/llvm/Support/Regex.h<br>
+++ include/llvm/Support/Regex.h<br>
@@ -74,7 +74,8 @@<br>
     /// the first group is always the entire pattern.<br>
     ///<br>
     /// This returns true on a successful match.<br>
-    bool match(StringRef String, SmallVectorImpl<StringRef> *Matches = nullptr);<br>
+    bool match(StringRef String,<br>
+               SmallVectorImpl<StringRef> *Matches = nullptr) const;<br>
<br>
     /// sub - Return the result of replacing the first match of the regex in<br>
     /// \p String with the \p Repl string. Backreferences like "\0" in the<br>
@@ -98,7 +99,7 @@<br>
<br>
   private:<br>
     struct llvm_regex *preg;<br>
-    int error;<br>
+    mutable int error;<br>
   };<br>
 }<br>
<br>
<br>
<br>
</blockquote></div></blockquote></div>