RE: [PATCH v4 4/4] build: enable MSVC specific compiler options

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Monday, 14 August 2023 18.47
> 
> On Mon, Aug 14, 2023 at 09:10:53AM -0700, Tyler Retzlaff wrote:
> > On Mon, Aug 14, 2023 at 09:30:20AM +0100, Bruce Richardson wrote:
> > > On Fri, Aug 11, 2023 at 11:24:47AM -0700, Tyler Retzlaff wrote:
> > > > * Enable optional use of C11 atomics support.  * Enable use of C23
> > > > typeof operator.  * Explicitly force intrinsics when building with
> > > > MSVC.  * Disable MSVC C runtime checks.
> > > >
> > > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > >
> > > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> > >
> > > If there is going to be a lot of this type of special handling for
> > > MSVC, we could look to add a separate config/msvc (and config/gcc-
> like)
> > > directory with separate meson.build files for the different
> toolchains.
> > > Might help centralize all such definitions in one place rather than
> > > having conditionals everywhere.

Please think twice before splitting into per-toolchain files! You must consider the ratio of differences vs. similarities.

<rant on>
For reference, consider the EAL, which has per-arch subdirectories basically containing the same files, whereof many contain nearly similar content for the different architectures.
And much of it is not even arch-specific at all, but plain C.
There is way too much copy-paste in this design pattern.
Although a "default fallback" implementation could alleviate some of this, minor arch-specific deviations in a function would still require a full copy-paste of the function's remaining code.
In my opinion, a lot of this would be better having in unified files with #ifdefs where required by one or more architectures. It would also allow arch A and arch B to share one implementation, while arch C and arch D share another, and arch E has its own.
<rant off>

The EAL might be a really bad comparison here, but it's a good example of a differences/similarities ratio moving in an unfavorable direction for the design pattern.
I also hope it serves as an example of how not to do it.

That being said, I admit that it could probably be done right. In this case, it's a mostly matter of taste if you prefer keeping the differences together in one file, or having the differences spread out at the locations where they have their actual effect.

> >
> > i think that would probably be a good idea. it would untangle the
> > toolchain detail from the flow of the build files.
> >
> > i don't propose introducing it in this series but when this is merged
> i
> > would like to reach out and get your thoughts on how to properly set
> up a
> > config/toolchain-xxx. in addition to the compiler flags and
> definitions
> > below it would be good to suppress (for now) warnings until i have an
> > opportunity to evaluate and address the code raising them.
> >
> Agree on not requiring it for this set. I'm not exactly sure how to
> split
> up the toolchain files, especially given that gcc and clang (and other
> llvm-based compilers like icx) are so very, very similar in what we have
> to
> do for them. It would be very wasteful to have individual toolchain
> files
> for each one, duplicating lots of settings. That's why my initial
> suggestion was for msvc and "gcc-like" compilers. Any suggestions for a
> better name for the latter, welcome! :-)