[Lldb-commits] [lldb] r255364 - Fix Clang-tidy modernize-use-nullptr and readability-simplify-boolean-expr warnings in source/Target/Target.cpp.
Zachary Turner via lldb-commits
lldb-commits at lists.llvm.org
Fri Dec 11 16:40:46 PST 2015
Hrmm, that's interesting. I guess I was mistaken in LLVM's rules then.
Are we willing to accept this format in LLDB?
LabelDecl(DeclContext *DC, SourceLocation IdentL, IdentifierInfo *II,
LabelStmt *S, SourceLocation StartL)
: NamedDecl(Label, DC, IdentL, II),
TheStmt(S),
MSAsmNameResolved(false),
LocStart(StartL) {}
If so that's less work I have to do on clang-format to get it up to spec.
On Fri, Dec 11, 2015 at 4:34 PM Jim Ingham <jingham at apple.com> wrote:
> On Dec 11, 2015, at 2:42 PM, Zachary Turner <zturner at google.com> wrote:
>
> Yes, but as I mentioned, two things are still unsupported due to
> limitations in clang-format. They are return-type-on-new-line (only in
> declarations. clang-format supports it for definitions) and the
> constructor initializer list comma at the end (clang-format puts it at the
> beginning).
>
> That said, the comma at the end of initializer list isn't documented on
> that page, and where we don't have a clearly documented rule, prefer the
> LLVM guidelines, so….
>
>
> That clang-format behavior seems weird to me, a quick scan through clang
> sources have the commas always at the end:
>
> LabelDecl(DeclContext *DC, SourceLocation IdentL, IdentifierInfo *II,
> LabelStmt *S, SourceLocation StartL)
> : NamedDecl(Label, DC, IdentL, II),
> TheStmt(S),
> MSAsmNameResolved(false),
> LocStart(StartL) {}
>
> etc. I can’t remember seeing code in clang that does:
>
> LabelDecl(DeclContext *DC, SourceLocation IdentL, IdentifierInfo *II,
> LabelStmt *S, SourceLocation StartL)
> : NamedDecl(Label, DC, IdentL, II)
> , TheStmt(S)
> , MSAsmNameResolved(false)
> , LocStart(StartL) {}
>
> That looks really weird, since you have to look past the noise of those
> commas to see what you really care about.
>
> We do differ in that we tend to write this:
>
> LabelDecl(DeclContext *DC, SourceLocation IdentL, IdentifierInfo *II,
> LabelStmt *S, SourceLocation StartL) :
> NamedDecl(Label, DC, IdentL, II),
> TheStmt(S),
> MSAsmNameResolved(false),
> LocStart(StartL) {}
>
> with the colon at the end of the argument list. I don’t think any of this
> behavior is prescribed one way or the other in the actual coding
> conventions… Maybe there’s a bug in clang-format that if you put the : in
> the unexpected place the commas get moved to the wrong place as well? But
> the llvm coding conventions make no prescription about this at all. I don’t
> think our code layout should be based on clang format bugs and all. We
> certainly shouldn’t do wholesale reformats that just make history harder to
> read for that reason.
>
> Jim
>
>
>
> On Fri, Dec 11, 2015 at 2:37 PM Todd Fiala <todd.fiala at gmail.com> wrote:
>
>> Okay, but does the format match the LLDB-modified format with some kind
>> of configuration file? We still need to match our guidelines here:
>>
>> http://lldb.llvm.org/lldb-coding-conventions.html
>>
>> We can achieve that with a config file for it, right? (Maybe already
>> existing, maybe in the lldb source tree already?)
>>
>> On Fri, Dec 11, 2015 at 2:35 PM, Zachary Turner <zturner at google.com>
>> wrote:
>>
>>> With git you can already run "git clang-format". You just need
>>> `git-clang-format` to be in your PATH (it's under llvm/tools/clang). Not
>>> sure how to hook it into SVN
>>>
>>> On Fri, Dec 11, 2015 at 2:32 PM Eugene Zelenko <eugene.zelenko at gmail.com>
>>> wrote:
>>>
>>>> At least clang-format should be applied to all newly added files before
>>>> commit.
>>>>
>>>> Eugene.
>>>>
>>>> On Fri, Dec 11, 2015 at 2:30 PM, Zachary Turner <zturner at google.com>
>>>> wrote:
>>>> > Back on the topic of clang-format, what would it take to make
>>>> clang-format a
>>>> > regular part of peoples' workflows?
>>>> >
>>>> > On Fri, Dec 11, 2015 at 2:27 PM Todd Fiala <todd.fiala at gmail.com>
>>>> wrote:
>>>> >>
>>>> >> Yep - sorry. I had been talking to Greg about this and
>>>> misunderstood his
>>>> >> comment on it. My mistake entirely. Kate and I just talked and she
>>>> pointed
>>>> >> me to your document, Jim.
>>>> >>
>>>> >> The description was:
>>>> >> where we had a clearly adhered to standard, keep it.
>>>> >> whee we didn't, we adopted LLVM.
>>>> >>
>>>> >> Sorry for rehashing!
>>>> >>
>>>> >> -Todd
>>>> >>
>>>> >> On Fri, Dec 11, 2015 at 2:12 PM, Jim Ingham <jingham at apple.com>
>>>> wrote:
>>>> >>>
>>>> >>>
>>>> >>> On Dec 11, 2015, at 2:01 PM, Todd Fiala via lldb-commits
>>>> >>> <lldb-commits at lists.llvm.org> wrote:
>>>> >>>
>>>> >>>
>>>> >>>
>>>> >>> On Fri, Dec 11, 2015 at 1:59 PM, Zachary Turner <zturner at google.com
>>>> >
>>>> >>> wrote:
>>>> >>>>
>>>> >>>> On Fri, Dec 11, 2015 at 1:55 PM Todd Fiala via lldb-commits
>>>> >>>> <lldb-commits at lists.llvm.org> wrote:
>>>> >>>>>
>>>> >>>>> Hey Eugene and Greg,
>>>> >>>>>
>>>> >>>>> I thought we were doing spaces before the open parens in places
>>>> like
>>>> >>>>> this:
>>>> >>>>>
>>>> >>>>> - BreakpointResolverSP resolver_sp(new
>>>> BreakpointResolverFileLine
>>>> >>>>> (NULL,
>>>> >>>>> ...
>>>> >>>>> + BreakpointResolverSP resolver_sp(new
>>>> >>>>> BreakpointResolverFileLine(nullptr,
>>>> >>>>>
>>>> >>>>> (see the removal of the space after BreakpointResolverFileLine
>>>> from the
>>>> >>>>> clang-tidy settings I presume).
>>>> >>>>>
>>>> >>>>> Did I misunderstand that?
>>>> >>>>
>>>> >>>>
>>>> >>>> This was officially removed from the coding standard some months
>>>> ago,
>>>> >>>
>>>> >>>
>>>> >>> Okay. Are we 100% in sync with LLVM coding standard guidelines?
>>>> If so I
>>>> >>> can just look there to see what we're supposed to be doing.
>>>> >>>
>>>> >>>
>>>> >>> No, the differences between the lldb and llvm coding standards are
>>>> >>> documented in:
>>>> >>>
>>>> >>> http://lldb.llvm.org/lldb-coding-conventions.html
>>>> >>>
>>>> >>> Jim
>>>> >>>
>>>> >>>
>>>> >>>>
>>>> >>>> but not everyone has adopted this unfortunately. See r228860. It
>>>> pains
>>>> >>>> me to no end that we differ from LLVM, because it leads to exactly
>>>> these
>>>> >>>> type of problems where people aren't sure what the exact set of
>>>> rules are.
>>>> >>>
>>>> >>>
>>>> >>>
>>>> >>>
>>>> >>> --
>>>> >>> -Todd
>>>> >>> _______________________________________________
>>>> >>> lldb-commits mailing list
>>>> >>> lldb-commits at lists.llvm.org
>>>> >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>>> >>>
>>>> >>>
>>>> >>
>>>> >>
>>>> >>
>>>> >> --
>>>> >> -Todd
>>>>
>>>
>>
>>
>> --
>> -Todd
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20151212/5580a498/attachment.html>
More information about the lldb-commits
mailing list