dexonsmith added a comment.
Thanks for working on this! I have a couple of comments inline.
================
Comment at: clang/include/clang/Basic/Diagnostic.h:918-927
/// The ID of the current diagnostic that is in flight.
///
/// This is set to std::numeric_limits<unsigned>::max() when there is no
/// diagnostic in flight.
unsigned CurDiagID;
+ /// The ID of the current delayed diagnostic being reported.
----------------
`CurDelayedDiagID` has a different kind of "in-flight" lifetime than
`CurDiagID`. I think the comment here should be more explicit about what
precisely the variable is doing, and perhaps it should have another name. Do
we actually need the ID kept as state? If not, this would be simpler as:
```
bool IsReportingDelayedDiag = false;
```
Also, a couple of nit-picks:
- Periods at the end of sentences in comments.
- You seem to have dropped out of doxygen comments (`//` instead of `///`).
================
Comment at: clang/lib/Basic/Diagnostic.cpp:160-161
void DiagnosticsEngine::ReportDelayed() {
- unsigned ID = DelayedDiagID;
+ CurDelayedDiagID = DelayedDiagID;
DelayedDiagID = 0;
+ Report(CurDelayedDiagID) << DelayedDiagArg1 << DelayedDiagArg2
----------------
If you take my suggestion, then you wouldn't need to change this line, just add:
```
IsReportingDelayedDiag = true;
```
but would these assertions be valid after your change?
```
assert(DelayedDiagID && "Called without a delayed diagnostic?");
assert(!IsReportingDelayedDiag &&
"Called while reporting another delayed diagnostic?");
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74795/new/
https://reviews.llvm.org/D74795
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
