[cfe-dev] [patch] Add configure option for C include search path

[Daniel Dunbar]

Hi Rafael,

> diff --git a/include/llvm/ADT/StringRef.h b/include/llvm/ADT/StringRef.h
> index 12e2c56..9410bc8 100644
> --- a/include/llvm/ADT/StringRef.h
> +++ b/include/llvm/ADT/StringRef.h
> @@ -15,6 +15,14 @@
>  #include <cstring>
>  #include <string>
>
> +namespace std {
> + template<typename _Tp>
> + class allocator;
> +
> + template<typename _Tp, typename _Alloc>
> + class vector;
> +}
> +
>  namespace llvm {
>
>    /// StringRef - Represent a constant reference to a string, i.e. a character
> @@ -306,6 +314,10 @@ namespace llvm {
>        return std::make_pair(slice(0, Idx), slice(Idx + Separator.size(), npos));
>      }
>
> +    void split(std::vector<StringRef, std::allocator<StringRef> > &A,
> +               StringRef Separators, unsigned MaxSplit = -1,
> +               bool KeepEmpty = false) const;

Needs a doxyment.

Please make KeepEmpty = true the default. An excellent invariant is
that on return:
 Separators.join(A) == *this
with MaxSplit == -1 and KeepEmpty == true. This is worth mentioning in
the doxyment.

Please document the semantics of splitting an empty string, I would
argue to follow the Python semantics were it results in an empty list.

> diff --git a/lib/Support/StringExtras.cpp b/lib/Support/StringExtras.cpp
> index c72f121..d904b2a 100644
> --- a/lib/Support/StringExtras.cpp
> +++ b/lib/Support/StringExtras.cpp
> @@ -56,3 +56,25 @@ void llvm::SplitString(const std::string &Source,
>      S2 = getToken(S, Delimiters);
>    }
>  }
> +
> +void llvm::StringRef::split(std::vector<StringRef> &A,
> +                            StringRef Separators, unsigned MaxSplit,
> +                            bool KeepEmpty) const {
> +  StringRef rest = *this;
> +  for (unsigned splits = 0;; ++splits) {
> +    if (MaxSplit >= 0 && splits >= MaxSplit)
> +      break;
> +
> +    std::pair<llvm::StringRef, llvm::StringRef> p = rest.split(Separators);
> +
> +    if (p.first.size() == 0 && rest.size() == 0)
> +      break;
> +
> +    if (p.first.size() != 0 || KeepEmpty)
> +      A.push_back(p.first);
> +    rest = p.second;
> +  }
> +
> +  if (rest.size() != 0 || KeepEmpty)
> +    A.push_back(rest);
> +}

Assuming you agree with empty string -> empty list, this is simpler as:
--
void llvm::StringRef::split(std::vector<StringRef> &A,
                            StringRef Separators, unsigned MaxSplit,
                            bool KeepEmpty) const {
  StringRef Str = *this;

  while (!Str.empty() && A.size() != MaxSplit) {
    std::pair<llvm::StringRef, llvm::StringRef> split = Str.split(Separators);

    if (KeepEmptyLines || !split.first.empty())
      A.push_back(split.first);

    Str = split.second;
  }
}
--


On Thu, Nov 12, 2009 at 1:15 PM, Rafael Espindola <espindola at google.com> wrote:
>> I prefer having a MaxSplit argument, which is easier to support, than
>> making SplitAll.
>
> OK. The attached patch includes the API you proposed and some tests
> for it.The only difference is that the MaxSplit default is -1 to match
> python. Please check that semantics is what you expect. In particular,
> check the KeepEmpty = false, MaxSplit != -1 cases.
>
>>  - Daniel
>>
>
> Cheers,
> --
> Rafael Ávila de Espíndola
>