================
Comment at: clang-tidy/readability/RemoveVoidArg.cpp:174
@@ +173,3 @@
+void RemoveVoidArg::processFieldDecl(const MatchFinder::MatchResult &Result,
FieldDecl const *const Member) {
+ std::string const Text = getText(Result, *Member);
+ removeVoidArgumentTokens(Result, Member->getLocStart(), Text, "Redundant
void argument list in field declaration.");
----------------
sbenza wrote:
> LegalizeAdulthood wrote:
> > sbenza wrote:
> > > LegalizeAdulthood wrote:
> > > > sbenza wrote:
> > > > > Any reason all of these convert the StringRef into a std::string?
> > > > > Couldn't they work with the StringRef directly?
> > > > I tried switching it to a StringRef, but when I tried to combine it
> > > > with other text to build a replacement for diag(), it got turned into
> > > > Twine and then failed to match any signature for diag(). I'm open to
> > > > suggestions for what you'd like to see instead.
> > > Twine::str() constructs the string.
> > > I don't mind constructing a string when a match happens. Those are rare.
> > > But we should avoid allocating memory when no match happens.
> > > Right now temporary memory allocations are a considerable part of
> > > clang-tidy's runtime. We should try to minimize that.
> > I think we are at that stage now; I do all early-out testing before
> > constructing the replacement string.
> You are still generating a copy of the code before re-parsing it.
> Those will happen for anything that passes the matcher, even if they have no
> 'void' argument.
> removeVoidArgumentTokens should take the code as a StringRef directly from
> getText(), without converting to std::string first. GrammarLocation should
> also be a StringRef.
> Basically, avoid std::string until you are about to call diag().
I'll look into having getText return the StringRef and using that. I don't
understand the fuss over GrammarLocation, however. Before I had it as a
constant string that was just passed through and then another review wanted the
duplication in the message eliminated, so I pulled that out. Now you're saying
that the string joining induced by the elimination of duplication is bad. It
doesn't seem possible to satisfy both comments.
http://reviews.llvm.org/D7639
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
