I've started to do some cleanup of the include header order (r278222).
It doesn't get everything compiling after a clang-format, but it got
me about half way.

+1 on the include order proposed by Zach. I agree that better
modularization would improve testability and have a positive impact on
the size of lldb-server (e.g., right now we have to link it against
libclang, even though it shouldn't need any of that functionality).

pl

On 9 August 2016 at 23:01, Zachary Turner via lldb-dev
<lldb-dev@lists.llvm.org> wrote:
> Standardizing include order would really help.  LLVM's style is documented
> here, but to quote it:
>
> Main Module Header
> Local/Private Headers
> llvm/...
> System #includes
>
> So perhaps it would be reasonable for us to standardize on something like
> this:
>
> Main Module Header
> Local/Private Headers
> lldb/...
> llvm/...
> System #includes
>
> Putting LLDB headers before LLVM headers is a nice way to enforce "include
> what you use".  Otherwise you might have something like this:
>
> // lldb.cpp
> #include "llvm/LLVM1.h"
> #include "lldb/lldb2.h
>
> // lldb2.h
> llvm::MyType foo();
>
> And this will work, even though lldb2.h needs to have #include
> "llvm/LLVM1.h"
>
> So the above ordering fixes that.  clang-format won't solve this for us
> automatically, but it would be nice to at least move towards it.
>
> On Tue, Aug 9, 2016 at 2:49 PM Kate Stone <k8st...@apple.com> wrote:
>>
>> Great catch!  If refactoring along those lines doesn’t clean up 100% of
>> the cases then it’s worth explicitly breaking up groups of #include
>> directives with comments.  clang-format won’t reorder any non-contiguous
>> groups and it’s a great way to explicitly call out dependencies.
>>
>> Ideally we should be focused on committing changes along these lines so
>> that there’s no post-format tweaking required to build cleanly again.
>>
>> Kate Stone k8st...@apple.com
>>  Xcode Low Level Tools
>>
>> On Aug 9, 2016, at 1:03 PM, Zachary Turner <ztur...@google.com> wrote:
>>
>> I ran clang-format and tried to build and got a bunch of compiler errors.
>> Most of them are order of include errors.  I fixed everything in the
>> attached patch.  I doubt this will apply cleanly for anyone unless you are
>> at the exact same revision as me, but at least you can look at it and get an
>> idea of what had to change.
>>
>> The #include win32.h thing is really annoying and hard for people to
>> remember the right incantation.  I'm going to make a file called
>> Host/PosixApi.h which you can just include, no matter what platform you're
>> on, and everything will just work.  That should clean up a lot of this
>> nonsense.
>>
>> On Tue, Aug 9, 2016 at 11:10 AM Zachary Turner <ztur...@google.com> wrote:
>>>
>>> Another thing worth thinking about for the long term is library layering
>>> and breaking the massive dependency cycle in LLDB.  Every library currently
>>> depends on every other library.  This isn't good for build times, code size,
>>> or reusability (especially where size matters like in lldb-server).  I think
>>> the massive Python dependency was removed after my work earlier this year.
>>> But I'm not sure what the status of that is now, and there's still the rest
>>> of LLDB.
>>>
>>> In the future it would be nice to have a modules build of LLDB.  And
>>> sure, we could just have liblldb be one giant module, but that seems to kind
>>> of defeat the purpose of modules in the first place.
>>>
>>> For unit tests in particular, it's nice to be able to link in just the
>>> set of things you need, and that's difficult / impossible right now.
>>>
>>> On Tue, Aug 9, 2016 at 10:00 AM Zachary Turner <ztur...@google.com>
>>> wrote:
>>>>
>>>> On Mon, Aug 8, 2016 at 2:40 PM Kate Stone via lldb-dev
>>>> <lldb-dev@lists.llvm.org> wrote:
>>>>>
>>>>>
>>>>> Near-Term Goal: Standardizing on LLVM-style clang-format Rules
>>>>>
>>>>> We’ve heard from several in the community that would prefer to have a
>>>>> single code formatting style to further unify the two code bases.  Using
>>>>> clang-format with the default LLVM conventions would simplify code
>>>>> migration, editor configuration, and coding habits for developers who work
>>>>> in multiple LLVM projects.  There are non-trivial implications to
>>>>> reformatting a code base with this much history.  It can obfuscate history
>>>>> and impact downstream projects by complicating merges.  Ideally, it should
>>>>> be done once with as much advance notice as is practical.  Here’s the
>>>>> timeline we’re proposing:
>>>>>
>>>>> Today - mechanical reformatting proposed, comment period begins
>>>>>
>>>>> To get a preview of what straightforward reformatting of the code looks
>>>>> like, just follow these steps to get a clean copy of the repository and
>>>>> reformat it:
>>>>>
>>>>> Check out a clean copy of the existing repository
>>>>> Edit .clang-format in the root of the tree, remove all but the line
>>>>> “BasedOnStyle: LLVM”
>>>>> Change your current working directory to the root of the tree to
>>>>> reformat
>>>>> Double-check to make sure you did step 3 ;-)
>>>>> Run the following shell command: "find . -name "*.[c,cpp,h] -exec
>>>>> clang-format -i {} +"
>>>>>
>>>>>
>>>>> Aug 20th - comment period closes, final schedule proposed
>>>>> TBD (early September?) - patches land in svn
>>>>>
>>>>> The purpose of the comment period is to review the straightforward
>>>>> diffs to identify areas where comment pragmas should be used to avoid
>>>>> undesirable formatting (tables laid out in code are a classic example.)
>>>>> It’s also a time when feedback on the final timetable can be discussed, 
>>>>> and
>>>>> any unforeseen implications can be discovered.  We understand that LLDB
>>>>> tends toward relatively long names that may not always work well with the
>>>>> LLVM convention of wrapping at 80 columns.  Worst case scenarios will be
>>>>> evaluated to determine the desired course of action.
>>>>
>>>>
>>>> One thing we will need to take a look at is functions which have a very
>>>> deep indentation level. They have the potential to be made really ugly by
>>>> clang-format.  The default indentation will be reduced from 4 to 2, so that
>>>> will help, but I recall we had some lines that began very far to the right.
>>>>
>>>> Here's a little bash command shows all lines with >= 50 leading spaces,
>>>> sorted in descending order by number of leading spaces.
>>>>
>>>> grep -n '^ \+' . -r -o | awk '{t=length($0);sub("
>>>> *$","");printf("%s%d\n", $0, t-length($0));}' |  sort -t: -n -k 3 -r | awk
>>>> 'BEGIN { FS = ":" } ; { if ($3 >= 50) print $0 }'
>>>>
>>>> It's less useful than I was hoping because most of the lines are noise
>>>> (line breaking in a function parameter list).
>>>>
>>>> If there were a way to detect indentation level that would be better.
>>>> Mostly just to identify places that we should manually inspect after 
>>>> running
>>>> clang-format.
>>
>> <reformat.patch>
>>
>>
>
> _______________________________________________
> lldb-dev mailing list
> lldb-dev@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
_______________________________________________
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Reply via email to