llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clangd

Author: kadir çetinkaya (kadircet)

<details>
<summary>Changes</summary>

Fixes https://github.com/llvm/llvm-project/issues/113926.
Fixes https://github.com/llvm/llvm-project/issues/63976.


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


11 Files Affected:

- (modified) clang-tools-extra/clangd/Hover.cpp (+3-2) 
- (modified) clang-tools-extra/clangd/index/SymbolCollector.cpp (+1-1) 
- (modified) 
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h 
(+1-1) 
- (modified) clang-tools-extra/include-cleaner/lib/Analysis.cpp (+2-2) 
- (modified) clang-tools-extra/include-cleaner/lib/AnalysisInternal.h (+5-2) 
- (modified) clang-tools-extra/include-cleaner/lib/FindHeaders.cpp (+4-2) 
- (modified) clang-tools-extra/include-cleaner/lib/HTMLReport.cpp (+10-8) 
- (modified) clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp (+8-4) 
- (modified) clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp (+3-4) 
- (modified) clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp 
(+29-6) 
- (modified) clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp 
(+11-8) 


``````````diff
diff --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index 5e136d0e76ece7..3ab3d890305204 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -1193,12 +1193,13 @@ void maybeAddSymbolProviders(ParsedAST &AST, HoverInfo 
&HI,
                              include_cleaner::Symbol Sym) {
   trace::Span Tracer("Hover::maybeAddSymbolProviders");
 
-  const SourceManager &SM = AST.getSourceManager();
   llvm::SmallVector<include_cleaner::Header> RankedProviders =
-      include_cleaner::headersForSymbol(Sym, SM, &AST.getPragmaIncludes());
+      include_cleaner::headersForSymbol(Sym, AST.getPreprocessor(),
+                                        &AST.getPragmaIncludes());
   if (RankedProviders.empty())
     return;
 
+  const SourceManager &SM = AST.getSourceManager();
   std::string Result;
   include_cleaner::Includes ConvertedIncludes = convertIncludes(AST);
   for (const auto &P : RankedProviders) {
diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp 
b/clang-tools-extra/clangd/index/SymbolCollector.cpp
index 6d0af20e31260c..1de7faf81746ee 100644
--- a/clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -888,7 +888,7 @@ void SymbolCollector::setIncludeLocation(const Symbol &S, 
SourceLocation DefLoc,
   // might run while parsing, rather than at the end of a translation unit.
   // Hence we see more and more redecls over time.
   SymbolProviders[S.ID] =
-      include_cleaner::headersForSymbol(Sym, SM, Opts.PragmaIncludes);
+      include_cleaner::headersForSymbol(Sym, *PP, Opts.PragmaIncludes);
 }
 
 llvm::StringRef getStdHeader(const Symbol *S, const LangOptions &LangOpts) {
diff --git 
a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h 
b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
index 46ca3c9d080740..c3241763237d14 100644
--- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -90,7 +90,7 @@ std::string fixIncludes(const AnalysisResults &Results,
 /// Returned headers are sorted by relevance, first element is the most
 /// likely provider for the symbol.
 llvm::SmallVector<Header> headersForSymbol(const Symbol &S,
-                                           const SourceManager &SM,
+                                           const Preprocessor &PP,
                                            const PragmaIncludes *PI);
 } // namespace include_cleaner
 } // namespace clang
diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp 
b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
index e3a4834cb19aeb..a1781f4e24f2e8 100644
--- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -64,7 +64,7 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
       // FIXME: Most of the work done here is repetitive. It might be useful to
       // have a cache/batching.
       SymbolReference SymRef{ND, Loc, RT};
-      return CB(SymRef, headersForSymbol(ND, SM, PI));
+      return CB(SymRef, headersForSymbol(ND, PP, PI));
     });
   }
   for (const SymbolReference &MacroRef : MacroRefs) {
@@ -72,7 +72,7 @@ void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
     if (!SM.isWrittenInMainFile(SM.getSpellingLoc(MacroRef.RefLocation)) ||
         shouldIgnoreMacroReference(PP, MacroRef.Target.macro()))
       continue;
-    CB(MacroRef, headersForSymbol(MacroRef.Target, SM, PI));
+    CB(MacroRef, headersForSymbol(MacroRef.Target, PP, PI));
   }
 }
 
diff --git a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h 
b/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
index cd796c2da7b805..7d170fd15014d2 100644
--- a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
+++ b/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
@@ -25,6 +25,8 @@
 #include "clang-include-cleaner/Analysis.h"
 #include "clang-include-cleaner/Record.h"
 #include "clang-include-cleaner/Types.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
 #include <vector>
 
@@ -57,13 +59,14 @@ llvm::SmallVector<Hinted<Header>> findHeaders(const 
SymbolLocation &Loc,
                                               const PragmaIncludes *PI);
 
 /// A set of locations that provides the declaration.
-std::vector<Hinted<SymbolLocation>> locateSymbol(const Symbol &S);
+std::vector<Hinted<SymbolLocation>> locateSymbol(const Symbol &S,
+                                                 const LangOptions &LO);
 
 /// Write an HTML summary of the analysis to the given stream.
 void writeHTMLReport(FileID File, const Includes &,
                      llvm::ArrayRef<Decl *> Roots,
                      llvm::ArrayRef<SymbolReference> MacroRefs, ASTContext 
&Ctx,
-                     const HeaderSearch &HS, PragmaIncludes *PI,
+                     const Preprocessor &PP, PragmaIncludes *PI,
                      llvm::raw_ostream &OS);
 
 } // namespace include_cleaner
diff --git a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp 
b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
index 7b28d1c252d715..b96d9a70728c23 100644
--- a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -18,6 +18,7 @@
 #include "clang/Basic/FileEntry.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
