krasimir added inline comments.
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2348
+// its not then rewind to the original position
+bool UnwrappedLineParser::tryToParseAttribute() {
+ unsigned StoredPosition = Tokens->getPosition();
----------------
MyDeveloperDay wrote:
> curdeius wrote:
> > MyDeveloperDay wrote:
> > > MyDeveloperDay wrote:
> > > > This so makes me feel like we need a peekNextToken()
> > > which is like all the time I have to write
> > >
> > > ```
> > > Tok->Next && Tok->Next->is(tok::XXX)
> > > Tok->Previous && Tok->Previous ->is(tok::XXX)
> > > ```
> > >
> > > would be so much smaller code if we had
> > >
> > > ```
> > > Tok->isNext(tok::XXX)
> > > Tok->isPrevious(tok::XXX)
> > > ```
> > `peekNextToken()` probably should be done in a separate revision, nope?
> > It would be good to have it IMO.
> yes I was just thinking out loud..
I think this should be more strict and check for the sequence of 5 tokens:
```
tok::l_square, tok::l_square, tok::identifier, tok::r_square, tok::r_square
```
Unfortunately `[[` may start a nested Objective-C-style method call, e.g.,
```
[[obj1 method1:arg1] method2:arg2]
```
(I'm not super familiar with Objective-C-syntax, please correct me if I'm wrong
about that.)
(Only checking for the last `]]`, or a combination of `[[` and `]]` that
doesn't examine //the meat in-between// would suffer from similar ambiguities.)
Of course, attributes can be more complicated and can have a rich internal
structure (looking at https://en.cppreference.com/w/cpp/language/attributes).
Consider renaming this to `tryToParseSimpleAttribute` to not give folks false
hopes that this deals with the general problem for now (we can rename this
later as we improve this C++ attribute parsing to handle more of the
interesting cases).
================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2351
+ FormatToken *Tok = Tokens->getNextToken();
+ FormatTok = Tokens->setPosition(StoredPosition);
+ if (Tok->is(tok::l_square)) {
----------------
curdeius wrote:
> Maybe add `assert(Tok);`. How can you be know for sure that there's a next
> token?
+1, but instead of `assert`-ing which may/will crash clang-format on weird
inputs and may lead to UB below in release builds, just `return false` and move
on.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80547/new/
https://reviews.llvm.org/D80547
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
