Re: [PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

Nice! I'm glad to see the patch being revived. Thanks for that :) Em seg, 5 de out de 2020 10:48, fiesh via Phabricator < revi...@reviews.llvm.org> escreveu: > fiesh added a comment. > > Continued in https://reviews.llvm.org/D88833 > > > CHANGES SINCE LAST ACTION >

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

fiesh added a comment. Continued in https://reviews.llvm.org/D88833 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D31130/new/ https://reviews.llvm.org/D31130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

aaron.ballman added a comment. In D31130#2220729 , @fiesh wrote: > Ping everybody? This is a rather important issue I think since it makes > clang-tidy not usable in a lot of cases, and the fix would be all done. > Please let's get this merged! Sorry

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

fiesh added a comment. Ping everybody? This is a rather important issue I think since it makes clang-tidy not usable in a lot of cases, and the fix would be all done. Please let's get this merged! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D31130/new/

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

fiesh added a comment. @JonasToth could you please clarify if the original author's comment is sufficient grants of rights? If so, from what I can tell if the commit message was adapted to reflect that this removes false positives with system macros like `assert`, this would be good to go?

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

brenoguim added a comment. Oh, and if I remember correctly, the reason for this patch was to avoid exactly the github issue mentioned here. I'm embarrassed I didn't explain that anywhere :( CHANGES SINCE LAST ACTION https://reviews.llvm.org/D31130/new/ https://reviews.llvm.org/D31130

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

brenoguim added a comment. Hey, I'm back. I actually kind of forgot about this. Unfortunately I cannot to continue the patch soon. How do I grant you full rights on this patch? If just saying here is enough, there you go, anyone can take it and change it. If there is more bureaucracy to it,

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

ClausKlein added a comment. I have problems with clang-tidy-9.0.0 on OS X and assert macro usage. see https://github.com/isocpp/CppCoreGuidelines/issues/1589 In which clang version is the patch included? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D31130/new/

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

fiesh added a comment. > I guess we need specific permission from the author. maybe writing a mail to > him/her would work? Then he/she can give permission (or decline it, which is > unexpected i guess). I already wrote an email a couple weeks ago but haven't received a reply so far. Let's

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

JonasToth added a comment. In D31130#1758453 , @fiesh wrote: > In D31130#1711841 , @lebedev.ri > wrote: > > > The description only says what the patch does, not why this is the desired > > behavior. > > Also,

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

fiesh added a comment. In D31130#1711841 , @lebedev.ri wrote: > The description only says what the patch does, not why this is the desired > behavior. > Also, this is now under a wrong license. What are the options in case the original author has

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

lebedev.ri added a comment. The description only says what the patch does, not why this is the desired behavior. Also, this is now under a wrong license. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D31130/new/ https://reviews.llvm.org/D31130

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

fiesh added a comment. Herald added a subscriber: wuzish. Ping! Am I correct in that basically everything's done here and this has been lingering ever since? It would be great if the change could make it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D31130/new/

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

aaron.ballman added inline comments. Comment at: test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp:60 + + sys_fn_decay_str(SYS_STRING); // Not Ok - should it be ok? + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: do not implicitly decay an array

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

brenoguim updated this revision to Diff 97385. brenoguim marked an inline comment as done. brenoguim added a comment. Never flag predefinedExpr() types. https://reviews.llvm.org/D31130 Files: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

mgehre added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp:83 + unless(hasSourceExpression(stringLiteral())), + unless(sysSymbolDecayInSysHeader())) .bind("cast"),

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

brenoguim updated this revision to Diff 93062. brenoguim added a comment. - Removed the "#include " which caused problems in environments without that header - Included a directory with -isystem to simulate system headers - Added a "macro.h" header with definitions of types, functions and macros

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

alexfh added a comment. In https://reviews.llvm.org/D31130#707197, @aaron.ballman wrote: > This change was reverted in r298470. The use of the include is a > problem because this is not a clang-supplied header file. Also, there's a > (good) question about what array decay is happening in the

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

aaron.ballman reopened this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. This change was reverted in r298470. The use of the include is a problem because this is not a clang-supplied header file. Also, there's a (good) question about what array

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

aaron.ballman closed this revision. aaron.ballman added a comment. I've commit in r298421. https://reviews.llvm.org/D31130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

brenoguim added a comment. I appreciate it! https://reviews.llvm.org/D31130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

aaron.ballman added a comment. In https://reviews.llvm.org/D31130#706623, @brenoguim wrote: > Hi @aaron.ballman, > > Thanks for the review! > > I don't have rights to push this. Who should I contact to push the change in > my behalf? I'm happy to do so on your behalf (it'll happen later this

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

brenoguim marked 4 inline comments as done. brenoguim added a comment. Hi @aaron.ballman, Thanks for the review! I don't have rights to push this. Who should I contact to push the change in my behalf? https://reviews.llvm.org/D31130 ___

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D31130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

brenoguim updated this revision to Diff 92419. brenoguim added a comment. - Using the ImplicitCastExpr type on the AST_MATCHER directly to avoid explicit cast. - Using isa<> instead of dyn_cast<> to just check for type - Move variable declaration into "if" condition

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

aaron.ballman added inline comments. Comment at: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp:50 +AST_MATCHER(Stmt, isArrayToPointerDecay) { + const auto *MatchedCast = cast(); Why not match on `ImplicitCastExpr` rather than `Stmt`?

[PATCH] D31130: B32239 clang-tidy should not warn about array to pointer decay on system macros

brenoguim added a reviewer: cfe-commits. brenoguim added a comment. Forgot to add cfe-commits... https://reviews.llvm.org/D31130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits