Skip to content

[libc][newheadergen]: yaml.load instead of safe_load #100024

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

Merged
merged 2 commits into from
Jul 23, 2024

Conversation

aaryanshukla
Copy link
Contributor

No description provided.

@llvmbot llvmbot added the libc label Jul 23, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 23, 2024

@llvm/pr-subscribers-libc

Author: None (aaryanshukla)

Changes

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

2 Files Affected:

  • (modified) libc/CMakeLists.txt (+1-1)
  • (modified) libc/newhdrgen/yaml_to_classes.py (+2-3)
diff --git a/libc/CMakeLists.txt b/libc/CMakeLists.txt
index 6e0760724d963..45cca17562d26 100644
--- a/libc/CMakeLists.txt
+++ b/libc/CMakeLists.txt
@@ -73,7 +73,7 @@ if(LIBC_BUILD_GPU_LOADER OR (LLVM_LIBC_GPU_BUILD AND NOT LLVM_RUNTIMES_BUILD))
   add_subdirectory(utils/gpu)
 endif()
 
-option(LIBC_USE_NEW_HEADER_GEN "Generate header files using new headergen instead of the old one" OFF)
+option(LIBC_USE_NEW_HEADER_GEN "Generate header files using new headergen instead of the old one" ON)
 
 set(NEED_LIBC_HDRGEN FALSE)
 if(NOT LLVM_RUNTIMES_BUILD)
diff --git a/libc/newhdrgen/yaml_to_classes.py b/libc/newhdrgen/yaml_to_classes.py
index 78588055cc960..d3bdfde2e2a22 100644
--- a/libc/newhdrgen/yaml_to_classes.py
+++ b/libc/newhdrgen/yaml_to_classes.py
@@ -118,7 +118,7 @@ def load_yaml_file(yaml_file, header_class, entry_points):
         HeaderFile: An instance of HeaderFile populated with the data.
     """
     with open(yaml_file, "r") as f:
-        yaml_data = yaml.safe_load(f)
+        yaml_data = yaml.load(f, Loader=yaml.FullLoader)
     return yaml_to_classes(yaml_data, header_class, entry_points)
 
 
@@ -173,8 +173,7 @@ def add_function_to_yaml(yaml_file, function_details):
     new_function = parse_function_details(function_details)
 
     with open(yaml_file, "r") as f:
-        yaml_data = yaml.safe_load(f)
-
+        yaml_data = yaml.load(f, Loader=yaml.FullLoader)
     if "functions" not in yaml_data:
         yaml_data["functions"] = []
 

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, but changing the default to new headergen should probably be in a separate patch since that may need to be reverted again. Also the description should explain that this is for python 3.8 support

@RoseZhang03 RoseZhang03 merged commit ba0744e into llvm:main Jul 23, 2024
4 of 5 checks passed
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary: Co-authored-by: Rose Zhang <[email protected]>

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251226
michaelrj-google added a commit to michaelrj-google/llvm-project that referenced this pull request Aug 20, 2024
In llvm#100024 we moved from safe_load to load for reading the yaml in
newheadergen due to dependency issues. Those should be resolved by now
so this should be a simple safety improvement.
michaelrj-google added a commit that referenced this pull request Aug 20, 2024
In #100024 we moved from safe_load to load for reading the yaml in
newheadergen due to dependency issues. Those should be resolved by now
so this should be a simple safety improvement.
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.

4 participants