[cfe-commits] clang formatter patch to move and rename RewriterTestContext.h

[jahanian]

On Dec 17, 2012, at 9:44 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:

> On Mon, Dec 17, 2012 at 7:13 PM, jahanian <fjahanian at apple.com> wrote:
>> Are you going to provide the new public header in clang/Tooling this week? I
>> have gone ahead and made the necessary changes for formatting ObjC
>> declarations and
>> and provide the tests in the attached patch which enforces our convention
>> for formatting them.  Here is my complete patch (including the plumbing for
>> declaration tags which you may ignore).  How do you want to proceed?
> 
> 
> +      std::string StringDecl = Declaration.str();
> +      StringDecl += ";";
> 
> Adding this semicolon should be the job of the pretty-printer.  For
> example, "namespace Foo {}" does not need a semicolon.  (Yes, I know
> it in objc-only now, but it is the declprinter that needs to be fixed,
> not this.)

Semicolon is needed by the current formatter. I did not want to change the current look
of the DeclPrinter for this purpose. If formatter can be changed to not require a semicolon,
this will not be needed.

> 
> -void CommentASTToXMLConverter::appendToResultWithXMLEscaping(StringRef S) {
> +void CommentASTToXMLConverter::appendToResultWithXMLEscaping(StringRef S,
> +                                                             bool SkipSemi) {
>   for (StringRef::iterator I = S.begin(), E = S.end(); I != E; ++I) {
>     const char C = *I;
>     switch (C) {
> @@ -1307,6 +1353,9 @@
>       case '\'':
>         Result << "'";
>         break;
> +      case ';':
> +        if (SkipSemi)
> +          break;
> 
> As per above, adding a hack and then reversing is not a good approach,
> thus "SkipSemi" does not belong here.
> 

I need to remove the semicolon because formatter, rightly, leaves it there. 

> +  RewriterTestContext &TContext;
> 
> A better variable name would be "FormatRewriterContext".
> 
> +void *cxcursor::createFormatContext() {
> +  return new RewriterTestContext();
> +}
> 
> +void cxcursor::disposeFormatContext(void *formatContext) {
> +  delete static_cast<RewriterTestContext*>(formatContext);
> +}
> 
> Do we need these functions?  Why can not its only user
> 
> --- tools/libclang/CIndex.cpp	(revision 170208)
> +++ tools/libclang/CIndex.cpp	(working copy)
> @@ -61,6 +61,8 @@
>   D->StringPool = createCXStringPool();
>   D->Diagnostics = 0;
>   D->OverridenCursorsPool = createOverridenCXCursorsPool();
> +  D->FormatContext = createFormatContext();
> 
> just call new/delete directly?

I am following the convention for similar use cases. But, it is trivial to do.

- Thanks, Fariborz

> 
> Dmitri
> 
> -- 
> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/