Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3C adds unnecessary initializers to global variables #741

Open
mattmccutchen-cci opened this issue Nov 16, 2021 · 0 comments
Open

3C adds unnecessary initializers to global variables #741

mattmccutchen-cci opened this issue Nov 16, 2021 · 0 comments

Comments

@mattmccutchen-cci
Copy link
Member

3C adds initializers to global variables that contain checked pointers, but these initializers are not required by Checked C because global variables are implicitly zero-initialized. 3C should not make unnecessary changes to the user's code.

Here's an example. At file scope, the following:

int *x;

becomes:

_Ptr<int> x = ((void *)0);

It should just be:

_Ptr<int> x;

The code fix is trivial:

diff --git a/clang/lib/3C/DeclRewriter.cpp b/clang/lib/3C/DeclRewriter.cpp
index 72ae4289df83..703931b758d0 100644
--- a/clang/lib/3C/DeclRewriter.cpp
+++ b/clang/lib/3C/DeclRewriter.cpp
@@ -445,7 +445,7 @@ void DeclRewriter::doDeclRewrite(SourceRange &SR, DeclReplacement *N) {
       // There is no initializer. Add it if we need one.
       // MWH -- Solves issue 43. Should make it so we insert NULL if stdlib.h or
       // stdlib_checked.h is included
-      if (VD->getStorageClass() != StorageClass::SC_Extern) {
+      if (VD->hasLocalStorage()) {
         const std::string NullPtrStr = "((void *)0)";
         if (isPointerType(VD)) {
           Replacement += " = " + NullPtrStr;

In fact, #657 is already making the corresponding change to StructInit because other cleanups to StructInit in #657 provided a natural excuse to do so. However, I'm holding off on making the corresponding change for plain pointer variables (either in #657 or as a separate PR) because of the amount of churn it will introduce in the regression tests and the potential for conflicts with other pending PRs.

This change would still leave us with the VD->hasLocalStorage() condition duplicated in DeclRewriter::doDeclRewrite and StructInit. #645 (comment) envisions a full centralization of the initializer addition logic that would avoid that duplication. However, the change proposed here is easy enough and has enough benefit that it may be worth doing first by itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant