danielmarjamaki added inline comments.
> MisleadingIndentationCheck.cpp:20
> +
> +void MisleadingIndentationCheck::danglingElseCheck(
> + const MatchFinder::MatchResult &Result) {
There is no handling of tabs and spaces by danglingElseCheck as far as I see.
The "if" might for example be indented with spaces. And then the corresponding
"else" is indented with a tab. Then I guess there is false positive.
If the "if" is indented with 2 spaces and the "else" is indented with 2 tabs.
then I assume there is false negative.
It's unfortunate. Is it ok?
> MisleadingIndentationCheck.cpp:34
> + Result.SourceManager->getExpansionColumnNumber(ElseLoc))
> + diag(ElseLoc, "potential dangling else");
> +}
I see no test case that says "potential dangling else". If I'm not mistaken
that should be added.
Is it really sufficient to write that? It's not obvious to me why clang-tidy
would think it's dangling.
> MisleadingIndentationCheck.cpp:44
> +
> + if (isa<IfStmt>(CurrentStmt)) {
> + IfStmt *CurrentIf = dyn_cast<IfStmt>(CurrentStmt);
You don't need to use both isa and dyn_cast:
if (const auto *CurrentIf = dyn_cast<IfStmt>(CurrentStmt)) {
Inside = CurrentIf->getElse() ? CurrentIf->getElse() :
CurrentIf->getThen();
} ....
> MisleadingIndentationCheck.h:20
> +/// Checks the code for dangling else,
> +/// and possible misleading indentations due to missing braces.
> +///
there are too many spaces in this comment
https://reviews.llvm.org/D19586
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
