[PATCH] D73993: [analyzer] Fix a couple of bugs in HTML report generation.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked an inline comment as done.
NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:612
+  return !(Range.getBegin().isMacroID() || Range.getEnd().isMacroID());
+}
+
----------------
Charusso wrote:
> Side note: I like the other form of De Morgan's laws because here I have to apply it in my head every time I see such a construct. Also we are using this function in negation, so I would write:
> ```lang=c
> static bool isMacro(const SourceRange &Range) {
>   return Range.getBegin().isMacroID() || Range.getEnd().isMacroID();
> }
> ```
> 
> The idiom is to write code for readability so that understandability over everything else.
Objection :) I want this function to figure out if a pop-up range should be displayed, not whether the range is in a macro. Like, if we come up with other excuses for not displaying a range, we'll update this function, not make a new one. And if we need to check whether something is a macro in another place, we'll make a new function, not re-use this one.

I could do something like `shouldSkipPopUpRange()` but that'd move the confusing negation into the function name, making call sites harder to read.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73993