This is an archive of the discontinued LLVM Phabricator instance.

Showing off the small emails
ClosedPublic

Authored by klimek on Aug 2 2012, 11:33 AM.

Details

Reviewers
klimek
chandlerc
Summary

Feel free to comment - in the comment emails, make sure you select everything you want included in the context. Try selecting "old" and "new" code

Diff Detail

Event Timeline

chandlerc added inline comments.Aug 2 2012, 11:38 AM
include/clang/ASTMatchers/ASTMatchers.h
78–83

Hmmm

131–134

curious what this will do

134–137

Can't seem to select the old side and the new side...

include/clang/ASTMatchers/ASTMatchersInternal.h
600–602

An edit!

klimek added inline comments.Aug 12 2012, 9:34 AM
include/clang/ASTMatchers/ASTMatchersInternal.h
600–602

Let's see whether you like that better...

klimek added inline comments.Aug 12 2012, 9:35 AM
include/clang/ASTMatchers/ASTMatchers.h
131–134

Me too.

chandlerc added inline comments.Aug 12 2012, 1:47 PM
include/clang/ASTMatchers/ASTMatchers.h
77–79

More added, but commented on old...

135–144

Some added stuff

include/clang/ASTMatchers/ASTMatchersInternal.h
744–748

Bad bad deleted stuff!

Lets see what a preface comment looks like in conjunction with an inline comment.

include/clang/ASTMatchers/ASTMatchersInternal.h
744–756

And if I select all of the deleted, I should get the added too? Even on the old side?

Lets see what a preface comment looks like in conjunction with an inline comment.

include/clang/ASTMatchers/ASTMatchersInternal.h
600–602

Replies are interesting!

744–756

What does it look like when replying inline?

Suggested ASCII art form:

http://pastie.org/4462496

Lets see what a preface comment looks like in conjunction with an inline
comment.

Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:744-756

@@ -743,15 +761,6 @@
BindableMatcher<T> makeDynCastAllOfComposite(

ArrayRef<const Matcher<InnerT> *> InnerMatchers) {
  • if (InnerMatchers.empty()) {
  • Matcher<InnerT> InnerMatcher = makeMatcher(new TrueMatcher<InnerT>);
  • return BindableMatcher<T>(new DynCastMatcher<T, InnerT>(InnerMatcher));
  • }
  • Matcher<InnerT> InnerMatcher = *InnerMatchers.back();
  • for (int i = InnerMatchers.size() - 2; i >= 0; --i) {
  • InnerMatcher = makeMatcher(
  • new AllOfMatcher<InnerT, Matcher<InnerT>, Matcher<InnerT> >(
  • *InnerMatchers[i], InnerMatcher));
  • }
  • return BindableMatcher<T>(new DynCastMatcher<T, InnerT>(InnerMatcher));

+ return BindableMatcher<T>(new DynCastMatcher<T, InnerT>(
+ makeAllOfComposite(InnerMatchers)));

}

What does it look like when replying inline?

Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:600-602

@@ -599,5 +599,5 @@
class BindableMatcher : public Matcher<T> {
public:

  • BindableMatcher(MatcherInterface<T> *Implementation)

+ explicit BindableMatcher(MatcherInterface<T> *Implementation)

: Matcher<T>(Implementation) {}

--------------------------------------------------------------------------

Replies are interesting!

klimek added inline comments.Aug 13 2012, 12:52 AM
include/clang/ASTMatchers/ASTMatchersInternal.h
744–752

Better?

And top

include/clang/ASTMatchers/ASTMatchersInternal.h
604–737

Multiple...

895–898

Comments...

ASCII art looks awesome to me...

klimek added inline comments.Aug 14 2012, 1:46 PM
include/clang/ASTMatchers/ASTMatchers.h
131–134

Replying to myself :P

include/clang/ASTMatchers/ASTMatchersInternal.h
600–602

And now hopefully work.

I bow before you sir.

klimek abandoned this revision.Aug 22 2012, 11:00 AM
klimek added inline comments.Oct 10 2012, 1:30 AM
include/clang/ASTMatchers/ASTMatchersInternal.h
607

Testing email stuff.

klimek reclaimed this revision.Oct 10 2012, 1:31 AM

hrm

doesn't work yet... :(

Yet another comment to test the Phab-mailing.

klimek accepted this revision.Oct 12 2012, 4:09 AM
klimek closed this revision.