Skip to content

Commit e6db271

Browse files
[clangd] Avoid libFormat's objective-c guessing heuristic where possible
This avoids a known libFormat bug where the heuristic can OOM on certain large files (particularly single-header libraries such as miniaudio.h). The OOM will still happen on affected files if you actually try to format them (this is harder to avoid since the underlyting issue affects the actual formatting logic, not just the language-guessing heuristic), but at least it's avoided during non-modifying operations like hover, and modifying operations that do local formatting like code completion. Fixes clangd/clangd#719 Fixes clangd/clangd#1384 Fixes #70945
1 parent bec7ad9 commit e6db271

File tree

7 files changed

+56
-19
lines changed

7 files changed

+56
-19
lines changed

clang-tools-extra/clangd/ClangdServer.cpp

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,8 @@ void ClangdServer::formatFile(PathRef File, std::optional<Range> Rng,
523523
auto Action = [File = File.str(), Code = std::move(*Code),
524524
Ranges = std::vector<tooling::Range>{RequestedRange},
525525
CB = std::move(CB), this]() mutable {
526-
format::FormatStyle Style = getFormatStyleForFile(File, Code, TFS);
526+
format::FormatStyle Style =
527+
getFormatStyleForFile(File, Code, TFS, FormatKind::EntireFileOrRange);
527528
tooling::Replacements IncludeReplaces =
528529
format::sortIncludes(Style, Code, Ranges, File);
529530
auto Changed = tooling::applyAllReplacements(Code, IncludeReplaces);
@@ -551,7 +552,8 @@ void ClangdServer::formatOnType(PathRef File, Position Pos,
551552
auto Action = [File = File.str(), Code = std::move(*Code),
552553
TriggerText = TriggerText.str(), CursorPos = *CursorPos,
553554
CB = std::move(CB), this]() mutable {
554-
auto Style = getFormatStyleForFile(File, Code, TFS);
555+
auto Style =
556+
getFormatStyleForFile(File, Code, TFS, FormatKind::Replacements);
555557
std::vector<TextEdit> Result;
556558
for (const tooling::Replacement &R :
557559
formatIncremental(Code, CursorPos, TriggerText, Style))
@@ -604,8 +606,9 @@ void ClangdServer::rename(PathRef File, Position Pos, llvm::StringRef NewName,
604606
return CB(R.takeError());
605607

606608
if (Opts.WantFormat) {
607-
auto Style = getFormatStyleForFile(File, InpAST->Inputs.Contents,
608-
*InpAST->Inputs.TFS);
609+
auto Style =
610+
getFormatStyleForFile(File, InpAST->Inputs.Contents,
611+
*InpAST->Inputs.TFS, FormatKind::Replacements);
609612
llvm::Error Err = llvm::Error::success();
610613
for (auto &E : R->GlobalChanges)
611614
Err =
@@ -761,8 +764,8 @@ void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
761764
// Format tweaks that require it centrally here.
762765
for (auto &It : (*Effect)->ApplyEdits) {
763766
Edit &E = It.second;
764-
format::FormatStyle Style =
765-
getFormatStyleForFile(File, E.InitialCode, TFS);
767+
format::FormatStyle Style = getFormatStyleForFile(
768+
File, E.InitialCode, TFS, FormatKind::Replacements);
766769
if (llvm::Error Err = reformatEdit(E, Style))
767770
elog("Failed to format {0}: {1}", It.first(), std::move(Err));
768771
}
@@ -824,8 +827,9 @@ void ClangdServer::findHover(PathRef File, Position Pos,
824827
this](llvm::Expected<InputsAndAST> InpAST) mutable {
825828
if (!InpAST)
826829
return CB(InpAST.takeError());
827-
format::FormatStyle Style = getFormatStyleForFile(
828-
File, InpAST->Inputs.Contents, *InpAST->Inputs.TFS);
830+
format::FormatStyle Style =
831+
getFormatStyleForFile(File, InpAST->Inputs.Contents,
832+
*InpAST->Inputs.TFS, FormatKind::Snippet);
829833
CB(clangd::getHover(InpAST->AST, Pos, std::move(Style), Index));
830834
};
831835

clang-tools-extra/clangd/CodeComplete.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1626,9 +1626,9 @@ class CodeCompleteFlow {
16261626
assert(Recorder && "Recorder is not set");
16271627
CCContextKind = Recorder->CCContext.getKind();
16281628
IsUsingDeclaration = Recorder->CCContext.isUsingDeclaration();
1629-
auto Style = getFormatStyleForFile(SemaCCInput.FileName,
1630-
SemaCCInput.ParseInput.Contents,
1631-
*SemaCCInput.ParseInput.TFS);
1629+
auto Style = getFormatStyleForFile(
1630+
SemaCCInput.FileName, SemaCCInput.ParseInput.Contents,
1631+
*SemaCCInput.ParseInput.TFS, FormatKind::Replacements);
16321632
const auto NextToken = findTokenAfterCompletionPoint(
16331633
Recorder->CCSema->getPreprocessor().getCodeCompletionLoc(),
16341634
Recorder->CCSema->getSourceManager(), Recorder->CCSema->LangOpts);
@@ -1719,7 +1719,8 @@ class CodeCompleteFlow {
17191719
ProxSources[FileName].Cost = 0;
17201720
FileProximity.emplace(ProxSources);
17211721

1722-
auto Style = getFormatStyleForFile(FileName, Content, TFS);
1722+
auto Style =
1723+
getFormatStyleForFile(FileName, Content, TFS, FormatKind::Replacements);
17231724
// This will only insert verbatim headers.
17241725
Inserter.emplace(FileName, Content, Style,
17251726
/*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);

clang-tools-extra/clangd/IncludeCleaner.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,8 @@ std::vector<Diag> generateMissingIncludeDiagnostics(
116116
const SourceManager &SM = AST.getSourceManager();
117117
const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
118118

119-
auto FileStyle = getFormatStyleForFile(AST.tuPath(), Code, TFS);
119+
auto FileStyle =
120+
getFormatStyleForFile(AST.tuPath(), Code, TFS, FormatKind::Replacements);
120121

121122
tooling::HeaderIncludes HeaderIncludes(AST.tuPath(), Code,
122123
FileStyle.IncludeStyle);

clang-tools-extra/clangd/ParsedAST.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -625,8 +625,8 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
625625
// Add IncludeFixer which can recover diagnostics caused by missing includes
626626
// (e.g. incomplete type) and attach include insertion fixes to diagnostics.
627627
if (Inputs.Index && !BuildDir.getError()) {
628-
auto Style =
629-
getFormatStyleForFile(Filename, Inputs.Contents, *Inputs.TFS);
628+
auto Style = getFormatStyleForFile(Filename, Inputs.Contents, *Inputs.TFS,
629+
FormatKind::Replacements);
630630
auto Inserter = std::make_shared<IncludeInserter>(
631631
Filename, Inputs.Contents, Style, BuildDir.get(),
632632
&Clang->getPreprocessor().getHeaderSearchInfo());

clang-tools-extra/clangd/SourceCode.cpp

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -582,10 +582,25 @@ std::optional<FileDigest> digestFile(const SourceManager &SM, FileID FID) {
582582

583583
format::FormatStyle getFormatStyleForFile(llvm::StringRef File,
584584
llvm::StringRef Content,
585-
const ThreadsafeFS &TFS) {
585+
const ThreadsafeFS &TFS,
586+
FormatKind Kind) {
587+
// Unless we're formatting a substantial amount of code (the entire file
588+
// or an arbitrarily large range), skip libFormat's heuristic check for
589+
// .h files that tries to determine whether the file contains objective-c
590+
// code. (This is accomplished by passing empty code contents to getStyle().
591+
// The heuristic is the only thing that looks at the contents.)
592+
// This is a workaround for PR60151, a known issue in libFormat where this
593+
// heuristic can OOM on large files. If we *are* formatting the entire file,
594+
// there's no point in doing this because the actual format::reformat() call
595+
// will run into the same OOM; we'd just be risking inconsistencies between
596+
// clangd and clang-format on smaller .h files where they disagree on what
597+
// language is detected.
598+
if (Kind != FormatKind::EntireFileOrRange)
599+
Content = {};
586600
auto Style = format::getStyle(format::DefaultFormatStyle, File,
587601
format::DefaultFallbackStyle, Content,
588-
TFS.view(/*CWD=*/std::nullopt).get());
602+
TFS.view(/*CWD=*/std::nullopt).get(),
603+
/*AllowUnknownOptions=*/false);
589604
if (!Style) {
590605
log("getStyle() failed for file {0}: {1}. Fallback is LLVM style.", File,
591606
Style.takeError());

clang-tools-extra/clangd/SourceCode.h

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,14 +166,29 @@ TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
166166
std::optional<std::string> getCanonicalPath(const FileEntryRef F,
167167
FileManager &FileMgr);
168168

169+
/// A flag passed to getFormatStyleForFile() that specifies what kind of
170+
/// formatting operation the returned FormatStyle will be used for.
171+
enum class FormatKind {
172+
// Formatting a snippet of synthesized code (e.g. a code snippet
173+
// shown in a hover) that's not part of the main file.
174+
Snippet,
175+
// Formatting edits made by an editor action such as code completion
176+
// or rename.
177+
Replacements,
178+
// Formatting the entire main file (or a range selected by the user,
179+
// which can be arbitrarily long).
180+
EntireFileOrRange
181+
};
182+
169183
/// Choose the clang-format style we should apply to a certain file.
170184
/// This will usually use FS to look for .clang-format directories.
171185
/// FIXME: should we be caching the .clang-format file search?
172186
/// This uses format::DefaultFormatStyle and format::DefaultFallbackStyle,
173187
/// though the latter may have been overridden in main()!
174188
format::FormatStyle getFormatStyleForFile(llvm::StringRef File,
175189
llvm::StringRef Content,
176-
const ThreadsafeFS &TFS);
190+
const ThreadsafeFS &TFS,
191+
FormatKind Kind);
177192

178193
/// Cleanup and format the given replacements.
179194
llvm::Expected<tooling::Replacements>

clang-tools-extra/clangd/tool/Check.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,8 @@ class Checker {
226226

227227
// FIXME: Check that resource-dir/built-in-headers exist?
228228

229-
Style = getFormatStyleForFile(File, Inputs.Contents, TFS);
229+
Style =
230+
getFormatStyleForFile(File, Inputs.Contents, TFS, FormatKind::Snippet);
230231

231232
return true;
232233
}

0 commit comments

Comments
 (0)