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

[clang] Use std::optional::value_or (NFC) #109894

Conversation

kazutakahirata
Copy link
Contributor

No description provided.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Sep 25, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 25, 2024

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Kazu Hirata (kazutakahirata)

Changes

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

3 Files Affected:

  • (modified) clang/include/clang/AST/PropertiesBase.td (+1-1)
  • (modified) clang/include/clang/Basic/FileManager.h (+1-1)
  • (modified) clang/lib/APINotes/APINotesYAMLCompiler.cpp (+2-2)
diff --git a/clang/include/clang/AST/PropertiesBase.td b/clang/include/clang/AST/PropertiesBase.td
index 9b934b20cf2559..3057669e3758b5 100644
--- a/clang/include/clang/AST/PropertiesBase.td
+++ b/clang/include/clang/AST/PropertiesBase.td
@@ -39,7 +39,7 @@ class EnumPropertyType<string typeName = ""> : PropertyType<typeName> {}
 /// Supports optional values by using the null representation.
 class RefPropertyType<string className> : PropertyType<className # "*"> {
   let PackOptional =
-    "value ? *value : nullptr";
+    "value.value_or(nullptr)";
   let UnpackOptional =
     "value ? std::optional<" # CXXName # ">(value) : std::nullopt";
 }
diff --git a/clang/include/clang/Basic/FileManager.h b/clang/include/clang/Basic/FileManager.h
index 527bbef24793ee..74029a91d1a6d0 100644
--- a/clang/include/clang/Basic/FileManager.h
+++ b/clang/include/clang/Basic/FileManager.h
@@ -293,7 +293,7 @@ class FileManager : public RefCountedBase<FileManager> {
                    bool RequiresNullTerminator = true,
                    std::optional<int64_t> MaybeLimit = std::nullopt) const {
     return getBufferForFileImpl(Filename,
-                                /*FileSize=*/(MaybeLimit ? *MaybeLimit : -1),
+                                /*FileSize=*/MaybeLimit.value_or(-1),
                                 isVolatile, RequiresNullTerminator);
   }
 
diff --git a/clang/lib/APINotes/APINotesYAMLCompiler.cpp b/clang/lib/APINotes/APINotesYAMLCompiler.cpp
index 16fd59244086fd..f72a1d65b5456f 100644
--- a/clang/lib/APINotes/APINotesYAMLCompiler.cpp
+++ b/clang/lib/APINotes/APINotesYAMLCompiler.cpp
@@ -757,8 +757,8 @@ class YAMLConverter {
       OutInfo.addTypeInfo(idx++, N);
     audited = Nullability.size() > 0 || ReturnNullability;
     if (audited)
-      OutInfo.addTypeInfo(0, ReturnNullability ? *ReturnNullability
-                                               : NullabilityKind::NonNull);
+      OutInfo.addTypeInfo(0,
+                          ReturnNullability.value_or(NullabilityKind::NonNull));
     if (!audited)
       return;
     OutInfo.NullabilityAudited = audited;

@kazutakahirata kazutakahirata merged commit 416f101 into llvm:main Sep 25, 2024
8 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_001_std_optional_value_or_clang branch September 25, 2024 06:01
augusto2112 pushed a commit to augusto2112/llvm-project that referenced this pull request Sep 26, 2024
@shafik
Copy link
Collaborator

shafik commented Sep 27, 2024

Next time please provide a better summary for your PR. This is super important for downstream folks debugging build breaks. In general this is important for reviews to compare what they expect with the actual diff.

In this case something like "replace the use of conditional operator on std::optional w/ value_or" would have been sufficient.

Thank you

@kazutakahirata
Copy link
Contributor Author

Next time please provide a better summary for your PR. This is super important for downstream folks debugging build breaks. In general this is important for reviews to compare what they expect with the actual diff.

In this case something like "replace the use of conditional operator on std::optional w/ value_or" would have been sufficient.

Thank you

Ack.

xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants