[PATCH] D24027: [LLVM/Support] - Make match() method of llvm::Regex to be const

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 30 08:43:03 PDT 2016


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)

Perhaps the right fix is for you to use MutableArrayRef instead?

On Tue, Aug 30, 2016 at 6:52 AM George Rimar <grimar at accesssoftek.com>
wrote:

> grimar created this revision.
> grimar added reviewers: rafael, davide, dblaikie, ruiu.
> grimar added subscribers: llvm-commits, grimar.
>
> This can help to D23829.
> I have a std::vector of Regex and use ArrayRefs to pass it.
> The fact of match() is not const forces me to use const_cast for
> removing it. Also there is no way to call match() on
> Regex const reference, at the same time the only thing match()
> can change is error field and method name and job it do does not sounds
> like something not-const.
>
>
> https://reviews.llvm.org/D24027
>
> Files:
>   include/llvm/Support/Regex.h
>   lib/Support/Regex.cpp
>
> Index: lib/Support/Regex.cpp
> ===================================================================
> --- lib/Support/Regex.cpp
> +++ lib/Support/Regex.cpp
> @@ -56,7 +56,7 @@
>    return preg->re_nsub;
>  }
>
> -bool Regex::match(StringRef String, SmallVectorImpl<StringRef> *Matches){
> +bool Regex::match(StringRef String, SmallVectorImpl<StringRef> *Matches)
> const {
>    unsigned nmatch = Matches ? preg->re_nsub+1 : 0;
>
>    // pmatch needs to have at least one element.
> Index: include/llvm/Support/Regex.h
> ===================================================================
> --- include/llvm/Support/Regex.h
> +++ include/llvm/Support/Regex.h
> @@ -74,7 +74,8 @@
>      /// the first group is always the entire pattern.
>      ///
>      /// This returns true on a successful match.
> -    bool match(StringRef String, SmallVectorImpl<StringRef> *Matches =
> nullptr);
> +    bool match(StringRef String,
> +               SmallVectorImpl<StringRef> *Matches = nullptr) const;
>
>      /// sub - Return the result of replacing the first match of the regex
> in
>      /// \p String with the \p Repl string. Backreferences like "\0" in the
> @@ -98,7 +99,7 @@
>
>    private:
>      struct llvm_regex *preg;
> -    int error;
> +    mutable int error;
>    };
>  }
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160830/2064a2c6/attachment.html>


More information about the llvm-commits mailing list