Manna added inline comments.
================
Comment at: clang/lib/Format/TokenAnnotator.cpp:2009-2010
(Line.MightBeFunctionDecl || Line.InPPDirective) &&
- Current.NestingLevel == 0 &&
+ Current.NestingLevel == 0 && Current.Previous &&
!Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
// not auto operator->() -> xxx;
----------------
owenpan wrote:
> tahonermann wrote:
> > owenpan wrote:
> > > `Current.Previous` can't be null here because `AutoFound` is `true`.
> > Could you please elaborate on why you believe it is safe to move the check
> > of `Current.Previous` inside the body of the `if` statement? Doing so will
> > short circuit the remaining `else if` cases such that `Current.setType()`
> > will not be called at all. It isn't obvious to me that those cases should
> > not be considered if the previous token was not one of `kw_operator` or
> > `identifier`. This looks like it has potential to change behavior.
> >
> > The change that was originally proposed is clearly safe.
> > Could you please elaborate on why you believe it is safe to move the check
> > of `Current.Previous` inside the body of the `if` statement? Doing so will
> > short circuit the remaining `else if` cases such that `Current.setType()`
> > will not be called at all. It isn't obvious to me that those cases should
> > not be considered if the previous token was not one of `kw_operator` or
> > `identifier`. This looks like it has potential to change behavior.
>
> Ahh, you are right.
>
> > The change that was originally proposed is clearly safe.
>
> My point that `Previous` can't be null still stands. So we should either make
> no changes here or add an assertion just before the `if` statement at line
> 1991 above.
Thank you @owenpan and @tahonermann for reviews and feedbacks, I have removed
my previous change since Current.Previous can't be null here.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158293/new/
https://reviews.llvm.org/D158293
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