@@ -239,8 +240,9 @@ llvm::SmallVector<Hinted<Header>> findHeaders(const 
SymbolLocation &Loc,
 }
 
 llvm::SmallVector<Header> headersForSymbol(const Symbol &S,
-                                           const SourceManager &SM,
+                                           const Preprocessor &PP,
                                            const PragmaIncludes *PI) {
+  const auto &SM = PP.getSourceManager();
   // Get headers for all the locations providing Symbol. Same header can be
   // reached through different traversals, deduplicate those into a single
   // Header by merging their hints.
@@ -248,7 +250,7 @@ llvm::SmallVector<Header> headersForSymbol(const Symbol &S,
   if (auto SpecialHeaders = headersForSpecialSymbol(S, SM, PI)) {
     Headers = std::move(*SpecialHeaders);
   } else {
-    for (auto &Loc : locateSymbol(S))
+    for (auto &Loc : locateSymbol(S, PP.getLangOpts()))
       Headers.append(applyHints(findHeaders(Loc, SM, PI), Loc.Hint));
   }
   // If two Headers probably refer to the same file (e.g. Verbatim(foo.h) and
diff --git a/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp 
b/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
index bbe8bc230c6e20..92c7c554ca50c9 100644
--- a/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
+++ b/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
@@ -21,6 +21,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
@@ -135,7 +136,7 @@ class Reporter {
   llvm::raw_ostream &OS;
   const ASTContext &Ctx;
   const SourceManager &SM;
-  const HeaderSearch &HS;
+  const Preprocessor &PP;
   const include_cleaner::Includes &Includes;
   const PragmaIncludes *PI;
   FileID MainFile;
@@ -170,9 +171,9 @@ class Reporter {
 
   void fillTarget(Ref &R) {
     // Duplicates logic from walkUsed(), which doesn't expose SymbolLocations.
-    for (auto &Loc : locateSymbol(R.Sym))
+    for (auto &Loc : locateSymbol(R.Sym, Ctx.getLangOpts()))
       R.Locations.push_back(Loc);
-    R.Headers = headersForSymbol(R.Sym, SM, PI);
+    R.Headers = headersForSymbol(R.Sym, PP, PI);
 
     for (const auto &H : R.Headers) {
       R.Includes.append(Includes.match(H));
@@ -189,14 +190,15 @@ class Reporter {
                      R.Includes.end());
 
     if (!R.Headers.empty())
-      R.Insert = spellHeader({R.Headers.front(), HS, MainFE});
+      R.Insert =
+          spellHeader({R.Headers.front(), PP.getHeaderSearchInfo(), MainFE});
   }
 
 public:
-  Reporter(llvm::raw_ostream &OS, ASTContext &Ctx, const HeaderSearch &HS,
+  Reporter(llvm::raw_ostream &OS, ASTContext &Ctx, const Preprocessor &PP,
            const include_cleaner::Includes &Includes, const PragmaIncludes *PI,
            FileID MainFile)
-      : OS(OS), Ctx(Ctx), SM(Ctx.getSourceManager()), HS(HS),
+      : OS(OS), Ctx(Ctx), SM(Ctx.getSourceManager()), PP(PP),
         Includes(Includes), PI(PI), MainFile(MainFile),
         MainFE(SM.getFileEntryForID(MainFile)) {}
 
@@ -498,9 +500,9 @@ class Reporter {
 void writeHTMLReport(FileID File, const include_cleaner::Includes &Includes,
                      llvm::ArrayRef<Decl *> Roots,
                      llvm::ArrayRef<SymbolReference> MacroRefs, ASTContext 
&Ctx,
-                     const HeaderSearch &HS, PragmaIncludes *PI,
+                     const Preprocessor &PP, PragmaIncludes *PI,
                      llvm::raw_ostream &OS) {
-  Reporter R(OS, Ctx, HS, Includes, PI, File);
+  Reporter R(OS, Ctx, PP, Includes, PI, File);
   const auto& SM = Ctx.getSourceManager();
   for (Decl *Root : Roots)
     walkAST(*Root, [&](SourceLocation Loc, const NamedDecl &D, RefType T) {
diff --git a/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp 
b/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
index 78e783a62eb27f..b7433305152f95 100644
--- a/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
+++ b/clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
@@ -54,20 +54,24 @@ std::vector<Hinted<SymbolLocation>> locateDecl(const Decl 
&D) {
   return Result;
 }
 
-std::vector<Hinted<SymbolLocation>> locateMacro(const Macro &M) {
+std::vector<Hinted<SymbolLocation>> locateMacro(const Macro &M,
+                                                const tooling::stdlib::Lang L) 
{
   // FIXME: Should we also provide physical locations?
-  if (auto SS = tooling::stdlib::Symbol::named("", M.Name->getName()))
+  if (auto SS = tooling::stdlib::Symbol::named("", M.Name->getName(), L))
     return {{*SS, Hints::CompleteSymbol}};
   return {{M.Definition, Hints::CompleteSymbol}};
 }
 } // namespace
 
-std::vector<Hinted<SymbolLocation>> locateSymbol(const Symbol &S) {
+std::vector<Hinted<SymbolLocation>> locateSymbol(const Symbol &S,
+                                                 const LangOptions &LO) {
+  const auto L = !LO.CPlusPlus && LO.C99 ? tooling::stdlib::Lang::C
+                                         : tooling::stdlib::Lang::CXX;
   switch (S.kind()) {
   case Symbol::Declaration:
     return locateDecl(S.declaration());
   case Symbol::Macro:
-    return locateMacro(S.macro());
+    return locateMacro(S.macro(), L);
   }
   llvm_unreachable("Unknown Symbol::Kind enum");
 }
diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp 
b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
index f85dbc0e0c31f2..1d9458ffc4d327 100644
--- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -216,10 +216,9 @@ class Action : public clang::ASTFrontendAction {
       ++Errors;
       return;
     }
-    writeHTMLReport(
-        AST.Ctx->getSourceManager().getMainFileID(), PP.Includes, AST.Roots,
-        PP.MacroReferences, *AST.Ctx,
-        getCompilerInstance().getPreprocessor().getHeaderSearchInfo(), &PI, 
OS);
+    writeHTMLReport(AST.Ctx->getSourceManager().getMainFileID(), PP.Includes,
+                    AST.Roots, PP.MacroReferences, *AST.Ctx,
+                    getCompilerInstance().getPreprocessor(), &PI, OS);
   }
 };
 class ActionFactory : public tooling::FrontendActionFactory {
diff --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp 
b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
index 84e02e1d0d621b..0ac243937e6e4f 100644
--- a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -306,7 +306,7 @@ class HeadersForSymbolTest : public FindHeadersTest {
     if (!V.Out)
       ADD_FAILURE() << "Couldn't find any decls named " << Name << ".";
     assert(V.Out);
-    return headersForSymbol(*V.Out, AST->sourceManager(), &PI);
+    return headersForSymbol(*V.Out, AST->preprocessor(), &PI);
   }
   llvm::SmallVector<Header> headersForFoo() { return headersFor("foo"); }
 };
@@ -611,13 +611,12 @@ TEST_F(HeadersForSymbolTest, 
AmbiguousStdSymbolsUsingShadow) {
   Visitor V;
   V.TraverseDecl(AST->context().getTranslationUnitDecl());
   ASSERT_TRUE(V.Out) << "Couldn't find a DeclRefExpr!";
-  EXPECT_THAT(headersForSymbol(*(V.Out->getFoundDecl()),
-                               AST->sourceManager(), &PI),
-              UnorderedElementsAre(
-                  Header(*tooling::stdlib::Header::named("<cstdio>"))));
+  EXPECT_THAT(
+      headersForSymbol(*(V.Out->getFoundDecl()), AST->preprocessor(), &PI),
+      UnorderedElementsAre(
+          Header(*tooling::stdlib::Header::named("<cstdio>"))));
 }
 
-
 TEST_F(HeadersForSymbolTest, StandardHeaders) {
   Inputs.Code = R"cpp(
     #include "stdlib_internal.h"
@@ -636,6 +635,30 @@ TEST_F(HeadersForSymbolTest, StandardHeaders) {
                            tooling::stdlib::Header::named("<assert.h>")));
 }
 
+TEST_F(HeadersForSymbolTest, StdlibLangForMacros) {
+  Inputs.Code = R"cpp(
+    #define EOF 0
+    void foo() { EOF; }
+  )cpp";
+  {
+    buildAST();
+    const Macro Eof{AST->preprocessor().getIdentifierInfo("EOF"), {}};
+    EXPECT_THAT(
+        headersForSymbol(Eof, AST->preprocessor(), nullptr),
+        UnorderedElementsAre(tooling::stdlib::Header::named("<cstdio>"),
+                             tooling::stdlib::Header::named("<stdio.h>")));
+  }
+
+  {
+    Inputs.ExtraArgs.push_back("-xc");
+    buildAST();
+    const Macro Eof{AST->preprocessor().getIdentifierInfo("EOF"), {}};
+    EXPECT_THAT(headersForSymbol(Eof, AST->preprocessor(), nullptr),
+                UnorderedElementsAre(tooling::stdlib::Header::named(
+                    "<stdio.h>", tooling::stdlib::Lang::C)));
+  }
+}
+
 TEST_F(HeadersForSymbolTest, ExporterNoNameMatch) {
   Inputs.Code = R"cpp(
     #include "exporter/foo.h"
diff --git a/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp 
b/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
index 756757cfd0f09d..1e7baf142a75ac 100644
--- a/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/LocateSymbolTest.cpp
@@ -11,6 +11,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Testing/TestAST.h"
@@ -96,6 +97,8 @@ struct LocateExample {
       Results.emplace_back(SM.getComposedLoc(FID, Offset));
     return Results;
   }
+
+  const LangOptions &langOpts() { return AST.preprocessor().getLangOpts(); }
 };
 
 TEST(LocateSymbol, Decl) {
@@ -110,7 +113,7 @@ TEST(LocateSymbol, Decl) {
   for (auto &Case : Cases) {
     SCOPED_TRACE(Case);
     LocateExample Test(Case);
-    EXPECT_THAT(locateSymbol(Test.findDecl("foo")),
+    EXPECT_THAT(locateSymbol(Test.findDecl("foo"), Test.langOpts()),
                 ElementsAreArray(Test.points()));
   }
 }
@@ -119,12 +122,12 @@ TEST(LocateSymbol, Stdlib) {
   {
     LocateExample Test("namespace std { struct vector; }");
     EXPECT_THAT(
-        locateSymbol(Test.findDecl("vector")),
+        locateSymbol(Test.findDecl("vector"), Test.langOpts()),
         ElementsAre(*tooling::stdlib::Symbol::named("std::", "vector")));
   }
   {
     LocateExample Test("#define assert(x)\nvoid foo() { assert(true); }");
-    EXPECT_THAT(locateSymbol(Test.findMacro("assert")),
+    EXPECT_THAT(locateSymbol(Test.findMacro("assert"), Test.langOpts()),
                 ElementsAre(*tooling::stdlib::Symbol::named("", "assert")));
   }
 }
@@ -132,7 +135,7 @@ TEST(LocateSymbol, Stdlib) {
 TEST(LocateSymbol, Macros) {
   // Make sure we preserve the last one.
   LocateExample Test("#define FOO\n#undef FOO\n#define ^FOO");
-  EXPECT_THAT(locateSymbol(Test.findMacro("FOO")),
+  EXPECT_THAT(locateSymbol(Test.findMacro("FOO"), Test.langOpts()),
               ElementsAreArray(Test.points()));
 }
 
@@ -143,7 +146,7 @@ TEST(LocateSymbol, CompleteSymbolHint) {
   {
     // stdlib symbols are always complete.
     LocateExample Test("namespace std { struct vector; }");
-    EXPECT_THAT(locateSymbol(Test.findDecl("vector")),
+    EXPECT_THAT(locateSymbol(Test.findDecl("vector"), Test.langOpts()),
                 ElementsAre(HintedSymbol(
                     *tooling::stdlib::Symbol::named("std::", "vector"),
                     Hints::CompleteSymbol)));
@@ -151,7 +154,7 @@ TEST(LocateSymbol, CompleteSymbolHint) {
   {
     // macros are always complete.
     LocateExample Test("#define ^FOO");
-    EXPECT_THAT(locateSymbol(Test.findMacro("FOO")),
+    EXPECT_THAT(locateSymbol(Test.findMacro("FOO"), Test.langOpts()),
                 ElementsAre(HintedSymbol(Test.points().front(),
                                          Hints::CompleteSymbol)));
   }
@@ -165,7 +168,7 @@ TEST(LocateSymbol, CompleteSymbolHint) {
     for (auto &Case : Cases) {
       SCOPED_TRACE(Case);
       LocateExample Test(Case);
-      EXPECT_THAT(locateSymbol(Test.findDecl("foo")),
+      EXPECT_THAT(locateSymbol(Test.findDecl("foo"), Test.langOpts()),
                   ElementsAre(HintedSymbol(Test.points().front(), Hints::None),
                               HintedSymbol(Test.points().back(),
                                            Hints::CompleteSymbol)));
@@ -181,7 +184,7 @@ TEST(LocateSymbol, CompleteSymbolHint) {
     for (auto &Case : Cases) {
       SCOPED_TRACE(Case);
       LocateExample Test(Case);
-      EXPECT_THAT(locateSymbol(Test.findDecl("foo")),
+      EXPECT_THAT(locateSymbol(Test.findDecl("foo"), Test.langOpts()),
                   Each(Field(&Hinted<SymbolLocation>::Hint,
                              Eq(Hints::CompleteSymbol))));
     }

``````````

</details>


https://github.com/llvm/llvm-project/pull/123634
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to