This revision was automatically updated to reflect the committed changes.
Closed by commit rG182b4652e542: [StringRef] Add enable-if to StringLiteral.
(authored by zturner).
Herald added subscribers: llvm-commits, dexonsmith.
Herald added a project: LLVM.
Changed prior to commit:
https://review
zturner added a comment.
Yea as I mentioned this whole plan might be killed by a blocker I ran into last
night. I'm still trying to figure out if there's a workaround.
https://reviews.llvm.org/D27780
___
lldb-commits mailing list
lldb-commits@list
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Ok, as long as the StringRef constructors are quick and efficient and not
running strlen() each time I am good.
https://reviews.llvm.org/D27780
So I ran into a nasty problem doing the rest of the conversions and at the
moment I'm not sure if there's even a workaround. So this is on hold while
I think about it.
On Thu, Dec 15, 2016 at 8:06 PM Zachary Turner via Phabricator <
revi...@reviews.llvm.org> wrote:
> zturner added a comment.
>
>
zturner added a comment.
Greg, did the comments about implicit construction of a `StringRef` from a char
literal being zero overhead make sense? If so, are there any other concerns?
https://reviews.llvm.org/D27780
___
lldb-commits mailing list
lld
zturner removed rL LLVM as the repository for this revision.
zturner updated this revision to Diff 81625.
zturner added a comment.
Re-upload the correct diff.
https://reviews.llvm.org/D27780
Files:
lldb/include/lldb/Interpreter/Options.h
lldb/include/lldb/lldb-private-types.h
lldb/source/
zturner reopened this revision.
zturner added a comment.
My bad, this revision was not actually closed, I attached the wrong diff to an
unrelated commit. I will need to re-upload this one.
Repository:
rL LLVM
https://reviews.llvm.org/D27780
___
This revision was automatically updated to reflect the committed changes.
Closed by commit rL289853: [StringRef] Add enable-if to StringLiteral.
(authored by zturner).
Changed prior to commit:
https://reviews.llvm.org/D27780?vs=81487&id=81624#toc
Repository:
rL LLVM
https://reviews.llvm.org
labath added a comment.
Looks reasonable. I look forward to using StringRef in more places.
https://reviews.llvm.org/D27780
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
zturner added inline comments.
Comment at: lldb/source/Interpreter/Options.cpp:728
+for (auto &def : range) {
+ std::string full_name("--");
+ full_name.append(def.long_option);
clayborg wrote:
> Do we still need std::string here for ful
zturner added a comment.
In response to all the questions about "Will a StringRef constructor be called
on XXX", the answer is usually yes, but the constructor will be inlined and
invoke `__builtin_strlen` (which is constexpr on GCC and Clang) when used on a
string literal. In other words, the
clayborg added a comment.
There seems to be a bunch of places that we are passing a string literal to
functions that take a StringRef. Can we pass L("--") instead of "--" to
functions that take a string ref and have it be more efficient so a StringRef
isn't implicitly constructed for us each ti
zturner created this revision.
zturner added reviewers: clayborg, labath.
zturner added a subscriber: lldb-commits.
The major blocker to this until now has been that we can't have global
constructors, no matter how trivial.
Recently LLVM obtained the `StringLiteral` class which addresses this.
13 matches
Mail list logo
