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

[libc] added grouping of guarded functions in header generation #98532

Merged
merged 4 commits into from
Jul 12, 2024

Conversation

RoseZhang03
Copy link
Contributor

Instead of #ifdef guards for each individual function, #ifdef and #endif
will surround all functions that have the same guard.

@llvmbot llvmbot added the libc label Jul 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 11, 2024

@llvm/pr-subscribers-libc

Author: None (RoseZhang03)

Changes

Instead of #ifdef guards for each individual function, #ifdef and #endif
will surround all functions that have the same guard.


Full diff: https://github.com/llvm/llvm-project/pull/98532.diff

2 Files Affected:

  • (modified) libc/newhdrgen/class_implementation/classes/function.py (-2)
  • (modified) libc/newhdrgen/header.py (+27-1)
diff --git a/libc/newhdrgen/class_implementation/classes/function.py b/libc/newhdrgen/class_implementation/classes/function.py
index da26358e74506..ea5e8223a538e 100644
--- a/libc/newhdrgen/class_implementation/classes/function.py
+++ b/libc/newhdrgen/class_implementation/classes/function.py
@@ -29,6 +29,4 @@ def __str__(self):
             result = f"{self.return_type} {self.name}({arguments_str}) __NOEXCEPT;"
         else:
             result = f"{attributes_str} {self.return_type} {self.name}({arguments_str}) __NOEXCEPT;"
-        if self.guard:
-            result = f"#ifdef {self.guard}\n{result}\n#endif // {self.guard}"
         return result
diff --git a/libc/newhdrgen/header.py b/libc/newhdrgen/header.py
index d9e9c68dfc5f4..eb8199550ba33 100644
--- a/libc/newhdrgen/header.py
+++ b/libc/newhdrgen/header.py
@@ -57,9 +57,35 @@ def __str__(self):
 
         content.append("\n__BEGIN_C_DECLS\n")
 
+        current_guard = None
         for function in self.functions:
-            content.append(str(function))
+            # due to sorting, all guarded functions will be at the end
+            if function.guard == None:
+                content.append(str(function))
+                content.append("")
+            else:
+                if current_guard == None:
+                    current_guard = function.guard
+                    content.append(f"#ifdef {current_guard}")
+                    content.append(str(function))
+                    content.append("")
+                elif current_guard == function.guard:
+                    content.append(str(function))
+                    content.append("")
+                else:
+                    content.pop()
+                    content.append(f"#endif // {current_guard}")
+                    content.append("")
+                    current_guard = function.guard
+                    content.append(f"#ifdef {current_guard}")
+                    content.append(str(function))
+                    content.append("")
+        if current_guard != None:
+            content.pop()
+            content.append(f"#endif // {current_guard}")
             content.append("")
+                                        
+
         for object in self.objects:
             content.append(str(object))
         content.append("__END_C_DECLS")

Copy link

github-actions bot commented Jul 11, 2024

⚠️ Python code formatter, darker found issues in your code. ⚠️

You can test this locally with the following command:
darker --check --diff -r ac304d5ffdea7676582e987f75caeba33355a5dd...da8c625ae8da7ebf7bf8e0162e4deee9b624a19a libc/newhdrgen/class_implementation/classes/function.py libc/newhdrgen/header.py
View the diff from darker here.
--- header.py	2024-07-12 17:39:26.000000 +0000
+++ header.py	2024-07-12 17:43:16.762413 +0000
@@ -80,11 +80,11 @@
                     content.append(str(function))
                     content.append("")
         if current_guard != None:
             content.pop()
             content.append(f"#endif // {current_guard}")
-            content.append("")      
+            content.append("")
 
         for object in self.objects:
             content.append(str(object))
         if self.objects:
             content.append("\n__END_C_DECLS")

Copy link
Contributor

@aaryanshukla aaryanshukla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this has been tested on guarded files then LGTM!

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code looks good, but you should add tests to check that it works before landing it

Instead of #ifdef guards for each individual function, #ifdef and #endif
will surround all functions that have the same guard.
@RoseZhang03 RoseZhang03 merged commit e3bd43b into llvm:main Jul 12, 2024
3 of 5 checks passed
@RoseZhang03 RoseZhang03 deleted the rosezhang11 branch July 12, 2024 18:07
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
…#98532)

Instead of #ifdef guards for each individual function, #ifdef and #endif
will surround all functions that have the same guard.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants