eric_niebler updated this revision to Diff 57657.
eric_niebler added a comment.
Factor out TrySimplifyPath from Preprocessor::HandleIncludeDirective. Other
review feedback.
http://reviews.llvm.org/D19843
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticLexKinds.td
include/clang/Basic/FileManager.h
include/clang/Basic/VirtualFileSystem.h
include/clang/Lex/DirectoryLookup.h
include/clang/Lex/HeaderSearch.h
lib/Basic/FileManager.cpp
lib/Basic/VirtualFileSystem.cpp
lib/Lex/HeaderSearch.cpp
lib/Lex/PPDirectives.cpp
test/Lexer/Inputs/case-insensitive-include.h
test/Lexer/case-insensitive-include.c
test/PCH/case-insensitive-include.c
Index: test/PCH/case-insensitive-include.c
===================================================================
--- test/PCH/case-insensitive-include.c
+++ test/PCH/case-insensitive-include.c
@@ -2,7 +2,7 @@
// Test this without pch.
// RUN: cp %S/Inputs/case-insensitive-include.h %T
-// RUN: %clang_cc1 -fsyntax-only %s -include %s -I %T -verify
+// RUN: %clang_cc1 -Wno-nonportable-include-path -fsyntax-only %s -include %s -I %T -verify
// Test with pch.
// RUN: %clang_cc1 -emit-pch -o %t.pch %s -I %T
Index: test/Lexer/case-insensitive-include.c
===================================================================
--- /dev/null
+++ test/Lexer/case-insensitive-include.c
@@ -0,0 +1,42 @@
+// REQUIRES: case-insensitive-filesystem
+
+// RUN: mkdir -p %T/apath
+// RUN: cp %S/Inputs/case-insensitive-include.h %T
+// RUN: %clang_cc1 -fsyntax-only %s -include %s -I %T -verify
+
+// RUN: cp %S/Inputs/case-insensitive-include.h %T
+// RUN: %clang_cc1 -fsyntax-only -fms-compatibility -DMS_COMPATIBILITY %s -include %s -I %T -verify
+
+#ifndef HEADER
+#define HEADER
+
+#include "case-insensitive-include.h"
+#include "Case-Insensitive-Include.h" // expected-warning {{Non-portable path}}
+
+#include "../Output/./case-insensitive-include.h"
+#include "../Output/./Case-Insensitive-Include.h" // expected-warning {{Non-portable path}}
+#include "../output/./case-insensitive-include.h" // expected-warning {{Non-portable path}}
+
+#include "apath/.././case-insensitive-include.h"
+#include "apath/.././Case-Insensitive-Include.h" // expected-warning {{Non-portable path}}
+#include "APath/.././case-insensitive-include.h" // For the sake of efficiency, this case is not diagnosed. :-(
+
+#include "../Output/./apath/.././case-insensitive-include.h"
+#include "../Output/./APath/.././case-insensitive-include.h" // For the sake of efficiency, this case is not diagnosed. :-(
+#include "../output/./apath/.././case-insensitive-include.h" // expected-warning {{Non-portable path}}
+
+#if MS_COMPATIBILITY
+#include "..\Output\.\case-insensitive-include.h"
+#include "..\Output\.\Case-Insensitive-Include.h" // expected-warning {{Non-portable path}}
+#include "..\output\.\case-insensitive-include.h" // expected-warning {{Non-portable path}}
+
+#include "apath\..\.\case-insensitive-include.h"
+#include "apath\..\.\Case-Insensitive-Include.h" // expected-warning {{Non-portable path}}
+#include "APath\..\.\case-insensitive-include.h" // For the sake of efficiency, this case is not diagnosed. :-(
+#endif
+
+#else
+
+#include "Case-Insensitive-Include.h" // expected-warning {{Non-portable path}}
+
+#endif
Index: test/Lexer/Inputs/case-insensitive-include.h
===================================================================
--- /dev/null
+++ test/Lexer/Inputs/case-insensitive-include.h
@@ -0,0 +1,5 @@
+#pragma once
+
+struct S {
+ int x;
+};
Index: lib/Lex/PPDirectives.cpp
===================================================================
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -24,6 +24,9 @@
#include "clang/Lex/ModuleLoader.h"
#include "clang/Lex/Pragma.h"
#include "llvm/ADT/APInt.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/iterator_range.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/SaveAndRestore.h"
@@ -1497,6 +1500,40 @@
("@import " + PathString + ";").str());
}
+namespace
+{
+ // Given a vector of path components and a string containing the real
+ // path to the file, build a properly-cased replacement in the vector,
+ // and return true if the replacement should be suggested.
+ bool TrySimplifyPath(SmallVectorImpl<StringRef>& Components,
+ StringRef RealPathName) {
+ auto iRealPathComponent = llvm::sys::path::rbegin(RealPathName);
+ auto iRealPathComponentEnd = llvm::sys::path::rend(RealPathName);
+ int Cnt = 0;
+ bool SuggestReplacement = false;
+ for(auto& Component : llvm::reverse(Components)) {
+ if ("." == Component) {
+ } else if (".." == Component) {
+ ++Cnt;
+ } else if (Cnt) {
+ --Cnt;
+ } else if (iRealPathComponent != iRealPathComponentEnd) {
+ if (Component != *iRealPathComponent) {
+ // If these path components differ by more than just case, then we
+ // may be looking at symlinked paths. Bail on this diagnostic to avoid
+ // noisy false positives.
+ SuggestReplacement = iRealPathComponent->equals_lower(Component);
+ if (!SuggestReplacement)
+ break;
+ Component = *iRealPathComponent;
+ }
+ ++iRealPathComponent;
+ }
+ }
+ return SuggestReplacement;
+ };
+}
+
/// HandleIncludeDirective - The "\#include" tokens have just been read, read
/// the file to be included from the lexer, then include it! This is a common
/// routine with functionality shared between \#include, \#include_next and
@@ -1661,6 +1698,33 @@
}
}
+ // Issue a diagnostic if the name of the file on disk has a different case
+ // than the one we're about to open.
+ //
+ // Because testing for non-portable paths is expensive, only do it if the
+ // warning is not currently ignored.
+ const bool CheckIncludePathPortability =
+ File && !File->tryGetRealPathName().empty() &&
+ !Diags->isIgnored(diag::pp_nonportable_path, FilenameTok.getLocation());
+
+ if (CheckIncludePathPortability) {
+ StringRef Name = LangOpts.MSVCCompat ? NormalizedPath.c_str() : Filename;
+ StringRef RealPathName = File->tryGetRealPathName();
+ SmallVector<StringRef, 16> Components(llvm::sys::path::begin(Name),
+ llvm::sys::path::end(Name));
+ if (TrySimplifyPath(Components, RealPathName)) {
+ SmallString<128> Path;
+ Path.reserve(Name.size());
+ for (auto Component : Components)
+ llvm::sys::path::append(Path, Component);
+ SourceRange Range(FilenameTok.getLocation(), CharEnd);
+ auto Replacement = isAngled ? "<" + Path.str().str() + ">"
+ : "\"" + Path.str().str() + "\"";
+ Diag(FilenameTok, diag::pp_nonportable_path) << Filename <<
+ FixItHint::CreateReplacement(Range, Replacement);
+ }
+ }
+
// Should we enter the source file? Set to false if either the source file is
// known to have no effect beyond its effect on module visibility -- that is,
// if it's got an include guard that is already defined or is a modular header
Index: lib/Lex/HeaderSearch.cpp
===================================================================
--- lib/Lex/HeaderSearch.cpp
+++ lib/Lex/HeaderSearch.cpp
@@ -249,8 +249,9 @@
}
const FileEntry *HeaderSearch::getFileAndSuggestModule(
- StringRef FileName, const DirectoryEntry *Dir, bool IsSystemHeaderDir,
- Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule) {
+ StringRef FileName, SourceLocation IncludeLoc, const DirectoryEntry *Dir,
+ bool IsSystemHeaderDir, Module *RequestingModule,
+ ModuleMap::KnownHeader *SuggestedModule) {
// If we have a module map that might map this header, load it and
// check whether we'll have a suggestion for a module.
const FileEntry *File = getFileMgr().getFile(FileName, /*OpenFile=*/true);
@@ -271,6 +272,7 @@
const FileEntry *DirectoryLookup::LookupFile(
StringRef &Filename,
HeaderSearch &HS,
+ SourceLocation IncludeLoc,
SmallVectorImpl<char> *SearchPath,
SmallVectorImpl<char> *RelativePath,
Module *RequestingModule,
@@ -296,7 +298,7 @@
RelativePath->append(Filename.begin(), Filename.end());
}
- return HS.getFileAndSuggestModule(TmpDir, getDir(),
+ return HS.getFileAndSuggestModule(TmpDir, IncludeLoc, getDir(),
isSystemHeaderDirectory(),
RequestingModule, SuggestedModule);
}
@@ -584,7 +586,7 @@
RelativePath->append(Filename.begin(), Filename.end());
}
// Otherwise, just return the file.
- return getFileAndSuggestModule(Filename, nullptr,
+ return getFileAndSuggestModule(Filename, IncludeLoc, nullptr,
/*IsSystemHeaderDir*/false,
RequestingModule, SuggestedModule);
}
@@ -620,7 +622,7 @@
bool IncluderIsSystemHeader =
Includer && getFileInfo(Includer).DirInfo != SrcMgr::C_User;
if (const FileEntry *FE = getFileAndSuggestModule(
- TmpDir, IncluderAndDir.second, IncluderIsSystemHeader,
+ TmpDir, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader,
RequestingModule, SuggestedModule)) {
if (!Includer) {
assert(First && "only first includer can have no file");
@@ -711,7 +713,7 @@
bool InUserSpecifiedSystemFramework = false;
bool HasBeenMapped = false;
const FileEntry *FE = SearchDirs[i].LookupFile(
- Filename, *this, SearchPath, RelativePath, RequestingModule,
+ Filename, *this, IncludeLoc, SearchPath, RelativePath, RequestingModule,
SuggestedModule, InUserSpecifiedSystemFramework, HasBeenMapped,
MappedName);
if (HasBeenMapped) {
Index: lib/Basic/VirtualFileSystem.cpp
===================================================================
--- lib/Basic/VirtualFileSystem.cpp
+++ lib/Basic/VirtualFileSystem.cpp
@@ -135,16 +135,19 @@
class RealFile : public File {
int FD;
Status S;
+ std::string RealName;
friend class RealFileSystem;
- RealFile(int FD, StringRef NewName)
+ RealFile(int FD, StringRef NewName, StringRef NewRealPathName)
: FD(FD), S(NewName, {}, {}, {}, {}, {},
- llvm::sys::fs::file_type::status_error, {}) {
+ llvm::sys::fs::file_type::status_error, {}),
+ RealName(NewRealPathName.str()) {
assert(FD >= 0 && "Invalid or inactive file descriptor");
}
public:
~RealFile() override;
ErrorOr<Status> status() override;
+ ErrorOr<StringRef> getName() override;
ErrorOr<std::unique_ptr<MemoryBuffer>> getBuffer(const Twine &Name,
int64_t FileSize,
bool RequiresNullTerminator,
@@ -165,6 +168,10 @@
return S;
}
+ErrorOr<StringRef> RealFile::getName() {
+ return RealName.empty() ? S.getName() : StringRef(RealName);
+}
+
ErrorOr<std::unique_ptr<MemoryBuffer>>
RealFile::getBuffer(const Twine &Name, int64_t FileSize,
bool RequiresNullTerminator, bool IsVolatile) {
@@ -202,9 +209,10 @@
ErrorOr<std::unique_ptr<File>>
RealFileSystem::openFileForRead(const Twine &Name) {
int FD;
- if (std::error_code EC = sys::fs::openFileForRead(Name, FD))
+ SmallString<256> RealName;
+ if (std::error_code EC = sys::fs::openFileForRead(Name, FD, &RealName))
return EC;
- return std::unique_ptr<File>(new RealFile(FD, Name.str()));
+ return std::unique_ptr<File>(new RealFile(FD, Name.str(), RealName.str()));
}
llvm::ErrorOr<std::string> RealFileSystem::getCurrentWorkingDirectory() const {
Index: lib/Basic/FileManager.cpp
===================================================================
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -312,6 +312,9 @@
UFE.InPCH = Data.InPCH;
UFE.File = std::move(F);
UFE.IsValid = true;
+ if (UFE.File)
+ if (auto RealPathName = UFE.File->getName())
+ UFE.RealPathName = RealPathName->str();
return &UFE;
}
Index: include/clang/Lex/HeaderSearch.h
===================================================================
--- include/clang/Lex/HeaderSearch.h
+++ include/clang/Lex/HeaderSearch.h
@@ -582,8 +582,9 @@
/// \brief Look up the file with the specified name and determine its owning
/// module.
const FileEntry *
- getFileAndSuggestModule(StringRef FileName, const DirectoryEntry *Dir,
- bool IsSystemHeaderDir, Module *RequestingModule,
+ getFileAndSuggestModule(StringRef FileName, SourceLocation IncludeLoc,
+ const DirectoryEntry *Dir, bool IsSystemHeaderDir,
+ Module *RequestingModule,
ModuleMap::KnownHeader *SuggestedModule);
public:
Index: include/clang/Lex/DirectoryLookup.h
===================================================================
--- include/clang/Lex/DirectoryLookup.h
+++ include/clang/Lex/DirectoryLookup.h
@@ -151,6 +151,9 @@
///
/// \param HS The header search instance to search with.
///
+ /// \param IncludeLoc the source location of the #include or #import
+ /// directive.
+ ///
/// \param SearchPath If not NULL, will be set to the search path relative
/// to which the file was found.
///
@@ -172,6 +175,7 @@
/// a framework include ("Foo.h" -> "Foo/Foo.h"), set the new name to this
/// vector and point Filename to it.
const FileEntry *LookupFile(StringRef &Filename, HeaderSearch &HS,
+ SourceLocation IncludeLoc,
SmallVectorImpl<char> *SearchPath,
SmallVectorImpl<char> *RelativePath,
Module *RequestingModule,
Index: include/clang/Basic/VirtualFileSystem.h
===================================================================
--- include/clang/Basic/VirtualFileSystem.h
+++ include/clang/Basic/VirtualFileSystem.h
@@ -90,6 +90,13 @@
virtual ~File();
/// \brief Get the status of the file.
virtual llvm::ErrorOr<Status> status() = 0;
+ /// \brief Get the name of the file
+ virtual llvm::ErrorOr<StringRef> getName() {
+ if (auto Status = status())
+ return Status->getName();
+ else
+ return Status.getError();
+ }
/// \brief Get the contents of the file as a \p MemoryBuffer.
virtual llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
getBuffer(const Twine &Name, int64_t FileSize = -1,
Index: include/clang/Basic/FileManager.h
===================================================================
--- include/clang/Basic/FileManager.h
+++ include/clang/Basic/FileManager.h
@@ -52,6 +52,7 @@
/// descriptor for the file.
class FileEntry {
const char *Name; // Name of the file.
+ std::string RealPathName; // Real path to the file; could be empty.
off_t Size; // File size in bytes.
time_t ModTime; // Modification time of file.
const DirectoryEntry *Dir; // Directory file lives in.
@@ -82,6 +83,7 @@
}
const char *getName() const { return Name; }
+ StringRef tryGetRealPathName() const { return RealPathName; }
bool isValid() const { return IsValid; }
off_t getSize() const { return Size; }
unsigned getUID() const { return UID; }
Index: include/clang/Basic/DiagnosticLexKinds.td
===================================================================
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -274,6 +274,9 @@
"whitespace required after macro name">;
def warn_missing_whitespace_after_macro_name : Warning<
"whitespace recommended after macro name">;
+def pp_nonportable_path : Warning<
+ "Non-portable path '%0' found in preprocessor directive.">,
+ InGroup<NonportableIncludePath>;
def pp_pragma_once_in_main_file : Warning<"#pragma once in main file">,
InGroup<DiagGroup<"pragma-once-outside-header">>;
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -374,6 +374,7 @@
def AmbiguousMacro : DiagGroup<"ambiguous-macro">;
def KeywordAsMacro : DiagGroup<"keyword-macro">;
def ReservedIdAsMacro : DiagGroup<"reserved-id-macro">;
+def NonportableIncludePath : DiagGroup<"nonportable-include-path">;
// Just silence warnings about -Wstrict-aliasing for now.
def : DiagGroup<"strict-aliasing=0">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
