[PATCH] D53025: [clang-tidy] implement new check for const return types.

Yitzhak Mandelbaum via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 12 15:00:49 PDT 2018


ymandel added inline comments.


================
Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:64
+
+  // Fix the definition.
+  llvm::Optional<SourceLocation> Loc = findConstToRemove(Def, Result);
----------------
JonasToth wrote:
> ymandel wrote:
> > JonasToth wrote:
> > > I feel that this comment doesn't add value. Could you please either make it more expressive or remove it?
> > Agreed. I merged the comment below into this one, so one comment describes the rest of the control flow in this block.
> Did I tell you the "only one diagnostic in-flight" thing? :D
> I told you wrong stuff as you already figured out in the code. Please adjust this comment and the additional scope is not necessary too.
But, I think you are *correct* in this assertion.  When I tried multiple in-flight diagnostics, it failed with error:

clang-tidy: /usr/local/google/home/yitzhakm/Projects/llvm/llvm/tools/clang/include/clang/Basic/Diagnostic.h:1297: clang::DiagnosticBuilder clang::DiagnosticsEngine::Report(clang::SourceLocation, unsigned int): Assertion `CurDiagID == std::numeric_limits<unsigned>::max() && "Multiple diagnostics in flight at once!"' failed.

Am I doing something wrong?




Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53025





More information about the cfe-commits mailing list