Skip to content

Commit

Permalink
Revert "[clang][pp] adds '#pragma include_instead'"
Browse files Browse the repository at this point in the history
> `#pragma clang include_instead(<header>)` is a pragma that can be used
> by system headers (and only system headers) to indicate to a tool that
> the file containing said pragma is an implementation-detail header and
> should not be directly included by user code.
>
> The library alternative is very messy code that can be seen in the first
> diff of D106124, and we'd rather avoid that with something more
> universal.
>
> This patch takes the first step by warning a user when they include a
> detail header in their code, and suggests alternative headers that the
> user should include instead. Future work will involve adding a fixit to
> automate the process, as well as cleaning up modules diagnostics to not
> suggest said detail headers. Other tools, such as clangd can also take
> advantage of this pragma to add the correct user headers.
>
> Differential Revision: https://reviews.llvm.org/D106394

This caused compiler crashes in Chromium builds involving PCH and an include
directive with macro expansion, when Token::getLiteralData() returned null. See
the code review for details.

This reverts commit e8a64e5.
  • Loading branch information
zmodem committed Jul 27, 2021
1 parent a8cfa4b commit 973de71
Show file tree
Hide file tree
Showing 20 changed files with 23 additions and 225 deletions.
6 changes: 0 additions & 6 deletions clang/include/clang/Basic/DiagnosticLexKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -300,12 +300,6 @@ def pp_pragma_once_in_main_file : Warning<"#pragma once in main file">,
def pp_pragma_sysheader_in_main_file : Warning<
"#pragma system_header ignored in main file">,
InGroup<DiagGroup<"pragma-system-header-outside-header">>;

def err_pragma_include_instead_not_sysheader : Error<
"'#pragma clang include_instead' cannot be used outside of system headers">;
def err_pragma_include_instead_system_reserved : Error<
"header '%0' is an implementation detail; #include %select{'%2'|either '%2' or '%3'|one of %2}1 instead">;

def pp_poisoning_existing_macro : Warning<"poisoning existing macro">;
def pp_out_of_date_dependency : Warning<
"current file is older than dependency %0">;
Expand Down
17 changes: 1 addition & 16 deletions clang/include/clang/Lex/HeaderSearch.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,9 @@
#include "clang/Lex/ModuleMap.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Allocator.h"
#include <cassert>
#include <cstddef>
Expand Down Expand Up @@ -113,14 +110,6 @@ struct HeaderFileInfo {
/// of the framework.
StringRef Framework;

/// List of aliases that this header is known as.
/// Most headers should only have at most one alias, but a handful
/// have two.
llvm::SetVector<llvm::SmallString<32>,
llvm::SmallVector<llvm::SmallString<32>, 2>,
llvm::SmallSet<llvm::SmallString<32>, 2>>
Aliases;

HeaderFileInfo()
: isImport(false), isPragmaOnce(false), DirInfo(SrcMgr::C_User),
External(false), isModuleHeader(false), isCompilingModuleHeader(false),
Expand Down Expand Up @@ -464,10 +453,6 @@ class HeaderSearch {
getFileInfo(File).DirInfo = SrcMgr::C_System;
}

void AddFileAlias(const FileEntry *File, StringRef Alias) {
getFileInfo(File).Aliases.insert(Alias);
}

/// Mark the specified file as part of a module.
void MarkFileModuleHeader(const FileEntry *FE,
ModuleMap::ModuleHeaderRole Role,
Expand Down
5 changes: 1 addition & 4 deletions clang/include/clang/Lex/Preprocessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -1953,8 +1953,7 @@ class Preprocessor {
/// This either returns the EOF token and returns true, or
/// pops a level off the include stack and returns false, at which point the
/// client should call lex again.
bool HandleEndOfFile(Token &Result, SourceLocation Loc,
bool isEndOfMacro = false);
bool HandleEndOfFile(Token &Result, bool isEndOfMacro = false);

/// Callback invoked when the current TokenLexer hits the end of its
/// token stream.
Expand Down Expand Up @@ -2364,14 +2363,12 @@ class Preprocessor {

// Pragmas.
void HandlePragmaDirective(PragmaIntroducer Introducer);
void ResolvePragmaIncludeInstead(SourceLocation Location) const;

public:
void HandlePragmaOnce(Token &OnceTok);
void HandlePragmaMark(Token &MarkTok);
void HandlePragmaPoison();
void HandlePragmaSystemHeader(Token &SysHeaderTok);
void HandlePragmaIncludeInstead(Token &Tok);
void HandlePragmaDependency(Token &DependencyTok);
void HandlePragmaPushMacro(Token &Tok);
void HandlePragmaPopMacro(Token &Tok);
Expand Down
20 changes: 1 addition & 19 deletions clang/include/clang/Lex/PreprocessorLexer.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,11 @@
#ifndef LLVM_CLANG_LEX_PREPROCESSORLEXER_H
#define LLVM_CLANG_LEX_PREPROCESSORLEXER_H

#include "clang/Basic/SourceLocation.h"
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/MultipleIncludeOpt.h"
#include "clang/Lex/Token.h"
#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringMap.h"
#include <cassert>

namespace clang {
Expand Down Expand Up @@ -76,13 +74,6 @@ class PreprocessorLexer {
/// we are currently in.
SmallVector<PPConditionalInfo, 4> ConditionalStack;

struct IncludeInfo {
const FileEntry *File;
SourceLocation Location;
};
// A complete history of all the files included by the current file.
llvm::StringMap<IncludeInfo> IncludeHistory;

PreprocessorLexer() : FID() {}
PreprocessorLexer(Preprocessor *pp, FileID fid);
virtual ~PreprocessorLexer() = default;
Expand Down Expand Up @@ -184,15 +175,6 @@ class PreprocessorLexer {
ConditionalStack.clear();
ConditionalStack.append(CL.begin(), CL.end());
}

void addInclude(StringRef Filename, const FileEntry &File,
SourceLocation Location) {
IncludeHistory.insert({Filename, {&File, Location}});
}

const llvm::StringMap<IncludeInfo> &getIncludeHistory() const {
return IncludeHistory;
}
};

} // namespace clang
Expand Down
4 changes: 2 additions & 2 deletions clang/lib/Lex/Lexer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2811,11 +2811,11 @@ bool Lexer::LexEndOfFile(Token &Result, const char *CurPtr) {
ConditionalStack.pop_back();
}

SourceLocation EndLoc = getSourceLocation(BufferEnd);
// C99 5.1.1.2p2: If the file is non-empty and didn't end in a newline, issue
// a pedwarn.
if (CurPtr != BufferStart && (CurPtr[-1] != '\n' && CurPtr[-1] != '\r')) {
DiagnosticsEngine &Diags = PP->getDiagnostics();
SourceLocation EndLoc = getSourceLocation(BufferEnd);
unsigned DiagID;

if (LangOpts.CPlusPlus11) {
Expand All @@ -2838,7 +2838,7 @@ bool Lexer::LexEndOfFile(Token &Result, const char *CurPtr) {
BufferPtr = CurPtr;

// Finally, let the preprocessor handle this.
return PP->HandleEndOfFile(Result, EndLoc, isPragmaLexer());
return PP->HandleEndOfFile(Result, isPragmaLexer());
}

/// isNextPPTokenLParen - Return 1 if the next unexpanded token lexed from
Expand Down
6 changes: 0 additions & 6 deletions clang/lib/Lex/PPDirectives.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2022,12 +2022,6 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
IsFrameworkFound, IsImportDecl, IsMapped, LookupFrom, LookupFromFile,
LookupFilename, RelativePath, SearchPath, SuggestedModule, isAngled);

// Record the header's filename for later use.
if (File)
CurLexer->addInclude(
{FilenameTok.getLiteralData(), FilenameTok.getLength()},
File->getFileEntry(), FilenameLoc);

if (usingPCHWithThroughHeader() && SkippingUntilPCHThroughHeader) {
if (File && isPCHThroughHeader(&File->getFileEntry()))
SkippingUntilPCHThroughHeader = false;
Expand Down
45 changes: 2 additions & 43 deletions clang/lib/Lex/PPLexerChange.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
//===----------------------------------------------------------------------===//

#include "clang/Basic/FileManager.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/LexDiagnostic.h"
Expand All @@ -23,7 +22,6 @@
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/MemoryBufferRef.h"
#include "llvm/Support/Path.h"

using namespace clang;

//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -301,46 +299,10 @@ void Preprocessor::diagnoseMissingHeaderInUmbrellaDir(const Module &Mod) {
}
}

void Preprocessor::ResolvePragmaIncludeInstead(
const SourceLocation Location) const {
assert(Location.isValid());
if (CurLexer == nullptr)
return;

if (SourceMgr.isInSystemHeader(Location))
return;

for (const auto &Include : CurLexer->getIncludeHistory()) {
StringRef Filename = Include.getKey();
const PreprocessorLexer::IncludeInfo &Info = Include.getValue();
ArrayRef<SmallString<32>> Aliases =
HeaderInfo.getFileInfo(Info.File).Aliases.getArrayRef();

if (Aliases.empty())
continue;

switch (Aliases.size()) {
case 1:
Diag(Info.Location, diag::err_pragma_include_instead_system_reserved)
<< Filename << 0 << Aliases[0];
continue;
case 2:
Diag(Info.Location, diag::err_pragma_include_instead_system_reserved)
<< Filename << 1 << Aliases[0] << Aliases[1];
continue;
default: {
Diag(Info.Location, diag::err_pragma_include_instead_system_reserved)
<< Filename << 2 << ("{'" + llvm::join(Aliases, "', '") + "'}");
}
}
}
}

/// HandleEndOfFile - This callback is invoked when the lexer hits the end of
/// the current file. This either returns the EOF token or pops a level off
/// the include stack and keeps going.
bool Preprocessor::HandleEndOfFile(Token &Result, SourceLocation EndLoc,
bool isEndOfMacro) {
bool Preprocessor::HandleEndOfFile(Token &Result, bool isEndOfMacro) {
assert(!CurTokenLexer &&
"Ending a file when currently in a macro!");

Expand Down Expand Up @@ -410,9 +372,6 @@ bool Preprocessor::HandleEndOfFile(Token &Result, SourceLocation EndLoc,
}
}

if (EndLoc.isValid())
ResolvePragmaIncludeInstead(EndLoc);

// Complain about reaching a true EOF within arc_cf_code_audited.
// We don't want to complain about reaching the end of a macro
// instantiation or a _Pragma.
Expand Down Expand Up @@ -601,7 +560,7 @@ bool Preprocessor::HandleEndOfTokenLexer(Token &Result) {
TokenLexerCache[NumCachedTokenLexers++] = std::move(CurTokenLexer);

// Handle this like a #include file being popped off the stack.
return HandleEndOfFile(Result, {}, true);
return HandleEndOfFile(Result, true);
}

/// RemoveTopOfLexerStack - Pop the current lexer/macro exp off the top of the
Expand Down

0 comments on commit 973de71

Please sign in to comment.