hwright marked an inline comment as done.
hwright added inline comments.
================
Comment at: clang-tidy/abseil/TimeSubtractionCheck.cpp:97
+void TimeSubtractionCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("binop");
+ std::string inverse_name =
----------------
hwright wrote:
> JonasToth wrote:
> > hwright wrote:
> > > JonasToth wrote:
> > > > Could you please split this function up into smaller ones. There are
> > > > three or four distinct cases that are easier to comprehend in isolation.
> > > The actual bodies of these if-statements are only one or two separate
> > > statements themselves. Moving those to separate functions seems like it
> > > would just obfuscate things a bit.
> > IMHO they are complicated statements and hide what is being done. Wrapping
> > them in a function with a name that states what is done seems appropriate.
> I would agree that they are complicated statements, which is why there are
> multi-line comments explaining what is being doing. Moving a two-line
> compound statement into a separate function which is only called once seems
> more confusing than simplifying. More information can be expressed in a
> prose comment than a single concise function name, no?
I've pulled out the common duplicated functionality which actually emits the
diagnostic, and I think that's simplified these branches a bit more.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58137/new/
https://reviews.llvm.org/D58137
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
