Details
- Reviewers
sammccall
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
| 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? | |
| 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 :-) | |
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); | |
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.