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