dexonsmith added a comment.

Thanks for the review!



================
Comment at: clang/lib/Basic/SourceManager.cpp:167
 
     Buffer.setInt(Buffer.getInt() | InvalidFlag);
+    return None;
----------------
JDevlieghere wrote:
> Orthogonal to this patch but an idea for a follow-up: this seems lightly 
> error prone and repetitive. We could put it in an `llvm::scope_exit` and then 
> release it before the final return on line `200`.
Good call; I refactored this a bit in https://reviews.llvm.org/D89431; after 
that lands I'll do what you suggest, or possibly start with `IsBufferInvalid = 
true` and then set it back to `false` at the end of the function (once we know 
it's good).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89430/new/

https://reviews.llvm.org/D89430

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D89430: cl... Duncan P. N. Exon Smith via Phabricator via cfe-commits
    • [PATCH] D8943... Jonas Devlieghere via Phabricator via cfe-commits
    • [PATCH] D8943... Duncan P. N. Exon Smith via Phabricator via cfe-commits
    • [PATCH] D8943... Duncan P. N. Exon Smith via Phabricator via cfe-commits
    • [PATCH] D8943... Jonas Devlieghere via Phabricator via cfe-commits
    • [PATCH] D8943... Duncan P. N. Exon Smith via Phabricator via cfe-commits

Reply via email to