Pedro and Oliver, Yes. Renaming the struct members is my preferred solution. This is why I did not send this as a code review as an official change request.
It was just to complete the set of options to consider * No code changes. Figure out compiler flags to address. STATUS: No complete solution found for all compilers. * Use C-Preprocessor to rename keywords. STATUS: Functional, but very bad style and hard to read and maintain. * Rename C structure fields that collide with C++ keywords STATUS: Functional. May break downstream consumers that have FW code that references those fields. Thanks, Mike > -----Original Message----- > From: Pedro Falcato <pedro.falc...@gmail.com> > Sent: Thursday, May 25, 2023 11:01 AM > To: Oliver Smith-Denny <o...@linux.microsoft.com> > Cc: devel@edk2.groups.io; Kinney, Michael D <michael.d.kin...@intel.com>; > Pop, Aaron <aaron...@microsoft.com> > Subject: Re: [edk2-devel] GoogleTest Compatibility with MdePkg's > IndustyStandard header files > > On Thu, May 25, 2023 at 6:43 PM Oliver Smith-Denny > <o...@linux.microsoft.com> wrote: > > > > Hi Mike, > > > > Thanks for looking for solutions here. This one feels like > > quite a back bend, I'm imagining reading code and coming > > across TpmStruct.CPLUSPLUS_OPERATOR_KEYWORD and having to > > dig around quite a lot to see what goodness is going > > on. Because we would have to update the C files, too, right, > > No, the idea is that current C code can use the > already-existing-and-standard .operator and .xor, and C++ can use > .operator_ and .xor_ (or the macros, although please no?). > But my idea was to leave this as a Tpm.h hack, not in Base.h (new > headers and structs should take C++ into account). > > > depending on the test (there exist tests that want to test > > static functions and so include the C file in the unit test > > file). Perhaps that is an anti-pattern and googletest has > > This is a hacky solution. Either write things in C++, or don't include > .c in .cpp. > C code is not C++ code. There's a lot of C code that does not and > should not compile in C++. > > So: > > 1) Write the actual functionality code in C++. This is not yet > supported in EDK2 (I'm a proponent of this) > 2) Don't make the functions you're testing static, or make them > conditionally static on something > > Note: Adding proper, actual C++ code to EDK2 requires some care, but > could result in actual good changes. I don't know how well this would > be received by the community though. > > > But, that being said, this is an issue we face, so perhaps > > it would be simpler to just rename the members to not conflict > > with the C++ keywords, as previously suggested, even though > > this may differ from the spec, but it would more align with > > EDKII's conventions (shouty case) where the C++ keywords seem > > to be lowercase. With the below patch, we would already be > > I think breaking all sorts of users for this sort of "silly" problems > is not a good option here. There's no actual need for this ATM, apart > from "We want to test this silly code in this new Google Test library > that just appeared upstream". > > But these are just my 2c of course. > > -- > Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#105323): https://edk2.groups.io/g/devel/message/105323 Mute This Topic: https://groups.io/mt/99079638/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
