Skip to content

[clang][NFC] fix name lookup for llvm::json::Value in SymbolGraphSerializer #94511

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

Conversation

yuxuanchen1997
Copy link
Member

@yuxuanchen1997 yuxuanchen1997 commented Jun 5, 2024

This code uses namespaces llvm and llvm::json. However, we have both llvm::Value and llvm::json::Value. Whenever any of the headers declare or include llvm::Value, the lookup becomes ambiguous.

Fixing this by qualifying the Value type.

@yuxuanchen1997 yuxuanchen1997 requested review from zixu-w and daniel-grumberg and removed request for daniel-grumberg June 5, 2024 18:06
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Jun 5, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 5, 2024

@llvm/pr-subscribers-clang

Author: Yuxuan Chen (yuxuanchen1997)

Changes

This code uses two namespaces, llvm and llvm::json. However, we have both llvm::Value and llvm::json::Value. Whenever any of the headers declare or include llvm::Value, the lookup becomes ambiguous.

Fixing this by qualifying the Value type.


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

1 Files Affected:

  • (modified) clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp (+2-3)
diff --git a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
index 08e711cafae28..6e56ee5b573f6 100644
--- a/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
+++ b/clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
@@ -31,7 +31,6 @@
 using namespace clang;
 using namespace clang::extractapi;
 using namespace llvm;
-using namespace llvm::json;
 
 namespace {
 
@@ -1036,9 +1035,9 @@ void SymbolGraphSerializer::serializeGraphToStream(
     ExtendedModule &&EM) {
   Object Root = serializeGraph(ModuleName, std::move(EM));
   if (Options.Compact)
-    OS << formatv("{0}", Value(std::move(Root))) << "\n";
+    OS << formatv("{0}", json::Value(std::move(Root))) << "\n";
   else
-    OS << formatv("{0:2}", Value(std::move(Root))) << "\n";
+    OS << formatv("{0:2}", json::Value(std::move(Root))) << "\n";
 }
 
 void SymbolGraphSerializer::serializeMainSymbolGraph(

Copy link
Contributor

@apolloww apolloww left a comment

Choose a reason for hiding this comment

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

LGTM.

@yuxuanchen1997 yuxuanchen1997 merged commit f078548 into llvm:main Jun 6, 2024
9 checks passed
@yuxuanchen1997 yuxuanchen1997 deleted the fix-name-lookup-symbol-graph-serializer branch June 6, 2024 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants