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

[Douglas Gregor]

On Dec 18, 2012, at 11:45 AM, Manuel Klimek <klimek at google.com> wrote:

> On Tue, Dec 18, 2012 at 8:36 PM, Douglas Gregor <dgregor at apple.com> wrote:
> 
> On Dec 18, 2012, at 11:31 AM, Manuel Klimek <klimek at google.com> wrote:
> 
>> On Tue, Dec 18, 2012 at 7:55 PM, Douglas Gregor <dgregor at apple.com> wrote:
>> [Re-adding cfe-commits]
>> 
>> On Dec 18, 2012, at 10:16 AM, Daniel Jasper <djasper at google.com> wrote:
>> 
>>> 
>>> 
>>> 
>>> On Tue, Dec 18, 2012 at 6:41 PM, Douglas Gregor <dgregor at apple.com> wrote:
>>> 
>>> On Dec 18, 2012, at 1:09 AM, Daniel Jasper <djasper at google.com> wrote:
>>> 
>>>> 
>>>> 
>>>> 
>>>> On Mon, Dec 17, 2012 at 11:04 PM, Douglas Gregor <dgregor at apple.com> wrote:
>>>> 
>>>> On Dec 17, 2012, at 9:55 AM, Daniel Jasper <djasper at google.com> wrote:
>>>> 
>>>>> Hi Fariborz,
>>>>> 
>>>>> I am taking a look at your patch, but please bear with me for another day or two. I am not yet convinced that mingling C++ and ObjC formatting that closely is a wise decision. I think we might need something that clearly separates the different modes. And you touch a lot of code that I intended to refactor into something halfway sane this week. I will take your patch into consideration when doing so, but it might need some changes afterwards.
>>>> 
>>>> If we're going to distinguish the modes, we need to do so syntactically. Objective-C++ is a very popular dialect; we can't simply rely on extrinsic knowledge of whether we're dealing with Objective-C or C++ to make these decisions.
>>>> 
>>>> I am not sure I fully understand this. We are currently relying on a Lexer which in turn requires LangOptions to create the correct tokens. Same for the identifier table we use to split raw_identifiers into identifiers and keywords. Are you saying the formatter should not capitalize on this knowledge of the input token stream?
>>> 
>>> Oh, it should certainly capitalize on this knowledge. My concern is that, when you mentioned that we might need something that "clearly separates the different modes", you were thinking of Objective-C and C++ as independent languages that aren't used together. Fortunately, we can tell syntactically whether we're in an Objective-C method declaration fairly easily: they start with + or -, almost always in the first column, so if it would be better to go down a different code-formatting path where the rules for spacing around (/)/: are different, we could do that instead.
>>> 
>>> 	- Doug
>>> 
>>> Yeah, "clearly separate" was probably stronger phrased than intended. One of the things I wonder is the following:
>>> If we are not in Objective-C according to the LangOptions and we discover a + or - at the beginning of a "line", should we assume an error has happened (e.g. forgotten "}" or ";") or try to format as an Objective-C declaration?
>> 
>> I think we should assume that an error has occurred. If clients don't know the language dialect, they should pick something maximally inclusive (e.g., Objective-C++11). 
>> 
>> For files, I assume it's easy enough to detect by the file extension?
> 
> And .h means what, exactly?
> 
> I assume with that comment you're telling me it's used for Objective-C, too? :) Reading up the wikipedia page suggests so … 

Yes, unfortunately.

> How different is Objective-C usually layed out? Will we need a configuration to tell us which files in a project are which language, or will we get away with auto-detection? I assume auto-detection is hard as there will be keywords that have special layout rules in both languages?

As Fariborz noted, it's actually not that hard to auto-detect that the code in question in Objective-C, and I think it's reasonable to try to do so.

In the narrow case we're applying the formatter within libclang, we actually know definitively what the language dialect is, but that's not generally going to be true. If auto-detection can work, we should do it.

	- Doug

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