This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Add ability to trace tokens.
Needs ReviewPublic

Authored by klimek on Mar 3 2023, 6:59 AM.

Details

Reviewers
sammccall

Diff Detail

Event Timeline

klimek created this revision.Mar 3 2023, 6:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2023, 6:59 AM
klimek requested review of this revision.Mar 3 2023, 6:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 3 2023, 6:59 AM
klimek updated this revision to Diff 502123.Mar 3 2023, 7:04 AM

Remove superfluous include.

sammccall added inline comments.Mar 6 2023, 1:58 AM
clang/lib/Format/ContinuationIndenter.cpp
289

annotating all exit paths from this function and mustBreak seems much more verbose and fragile than wrapping the function (making this version private) and adding the annotations in the wrapper.

clang/lib/Format/FormatDebug.h
22

it looks like you're not providing a dummy definition in NDEBUG mode - does this still build in that config?

28

Comment with an example?
(In particular, unclear what "Debug" means)

32

why empty else?

clang/lib/Format/TokenAnnotator.cpp
3258

move this out of the if() and remove the one on the other branch?

3258

this meaning of << in (...) << "MustBreakBefore is confusing.

consider << (...) << "MustBreakBefore" or dbgs() << (...) << "MustBreakBefore" or a twine or formatv or really anything that isn't a shift :-)

klimek added a comment.Mar 6 2023, 2:50 AM

Answering the fundamental design question first.

clang/lib/Format/ContinuationIndenter.cpp
289

How do we get the exact condition that triggered? The main trick here is the FILE:LINE in the debug output.

clang/lib/Format/FormatDebug.h
22

All use is within LLVM_DEBUG().

32

The idea was dangling else protection, but I guess we have dangling else warnings, so this is not necessary?

clang/lib/Format/TokenAnnotator.cpp
3258

The idea was to see which branch of the if is taken here.

Oops, sorry for the noise.

I'd forgotten that we're really interested in *why* we reach the conclusion, not the conclusion itself - and that file/line are our proxy for "why" as writing/maintaining descriptions is a burden.

clang/lib/Format/ContinuationIndenter.cpp
289

Doh. There *is* an a way in recent clang/gcc/msvc, though I suspect you don't want to go down this path...

template <typename T>
struct LogReturn {
  T Value;
  #ifdef NDEBUG
  LogReturn(T Value) : Value(Value) {}
  #else
  const char *File;
  int Line;
  // default args are evaluated at callsite
  LogReturn(T Value, const char *File=__builtin_FILE(), int Line=__builtin_LINE()) : Value(Value), File(File), Line(Line) {
    dbgs() << ...;
  }
  #endif
}

LogReturn<bool> priv() { return true; }
bool pub() { return priv().Value; }

Dunno how to get the token without resorting to TLS though. More realistically you could define trace(bool, Token&, const char*File=..., etc) and return trace(true, Tok);