[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

estan commandeered this revision. estan added a reviewer: fiesh. estan added a comment. I think we can close this one now that https://reviews.llvm.org/D116378 has landed though, since this revision was originally only about warnings in system macros case which is now solved (?) I'll commandeer

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

estan added a comment. Alright, yea I'll see what using std::array would mean for our code. In many cases we are just using plain arrays currently, and often they are just a way to get data in/out of these APIs (and for the out direction, we quickly go on to saving the data we got out into some

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

carlosgalvezp added a comment. I see, thanks for the examples! Since you mention modern C++, I can guess you are using std::array in your code. Maybe just pass "buffer.data()" to the third-party functions? I think it's a bit cleaner than casts. If you use C arrays, consider switching to std::ar

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

estan added a comment. Yep thanks a lot for that @carlosgalvezp, getting the system macros sorted out will get rid of many warnings we were getting in our code base (seems the qDebug / qWarning macros in the Qt version we're are using violate this check on their own). Regarding decay in calls

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

carlosgalvezp added a comment. My patch has now been merged, hope it helps a bit! Reading through the comments I'm not sure it will be enough though. In the cases you mention we've typically wrapped the third-party function into a helper function that adds a N

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

estan added a comment. In D88833#3225842 , @aaron.ballman wrote: > In D88833#3224389 , @carlosgalvezp > wrote: > >> Please note: I have a patch that disables warnings from system macros for >> all clang-tidy warn

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

aaron.ballman added a comment. In D88833#3224389 , @carlosgalvezp wrote: > Please note: I have a patch that disables warnings from system macros for all > clang-tidy warnings, it would be good to review it so we don't need to > implement the same mechan

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

estan added a comment. In D88833#3222829 , @estan wrote: > Sounds good @aaron.ballman, let's wait for @fiesh. > > Though I realize now that the scope of this patch is probably not enough to > solve a problem we have in our code base. The check will warn a

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

estan added a comment. @carlosgalvezp Oh yes, that would be even better. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88833/new/ https://reviews.llvm.org/D88833 ___ cfe-commits mailing list cfe-commits@

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

carlosgalvezp added a comment. Please note: I have a patch that disables warnings from system macros for all clang-tidy warnings, it would be good to review it so we don't need to implement the same mechanism in all 400+ checks :) https://reviews.llvm.org/D116378 Repository: rG LLVM Github

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

aaron.ballman added a comment. In D88833#3223077 , @estan wrote: > That did the trick, thanks @aaron.ballman. Glad that worked! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88833/new/ https://reviews.llvm

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

estan added a comment. That did the trick, thanks @aaron.ballman. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88833/new/ https://reviews.llvm.org/D88833 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

estan added a comment. Ah yes I had a hunch it had to be "clang;clang-tools-extra" so running a build with that now. Almost done. It crashed at the very end due to OOM while linking, but reduced parallelism now and running again. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

aaron.ballman added a comment. In D88833#3223055 , @estan wrote: > @aaron.ballman A bit off topic, but I followed > https://github.com/llvm/llvm-project/blob/main/README.md#getting-the-source-code-and-building-llvm > to build and install llvm-project mai

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

estan added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp:65 void ProBoundsArrayToPointerDecayCheck::registerMatchers(MatchFinder *Finder) { - // The only allowed array to pointer decay + // We only allowed a

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

estan added a comment. @aaron.ballman A bit off topic, but I followed https://github.com/llvm/llvm-project/blob/main/README.md#getting-the-source-code-and-building-llvm to build and install llvm-project main branch as follows: git clone https://github.com/llvm/llvm-project.git cd llvm-proje

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

aaron.ballman added a comment. In D88833#3222828 , @fiesh wrote: >> My expectation was that @fiesh would be updating the review if they wanted >> this to land. If they indicate they're no longer interested in working on >> the patch, then I think it's fi

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

estan added a comment. And there he is :) I've never worked on LLVM / clang-tidy but could have a look at making it opt-in. Now we know the status at least. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88833/new/ https://reviews.llvm.org/D88833

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

estan added a comment. Sounds good @aaron.ballman, let's wait for @fiesh. Though I realize now that the scope of this patch is probably not enough to solve a problem we have in our code base. The check will warn about (for example) things like this: In a third party lib outside our control:

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

fiesh added a comment. > My expectation was that @fiesh would be updating the review if they wanted > this to land. If they indicate they're no longer interested in working on the > patch, then I think it's fine for you to commandeer the patch. But you should > give them a chance to speak up in

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

aaron.ballman added a comment. In D88833#3222331 , @estan wrote: > @lebedev.ri @aaron.ballman I think the opt-in sounds good too. Are we waiting > for @fiesh to implement this, or is the patch abandoned? Would be sad if it > was stranded the way https://

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

estan added a comment. @lebedev.ri @aaron.ballman I think the opt-in sounds good too. Are we waiting for @fiesh to implement this, or is the patch abandoned? Would be sad if it was stranded the way https://reviews.llvm.org/D31130 was, since I think many people are skipping this check due to dia

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

arysef added a comment. Herald added a subscriber: nullptr.cpp. Here are also some bugs that seem like they could be closed once this is merged: https://bugs.llvm.org/show_bug.cgi?id=28480 https://bugs.llvm.org/show_bug.cgi?id=32239 Also about the current behavior, it seems like this behavior h

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

aaron.ballman added a comment. In D88833#2319799 , @lebedev.ri wrote: > I think the current behavior should instead be configurable, with a way to > opt-in into weaker guarantees. I think that's a good idea, thank you! Repository: rG LLVM Github Mono

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

lebedev.ri added a comment. I think the current behavior should instead be configurable, with a way to opt-in into weaker guarantees. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88833/new/ https://reviews.llvm.org/D88833 ___

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

aaron.ballman added a comment. In D88833#2319118 , @fiesh wrote: > @aaron.ballman valid point. It would seem natural to diagnose both since the > user is voluntarily causing the decay? I'm on the fence. If the argument to the function is one that's defi

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

fiesh added a comment. @aaron.ballman valid point. It would seem natural to diagnose both since the user is voluntarily causing the decay? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88833/new/ https://reviews.llvm.org/D88833 _

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

aaron.ballman added a comment. Despite my earlier happiness with the patch, it's now a bit unclear to me from the C++ Core Guideline wording what the right behavior here actually is. The bounds profile is trying to ensure you don't access out-of-range elements of a container and the decay part

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

fiesh marked 6 inline comments as done. fiesh added a comment. @njames93 Thank you for your review, I hopefully addressed all your points. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88833/new/ https://reviews.llvm.org/D88833 ___

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

fiesh updated this revision to Diff 296452. fiesh added a comment. Applied changes to address code review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88833/new/ https://reviews.llvm.org/D88833 Files: clang-tools-extra/clang-tidy/cppcoreguidel

[PATCH] D88833: [clang-tidy] Do not warn on pointer decays in system macros

njames93 added a comment. Can you please upload this diff with full context `-U99` Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp:50 + +AST_MATCHER(ImplicitCastExpr, isArrayToPointerDecay) { + return Node.getCastKind() ==