[edk2-devel] GoogleTest Compatibility with MdePkg's IndustyStandard header files

[Michael D Kinney]

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.falcato at gmail.com>
> Sent: Thursday, May 25, 2023 11:01 AM
> To: Oliver Smith-Denny <osde at linux.microsoft.com>
> Cc: devel at edk2.groups.io; Kinney, Michael D <michael.d.kinney at intel.com>;
> Pop, Aaron <aaronpop at 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
> <osde at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3943202/1813853/130120423/xyzzy [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-