[PATCH] clang-format properly handles try/catch blocks

Manuel Klimek klimek at google.com
Wed Jan 22 05:56:24 PST 2014



================
Comment at: lib/Format/UnwrappedLineParser.cpp:1042
@@ +1041,3 @@
+  bool NeedsUnwrappedLine = false;
+  if (FormatTok->Tok.is(tok::colon)) {
+      // We are in a function try block, what comes is an initializer list
----------------
Alexander Rojas wrote:
> Manuel Klimek wrote:
> > I'm wondering whether we just want to eat everything up to the next '{'. In general, we try to parse as little as possible in the structural parser.
> > 
> > If we decide we really need to parse the structure here, I think we should pull out a parseInitializerList() method.
> I've been thinking on how to parse the initializer list, and it might be quite cumbersome. I've been thinking on leaving out of this patch the ability of dealing with function-try blocks and leave it for latter (since I do this as a hobby, there's really a limit on how much time I can spend on this).
I think we're misunderstanding each other here... What I mean is that we should parse *less* accurately. The thing to remember is that clang-format must work well on incorrect code. Thus, the structural parser tries hard to find only the structural elements that matter.

Regarding the time limits you have: especially when you do this as a hobby, I'd say this is the perfect venue to try to get things "right". As nobody forces you to do it, there's also no deadline pressure, so you can take all the time you need... The worst that can happen is that somebody asks you whether they can take over and finish the patch if they need it more urgently.

================
Comment at: lib/Format/UnwrappedLineParser.cpp:1043
@@ +1042,3 @@
+  if (FormatTok->Tok.is(tok::colon)) {
+      // We are in a function try block, what comes is an initializer list
+      nextToken();
----------------
Alexander Rojas wrote:
> Manuel Klimek wrote:
> > Nit: add '.'.
> Sorry, but what do you mean by “Nit”?
A minor comment that's not really important :)

================
Comment at: lib/Format/UnwrappedLineParser.cpp:1076
@@ +1075,3 @@
+  while (FormatTok->Tok.is(tok::kw_catch)) {
+    nextToken();
+    if (FormatTok->Tok.is(tok::l_paren))
----------------
Alexander Rojas wrote:
> Manuel Klimek wrote:
> > Same here. The problem is if anybody ever finds a real use case to put anything in between (including macros) we might fail to parse the file...
> I don't get what you mean here. If the token is not tok::kw_catch, it won't start the while and NeedsUnwrappedLine will be true. Is there any other case that needs consideration?
Well, what do we do on this snippet:
  ...
  } catch (Type) ANNOTATION_MACRO(something else) {
     ...
  }
As I said, I'm not sure exactly how to resolve this.

Daniel, do you have ideas here?


http://llvm-reviews.chandlerc.com/D2555



More information about the cfe-commits mailing list