Re: [Mesa-dev] [PATCH v2] Rename the DEBUG macro to MESA_DEBUG
I don't like adding workarounds to our codebase for someone else's problem, generally, but specifically I think this is a bad idea because the name MESA_DEBUG is already used (it's an environment variable), and this is a completely separate meaning. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] Rename the DEBUG macro to MESA_DEBUG
On Tue, Jan 10, 2017 at 12:07 PM, Jan Vesely wrote: > On Tue, 2017-01-10 at 16:43 +, Emil Velikov wrote: >> On 10 January 2017 at 15:04, Vedran Miletić wrote: >> > On 09/19/2016 08:39 PM, Vedran Miletić wrote: >> > > On 09/07/2016 06:52 PM, Vedran Miletić wrote: >> > > > LLVM and Mesa both define the DEBUG macro in incompatible ways. As a >> > > > general practice, we should avoid using such generic names when it is >> > > > possible to do so. >> > > > >> > > > This patch renames all occurrences of the DEBUG macro to MESA_DEBUG, >> > > > and removes workarounds previously used to enable building Mesa with >> > > > LLVM (pop_macro() and push_macro() function calls). >> > > > >> > > > v2: >> > > > * Rename remaining occurences found by git grep '\' >> > > > * Use /* !MESA_DEBUG */ with #else instead of /* MESA_DEBUG */ >> > > > >> > > > Signed-off-by: Vedran Miletić >> > > > Acked-by: Christian König >> > > > --- >> > > >> > > Anyone? >> > > >> > > Regards, >> > > Vedran >> > > >> > >> > Emil and others, >> > >> > I can rebase this if there would be interest in getting it merged for 17.0. >> > >> >> Thanks for the reminder. >> >> Not meaning to flock a dead horse - I'm rarely keen adding workarounds >> and this looks like one. >> Then again consider me neutral on the topic and I'll leave the final >> call to Brian. > > if I may, I'd strongly argue _for_ this change. Using generic (non- > prefixed) names is a bad idea. generic names in *exported headers* are a bad idea.. otherwise we end up with silliness like MESA_MAX2(), etc.. I'm generally of the opinion that we can change this if we have to, it isn't really worth arguing against. It does seem like a workaround for problem in a dependency, rather than a problem in mesa.. but some times that is what you have to do. But wasn't it llvm that triggered this whole thing in the first place? If they are fixing, maybe we don't need the workaround? Anyways, if it is still something that needs working around, and someone wants to rebase the patch, go for it. BR, -R > The same goes for LLVM/clang and there are corresponding patches to do > that[0,1]. > > Jan > > [0]https://reviews.llvm.org/D11833 > [1]https://reviews.llvm.org/D11834 > >> >> -Emil >> ___ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] Rename the DEBUG macro to MESA_DEBUG
On Tue, 2017-01-10 at 16:43 +, Emil Velikov wrote: > On 10 January 2017 at 15:04, Vedran Miletić wrote: > > On 09/19/2016 08:39 PM, Vedran Miletić wrote: > > > On 09/07/2016 06:52 PM, Vedran Miletić wrote: > > > > LLVM and Mesa both define the DEBUG macro in incompatible ways. As a > > > > general practice, we should avoid using such generic names when it is > > > > possible to do so. > > > > > > > > This patch renames all occurrences of the DEBUG macro to MESA_DEBUG, > > > > and removes workarounds previously used to enable building Mesa with > > > > LLVM (pop_macro() and push_macro() function calls). > > > > > > > > v2: > > > > * Rename remaining occurences found by git grep '\' > > > > * Use /* !MESA_DEBUG */ with #else instead of /* MESA_DEBUG */ > > > > > > > > Signed-off-by: Vedran Miletić > > > > Acked-by: Christian König > > > > --- > > > > > > Anyone? > > > > > > Regards, > > > Vedran > > > > > > > Emil and others, > > > > I can rebase this if there would be interest in getting it merged for 17.0. > > > > Thanks for the reminder. > > Not meaning to flock a dead horse - I'm rarely keen adding workarounds > and this looks like one. > Then again consider me neutral on the topic and I'll leave the final > call to Brian. if I may, I'd strongly argue _for_ this change. Using generic (non- prefixed) names is a bad idea. The same goes for LLVM/clang and there are corresponding patches to do that[0,1]. Jan [0]https://reviews.llvm.org/D11833 [1]https://reviews.llvm.org/D11834 > > -Emil > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev signature.asc Description: This is a digitally signed message part ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] Rename the DEBUG macro to MESA_DEBUG
On 10 January 2017 at 15:04, Vedran Miletić wrote: > On 09/19/2016 08:39 PM, Vedran Miletić wrote: >> On 09/07/2016 06:52 PM, Vedran Miletić wrote: >>> LLVM and Mesa both define the DEBUG macro in incompatible ways. As a >>> general practice, we should avoid using such generic names when it is >>> possible to do so. >>> >>> This patch renames all occurrences of the DEBUG macro to MESA_DEBUG, >>> and removes workarounds previously used to enable building Mesa with >>> LLVM (pop_macro() and push_macro() function calls). >>> >>> v2: >>> * Rename remaining occurences found by git grep '\' >>> * Use /* !MESA_DEBUG */ with #else instead of /* MESA_DEBUG */ >>> >>> Signed-off-by: Vedran Miletić >>> Acked-by: Christian König >>> --- >> >> Anyone? >> >> Regards, >> Vedran >> > > Emil and others, > > I can rebase this if there would be interest in getting it merged for 17.0. > Thanks for the reminder. Not meaning to flock a dead horse - I'm rarely keen adding workarounds and this looks like one. Then again consider me neutral on the topic and I'll leave the final call to Brian. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] Rename the DEBUG macro to MESA_DEBUG
On 09/19/2016 08:39 PM, Vedran Miletić wrote: > On 09/07/2016 06:52 PM, Vedran Miletić wrote: >> LLVM and Mesa both define the DEBUG macro in incompatible ways. As a >> general practice, we should avoid using such generic names when it is >> possible to do so. >> >> This patch renames all occurrences of the DEBUG macro to MESA_DEBUG, >> and removes workarounds previously used to enable building Mesa with >> LLVM (pop_macro() and push_macro() function calls). >> >> v2: >> * Rename remaining occurences found by git grep '\' >> * Use /* !MESA_DEBUG */ with #else instead of /* MESA_DEBUG */ >> >> Signed-off-by: Vedran Miletić >> Acked-by: Christian König >> --- > > Anyone? > > Regards, > Vedran > Emil and others, I can rebase this if there would be interest in getting it merged for 17.0. Regards, Vedran ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2] Rename the DEBUG macro to MESA_DEBUG
On 09/07/2016 06:52 PM, Vedran Miletić wrote: > LLVM and Mesa both define the DEBUG macro in incompatible ways. As a > general practice, we should avoid using such generic names when it is > possible to do so. > > This patch renames all occurrences of the DEBUG macro to MESA_DEBUG, > and removes workarounds previously used to enable building Mesa with > LLVM (pop_macro() and push_macro() function calls). > > v2: > * Rename remaining occurences found by git grep '\' > * Use /* !MESA_DEBUG */ with #else instead of /* MESA_DEBUG */ > > Signed-off-by: Vedran Miletić > Acked-by: Christian König > --- Anyone? Regards, Vedran -- Vedran Miletić vedran.miletic.net ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev