Re: [Mesa-dev] [PATCH] st/nir: fix illegal designated initializer in st_glsl_to_nir.cpp
> How about something simple like this instead: > > > diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp > b/src/mesa/state_tracker > index d6a0264..4f5acfd 100644 > --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp > +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp > @@ -687,9 +687,14 @@ st_link_nir(struct gl_context *ctx, > * st_nir_preprocess. > */ > if (shader_program->data->spirv) { > - static const gl_nir_linker_options opts = { > - true /*fill_parameters */ > - }; > + /* Note: this object could be static const but designated > + * initializers are not part of the C++ standard (allowed by GCC > + * but not MSVC.) > + */ > + gl_nir_linker_options opts = { 0 }; > + > + opts.fill_parameters = true; > + >if (!gl_nir_link(ctx, shader_program, &opts)) > return GL_FALSE; That looks fine to me. Caio ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/nir: fix illegal designated initializer in st_glsl_to_nir.cpp
On 09/11/2019 03:06 PM, Ian Romanick wrote: On 9/10/19 10:53 PM, Brian Paul wrote: IIRC, designated initializers are not legal C++. Fixes the MSVC build. Fixes: 83fd1e58 ("glsl/nir: Add and use a gl_nir_link() function") --- src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker/st_glsl_to_nir.cpp index 280a778..d6a0264 100644 --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp @@ -688,7 +688,7 @@ st_link_nir(struct gl_context *ctx, */ if (shader_program->data->spirv) { static const gl_nir_linker_options opts = { - .fill_parameters = true, + true /*fill_parameters */ Could we get a comment in the definition of gl_nir_linker_options to remind people to either add options only to the end or double check all of the places that initialize the structures? If someone adds 'bool do_foo_instead_of_bar' option at the beginning of that struct, it will cause problems. }; if (!gl_nir_link(ctx, shader_program, &opts)) return GL_FALSE; How about something simple like this instead: diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker index d6a0264..4f5acfd 100644 --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp @@ -687,9 +687,14 @@ st_link_nir(struct gl_context *ctx, * st_nir_preprocess. */ if (shader_program->data->spirv) { - static const gl_nir_linker_options opts = { - true /*fill_parameters */ - }; + /* Note: this object could be static const but designated + * initializers are not part of the C++ standard (allowed by GCC + * but not MSVC.) + */ + gl_nir_linker_options opts = { 0 }; + + opts.fill_parameters = true; + if (!gl_nir_link(ctx, shader_program, &opts)) return GL_FALSE; -Brian ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/nir: fix illegal designated initializer in st_glsl_to_nir.cpp
On 9/10/19 10:53 PM, Brian Paul wrote: > IIRC, designated initializers are not legal C++. > Fixes the MSVC build. > > Fixes: 83fd1e58 ("glsl/nir: Add and use a gl_nir_link() function") > --- > src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp > b/src/mesa/state_tracker/st_glsl_to_nir.cpp > index 280a778..d6a0264 100644 > --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp > +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp > @@ -688,7 +688,7 @@ st_link_nir(struct gl_context *ctx, > */ > if (shader_program->data->spirv) { >static const gl_nir_linker_options opts = { > - .fill_parameters = true, > + true /*fill_parameters */ Could we get a comment in the definition of gl_nir_linker_options to remind people to either add options only to the end or double check all of the places that initialize the structures? If someone adds 'bool do_foo_instead_of_bar' option at the beginning of that struct, it will cause problems. >}; >if (!gl_nir_link(ctx, shader_program, &opts)) > return GL_FALSE; > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/nir: fix illegal designated initializer in st_glsl_to_nir.cpp
On Tue, Sep 10, 2019 at 10:54 PM Brian Paul wrote: > > IIRC, designated initializers are not legal C++. > Fixes the MSVC build. > > Fixes: 83fd1e58 ("glsl/nir: Add and use a gl_nir_link() function") > --- > src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp > b/src/mesa/state_tracker/st_glsl_to_nir.cpp > index 280a778..d6a0264 100644 > --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp > +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp > @@ -688,7 +688,7 @@ st_link_nir(struct gl_context *ctx, > */ > if (shader_program->data->spirv) { >static const gl_nir_linker_options opts = { > - .fill_parameters = true, > + true /*fill_parameters */ Probably intended to have a space after /* ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/nir: fix illegal designated initializer in st_glsl_to_nir.cpp
Looks good to me. Reviewed-by: Neha Bhende Regards, Neha From: Brian Paul Sent: Wednesday, September 11, 2019 11:23 AM To: mesa-dev@lists.freedesktop.org Cc: Neha Bhende; Charmaine Lee; aio.olive...@intel.com Subject: [PATCH] st/nir: fix illegal designated initializer in st_glsl_to_nir.cpp IIRC, designated initializers are not legal C++. Fixes the MSVC build. Fixes: 83fd1e58 ("glsl/nir: Add and use a gl_nir_link() function") --- src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker/st_glsl_to_nir.cpp index 280a778..d6a0264 100644 --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp @@ -688,7 +688,7 @@ st_link_nir(struct gl_context *ctx, */ if (shader_program->data->spirv) { static const gl_nir_linker_options opts = { - .fill_parameters = true, + true /*fill_parameters */ }; if (!gl_nir_link(ctx, shader_program, &opts)) return GL_FALSE; -- 1.8.5.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] st/nir: fix illegal designated initializer in st_glsl_to_nir.cpp
IIRC, designated initializers are not legal C++. Fixes the MSVC build. Fixes: 83fd1e58 ("glsl/nir: Add and use a gl_nir_link() function") --- src/mesa/state_tracker/st_glsl_to_nir.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/state_tracker/st_glsl_to_nir.cpp b/src/mesa/state_tracker/st_glsl_to_nir.cpp index 280a778..d6a0264 100644 --- a/src/mesa/state_tracker/st_glsl_to_nir.cpp +++ b/src/mesa/state_tracker/st_glsl_to_nir.cpp @@ -688,7 +688,7 @@ st_link_nir(struct gl_context *ctx, */ if (shader_program->data->spirv) { static const gl_nir_linker_options opts = { - .fill_parameters = true, + true /*fill_parameters */ }; if (!gl_nir_link(ctx, shader_program, &opts)) return GL_FALSE; -- 1.8.5.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev