* [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` @ 2024-10-11 1:20 Rebecca Cran 2024-10-11 3:41 ` Sean 2024-10-15 21:12 ` Michael D Kinney 0 siblings, 2 replies; 17+ messages in thread From: Rebecca Cran @ 2024-10-11 1:20 UTC (permalink / raw) To: Michael Kinney, devel; +Cc: Rebecca Cran A while ago a decision was made on the edk2-devel mailing list that the `STATIC` EDK2 type should be replaced with the C keyword `static`. Update the Coding Specification to match. While here, remove the outdated section disallowing static functions since they're no longer a problem. Bump the revision to 2.4 and turn off the draft status. Signed-off-by: Rebecca Cran <rebecca@bsdio.com> --- 5_source_files/54_code_file_structure.md | 8 +------- 5_source_files/56_declarations_and_types.md | 2 +- README.md | 3 ++- book.json | 4 ++-- 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/5_source_files/54_code_file_structure.md b/5_source_files/54_code_file_structure.md index 0c4d6a2..6decc60 100644 --- a/5_source_files/54_code_file_structure.md +++ b/5_source_files/54_code_file_structure.md @@ -269,7 +269,7 @@ other than at the top level of a file as specified by this document. #### 5.4.2.2 Static -An object declared `STATIC` has either file or block scope. +An object declared `static` has either file or block scope. ##### 5.4.2.2.1 Do not reuse an object or function identifier with static storage duration. @@ -277,9 +277,3 @@ Throughout the set of source files defined within a single .inf file, do not reuse an identifier with static storage duration. The compiler may not be confused by this, but the user may confuse unrelated variables with the same name. - -##### 5.4.2.2.2 Functions should not be declared STATIC. - -Some source-level debuggers are unable to resolve static functions. Until it -can be verified that no one is dependent upon a debugger with this limitation, -it is strongly recommended that functions not be declared static. diff --git a/5_source_files/56_declarations_and_types.md b/5_source_files/56_declarations_and_types.md index ec1803d..77e3ce9 100644 --- a/5_source_files/56_declarations_and_types.md +++ b/5_source_files/56_declarations_and_types.md @@ -38,7 +38,7 @@ Any abstract type that is defined must be constructed from other abstract types or from common EFI data types. -#### 5.6.1.2 The use of int, unsigned, char, void, static, long is a violation of the coding convention. +#### 5.6.1.2 The use of int, unsigned, char, void, long is a violation of the coding convention. The corresponding EFI types must be used instead. diff --git a/README.md b/README.md index 77cfdc8..b543bcf 100644 --- a/README.md +++ b/README.md @@ -114,4 +114,5 @@ Copyright (c) 2006-2017, Intel Corporation. All rights reserved. | | [#425](https://bugzilla.tianocore.org/show_bug.cgi?id=425) [CCS] clarify line breaking and indentation requirements for multi-line function calls | | | | [#1656](https://bugzilla.tianocore.org/show_bug.cgi?id=1656) Update all Wiki pages for the BSD+Patent license change with SPDX identifiers | | | | [#607](https://bugzilla.tianocore.org/show_bug.cgi?id=607) Document code comment requirements for spurious variable assignments | | -| 2.3 | Add 4.2 Directory names section and update File names section for the guidelines of module directory and file naming|September 2022|| +| 2.3 | Add 4.2 Directory names section and update File names section for the guidelines of module directory and file naming |September 2022| +| 2.4 | The use of the 'static' C keyword is now preferred over the EDK2 type 'STATIC' |October 2024| diff --git a/book.json b/book.json index d112b26..3ec7a93 100644 --- a/book.json +++ b/book.json @@ -1,8 +1,8 @@ { "variables" : { - "draft" : "yes", + "draft" : "no", "title" : "EDK II C Coding Standards Specification", - "version" : "Revision 2.2" + "version" : "Revision 2.4" }, "plugins": ["puml-aleung"], "pluginsConfig": {} -- 2.46.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120616): https://edk2.groups.io/g/devel/message/120616 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-11 1:20 [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` Rebecca Cran @ 2024-10-11 3:41 ` Sean 2024-10-11 16:47 ` Rebecca Cran 2024-10-15 21:12 ` Michael D Kinney 1 sibling, 1 reply; 17+ messages in thread From: Sean @ 2024-10-11 3:41 UTC (permalink / raw) To: devel@edk2.groups.io, rebecca@bsdio.com, Michael Kinney, Oliver Smith-Denny Cc: Rebecca Cran [-- Attachment #1: Type: text/plain, Size: 5468 bytes --] I think @Oliver Smith-Denny<mailto:osde@linux.microsoft.com> has proposed keeping the macro STATIC as a way to enable cleaner and easier unit tests. Did that get resolved? Thanks Sean ________________________________ From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Rebecca Cran <rebecca@bsdio.com> Sent: Thursday, October 10, 2024 6:20:39 PM To: Michael Kinney <michael.d.kinney@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io> Cc: Rebecca Cran <rebecca@bsdio.com> Subject: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` A while ago a decision was made on the edk2-devel mailing list that the `STATIC` EDK2 type should be replaced with the C keyword `static`. Update the Coding Specification to match. While here, remove the outdated section disallowing static functions since they're no longer a problem. Bump the revision to 2.4 and turn off the draft status. Signed-off-by: Rebecca Cran <rebecca@bsdio.com> --- 5_source_files/54_code_file_structure.md | 8 +------- 5_source_files/56_declarations_and_types.md | 2 +- README.md | 3 ++- book.json | 4 ++-- 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/5_source_files/54_code_file_structure.md b/5_source_files/54_code_file_structure.md index 0c4d6a2..6decc60 100644 --- a/5_source_files/54_code_file_structure.md +++ b/5_source_files/54_code_file_structure.md @@ -269,7 +269,7 @@ other than at the top level of a file as specified by this document. #### 5.4.2.2 Static -An object declared `STATIC` has either file or block scope. +An object declared `static` has either file or block scope. ##### 5.4.2.2.1 Do not reuse an object or function identifier with static storage duration. @@ -277,9 +277,3 @@ Throughout the set of source files defined within a single .inf file, do not reuse an identifier with static storage duration. The compiler may not be confused by this, but the user may confuse unrelated variables with the same name. - -##### 5.4.2.2.2 Functions should not be declared STATIC. - -Some source-level debuggers are unable to resolve static functions. Until it -can be verified that no one is dependent upon a debugger with this limitation, -it is strongly recommended that functions not be declared static. diff --git a/5_source_files/56_declarations_and_types.md b/5_source_files/56_declarations_and_types.md index ec1803d..77e3ce9 100644 --- a/5_source_files/56_declarations_and_types.md +++ b/5_source_files/56_declarations_and_types.md @@ -38,7 +38,7 @@ Any abstract type that is defined must be constructed from other abstract types or from common EFI data types. -#### 5.6.1.2 The use of int, unsigned, char, void, static, long is a violation of the coding convention. +#### 5.6.1.2 The use of int, unsigned, char, void, long is a violation of the coding convention. The corresponding EFI types must be used instead. diff --git a/README.md b/README.md index 77cfdc8..b543bcf 100644 --- a/README.md +++ b/README.md @@ -114,4 +114,5 @@ Copyright (c) 2006-2017, Intel Corporation. All rights reserved. | | [#425](https://bugzilla.tianocore.org/show_bug.cgi?id=425) [CCS] clarify line breaking and indentation requirements for multi-line function calls | | | | [#1656](https://bugzilla.tianocore.org/show_bug.cgi?id=1656) Update all Wiki pages for the BSD+Patent license change with SPDX identifiers | | | | [#607](https://bugzilla.tianocore.org/show_bug.cgi?id=607) Document code comment requirements for spurious variable assignments | | -| 2.3 | Add 4.2 Directory names section and update File names section for the guidelines of module directory and file naming|September 2022|| +| 2.3 | Add 4.2 Directory names section and update File names section for the guidelines of module directory and file naming |September 2022| +| 2.4 | The use of the 'static' C keyword is now preferred over the EDK2 type 'STATIC' |October 2024| diff --git a/book.json b/book.json index d112b26..3ec7a93 100644 --- a/book.json +++ b/book.json @@ -1,8 +1,8 @@ { "variables" : { - "draft" : "yes", + "draft" : "no", "title" : "EDK II C Coding Standards Specification", - "version" : "Revision 2.2" + "version" : "Revision 2.4" }, "plugins": ["puml-aleung"], "pluginsConfig": {} -- 2.46.1 -=-=-=-=-=-= Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120616): https://edk2.groups.io/g/devel/message/120616 Mute This Topic: https://groups.io/mt/108941574/1686594 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [spbrogan@outlook.com] -=-=-=-=-=-= -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120617): https://edk2.groups.io/g/devel/message/120617 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- [-- Attachment #2: Type: text/html, Size: 9094 bytes --] ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-11 3:41 ` Sean @ 2024-10-11 16:47 ` Rebecca Cran 2024-10-14 15:22 ` Oliver Smith-Denny 0 siblings, 1 reply; 17+ messages in thread From: Rebecca Cran @ 2024-10-11 16:47 UTC (permalink / raw) To: Sean Brogan, devel@edk2.groups.io, Michael Kinney, Oliver Smith-Denny I don't know, but I saw a message from Mike recently saying that someone should use 'static' instead of 'STATIC' - so we need to make a clear decision one way or the other. -- Rebecca On 10/10/24 9:41 PM, Sean Brogan wrote: > I think @Oliver Smith-Denny <mailto:osde@linux.microsoft.com> has > proposed keeping the macro STATIC as a way to enable cleaner and > easier unit tests. Did that get resolved? > > Thanks > Sean > ------------------------------------------------------------------------ > *From:* devel@edk2.groups.io <devel@edk2.groups.io> on behalf of > Rebecca Cran <rebecca@bsdio.com> > *Sent:* Thursday, October 10, 2024 6:20:39 PM > *To:* Michael Kinney <michael.d.kinney@intel.com>; > devel@edk2.groups.io <devel@edk2.groups.io> > *Cc:* Rebecca Cran <rebecca@bsdio.com> > *Subject:* [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] > Prefer use of `static` C keyword over EDK2 type `STATIC` > A while ago a decision was made on the edk2-devel mailing list that > the `STATIC` EDK2 type should be replaced with the C keyword `static`. > > Update the Coding Specification to match. While here, remove the > outdated section disallowing static functions since they're no longer > a problem. > > Bump the revision to 2.4 and turn off the draft status. > > Signed-off-by: Rebecca Cran <rebecca@bsdio.com> > --- > 5_source_files/54_code_file_structure.md | 8 +------- > 5_source_files/56_declarations_and_types.md | 2 +- > README.md | 3 ++- > book.json | 4 ++-- > 4 files changed, 6 insertions(+), 11 deletions(-) > > diff --git a/5_source_files/54_code_file_structure.md > b/5_source_files/54_code_file_structure.md > index 0c4d6a2..6decc60 100644 > --- a/5_source_files/54_code_file_structure.md > +++ b/5_source_files/54_code_file_structure.md > @@ -269,7 +269,7 @@ other than at the top level of a file as specified > by this document. > > > #### 5.4.2.2 Static > > > > -An object declared `STATIC` has either file or block scope. > > +An object declared `static` has either file or block scope. > > > > ##### 5.4.2.2.1 Do not reuse an object or function identifier with > static storage duration. > > > > @@ -277,9 +277,3 @@ Throughout the set of source files defined within > a single .inf file, do not > reuse an identifier with static storage duration. The compiler may not be > > confused by this, but the user may confuse unrelated variables with > the same > > name. > > - > > -##### 5.4.2.2.2 Functions should not be declared STATIC. > > - > > -Some source-level debuggers are unable to resolve static functions. > Until it > > -can be verified that no one is dependent upon a debugger with this > limitation, > > -it is strongly recommended that functions not be declared static. > > diff --git a/5_source_files/56_declarations_and_types.md > b/5_source_files/56_declarations_and_types.md > index ec1803d..77e3ce9 100644 > --- a/5_source_files/56_declarations_and_types.md > +++ b/5_source_files/56_declarations_and_types.md > @@ -38,7 +38,7 @@ > Any abstract type that is defined must be constructed from other > abstract types > > or from common EFI data types. > > > > -#### 5.6.1.2 The use of int, unsigned, char, void, static, long is a > violation of the coding convention. > > +#### 5.6.1.2 The use of int, unsigned, char, void, long is a > violation of the coding convention. > > > > The corresponding EFI types must be used instead. > > > > diff --git a/README.md b/README.md > index 77cfdc8..b543bcf 100644 > --- a/README.md > +++ b/README.md > @@ -114,4 +114,5 @@ Copyright (c) 2006-2017, Intel Corporation. All > rights reserved. > | | > [#425](https://bugzilla.tianocore.org/show_bug.cgi?id=425) [CCS] > clarify line breaking and indentation requirements for multi-line > function calls | | > > | | > [#1656](https://bugzilla.tianocore.org/show_bug.cgi?id=1656) Update > all Wiki pages for the BSD+Patent license change with SPDX > identifiers | | > > | | > [#607](https://bugzilla.tianocore.org/show_bug.cgi?id=607) Document > code comment requirements for spurious variable > assignments | | > > -| 2.3 | Add 4.2 Directory names section and update File names > section for the guidelines of module directory and file > naming|September 2022|| > > +| 2.3 | Add 4.2 Directory names section and update File names > section for the guidelines of module directory and file > naming |September 2022| > > +| 2.4 | The use of the 'static' C keyword is now preferred over > the EDK2 type 'STATIC' |October 2024| > > diff --git a/book.json b/book.json > index d112b26..3ec7a93 100644 > --- a/book.json > +++ b/book.json > @@ -1,8 +1,8 @@ > { > > "variables" : { > > - "draft" : "yes", > > + "draft" : "no", > > "title" : "EDK II C Coding Standards Specification", > > - "version" : "Revision 2.2" > > + "version" : "Revision 2.4" > > }, > > "plugins": ["puml-aleung"], > > "pluginsConfig": {} > > -- > 2.46.1 > > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > View/Reply Online (#120616): https://edk2.groups.io/g/devel/message/120616 > Mute This Topic: https://groups.io/mt/108941574/1686594 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [spbrogan@outlook.com] > -=-=-=-=-=-= > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120618): https://edk2.groups.io/g/devel/message/120618 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-11 16:47 ` Rebecca Cran @ 2024-10-14 15:22 ` Oliver Smith-Denny 2024-10-14 16:47 ` Rebecca Cran 0 siblings, 1 reply; 17+ messages in thread From: Oliver Smith-Denny @ 2024-10-14 15:22 UTC (permalink / raw) To: devel, rebecca, Sean Brogan, Michael Kinney On 10/11/2024 9:47 AM, Rebecca Cran wrote: > I don't know, but I saw a message from Mike recently saying that someone > should use 'static' instead of 'STATIC' - so we need to make a clear > decision one way or the other. > > The problem here is for GoogleTest. In CMocka, if you want to unit test a static function, regardless of STATIC vs static, you can include the C file under test in the CMocka test file. Not necessarily the most elegant solution, but it works. In GoogleTest, you often cannot directly include the C file in the GoogleTest file, because C++ complains about many Cisms, mostly our use of casting. And I definitely don't think we should update the entire codebase to support compiling under C++ :). Mike, Sean, and I had a discussion on a Project Mu PR where I undef'd STATIC while building GoogleTest modules so that static functions could be unit tested there, too. Also not the most elegant of solutions. If we switch to "static", we lose the ability to unit test static functions under GoogleTest in many cases. I don't know if GoogleTest has some other solution for this, but I don't think so. It may be we say, yep, that's the limitation of GoogleTest, add it to the UnitTestFrameworkPkg README, if you need to unit test a static function, do it in CMocka. But that seems lame, also. I do believe that unit testing static functions is critical. We should be doing interface level testing, as well, where static functions are hidden from us, but there is great value in ensuring each piece of your code is working as intended from the inside. Mike, do you have further thoughts here? Thanks, Oliver -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120620): https://edk2.groups.io/g/devel/message/120620 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-14 15:22 ` Oliver Smith-Denny @ 2024-10-14 16:47 ` Rebecca Cran 2024-10-14 17:09 ` Oliver Smith-Denny 0 siblings, 1 reply; 17+ messages in thread From: Rebecca Cran @ 2024-10-14 16:47 UTC (permalink / raw) To: Oliver Smith-Denny, devel, Sean Brogan, Michael Kinney On 10/14/24 9:22 AM, Oliver Smith-Denny wrote: > In GoogleTest, you often cannot directly include the C file in the > GoogleTest file, because C++ complains about many Cisms, mostly > our use of casting. Do you have an example of that? Also, I've seen suggestions (I think on the FreeBSD mailing lists) of using C++ as "a better C". That is, not using any C++ specific features but building the code as C++ because apparently you get better code checking and diagnostics with it. Though you do lose some features like implicit casts from void* when using C++. Rebecca -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120621): https://edk2.groups.io/g/devel/message/120621 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-14 16:47 ` Rebecca Cran @ 2024-10-14 17:09 ` Oliver Smith-Denny 0 siblings, 0 replies; 17+ messages in thread From: Oliver Smith-Denny @ 2024-10-14 17:09 UTC (permalink / raw) To: devel, rebecca, Sean Brogan, Michael Kinney On 10/14/2024 9:47 AM, Rebecca Cran wrote: > On 10/14/24 9:22 AM, Oliver Smith-Denny wrote: >> In GoogleTest, you often cannot directly include the C file in the >> GoogleTest file, because C++ complains about many Cisms, mostly >> our use of casting. > > Do you have an example of that? This was a little while back, so my only memory is of the Mu code I updated the STATIC undef'ing for, one of these two: https://github.com/microsoft/mu_plus/blob/dev/202405/AdvLoggerPkg/AdvLoggerOsConnectorPrm/GoogleTest/AdvLoggerOsConnectorPrmGoogleTest.cpp https://github.com/microsoft/mu_plus/blob/dev/202405/AdvLoggerPkg/AdvLoggerOsConnectorPrm/Library/AdvLoggerOsConnectorPrmConfigLib/GoogleTest/AdvLoggerPrmConfigLibGoogleTest.cpp We had issues in edk2 as well, I just don't remember specific examples. It is more likely than not that if you try to write a GoogleTest and include the C file you'll fail compilation. > > Also, I've seen suggestions (I think on the FreeBSD mailing lists) of > using C++ as "a better C". That is, not using any C++ specific features > but building the code as C++ because apparently you get better code > checking and diagnostics with it. Though you do lose some features like > implicit casts from void* when using C++. I would take umbrage with the statement that C++ is a better C :). I do not think we gain value and in fact lose quite a lot of things, name mangling, lots of churn, etc. TBH, I have not been convinced of the value of adding C++ into edk2 with GoogleTest, I think it has added complications and very few new edk2 unit tests. Hopefully platform owners are getting good usage out of it. Oliver -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120622): https://edk2.groups.io/g/devel/message/120622 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-11 1:20 [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` Rebecca Cran 2024-10-11 3:41 ` Sean @ 2024-10-15 21:12 ` Michael D Kinney 2024-10-21 14:49 ` Rebecca Cran 1 sibling, 1 reply; 17+ messages in thread From: Michael D Kinney @ 2024-10-15 21:12 UTC (permalink / raw) To: Rebecca Cran, devel@edk2.groups.io; +Cc: Kinney, Michael D The draft status is never disabled in the master branch. Please see the release process for documents. https://github.com/tianocore-docs/edk2-TemplateSpecification/wiki/TianoCore-Documents-Releasing Document changes should be made on the master branch, and then the release branch is created with no content changes. Mike > -----Original Message----- > From: Rebecca Cran <rebecca@bsdio.com> > Sent: Thursday, October 10, 2024 6:21 PM > To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io > Cc: Rebecca Cran <rebecca@bsdio.com> > Subject: [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of > `static` C keyword over EDK2 type `STATIC` > > A while ago a decision was made on the edk2-devel mailing list that > the `STATIC` EDK2 type should be replaced with the C keyword `static`. > > Update the Coding Specification to match. While here, remove the > outdated section disallowing static functions since they're no longer > a problem. > > Bump the revision to 2.4 and turn off the draft status. > > Signed-off-by: Rebecca Cran <rebecca@bsdio.com> > --- > 5_source_files/54_code_file_structure.md | 8 +------- > 5_source_files/56_declarations_and_types.md | 2 +- > README.md | 3 ++- > book.json | 4 ++-- > 4 files changed, 6 insertions(+), 11 deletions(-) > > diff --git a/5_source_files/54_code_file_structure.md > b/5_source_files/54_code_file_structure.md > index 0c4d6a2..6decc60 100644 > --- a/5_source_files/54_code_file_structure.md > +++ b/5_source_files/54_code_file_structure.md > @@ -269,7 +269,7 @@ other than at the top level of a file as specified > by this document. > > > #### 5.4.2.2 Static > > > > -An object declared `STATIC` has either file or block scope. > > +An object declared `static` has either file or block scope. > > > > ##### 5.4.2.2.1 Do not reuse an object or function identifier with > static storage duration. > > > > @@ -277,9 +277,3 @@ Throughout the set of source files defined within a > single .inf file, do not > reuse an identifier with static storage duration. The compiler may not > be > > confused by this, but the user may confuse unrelated variables with the > same > > name. > > - > > -##### 5.4.2.2.2 Functions should not be declared STATIC. > > - > > -Some source-level debuggers are unable to resolve static functions. > Until it > > -can be verified that no one is dependent upon a debugger with this > limitation, > > -it is strongly recommended that functions not be declared static. > > diff --git a/5_source_files/56_declarations_and_types.md > b/5_source_files/56_declarations_and_types.md > index ec1803d..77e3ce9 100644 > --- a/5_source_files/56_declarations_and_types.md > +++ b/5_source_files/56_declarations_and_types.md > @@ -38,7 +38,7 @@ > Any abstract type that is defined must be constructed from other > abstract types > > or from common EFI data types. > > > > -#### 5.6.1.2 The use of int, unsigned, char, void, static, long is a > violation of the coding convention. > > +#### 5.6.1.2 The use of int, unsigned, char, void, long is a violation > of the coding convention. > > > > The corresponding EFI types must be used instead. > > > > diff --git a/README.md b/README.md > index 77cfdc8..b543bcf 100644 > --- a/README.md > +++ b/README.md > @@ -114,4 +114,5 @@ Copyright (c) 2006-2017, Intel Corporation. All > rights reserved. > | | [#425](https://bugzilla.tianocore.org/show_bug.cgi?id=425) > [CCS] clarify line breaking and indentation requirements for multi-line > function calls | | > > | | > [#1656](https://bugzilla.tianocore.org/show_bug.cgi?id=1656) Update all > Wiki pages for the BSD+Patent license change with SPDX identifiers > | | > > | | [#607](https://bugzilla.tianocore.org/show_bug.cgi?id=607) > Document code comment requirements for spurious variable assignments > | | > > -| 2.3 | Add 4.2 Directory names section and update File names > section for the guidelines of module directory and file naming|September > 2022|| > > +| 2.3 | Add 4.2 Directory names section and update File names > section for the guidelines of module directory and file naming > |September 2022| > > +| 2.4 | The use of the 'static' C keyword is now preferred over > the EDK2 type 'STATIC' > |October 2024| > > diff --git a/book.json b/book.json > index d112b26..3ec7a93 100644 > --- a/book.json > +++ b/book.json > @@ -1,8 +1,8 @@ > { > > "variables" : { > > - "draft" : "yes", > > + "draft" : "no", > > "title" : "EDK II C Coding Standards Specification", > > - "version" : "Revision 2.2" > > + "version" : "Revision 2.4" > > }, > > "plugins": ["puml-aleung"], > > "pluginsConfig": {} > > -- > 2.46.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120625): https://edk2.groups.io/g/devel/message/120625 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-15 21:12 ` Michael D Kinney @ 2024-10-21 14:49 ` Rebecca Cran 2024-10-21 15:46 ` Michael D Kinney 2024-10-21 19:42 ` Pedro Falcato 0 siblings, 2 replies; 17+ messages in thread From: Rebecca Cran @ 2024-10-21 14:49 UTC (permalink / raw) To: Kinney, Michael D, devel@edk2.groups.io Thanks, I'll fix it. Could you confirm whether the change from STATIC to static is something we want to go ahead with, or do we want to keep STATIC to allow GoogleTest to work? -- Rebecca On 10/15/24 3:12 PM, Kinney, Michael D wrote: > The draft status is never disabled in the master branch. > > Please see the release process for documents. > > https://github.com/tianocore-docs/edk2-TemplateSpecification/wiki/TianoCore-Documents-Releasing > > Document changes should be made on the master branch, and then the release branch is created with no content changes. > > Mike > >> -----Original Message----- >> From: Rebecca Cran <rebecca@bsdio.com> >> Sent: Thursday, October 10, 2024 6:21 PM >> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io >> Cc: Rebecca Cran <rebecca@bsdio.com> >> Subject: [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of >> `static` C keyword over EDK2 type `STATIC` >> >> A while ago a decision was made on the edk2-devel mailing list that >> the `STATIC` EDK2 type should be replaced with the C keyword `static`. >> >> Update the Coding Specification to match. While here, remove the >> outdated section disallowing static functions since they're no longer >> a problem. >> >> Bump the revision to 2.4 and turn off the draft status. >> >> Signed-off-by: Rebecca Cran <rebecca@bsdio.com> >> --- >> 5_source_files/54_code_file_structure.md | 8 +------- >> 5_source_files/56_declarations_and_types.md | 2 +- >> README.md | 3 ++- >> book.json | 4 ++-- >> 4 files changed, 6 insertions(+), 11 deletions(-) >> >> diff --git a/5_source_files/54_code_file_structure.md >> b/5_source_files/54_code_file_structure.md >> index 0c4d6a2..6decc60 100644 >> --- a/5_source_files/54_code_file_structure.md >> +++ b/5_source_files/54_code_file_structure.md >> @@ -269,7 +269,7 @@ other than at the top level of a file as specified >> by this document. >> >> >> #### 5.4.2.2 Static >> >> >> >> -An object declared `STATIC` has either file or block scope. >> >> +An object declared `static` has either file or block scope. >> >> >> >> ##### 5.4.2.2.1 Do not reuse an object or function identifier with >> static storage duration. >> >> >> >> @@ -277,9 +277,3 @@ Throughout the set of source files defined within a >> single .inf file, do not >> reuse an identifier with static storage duration. The compiler may not >> be >> >> confused by this, but the user may confuse unrelated variables with the >> same >> >> name. >> >> - >> >> -##### 5.4.2.2.2 Functions should not be declared STATIC. >> >> - >> >> -Some source-level debuggers are unable to resolve static functions. >> Until it >> >> -can be verified that no one is dependent upon a debugger with this >> limitation, >> >> -it is strongly recommended that functions not be declared static. >> >> diff --git a/5_source_files/56_declarations_and_types.md >> b/5_source_files/56_declarations_and_types.md >> index ec1803d..77e3ce9 100644 >> --- a/5_source_files/56_declarations_and_types.md >> +++ b/5_source_files/56_declarations_and_types.md >> @@ -38,7 +38,7 @@ >> Any abstract type that is defined must be constructed from other >> abstract types >> >> or from common EFI data types. >> >> >> >> -#### 5.6.1.2 The use of int, unsigned, char, void, static, long is a >> violation of the coding convention. >> >> +#### 5.6.1.2 The use of int, unsigned, char, void, long is a violation >> of the coding convention. >> >> >> >> The corresponding EFI types must be used instead. >> >> >> >> diff --git a/README.md b/README.md >> index 77cfdc8..b543bcf 100644 >> --- a/README.md >> +++ b/README.md >> @@ -114,4 +114,5 @@ Copyright (c) 2006-2017, Intel Corporation. All >> rights reserved. >> | | [#425](https://bugzilla.tianocore.org/show_bug.cgi?id=425) >> [CCS] clarify line breaking and indentation requirements for multi-line >> function calls | | >> >> | | >> [#1656](https://bugzilla.tianocore.org/show_bug.cgi?id=1656) Update all >> Wiki pages for the BSD+Patent license change with SPDX identifiers >> | | >> >> | | [#607](https://bugzilla.tianocore.org/show_bug.cgi?id=607) >> Document code comment requirements for spurious variable assignments >> | | >> >> -| 2.3 | Add 4.2 Directory names section and update File names >> section for the guidelines of module directory and file naming|September >> 2022|| >> >> +| 2.3 | Add 4.2 Directory names section and update File names >> section for the guidelines of module directory and file naming >> |September 2022| >> >> +| 2.4 | The use of the 'static' C keyword is now preferred over >> the EDK2 type 'STATIC' >> |October 2024| >> >> diff --git a/book.json b/book.json >> index d112b26..3ec7a93 100644 >> --- a/book.json >> +++ b/book.json >> @@ -1,8 +1,8 @@ >> { >> >> "variables" : { >> >> - "draft" : "yes", >> >> + "draft" : "no", >> >> "title" : "EDK II C Coding Standards Specification", >> >> - "version" : "Revision 2.2" >> >> + "version" : "Revision 2.4" >> >> }, >> >> "plugins": ["puml-aleung"], >> >> "pluginsConfig": {} >> >> -- >> 2.46.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120633): https://edk2.groups.io/g/devel/message/120633 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-21 14:49 ` Rebecca Cran @ 2024-10-21 15:46 ` Michael D Kinney 2024-10-21 19:42 ` Pedro Falcato 1 sibling, 0 replies; 17+ messages in thread From: Michael D Kinney @ 2024-10-21 15:46 UTC (permalink / raw) To: Rebecca Cran, devel@edk2.groups.io; +Cc: Kinney, Michael D This needs further discussion. Before GoogleTest was added, there was a clear direction to move from STATIC->static. The issue at hand is really a C++/C compatibility issue. We need to enumerate the full set of issues and see if there are reasonable ways to address. Mike > -----Original Message----- > From: Rebecca Cran <rebecca@bsdio.com> > Sent: Monday, October 21, 2024 7:49 AM > To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io > Subject: Re: [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use > of `static` C keyword over EDK2 type `STATIC` > > Thanks, I'll fix it. > > Could you confirm whether the change from STATIC to static is something > we want to go ahead with, or do we want to keep STATIC to allow > GoogleTest to work? > > > -- > Rebecca > > > On 10/15/24 3:12 PM, Kinney, Michael D wrote: > > The draft status is never disabled in the master branch. > > > > Please see the release process for documents. > > > > https://github.com/tianocore-docs/edk2- > TemplateSpecification/wiki/TianoCore-Documents-Releasing > > > > Document changes should be made on the master branch, and then the > release branch is created with no content changes. > > > > Mike > > > >> -----Original Message----- > >> From: Rebecca Cran <rebecca@bsdio.com> > >> Sent: Thursday, October 10, 2024 6:21 PM > >> To: Kinney, Michael D <michael.d.kinney@intel.com>; > devel@edk2.groups.io > >> Cc: Rebecca Cran <rebecca@bsdio.com> > >> Subject: [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of > >> `static` C keyword over EDK2 type `STATIC` > >> > >> A while ago a decision was made on the edk2-devel mailing list that > >> the `STATIC` EDK2 type should be replaced with the C keyword > `static`. > >> > >> Update the Coding Specification to match. While here, remove the > >> outdated section disallowing static functions since they're no longer > >> a problem. > >> > >> Bump the revision to 2.4 and turn off the draft status. > >> > >> Signed-off-by: Rebecca Cran <rebecca@bsdio.com> > >> --- > >> 5_source_files/54_code_file_structure.md | 8 +------- > >> 5_source_files/56_declarations_and_types.md | 2 +- > >> README.md | 3 ++- > >> book.json | 4 ++-- > >> 4 files changed, 6 insertions(+), 11 deletions(-) > >> > >> diff --git a/5_source_files/54_code_file_structure.md > >> b/5_source_files/54_code_file_structure.md > >> index 0c4d6a2..6decc60 100644 > >> --- a/5_source_files/54_code_file_structure.md > >> +++ b/5_source_files/54_code_file_structure.md > >> @@ -269,7 +269,7 @@ other than at the top level of a file as > specified > >> by this document. > >> > >> > >> #### 5.4.2.2 Static > >> > >> > >> > >> -An object declared `STATIC` has either file or block scope. > >> > >> +An object declared `static` has either file or block scope. > >> > >> > >> > >> ##### 5.4.2.2.1 Do not reuse an object or function identifier with > >> static storage duration. > >> > >> > >> > >> @@ -277,9 +277,3 @@ Throughout the set of source files defined within > a > >> single .inf file, do not > >> reuse an identifier with static storage duration. The compiler may > not > >> be > >> > >> confused by this, but the user may confuse unrelated variables with > the > >> same > >> > >> name. > >> > >> - > >> > >> -##### 5.4.2.2.2 Functions should not be declared STATIC. > >> > >> - > >> > >> -Some source-level debuggers are unable to resolve static functions. > >> Until it > >> > >> -can be verified that no one is dependent upon a debugger with this > >> limitation, > >> > >> -it is strongly recommended that functions not be declared static. > >> > >> diff --git a/5_source_files/56_declarations_and_types.md > >> b/5_source_files/56_declarations_and_types.md > >> index ec1803d..77e3ce9 100644 > >> --- a/5_source_files/56_declarations_and_types.md > >> +++ b/5_source_files/56_declarations_and_types.md > >> @@ -38,7 +38,7 @@ > >> Any abstract type that is defined must be constructed from other > >> abstract types > >> > >> or from common EFI data types. > >> > >> > >> > >> -#### 5.6.1.2 The use of int, unsigned, char, void, static, long is a > >> violation of the coding convention. > >> > >> +#### 5.6.1.2 The use of int, unsigned, char, void, long is a > violation > >> of the coding convention. > >> > >> > >> > >> The corresponding EFI types must be used instead. > >> > >> > >> > >> diff --git a/README.md b/README.md > >> index 77cfdc8..b543bcf 100644 > >> --- a/README.md > >> +++ b/README.md > >> @@ -114,4 +114,5 @@ Copyright (c) 2006-2017, Intel Corporation. All > >> rights reserved. > >> | | > [#425](https://bugzilla.tianocore.org/show_bug.cgi?id=425) > >> [CCS] clarify line breaking and indentation requirements for multi- > line > >> function calls | | > >> > >> | | > >> [#1656](https://bugzilla.tianocore.org/show_bug.cgi?id=1656) Update > all > >> Wiki pages for the BSD+Patent license change with SPDX identifiers > >> | | > >> > >> | | > [#607](https://bugzilla.tianocore.org/show_bug.cgi?id=607) > >> Document code comment requirements for spurious variable assignments > >> | | > >> > >> -| 2.3 | Add 4.2 Directory names section and update File names > >> section for the guidelines of module directory and file > naming|September > >> 2022|| > >> > >> +| 2.3 | Add 4.2 Directory names section and update File names > >> section for the guidelines of module directory and file naming > >> |September 2022| > >> > >> +| 2.4 | The use of the 'static' C keyword is now preferred over > >> the EDK2 type 'STATIC' > >> |October 2024| > >> > >> diff --git a/book.json b/book.json > >> index d112b26..3ec7a93 100644 > >> --- a/book.json > >> +++ b/book.json > >> @@ -1,8 +1,8 @@ > >> { > >> > >> "variables" : { > >> > >> - "draft" : "yes", > >> > >> + "draft" : "no", > >> > >> "title" : "EDK II C Coding Standards Specification", > >> > >> - "version" : "Revision 2.2" > >> > >> + "version" : "Revision 2.4" > >> > >> }, > >> > >> "plugins": ["puml-aleung"], > >> > >> "pluginsConfig": {} > >> > >> -- > >> 2.46.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120634): https://edk2.groups.io/g/devel/message/120634 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-21 14:49 ` Rebecca Cran 2024-10-21 15:46 ` Michael D Kinney @ 2024-10-21 19:42 ` Pedro Falcato 2024-10-21 20:05 ` Oliver Smith-Denny 1 sibling, 1 reply; 17+ messages in thread From: Pedro Falcato @ 2024-10-21 19:42 UTC (permalink / raw) To: devel, rebecca; +Cc: Kinney, Michael D On Mon, Oct 21, 2024 at 3:49 PM Rebecca Cran via groups.io <rebecca=bsdio.com@groups.io> wrote: > > Thanks, I'll fix it. > > Could you confirm whether the change from STATIC to static is something > we want to go ahead with, or do we want to keep STATIC to allow > GoogleTest to work? You don't need STATIC, doing stuff like -Dstatic= (or just #define static in C code) Just Works. For GCC at least. proof of horribleness: https://godbolt.org/z/EvMd6hev8 -- Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120637): https://edk2.groups.io/g/devel/message/120637 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-21 19:42 ` Pedro Falcato @ 2024-10-21 20:05 ` Oliver Smith-Denny 2024-10-21 20:21 ` Pedro Falcato 0 siblings, 1 reply; 17+ messages in thread From: Oliver Smith-Denny @ 2024-10-21 20:05 UTC (permalink / raw) To: devel, pedro.falcato, rebecca; +Cc: Kinney, Michael D On 10/21/2024 12:42 PM, Pedro Falcato wrote: > On Mon, Oct 21, 2024 at 3:49 PM Rebecca Cran via groups.io > <rebecca=bsdio.com@groups.io> wrote: >> >> Thanks, I'll fix it. >> >> Could you confirm whether the change from STATIC to static is something >> we want to go ahead with, or do we want to keep STATIC to allow >> GoogleTest to work? > > You don't need STATIC, doing stuff like -Dstatic= (or just #define > static in C code) Just Works. For GCC at least. > > proof of horribleness: https://godbolt.org/z/EvMd6hev8 > The issue here is that C uses one keyword for two distinct things: file private members and local variables that keep state across calls, i.e. real static variables. If you only have static, then #define static <nothing>, you'll mess up the local variable case under test. Don't get me wrong, I think all of this tinkering is horribleness and I think that mixing languages often brings these kind of compromises, which is unfortunate. I'm not advocating we introduce these kind of hacks, but I do think we should be able to unit test static functions, so I'd like to see a solution for that, which again, might just be CMocka. Oliver -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120638): https://edk2.groups.io/g/devel/message/120638 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-21 20:05 ` Oliver Smith-Denny @ 2024-10-21 20:21 ` Pedro Falcato 2024-10-21 20:26 ` Oliver Smith-Denny 0 siblings, 1 reply; 17+ messages in thread From: Pedro Falcato @ 2024-10-21 20:21 UTC (permalink / raw) To: Oliver Smith-Denny; +Cc: devel, rebecca, Kinney, Michael D On Mon, Oct 21, 2024 at 9:05 PM Oliver Smith-Denny <osde@linux.microsoft.com> wrote: > > On 10/21/2024 12:42 PM, Pedro Falcato wrote: > > On Mon, Oct 21, 2024 at 3:49 PM Rebecca Cran via groups.io > > <rebecca=bsdio.com@groups.io> wrote: > >> > >> Thanks, I'll fix it. > >> > >> Could you confirm whether the change from STATIC to static is something > >> we want to go ahead with, or do we want to keep STATIC to allow > >> GoogleTest to work? > > > > You don't need STATIC, doing stuff like -Dstatic= (or just #define > > static in C code) Just Works. For GCC at least. > > > > proof of horribleness: https://godbolt.org/z/EvMd6hev8 > > > > The issue here is that C uses one keyword for two distinct things: > file private members and local variables that keep state across calls, > i.e. real static variables. That's an interesting problem, but I'm afraid the current EDK2 coding style would recommend STATIC for local variables too. https://tianocore-docs.github.io/edk2-CCodingStandardsSpecification/release-2.20/5_source_files/56_declarations_and_types.html#56-declarations-and-types says: 5.6.1.2 The use of int, unsigned, char, void, static, long is a violation of the coding convention. so: INT Foo() { STATIC INT Variable = 0; return Variable++; } would be the sanctioned way to pull off this kind of stuff. So STATIC gains us nothing (neither does INT, LONG, CHAR, VOID, or CONST). -- Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120639): https://edk2.groups.io/g/devel/message/120639 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-21 20:21 ` Pedro Falcato @ 2024-10-21 20:26 ` Oliver Smith-Denny 2024-10-21 20:35 ` Michael D Kinney 0 siblings, 1 reply; 17+ messages in thread From: Oliver Smith-Denny @ 2024-10-21 20:26 UTC (permalink / raw) To: devel, pedro.falcato; +Cc: rebecca, Kinney, Michael D On 10/21/2024 1:21 PM, Pedro Falcato wrote: > On Mon, Oct 21, 2024 at 9:05 PM Oliver Smith-Denny > <osde@linux.microsoft.com> wrote: >> >> On 10/21/2024 12:42 PM, Pedro Falcato wrote: >>> On Mon, Oct 21, 2024 at 3:49 PM Rebecca Cran via groups.io >>> <rebecca=bsdio.com@groups.io> wrote: >>>> >>>> Thanks, I'll fix it. >>>> >>>> Could you confirm whether the change from STATIC to static is something >>>> we want to go ahead with, or do we want to keep STATIC to allow >>>> GoogleTest to work? >>> >>> You don't need STATIC, doing stuff like -Dstatic= (or just #define >>> static in C code) Just Works. For GCC at least. >>> >>> proof of horribleness: https://godbolt.org/z/EvMd6hev8 >>> >> >> The issue here is that C uses one keyword for two distinct things: >> file private members and local variables that keep state across calls, >> i.e. real static variables. > > That's an interesting problem, but I'm afraid the current EDK2 coding > style would recommend STATIC for local variables too. > > https://tianocore-docs.github.io/edk2-CCodingStandardsSpecification/release-2.20/5_source_files/56_declarations_and_types.html#56-declarations-and-types > says: > > 5.6.1.2 The use of int, unsigned, char, void, static, long is a > violation of the coding convention. > > so: > > INT Foo() > { > STATIC INT Variable = 0; > return Variable++; > } > > would be the sanctioned way to pull off this kind of stuff. So STATIC > gains us nothing (neither does INT, LONG, CHAR, VOID, or CONST). > Yeah, so what we did in Mu was leverage the fact that static locals are relatively uncommon and code coverage is extremely low, so that there was only one current instance of a static local under test. And we changed that to lowercase static. So now STATIC bought us that we could undef it when building HOST_APPLICATIONs. Was I happy with that approach? No. But, we were trying to evaluate the usefulness of GoogleTest and this was one issue that had to be fixed in order for us to use it. Oliver -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120640): https://edk2.groups.io/g/devel/message/120640 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-21 20:26 ` Oliver Smith-Denny @ 2024-10-21 20:35 ` Michael D Kinney 2024-10-21 20:37 ` Oliver Smith-Denny 0 siblings, 1 reply; 17+ messages in thread From: Michael D Kinney @ 2024-10-21 20:35 UTC (permalink / raw) To: Oliver Smith-Denny, devel@edk2.groups.io, pedro.falcato@gmail.com Cc: rebecca@bsdio.com, Kinney, Michael D Would it help if code was modified to completely remove the use of static in local variables and enforce that going forward and limit use of static to functions and global variables? Mike > -----Original Message----- > From: Oliver Smith-Denny <osde@linux.microsoft.com> > Sent: Monday, October 21, 2024 1:26 PM > To: devel@edk2.groups.io; pedro.falcato@gmail.com > Cc: rebecca@bsdio.com; Kinney, Michael D <michael.d.kinney@intel.com> > Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] > Prefer use of `static` C keyword over EDK2 type `STATIC` > > On 10/21/2024 1:21 PM, Pedro Falcato wrote: > > On Mon, Oct 21, 2024 at 9:05 PM Oliver Smith-Denny > > <osde@linux.microsoft.com> wrote: > >> > >> On 10/21/2024 12:42 PM, Pedro Falcato wrote: > >>> On Mon, Oct 21, 2024 at 3:49 PM Rebecca Cran via groups.io > >>> <rebecca=bsdio.com@groups.io> wrote: > >>>> > >>>> Thanks, I'll fix it. > >>>> > >>>> Could you confirm whether the change from STATIC to static is > something > >>>> we want to go ahead with, or do we want to keep STATIC to allow > >>>> GoogleTest to work? > >>> > >>> You don't need STATIC, doing stuff like -Dstatic= (or just #define > >>> static in C code) Just Works. For GCC at least. > >>> > >>> proof of horribleness: https://godbolt.org/z/EvMd6hev8 > >>> > >> > >> The issue here is that C uses one keyword for two distinct things: > >> file private members and local variables that keep state across > calls, > >> i.e. real static variables. > > > > That's an interesting problem, but I'm afraid the current EDK2 coding > > style would recommend STATIC for local variables too. > > > > https://tianocore-docs.github.io/edk2- > CCodingStandardsSpecification/release- > 2.20/5_source_files/56_declarations_and_types.html#56-declarations-and- > types > > says: > > > > 5.6.1.2 The use of int, unsigned, char, void, static, long is a > > violation of the coding convention. > > > > so: > > > > INT Foo() > > { > > STATIC INT Variable = 0; > > return Variable++; > > } > > > > would be the sanctioned way to pull off this kind of stuff. So STATIC > > gains us nothing (neither does INT, LONG, CHAR, VOID, or CONST). > > > > Yeah, so what we did in Mu was leverage the fact that static locals are > relatively uncommon and code coverage is extremely low, so that there > was only one current instance of a static local under test. And we > changed that to lowercase static. So now STATIC bought us that we > could undef it when building HOST_APPLICATIONs. > > Was I happy with that approach? No. But, we were trying to evaluate > the usefulness of GoogleTest and this was one issue that had to be > fixed in order for us to use it. > > Oliver -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120641): https://edk2.groups.io/g/devel/message/120641 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-21 20:35 ` Michael D Kinney @ 2024-10-21 20:37 ` Oliver Smith-Denny 2024-10-21 21:04 ` Michael D Kinney 0 siblings, 1 reply; 17+ messages in thread From: Oliver Smith-Denny @ 2024-10-21 20:37 UTC (permalink / raw) To: Kinney, Michael D, devel@edk2.groups.io, pedro.falcato@gmail.com Cc: rebecca@bsdio.com On 10/21/2024 1:35 PM, Kinney, Michael D wrote: > Would it help if code was modified to completely remove the use of > static in local variables and enforce that going forward and limit > use of static to functions and global variables? > We'd have to test further, but I believe that would allow Pedro's scheme of simply `#define static <nothing>` to work. I ran a quick godbolt test and it seems MSVC is also okay with that. Oliver -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120642): https://edk2.groups.io/g/devel/message/120642 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-21 20:37 ` Oliver Smith-Denny @ 2024-10-21 21:04 ` Michael D Kinney 2024-10-21 21:11 ` Pedro Falcato 0 siblings, 1 reply; 17+ messages in thread From: Michael D Kinney @ 2024-10-21 21:04 UTC (permalink / raw) To: Oliver Smith-Denny, devel@edk2.groups.io, pedro.falcato@gmail.com Cc: rebecca@bsdio.com, Kinney, Michael D I recall clang having more restrictions on redefining keywords. Mike > -----Original Message----- > From: Oliver Smith-Denny <osde@linux.microsoft.com> > Sent: Monday, October 21, 2024 1:38 PM > To: Kinney, Michael D <michael.d.kinney@intel.com>; > devel@edk2.groups.io; pedro.falcato@gmail.com > Cc: rebecca@bsdio.com > Subject: Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] > Prefer use of `static` C keyword over EDK2 type `STATIC` > > On 10/21/2024 1:35 PM, Kinney, Michael D wrote: > > Would it help if code was modified to completely remove the use of > > static in local variables and enforce that going forward and limit > > use of static to functions and global variables? > > > > We'd have to test further, but I believe that would allow Pedro's > scheme of simply `#define static <nothing>` to work. I ran a > quick godbolt test and it seems MSVC is also okay with that. > > Oliver -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120643): https://edk2.groups.io/g/devel/message/120643 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` 2024-10-21 21:04 ` Michael D Kinney @ 2024-10-21 21:11 ` Pedro Falcato 0 siblings, 0 replies; 17+ messages in thread From: Pedro Falcato @ 2024-10-21 21:11 UTC (permalink / raw) To: Kinney, Michael D Cc: Oliver Smith-Denny, devel@edk2.groups.io, rebecca@bsdio.com On Mon, Oct 21, 2024 at 10:04 PM Kinney, Michael D <michael.d.kinney@intel.com> wrote: > > I recall clang having more restrictions on redefining keywords. Clang has restrictions on redefining macros and builtin functions. It appears to not have any issues doing horrible unspeakable things with the C keywords and the C preprocessor: https://godbolt.org/z/eT6hWsEG5 -- Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#120644): https://edk2.groups.io/g/devel/message/120644 Mute This Topic: https://groups.io/mt/108941574/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=- ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2024-10-21 21:11 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-11 1:20 [edk2-devel] [edk2-CCodingStandardsSpecification PATCH 1/1] Prefer use of `static` C keyword over EDK2 type `STATIC` Rebecca Cran 2024-10-11 3:41 ` Sean 2024-10-11 16:47 ` Rebecca Cran 2024-10-14 15:22 ` Oliver Smith-Denny 2024-10-14 16:47 ` Rebecca Cran 2024-10-14 17:09 ` Oliver Smith-Denny 2024-10-15 21:12 ` Michael D Kinney 2024-10-21 14:49 ` Rebecca Cran 2024-10-21 15:46 ` Michael D Kinney 2024-10-21 19:42 ` Pedro Falcato 2024-10-21 20:05 ` Oliver Smith-Denny 2024-10-21 20:21 ` Pedro Falcato 2024-10-21 20:26 ` Oliver Smith-Denny 2024-10-21 20:35 ` Michael D Kinney 2024-10-21 20:37 ` Oliver Smith-Denny 2024-10-21 21:04 ` Michael D Kinney 2024-10-21 21:11 ` Pedro Falcato
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox