[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

kastiglione added a comment. @Tyker in case you didn't see my previous message, I'm curious if you'd be willing to take a look at the bug. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/ https://reviews.llvm.org/D70638

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

kastiglione added a comment. @Tyker Hi, I noticed a case that isn't caught by `-Wmisleading-indentation`. In code that uses two space indents, there's a corner case that isn't caught when the preceding `if` uses curly braces. I've noticed a couple instances of this in lldb. For example: if

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

Tyker added a comment. In D70638#1804138 , @aaron.ballman wrote: > In D70638#1803364 , @xbolva00 wrote: > > > (re-ping; I think this false positive for goto label case is important to > > be fixed before 10

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

aaron.ballman added a comment. In D70638#1803364 , @xbolva00 wrote: > (re-ping; I think this false positive for goto label case is important to be > fixed before 10 release) I agree -- @Tyker, are you planning to work on that fix? Repository: rG

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

xbolva00 added a comment. (re-ping; I think this false positive for goto label case is important to be fixed before 10 release) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/ https://reviews.llvm.org/D70638

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

nathanchance added a comment. As a follow up to my previous post, I have sent patches to clean up all of the warnings that I see in the Linux kernel. However, I found one that I do believe is a false positive: ../drivers/staging/uwb/allocator.c:353:3: warning: misleading indentation;

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

Tyker added a comment. i have not tested the performance impact. but i tried to do the heavier checks last to minimize the impact. i added your tests file to the improvement commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

thakis added a comment. Also, did you check if this has a measurable impact on compilation speed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/ https://reviews.llvm.org/D70638 ___

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

thakis added a comment. Hey, cool! I gave this a shot myself many years ago at http://llvm.org/bugs/show_bug.cgi?id=18938 That bug has a list of interesting test cases I had collected back then. Maybe you want to check and see how this does on those cases? Repository: rG LLVM Github

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

nathanchance added a comment. > IMO that is misleading indentation. A single space followed by a tab? WTF I guess I will take a look over all of the warnings that cropped up and see if this is the case for all of them. If it is, I will just send patches to fix those (with the justification of

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

Tyker added a comment. the warning was not affected by -ftabstop. i made a patch that fix this. https://reviews.llvm.org/D71037 and I agree that mixing tabs and space is a horrible idea for many reasons. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

nickdesaulniers added a comment. In D70638#1768200 , @nathanchance wrote: > As an FYI, this appears to cause several false positive warnings with the > Linux kernel: > > ../drivers/video/fbdev/core/fbmem.c:665:3: warning: misleading indentation; >

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

Tyker marked 6 inline comments as done. Tyker added a comment. i did a few test on the linux kernel on prior version of this patchs and the mix of spaces and tabs caused some false positives. but i do believe there is still a bug here. for the mix of space and tabs. gcc has a -ftabstop=//X// to

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

xbolva00 added inline comments. Comment at: clang/test/Parser/warn-misleading-indentation.cpp:209 +} \ No newline at end of file Please add newline. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

xbolva00 added a comment. fbmem.c case reduced: https://godbolt.org/z/ma4aFA Looks like kernel uses tabs here So we have BinaryOperator If we use spaces: BinaryOperator 'int' '=' Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

nathanchance added a comment. As an FYI, this appears to cause several false positive warnings with the Linux kernel: ../drivers/video/fbdev/core/fbmem.c:665:3: warning: misleading indentation; statement is not part of the previous 'else' [-Wmisleading-indentation] if

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

This revision was automatically updated to reflect the committed changes. Closed by commit rGbc840b21e161: [Diagnostic] add a warning which warns about misleading indentation (authored by Tyker). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

Tyker updated this revision to Diff 231949. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/ https://reviews.llvm.org/D70638 Files: clang/include/clang/Basic/DiagnosticGroups.td clang/include/clang/Basic/DiagnosticParseKinds.td clang/include/clang/Lex/Preprocessor.h

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM with some minor nits. Comment at: clang/lib/Parse/ParseStmt.cpp:1219 + void Check() { + +NeedsChecking = false; Spurious whitespace, but it would be useful to add a newline above

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

Tyker updated this revision to Diff 231792. Tyker added a comment. improved based on aaron's comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/ https://reviews.llvm.org/D70638 Files: clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

aaron.ballman added inline comments. Comment at: clang/lib/Parse/ParseStmt.cpp:1369-1370 /*ShouldEnter=*/ConstexprCondition && *ConstexprCondition); -ElseStmt = ParseStatement(); +if (Tok.is(tok::kw_if)) + ElseStmt = ParseIfStatement(nullptr, ElseLoc); +

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

xbolva00 added a comment. Looks better, thanks. Aaron ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/ https://reviews.llvm.org/D70638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

Tyker updated this revision to Diff 231585. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/ https://reviews.llvm.org/D70638 Files: clang/include/clang/Basic/DiagnosticGroups.td clang/include/clang/Basic/DiagnosticParseKinds.td clang/include/clang/Lex/Preprocessor.h

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

Tyker marked an inline comment as done. Tyker added inline comments. Comment at: clang/lib/Parse/ParseStmt.cpp:1376 + +MIChecker.Check(ElseStmt.isUsable()); Tyker wrote: > xbolva00 wrote: > > What is wrong with code you used some rev ago? > > > > if

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

Tyker marked an inline comment as done. Tyker added inline comments. Comment at: clang/lib/Parse/ParseStmt.cpp:1376 + +MIChecker.Check(ElseStmt.isUsable()); xbolva00 wrote: > What is wrong with code you used some rev ago? > > if (usable) check(); > > Now

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

Tyker updated this revision to Diff 231584. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/ https://reviews.llvm.org/D70638 Files: clang/include/clang/Basic/DiagnosticGroups.td clang/include/clang/Basic/DiagnosticParseKinds.td clang/include/clang/Lex/Preprocessor.h

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

Tyker updated this revision to Diff 231583. Tyker added a comment. Improve the warning for else if CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/ https://reviews.llvm.org/D70638 Files: clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

Tyker updated this revision to Diff 231561. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/ https://reviews.llvm.org/D70638 Files: clang/include/clang/Basic/DiagnosticGroups.td clang/include/clang/Basic/DiagnosticParseKinds.td clang/include/clang/Lex/Preprocessor.h

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

xbolva00 added a comment. >> there is only one case in llvm's code in which this warning is more >> agressive thant GCC's. I think that's okay. If after-commit feedback shows this is bad, we can always tune this diagnostic more and do not warn in that case. CHANGES SINCE LAST ACTION

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

xbolva00 added inline comments. Comment at: clang/include/clang/Parse/Parser.h:2269 +public: /// isCXXDeclarationStatement - C++-specialized function that disambiguates Extra change? Your code does not use isCXXDeclarationStatement. CHANGES SINCE LAST

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

Tyker updated this revision to Diff 231545. Tyker added a comment. I just found out that `Parser::isCXXDeclarationStatement` is does more then just disambiguation it can emit diagnostics. which can cause error on correct code. so we can't use it in this context to disambiguate. so it is not

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

xbolva00 added a comment. Please land code fixes as NFC patch and then try to reland this patch. Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2532 +if (Val.getAsLocSymbol() == Sym) { + const VarRegion *VR = MR->getBaseRegion()->getAs(); +

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

Tyker updated this revision to Diff 231534. Tyker added a comment. yeah i saw. this version of the patch builds all llvm subproject(not tested on llgo) without warnings. there is only one case in llvm's code in which this warning is more agressive thant GCC's. if (1) i = 0; //

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

xbolva00 reopened this revision. xbolva00 added a subscriber: tstellar. xbolva00 added a comment. This revision is now accepted and ready to land. @tstellar reverted this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

Tyker added a comment. Thanks for the review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70638/new/ https://reviews.llvm.org/D70638 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

This revision was automatically updated to reflect the committed changes. Closed by commit rG7b86188b50bf: [Diagnostic] add a warning which warns about misleading indentation (authored by Tyker). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github