This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Allow to set token types final
ClosedPublic

Authored by HazardyKnusperkeks on Feb 24 2022, 12:33 PM.

Details

Summary

We have a little problem. TokenAnnotator::resetTokenMetadata() resets the type, except for a (growing) whitelist. This is because the TokenAnnotator visits some tokens multiple times. E.g. trying to identify if a < is an operator less or a template opener. And in some runs, which are bascially "reverted" the types are reset.

On the other hand, if the parser does already know the type, it should be able to set it, without it being reset. So we introduce the ability to set a type and make that final.

Diff Detail

Event Timeline

HazardyKnusperkeks requested review of this revision.Feb 24 2022, 12:33 PM
HazardyKnusperkeks created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 24 2022, 12:33 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
curdeius accepted this revision.Feb 24 2022, 1:17 PM
curdeius added a subscriber: krasimir.

Great!
@krasimir, FYI, that should fix the issue you reported.

clang/lib/Format/FormatToken.h
379

Maybe we should assert here and in setType that: assert(!TypeIsFinalized);?

This revision is now accepted and ready to land.Feb 24 2022, 1:17 PM
clang/lib/Format/FormatToken.h
379

In setType we could. Here we would have the same problem I fixed in D120512.

owenpan added inline comments.Feb 25 2022, 12:57 PM
clang/lib/Format/FormatToken.h
382

I thought you didn't like using the same name for both a variable and function. :)

HazardyKnusperkeks marked 2 inline comments as done.
  • Format
  • Add Assert
  • And some stuff to deal with the assert - Should be removed in the long run
  • Add some documentation
curdeius added inline comments.Feb 26 2022, 10:51 PM
clang/lib/Format/FormatToken.h
394

Nit, to make everyone happy, the function could be called isTypeFinalized.

owenpan added inline comments.Feb 26 2022, 11:28 PM
clang/lib/Format/FormatToken.h
394

+1.

HazardyKnusperkeks marked 2 inline comments as done.

Renamed member function.

clang/lib/Format/FormatToken.h
382

Naming a getter after the member you get should (nearly) always be done.

What I still stand to is having a local variable named after the function it is initialized from (this alone is fine), but then still calling the function in a loop. IsPrecededByCommentOrPPDirective is not the same as precededByCommentOrPPDirective() since the function relies on the changing token. I still find that very confusing, but for now I know that this code does not affect me while debugging, because I don't use the option.

owenpan added inline comments.Feb 27 2022, 2:06 AM
clang/lib/Format/FormatToken.h
382

Point taken.

owenpan accepted this revision.Feb 27 2022, 2:07 AM
This revision was landed with ongoing or failed builds.Mar 1 2022, 12:55 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2022, 12:55 PM