[PATCH] D19063: clang-format: Fixed line merging of more than two lines
cameron314 added a comment. I'll commit this in a few days, when I have some time to reintegrate it to the trunk. Thanks! https://reviews.llvm.org/D19063 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20132: [libclang] Add clang_getAllSkippedRanges function
cameron314 added inline comments. Comment at: unittests/libclang/LibclangTest.cpp:16-20 @@ -15,4 +15,7 @@ #include "gtest/gtest.h" #include #include +#include +#include +#include #define DEBUG_TYPE "libclang-test" rsmith wrote: > Please put these in alphabetical order. Will do! https://reviews.llvm.org/D20132 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20132: [libclang] Add clang_getAllSkippedRanges function
This revision was automatically updated to reflect the committed changes.
Closed by commit rL279076: [libclang] Add clang_getAllSkippedRanges function
(authored by cameron314).
Changed prior to commit:
https://reviews.llvm.org/D20132?vs=56964&id=68548#toc
Repository:
rL LLVM
https://reviews.llvm.org/D20132
Files:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/tools/libclang/CIndex.cpp
cfe/trunk/unittests/libclang/LibclangTest.cpp
Index: cfe/trunk/tools/libclang/CIndex.cpp
===
--- cfe/trunk/tools/libclang/CIndex.cpp
+++ cfe/trunk/tools/libclang/CIndex.cpp
@@ -7773,6 +7773,33 @@
return skipped;
}
+CXSourceRangeList *clang_getAllSkippedRanges(CXTranslationUnit TU) {
+ CXSourceRangeList *skipped = new CXSourceRangeList;
+ skipped->count = 0;
+ skipped->ranges = nullptr;
+
+ if (isNotUsableTU(TU)) {
+LOG_BAD_TU(TU);
+return skipped;
+ }
+
+ ASTUnit *astUnit = cxtu::getASTUnit(TU);
+ PreprocessingRecord *ppRec = astUnit->getPreprocessor().getPreprocessingRecord();
+ if (!ppRec)
+return skipped;
+
+ ASTContext &Ctx = astUnit->getASTContext();
+
+ const std::vector &SkippedRanges = ppRec->getSkippedRanges();
+
+ skipped->count = SkippedRanges.size();
+ skipped->ranges = new CXSourceRange[skipped->count];
+ for (unsigned i = 0, ei = skipped->count; i != ei; ++i)
+skipped->ranges[i] = cxloc::translateSourceRange(Ctx, SkippedRanges[i]);
+
+ return skipped;
+}
+
void clang_disposeSourceRangeList(CXSourceRangeList *ranges) {
if (ranges) {
delete[] ranges->ranges;
Index: cfe/trunk/include/clang-c/Index.h
===
--- cfe/trunk/include/clang-c/Index.h
+++ cfe/trunk/include/clang-c/Index.h
@@ -627,6 +627,15 @@
CXFile file);
/**
+ * \brief Retrieve all ranges from all files that were skipped by the
+ * preprocessor.
+ *
+ * The preprocessor will skip lines when they are surrounded by an
+ * if/ifdef/ifndef directive whose condition does not evaluate to true.
+ */
+CINDEX_LINKAGE CXSourceRangeList *clang_getAllSkippedRanges(CXTranslationUnit tu);
+
+/**
* \brief Destroy the given \c CXSourceRangeList.
*/
CINDEX_LINKAGE void clang_disposeSourceRangeList(CXSourceRangeList *ranges);
Index: cfe/trunk/unittests/libclang/LibclangTest.cpp
===
--- cfe/trunk/unittests/libclang/LibclangTest.cpp
+++ cfe/trunk/unittests/libclang/LibclangTest.cpp
@@ -14,6 +14,9 @@
#include "llvm/Support/raw_ostream.h"
#include "gtest/gtest.h"
#include
+#include
+#include
+#include
#include
#define DEBUG_TYPE "libclang-test"
@@ -349,21 +352,25 @@
clang_ModuleMapDescriptor_dispose(MMD);
}
-class LibclangReparseTest : public ::testing::Test {
+class LibclangParseTest : public ::testing::Test {
std::set Files;
+ typedef std::unique_ptr fixed_addr_string;
+ std::map UnsavedFileContents;
public:
std::string TestDir;
CXIndex Index;
CXTranslationUnit ClangTU;
unsigned TUFlags;
+ std::vector UnsavedFiles;
void SetUp() override {
llvm::SmallString<256> Dir;
ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("libclang-test", Dir));
TestDir = Dir.str();
TUFlags = CXTranslationUnit_DetailedPreprocessingRecord |
- clang_defaultEditingTranslationUnitOptions();
+ clang_defaultEditingTranslationUnitOptions();
Index = clang_createIndex(0, 0);
+ClangTU = nullptr;
}
void TearDown() override {
clang_disposeTranslationUnit(ClangTU);
@@ -384,6 +391,77 @@
OS << Contents;
assert(OS.good());
}
+ void MapUnsavedFile(std::string Filename, const std::string &Contents) {
+if (!llvm::sys::path::is_absolute(Filename)) {
+ llvm::SmallString<256> Path(TestDir);
+ llvm::sys::path::append(Path, Filename);
+ Filename = Path.str();
+}
+auto it = UnsavedFileContents.emplace(
+fixed_addr_string(new std::string(Filename)),
+fixed_addr_string(new std::string(Contents)));
+UnsavedFiles.push_back({
+it.first->first->c_str(), // filename
+it.first->second->c_str(), // contents
+it.first->second->size()// length
+});
+ }
+ template
+ void Traverse(const F &TraversalFunctor) {
+CXCursor TuCursor = clang_getTranslationUnitCursor(ClangTU);
+std::reference_wrapper FunctorRef = std::cref(TraversalFunctor);
+clang_visitChildren(TuCursor,
+&TraverseStateless>,
+&FunctorRef);
+ }
+private:
+ template
+ static CXChildVisitResult TraverseStateless(CXCursor cx, CXCursor parent,
+ CXClientData data) {
+TState *State = static_cast(data);
+return State->get()(cx, parent);
+ }
+};
+
+TEST_F(LibclangParseTest, AllSkippedRanges) {
+ std::string Header = "header.h", Main = "main.cpp";
+ WriteFile(Header,
+"#ifdef MANGOS\n"
+"printf(\"mmm\");\n"
+"#endif")
Re: Record ranges skipped by the preprocessor and expose them with libclang.
Ah, is that why! My fault, sorry. I couldn't figure out why it wouldn't link on the build machines when it worked fine for me locally (on Windows)... Thank you, I will fix this shortly. On Thu, Aug 18, 2016 at 12:52 PM, Will Dietz via cfe-commits < cfe-commits@lists.llvm.org> wrote: > (Sending again, somehow the uiuc.edu mailing list address was in my > reply-all! Sorry for the duplicates :)) > > This breaks things for me, I think an entry in 'libclang.exports' is > needed for clang_getAllSkippedRanges ? > > If that sounds right, can someone commit the fix? > > Thanks! :) > > ~Will, he-who-builds-from-trunk-more-often-than-he-should > > On Thu, Aug 18, 2016 at 11:48 AM Will Dietz wrote: > >> >> >> On Thu, Dec 5, 2013 at 2:25 AM Argyrios Kyrtzidis >> wrote: >> >>> >>> On Nov 15, 2013, at 7:57 AM, Erik Verbruggen >>> wrote: >>> >>> > Hi Argyrios, >>> > >>> > New patch attached. Your PCH comments do make sense, and I'll have to >>> ponder about it a bit for my integration :) >>> >>> Committed in r196487, thanks! >>> >>> > >>> > Cheers, >>> > Erik. >>> > >>> > Ps: sorry about not including the comments from your mail, but my >>> Mavericks upgrade managed to hose Mail.app or the index or something. >>> > >>> > <0001-Record-ranges-skipped-by-the-preprocessor-and- >>> expose.patch>___ >>> > cfe-commits mailing list >>> > cfe-comm...@cs.uiuc.edu >>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> >>> ___ >>> cfe-commits mailing list >>> cfe-comm...@cs.uiuc.edu >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> >> > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D19063: clang-format: Fixed line merging of more than two lines
This revision was automatically updated to reflect the committed changes.
Closed by commit rL286973: [clang-format] Fixed line merging of more than two
lines (authored by cameron314).
Changed prior to commit:
https://reviews.llvm.org/D19063?vs=53572&id=77996#toc
Repository:
rL LLVM
https://reviews.llvm.org/D19063
Files:
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
@@ -151,7 +151,7 @@
MergedLines = 0;
if (!DryRun)
for (unsigned i = 0; i < MergedLines; ++i)
-join(*Next[i], *Next[i + 1]);
+join(*Next[0], *Next[i + 1]);
Next = Next + MergedLines + 1;
return Current;
}
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -276,6 +276,30 @@
"int i;\n"
"\n"
"} // namespace"));
+
+ FormatStyle Style = getLLVMStyle();
+ Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+ Style.MaxEmptyLinesToKeep = 2;
+ Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+ Style.BraceWrapping.AfterClass = true;
+ Style.BraceWrapping.AfterFunction = true;
+ Style.KeepEmptyLinesAtTheStartOfBlocks = false;
+
+ EXPECT_EQ("class Foo\n"
+"{\n"
+" Foo() {}\n"
+"\n"
+" void funk() {}\n"
+"};",
+format("class Foo\n"
+ "{\n"
+ " Foo()\n"
+ " {\n"
+ " }\n"
+ "\n"
+ " void funk() {}\n"
+ "};",
+ Style));
}
TEST_F(FormatTest, RecognizesBinaryOperatorKeywords) {
Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
@@ -151,7 +151,7 @@
MergedLines = 0;
if (!DryRun)
for (unsigned i = 0; i < MergedLines; ++i)
-join(*Next[i], *Next[i + 1]);
+join(*Next[0], *Next[i + 1]);
Next = Next + MergedLines + 1;
return Current;
}
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -276,6 +276,30 @@
"int i;\n"
"\n"
"} // namespace"));
+
+ FormatStyle Style = getLLVMStyle();
+ Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
+ Style.MaxEmptyLinesToKeep = 2;
+ Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+ Style.BraceWrapping.AfterClass = true;
+ Style.BraceWrapping.AfterFunction = true;
+ Style.KeepEmptyLinesAtTheStartOfBlocks = false;
+
+ EXPECT_EQ("class Foo\n"
+"{\n"
+" Foo() {}\n"
+"\n"
+" void funk() {}\n"
+"};",
+format("class Foo\n"
+ "{\n"
+ " Foo()\n"
+ " {\n"
+ " }\n"
+ "\n"
+ " void funk() {}\n"
+ "};",
+ Style));
}
TEST_F(FormatTest, RecognizesBinaryOperatorKeywords) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged
cameron314 added a comment. Anyone have time to check this out this week? It's a one-line fix, includes a test, and is for a fairly important bug :-) https://reviews.llvm.org/D20338 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D20124: [PCH] Serialize skipped preprocessor ranges
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.
cameron314 set the repository for this revision to rL LLVM.
This fixes, for example, libclang's `clang_getAllSkippedRanges` returning zero
ranges after reparsing a translation unit.
Repository:
rL LLVM
http://reviews.llvm.org/D20124
Files:
include/clang/Lex/PreprocessingRecord.h
include/clang/Serialization/ASTBitCodes.h
include/clang/Serialization/ASTReader.h
include/clang/Serialization/Module.h
lib/Lex/PreprocessingRecord.cpp
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTWriter.cpp
lib/Serialization/Module.cpp
Index: lib/Serialization/Module.cpp
===
--- lib/Serialization/Module.cpp
+++ lib/Serialization/Module.cpp
@@ -31,6 +31,8 @@
LocalNumMacros(0), MacroOffsets(nullptr),
BasePreprocessedEntityID(0),
PreprocessedEntityOffsets(nullptr), NumPreprocessedEntities(0),
+BasePreprocessedSkippedRangeID(0),
+PreprocessedSkippedRangeOffsets(nullptr), NumPreprocessedSkippedRanges(0),
LocalNumHeaderFileInfos(0),
HeaderFileInfoTableData(nullptr), HeaderFileInfoTable(nullptr),
LocalNumSubmodules(0), BaseSubmoduleID(0),
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -953,6 +953,7 @@
RECORD(UNUSED_FILESCOPED_DECLS);
RECORD(PPD_ENTITIES_OFFSETS);
RECORD(VTABLE_USES);
+ RECORD(PPD_SKIPPED_RANGES);
RECORD(REFERENCED_SELECTOR_POOL);
RECORD(TU_UPDATE_LEXICAL);
RECORD(SEMA_DECL_REFS);
@@ -2408,6 +2409,26 @@
Stream.EmitRecordWithBlob(PPEOffsetAbbrev, Record,
bytes(PreprocessedEntityOffsets));
}
+
+ // Write the skipped region table for the preprocessing record.
+ const std::vector &SkippedRanges = PPRec.getSkippedRanges();
+ if (SkippedRanges.size() > 0) {
+std::vector SerializedSkippedRanges;
+SerializedSkippedRanges.reserve(SkippedRanges.size());
+for (auto const& Range : SkippedRanges)
+ SerializedSkippedRanges.emplace_back(Range);
+
+using namespace llvm;
+BitCodeAbbrev *Abbrev = new BitCodeAbbrev();
+Abbrev->Add(BitCodeAbbrevOp(PPD_SKIPPED_RANGES));
+Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob));
+unsigned PPESkippedRangeAbbrev = Stream.EmitAbbrev(Abbrev);
+
+Record.clear();
+Record.push_back(PPD_SKIPPED_RANGES);
+Stream.EmitRecordWithBlob(PPESkippedRangeAbbrev, Record,
+ bytes(SerializedSkippedRanges));
+ }
}
unsigned ASTWriter::getLocalOrImportedSubmoduleID(Module *Mod) {
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -3020,6 +3020,24 @@
break;
}
+
+case PPD_SKIPPED_RANGES: {
+ F.PreprocessedSkippedRangeOffsets = (const PPSkippedRange*)Blob.data();
+ assert(Blob.size() % sizeof(PPSkippedRange) == 0);
+ F.NumPreprocessedSkippedRanges = Blob.size() / sizeof(PPSkippedRange);
+
+ if (!PP.getPreprocessingRecord())
+PP.createPreprocessingRecord();
+ if (!PP.getPreprocessingRecord()->getExternalSource())
+PP.getPreprocessingRecord()->SetExternalSource(*this);
+ F.BasePreprocessedSkippedRangeID = PP.getPreprocessingRecord()
+->allocateSkippedRanges(F.NumPreprocessedSkippedRanges);
+
+ if (F.NumPreprocessedSkippedRanges > 0)
+GlobalSkippedRangeMap.insert(
+std::make_pair(F.BasePreprocessedSkippedRangeID, &F));
+ break;
+}
case DECL_UPDATE_OFFSETS: {
if (Record.size() % 2 != 0) {
@@ -4874,6 +4892,20 @@
Mod.FileSortedDecls + Mod.NumFileSortedDecls));
}
+SourceRange ASTReader::ReadSkippedRange(unsigned GlobalIndex) {
+ auto I = GlobalSkippedRangeMap.find(GlobalIndex);
+ assert(I != GlobalSkippedRangeMap.end() &&
+"Corrupted global skipped range map");
+ ModuleFile *M = I->second;
+ unsigned LocalIndex = GlobalIndex - M->BasePreprocessedSkippedRangeID;
+ assert(LocalIndex < M->NumPreprocessedSkippedRanges);
+ PPSkippedRange RawRange = M->PreprocessedSkippedRangeOffsets[LocalIndex];
+ SourceRange Range(TranslateSourceLocation(*M, RawRange.getBegin()),
+TranslateSourceLocation(*M, RawRange.getEnd()));
+ assert(Range.isValid());
+ return Range;
+}
+
PreprocessedEntity *ASTReader::ReadPreprocessedEntity(unsigned Index) {
PreprocessedEntityID PPID = Index+1;
std::pair PPInfo = getModulePreprocessedEntity(Index);
Index: lib/Lex/PreprocessingRecord.cpp
===
--- lib/Lex/PreprocessingRecord.cpp
+++ lib/Lex/PreprocessingRecord.cpp
@@ -40,7 +40,8 @@
PreprocessingRecord::PreprocessingRecord(SourceManager &SM)
: SourceMgr(
[PATCH] D20125: [libclang] Added clang_getRealSpellingLocation to compensate for clang_getSpellingLocation returning the expansion location
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.
All the libclang functions for expanding a location, namely
`clang_getExpansionLocation`, `clang_getPresumedLocation`,
`clang_getInstantiationLocation`, and most surprisingly
`clang_getSpellingLocation` return expansion locations, not spelling locations.
As it turns out, there is no way to get a spelling location via libclang. This
patch adds such a function.
Note that there's a FIXME in `clang_getSpellingLocation` about this, but
changing `clang_getSpellingLocation` to actually return a spelling location
would almost certainly break a large body of existing (third-party) code,
mostly in subtle ways. I think it's better to introduce a new function, though
I know it's ugly. But, this is what we've been using for the past year, and
we've gotten quite comfortable with it.
http://reviews.llvm.org/D20125
Files:
include/clang-c/Index.h
tools/libclang/CXSourceLocation.cpp
Index: tools/libclang/CXSourceLocation.cpp
===
--- tools/libclang/CXSourceLocation.cpp
+++ tools/libclang/CXSourceLocation.cpp
@@ -346,6 +346,42 @@
*offset = FileOffset;
}
+void clang_getRealSpellingLocation(CXSourceLocation location,
+ CXFile *file,
+ unsigned *line,
+ unsigned *column,
+ unsigned *offset) {
+
+ if (!isASTUnitSourceLocation(location)) {
+CXLoadedDiagnostic::decodeLocation(location, file, line, column, offset);
+return;
+ }
+
+ SourceLocation Loc = SourceLocation::getFromRawEncoding(location.int_data);
+
+ if (!location.ptr_data[0] || Loc.isInvalid())
+return createNullLocation(file, line, column, offset);
+
+ const SourceManager &SM =
+ *static_cast(location.ptr_data[0]);
+ SourceLocation SpellLoc = SM.getSpellingLoc(Loc);
+ std::pair LocInfo = SM.getDecomposedLoc(SpellLoc);
+ FileID FID = LocInfo.first;
+ unsigned FileOffset = LocInfo.second;
+
+ if (FID.isInvalid())
+return createNullLocation(file, line, column, offset);
+
+ if (file)
+*file = const_cast(SM.getFileEntryForID(FID));
+ if (line)
+*line = SM.getLineNumber(FID, FileOffset);
+ if (column)
+*column = SM.getColumnNumber(FID, FileOffset);
+ if (offset)
+*offset = FileOffset;
+}
+
void clang_getFileLocation(CXSourceLocation location,
CXFile *file,
unsigned *line,
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -568,6 +568,36 @@
* \brief Retrieve the file, line, column, and offset represented by
* the given source location.
*
+ * If the location refers into a macro instantiation, return where the
+ * location was originally spelled in the source file (i.e. where the token
+ * text is located). clang_getSpellingLocation() is supposed to do this
+ * but doesn't, hence this function.
+ *
+ * \param location the location within a source file that will be decomposed
+ * into its parts.
+ *
+ * \param file [out] if non-NULL, will be set to the file to which the given
+ * source location points.
+ *
+ * \param line [out] if non-NULL, will be set to the line to which the given
+ * source location points.
+ *
+ * \param column [out] if non-NULL, will be set to the column to which the
given
+ * source location points.
+ *
+ * \param offset [out] if non-NULL, will be set to the offset into the
+ * buffer to which the given source location points.
+ */
+CINDEX_LINKAGE void clang_getRealSpellingLocation(CXSourceLocation location,
+ CXFile *file,
+ unsigned *line,
+ unsigned *column,
+ unsigned *offset);
+
+/**
+ * \brief Retrieve the file, line, column, and offset represented by
+ * the given source location.
+ *
* If the location refers into a macro expansion, return where the macro was
* expanded or where the macro argument was written, if the location points at
* a macro argument.
Index: tools/libclang/CXSourceLocation.cpp
===
--- tools/libclang/CXSourceLocation.cpp
+++ tools/libclang/CXSourceLocation.cpp
@@ -346,6 +346,42 @@
*offset = FileOffset;
}
+void clang_getRealSpellingLocation(CXSourceLocation location,
+ CXFile *file,
+ unsigned *line,
+ unsigned *column,
+ unsigned *offset) {
+
+ if (!isASTUnitSourceLocation(location)) {
+CXLoadedDiagnostic::decodeLocation(location, file, line, column, offset);
+return;
[PATCH] D20127: [libclang] Expose cursors for alias/weak attributes
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.
This patch introduces `CXCursor_AliasAttr`/`CXCursor_WeakAttr` and
`clang_getAliasTargetSpelling()` for inspecting alias attributes via libclang.
http://reviews.llvm.org/D20127
Files:
include/clang-c/Index.h
tools/libclang/CIndex.cpp
tools/libclang/CXCursor.cpp
Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -61,6 +61,8 @@
case attr::Visibility: return CXCursor_VisibilityAttr;
case attr::DLLExport: return CXCursor_DLLExport;
case attr::DLLImport: return CXCursor_DLLImport;
+case attr::Weak: return CXCursor_WeakAttr;
+case attr::Alias: return CXCursor_AliasAttr;
}
return CXCursor_UnexposedAttr;
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -4698,6 +4698,10 @@
return cxstring::createRef("attribute(dllexport)");
case CXCursor_DLLImport:
return cxstring::createRef("attribute(dllimport)");
+ case CXCursor_WeakAttr:
+return cxstring::createRef("attribute(weak)");
+ case CXCursor_AliasAttr:
+return cxstring::createRef("attribute(alias)");
case CXCursor_PreprocessingDirective:
return cxstring::createRef("preprocessing directive");
case CXCursor_MacroDefinition:
@@ -5861,6 +5865,16 @@
return clang_getNullRange();
}
+CXString clang_getAliasTargetSpelling(CXCursor C) {
+ if (C.kind != CXCursor_AliasAttr)
+return cxstring::createEmpty();
+
+ const AliasAttr *A =
+cast(cxcursor::getCursorAttr(C));
+
+ return cxstring::createDup(A->getAliasee());
+}
+
void clang_enableStackTraces(void) {
llvm::sys::PrintStackTraceOnErrorSignal();
}
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -2338,7 +2338,9 @@
CXCursor_VisibilityAttr= 417,
CXCursor_DLLExport = 418,
CXCursor_DLLImport = 419,
- CXCursor_LastAttr = CXCursor_DLLImport,
+ CXCursor_WeakAttr = 420,
+ CXCursor_AliasAttr = 421,
+ CXCursor_LastAttr = CXCursor_AliasAttr,
/* Preprocessing */
CXCursor_PreprocessingDirective= 500,
@@ -3589,6 +3591,12 @@
CINDEX_LINKAGE CXType clang_getIBOutletCollectionType(CXCursor);
/**
+ * \brief For cursors representing an alias attribute,
+ * this function returns the aliased symbol name as written.
+ */
+CINDEX_LINKAGE CXString clang_getAliasTargetSpelling(CXCursor);
+
+/**
* @}
*/
Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -61,6 +61,8 @@
case attr::Visibility: return CXCursor_VisibilityAttr;
case attr::DLLExport: return CXCursor_DLLExport;
case attr::DLLImport: return CXCursor_DLLImport;
+case attr::Weak: return CXCursor_WeakAttr;
+case attr::Alias: return CXCursor_AliasAttr;
}
return CXCursor_UnexposedAttr;
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -4698,6 +4698,10 @@
return cxstring::createRef("attribute(dllexport)");
case CXCursor_DLLImport:
return cxstring::createRef("attribute(dllimport)");
+ case CXCursor_WeakAttr:
+return cxstring::createRef("attribute(weak)");
+ case CXCursor_AliasAttr:
+return cxstring::createRef("attribute(alias)");
case CXCursor_PreprocessingDirective:
return cxstring::createRef("preprocessing directive");
case CXCursor_MacroDefinition:
@@ -5861,6 +5865,16 @@
return clang_getNullRange();
}
+CXString clang_getAliasTargetSpelling(CXCursor C) {
+ if (C.kind != CXCursor_AliasAttr)
+return cxstring::createEmpty();
+
+ const AliasAttr *A =
+cast(cxcursor::getCursorAttr(C));
+
+ return cxstring::createDup(A->getAliasee());
+}
+
void clang_enableStackTraces(void) {
llvm::sys::PrintStackTraceOnErrorSignal();
}
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -2338,7 +2338,9 @@
CXCursor_VisibilityAttr= 417,
CXCursor_DLLExport = 418,
CXCursor_DLLImport = 419,
- CXCursor_LastAttr = CXCursor_DLLImport,
+ CXCursor_WeakAttr = 420,
+ CXCursor_AliasAttr = 421,
+ CXCursor_LastAttr = CXCursor_AliasAttr,
/* Preprocessing */
CXCursor_PreprocessingDirective= 500,
@@ -3589,6 +3591,12 @@
CINDEX_LINKAGE CX
[PATCH] D20129: [libclang] Properly expose linkage spec cursors
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.
All the groundwork for `CXCursor_LinkageSpec` already existed, but because of a
missing case in a switch, cursors of that type were never actually created for
linkage specifications in the AST. This patch fixes that.
I remember seeing another patch that fixed this a long time ago (that I found
after fixing it myself), but it was never committed (it was just forgotten, if
I recall correctly).
http://reviews.llvm.org/D20129
Files:
lib/Sema/SemaCodeComplete.cpp
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3052,7 +3052,10 @@
case Decl::UnresolvedUsingValue:
case Decl::UnresolvedUsingTypename:
return CXCursor_UsingDeclaration;
-
+
+case Decl::LinkageSpec:
+ return CXCursor_LinkageSpec;
+
case Decl::ObjCPropertyImpl:
switch (cast(D)->getPropertyImplementation()) {
case ObjCPropertyImplDecl::Dynamic:
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -3052,7 +3052,10 @@
case Decl::UnresolvedUsingValue:
case Decl::UnresolvedUsingTypename:
return CXCursor_UsingDeclaration;
-
+
+case Decl::LinkageSpec:
+ return CXCursor_LinkageSpec;
+
case Decl::ObjCPropertyImpl:
switch (cast(D)->getPropertyImplementation()) {
case ObjCPropertyImplDecl::Dynamic:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D20131: Fixed crash during code completion in file included within declaration
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.
When triggering code completion within a file that is included in the middle of
a declaration in another file, clang would crash while parsing the code.
This occurred with real-world code; there was an enum declaration that included
a header in the middle of its declaration to specify the enum members.
http://reviews.llvm.org/D20131
Files:
lib/Lex/PPLexerChange.cpp
Index: lib/Lex/PPLexerChange.cpp
===
--- lib/Lex/PPLexerChange.cpp
+++ lib/Lex/PPLexerChange.cpp
@@ -378,6 +378,8 @@
Result.startToken();
CurLexer->FormTokenWithChars(Result, CurLexer->BufferEnd, tok::eof);
CurLexer.reset();
+if (CurLexerKind == CLK_Lexer)
+ CurLexerKind = CLK_CachingLexer;
} else {
assert(CurPTHLexer && "Got EOF but no current lexer set!");
CurPTHLexer->getEOF(Result);
Index: lib/Lex/PPLexerChange.cpp
===
--- lib/Lex/PPLexerChange.cpp
+++ lib/Lex/PPLexerChange.cpp
@@ -378,6 +378,8 @@
Result.startToken();
CurLexer->FormTokenWithChars(Result, CurLexer->BufferEnd, tok::eof);
CurLexer.reset();
+if (CurLexerKind == CLK_Lexer)
+ CurLexerKind = CLK_CachingLexer;
} else {
assert(CurPTHLexer && "Got EOF but no current lexer set!");
CurPTHLexer->getEOF(Result);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D20132: [libclang] Add clang_getAllSkippedRanges function
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.
This complements the `clang_getSkippedRanges` function which returns skipped
ranges filtered by a specific file.
This function is useful when all the ranges are desired (and a lot more
efficient than the equivalent of asking for the ranges file by file, since the
implementation of `clang_getSkippedRanges` iterates over all ranges anyway). We
use this internally to populate a database that later gets queried by our IDE.
http://reviews.llvm.org/D20132
Files:
include/clang-c/Index.h
tools/libclang/CIndex.cpp
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7680,6 +7680,33 @@
return skipped;
}
+CXSourceRangeList *clang_getAllSkippedRanges(CXTranslationUnit TU) {
+ CXSourceRangeList *skipped = new CXSourceRangeList;
+ skipped->count = 0;
+ skipped->ranges = nullptr;
+
+ if (isNotUsableTU(TU)) {
+LOG_BAD_TU(TU);
+return skipped;
+ }
+
+ ASTUnit *astUnit = cxtu::getASTUnit(TU);
+ PreprocessingRecord *ppRec =
astUnit->getPreprocessor().getPreprocessingRecord();
+ if (!ppRec)
+return skipped;
+
+ ASTContext &Ctx = astUnit->getASTContext();
+
+ const std::vector &SkippedRanges = ppRec->getSkippedRanges();
+
+ skipped->count = SkippedRanges.size();
+ skipped->ranges = new CXSourceRange[skipped->count];
+ for (unsigned i = 0, ei = skipped->count; i != ei; ++i)
+skipped->ranges[i] = cxloc::translateSourceRange(Ctx, SkippedRanges[i]);
+
+ return skipped;
+}
+
void clang_disposeSourceRangeList(CXSourceRangeList *ranges) {
if (ranges) {
delete[] ranges->ranges;
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -627,6 +627,15 @@
CXFile file);
/**
+ * \brief Retrieve all ranges from all files that were skipped by the
+ * preprocessor.
+ *
+ * The preprocessor will skip lines when they are surrounded by an
+ * if/ifdef/ifndef directive whose condition does not evaluate to true.
+ */
+CINDEX_LINKAGE CXSourceRangeList *clang_getAllSkippedRanges(CXTranslationUnit
tu);
+
+/**
* \brief Destroy the given \c CXSourceRangeList.
*/
CINDEX_LINKAGE void clang_disposeSourceRangeList(CXSourceRangeList *ranges);
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7680,6 +7680,33 @@
return skipped;
}
+CXSourceRangeList *clang_getAllSkippedRanges(CXTranslationUnit TU) {
+ CXSourceRangeList *skipped = new CXSourceRangeList;
+ skipped->count = 0;
+ skipped->ranges = nullptr;
+
+ if (isNotUsableTU(TU)) {
+LOG_BAD_TU(TU);
+return skipped;
+ }
+
+ ASTUnit *astUnit = cxtu::getASTUnit(TU);
+ PreprocessingRecord *ppRec = astUnit->getPreprocessor().getPreprocessingRecord();
+ if (!ppRec)
+return skipped;
+
+ ASTContext &Ctx = astUnit->getASTContext();
+
+ const std::vector &SkippedRanges = ppRec->getSkippedRanges();
+
+ skipped->count = SkippedRanges.size();
+ skipped->ranges = new CXSourceRange[skipped->count];
+ for (unsigned i = 0, ei = skipped->count; i != ei; ++i)
+skipped->ranges[i] = cxloc::translateSourceRange(Ctx, SkippedRanges[i]);
+
+ return skipped;
+}
+
void clang_disposeSourceRangeList(CXSourceRangeList *ranges) {
if (ranges) {
delete[] ranges->ranges;
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -627,6 +627,15 @@
CXFile file);
/**
+ * \brief Retrieve all ranges from all files that were skipped by the
+ * preprocessor.
+ *
+ * The preprocessor will skip lines when they are surrounded by an
+ * if/ifdef/ifndef directive whose condition does not evaluate to true.
+ */
+CINDEX_LINKAGE CXSourceRangeList *clang_getAllSkippedRanges(CXTranslationUnit tu);
+
+/**
* \brief Destroy the given \c CXSourceRangeList.
*/
CINDEX_LINKAGE void clang_disposeSourceRangeList(CXSourceRangeList *ranges);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D20134: [libclang] Fixed bug where ranges in spelling locations (in macro expansions) would get mangled
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.
The end location of the range would be changed into its expansion location, but
the beginning of the range would stay as a spelling location. This would lead
to very weird ranges spanning large parts of a file (starting in a macro until
its expansion).
This bug was not previously caught because there was no way to actually obtain
a spelling location via libclang (see my patch at D20125 which adds support for
this), so later obtaining the start location of a range would result in an
expansion location anyway, and wind up matching the expansion location that was
stored in the end of the range.
http://reviews.llvm.org/D20134
Files:
tools/libclang/CIndex.cpp
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -142,8 +142,6 @@
// We want the last character in this location, so we will adjust the
// location accordingly.
SourceLocation EndLoc = R.getEnd();
- if (EndLoc.isValid() && EndLoc.isMacroID() &&
!SM.isMacroArgExpansion(EndLoc))
-EndLoc = SM.getExpansionRange(EndLoc).second;
if (R.isTokenRange() && EndLoc.isValid()) {
unsigned Length = Lexer::MeasureTokenLength(SM.getSpellingLoc(EndLoc),
SM, LangOpts);
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -142,8 +142,6 @@
// We want the last character in this location, so we will adjust the
// location accordingly.
SourceLocation EndLoc = R.getEnd();
- if (EndLoc.isValid() && EndLoc.isMacroID() && !SM.isMacroArgExpansion(EndLoc))
-EndLoc = SM.getExpansionRange(EndLoc).second;
if (R.isTokenRange() && EndLoc.isValid()) {
unsigned Length = Lexer::MeasureTokenLength(SM.getSpellingLoc(EndLoc),
SM, LangOpts);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D20137: [PCH] Fixed bugs with preamble invalidation when files change (on Windows)
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.
There were two bugs relating to remapped files, that are not specific to
Windows but happen more often there:
- When remapped files were changed, they would never cause the preamble's PCH
to be invalidated, because the remapped path didn't necessarily match the
include path (e.g. slash direction). I fixed this by moving to a
`llvm::sys::fs::UniqueID`-based map instead of using strings for paths and
hoping they match.
- Fixing the above bug revealed a new bug where remapped files would always
cause the preamble's PCH to be invalidated (even if they hadn't changed)
because the file manager was replacing the remapped virtual entry (no modified
time) with a real file entry (which has a modified time) when the include
directive was processed, again because it had a slightly different path due to
slash direction.
http://reviews.llvm.org/D20137
Files:
include/clang/Basic/FileManager.h
include/clang/Frontend/ASTUnit.h
lib/Basic/FileManager.cpp
lib/Frontend/ASTUnit.cpp
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1378,7 +1378,7 @@
// First, make a record of those files that have been overridden via
// remapping or unsaved_files.
- llvm::StringMap OverriddenFiles;
+ std::map OverriddenFiles;
for (const auto &R : PreprocessorOpts.RemappedFiles) {
if (AnyFileChanged)
break;
@@ -1391,40 +1391,47 @@
break;
}
-OverriddenFiles[R.first] = PreambleFileHash::createForFile(
+OverriddenFiles[Status.getUniqueID()] = PreambleFileHash::createForFile(
Status.getSize(), Status.getLastModificationTime().toEpochTime());
}
for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
if (AnyFileChanged)
break;
-OverriddenFiles[RB.first] =
+
+vfs::Status Status;
+if (FileMgr->getNoncachedStatValue(RB.first, Status)) {
+ AnyFileChanged = true;
+ break;
+}
+
+OverriddenFiles[Status.getUniqueID()] =
PreambleFileHash::createForMemoryBuffer(RB.second);
}
// Check whether anything has changed.
- for (llvm::StringMap::iterator
+ for (FilesInPreambleMap::iterator
F = FilesInPreamble.begin(), FEnd = FilesInPreamble.end();
!AnyFileChanged && F != FEnd;
++F) {
-llvm::StringMap::iterator Overridden
- = OverriddenFiles.find(F->first());
+std::map::iterator Overridden
+ = OverriddenFiles.find(F->first);
if (Overridden != OverriddenFiles.end()) {
// This file was remapped; check whether the newly-mapped file
// matches up with the previous mapping.
- if (Overridden->second != F->second)
+ if (Overridden->second != F->second.second)
AnyFileChanged = true;
continue;
}
// The file was not remapped; check whether it has changed on disk.
vfs::Status Status;
-if (FileMgr->getNoncachedStatValue(F->first(), Status)) {
+if (FileMgr->getNoncachedStatValue(F->second.first, Status)) {
// If we can't stat the file, assume that something horrible happened.
AnyFileChanged = true;
-} else if (Status.getSize() != uint64_t(F->second.Size) ||
+} else if (Status.getSize() != uint64_t(F->second.second.Size) ||
Status.getLastModificationTime().toEpochTime() !=
- uint64_t(F->second.ModTime))
+ uint64_t(F->second.second.ModTime))
AnyFileChanged = true;
}
@@ -1612,12 +1619,14 @@
if (!File || File == SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()))
continue;
if (time_t ModTime = File->getModificationTime()) {
- FilesInPreamble[File->getName()] = PreambleFileHash::createForFile(
- File->getSize(), ModTime);
+ FilesInPreamble[File->getUniqueID()] = std::make_pair(
+StringRef(File->getName()),
+PreambleFileHash::createForFile(File->getSize(), ModTime));
} else {
llvm::MemoryBuffer *Buffer = SourceMgr.getMemoryBufferForFile(File);
- FilesInPreamble[File->getName()] =
- PreambleFileHash::createForMemoryBuffer(Buffer);
+ FilesInPreamble[File->getUniqueID()] = std::make_pair(
+StringRef(File->getName()),
+PreambleFileHash::createForMemoryBuffer(Buffer));
}
}
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -301,6 +301,11 @@
return &UFE;
}
+ if (UFE.isVirtual()) {
+UFE.Name = InterndFileName;
+return &UFE;
+ }
+
Re: [PATCH] D20125: [libclang] Added clang_getRealSpellingLocation to compensate for clang_getSpellingLocation returning the expansion location
cameron314 added a comment. Ah, I was wondering where the tests were. I found this in the CMake file: # FIXME: libclang unit tests are disabled on Windows due # to failures, mostly in libclang.VirtualFileOverlay_*. if(NOT WIN32 AND CLANG_TOOL_LIBCLANG_BUILD) add_subdirectory(libclang) endif() I'll ignore the VFO failures for now, and add a new test for clang_getRealSpellingLocation :-) http://reviews.llvm.org/D20125 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20125: [libclang] Added clang_getRealSpellingLocation to compensate for clang_getSpellingLocation returning the expansion location
cameron314 updated this revision to Diff 56916.
cameron314 added a comment.
I've added a unit test.
http://reviews.llvm.org/D20125
Files:
include/clang-c/Index.h
tools/libclang/CXSourceLocation.cpp
unittests/libclang/LibclangTest.cpp
Index: unittests/libclang/LibclangTest.cpp
===
--- unittests/libclang/LibclangTest.cpp
+++ unittests/libclang/LibclangTest.cpp
@@ -15,6 +15,9 @@
#include "gtest/gtest.h"
#include
#include
+#include
+#include
+#include
#define DEBUG_TYPE "libclang-test"
TEST(libclang, clang_parseTranslationUnit2_InvalidArgs) {
@@ -349,21 +352,25 @@
clang_ModuleMapDescriptor_dispose(MMD);
}
-class LibclangReparseTest : public ::testing::Test {
+class LibclangParseTest : public ::testing::Test {
std::set Files;
+ typedef std::unique_ptr fixed_addr_string;
+ std::map UnsavedFileContents;
public:
std::string TestDir;
CXIndex Index;
CXTranslationUnit ClangTU;
unsigned TUFlags;
+ std::vector UnsavedFiles;
void SetUp() override {
llvm::SmallString<256> Dir;
ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("libclang-test", Dir));
TestDir = Dir.str();
TUFlags = CXTranslationUnit_DetailedPreprocessingRecord |
- clang_defaultEditingTranslationUnitOptions();
+ clang_defaultEditingTranslationUnitOptions();
Index = clang_createIndex(0, 0);
+ClangTU = nullptr;
}
void TearDown() override {
clang_disposeTranslationUnit(ClangTU);
@@ -384,6 +391,64 @@
OS << Contents;
assert(OS.good());
}
+ void MapUnsavedFile(const char* name, const std::string &contents) {
+auto it = UnsavedFileContents.emplace(
+fixed_addr_string(new std::string(name)),
+fixed_addr_string(new std::string(contents)));
+UnsavedFiles.push_back({
+it.first->first->c_str(), // filename
+it.first->second->c_str(), // contents
+it.first->second->size()// length
+});
+ }
+ template
+ void Traverse(const F &TraversalFunctor) {
+CXCursor TuCursor = clang_getTranslationUnitCursor(ClangTU);
+std::reference_wrapper FunctorRef = std::cref(TraversalFunctor);
+clang_visitChildren(TuCursor,
+&TraverseStateless>,
+&FunctorRef);
+ }
+
+private:
+ template
+ static CXChildVisitResult TraverseStateless(CXCursor cx, CXCursor parent,
+ CXClientData data) {
+TState *State = static_cast(data);
+return State->get()(cx, parent);
+ }
+};
+
+TEST_F(LibclangParseTest, SpellingLocation) {
+ MapUnsavedFile("main.cpp",
+"#define BANANAS 4011\n"
+"int plu = BANANAS;");
+
+ ClangTU = clang_parseTranslationUnit(Index, "main.cpp", nullptr, 0,
+UnsavedFiles.data(), UnsavedFiles.size(), TUFlags);
+
+ bool sawInt;
+ Traverse([&](CXCursor cx, CXCursor) {
+if (cx.kind == CXCursor_IntegerLiteral) {
+ sawInt = true;
+ auto cxl = clang_getCursorLocation(cx);
+ CXFile expansionFile, spellingFile;
+ unsigned line, column, offset;
+ clang_getSpellingLocation(cxl, &expansionFile, &line, nullptr, nullptr);
+ EXPECT_EQ(2, line); // getSpellingLocation returns expansion location
+ clang_getRealSpellingLocation(cxl, &spellingFile, &line, &column, &offset);
+ EXPECT_TRUE(clang_File_isEqual(expansionFile, spellingFile));
+ EXPECT_EQ(1, line);
+ EXPECT_EQ(17, column);
+ EXPECT_EQ(16, offset);
+}
+return CXChildVisit_Recurse;
+ });
+ EXPECT_TRUE(sawInt);
+}
+
+class LibclangReparseTest : public LibclangParseTest {
+public:
void DisplayDiagnostics() {
unsigned NumDiagnostics = clang_getNumDiagnostics(ClangTU);
for (unsigned i = 0; i < NumDiagnostics; ++i) {
Index: tools/libclang/CXSourceLocation.cpp
===
--- tools/libclang/CXSourceLocation.cpp
+++ tools/libclang/CXSourceLocation.cpp
@@ -346,6 +346,42 @@
*offset = FileOffset;
}
+void clang_getRealSpellingLocation(CXSourceLocation location,
+ CXFile *file,
+ unsigned *line,
+ unsigned *column,
+ unsigned *offset) {
+
+ if (!isASTUnitSourceLocation(location)) {
+CXLoadedDiagnostic::decodeLocation(location, file, line, column, offset);
+return;
+ }
+
+ SourceLocation Loc = SourceLocation::getFromRawEncoding(location.int_data);
+
+ if (!location.ptr_data[0] || Loc.isInvalid())
+return createNullLocation(file, line, column, offset);
+
+ const SourceManager &SM =
+ *static_cast(location.ptr_data[0]);
+ SourceLocation SpellLoc = SM.getSpellingLoc(Loc);
+ std::pair LocInfo = SM.getDecomposedLoc(SpellLoc);
+ FileID FID = LocInfo.first;
+ unsigned FileOffset = LocInfo.second;
+
+ if (FID.isInvalid())
+return createNullLocation(file, line, column, offset);
+
+ if (file)
+*file = const_cast(SM.getFileEntryForID(FID));
+ if (line)
+*line
Re: [PATCH] D20125: [libclang] Added clang_getRealSpellingLocation to compensate for clang_getSpellingLocation returning the expansion location
cameron314 updated this revision to Diff 56917.
http://reviews.llvm.org/D20125
Files:
include/clang-c/Index.h
tools/libclang/CXSourceLocation.cpp
unittests/libclang/LibclangTest.cpp
Index: unittests/libclang/LibclangTest.cpp
===
--- unittests/libclang/LibclangTest.cpp
+++ unittests/libclang/LibclangTest.cpp
@@ -15,6 +15,9 @@
#include "gtest/gtest.h"
#include
#include
+#include
+#include
+#include
#define DEBUG_TYPE "libclang-test"
TEST(libclang, clang_parseTranslationUnit2_InvalidArgs) {
@@ -349,21 +352,25 @@
clang_ModuleMapDescriptor_dispose(MMD);
}
-class LibclangReparseTest : public ::testing::Test {
+class LibclangParseTest : public ::testing::Test {
std::set Files;
+ typedef std::unique_ptr fixed_addr_string;
+ std::map UnsavedFileContents;
public:
std::string TestDir;
CXIndex Index;
CXTranslationUnit ClangTU;
unsigned TUFlags;
+ std::vector UnsavedFiles;
void SetUp() override {
llvm::SmallString<256> Dir;
ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("libclang-test", Dir));
TestDir = Dir.str();
TUFlags = CXTranslationUnit_DetailedPreprocessingRecord |
- clang_defaultEditingTranslationUnitOptions();
+ clang_defaultEditingTranslationUnitOptions();
Index = clang_createIndex(0, 0);
+ClangTU = nullptr;
}
void TearDown() override {
clang_disposeTranslationUnit(ClangTU);
@@ -384,6 +391,64 @@
OS << Contents;
assert(OS.good());
}
+ void MapUnsavedFile(const char* name, const std::string &contents) {
+auto it = UnsavedFileContents.emplace(
+fixed_addr_string(new std::string(name)),
+fixed_addr_string(new std::string(contents)));
+UnsavedFiles.push_back({
+it.first->first->c_str(), // filename
+it.first->second->c_str(), // contents
+it.first->second->size()// length
+});
+ }
+ template
+ void Traverse(const F &TraversalFunctor) {
+CXCursor TuCursor = clang_getTranslationUnitCursor(ClangTU);
+std::reference_wrapper FunctorRef = std::cref(TraversalFunctor);
+clang_visitChildren(TuCursor,
+&TraverseStateless>,
+&FunctorRef);
+ }
+
+private:
+ template
+ static CXChildVisitResult TraverseStateless(CXCursor cx, CXCursor parent,
+ CXClientData data) {
+TState *State = static_cast(data);
+return State->get()(cx, parent);
+ }
+};
+
+TEST_F(LibclangParseTest, SpellingLocation) {
+ MapUnsavedFile("main.cpp",
+"#define BANANAS 4011\n"
+"int plu = BANANAS;");
+
+ ClangTU = clang_parseTranslationUnit(Index, "main.cpp", nullptr, 0,
+UnsavedFiles.data(), UnsavedFiles.size(), TUFlags);
+
+ bool sawInt = false;
+ Traverse([&](CXCursor cx, CXCursor) {
+if (cx.kind == CXCursor_IntegerLiteral) {
+ sawInt = true;
+ auto cxl = clang_getCursorLocation(cx);
+ CXFile expansionFile, spellingFile;
+ unsigned line, column, offset;
+ clang_getSpellingLocation(cxl, &expansionFile, &line, nullptr, nullptr);
+ EXPECT_EQ(2, line); // getSpellingLocation returns expansion location
+ clang_getRealSpellingLocation(cxl, &spellingFile, &line, &column, &offset);
+ EXPECT_TRUE(clang_File_isEqual(expansionFile, spellingFile));
+ EXPECT_EQ(1, line);
+ EXPECT_EQ(17, column);
+ EXPECT_EQ(16, offset);
+}
+return CXChildVisit_Recurse;
+ });
+ EXPECT_TRUE(sawInt);
+}
+
+class LibclangReparseTest : public LibclangParseTest {
+public:
void DisplayDiagnostics() {
unsigned NumDiagnostics = clang_getNumDiagnostics(ClangTU);
for (unsigned i = 0; i < NumDiagnostics; ++i) {
Index: tools/libclang/CXSourceLocation.cpp
===
--- tools/libclang/CXSourceLocation.cpp
+++ tools/libclang/CXSourceLocation.cpp
@@ -346,6 +346,42 @@
*offset = FileOffset;
}
+void clang_getRealSpellingLocation(CXSourceLocation location,
+ CXFile *file,
+ unsigned *line,
+ unsigned *column,
+ unsigned *offset) {
+
+ if (!isASTUnitSourceLocation(location)) {
+CXLoadedDiagnostic::decodeLocation(location, file, line, column, offset);
+return;
+ }
+
+ SourceLocation Loc = SourceLocation::getFromRawEncoding(location.int_data);
+
+ if (!location.ptr_data[0] || Loc.isInvalid())
+return createNullLocation(file, line, column, offset);
+
+ const SourceManager &SM =
+ *static_cast(location.ptr_data[0]);
+ SourceLocation SpellLoc = SM.getSpellingLoc(Loc);
+ std::pair LocInfo = SM.getDecomposedLoc(SpellLoc);
+ FileID FID = LocInfo.first;
+ unsigned FileOffset = LocInfo.second;
+
+ if (FID.isInvalid())
+return createNullLocation(file, line, column, offset);
+
+ if (file)
+*file = const_cast(SM.getFileEntryForID(FID));
+ if (line)
+*line = SM.getLineNumber(FID, FileOffset);
+ if (c
Re: [PATCH] D20131: Fixed crash during code completion in file included within declaration
cameron314 added a comment.
I fixed this last July so the details are a little hazy, but fortunately it
turns out I documented what I had found:
> It seems when clang parses an enum declaration, it first parses the
> declaration specifiers, then stops if a semicolon is present.
> The trouble is, an #include could cause the file to change part-way through
> the declaration. If completion is enabled, then when leaving the included
> file in which the completion was requested, the tokenizer is cleared (clang
> tries to simulate an EOF at that point to prevent further parsing).
> But, while the tokenizer is cleared, the rest of the declaration still needs
> to be parsed, and the peeked token (a semicolon here) can no longer be
> advanced over, since it refers in this case to a token in a tokenizer that
> has been freed. Boom, null reference exception!
> It seems changing the lexer kind to the caching lexer when the artificial
> EOF is inserted does the right thing in this case – a cached EOF token is
> returned on the advancement of the peeked semicolon.
I tried to add a test; I can reproduce the crash, and with this patch it no
longer crashes, but because there's a subsequent parsing error after the
completion point, when passing via `clang.exe` only the error diagnostics are
printed without any of the completions (compared to libclang which allows you
to retrieve both the completions and the diagnostics). This means I cannot
write a RUN command in the lit test without it failing (the exit code is
non-zero). I have no way to distinguish between a crash and the normal errors
(with hidden completions). Is there any other way to test this?
Comment at: lib/Lex/PPLexerChange.cpp:380-382
@@ -379,3 +379,5 @@
CurLexer->FormTokenWithChars(Result, CurLexer->BufferEnd, tok::eof);
CurLexer.reset();
+if (CurLexerKind == CLK_Lexer)
+ CurLexerKind = CLK_CachingLexer;
} else {
rsmith wrote:
> Can you use `CurLexer->cutOffLexing()` instead?
Nope, still crashes when I try (before the reset, obviously).
http://reviews.llvm.org/D20131
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20131: Fixed crash during code completion in file included within declaration
cameron314 updated this revision to Diff 56961.
cameron314 added a comment.
Ah, perfect, thanks.
Behold, a test that fails without the patch and passes with it :-)
http://reviews.llvm.org/D20131
Files:
lib/Lex/PPLexerChange.cpp
test/CodeCompletion/include-within-declaration.c
Index: test/CodeCompletion/include-within-declaration.c
===
--- /dev/null
+++ test/CodeCompletion/include-within-declaration.c
@@ -0,0 +1,11 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: echo 'MACRO(here)' > %t/y.h
+// Clang will generate an error in this case (though internally the correct
completions
+// are present -- this can be seen via libclang) but it should not crash. If
it crashes
+// then `not` will fail, otherwise the test succeeds.
+// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%t/y.h:1:7 -I %t %s
+
+enum {
+#define MACRO(a) FOO
+#include "y.h"
+};
Index: lib/Lex/PPLexerChange.cpp
===
--- lib/Lex/PPLexerChange.cpp
+++ lib/Lex/PPLexerChange.cpp
@@ -378,6 +378,8 @@
Result.startToken();
CurLexer->FormTokenWithChars(Result, CurLexer->BufferEnd, tok::eof);
CurLexer.reset();
+if (CurLexerKind == CLK_Lexer)
+ CurLexerKind = CLK_CachingLexer;
} else {
assert(CurPTHLexer && "Got EOF but no current lexer set!");
CurPTHLexer->getEOF(Result);
Index: test/CodeCompletion/include-within-declaration.c
===
--- /dev/null
+++ test/CodeCompletion/include-within-declaration.c
@@ -0,0 +1,11 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: echo 'MACRO(here)' > %t/y.h
+// Clang will generate an error in this case (though internally the correct completions
+// are present -- this can be seen via libclang) but it should not crash. If it crashes
+// then `not` will fail, otherwise the test succeeds.
+// RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%t/y.h:1:7 -I %t %s
+
+enum {
+#define MACRO(a) FOO
+#include "y.h"
+};
Index: lib/Lex/PPLexerChange.cpp
===
--- lib/Lex/PPLexerChange.cpp
+++ lib/Lex/PPLexerChange.cpp
@@ -378,6 +378,8 @@
Result.startToken();
CurLexer->FormTokenWithChars(Result, CurLexer->BufferEnd, tok::eof);
CurLexer.reset();
+if (CurLexerKind == CLK_Lexer)
+ CurLexerKind = CLK_CachingLexer;
} else {
assert(CurPTHLexer && "Got EOF but no current lexer set!");
CurPTHLexer->getEOF(Result);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20131: Fixed crash during code completion in file included within declaration
cameron314 added inline comments.
Comment at: lib/Lex/PPLexerChange.cpp:380-383
@@ -379,4 +379,6 @@
CurLexer->FormTokenWithChars(Result, CurLexer->BufferEnd, tok::eof);
CurLexer.reset();
+if (CurLexerKind == CLK_Lexer)
+ CurLexerKind = CLK_CachingLexer;
} else {
assert(CurPTHLexer && "Got EOF but no current lexer set!");
Ah, wait, yes this also prevents the crash if I remove the `CurLexer.reset()`
and use `CurLexer->cutOffLexing()`, but it not only produces an error about a
missing '}', but an error about an extra '}' just after, which doesn't really
make sense. That means it's inspecting tokens past the EOF that was injected.
http://reviews.llvm.org/D20131
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20132: [libclang] Add clang_getAllSkippedRanges function
cameron314 updated this revision to Diff 56964.
cameron314 added a comment.
Here's a test!
http://reviews.llvm.org/D20132
Files:
include/clang-c/Index.h
tools/libclang/CIndex.cpp
unittests/libclang/LibclangTest.cpp
Index: unittests/libclang/LibclangTest.cpp
===
--- unittests/libclang/LibclangTest.cpp
+++ unittests/libclang/LibclangTest.cpp
@@ -15,6 +15,9 @@
#include "gtest/gtest.h"
#include
#include
+#include
+#include
+#include
#define DEBUG_TYPE "libclang-test"
TEST(libclang, clang_parseTranslationUnit2_InvalidArgs) {
@@ -349,21 +352,25 @@
clang_ModuleMapDescriptor_dispose(MMD);
}
-class LibclangReparseTest : public ::testing::Test {
+class LibclangParseTest : public ::testing::Test {
std::set Files;
+ typedef std::unique_ptr fixed_addr_string;
+ std::map UnsavedFileContents;
public:
std::string TestDir;
CXIndex Index;
CXTranslationUnit ClangTU;
unsigned TUFlags;
+ std::vector UnsavedFiles;
void SetUp() override {
llvm::SmallString<256> Dir;
ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("libclang-test", Dir));
TestDir = Dir.str();
TUFlags = CXTranslationUnit_DetailedPreprocessingRecord |
- clang_defaultEditingTranslationUnitOptions();
+ clang_defaultEditingTranslationUnitOptions();
Index = clang_createIndex(0, 0);
+ClangTU = nullptr;
}
void TearDown() override {
clang_disposeTranslationUnit(ClangTU);
@@ -384,6 +391,77 @@
OS << Contents;
assert(OS.good());
}
+ void MapUnsavedFile(std::string Filename, const std::string &Contents) {
+if (!llvm::sys::path::is_absolute(Filename)) {
+ llvm::SmallString<256> Path(TestDir);
+ llvm::sys::path::append(Path, Filename);
+ Filename = Path.str();
+}
+auto it = UnsavedFileContents.emplace(
+fixed_addr_string(new std::string(Filename)),
+fixed_addr_string(new std::string(Contents)));
+UnsavedFiles.push_back({
+it.first->first->c_str(), // filename
+it.first->second->c_str(), // contents
+it.first->second->size()// length
+});
+ }
+ template
+ void Traverse(const F &TraversalFunctor) {
+CXCursor TuCursor = clang_getTranslationUnitCursor(ClangTU);
+std::reference_wrapper FunctorRef = std::cref(TraversalFunctor);
+clang_visitChildren(TuCursor,
+&TraverseStateless>,
+&FunctorRef);
+ }
+private:
+ template
+ static CXChildVisitResult TraverseStateless(CXCursor cx, CXCursor parent,
+ CXClientData data) {
+TState *State = static_cast(data);
+return State->get()(cx, parent);
+ }
+};
+
+TEST_F(LibclangParseTest, AllSkippedRanges) {
+ std::string Header = "header.h", Main = "main.cpp";
+ WriteFile(Header,
+"#ifdef MANGOS\n"
+"printf(\"mmm\");\n"
+"#endif");
+ WriteFile(Main,
+"#include \"header.h\"\n"
+"#ifdef KIWIS\n"
+"printf(\"mmm!!\");\n"
+"#endif");
+
+ ClangTU = clang_parseTranslationUnit(Index, Main.c_str(), nullptr, 0,
+ nullptr, 0, TUFlags);
+
+ CXSourceRangeList *Ranges = clang_getAllSkippedRanges(ClangTU);
+ EXPECT_EQ(2, Ranges->count);
+
+ CXSourceLocation cxl;
+ unsigned line;
+ cxl = clang_getRangeStart(Ranges->ranges[0]);
+ clang_getSpellingLocation(cxl, nullptr, &line, nullptr, nullptr);
+ EXPECT_EQ(1, line);
+ cxl = clang_getRangeEnd(Ranges->ranges[0]);
+ clang_getSpellingLocation(cxl, nullptr, &line, nullptr, nullptr);
+ EXPECT_EQ(3, line);
+
+ cxl = clang_getRangeStart(Ranges->ranges[1]);
+ clang_getSpellingLocation(cxl, nullptr, &line, nullptr, nullptr);
+ EXPECT_EQ(2, line);
+ cxl = clang_getRangeEnd(Ranges->ranges[1]);
+ clang_getSpellingLocation(cxl, nullptr, &line, nullptr, nullptr);
+ EXPECT_EQ(4, line);
+
+ clang_disposeSourceRangeList(Ranges);
+}
+
+class LibclangReparseTest : public LibclangParseTest {
+public:
void DisplayDiagnostics() {
unsigned NumDiagnostics = clang_getNumDiagnostics(ClangTU);
for (unsigned i = 0; i < NumDiagnostics; ++i) {
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -7680,6 +7680,33 @@
return skipped;
}
+CXSourceRangeList *clang_getAllSkippedRanges(CXTranslationUnit TU) {
+ CXSourceRangeList *skipped = new CXSourceRangeList;
+ skipped->count = 0;
+ skipped->ranges = nullptr;
+
+ if (isNotUsableTU(TU)) {
+LOG_BAD_TU(TU);
+return skipped;
+ }
+
+ ASTUnit *astUnit = cxtu::getASTUnit(TU);
+ PreprocessingRecord *ppRec = astUnit->getPreprocessor().getPreprocessingRecord();
+ if (!ppRec)
+return skipped;
+
+ ASTContext &Ctx = astUnit->getASTContext();
+
+ const std::vector &SkippedRanges = ppRec->getSkippedRanges();
+
+ skipped->count = SkippedRanges.size();
+ skipped->ranges = new CXSourceRange[skipped->count];
+ for (unsigned i = 0, ei = skipped->count; i != ei; ++i
Re: [PATCH] D20134: [libclang] Fixed bug where ranges in spelling locations (in macro expansions) would get mangled
cameron314 added a comment. You're right, this breaks that test. The corner case that it exercises is arguably less important than breaking all spelling ranges, though. But I'm going to see if I can fix both with a little wizardry (ideally there wouldn't be open ranges in the first place, since clang doesn't really support them, but failing that we can distinguish between an inclusive and exclusive source location internally and that might be enough). http://reviews.llvm.org/D20134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20137: [PCH] Fixed bugs with preamble invalidation when files change (on Windows)
cameron314 updated this revision to Diff 57222.
cameron314 added a comment.
Updated patch to include later fixes I had made after the initial change.
http://reviews.llvm.org/D20137
Files:
include/clang/Basic/FileManager.h
include/clang/Frontend/ASTUnit.h
lib/Basic/FileManager.cpp
lib/Frontend/ASTUnit.cpp
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1378,7 +1378,7 @@
// First, make a record of those files that have been overridden via
// remapping or unsaved_files.
- llvm::StringMap OverriddenFiles;
+ std::map OverriddenFiles;
for (const auto &R : PreprocessorOpts.RemappedFiles) {
if (AnyFileChanged)
break;
@@ -1391,40 +1391,47 @@
break;
}
-OverriddenFiles[R.first] = PreambleFileHash::createForFile(
+OverriddenFiles[Status.getUniqueID()] = PreambleFileHash::createForFile(
Status.getSize(), Status.getLastModificationTime().toEpochTime());
}
for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
if (AnyFileChanged)
break;
-OverriddenFiles[RB.first] =
+
+vfs::Status Status;
+if (FileMgr->getNoncachedStatValue(RB.first, Status)) {
+ AnyFileChanged = true;
+ break;
+}
+
+OverriddenFiles[Status.getUniqueID()] =
PreambleFileHash::createForMemoryBuffer(RB.second);
}
// Check whether anything has changed.
- for (llvm::StringMap::iterator
+ for (FilesInPreambleMap::iterator
F = FilesInPreamble.begin(), FEnd = FilesInPreamble.end();
!AnyFileChanged && F != FEnd;
++F) {
-llvm::StringMap::iterator Overridden
- = OverriddenFiles.find(F->first());
+std::map::iterator Overridden
+ = OverriddenFiles.find(F->first);
if (Overridden != OverriddenFiles.end()) {
// This file was remapped; check whether the newly-mapped file
// matches up with the previous mapping.
- if (Overridden->second != F->second)
+ if (Overridden->second != F->second.second)
AnyFileChanged = true;
continue;
}
// The file was not remapped; check whether it has changed on disk.
vfs::Status Status;
-if (FileMgr->getNoncachedStatValue(F->first(), Status)) {
+if (FileMgr->getNoncachedStatValue(F->second.first, Status)) {
// If we can't stat the file, assume that something horrible happened.
AnyFileChanged = true;
-} else if (Status.getSize() != uint64_t(F->second.Size) ||
+} else if (Status.getSize() != uint64_t(F->second.second.Size) ||
Status.getLastModificationTime().toEpochTime() !=
- uint64_t(F->second.ModTime))
+ uint64_t(F->second.second.ModTime))
AnyFileChanged = true;
}
@@ -1612,12 +1619,14 @@
if (!File || File == SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()))
continue;
if (time_t ModTime = File->getModificationTime()) {
- FilesInPreamble[File->getName()] = PreambleFileHash::createForFile(
- File->getSize(), ModTime);
+ FilesInPreamble[File->getUniqueID()] = std::make_pair(
+File->getName(),
+PreambleFileHash::createForFile(File->getSize(), ModTime));
} else {
llvm::MemoryBuffer *Buffer = SourceMgr.getMemoryBufferForFile(File);
- FilesInPreamble[File->getName()] =
- PreambleFileHash::createForMemoryBuffer(Buffer);
+ FilesInPreamble[File->getUniqueID()] = std::make_pair(
+File->getName(),
+PreambleFileHash::createForMemoryBuffer(Buffer));
}
}
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -301,6 +301,11 @@
return &UFE;
}
+ if (UFE.isVirtual()) {
+UFE.Name = InterndFileName;
+return &UFE;
+ }
+
// Otherwise, we don't have this file yet, add it.
UFE.Name= InterndFileName;
UFE.Size = Data.Size;
@@ -312,6 +317,7 @@
UFE.InPCH = Data.InPCH;
UFE.File = std::move(F);
UFE.IsValid = true;
+ UFE.IsVirtual = true;
return &UFE;
}
Index: include/clang/Frontend/ASTUnit.h
===
--- include/clang/Frontend/ASTUnit.h
+++ include/clang/Frontend/ASTUnit.h
@@ -31,6 +31,7 @@
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/MD5.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/FileSystem.h"
#include
#include
#include
@@ -264,12 +265,15 @@
/// a line after skipping the preamble.
bool PreambleEndsAtStartOfLine;
+ typedef std::map> FilesInPreambleMap;
+
/// \brief Keeps track of the files that we
Re: [PATCH] D20137: [PCH] Fixed bugs with preamble invalidation when files change (on Windows)
cameron314 added inline comments.
Comment at: lib/Basic/FileManager.cpp:304-307
@@ -303,1 +303,6 @@
+ if (UFE.isVirtual()) {
+UFE.Name = InterndFileName;
+return &UFE;
+ }
+
rsmith wrote:
> It looks like this is unreachable: `IsVirtual` is only ever `true` when
> `IsValid` is also `true`, and we don't reach this line if `UFE.isValid()`. As
> this is the only consumer of `FileEntry::IsValid`, it looks like it does
> nothing. Am I missing something?
Hmm, interesting, good catch. Back when I made this change UFE.IsValid was
//not// set to true in the fall-through after this if, which is why I had to
introduce `IsVirtual` in the first place.
But looking at the history on this file, it seems it //was// set to true since
the very beginning. I think we'd removed it in our local fork at one point to
work around a different bug, but it's back to normal in ours as well. So I
guess this part of the patch is unnecessary now. Yay! I'll remove it, it wasn't
very pretty anyway.
Comment at: lib/Frontend/ASTUnit.cpp:1402-1406
@@ +1401,7 @@
+
+vfs::Status Status;
+if (FileMgr->getNoncachedStatValue(RB.first, Status)) {
+ AnyFileChanged = true;
+ break;
+}
+
rsmith wrote:
> If there are multiple file names with the same inode, `RB.first` is an
> arbitrarily-chosen one of those names, and stat'ing it isn't sufficient to
> check that none of the other names for the file have changed. Maybe we need
> to store a list of filenames used for each `UniqueID`?
I'm not sure I understand -- if the file changed on disk, why would it matter
which name we're accessing it by?
http://reviews.llvm.org/D20137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20137: [PCH] Fixed bugs with preamble invalidation when files change (on Windows)
cameron314 updated this revision to Diff 57253.
cameron314 added a comment.
Removed workaround for case that can no longer happen.
http://reviews.llvm.org/D20137
Files:
include/clang/Frontend/ASTUnit.h
lib/Frontend/ASTUnit.cpp
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1378,7 +1378,7 @@
// First, make a record of those files that have been overridden via
// remapping or unsaved_files.
- llvm::StringMap OverriddenFiles;
+ std::map OverriddenFiles;
for (const auto &R : PreprocessorOpts.RemappedFiles) {
if (AnyFileChanged)
break;
@@ -1391,40 +1391,47 @@
break;
}
-OverriddenFiles[R.first] = PreambleFileHash::createForFile(
+OverriddenFiles[Status.getUniqueID()] = PreambleFileHash::createForFile(
Status.getSize(), Status.getLastModificationTime().toEpochTime());
}
for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
if (AnyFileChanged)
break;
-OverriddenFiles[RB.first] =
+
+vfs::Status Status;
+if (FileMgr->getNoncachedStatValue(RB.first, Status)) {
+ AnyFileChanged = true;
+ break;
+}
+
+OverriddenFiles[Status.getUniqueID()] =
PreambleFileHash::createForMemoryBuffer(RB.second);
}
// Check whether anything has changed.
- for (llvm::StringMap::iterator
+ for (FilesInPreambleMap::iterator
F = FilesInPreamble.begin(), FEnd = FilesInPreamble.end();
!AnyFileChanged && F != FEnd;
++F) {
-llvm::StringMap::iterator Overridden
- = OverriddenFiles.find(F->first());
+std::map::iterator Overridden
+ = OverriddenFiles.find(F->first);
if (Overridden != OverriddenFiles.end()) {
// This file was remapped; check whether the newly-mapped file
// matches up with the previous mapping.
- if (Overridden->second != F->second)
+ if (Overridden->second != F->second.second)
AnyFileChanged = true;
continue;
}
// The file was not remapped; check whether it has changed on disk.
vfs::Status Status;
-if (FileMgr->getNoncachedStatValue(F->first(), Status)) {
+if (FileMgr->getNoncachedStatValue(F->second.first, Status)) {
// If we can't stat the file, assume that something horrible happened.
AnyFileChanged = true;
-} else if (Status.getSize() != uint64_t(F->second.Size) ||
+} else if (Status.getSize() != uint64_t(F->second.second.Size) ||
Status.getLastModificationTime().toEpochTime() !=
- uint64_t(F->second.ModTime))
+ uint64_t(F->second.second.ModTime))
AnyFileChanged = true;
}
@@ -1612,12 +1619,14 @@
if (!File || File == SourceMgr.getFileEntryForID(SourceMgr.getMainFileID()))
continue;
if (time_t ModTime = File->getModificationTime()) {
- FilesInPreamble[File->getName()] = PreambleFileHash::createForFile(
- File->getSize(), ModTime);
+ FilesInPreamble[File->getUniqueID()] = std::make_pair(
+File->getName(),
+PreambleFileHash::createForFile(File->getSize(), ModTime));
} else {
llvm::MemoryBuffer *Buffer = SourceMgr.getMemoryBufferForFile(File);
- FilesInPreamble[File->getName()] =
- PreambleFileHash::createForMemoryBuffer(Buffer);
+ FilesInPreamble[File->getUniqueID()] = std::make_pair(
+File->getName(),
+PreambleFileHash::createForMemoryBuffer(Buffer));
}
}
Index: include/clang/Frontend/ASTUnit.h
===
--- include/clang/Frontend/ASTUnit.h
+++ include/clang/Frontend/ASTUnit.h
@@ -31,6 +31,7 @@
#include "llvm/ADT/StringMap.h"
#include "llvm/Support/MD5.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/FileSystem.h"
#include
#include
#include
@@ -264,12 +265,15 @@
/// a line after skipping the preamble.
bool PreambleEndsAtStartOfLine;
+ typedef std::map> FilesInPreambleMap;
+
/// \brief Keeps track of the files that were used when computing the
/// preamble, with both their buffer size and their modification time.
///
/// If any of the files have changed from one compile to the next,
/// the preamble must be thrown away.
- llvm::StringMap FilesInPreamble;
+ FilesInPreambleMap FilesInPreamble;
/// \brief When non-NULL, this is the buffer used to store the contents of
/// the main file when it has been padded for use with the precompiled
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20137: [PCH] Fixed bugs with preamble invalidation when files change (on Windows)
cameron314 added inline comments.
Comment at: lib/Frontend/ASTUnit.cpp:1402-1406
@@ +1401,7 @@
+
+vfs::Status Status;
+if (FileMgr->getNoncachedStatValue(RB.first, Status)) {
+ AnyFileChanged = true;
+ break;
+}
+
rsmith wrote:
> Suppose file names A and B refer to file (inode) X, and the map contains X ->
> (A, hash). If B is deleted (and maybe recreated pointing at a new inode),
> this approach won't detect that anything has changed.
Ah, I understand. The problem isn't really with the overridden file map, which
still has to be by inode in order for lookups to find the right file regardless
of name, but with the FilesInPreamble map, which needs to stay as a list of
independent filenames in case the inodes associated to those filenames changes
in between parses.
I'll make the change, thanks!
http://reviews.llvm.org/D20137
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20137: [PCH] Fixed bugs with preamble invalidation when files change (on Windows)
cameron314 updated the summary for this revision.
cameron314 updated this revision to Diff 57367.
cameron314 added a comment.
This version of the patch takes into account potential changes between
filenames and their unique IDs between parses.
http://reviews.llvm.org/D20137
Files:
lib/Frontend/ASTUnit.cpp
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1378,7 +1378,7 @@
// First, make a record of those files that have been overridden via
// remapping or unsaved_files.
- llvm::StringMap OverriddenFiles;
+ std::map OverriddenFiles;
for (const auto &R : PreprocessorOpts.RemappedFiles) {
if (AnyFileChanged)
break;
@@ -1391,24 +1391,38 @@
break;
}
-OverriddenFiles[R.first] = PreambleFileHash::createForFile(
+OverriddenFiles[Status.getUniqueID()] =
PreambleFileHash::createForFile(
Status.getSize(), Status.getLastModificationTime().toEpochTime());
}
for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
if (AnyFileChanged)
break;
-OverriddenFiles[RB.first] =
+
+vfs::Status Status;
+if (FileMgr->getNoncachedStatValue(RB.first, Status)) {
+ AnyFileChanged = true;
+ break;
+}
+
+OverriddenFiles[Status.getUniqueID()] =
PreambleFileHash::createForMemoryBuffer(RB.second);
}
// Check whether anything has changed.
- for (llvm::StringMap::iterator
+ for (llvm::StringMap::iterator
F = FilesInPreamble.begin(), FEnd = FilesInPreamble.end();
!AnyFileChanged && F != FEnd;
++F) {
-llvm::StringMap::iterator Overridden
- = OverriddenFiles.find(F->first());
+vfs::Status Status;
+if (FileMgr->getNoncachedStatValue(F->first(), Status)) {
+ // If we can't stat the file, assume that something horrible
happened.
+ AnyFileChanged = true;
+ break;
+}
+
+std::map::iterator
Overridden
+ = OverriddenFiles.find(Status.getUniqueID());
if (Overridden != OverriddenFiles.end()) {
// This file was remapped; check whether the newly-mapped file
// matches up with the previous mapping.
@@ -1418,11 +1432,7 @@
}
// The file was not remapped; check whether it has changed on disk.
-vfs::Status Status;
-if (FileMgr->getNoncachedStatValue(F->first(), Status)) {
- // If we can't stat the file, assume that something horrible
happened.
- AnyFileChanged = true;
-} else if (Status.getSize() != uint64_t(F->second.Size) ||
+if (Status.getSize() != uint64_t(F->second.Size) ||
Status.getLastModificationTime().toEpochTime() !=
uint64_t(F->second.ModTime))
AnyFileChanged = true;
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1378,7 +1378,7 @@
// First, make a record of those files that have been overridden via
// remapping or unsaved_files.
- llvm::StringMap OverriddenFiles;
+ std::map OverriddenFiles;
for (const auto &R : PreprocessorOpts.RemappedFiles) {
if (AnyFileChanged)
break;
@@ -1391,24 +1391,38 @@
break;
}
-OverriddenFiles[R.first] = PreambleFileHash::createForFile(
+OverriddenFiles[Status.getUniqueID()] = PreambleFileHash::createForFile(
Status.getSize(), Status.getLastModificationTime().toEpochTime());
}
for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
if (AnyFileChanged)
break;
-OverriddenFiles[RB.first] =
+
+vfs::Status Status;
+if (FileMgr->getNoncachedStatValue(RB.first, Status)) {
+ AnyFileChanged = true;
+ break;
+}
+
+OverriddenFiles[Status.getUniqueID()] =
PreambleFileHash::createForMemoryBuffer(RB.second);
}
// Check whether anything has changed.
- for (llvm::StringMap::iterator
+ for (llvm::StringMap::iterator
F = FilesInPreamble.begin(), FEnd = FilesInPreamble.end();
!AnyFileChanged && F != FEnd;
++F) {
-llvm::StringMap::iterator Overridden
- = OverriddenFiles.find(F->first());
+vfs::Status Status;
+if (FileMgr->getNoncachedStatValue(F->first(), Status)) {
+ // If we can't stat the file, assume that something horrible happened.
+ AnyFileChanged = true;
+ break;
+}
+
+std::map::iterator Overridden
+ = OverriddenFiles.find(Status.getUniqueID());
if (Overridden != OverriddenF
Re: [PATCH] D20137: [PCH] Fixed bugs with preamble invalidation when files change (on Windows)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL269769: [PCH] Fixed bug with preamble invalidation when
overridden files change (authored by cameron314).
Changed prior to commit:
http://reviews.llvm.org/D20137?vs=57367&id=57477#toc
Repository:
rL LLVM
http://reviews.llvm.org/D20137
Files:
cfe/trunk/lib/Frontend/ASTUnit.cpp
Index: cfe/trunk/lib/Frontend/ASTUnit.cpp
===
--- cfe/trunk/lib/Frontend/ASTUnit.cpp
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp
@@ -1378,7 +1378,7 @@
// First, make a record of those files that have been overridden via
// remapping or unsaved_files.
- llvm::StringMap OverriddenFiles;
+ std::map OverriddenFiles;
for (const auto &R : PreprocessorOpts.RemappedFiles) {
if (AnyFileChanged)
break;
@@ -1391,24 +1391,38 @@
break;
}
-OverriddenFiles[R.first] = PreambleFileHash::createForFile(
+OverriddenFiles[Status.getUniqueID()] =
PreambleFileHash::createForFile(
Status.getSize(), Status.getLastModificationTime().toEpochTime());
}
for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
if (AnyFileChanged)
break;
-OverriddenFiles[RB.first] =
+
+vfs::Status Status;
+if (FileMgr->getNoncachedStatValue(RB.first, Status)) {
+ AnyFileChanged = true;
+ break;
+}
+
+OverriddenFiles[Status.getUniqueID()] =
PreambleFileHash::createForMemoryBuffer(RB.second);
}
// Check whether anything has changed.
- for (llvm::StringMap::iterator
+ for (llvm::StringMap::iterator
F = FilesInPreamble.begin(), FEnd = FilesInPreamble.end();
!AnyFileChanged && F != FEnd;
++F) {
-llvm::StringMap::iterator Overridden
- = OverriddenFiles.find(F->first());
+vfs::Status Status;
+if (FileMgr->getNoncachedStatValue(F->first(), Status)) {
+ // If we can't stat the file, assume that something horrible
happened.
+ AnyFileChanged = true;
+ break;
+}
+
+std::map::iterator
Overridden
+ = OverriddenFiles.find(Status.getUniqueID());
if (Overridden != OverriddenFiles.end()) {
// This file was remapped; check whether the newly-mapped file
// matches up with the previous mapping.
@@ -1418,13 +1432,9 @@
}
// The file was not remapped; check whether it has changed on disk.
-vfs::Status Status;
-if (FileMgr->getNoncachedStatValue(F->first(), Status)) {
- // If we can't stat the file, assume that something horrible
happened.
- AnyFileChanged = true;
-} else if (Status.getSize() != uint64_t(F->second.Size) ||
- Status.getLastModificationTime().toEpochTime() !=
- uint64_t(F->second.ModTime))
+if (Status.getSize() != uint64_t(F->second.Size) ||
+Status.getLastModificationTime().toEpochTime() !=
+uint64_t(F->second.ModTime))
AnyFileChanged = true;
}
Index: cfe/trunk/lib/Frontend/ASTUnit.cpp
===
--- cfe/trunk/lib/Frontend/ASTUnit.cpp
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp
@@ -1378,7 +1378,7 @@
// First, make a record of those files that have been overridden via
// remapping or unsaved_files.
- llvm::StringMap OverriddenFiles;
+ std::map OverriddenFiles;
for (const auto &R : PreprocessorOpts.RemappedFiles) {
if (AnyFileChanged)
break;
@@ -1391,24 +1391,38 @@
break;
}
-OverriddenFiles[R.first] = PreambleFileHash::createForFile(
+OverriddenFiles[Status.getUniqueID()] = PreambleFileHash::createForFile(
Status.getSize(), Status.getLastModificationTime().toEpochTime());
}
for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
if (AnyFileChanged)
break;
-OverriddenFiles[RB.first] =
+
+vfs::Status Status;
+if (FileMgr->getNoncachedStatValue(RB.first, Status)) {
+ AnyFileChanged = true;
+ break;
+}
+
+OverriddenFiles[Status.getUniqueID()] =
PreambleFileHash::createForMemoryBuffer(RB.second);
}
// Check whether anything has changed.
- for (llvm::StringMap::iterator
+ for (llvm::StringMap::iterator
F = FilesInPreamble.begin(), FEnd = FilesInPreamble.end();
!AnyFileChanged && F != FEnd;
++F) {
-llvm::StringMap::iterator Overridden
- = OverriddenFiles.find(F->first());
+vfs::Status Status;
+if (FileMgr->getNoncachedStatValue(F->first(), Status)) {
+ // If we can'
[PATCH] D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged
cameron314 created this revision.
cameron314 added a reviewer: rsmith.
cameron314 added a subscriber: cfe-commits.
cameron314 set the repository for this revision to rL LLVM.
Remapped files would always cause the preamble's PCH to be invalidated (even if
they hadn't changed) because the file manager was replacing the remapped
virtual entry (no modified time) with a real file entry (which has a modified
time) when an include directive was processed. This happens when the include
directive results in a slightly different path for the same file (e.g.
different slash direction, which happens very often on Windows).
Note: This was initially part of D20137 but I had incorrectly applied my own
patch, so the `IsVirtual = true` line was in the wrong spot and served no
purpose (and was subsequently removed from the patch). Now it actually does
something!
Repository:
rL LLVM
http://reviews.llvm.org/D20338
Files:
include/clang/Basic/FileManager.h
lib/Basic/FileManager.cpp
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -301,6 +301,11 @@
return &UFE;
}
+ if (UFE.isVirtual()) {
+UFE.Name = InterndFileName;
+return &UFE;
+ }
+
// Otherwise, we don't have this file yet, add it.
UFE.Name= InterndFileName;
UFE.Size = Data.Size;
@@ -381,6 +386,7 @@
UFE->Dir = DirInfo;
UFE->UID = NextFileUID++;
UFE->File.reset();
+ UFE->IsVirtual = true;
return UFE;
}
Index: include/clang/Basic/FileManager.h
===
--- include/clang/Basic/FileManager.h
+++ include/clang/Basic/FileManager.h
@@ -60,6 +60,7 @@
bool IsNamedPipe;
bool InPCH;
bool IsValid; // Is this \c FileEntry initialized and valid?
+ bool IsVirtual; // Is this \c FileEntry a virtual file?
/// \brief The open file, if it is owned by the \p FileEntry.
mutable std::unique_ptr File;
@@ -69,20 +70,23 @@
public:
FileEntry()
- : UniqueID(0, 0), IsNamedPipe(false), InPCH(false), IsValid(false)
+ : UniqueID(0, 0), IsNamedPipe(false), InPCH(false), IsValid(false),
+IsVirtual(false)
{}
// FIXME: this is here to allow putting FileEntry in std::map. Once we have
// emplace, we shouldn't need a copy constructor anymore.
/// Intentionally does not copy fields that are not set in an uninitialized
/// \c FileEntry.
FileEntry(const FileEntry &FE) : UniqueID(FE.UniqueID),
- IsNamedPipe(FE.IsNamedPipe), InPCH(FE.InPCH), IsValid(FE.IsValid) {
+ IsNamedPipe(FE.IsNamedPipe), InPCH(FE.InPCH), IsValid(FE.IsValid),
+ IsVirtual(FE.IsVirtual) {
assert(!isValid() && "Cannot copy an initialized FileEntry");
}
const char *getName() const { return Name; }
bool isValid() const { return IsValid; }
+ bool isVirtual() const { return IsVirtual; }
off_t getSize() const { return Size; }
unsigned getUID() const { return UID; }
const llvm::sys::fs::UniqueID &getUniqueID() const { return UniqueID; }
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -301,6 +301,11 @@
return &UFE;
}
+ if (UFE.isVirtual()) {
+UFE.Name = InterndFileName;
+return &UFE;
+ }
+
// Otherwise, we don't have this file yet, add it.
UFE.Name= InterndFileName;
UFE.Size = Data.Size;
@@ -381,6 +386,7 @@
UFE->Dir = DirInfo;
UFE->UID = NextFileUID++;
UFE->File.reset();
+ UFE->IsVirtual = true;
return UFE;
}
Index: include/clang/Basic/FileManager.h
===
--- include/clang/Basic/FileManager.h
+++ include/clang/Basic/FileManager.h
@@ -60,6 +60,7 @@
bool IsNamedPipe;
bool InPCH;
bool IsValid; // Is this \c FileEntry initialized and valid?
+ bool IsVirtual; // Is this \c FileEntry a virtual file?
/// \brief The open file, if it is owned by the \p FileEntry.
mutable std::unique_ptr File;
@@ -69,20 +70,23 @@
public:
FileEntry()
- : UniqueID(0, 0), IsNamedPipe(false), InPCH(false), IsValid(false)
+ : UniqueID(0, 0), IsNamedPipe(false), InPCH(false), IsValid(false),
+IsVirtual(false)
{}
// FIXME: this is here to allow putting FileEntry in std::map. Once we have
// emplace, we shouldn't need a copy constructor anymore.
/// Intentionally does not copy fields that are not set in an uninitialized
/// \c FileEntry.
FileEntry(const FileEntry &FE) : UniqueID(FE.UniqueID),
- IsNamedPipe(FE.IsNamedPipe), InPCH(FE.InPCH), IsValid(FE.IsValid) {
+ IsNamedPipe(FE.IsNamedPipe), InPCH(FE.InPCH), IsValid(FE.IsValid),
+ IsVirtual(FE.IsVirtual) {
assert(!isValid() && "Cannot copy an initialized FileEntry");
}
const char *getName() const { return Name; }
b
Re: [PATCH] D20243: [PCH] Disable inclusion of timestamps when generating pch files on windows.
cameron314 added a subscriber: cameron314. cameron314 added a comment. I've seen rare cases where parses using the PCH files were yielding completely invalid data, almost as if they were corrupted. I wonder now if this could be the cause. (We're on Windows too.) http://reviews.llvm.org/D20243 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19031: clang-format: Flexible line endings
cameron314 added a comment. ClangFormat still needs to know what kind of newline to insert when reformatting code. It already had the "auto-detect newline" behaviour; this patch simply allows the auto-detection to be overridden with an explicit setting. This is important for short files that have no newlines to start with, in which case the autodetection cannot work. http://reviews.llvm.org/D19031 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19031: clang-format: Flexible line endings
cameron314 added a comment. Sure :-) The motivation behind this patch (and a few more upcoming ones) is to make ClangFormat (the library) more friendly for IDE integration (which is our primary use case), in particular for on-the-fly auto-formatting. In an IDE auto-format setting, it's often the case that only fragments of the file at a time are being formatted. Often that means only a single line is passed as context. Or, sometimes, the file really is only one line to start with (e.g. formatting a file containing a one-line function declaration with the 'wrap after return type' option on needs to insert a newline). http://reviews.llvm.org/D19031 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19031: clang-format: Flexible line endings
cameron314 added a comment. Too late ;-) We've successfully integrated it as the engine behind our auto-formatting feature, passing only the minimal context necessary. We tried at first to supply the full file context, but that turned out to be much too slow in larger files. (We did a fair bit of testing with some of the real code our users have written.) We can, of course, change the newlines on the IDE side instead (indeed, we already have a few transforms on both the input and output). But it seemed to us that it would make more sense to have ClangFormat generate the correct newlines in the first place given that this is what it already attempts to do with its newline detection feature. And we felt this minor extension would be useful to others. http://reviews.llvm.org/D19031 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19031: clang-format: Flexible line endings
cameron314 added a comment. We did profile at one point, to see if it was something we could trivially speed up (it wasn't). Most of it was on the parsing/annotation side, yes (which makes sense, since that's the majority of the work when the format region itself is limited). I'm afraid I don't recall more details (this was a few months ago after we had just started). The memory usage spikes significantly, too. Anyway, we didn't want the size of the file to affect the insertion time of a semicolon if at all possible. All of our optimization boils down to only giving ClangFormat the minimum amount of code necessary to format the statement or block we want. The way we determine this context is almost certainly not something you want to see in ClangFormat :-) It's quite heuristic in some places, and is very specific to C/C++. Sometimes it will even bail out and not format anything if the context spans too many (e.g. 2000+) lines. There's multiple levels of context involved, too (e.g. when declaration alignment is on, adjacent statements that look like declarations are also included in the context to get things to line up). And fake braces to set up the indentation correctly. It's also all written in C#, leveraging a large body of existing parsing code that we'd already written (for features like smart indent); things taken for granted in the IDE, like O(1) access to the tokens that make up a given line (which is constantly maintained as the user types instead of having to reparse from the top of the file), are simply not possible to do as efficiently in ClangFormat (barring persistent caching between calls, which of course is a whole can of worms in itself). Anyway, all that stuff is very "porcelain-y" code (to borrow a term from the git developers) that would no doubt vary significantly from IDE to IDE. http://reviews.llvm.org/D19031 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D19031: clang-format: Flexible line endings
cameron314 added a comment. Ooh, and I should also add, there's a non-trivial cost to grabbing the entire file contents as a string and then converting it to UTF-8. If ClangFormat was faster, this would certainly become a significant bottleneck that can't really be worked around. http://reviews.llvm.org/D19031 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged
cameron314 added a comment. I'm not sure how to test this (originally I found this bug by stepping through with a debugger) -- is there a way to determine if an ASTUnit used a PCH for the preamble or not? I'd call the `getMainBufferWithPrecompiledPreamble` method manually but it's private. Repository: rL LLVM http://reviews.llvm.org/D20338 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged
cameron314 added a comment. Thanks @bruno, I'll have a look at using a VFS for the test. Comment at: lib/Basic/FileManager.cpp:389 @@ -383,2 +388,3 @@ UFE->File.reset(); + UFE->IsVirtual = true; return UFE; rsmith wrote: > Rather than adding this `IsVirtual` flag, could you just set `UFE->IsValid` > to `true` here? It looks like a simple oversight that this code fails to set > the `IsValid` flag properly. I could, but I didn't want to accidentally break something else that depends on virtual files not being valid. That's the type of change that can easily introduce a subtle bug not caught by the tests. Semantically, is a virtual file always valid? Repository: rL LLVM http://reviews.llvm.org/D20338 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [Diffusion] rL249410: [Tooling] Reuse FileManager in ASTUnit.
cameron314 added a subscriber: cfe-commits. http://reviews.llvm.org/rL249410 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged
cameron314 updated the summary for this revision.
cameron314 removed rL LLVM as the repository for this revision.
cameron314 updated this revision to Diff 59577.
cameron314 added a comment.
It took some modifications to the ASTUnit to support a virtual file system with
a PCH parse/reparse (preliminary VFS support had already been added in
http://reviews.llvm.org/rL249410 but it did not support initial parses using
PCHs, nor reparses), but I was finally able to write a test that checks that
the reparse actually uses the PCH with my fix, and rejects the PCH (rereading
everything and failing the test) without it.
http://reviews.llvm.org/D20338
Files:
include/clang/Basic/FileManager.h
include/clang/Frontend/ASTUnit.h
lib/Basic/FileManager.cpp
lib/Frontend/ASTUnit.cpp
unittests/Frontend/CMakeLists.txt
unittests/Frontend/PchPreambleTest.cpp
Index: unittests/Frontend/PchPreambleTest.cpp
===
--- /dev/null
+++ unittests/Frontend/PchPreambleTest.cpp
@@ -0,0 +1,155 @@
+//-- unittests/Frontend/PchPreambleTest.cpp - FrontendAction tests ---//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/FrontendOptions.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/FileManager.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+#include
+
+using namespace llvm;
+using namespace clang;
+
+namespace {
+
+class ReadCountingInMemoryFileSystem : public vfs::InMemoryFileSystem
+{
+ std::map ReadCounts;
+
+public:
+ ErrorOr> openFileForRead(const Twine &Path) override
+ {
+SmallVector PathVec;
+Path.toVector(PathVec);
+llvm::sys::path::remove_dots(PathVec, true);
+++ReadCounts[std::string(PathVec.begin(), PathVec.end())];
+return InMemoryFileSystem::openFileForRead(Path);
+ }
+
+ unsigned GetReadCount(const Twine &Path) const
+ {
+auto it = ReadCounts.find(Path.str());
+return it == ReadCounts.end() ? 0 : it->second;
+ }
+};
+
+class PchPreambleTest : public ::testing::Test {
+ IntrusiveRefCntPtr VFS;
+ StringMap RemappedFiles;
+ std::shared_ptr PCHContainerOpts;
+ FileSystemOptions FSOpts;
+
+public:
+ void SetUp() override {
+VFS = new ReadCountingInMemoryFileSystem();
+// We need the working directory to be set to something absolute,
+// otherwise it ends up being inadvertently set to the current
+// working directory in the real file system due to a series of
+// unfortunate conditions interacting badly.
+// What's more, this path *must* be absolute on all (real)
+// filesystems, so just '/' won't work (e.g. on Win32).
+VFS->setCurrentWorkingDirectory("//./");
+ }
+
+ void TearDown() override {
+ }
+
+ void AddFile(const std::string &Filename, const std::string &Contents) {
+::time_t now;
+::time(&now);
+VFS->addFile(Filename, now, MemoryBuffer::getMemBufferCopy(Contents, Filename));
+ }
+
+ void RemapFile(const std::string &Filename, const std::string &Contents) {
+RemappedFiles[Filename] = Contents;
+ }
+
+ std::unique_ptr ParseAST(const std::string &EntryFile) {
+PCHContainerOpts = std::make_shared();
+CompilerInvocation *CI = new CompilerInvocation;
+CI->getFrontendOpts().Inputs.push_back(
+ FrontendInputFile(EntryFile, FrontendOptions::getInputKindForExtension(
+llvm::sys::path::extension(EntryFile).substr(1;
+
+CI->getTargetOpts().Triple = "i386-unknown-linux-gnu";
+
+CI->getPreprocessorOpts().RemappedFileBuffers = GetRemappedFiles();
+
+PreprocessorOptions &PPOpts = CI->getPreprocessorOpts();
+PPOpts.RemappedFilesKeepOriginalName = true;
+
+IntrusiveRefCntPtr
+ Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions));
+
+FileManager *FileMgr = new FileManager(FSOpts, VFS);
+
+std::unique_ptr AST = ASTUnit::LoadFromCompilerInvocation(
+ CI, PCHContainerOpts, Diags, FileMgr, false, false,
+ /*PrecompilePreambleAfterNParses=*/1);
+return AST;
+ }
+
+ bool ReparseAST(const std::unique_ptr &AST) {
+FileManager *FileMgr = new FileManager(FSOpts, VFS);
+bool reparseFailed = AST->Reparse(PCHContainerOpts, GetRemappedFiles(), FileMgr);
+return reparseFailed;
+ }
+
+ unsigned GetFileReadCount(const std::string &Filename) const {
+return VFS->GetReadCount(Filename);
+ }
+
+private:
+ std::vector>
+ GetRemappedFiles() const {
+std::vector> Remapped;
+for (const auto &RemappedFile : RemappedFiles) {
+ std::unique_ptr buf = MemoryBuffer::getMemBufferC
Re: [PATCH] D20132: [libclang] Add clang_getAllSkippedRanges function
cameron314 added a comment. Can someone have a look at this now that there's a test? http://reviews.llvm.org/D20132 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged
cameron314 added a comment. This is a fairly important bug for anyone hosting clang as a library (e.g. IDEs). Can someone have a look at this patch when they have a free moment? http://reviews.llvm.org/D20338 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged
cameron314 added inline comments. Comment at: lib/Basic/FileManager.cpp:389 @@ -383,2 +388,3 @@ UFE->File.reset(); + UFE->IsVirtual = true; return UFE; rsmith wrote: > Yes. The `IsValid` flag is just supposed to mean that this file has actually > been added to the `UniqueRealFiles` map rather than having been > default-constructed by `operator[]`. Excellent then, I'll get rid of `IsVirtual` and use `IsValid` in place. This will condense things down to a one-line change plus a large diff for the test ^^ http://reviews.llvm.org/D20338 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged
cameron314 updated this revision to Diff 60250.
cameron314 added a comment.
Here's the final fix (it's the line in FileManager.cpp, plus a test).
http://reviews.llvm.org/D20338
Files:
include/clang/Frontend/ASTUnit.h
lib/Basic/FileManager.cpp
lib/Frontend/ASTUnit.cpp
unittests/Frontend/CMakeLists.txt
unittests/Frontend/PchPreambleTest.cpp
Index: unittests/Frontend/PchPreambleTest.cpp
===
--- /dev/null
+++ unittests/Frontend/PchPreambleTest.cpp
@@ -0,0 +1,155 @@
+//-- unittests/Frontend/PchPreambleTest.cpp - FrontendAction tests ---//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/FrontendOptions.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/FileManager.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "gtest/gtest.h"
+#include
+
+using namespace llvm;
+using namespace clang;
+
+namespace {
+
+class ReadCountingInMemoryFileSystem : public vfs::InMemoryFileSystem
+{
+ std::map ReadCounts;
+
+public:
+ ErrorOr> openFileForRead(const Twine &Path) override
+ {
+SmallVector PathVec;
+Path.toVector(PathVec);
+llvm::sys::path::remove_dots(PathVec, true);
+++ReadCounts[std::string(PathVec.begin(), PathVec.end())];
+return InMemoryFileSystem::openFileForRead(Path);
+ }
+
+ unsigned GetReadCount(const Twine &Path) const
+ {
+auto it = ReadCounts.find(Path.str());
+return it == ReadCounts.end() ? 0 : it->second;
+ }
+};
+
+class PchPreambleTest : public ::testing::Test {
+ IntrusiveRefCntPtr VFS;
+ StringMap RemappedFiles;
+ std::shared_ptr PCHContainerOpts;
+ FileSystemOptions FSOpts;
+
+public:
+ void SetUp() override {
+VFS = new ReadCountingInMemoryFileSystem();
+// We need the working directory to be set to something absolute,
+// otherwise it ends up being inadvertently set to the current
+// working directory in the real file system due to a series of
+// unfortunate conditions interacting badly.
+// What's more, this path *must* be absolute on all (real)
+// filesystems, so just '/' won't work (e.g. on Win32).
+VFS->setCurrentWorkingDirectory("//./");
+ }
+
+ void TearDown() override {
+ }
+
+ void AddFile(const std::string &Filename, const std::string &Contents) {
+::time_t now;
+::time(&now);
+VFS->addFile(Filename, now, MemoryBuffer::getMemBufferCopy(Contents, Filename));
+ }
+
+ void RemapFile(const std::string &Filename, const std::string &Contents) {
+RemappedFiles[Filename] = Contents;
+ }
+
+ std::unique_ptr ParseAST(const std::string &EntryFile) {
+PCHContainerOpts = std::make_shared();
+CompilerInvocation *CI = new CompilerInvocation;
+CI->getFrontendOpts().Inputs.push_back(
+ FrontendInputFile(EntryFile, FrontendOptions::getInputKindForExtension(
+llvm::sys::path::extension(EntryFile).substr(1;
+
+CI->getTargetOpts().Triple = "i386-unknown-linux-gnu";
+
+CI->getPreprocessorOpts().RemappedFileBuffers = GetRemappedFiles();
+
+PreprocessorOptions &PPOpts = CI->getPreprocessorOpts();
+PPOpts.RemappedFilesKeepOriginalName = true;
+
+IntrusiveRefCntPtr
+ Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions));
+
+FileManager *FileMgr = new FileManager(FSOpts, VFS);
+
+std::unique_ptr AST = ASTUnit::LoadFromCompilerInvocation(
+ CI, PCHContainerOpts, Diags, FileMgr, false, false,
+ /*PrecompilePreambleAfterNParses=*/1);
+return AST;
+ }
+
+ bool ReparseAST(const std::unique_ptr &AST) {
+FileManager *FileMgr = new FileManager(FSOpts, VFS);
+bool reparseFailed = AST->Reparse(PCHContainerOpts, GetRemappedFiles(), FileMgr);
+return reparseFailed;
+ }
+
+ unsigned GetFileReadCount(const std::string &Filename) const {
+return VFS->GetReadCount(Filename);
+ }
+
+private:
+ std::vector>
+ GetRemappedFiles() const {
+std::vector> Remapped;
+for (const auto &RemappedFile : RemappedFiles) {
+ std::unique_ptr buf = MemoryBuffer::getMemBufferCopy(
+RemappedFile.second, RemappedFile.first());
+ Remapped.emplace_back(RemappedFile.first(), buf.release());
+}
+return Remapped;
+ }
+};
+
+TEST_F(PchPreambleTest, ReparseWithOverriddenFileDoesNotInvalidatePreamble) {
+ std::string Header1 = "//./header1.h";
+ std::string Header2 = "//./header2.h";
+ std::string MainName = "//./main.cpp";
+ AddFile(Header1, "");
+ AddFile(Header2, "#pragma once");
+ AddFile(MainName,
+"#include \"//./foo/../header1.h\"\n"
+"#include \"
Re: [PATCH] D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged
cameron314 added a comment. Ping? :-) http://reviews.llvm.org/D20338 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D20338: [PCH] Fixed overridden files always invalidating preamble even when unchanged
cameron314 added a comment. Anyone have a few minutes to look at this? http://reviews.llvm.org/D20338 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
