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

jahanian fjahanian at apple.com
Mon Dec 17 09:47:06 PST 2012


On Dec 17, 2012, at 9:36 AM, Manuel Klimek <klimek at google.com> wrote:

> I don't think it's the right design to put something that is generally thought for testing into a public header and use it from libclang.
> If we want something similar to RewriterTestContext, it should:
> a) live next to the Rewriter in clang - as it provides everything needed to rewrite files
> b) leave out all functionality that's not needed outside of tests
> c) be proposed and reviewed in an extra patch, which I'd assume would need approval from Doug - it's a non-trivial addition to the public api

I agree with all these. I wanted to move forward with ObjC formatting and it was a temporary refactoring allowing me to move forward. Form and placement
of the public header rests with Daniel. Also my patch includes complete formatting of ObjC declarations which I hope will be useful.

- Fariborz

> 
> Now, I'm all for having some nicer "just bundle me something up" APIs so a lot of Clang's infrastructure like the Rewriter gets simpler to use, but I'd rather see it introduced in a deliberate way.
> 
> Thanks!
> /Manuel
> 
> 
> 
> On Mon, Dec 17, 2012 at 6:13 PM, jahanian <fjahanian at apple.com> wrote:
> Hi Daniel,
> 
> 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?
> 
> - Thanks, Fariborz
> 
> 	
> 
> 
> On Dec 16, 2012, at 10:21 PM, Daniel Jasper <djasper at google.com> wrote:
> 
>> I think the RewriterTestContext should remain in Tooling as it has nothing to do with formatting. I guess, we could make it a public header there (clang/Tooling). And it should remain being called RewriterTestContext.
>> 
>> The ObjC change looks fine as a temporary fix. Especially the part in Format.cpp will undergo a rewrite anyway, hopefully this week. I will look into what else needs to change to support ObjC. At the very least, we probably shouldn't set CPlusPlus LangOpts for the Lexer..
>> 
>> 
>> 
>> On Wed, Dec 12, 2012 at 8:50 PM, Sean Silva <silvas at purdue.edu> wrote:
>> On Wed, Dec 12, 2012 at 1:00 PM, jahanian <fjahanian at apple.com> wrote:
>> > ter. This patch is an attempt to have a header in Format directory
>> > that others can use. Any suggestion is welcome.
>> 
>> +djasper
>> 
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121217/64ebaa7e/attachment.html>


More information about the cfe-commits mailing list