[PATCH] D33042: [libclang] Allow to suspend a translation unit.
nik marked an inline comment as done. nik added a comment. Adapted to the changes that happened in the meanwhile (CINDEX_VERSION_MINOR increased, CleanTemporaryFiles got removed). https://reviews.llvm.org/D33042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33042: [libclang] Allow to suspend a translation unit.
nik updated this revision to Diff 99718.
nik edited the summary of this revision.
https://reviews.llvm.org/D33042
Files:
include/clang-c/Index.h
include/clang/Frontend/ASTUnit.h
lib/Frontend/ASTUnit.cpp
tools/c-index-test/c-index-test.c
tools/libclang/CIndex.cpp
tools/libclang/libclang.exports
Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -305,6 +305,7 @@
clang_remap_getNumFiles
clang_reparseTranslationUnit
clang_saveTranslationUnit
+clang_suspendTranslationUnit
clang_sortCodeCompletionResults
clang_toggleCrashRecovery
clang_tokenize
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3918,6 +3918,20 @@
}
}
+unsigned clang_suspendTranslationUnit(CXTranslationUnit CTUnit) {
+ if (CTUnit) {
+ASTUnit *Unit = cxtu::getASTUnit(CTUnit);
+
+if (Unit && Unit->isUnsafeToFree())
+ return false;
+
+Unit->ResetForParse();
+return true;
+ }
+
+ return false;
+}
+
unsigned clang_defaultReparseOptions(CXTranslationUnit TU) {
return CXReparse_None;
}
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -1742,6 +1742,8 @@
return -1;
if (Repeats > 1) {
+ clang_suspendTranslationUnit(TU);
+
Err = clang_reparseTranslationUnit(TU, num_unsaved_files, unsaved_files,
clang_defaultReparseOptions(TU));
if (Err != CXError_Success) {
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1020,8 +1020,6 @@
/// contain any translation-unit information, false otherwise.
bool ASTUnit::Parse(std::shared_ptr PCHContainerOps,
std::unique_ptr OverrideMainBuffer) {
- SavedMainFileBuffer.reset();
-
if (!Invocation)
return true;
@@ -1068,17 +1066,11 @@
Clang->createFileManager();
FileMgr = &Clang->getFileManager();
}
- SourceMgr = new SourceManager(getDiagnostics(), *FileMgr,
-UserFilesAreVolatile);
- TheSema.reset();
- Ctx = nullptr;
- PP = nullptr;
- Reader = nullptr;
- // Clear out old caches and data.
- TopLevelDecls.clear();
- clearFileLevelDecls();
+ ResetForParse();
+ SourceMgr = new SourceManager(getDiagnostics(), *FileMgr,
+UserFilesAreVolatile);
if (!OverrideMainBuffer) {
checkAndRemoveNonDriverDiags(StoredDiagnostics);
TopLevelDeclsInPreamble.clear();
@@ -2071,6 +2063,19 @@
return Result;
}
+void ASTUnit::ResetForParse() {
+ SavedMainFileBuffer.reset();
+
+ SourceMgr.reset();
+ TheSema.reset();
+ Ctx.reset();
+ PP.reset();
+ Reader.reset();
+
+ TopLevelDecls.clear();
+ clearFileLevelDecls();
+}
+
////
// Code completion
////
Index: include/clang/Frontend/ASTUnit.h
===
--- include/clang/Frontend/ASTUnit.h
+++ include/clang/Frontend/ASTUnit.h
@@ -852,6 +852,11 @@
bool Reparse(std::shared_ptr PCHContainerOps,
ArrayRef RemappedFiles = None);
+ /// \brief Free data that will be re-generated on the next parse.
+ ///
+ /// Preamble-related data is not affected.
+ void ResetForParse();
+
/// \brief Perform code completion at the given file, line, and
/// column within this translation unit.
///
Index: include/clang-c/Index.h
===
--- include/clang-c/Index.h
+++ include/clang-c/Index.h
@@ -32,7 +32,7 @@
* compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
*/
#define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 39
+#define CINDEX_VERSION_MINOR 40
#define CINDEX_VERSION_ENCODE(major, minor) ( \
((major) * 1) \
@@ -1419,6 +1419,15 @@
unsigned options);
/**
+ * \brief Suspend a translation unit in order to free memory associated with it.
+ *
+ * A suspended translation unit uses significantly less memory but on the other
+ * side does not support any other calls than \c clang_reparseTranslationUnit
+ * to resume it or \c clang_disposeTranslationUnit to dispose it completely.
+ */
+CINDEX_LINKAGE unsigned clang_suspendTranslationUnit(CXTranslationUnit);
+
+/**
* \brief Destroy the specified CXTranslationUnit object.
*/
CINDEX_LINKAGE void clang_disposeTranslationUnit(CXTranslationUnit);
___
cfe-commits mailing list
cf
[PATCH] D33393: [PATCH] Libcxxabi Demangler PR32890
marcel created this revision. Check whether the string (db.names.back().first) is longer than 7 characters before inserting another string at index 8. https://reviews.llvm.org/D33393 Files: CREDITS.TXT src/cxa_demangle.cpp test/test_demangle.pass.cpp Index: test/test_demangle.pass.cpp === --- test/test_demangle.pass.cpp +++ test/test_demangle.pass.cpp @@ -29665,6 +29665,7 @@ "\x44\x74\x70\x74\x71\x75\x32\x43\x41\x38\x65\x6E\x9B\x72\x4D\xC1\x43\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\x6C\x53\xF9\x5F\x70\x74\x70\x69\x45\x38\xD3\x73\x9E\x2A\x37", "\x46\x44\x74\x70\x74\x71\x75\x32\x43\x41\x72\x4D\x6E\x65\x34\x9F\xC1\x43\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\x6C\x53\xF9\x5F\x70\x74\x70\x69\x45\x34\xD3\x73\x9E\x2A\x37\x72\x33\x8E\x3A\x29\x8E\x44\x35", "_ZcvCiIJEEDvT__T_vT_v", +"Z1JIJ1_T_EE3o00EUlT_E0", }; const unsigned NI = sizeof(invalid_cases) / sizeof(invalid_cases[0]); Index: src/cxa_demangle.cpp === --- src/cxa_demangle.cpp +++ src/cxa_demangle.cpp @@ -3075,7 +3075,8 @@ const char* t1 = t0 + 1; while (t1 != last && std::isdigit(*t1)) ++t1; -db.names.back().first.insert(db.names.back().first.begin()+7, t0, t1); +if (db.names.back().first.length() > 7) + db.names.back().first.insert(db.names.back().first.begin() + 7, t0, t1); t0 = t1; } if (t0 == last || *t0 != '_') Index: CREDITS.TXT === --- CREDITS.TXT +++ CREDITS.TXT @@ -12,6 +12,10 @@ E: aa...@aaronballman.com D: Minor patches +N: Marcel Boehme +E: marcel.boe...@acm.org +D: Minor patches + N: Logan Chien E: logan.ch...@mediatek.com D: ARM EHABI Unwind & Exception Handling Index: test/test_demangle.pass.cpp === --- test/test_demangle.pass.cpp +++ test/test_demangle.pass.cpp @@ -29665,6 +29665,7 @@ "\x44\x74\x70\x74\x71\x75\x32\x43\x41\x38\x65\x6E\x9B\x72\x4D\xC1\x43\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\x6C\x53\xF9\x5F\x70\x74\x70\x69\x45\x38\xD3\x73\x9E\x2A\x37", "\x46\x44\x74\x70\x74\x71\x75\x32\x43\x41\x72\x4D\x6E\x65\x34\x9F\xC1\x43\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\x6C\x53\xF9\x5F\x70\x74\x70\x69\x45\x34\xD3\x73\x9E\x2A\x37\x72\x33\x8E\x3A\x29\x8E\x44\x35", "_ZcvCiIJEEDvT__T_vT_v", +"Z1JIJ1_T_EE3o00EUlT_E0", }; const unsigned NI = sizeof(invalid_cases) / sizeof(invalid_cases[0]); Index: src/cxa_demangle.cpp === --- src/cxa_demangle.cpp +++ src/cxa_demangle.cpp @@ -3075,7 +3075,8 @@ const char* t1 = t0 + 1; while (t1 != last && std::isdigit(*t1)) ++t1; -db.names.back().first.insert(db.names.back().first.begin()+7, t0, t1); +if (db.names.back().first.length() > 7) + db.names.back().first.insert(db.names.back().first.begin() + 7, t0, t1); t0 = t1; } if (t0 == last || *t0 != '_') Index: CREDITS.TXT === --- CREDITS.TXT +++ CREDITS.TXT @@ -12,6 +12,10 @@ E: aa...@aaronballman.com D: Minor patches +N: Marcel Boehme +E: marcel.boe...@acm.org +D: Minor patches + N: Logan Chien E: logan.ch...@mediatek.com D: ARM EHABI Unwind & Exception Handling ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303534 - clang-format: Allow customizing the penalty for breaking assignment
Author: typz
Date: Mon May 22 03:28:17 2017
New Revision: 303534
URL: http://llvm.org/viewvc/llvm-project?rev=303534&view=rev
Log:
clang-format: Allow customizing the penalty for breaking assignment
Summary:
Add option to customize the penalty for breaking assignment
This allows increasing the priority of the assignment, to prefer spliting
an operation instead of splitting the assignment, e.g. :
int a = +
;
Reviewers: krasimir, djasper
Reviewed By: djasper
Subscribers: cfe-commits, klimek
Differential Revision: https://reviews.llvm.org/D32477
Modified:
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Modified: cfe/trunk/include/clang/Format/Format.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=303534&r1=303533&r2=303534&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Mon May 22 03:28:17 2017
@@ -1133,6 +1133,9 @@ struct FormatStyle {
/// ``Foo `` instead of ``Foo``.
bool ObjCSpaceBeforeProtocolList;
+ /// \brief The penalty for breaking around an assignment operator.
+ unsigned PenaltyBreakAssignment;
+
/// \brief The penalty for breaking a function call after ``call(``.
unsigned PenaltyBreakBeforeFirstCallParameter;
@@ -1420,6 +1423,8 @@ struct FormatStyle {
ObjCBlockIndentWidth == R.ObjCBlockIndentWidth &&
ObjCSpaceAfterProperty == R.ObjCSpaceAfterProperty &&
ObjCSpaceBeforeProtocolList == R.ObjCSpaceBeforeProtocolList &&
+ PenaltyBreakAssignment ==
+ R.PenaltyBreakAssignment &&
PenaltyBreakBeforeFirstCallParameter ==
R.PenaltyBreakBeforeFirstCallParameter &&
PenaltyBreakComment == R.PenaltyBreakComment &&
Modified: cfe/trunk/lib/Format/Format.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=303534&r1=303533&r2=303534&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Mon May 22 03:28:17 2017
@@ -343,6 +343,8 @@ template <> struct MappingTraitshttp://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=303534&r1=303533&r2=303534&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon May 22 03:28:17 2017
@@ -2093,9 +2093,10 @@ unsigned TokenAnnotator::splitPenalty(co
if (Left.is(TT_ConditionalExpr))
return prec::Conditional;
prec::Level Level = Left.getPrecedence();
- if (Level != prec::Unknown)
-return Level;
- Level = Right.getPrecedence();
+ if (Level == prec::Unknown)
+Level = Right.getPrecedence();
+ if (Level == prec::Assignment)
+return Style.PenaltyBreakAssignment;
if (Level != prec::Unknown)
return Level;
Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=303534&r1=303533&r2=303534&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Mon May 22 03:28:17 2017
@@ -3457,6 +3457,18 @@ TEST_F(FormatTest, BreaksAfterAssignment
"1;");
}
+TEST_F(FormatTest, ConfigurableBreakAssignmentPenalty) {
+ FormatStyle Style = getLLVMStyle();
+ verifyFormat("int aa =\n"
+ "bb + cc;",
+ Style);
+
+ Style.PenaltyBreakAssignment = 20;
+ verifyFormat("int aa = bb
+\n"
+ " cc;",
+ Style);
+}
+
TEST_F(FormatTest, AlignsAfterAssignments) {
verifyFormat(
"int Result = a + a +\n"
@@ -8802,6 +8814,8 @@ TEST_F(FormatTest, ParsesConfiguration)
CHECK_PARSE("ObjCBlockIndentWidth: 1234", ObjCBlockIndentWidth, 1234u);
CHECK_PARSE("ColumnLimit: 1234", ColumnLimit, 1234u);
CHECK_PARSE("MaxEmptyLinesToKeep: 1234", MaxEmptyLinesToKeep, 1234u);
+ CHECK_PARSE("PenaltyBreakAssignment: 1234",
+ PenaltyBreakAssignment, 1234u);
CHECK_PARSE("PenaltyBreakBeforeFirstCallParameter: 1234",
PenaltyBreakBeforeFirstCallParameter, 1234u);
CHECK_PARSE("PenaltyExcessCharacter: 1234", PenaltyExcessCharacter, 1234u);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listin
[PATCH] D32477: clang-format: Allow customizing the penalty for breaking assignment
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303534: clang-format: Allow customizing the penalty for
breaking assignment (authored by Typz).
Changed prior to commit:
https://reviews.llvm.org/D32477?vs=99415&id=99720#toc
Repository:
rL LLVM
https://reviews.llvm.org/D32477
Files:
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp
Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -1133,6 +1133,9 @@
/// ``Foo `` instead of ``Foo``.
bool ObjCSpaceBeforeProtocolList;
+ /// \brief The penalty for breaking around an assignment operator.
+ unsigned PenaltyBreakAssignment;
+
/// \brief The penalty for breaking a function call after ``call(``.
unsigned PenaltyBreakBeforeFirstCallParameter;
@@ -1420,6 +1423,8 @@
ObjCBlockIndentWidth == R.ObjCBlockIndentWidth &&
ObjCSpaceAfterProperty == R.ObjCSpaceAfterProperty &&
ObjCSpaceBeforeProtocolList == R.ObjCSpaceBeforeProtocolList &&
+ PenaltyBreakAssignment ==
+ R.PenaltyBreakAssignment &&
PenaltyBreakBeforeFirstCallParameter ==
R.PenaltyBreakBeforeFirstCallParameter &&
PenaltyBreakComment == R.PenaltyBreakComment &&
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2093,9 +2093,10 @@
if (Left.is(TT_ConditionalExpr))
return prec::Conditional;
prec::Level Level = Left.getPrecedence();
- if (Level != prec::Unknown)
-return Level;
- Level = Right.getPrecedence();
+ if (Level == prec::Unknown)
+Level = Right.getPrecedence();
+ if (Level == prec::Assignment)
+return Style.PenaltyBreakAssignment;
if (Level != prec::Unknown)
return Level;
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -343,6 +343,8 @@
IO.mapOptional("ObjCSpaceAfterProperty", Style.ObjCSpaceAfterProperty);
IO.mapOptional("ObjCSpaceBeforeProtocolList",
Style.ObjCSpaceBeforeProtocolList);
+IO.mapOptional("PenaltyBreakAssignment",
+ Style.PenaltyBreakAssignment);
IO.mapOptional("PenaltyBreakBeforeFirstCallParameter",
Style.PenaltyBreakBeforeFirstCallParameter);
IO.mapOptional("PenaltyBreakComment", Style.PenaltyBreakComment);
@@ -582,6 +584,7 @@
LLVMStyle.SpaceBeforeAssignmentOperators = true;
LLVMStyle.SpacesInAngles = false;
+ LLVMStyle.PenaltyBreakAssignment = prec::Assignment;
LLVMStyle.PenaltyBreakComment = 300;
LLVMStyle.PenaltyBreakFirstLessLess = 120;
LLVMStyle.PenaltyBreakString = 1000;
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -3457,6 +3457,18 @@
"1;");
}
+TEST_F(FormatTest, ConfigurableBreakAssignmentPenalty) {
+ FormatStyle Style = getLLVMStyle();
+ verifyFormat("int aa =\n"
+ "bb + cc;",
+ Style);
+
+ Style.PenaltyBreakAssignment = 20;
+ verifyFormat("int aa = bb
+\n"
+ " cc;",
+ Style);
+}
+
TEST_F(FormatTest, AlignsAfterAssignments) {
verifyFormat(
"int Result = a + a +\n"
@@ -8802,6 +8814,8 @@
CHECK_PARSE("ObjCBlockIndentWidth: 1234", ObjCBlockIndentWidth, 1234u);
CHECK_PARSE("ColumnLimit: 1234", ColumnLimit, 1234u);
CHECK_PARSE("MaxEmptyLinesToKeep: 1234", MaxEmptyLinesToKeep, 1234u);
+ CHECK_PARSE("PenaltyBreakAssignment: 1234",
+ PenaltyBreakAssignment, 1234u);
CHECK_PARSE("PenaltyBreakBeforeFirstCallParameter: 1234",
PenaltyBreakBeforeFirstCallParameter, 1234u);
CHECK_PARSE("PenaltyExcessCharacter: 1234", PenaltyExcessCharacter, 1234u);
Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -1133,6 +1133,9 @@
/// ``Foo `` instead of ``Foo``.
bool ObjCSpaceBeforeProtocolList;
+ /// \brief The penalty for breaking around an assignment operator.
+ unsigned PenaltyBreakAssignment;
+
/// \brief The penalty for breaking a function call after ``call(``.
uns
[PATCH] D32479: clang-format: Introduce BreakConstructorInitializers option
djasper added inline comments. Comment at: include/clang/Format/Format.h:699 + /// This option is **deprecated* and is retained for backwards compatibility. bool BreakConstructorInitializersBeforeComma; I don't think you need to keep this around. The YAML parsing logic can correctly set the new field instead. So basically just call mapOptional for both names of configuration fields but always set BreakConstructorInitializers. And then map true/false to the appropriate enum values. Comment at: include/clang/Format/Format.h:710 +/// \endcode +BCIS_BeforeColonAfterComma, +/// Break constructor initializers before the colon and commas, and align Call this just "BeforeColon". Comment at: include/clang/Format/Format.h:718 +/// \endcode +BCIS_BeforeColonAndComma, +/// Break constructor initializers after the colon and commas. Call this just "BeforeComma". Comment at: include/clang/Format/Format.h:725 +/// \endcode +BCIS_AfterColonAndComma + }; Call this just "AfterColon". Comment at: lib/Format/ContinuationIndenter.cpp:61 ((Previous.isNot(TT_CtorInitializerComma) || - !Style.BreakConstructorInitializersBeforeComma) && + (Style.BreakConstructorInitializers != +FormatStyle::BCIS_BeforeColonAndComma)) && You don't need parentheses to surround comparisons. Remove them here and elsewhere. Comment at: lib/Format/ContinuationIndenter.cpp:196 + FormatStyle::BCIS_AfterColonAndComma) && + (State.Column + State.Line->Last->TotalLength - Previous.TotalLength > + getColumnLimit(State) || Why can you drop the "+2" here? Also, I'd like to structure this so we have to duplicate less of the logic. But I am not really sure it's possible. https://reviews.llvm.org/D32479 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33394: [clang-format] Keep trailing preprocessor line comments separate from the following section comments
krasimir created this revision.
Herald added a subscriber: klimek.
r303415 changed the way a sequence of line comments following a preprocessor
macro is handled, which has the unfortunate effect of aligning a trailing
preprocessor line comment and following unrelated section comments, so:
#ifdef A // comment about A
// section comment
#endif
gets turned into:
#ifdef A // comment about A
// section comment
#endif
This patch fixes this by additionally checking the original start columns of
the line comments.
https://reviews.llvm.org/D33394
Files:
lib/Format/UnwrappedLineParser.cpp
unittests/Format/FormatTestComments.cpp
Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -1020,6 +1020,24 @@
getLLVMStyleWithColumns(20)));
}
+TEST_F(FormatTestComments, KeepsTrailingPPCommentsAndSectionCommentsSeparate) {
+ verifyFormat("#ifdef A // line about A\n"
+ "// section comment\n"
+ "#endif",
+ getLLVMStyleWithColumns(80));
+ verifyFormat("int f() {\n"
+ " int i;\n"
+ "#ifdef A // comment about A\n"
+ " // section comment 1\n"
+ " // section comment 2\n"
+ " i = 2;\n"
+ "#else // comment about #else\n"
+ " // section comment 3\n"
+ " i = 4;\n"
+ "#endif\n"
+ "}", getLLVMStyleWithColumns(80));
+}
+
TEST_F(FormatTestComments, CommentsInStaticInitializers) {
EXPECT_EQ(
"static SomeType type = {, /* comment */\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -102,7 +102,8 @@
bool eof() {
return Token && Token->HasUnescapedNewline &&
!(PreviousToken && isLineComment(*PreviousToken) &&
- isLineComment(*Token) && Token->NewlinesBefore == 1);
+ isLineComment(*Token) && Token->NewlinesBefore == 1 &&
+ Token->OriginalColumn == PreviousToken->OriginalColumn);
}
FormatToken *getFakeEOF() {
Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -1020,6 +1020,24 @@
getLLVMStyleWithColumns(20)));
}
+TEST_F(FormatTestComments, KeepsTrailingPPCommentsAndSectionCommentsSeparate) {
+ verifyFormat("#ifdef A // line about A\n"
+ "// section comment\n"
+ "#endif",
+ getLLVMStyleWithColumns(80));
+ verifyFormat("int f() {\n"
+ " int i;\n"
+ "#ifdef A // comment about A\n"
+ " // section comment 1\n"
+ " // section comment 2\n"
+ " i = 2;\n"
+ "#else // comment about #else\n"
+ " // section comment 3\n"
+ " i = 4;\n"
+ "#endif\n"
+ "}", getLLVMStyleWithColumns(80));
+}
+
TEST_F(FormatTestComments, CommentsInStaticInitializers) {
EXPECT_EQ(
"static SomeType type = {, /* comment */\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -102,7 +102,8 @@
bool eof() {
return Token && Token->HasUnescapedNewline &&
!(PreviousToken && isLineComment(*PreviousToken) &&
- isLineComment(*Token) && Token->NewlinesBefore == 1);
+ isLineComment(*Token) && Token->NewlinesBefore == 1 &&
+ Token->OriginalColumn == PreviousToken->OriginalColumn);
}
FormatToken *getFakeEOF() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33395: [clangd] Split clangd into library+executable (mainly for unit tests).
ilya-biryukov created this revision. Herald added a subscriber: mgorny. This commit itself doesn't add any unit tests, but one that does will follow shortly. https://reviews.llvm.org/D33395 Files: clangd/CMakeLists.txt clangd/ClangdMain.cpp clangd/tool/CMakeLists.txt clangd/tool/ClangdMain.cpp Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -7,8 +7,8 @@ // //===--===// -#include "ClangdLSPServer.h" -#include "JSONRPCDispatcher.h" +#include "../ClangdLSPServer.h" +#include "../JSONRPCDispatcher.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Program.h" Index: clangd/tool/CMakeLists.txt === --- clangd/tool/CMakeLists.txt +++ clangd/tool/CMakeLists.txt @@ -1,20 +1,12 @@ add_clang_executable(clangd - ClangdLSPServer.cpp ClangdMain.cpp - ClangdServer.cpp - ClangdUnit.cpp - ClangdUnitStore.cpp - DraftStore.cpp - GlobalCompilationDatabase.cpp - JSONRPCDispatcher.cpp - Protocol.cpp - ProtocolHandlers.cpp ) install(TARGETS clangd RUNTIME DESTINATION bin) target_link_libraries(clangd clangBasic + clangDaemon clangFormat clangFrontend clangSema Index: clangd/CMakeLists.txt === --- clangd/CMakeLists.txt +++ clangd/CMakeLists.txt @@ -1,6 +1,5 @@ -add_clang_executable(clangd +add_clang_library(clangDaemon ClangdLSPServer.cpp - ClangdMain.cpp ClangdServer.cpp ClangdUnit.cpp ClangdUnitStore.cpp @@ -11,14 +10,14 @@ ProtocolHandlers.cpp ) -install(TARGETS clangd RUNTIME DESTINATION bin) - -target_link_libraries(clangd +target_link_libraries(clangDaemon clangBasic clangFormat clangFrontend clangSema clangTooling clangToolingCore LLVMSupport ) + +add_subdirectory(tool) Index: clangd/tool/ClangdMain.cpp === --- clangd/tool/ClangdMain.cpp +++ clangd/tool/ClangdMain.cpp @@ -7,8 +7,8 @@ // //===--===// -#include "ClangdLSPServer.h" -#include "JSONRPCDispatcher.h" +#include "../ClangdLSPServer.h" +#include "../JSONRPCDispatcher.h" #include "llvm/Support/CommandLine.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Program.h" Index: clangd/tool/CMakeLists.txt === --- clangd/tool/CMakeLists.txt +++ clangd/tool/CMakeLists.txt @@ -1,20 +1,12 @@ add_clang_executable(clangd - ClangdLSPServer.cpp ClangdMain.cpp - ClangdServer.cpp - ClangdUnit.cpp - ClangdUnitStore.cpp - DraftStore.cpp - GlobalCompilationDatabase.cpp - JSONRPCDispatcher.cpp - Protocol.cpp - ProtocolHandlers.cpp ) install(TARGETS clangd RUNTIME DESTINATION bin) target_link_libraries(clangd clangBasic + clangDaemon clangFormat clangFrontend clangSema Index: clangd/CMakeLists.txt === --- clangd/CMakeLists.txt +++ clangd/CMakeLists.txt @@ -1,6 +1,5 @@ -add_clang_executable(clangd +add_clang_library(clangDaemon ClangdLSPServer.cpp - ClangdMain.cpp ClangdServer.cpp ClangdUnit.cpp ClangdUnitStore.cpp @@ -11,14 +10,14 @@ ProtocolHandlers.cpp ) -install(TARGETS clangd RUNTIME DESTINATION bin) - -target_link_libraries(clangd +target_link_libraries(clangDaemon clangBasic clangFormat clangFrontend clangSema clangTooling clangToolingCore LLVMSupport ) + +add_subdirectory(tool) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33094: [ASTMatchers] Add clang-query support for equals matcher
Lekensteyn added inline comments.
Comment at: include/clang/ASTMatchers/ASTMatchers.h:3846
+ CXXBoolLiteralExpr,
+ IntegerLiteral),
+ unsigned, Value, 1) {
aaron.ballman wrote:
> Is there a reason to not allow the equals matcher to do something like
> `floatingLiteral(equals(1))`? Sure, the user could always write `1.0`, but it
> seems somewhat hostile to require it.
The ValueMatcher for float does not accept integers at the moment, adding
FloatingLiteral now results in a compile error. It can be added though (might
do this as well in the next revision, either in the existing patches or a new
one).
Comment at: unittests/ASTMatchers/Dynamic/RegistryTest.cpp:534
+ EXPECT_TRUE(matches("double x = 12e-1;", DoubleStmt));
+ EXPECT_FALSE(matches("double x = 1.23;", DoubleStmt));
+
aaron.ballman wrote:
> Can you add tests for floating literals with suffixes (f, l)?
will do
Comment at: unittests/ASTMatchers/Dynamic/RegistryTest.cpp:545
+ .getTypedMatcher();
+ EXPECT_TRUE(matches("int x = 'x';", CharStmt));
+ EXPECT_FALSE(matches("int x = 120;", CharStmt));
aaron.ballman wrote:
> Can you add some tests involving the other character literal types (L, u, U,
> u8)?
will do
https://reviews.llvm.org/D33094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32480: clang-format: Add CompactNamespaces option
djasper added inline comments.
Comment at: include/clang/Format/Format.h:358
+ /// \endcode
+ bool BinPackNamespaces;
+
Typz wrote:
> djasper wrote:
> > Typz wrote:
> > > djasper wrote:
> > > > This is not bin packing at all. Maybe CompactNamespaces? Or
> > > > SingleLineNamespaces?
> > > >
> > > > Also, could you clarify what happens if the namespaces don't fit within
> > > > the column limit (both in the comment describing the flag and by adding
> > > > tests for it)?
> > > This is not binpacking, but has a similar effect and made the option name
> > > somewhat consistent with the other binpack options :-)
> > > I'll change to CompactNamespaces then.
> > How does this option interact with NamespaceIndentation. Do all the values
> > you can set there make sense and work as expected?
> >
> > I am wondering whether we should generally rename this to NamespaceStyle
> > and make it an enum. That way, we can start to also support C++17
> > namespace, but people that don't want to use C++17 yet, can still use this
> > style of compact namespace.
> > How does this option interact with NamespaceIndentation. Do all the values
> > you can set there make sense and work as expected?
>
> NamespaceIndentation is not affected, indent is done as before (e.g. just
> "counting" the imbricated namespaces).
>
> In 'NI_All' we may want to reduce the indent when multiple namespaces are
> declared in the same line, but this would become inconsistent if the
> beginning and end of all namespaces do not match:
>
> namepace A { namespace B {
> int i;
> } // namespace B
> int i;
> } // namespace A
>
> So I think reducing indent in that case should be (if ever needed) another
> value in NamespaceIndentation...
>
> > I am wondering whether we should generally rename this to NamespaceStyle
> > and make it an enum. That way, we can start to also support C++17
> > namespace, but people that don't want to use C++17 yet, can still use this
> > style of compact namespace.
>
> As for C++17, I am not sure we need another option: just having
> CompactNamespaces=true and Standard=C++17 would use the "real" C++17 mode.
> That said converting to C++17 namespace blocks is slightly more restrictive,
> as it will require that both the beginning and end of the inner & outer
> blocks to match...
>
> I will keep the boolean flag for now, just let me know if you prefer to have
> the enum in case other modes are needed and I will update the patch.
Yeah, this is probably something nobody will ever want:
namepace A { namespace B {
int i;
} // namespace B
int i;
} // namespace A
And you have the same problem for NI_Inner as soon as you have more than two
levels of namespace.
I see two ways forward:
1. Make "compacted" namespaces always add at most one level of indentation.
2. Assume that this can only ever usefully work with the behavior of NI_None
and add an additional enum value NI_Compact.
The problem I have with #1 is that it increases the complexity quite a bit and
the behavior is even difficult to predict to users. Remove a comment somewhere
might enable clang-format to make two namespaces "compact" and then remove
indentation throughout the file.
So I would lean more towards solution #2. The name NamespaceIndentation might
then be a bit confusing, but not sure whether it's worth changing it. And of
course I don't know whether some people would want indentation in compacted
namespaces.
What do you think?
https://reviews.llvm.org/D32480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33135: [ASTMatchers] Add support for floatLiterals
Lekensteyn added inline comments.
Comment at: include/clang/ASTMatchers/Dynamic/VariantValue.h:335
unsigned Unsigned;
+double Double;
bool Boolean;
aaron.ballman wrote:
> Lekensteyn wrote:
> > aaron.ballman wrote:
> > > This may or may not be a good idea, but do we want to put the values into
> > > an APFloat rather than a double? My concern with double is that (0) it
> > > may be subtly different if the user wants a 16- or 32-bit float
> > > explicitly, (1) it won't be able to represent long double values, or quad
> > > double.
> > >
> > > I'm thinking this value could be passed directly from the C++ API as an
> > > APFloat, float, or double, or provided using a StringRef for the dynamic
> > > API.
> > (32-bit) double values are a superset of (16-bit) float values, that should
> > be OK.
> > Long doubles are possible in the AST (e.g. for `0.1L`), but neither C11 nor
> > C++14 seem to define a quad double literal type (so that should be of a
> > lesser concern).
> >
> > Reasons why I chose for double instead of APFloat:
> > - `strtod` is readily available and does not abort the program. By
> > contrast, `APFloat(StringRef)` trips on assertions if the input is invalid.
> > - I was not sure if the APFloat class can be used in an union.
> The downside to using `strtod()` is that invalid input is silently accepted.
> However, assertions on invalid input is certainly not good either. It might
> be worth modifying `APFloat::convertFromString()` to accept invalid input and
> return an error.
>
> I think instead of an `APFloat`, maybe using an `APValue` for both the
> `Unsigned` and `Double` fields might work. At the very least, it should give
> you implementation ideas.
>
> There is a quad double literal suffix: `q`. It's only supported on some
> architectures, however. There are also imaginary numbers (`i`) and half (`h`).
The strtod conversion was based on parseDouble in lib/Support/CommandLine.cpp,
so any conversion issues also exist there.
Same question, can APFloat/APValue be used in a union?
float (or quad-double suffixes) are explicitly not supported now in this
matcher, maybe they can be added later but for now I decided to keep the
grammar simple (that is, do not express double/float data types via the
literal).
Comment at: lib/ASTMatchers/Dynamic/Parser.cpp:209
+ double doubleValue = strtod(Result->Text.str().c_str(), &end);
+ if (*end == 0) {
+Result->Kind = TokenInfo::TK_Literal;
aaron.ballman wrote:
> You're failing to check errno here to ensure the value is actually valid.
will check this later, see also the previous comment
https://reviews.llvm.org/D33135
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33394: [clang-format] Keep trailing preprocessor line comments separate from the following section comments
djasper added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:106 + isLineComment(*Token) && Token->NewlinesBefore == 1 && + Token->OriginalColumn == PreviousToken->OriginalColumn); } Any chance of moving this logic to distributeComments? I think that's the place we previously tried to encapsulate this logic in. If not, this is fine for now. Comment at: unittests/Format/FormatTestComments.cpp:1030 + " int i;\n" + "#ifdef A // comment about A\n" + " // section comment 1\n" Would it make sense to also add a test case in which the #ifdef comment is actually continued? https://reviews.llvm.org/D33394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33397: Allow to use vfs::FileSystem for file accesses inside ASTUnit.
ilya-biryukov created this revision.
https://reviews.llvm.org/D33397
Files:
include/clang/Frontend/ASTUnit.h
include/clang/Frontend/CompilerInvocation.h
lib/Frontend/ASTUnit.cpp
lib/Frontend/CompilerInvocation.cpp
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2747,15 +2747,22 @@
IntrusiveRefCntPtr
createVFSFromCompilerInvocation(const CompilerInvocation &CI,
DiagnosticsEngine &Diags) {
+ return createVFSFromCompilerInvocation(CI, Diags, vfs::getRealFileSystem());
+}
+
+IntrusiveRefCntPtr
+createVFSFromCompilerInvocation(const CompilerInvocation &CI,
+DiagnosticsEngine &Diags,
+IntrusiveRefCntPtr BaseFS) {
if (CI.getHeaderSearchOpts().VFSOverlayFiles.empty())
-return vfs::getRealFileSystem();
+return BaseFS;
- IntrusiveRefCntPtr
-Overlay(new vfs::OverlayFileSystem(vfs::getRealFileSystem()));
+ IntrusiveRefCntPtr Overlay(
+ new vfs::OverlayFileSystem(BaseFS));
// earlier vfs files are on the bottom
for (const std::string &File : CI.getHeaderSearchOpts().VFSOverlayFiles) {
llvm::ErrorOr> Buffer =
-llvm::MemoryBuffer::getFile(File);
+BaseFS->getBufferForFile(File);
if (!Buffer) {
Diags.Report(diag::err_missing_vfs_overlay_file) << File;
return IntrusiveRefCntPtr();
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -97,6 +97,21 @@
/// \brief Erase temporary files and the preamble file.
void Cleanup();
};
+
+ template
+ std::unique_ptr valueOrNull(llvm::ErrorOr> Val) {
+if (!Val)
+ return nullptr;
+return std::move(*Val);
+ }
+
+ template
+ bool moveOnNoError(llvm::ErrorOr Val, T &Output) {
+if (!Val)
+ return false;
+Output = std::move(*Val);
+return true;
+ }
}
static llvm::sys::SmartMutex &getOnDiskMutex() {
@@ -1041,15 +1056,22 @@
/// \returns True if a failure occurred that causes the ASTUnit not to
/// contain any translation-unit information, false otherwise.
bool ASTUnit::Parse(std::shared_ptr PCHContainerOps,
-std::unique_ptr OverrideMainBuffer) {
+std::unique_ptr OverrideMainBuffer,
+IntrusiveRefCntPtr VFS) {
SavedMainFileBuffer.reset();
if (!Invocation)
return true;
// Create the compiler instance to use for building the AST.
std::unique_ptr Clang(
new CompilerInstance(std::move(PCHContainerOps)));
+ if (FileMgr && VFS) {
+assert(VFS == FileMgr->getVirtualFileSystem() &&
+ "VFS passed to Parse and VFS in FileMgr are different");
+ } else if (VFS) {
+Clang->setVirtualFileSystem(VFS);
+ }
// Recover resources if we crash before exiting this method.
llvm::CrashRecoveryContextCleanupRegistrar
@@ -1193,7 +1215,8 @@
/// that corresponds to the main file along with a pair (bytes, start-of-line)
/// that describes the preamble.
ASTUnit::ComputedPreamble
-ASTUnit::ComputePreamble(CompilerInvocation &Invocation, unsigned MaxLines) {
+ASTUnit::ComputePreamble(CompilerInvocation &Invocation, unsigned MaxLines,
+ IntrusiveRefCntPtr VFS) {
FrontendOptions &FrontendOpts = Invocation.getFrontendOpts();
PreprocessorOptions &PreprocessorOpts = Invocation.getPreprocessorOpts();
@@ -1203,28 +1226,32 @@
llvm::MemoryBuffer *Buffer = nullptr;
std::unique_ptr BufferOwner;
std::string MainFilePath(FrontendOpts.Inputs[0].getFile());
- llvm::sys::fs::UniqueID MainFileID;
- if (!llvm::sys::fs::getUniqueID(MainFilePath, MainFileID)) {
+ auto MainFileStatus = VFS->status(MainFilePath);
+ if (MainFileStatus) {
+llvm::sys::fs::UniqueID MainFileID = MainFileStatus->getUniqueID();
+
// Check whether there is a file-file remapping of the main file
for (const auto &RF : PreprocessorOpts.RemappedFiles) {
std::string MPath(RF.first);
- llvm::sys::fs::UniqueID MID;
- if (!llvm::sys::fs::getUniqueID(MPath, MID)) {
+ auto MPathStatus = VFS->status(MPath);
+ if (MPathStatus) {
+llvm::sys::fs::UniqueID MID = MPathStatus->getUniqueID();
if (MainFileID == MID) {
// We found a remapping. Try to load the resulting, remapped source.
- BufferOwner = getBufferForFile(RF.second);
+ BufferOwner = valueOrNull(VFS->getBufferForFile(RF.second));
if (!BufferOwner)
return ComputedPreamble(nullptr, nullptr, 0, true);
}
}
}
-
+
// Check whether there is a file-buffer remapping. It supercedes the
// file-file remapping.
for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
std::string MPath(RB.first);
- llvm::sys::fs::UniqueID MID;
-
[PATCH] D32178: Delete unstable integration tests
arphaman added a comment. Ok, I will commit them sometime this week then. https://reviews.llvm.org/D32178 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33398: Remove __unaligned preventively when mangling types in Itanium ABI
rogfer01 created this revision.
`__unaligned` is not currently mangled in any way and as such is not a relevant
qualifier for substitutions. This may cause that, when it is the only
qualifier, the substitution is added twice which triggers an assertion.
The simplest thing to do is to remove the qualifier preemptively.
https://reviews.llvm.org/D33398
Files:
lib/AST/ItaniumMangle.cpp
test/CodeGenCXX/pr33080.cpp
Index: test/CodeGenCXX/pr33080.cpp
===
--- /dev/null
+++ test/CodeGenCXX/pr33080.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm -o-
%s | FileCheck %s
+
+void f(__unaligned struct A*) {}
+
+// CHECK: define void @_Z1fP1A(%struct.A*)
Index: lib/AST/ItaniumMangle.cpp
===
--- lib/AST/ItaniumMangle.cpp
+++ lib/AST/ItaniumMangle.cpp
@@ -2326,6 +2326,12 @@
// substitution at the original type.
}
+ // __unaligned is not currently mangled in any way. This implies that it is
+ // not a relevant qualifier for substitutions (while CVR and maybe others
+ // are). This triggers an assertion when this is the only qualifier and the
+ // unqualified type is a class. So let's remove it preventively here.
+ quals.removeUnaligned();
+
if (quals) {
mangleQualifiers(quals);
// Recurse: even if the qualified type isn't yet substitutable,
Index: test/CodeGenCXX/pr33080.cpp
===
--- /dev/null
+++ test/CodeGenCXX/pr33080.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm -o- %s | FileCheck %s
+
+void f(__unaligned struct A*) {}
+
+// CHECK: define void @_Z1fP1A(%struct.A*)
Index: lib/AST/ItaniumMangle.cpp
===
--- lib/AST/ItaniumMangle.cpp
+++ lib/AST/ItaniumMangle.cpp
@@ -2326,6 +2326,12 @@
// substitution at the original type.
}
+ // __unaligned is not currently mangled in any way. This implies that it is
+ // not a relevant qualifier for substitutions (while CVR and maybe others
+ // are). This triggers an assertion when this is the only qualifier and the
+ // unqualified type is a class. So let's remove it preventively here.
+ quals.removeUnaligned();
+
if (quals) {
mangleQualifiers(quals);
// Recurse: even if the qualified type isn't yet substitutable,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32525: [clang-format] Add SpaceBeforeColon option
djasper added a comment. Hm, can't really remember what I meant by "my comment". Probably not important. So, I still see two problems: - I would not count the link you mentioned as a publicly accessible style guide. - I don't think the naming and granularity of this option is right. You specifically want to control two things. The space in front of constructor initializers colons and of range-based for loop colons. Then you also (accidentally?) change the spacing in dict literals. However, the latter is already controlled by SpacesInContainerLiterals. So there even seems to be a conflict here. You do not want to change anything for ternary expressions or labels. Oh, and there are also colons in inline assembly. Colons are probably the most overloaded tokens. Now, we could of course allow much more fine-grained control over colons, but really, that adds quite a bit of complexity to the clang-format configuration. And I am not sure whether the added costs (both in maintenance of clang-format and discoverability for users) are covered by the very small added value of that removed space for you. I am sorry I don't have to better answer, but I have to weigh off the benefit this adds for you vs. the reduced usefulness to all other users of clang-format. In general, we'd rather have clang-format have fewer configuration options and support them really well. For now, I am against moving forward with this patch, unless we find a good way to structure the options and a publicly accessible coding style that really supports this (explicitly, not just in an example somewhere that's actually meant to show something else). https://reviews.llvm.org/D32525 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32480: clang-format: Add CompactNamespaces option
Typz added inline comments.
Comment at: include/clang/Format/Format.h:358
+ /// \endcode
+ bool BinPackNamespaces;
+
djasper wrote:
> Typz wrote:
> > djasper wrote:
> > > Typz wrote:
> > > > djasper wrote:
> > > > > This is not bin packing at all. Maybe CompactNamespaces? Or
> > > > > SingleLineNamespaces?
> > > > >
> > > > > Also, could you clarify what happens if the namespaces don't fit
> > > > > within the column limit (both in the comment describing the flag and
> > > > > by adding tests for it)?
> > > > This is not binpacking, but has a similar effect and made the option
> > > > name somewhat consistent with the other binpack options :-)
> > > > I'll change to CompactNamespaces then.
> > > How does this option interact with NamespaceIndentation. Do all the
> > > values you can set there make sense and work as expected?
> > >
> > > I am wondering whether we should generally rename this to NamespaceStyle
> > > and make it an enum. That way, we can start to also support C++17
> > > namespace, but people that don't want to use C++17 yet, can still use
> > > this style of compact namespace.
> > > How does this option interact with NamespaceIndentation. Do all the
> > > values you can set there make sense and work as expected?
> >
> > NamespaceIndentation is not affected, indent is done as before (e.g. just
> > "counting" the imbricated namespaces).
> >
> > In 'NI_All' we may want to reduce the indent when multiple namespaces are
> > declared in the same line, but this would become inconsistent if the
> > beginning and end of all namespaces do not match:
> >
> > namepace A { namespace B {
> > int i;
> > } // namespace B
> > int i;
> > } // namespace A
> >
> > So I think reducing indent in that case should be (if ever needed) another
> > value in NamespaceIndentation...
> >
> > > I am wondering whether we should generally rename this to NamespaceStyle
> > > and make it an enum. That way, we can start to also support C++17
> > > namespace, but people that don't want to use C++17 yet, can still use
> > > this style of compact namespace.
> >
> > As for C++17, I am not sure we need another option: just having
> > CompactNamespaces=true and Standard=C++17 would use the "real" C++17 mode.
> > That said converting to C++17 namespace blocks is slightly more
> > restrictive, as it will require that both the beginning and end of the
> > inner & outer blocks to match...
> >
> > I will keep the boolean flag for now, just let me know if you prefer to
> > have the enum in case other modes are needed and I will update the patch.
> Yeah, this is probably something nobody will ever want:
>
> namepace A { namespace B {
> int i;
> } // namespace B
> int i;
> } // namespace A
>
> And you have the same problem for NI_Inner as soon as you have more than two
> levels of namespace.
>
> I see two ways forward:
>
> 1. Make "compacted" namespaces always add at most one level of indentation.
> 2. Assume that this can only ever usefully work with the behavior of NI_None
> and add an additional enum value NI_Compact.
>
> The problem I have with #1 is that it increases the complexity quite a bit
> and the behavior is even difficult to predict to users. Remove a comment
> somewhere might enable clang-format to make two namespaces "compact" and then
> remove indentation throughout the file.
>
> So I would lean more towards solution #2. The name NamespaceIndentation might
> then be a bit confusing, but not sure whether it's worth changing it. And of
> course I don't know whether some people would want indentation in compacted
> namespaces.
>
> What do you think?
I think current behavior is at least consistent (and simple to implement), and
I agree that #2 would be the way to go to implement special nanespace indent
rules for compacted namespaces.
Personnally, I don't need namespace indentation (we use NI_None), but i can add
try to add an extra style NI_Compact to indent one level when in any number of
namespaces (e.g. indentLevel = namespaceDepth>0).
That should probably be a separate patch however?
https://reviews.llvm.org/D32480
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32480: clang-format: Add CompactNamespaces option
djasper added a comment. What I mean is that you should remove the CompactNamespace option and instead let this be configured by an additional enum value in NamespaceIndentation. https://reviews.llvm.org/D32480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33366: Fix that global delete operator get's assigned to a submodule.
teemperor updated this revision to Diff 99728.
teemperor edited the summary of this revision.
teemperor added a comment.
- Now unhiding/unowning the created functions like Richard suggested.
- Extended the test case to stress test the lookup with the new visibility
settings.
https://reviews.llvm.org/D33366
Files:
lib/Sema/SemaExprCXX.cpp
test/Modules/Inputs/local-submodule-globaldelete/a.h
test/Modules/Inputs/local-submodule-globaldelete/b.h
test/Modules/Inputs/local-submodule-globaldelete/c.h
test/Modules/Inputs/local-submodule-globaldelete/module.modulemap
test/Modules/local-submodule-globaldelete.cpp
Index: test/Modules/local-submodule-globaldelete.cpp
===
--- /dev/null
+++ test/Modules/local-submodule-globaldelete.cpp
@@ -0,0 +1,23 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cp -r %S/Inputs/local-submodule-globaldelete
%t/local-submodule-globaldelete
+// RUN: cd %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -verify
-fmodules-cache-path=%t -fimplicit-module-maps -I local-submodule-globaldelete
%s -H
+
+// expected-no-diagnostics
+
+// This tests that the global delete operator is not by accident assigned
+// to a submodule in local-submodule-visibility mode. This could happen
+// when the operator is created on demand when we discover a virtual destructor
+// inside a module.
+
+// Build a module with submodules that each need the global delete operator.
+#include "a.h"
+// Build another module with another global delete operator to check that
+// we didn't introduced ambiguity in the lookup.
+#include "c.h"
+
+// Require a lookup of the global delete operator.
+class T {
+ virtual ~T() {}
+};
Index: test/Modules/Inputs/local-submodule-globaldelete/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/local-submodule-globaldelete/module.modulemap
@@ -0,0 +1,7 @@
+module M {
+ module a { header "a.h" export * }
+ module b { header "b.h" export * }
+}
+module Other {
+ module c { header "c.h" export * }
+}
Index: test/Modules/Inputs/local-submodule-globaldelete/c.h
===
--- /dev/null
+++ test/Modules/Inputs/local-submodule-globaldelete/c.h
@@ -0,0 +1,6 @@
+#ifndef C_H
+#define C_H
+class C {
+ virtual ~C(){};
+};
+#endif
Index: test/Modules/Inputs/local-submodule-globaldelete/b.h
===
--- /dev/null
+++ test/Modules/Inputs/local-submodule-globaldelete/b.h
@@ -0,0 +1,6 @@
+#ifndef B_H
+#define B_H
+class B {
+ virtual ~B(){};
+};
+#endif
Index: test/Modules/Inputs/local-submodule-globaldelete/a.h
===
--- /dev/null
+++ test/Modules/Inputs/local-submodule-globaldelete/a.h
@@ -0,0 +1,6 @@
+#ifndef A_H
+#define A_H
+class A {
+ virtual ~A(){};
+};
+#endif
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -2658,6 +2658,15 @@
Context, GlobalCtx, SourceLocation(), SourceLocation(), Name,
FnType, /*TInfo=*/nullptr, SC_None, false, true);
Alloc->setImplicit();
+// If we're in local visibility mode, we reuse this allocation function
+// in all submodules of the current module. To make sure that the other
+// submodules can lookup this function, we unhide it and make sure it's
+// not owned by the current submodule.
+if (getLangOpts().ModulesLocalVisibility &&
+!getLangOpts().CurrentModule.empty()) {
+ Alloc->setLocalOwningModule(nullptr);
+ Alloc->setHidden(false);
+}
// Implicit sized deallocation functions always have default visibility.
Alloc->addAttr(
Index: test/Modules/local-submodule-globaldelete.cpp
===
--- /dev/null
+++ test/Modules/local-submodule-globaldelete.cpp
@@ -0,0 +1,23 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cp -r %S/Inputs/local-submodule-globaldelete %t/local-submodule-globaldelete
+// RUN: cd %t
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -verify -fmodules-cache-path=%t -fimplicit-module-maps -I local-submodule-globaldelete %s -H
+
+// expected-no-diagnostics
+
+// This tests that the global delete operator is not by accident assigned
+// to a submodule in local-submodule-visibility mode. This could happen
+// when the operator is created on demand when we discover a virtual destructor
+// inside a module.
+
+// Build a module with submodules that each need the global delete operator.
+#include "a.h"
+// Build another module with another global delete operator to check that
+// we didn't introduced ambiguity in the lookup.
+#include "c.h"
+
+// Require a lookup of the global delete operator.
+class T {
+ virtual ~T() {}
+};
Index: test/Mo
[PATCH] D33394: [clang-format] Keep trailing preprocessor line comments separate from the following section comments
krasimir updated this revision to Diff 99729.
krasimir marked an inline comment as done.
krasimir added a comment.
- Address review comments
https://reviews.llvm.org/D33394
Files:
lib/Format/UnwrappedLineParser.cpp
unittests/Format/FormatTestComments.cpp
Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -1020,6 +1020,38 @@
getLLVMStyleWithColumns(20)));
}
+TEST_F(FormatTestComments, KeepsTrailingPPCommentsAndSectionCommentsSeparate) {
+ verifyFormat("#ifdef A // line about A\n"
+ "// section comment\n"
+ "#endif",
+ getLLVMStyleWithColumns(80));
+ verifyFormat("#ifdef A // line 1 about A\n"
+ " // line 2 about A\n"
+ "// section comment\n"
+ "#endif",
+ getLLVMStyleWithColumns(80));
+ EXPECT_EQ("#ifdef A // line 1 about A\n"
+" // line 2 about A\n"
+"// section comment\n"
+"#endif",
+format("#ifdef A // line 1 about A\n"
+ " // line 2 about A\n"
+ "// section comment\n"
+ "#endif",
+ getLLVMStyleWithColumns(80)));
+ verifyFormat("int f() {\n"
+ " int i;\n"
+ "#ifdef A // comment about A\n"
+ " // section comment 1\n"
+ " // section comment 2\n"
+ " i = 2;\n"
+ "#else // comment about #else\n"
+ " // section comment 3\n"
+ " i = 4;\n"
+ "#endif\n"
+ "}", getLLVMStyleWithColumns(80));
+}
+
TEST_F(FormatTestComments, CommentsInStaticInitializers) {
EXPECT_EQ(
"static SomeType type = {, /* comment */\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -60,6 +60,21 @@
FormatTok.TokenText.startswith("//");
}
+// Checks if \p FormatTok is a line comment that continues the line comment
+// \p Previous. The original column of \p MinColumnToken is used to determine
+// whether \p FormatTok is indented enough to the right to continue \p Previous.
+static bool continuesLineComment(const FormatToken &FormatTok,
+ const FormatToken *Previous,
+ const FormatToken *MinColumnToken) {
+ if (!Previous || !MinColumnToken)
+return false;
+ unsigned MinContinueColumn =
+ MinColumnToken->OriginalColumn + (isLineComment(*MinColumnToken) ? 0 : 1);
+ return isLineComment(FormatTok) && FormatTok.NewlinesBefore == 1 &&
+ isLineComment(*Previous) &&
+ FormatTok.OriginalColumn >= MinContinueColumn;
+}
+
class ScopedMacroState : public FormatTokenSource {
public:
ScopedMacroState(UnwrappedLine &Line, FormatTokenSource *&TokenSource,
@@ -101,8 +116,8 @@
private:
bool eof() {
return Token && Token->HasUnescapedNewline &&
- !(PreviousToken && isLineComment(*PreviousToken) &&
- isLineComment(*Token) && Token->NewlinesBefore == 1);
+ !(continuesLineComment(*Token, PreviousToken,
+ /*MinColumnToken=*/PreviousToken));
}
FormatToken *getFakeEOF() {
@@ -2106,9 +2121,9 @@
// Checks if \p FormatTok is a line comment that continues the line comment
// section on \p Line.
-static bool continuesLineComment(const FormatToken &FormatTok,
- const UnwrappedLine &Line,
- llvm::Regex &CommentPragmasRegex) {
+static bool continuesLineCommentSection(const FormatToken &FormatTok,
+const UnwrappedLine &Line,
+llvm::Regex &CommentPragmasRegex) {
if (Line.Tokens.empty())
return false;
@@ -2207,12 +,8 @@
MinColumnToken = PreviousToken;
}
- unsigned MinContinueColumn =
- MinColumnToken->OriginalColumn +
- (isLineComment(*MinColumnToken) ? 0 : 1);
- return isLineComment(FormatTok) && FormatTok.NewlinesBefore == 1 &&
- isLineComment(*(Line.Tokens.back().Tok)) &&
- FormatTok.OriginalColumn >= MinContinueColumn;
+ return continuesLineComment(FormatTok, /*Previous=*/Line.Tokens.back().Tok,
+ MinColumnToken);
}
void UnwrappedLineParser::flushComments(bool NewlineBeforeNext) {
@@ -2230,7 +2241,7 @@
// FIXME: Consider putting separate line comment sections as children to the
// unwrapped line instead.
(*I)->ContinuesLineCommentSection =
-continuesLineComment(**I, *Line, CommentPragmasRegex);
+continuesLineCommentSection(**I, *Line, CommentPragmasRegex);
if (isOnNewLine(**I) && Ju
[PATCH] D33394: [clang-format] Keep trailing preprocessor line comments separate from the following section comments
krasimir marked an inline comment as done. krasimir added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:106 + isLineComment(*Token) && Token->NewlinesBefore == 1 && + Token->OriginalColumn == PreviousToken->OriginalColumn); } djasper wrote: > Any chance of moving this logic to distributeComments? I think that's the > place we previously tried to encapsulate this logic in. If not, this is fine > for now. Because of the way parsing around macros works, moving this to distributeComments is gonna be super-tricky: I tried this approach first for a while and gave up. However, I can extract the common logic here and in continuesLineComment at least. https://reviews.llvm.org/D33394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33394: [clang-format] Keep trailing preprocessor line comments separate from the following section comments
djasper accepted this revision. djasper added inline comments. This revision is now accepted and ready to land. Comment at: lib/Format/UnwrappedLineParser.cpp:106 + isLineComment(*Token) && Token->NewlinesBefore == 1 && + Token->OriginalColumn == PreviousToken->OriginalColumn); } krasimir wrote: > djasper wrote: > > Any chance of moving this logic to distributeComments? I think that's the > > place we previously tried to encapsulate this logic in. If not, this is > > fine for now. > Because of the way parsing around macros works, moving this to > distributeComments is gonna be super-tricky: I tried this approach first for > a while and gave up. However, I can extract the common logic here and in > continuesLineComment at least. Yeah. Much nicer. Thank you. Comment at: lib/Format/UnwrappedLineParser.cpp:119 return Token && Token->HasUnescapedNewline && - !(PreviousToken && isLineComment(*PreviousToken) && - isLineComment(*Token) && Token->NewlinesBefore == 1); + !(continuesLineComment(*Token, PreviousToken, + /*MinColumnToken=*/PreviousToken)); Remove parentheses? https://reviews.llvm.org/D33394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33394: [clang-format] Keep trailing preprocessor line comments separate from the following section comments
krasimir updated this revision to Diff 99731.
krasimir marked an inline comment as done.
krasimir added a comment.
- Remove parentheses
https://reviews.llvm.org/D33394
Files:
lib/Format/UnwrappedLineParser.cpp
unittests/Format/FormatTestComments.cpp
Index: unittests/Format/FormatTestComments.cpp
===
--- unittests/Format/FormatTestComments.cpp
+++ unittests/Format/FormatTestComments.cpp
@@ -1020,6 +1020,38 @@
getLLVMStyleWithColumns(20)));
}
+TEST_F(FormatTestComments, KeepsTrailingPPCommentsAndSectionCommentsSeparate) {
+ verifyFormat("#ifdef A // line about A\n"
+ "// section comment\n"
+ "#endif",
+ getLLVMStyleWithColumns(80));
+ verifyFormat("#ifdef A // line 1 about A\n"
+ " // line 2 about A\n"
+ "// section comment\n"
+ "#endif",
+ getLLVMStyleWithColumns(80));
+ EXPECT_EQ("#ifdef A // line 1 about A\n"
+" // line 2 about A\n"
+"// section comment\n"
+"#endif",
+format("#ifdef A // line 1 about A\n"
+ " // line 2 about A\n"
+ "// section comment\n"
+ "#endif",
+ getLLVMStyleWithColumns(80)));
+ verifyFormat("int f() {\n"
+ " int i;\n"
+ "#ifdef A // comment about A\n"
+ " // section comment 1\n"
+ " // section comment 2\n"
+ " i = 2;\n"
+ "#else // comment about #else\n"
+ " // section comment 3\n"
+ " i = 4;\n"
+ "#endif\n"
+ "}", getLLVMStyleWithColumns(80));
+}
+
TEST_F(FormatTestComments, CommentsInStaticInitializers) {
EXPECT_EQ(
"static SomeType type = {, /* comment */\n"
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -60,6 +60,21 @@
FormatTok.TokenText.startswith("//");
}
+// Checks if \p FormatTok is a line comment that continues the line comment
+// \p Previous. The original column of \p MinColumnToken is used to determine
+// whether \p FormatTok is indented enough to the right to continue \p Previous.
+static bool continuesLineComment(const FormatToken &FormatTok,
+ const FormatToken *Previous,
+ const FormatToken *MinColumnToken) {
+ if (!Previous || !MinColumnToken)
+return false;
+ unsigned MinContinueColumn =
+ MinColumnToken->OriginalColumn + (isLineComment(*MinColumnToken) ? 0 : 1);
+ return isLineComment(FormatTok) && FormatTok.NewlinesBefore == 1 &&
+ isLineComment(*Previous) &&
+ FormatTok.OriginalColumn >= MinContinueColumn;
+}
+
class ScopedMacroState : public FormatTokenSource {
public:
ScopedMacroState(UnwrappedLine &Line, FormatTokenSource *&TokenSource,
@@ -101,8 +116,8 @@
private:
bool eof() {
return Token && Token->HasUnescapedNewline &&
- !(PreviousToken && isLineComment(*PreviousToken) &&
- isLineComment(*Token) && Token->NewlinesBefore == 1);
+ !continuesLineComment(*Token, PreviousToken,
+ /*MinColumnToken=*/PreviousToken);
}
FormatToken *getFakeEOF() {
@@ -2106,9 +2121,9 @@
// Checks if \p FormatTok is a line comment that continues the line comment
// section on \p Line.
-static bool continuesLineComment(const FormatToken &FormatTok,
- const UnwrappedLine &Line,
- llvm::Regex &CommentPragmasRegex) {
+static bool continuesLineCommentSection(const FormatToken &FormatTok,
+const UnwrappedLine &Line,
+llvm::Regex &CommentPragmasRegex) {
if (Line.Tokens.empty())
return false;
@@ -2207,12 +,8 @@
MinColumnToken = PreviousToken;
}
- unsigned MinContinueColumn =
- MinColumnToken->OriginalColumn +
- (isLineComment(*MinColumnToken) ? 0 : 1);
- return isLineComment(FormatTok) && FormatTok.NewlinesBefore == 1 &&
- isLineComment(*(Line.Tokens.back().Tok)) &&
- FormatTok.OriginalColumn >= MinContinueColumn;
+ return continuesLineComment(FormatTok, /*Previous=*/Line.Tokens.back().Tok,
+ MinColumnToken);
}
void UnwrappedLineParser::flushComments(bool NewlineBeforeNext) {
@@ -2230,7 +2241,7 @@
// FIXME: Consider putting separate line comment sections as children to the
// unwrapped line instead.
(*I)->ContinuesLineCommentSection =
-continuesLineComment(**I, *Line, CommentPragmasRegex);
+continuesLineCommentSection(**I, *Line, CommentPragmasRegex);
if (isOnNewLine(**I) && JustCommen
r303541 - [clang-format] Keep trailing preprocessor line comments separate from the following section comments
Author: krasimir
Date: Mon May 22 05:07:56 2017
New Revision: 303541
URL: http://llvm.org/viewvc/llvm-project?rev=303541&view=rev
Log:
[clang-format] Keep trailing preprocessor line comments separate from the
following section comments
Summary:
r303415 changed the way a sequence of line comments following a preprocessor
macro is handled, which has the unfortunate effect of aligning a trailing
preprocessor line comment and following unrelated section comments, so:
```
#ifdef A // comment about A
// section comment
#endif
```
gets turned into:
```
#ifdef A // comment about A
// section comment
#endif
```
This patch fixes this by additionally checking the original start columns of
the line comments.
Reviewers: djasper
Reviewed By: djasper
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D33394
Modified:
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTestComments.cpp
Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=303541&r1=303540&r2=303541&view=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Mon May 22 05:07:56 2017
@@ -60,6 +60,21 @@ static bool isLineComment(const FormatTo
FormatTok.TokenText.startswith("//");
}
+// Checks if \p FormatTok is a line comment that continues the line comment
+// \p Previous. The original column of \p MinColumnToken is used to determine
+// whether \p FormatTok is indented enough to the right to continue \p
Previous.
+static bool continuesLineComment(const FormatToken &FormatTok,
+ const FormatToken *Previous,
+ const FormatToken *MinColumnToken) {
+ if (!Previous || !MinColumnToken)
+return false;
+ unsigned MinContinueColumn =
+ MinColumnToken->OriginalColumn + (isLineComment(*MinColumnToken) ? 0 :
1);
+ return isLineComment(FormatTok) && FormatTok.NewlinesBefore == 1 &&
+ isLineComment(*Previous) &&
+ FormatTok.OriginalColumn >= MinContinueColumn;
+}
+
class ScopedMacroState : public FormatTokenSource {
public:
ScopedMacroState(UnwrappedLine &Line, FormatTokenSource *&TokenSource,
@@ -101,8 +116,8 @@ public:
private:
bool eof() {
return Token && Token->HasUnescapedNewline &&
- !(PreviousToken && isLineComment(*PreviousToken) &&
- isLineComment(*Token) && Token->NewlinesBefore == 1);
+ !continuesLineComment(*Token, PreviousToken,
+ /*MinColumnToken=*/PreviousToken);
}
FormatToken *getFakeEOF() {
@@ -2106,9 +2121,9 @@ bool UnwrappedLineParser::isOnNewLine(co
// Checks if \p FormatTok is a line comment that continues the line comment
// section on \p Line.
-static bool continuesLineComment(const FormatToken &FormatTok,
- const UnwrappedLine &Line,
- llvm::Regex &CommentPragmasRegex) {
+static bool continuesLineCommentSection(const FormatToken &FormatTok,
+const UnwrappedLine &Line,
+llvm::Regex &CommentPragmasRegex) {
if (Line.Tokens.empty())
return false;
@@ -2207,12 +,8 @@ static bool continuesLineComment(const F
MinColumnToken = PreviousToken;
}
- unsigned MinContinueColumn =
- MinColumnToken->OriginalColumn +
- (isLineComment(*MinColumnToken) ? 0 : 1);
- return isLineComment(FormatTok) && FormatTok.NewlinesBefore == 1 &&
- isLineComment(*(Line.Tokens.back().Tok)) &&
- FormatTok.OriginalColumn >= MinContinueColumn;
+ return continuesLineComment(FormatTok, /*Previous=*/Line.Tokens.back().Tok,
+ MinColumnToken);
}
void UnwrappedLineParser::flushComments(bool NewlineBeforeNext) {
@@ -2230,7 +2241,7 @@ void UnwrappedLineParser::flushComments(
// FIXME: Consider putting separate line comment sections as children to
the
// unwrapped line instead.
(*I)->ContinuesLineCommentSection =
-continuesLineComment(**I, *Line, CommentPragmasRegex);
+continuesLineCommentSection(**I, *Line, CommentPragmasRegex);
if (isOnNewLine(**I) && JustComments && !(*I)->ContinuesLineCommentSection)
addUnwrappedLine();
pushToken(*I);
@@ -2263,7 +2274,7 @@ void UnwrappedLineParser::distributeComm
const SmallVectorImpl &Comments,
const FormatToken *NextTok) {
// Whether or not a line comment token continues a line is controlled by
- // the method continuesLineComment, with the following caveat:
+ // the method continuesLineCommentSection, with the following caveat:
//
// Define a trail of Comments to be a nonempty proper postfix of Comments
such
// that each comment line from the trail is alig
[PATCH] D33394: [clang-format] Keep trailing preprocessor line comments separate from the following section comments
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303541: [clang-format] Keep trailing preprocessor line
comments separate from the… (authored by krasimir).
Changed prior to commit:
https://reviews.llvm.org/D33394?vs=99731&id=99734#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33394
Files:
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTestComments.cpp
Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -60,6 +60,21 @@
FormatTok.TokenText.startswith("//");
}
+// Checks if \p FormatTok is a line comment that continues the line comment
+// \p Previous. The original column of \p MinColumnToken is used to determine
+// whether \p FormatTok is indented enough to the right to continue \p Previous.
+static bool continuesLineComment(const FormatToken &FormatTok,
+ const FormatToken *Previous,
+ const FormatToken *MinColumnToken) {
+ if (!Previous || !MinColumnToken)
+return false;
+ unsigned MinContinueColumn =
+ MinColumnToken->OriginalColumn + (isLineComment(*MinColumnToken) ? 0 : 1);
+ return isLineComment(FormatTok) && FormatTok.NewlinesBefore == 1 &&
+ isLineComment(*Previous) &&
+ FormatTok.OriginalColumn >= MinContinueColumn;
+}
+
class ScopedMacroState : public FormatTokenSource {
public:
ScopedMacroState(UnwrappedLine &Line, FormatTokenSource *&TokenSource,
@@ -101,8 +116,8 @@
private:
bool eof() {
return Token && Token->HasUnescapedNewline &&
- !(PreviousToken && isLineComment(*PreviousToken) &&
- isLineComment(*Token) && Token->NewlinesBefore == 1);
+ !continuesLineComment(*Token, PreviousToken,
+ /*MinColumnToken=*/PreviousToken);
}
FormatToken *getFakeEOF() {
@@ -2106,9 +2121,9 @@
// Checks if \p FormatTok is a line comment that continues the line comment
// section on \p Line.
-static bool continuesLineComment(const FormatToken &FormatTok,
- const UnwrappedLine &Line,
- llvm::Regex &CommentPragmasRegex) {
+static bool continuesLineCommentSection(const FormatToken &FormatTok,
+const UnwrappedLine &Line,
+llvm::Regex &CommentPragmasRegex) {
if (Line.Tokens.empty())
return false;
@@ -2207,12 +,8 @@
MinColumnToken = PreviousToken;
}
- unsigned MinContinueColumn =
- MinColumnToken->OriginalColumn +
- (isLineComment(*MinColumnToken) ? 0 : 1);
- return isLineComment(FormatTok) && FormatTok.NewlinesBefore == 1 &&
- isLineComment(*(Line.Tokens.back().Tok)) &&
- FormatTok.OriginalColumn >= MinContinueColumn;
+ return continuesLineComment(FormatTok, /*Previous=*/Line.Tokens.back().Tok,
+ MinColumnToken);
}
void UnwrappedLineParser::flushComments(bool NewlineBeforeNext) {
@@ -2230,7 +2241,7 @@
// FIXME: Consider putting separate line comment sections as children to the
// unwrapped line instead.
(*I)->ContinuesLineCommentSection =
-continuesLineComment(**I, *Line, CommentPragmasRegex);
+continuesLineCommentSection(**I, *Line, CommentPragmasRegex);
if (isOnNewLine(**I) && JustComments && !(*I)->ContinuesLineCommentSection)
addUnwrappedLine();
pushToken(*I);
@@ -2263,7 +2274,7 @@
const SmallVectorImpl &Comments,
const FormatToken *NextTok) {
// Whether or not a line comment token continues a line is controlled by
- // the method continuesLineComment, with the following caveat:
+ // the method continuesLineCommentSection, with the following caveat:
//
// Define a trail of Comments to be a nonempty proper postfix of Comments such
// that each comment line from the trail is aligned with the next token, if
@@ -2301,7 +2312,7 @@
FormatTok->ContinuesLineCommentSection = false;
} else {
FormatTok->ContinuesLineCommentSection =
- continuesLineComment(*FormatTok, *Line, CommentPragmasRegex);
+ continuesLineCommentSection(*FormatTok, *Line, CommentPragmasRegex);
}
if (!FormatTok->ContinuesLineCommentSection &&
(isOnNewLine(*FormatTok) || FormatTok->IsFirst)) {
Index: cfe/trunk/unittests/Format/FormatTestComments.cpp
===
--- cfe/trunk/unittests/Format/FormatTestComments.cpp
+++ cfe/trunk/unittests/Format/FormatTestComments.cpp
@@ -1020,6 +1020,38 @@
getLLVMStyleWithColumns(20)));
}
+TEST_F(FormatTestComments, KeepsTrailingPPCommentsAndSectionCommentsSeparate) {
+ verifyFormat("#ifdef A // line about A\n"
+ "// section comment\
[PATCH] D33401: [mips] Add runtime options to enable/disable generation of madd.fmt, msub.fmt
smaksimovic created this revision.
Herald added a subscriber: arichardson.
Added options to clang are -mmadd4 and -mno-madd4, used to enable or disable
generation
of madd.fmt and similar instructions respectively, as per GCC.
https://reviews.llvm.org/D33401
Files:
include/clang/Driver/Options.td
lib/Driver/ToolChains/Arch/Mips.cpp
test/CodeGen/mips-madd4.c
Index: test/CodeGen/mips-madd4.c
===
--- test/CodeGen/mips-madd4.c
+++ test/CodeGen/mips-madd4.c
@@ -0,0 +1,86 @@
+// RUN: %clang --target=mips64-unknown-linux -S -mmadd4%s -o -| FileCheck %s -check-prefix=MADD4
+// RUN: %clang --target=mips64-unknown-linux -S -mno-madd4 %s -o -| FileCheck %s -check-prefix=NOMADD4
+// RUN: %clang --target=mips64-unknown-linux -S -mmadd4-fno-honor-nans %s -o -| FileCheck %s -check-prefix=MADD4-NONAN
+// RUN: %clang --target=mips64-unknown-linux -S -mno-madd4 -fno-honor-nans %s -o -| FileCheck %s -check-prefix=NOMADD4-NONAN
+
+float madd_s (float f, float g, float h)
+{
+ return (f * g) + h;
+}
+// MADD4: madd.s
+// NOMADD4: mul.s
+// NOMADD4: add.s
+
+float msub_s (float f, float g, float h)
+{
+ return (f * g) - h;
+}
+// MADD4: msub.s
+// NOMADD4: mul.s
+// NOMADD4: sub.s
+
+double madd_d (double f, double g, double h)
+{
+ return (f * g) + h;
+}
+// MADD4: madd.d
+// NOMADD4: mul.d
+// NOMADD4: add.d
+
+double msub_d (double f, double g, double h)
+{
+ return (f * g) - h;
+}
+// MADD4: msub.d
+// NOMADD4: mul.d
+// NOMADD4: sub.d
+
+
+float nmadd_s (float f, float g, float h)
+{
+ // FIXME: Zero has been explicitly placed to force generation of a positive
+ // zero in IR until pattern used to match this instruction is changed to
+ // comply with negative zero as well.
+ return 0-((f * g) + h);
+}
+// MADD4-NONAN: nmadd.s
+// NOMADD4-NONAN: mul.s
+// NOMADD4-NONAN: add.s
+// NOMADD4-NONAN: sub.s
+
+float nmsub_s (float f, float g, float h)
+{
+ // FIXME: Zero has been explicitly placed to force generation of a positive
+ // zero in IR until pattern used to match this instruction is changed to
+ // comply with negative zero as well.
+ return 0-((f * g) - h);
+}
+// MADD4-NONAN: nmsub.s
+// NOMADD4-NONAN: mul.s
+// NOMADD4-NONAN: sub.s
+// NOMADD4-NONAN: sub.s
+
+double nmadd_d (double f, double g, double h)
+{
+ // FIXME: Zero has been explicitly placed to force generation of a positive
+ // zero in IR until pattern used to match this instruction is changed to
+ // comply with negative zero as well.
+ return 0-((f * g) + h);
+}
+// MADD4-NONAN: nmadd.d
+// NOMADD4-NONAN: mul.d
+// NOMADD4-NONAN: add.d
+// NOMADD4-NONAN: sub.d
+
+double nmsub_d (double f, double g, double h)
+{
+ // FIXME: Zero has been explicitly placed to force generation of a positive
+ // zero in IR until pattern used to match this instruction is changed to
+ // comply with negative zero as well.
+ return 0-((f * g) - h);
+}
+// MADD4-NONAN: nmsub.d
+// NOMADD4-NONAN: mul.d
+// NOMADD4-NONAN: sub.d
+// NOMADD4-NONAN: sub.d
+
Index: lib/Driver/ToolChains/Arch/Mips.cpp
===
--- lib/Driver/ToolChains/Arch/Mips.cpp
+++ lib/Driver/ToolChains/Arch/Mips.cpp
@@ -298,6 +298,13 @@
AddTargetFeature(Args, Features, options::OPT_mno_odd_spreg,
options::OPT_modd_spreg, "nooddspreg");
+
+ if(Arg *A = Args.getLastArg(options::OPT_mmadd4, options::OPT_mno_madd4)) {
+if(A->getOption().matches(options::OPT_mmadd4))
+ Features.push_back("+madd4");
+else
+ Features.push_back("-madd4");
+ }
}
mips::NanEncoding mips::getSupportedNanEncoding(StringRef &CPU) {
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1998,6 +1998,10 @@
def mno_dspr2 : Flag<["-"], "mno-dspr2">, Group;
def msingle_float : Flag<["-"], "msingle-float">, Group;
def mdouble_float : Flag<["-"], "mdouble-float">, Group;
+def mmadd4 : Flag<["-"], "mmadd4">, Group,
+ HelpText<"Enable the generation of 4-operand madd.s, madd.d and related instructions.">;
+def mno_madd4 : Flag<["-"], "mno-madd4">, Group,
+ HelpText<"Disable the generation of 4-operand madd.s, madd.d and related instructions.">;
def mmsa : Flag<["-"], "mmsa">, Group,
HelpText<"Enable MSA ASE (MIPS only)">;
def mno_msa : Flag<["-"], "mno-msa">, Group,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33363: [mips] Support `micromips` attribute
sdardis accepted this revision. sdardis added a comment. This revision is now accepted and ready to land. LGTM. Repository: rL LLVM https://reviews.llvm.org/D33363 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33364: [mips] Support micromips attribute passed by front-end
sdardis accepted this revision. sdardis added a comment. This revision is now accepted and ready to land. LGTM. Repository: rL LLVM https://reviews.llvm.org/D33364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33364: [mips] Support micromips attribute passed by front-end
sdardis added a comment. You should also add a test case to test/CodeGen/Mips/ showing that this attribute sets the assembler into micromips mode for some trivial function, along with another function without micromips being generated as non micromips code. Repository: rL LLVM https://reviews.llvm.org/D33364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32896: [OpenCL] Make CLK_NULL_RESERVE_ID invalid reserve id.
bader added inline comments. Comment at: lib/Headers/opencl-c.h:16020 +// The macro CLK_NULL_RESERVE_ID refers to an invalid reservation ID. +#define CLK_NULL_RESERVE_ID (__builtin_astype((void *)0, reserve_id_t)) bool __ovld is_valid_reserve_id(reserve_id_t reserve_id); yaxunl wrote: > yaxunl wrote: > > bader wrote: > > > yaxunl wrote: > > > > Anastasia wrote: > > > > > echuraev wrote: > > > > > > yaxunl wrote: > > > > > > > Anastasia wrote: > > > > > > > > yaxunl wrote: > > > > > > > > > Anastasia wrote: > > > > > > > > > > Looks good from my side. > > > > > > > > > > > > > > > > > > > > @yaxunl , since you originally committed this. Could you > > > > > > > > > > please verify that changing from `SIZE_MAX` to `0` would be > > > > > > > > > > fine. > > > > > > > > > > > > > > > > > > > > Btw, we have a similar definition for `CLK_NULL_EVENT`. > > > > > > > > > `__PIPE_RESERVE_ID_VALID_BIT` is implementation detail and > > > > > > > > > not part of the spec. I would suggest to remove it from this > > > > > > > > > header file. > > > > > > > > > > > > > > > > > > The spec only requires CLK_NULL_RESERVE_ID to be defined but > > > > > > > > > does not define its value. Naturally a valid id starts from 0 > > > > > > > > > and increases. I don't see significant advantage to change > > > > > > > > > CLK_NULL_RESERVE_ID from __SIZE_MAX to 0. > > > > > > > > > > > > > > > > > > Is there any reason that this change is needed? > > > > > > > > I don't see issues to commit things outside of spec as soon as > > > > > > > > they prefixed properly with "__". But I agree it would be nice > > > > > > > > to see if it's any useful and what the motivation is for having > > > > > > > > different implementation. > > > > > > > For `__PIPE_RESERVE_ID_VALID_BIT`, it assumes that the > > > > > > > implementation uses one specific bit of a reserve id to indicate > > > > > > > that the reserve id is valid. Not all implementations assume > > > > > > > that. Actually I am curious why that is needed too. > > > > > > About `CLK_NULL_RESERVE_ID`: we check that reserve id is valid if > > > > > > significant bit equal to one. `CLK_NULL_RESERVE_ID refers to an > > > > > > invalid reservation, so if `CLK_NULL_RESERVE_ID equal to 0, we can > > > > > > be sure that significant bit doesn't equal to 1 and it is invalid > > > > > > reserve id. Also it is more obviously if CLK_**NULL**_RESERVE_ID is > > > > > > equal to 0. > > > > > > > > > > > > What about `__PIPE_RESERVE_ID_VALID_BIT`: As I understand previous > > > > > > implementation also assumes that one specific bit was of a reverse > > > > > > id was used to indicate that the reserve id is valid. So, we just > > > > > > increased reserve id size by one bit on 32-bit platforms and by 33 > > > > > > bits on 64-bit platforms. > > > > > It is more logical to me that `CLK_NULL_RESERVE_ID` is 0, but spec > > > > > doesn't define it of course. > > > > In our implementation, valid reserve id starts at 0 and increasing > > > > linearly until `__SIZE_MAX-1`. This change will break our > > > > implementation. > > > > > > > > However, we can modify our implementation to adopt this change since it > > > > brings about benefits overall. > > > Ideally it would be great to have unified implementation, but we can > > > define device specific value for CLK_NULL_RESERVE_ID by using ifdef > > > directive. > > How about > > > > ``` > > __attribute__((const)) size_t __clk_null_reserve_id(); > > #define CLK_NULL_RESERVE_ID __clk_null_reserve_id() > > > > ``` > > I think the spec does not require it to be compile time constant. Then each > > library can implement its own __clk_null_reserve_id() whereas the IR is > > target independent. > Or we only do this for SPIR and define it as target specific value for other > targets. Defining CLK_NULL_RESERVE_ID as a function call should also work, but IMHO compile time constant is preferable option. I don't think making it compile time constant for SPIR only makes sense to me - in this case we can use constant for all targets. How about following approach: use 0 by default and allow other targets re-define CLK_NULL_RESERVE_ID value. ``` #ifndef CLK_NULL_RESERVE_ID #define CLK_NULL_RESERVE_ID 0 #endif // CLK_NULL_RESERVE_ID ``` If CLK_NULL_RESERVE_ID defined via -D command line option or included before OpenCL C header file (via -include option), the defined value will be used, otherwise 0. Will it work for you? https://reviews.llvm.org/D32896 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D33393: [PATCH] Libcxxabi Demangler PR32890
FYI: Passes "make check-cxxabi" for LLVM in trunk on my machine (x86-64, Ubuntu 16.04) with following standard configuration: lit.py: /src/try/llvm/projects/libcxx/utils/libcxx/test/config.py:150: note: Using compiler: /usr/local/bin/clang++ lit.py: /src/try/llvm/projects/libcxx/utils/libcxx/test/config.py:151: note: Using flags: ['-v', '-D_LIBCPP_DISABLE_AVAILABILITY'] lit.py: /src/try/llvm/projects/libcxx/utils/libcxx/test/config.py:156: note: Using compile flags: ['-Werror=thread-safety', '-DLIBCXXABI_NO_TIMER', '-funwind-tables', '-std=c++1z', '-nostdinc++', '-I/src/try/llvm/projects/libcxx/include', '-I/src/try/llvm/projects/libcxxabi/include', '-D__STDC_FORMAT_MACROS', '-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', '-I/src/try/llvm/projects/libcxx/test/support'] lit.py: /src/try/llvm/projects/libcxx/utils/libcxx/test/config.py:158: note: Using warnings: ['-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER', '-Wall', '-Wextra', '-Werror', '-Wuser-defined-warnings', '-Wshadow', '-Wno-unused-command-line-argument', '-Wno-attributes', '-Wno-pessimizing-move', '-Wno-c++11-extensions', '-Wno-user-defined-literals', '-Wsign-compare', '-Wunused-variable', '-Wunused-parameter', '-Wunreachable-code', '-Wno-conversion', '-Wno-unused-local-typedef', '-Wno-#warnings'] lit.py: /src/try/llvm/projects/libcxx/utils/libcxx/test/config.py:159: note: Using link flags: ['-L/src/try/build/./lib', '-Wl,-rpath,/src/try/build/./lib', '-nodefaultlibs', '-lc++', '-lc++abi', '-lm', '-lgcc_s', '-lgcc', '-lpthread', '-lc', '-lgcc_s', '-lgcc'] lit.py: /src/try/llvm/projects/libcxx/utils/libcxx/test/config.py:162: note: Using available_features: ['thread-safety', 'glibc', 'diagnose-if-support', 'glibc-2.23', 'glibc-2', 'clang', 'ubuntu', 'linux', 'modules-support', 'clang-5', 'clang-5.0', 'ubuntu-16.04', 'verify-support', 'libcxxabi-has-system-unwinder', '-faligned-allocation', 'libc++', 'fsized-deallocation', 'long_tests', 'c++1z'] > On 22 May 2017, at 4:02 PM, Marcel Boehme via Phabricator > wrote: > > marcel created this revision. > > Check whether the string (db.names.back().first) is longer than 7 characters > before inserting another string at index 8. > > > https://reviews.llvm.org/D33393 > > Files: > CREDITS.TXT > src/cxa_demangle.cpp > test/test_demangle.pass.cpp > > > Index: test/test_demangle.pass.cpp > === > --- test/test_demangle.pass.cpp > +++ test/test_demangle.pass.cpp > @@ -29665,6 +29665,7 @@ > > "\x44\x74\x70\x74\x71\x75\x32\x43\x41\x38\x65\x6E\x9B\x72\x4D\xC1\x43\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\x6C\x53\xF9\x5F\x70\x74\x70\x69\x45\x38\xD3\x73\x9E\x2A\x37", > > "\x46\x44\x74\x70\x74\x71\x75\x32\x43\x41\x72\x4D\x6E\x65\x34\x9F\xC1\x43\x41\x72\x4D\x6E\x77\x38\x9A\x8E\x44\x6F\x64\x6C\x53\xF9\x5F\x70\x74\x70\x69\x45\x34\xD3\x73\x9E\x2A\x37\x72\x33\x8E\x3A\x29\x8E\x44\x35", >"_ZcvCiIJEEDvT__T_vT_v", > +"Z1JIJ1_T_EE3o00EUlT_E0", > }; > > const unsigned NI = sizeof(invalid_cases) / sizeof(invalid_cases[0]); > Index: src/cxa_demangle.cpp > === > --- src/cxa_demangle.cpp > +++ src/cxa_demangle.cpp > @@ -3075,7 +3075,8 @@ > const char* t1 = t0 + 1; > while (t1 != last && std::isdigit(*t1)) > ++t1; > - > db.names.back().first.insert(db.names.back().first.begin()+7, t0, t1); > +if (db.names.back().first.length() > 7) > + db.names.back().first.insert(db.names.back().first.begin() > + 7, t0, t1); > t0 = t1; > } > if (t0 == last || *t0 != '_') > Index: CREDITS.TXT > === > --- CREDITS.TXT > +++ CREDITS.TXT > @@ -12,6 +12,10 @@ > E: aa...@aaronballman.com > D: Minor patches > > +N: Marcel Boehme > +E: marcel.boe...@acm.org > +D: Minor patches > + > N: Logan Chien > E: logan.ch...@mediatek.com > D: ARM EHABI Unwind & Exception Handling > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33045: [libclang] Avoid more stats than necessary for reparse.
nik added a comment. In https://reviews.llvm.org/D33045#759436, @ilya-biryukov wrote: > Are there any other callers to getMainBufferWithPrecompiledPreamble? Yes. Huch, right... don't know why I didn't adapted those. Done now. > Maybe they cause LibclangReparseTest.ReparseWithModule to fail? So it looks like. Actually that one become flaky now and the other failing ones are: Failing Tests (74): Clang-Unit :: AST/ASTTests/ImportExpr.ImportAtomicExpr Clang-Unit :: AST/ASTTests/ImportExpr.ImportBinaryConditionalOperator Clang-Unit :: AST/ASTTests/ImportExpr.ImportCXXNullPtrLiteralExpr Clang-Unit :: AST/ASTTests/ImportExpr.ImportCXXThisExpr Clang-Unit :: AST/ASTTests/ImportExpr.ImportCompoundLiteralExpr Clang-Unit :: AST/ASTTests/ImportExpr.ImportConditionalOperator Clang-Unit :: AST/ASTTests/ImportExpr.ImportDesignatedInitExpr Clang-Unit :: AST/ASTTests/ImportExpr.ImportFloatinglLiteralExpr Clang-Unit :: AST/ASTTests/ImportExpr.ImportGNUNullExpr Clang-Unit :: AST/ASTTests/ImportExpr.ImportInitListExpr Clang-Unit :: AST/ASTTests/ImportExpr.ImportLabelDeclAndAddrLabelExpr Clang-Unit :: AST/ASTTests/ImportExpr.ImportParenListExpr Clang-Unit :: AST/ASTTests/ImportExpr.ImportPredefinedExpr Clang-Unit :: AST/ASTTests/ImportExpr.ImportStmtExpr Clang-Unit :: AST/ASTTests/ImportExpr.ImportStringLiteral Clang-Unit :: AST/ASTTests/ImportExpr.ImportVAArgExpr Clang-Unit :: AST/ASTTests/ImportType.ImportAtomicType Clang-Unit :: AST/ASTTests/RecursiveASTVisitor.NoPostOrderTraversal Clang-Unit :: AST/ASTTests/RecursiveASTVisitor.PostOrderTraversal Clang-Unit :: ASTMatchers/ASTMatchersTests/AstMatcherPMacro.Works Clang-Unit :: ASTMatchers/ASTMatchersTests/AstPolymorphicMatcherPMacro.Works Clang-Unit :: ASTMatchers/ASTMatchersTests/EachOf.BehavesLikeAnyOfUnlessBothMatch Clang-Unit :: ASTMatchers/ASTMatchersTests/EachOf.TriggersForEachMatch Clang-Unit :: ASTMatchers/ASTMatchersTests/EqualsBoundNodeMatcher.FiltersMatchedCombinations Clang-Unit :: ASTMatchers/ASTMatchersTests/EqualsBoundNodeMatcher.UnlessDescendantsOfAncestorsMatch Clang-Unit :: ASTMatchers/ASTMatchersTests/EqualsBoundNodeMatcher.UsingForEachDescendant Clang-Unit :: ASTMatchers/ASTMatchersTests/FindAll.BindsDescendantNodeOnMatch Clang-Unit :: ASTMatchers/ASTMatchersTests/FindAll.BindsNodeAndDescendantNodesOnOneMatch Clang-Unit :: ASTMatchers/ASTMatchersTests/FindAll.BindsNodeOnMatch Clang-Unit :: ASTMatchers/ASTMatchersTests/ForEach.BindsMultipleNodes Clang-Unit :: ASTMatchers/ASTMatchersTests/ForEach.BindsOneNode Clang-Unit :: ASTMatchers/ASTMatchersTests/ForEach.BindsRecursiveCombinations Clang-Unit :: ASTMatchers/ASTMatchersTests/ForEachArgumentWithParam.HandlesBoundNodesForNonMatches Clang-Unit :: ASTMatchers/ASTMatchersTests/ForEachArgumentWithParam.MatchesCXXMemberCallExpr Clang-Unit :: ASTMatchers/ASTMatchersTests/ForEachArgumentWithParam.MatchesCallExpr Clang-Unit :: ASTMatchers/ASTMatchersTests/ForEachArgumentWithParam.MatchesConstructExpr Clang-Unit :: ASTMatchers/ASTMatchersTests/ForEachDescendant.BindsCombinations Clang-Unit :: ASTMatchers/ASTMatchersTests/ForEachDescendant.BindsCorrectNodes Clang-Unit :: ASTMatchers/ASTMatchersTests/ForEachDescendant.BindsMultipleNodes Clang-Unit :: ASTMatchers/ASTMatchersTests/ForEachDescendant.BindsOneNode Clang-Unit :: ASTMatchers/ASTMatchersTests/ForEachDescendant.BindsRecursiveCombinations Clang-Unit :: ASTMatchers/ASTMatchersTests/ForEachDescendant.NestedForEachDescendant Clang-Unit :: ASTMatchers/ASTMatchersTests/Has.DoesNotDeleteBindings Clang-Unit :: ASTMatchers/ASTMatchersTests/Has.MatchesChildrenOfTypes Clang-Unit :: ASTMatchers/ASTMatchersTests/HasAncestor.BindsCombinationsWithHasDescendant Clang-Unit :: ASTMatchers/ASTMatchersTests/HasAncestor.BindsRecursiveCombinations Clang-Unit :: ASTMatchers/ASTMatchersTests/HasAncestor.MatchesClosestAncestor Clang-Unit :: ASTMatchers/ASTMatchersTests/HasDescendant.MatchesDescendantTypes Clang-Unit :: ASTMatchers/ASTMatchersTests/HasDescendant.MatchesDescendantsOfTypes Clang-Unit :: ASTMatchers/ASTMatchersTests/IsEqualTo.MatchesNodesByIdentity Clang-Unit :: ASTMatchers/ASTMatchersTests/LoopingMatchers.DoNotOverwritePreviousMatchResultOnFailure Clang-Unit :: ASTMatchers/ASTMatchersTests/MatchFinder.CanMatchDeclarationsRecursively Clang-Unit :: ASTMatchers/ASTMatchersTests/MatchFinder.CanMatchSingleNodesRecursively Clang-Unit :: ASTMatchers/ASTMatchersTests/MatchFinder.CanMatchStatementsRecursively Clang-Unit :: ASTMatchers/ASTMatchersTests/MatchFinder.InterceptsEndOfTranslationUnit Clang-Unit :: ASTMatchers/ASTMatchersTests/MatchFinder.InterceptsStartOfTranslationUnit Clang-Unit :: ASTMatchers/A
[PATCH] D33045: [libclang] Avoid more stats than necessary for reparse.
nik updated this revision to Diff 99740.
https://reviews.llvm.org/D33045
Files:
include/clang/Frontend/ASTUnit.h
lib/Frontend/ASTUnit.cpp
tools/libclang/CIndexCodeCompletion.cpp
Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -680,6 +680,7 @@
}
// Parse the resulting source file to find code-completion results.
+ AST->recreateFileManager();
AllocatedCXCodeCompleteResults *Results = new AllocatedCXCodeCompleteResults(
&AST->getFileManager());
Results->Results = nullptr;
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -241,6 +241,10 @@
this->PP = std::move(PP);
}
+void ASTUnit::recreateFileManager() {
+FileMgr = new FileManager(FileMgr->getFileSystemOpts());
+}
+
/// \brief Determine the set of code-completion contexts in which this
/// declaration should be shown.
static unsigned getDeclShowContexts(const NamedDecl *ND,
@@ -1311,6 +1315,10 @@
/// this routine will determine if it is still valid and, if so, avoid
/// rebuilding the precompiled preamble.
///
+/// The caller is required to recreate the FileMgr before calling this routine
+/// to ensure a clean stat cache and on the other hand that the stats done here
+/// can be reused for e.g. Reparse().
+///
/// \param AllowRebuild When true (the default), this routine is
/// allowed to rebuild the precompiled preamble if it is found to be
/// out-of-date.
@@ -1368,59 +1376,58 @@
if (AnyFileChanged)
break;
-vfs::Status Status;
-if (FileMgr->getNoncachedStatValue(R.second, Status)) {
+const FileEntry *fileEntry = FileMgr->getFile(R.second);
+if (!fileEntry) {
// If we can't stat the file we're remapping to, assume that something
// horrible happened.
AnyFileChanged = true;
break;
}
-OverriddenFiles[Status.getUniqueID()] = PreambleFileHash::createForFile(
-Status.getSize(),
-llvm::sys::toTimeT(Status.getLastModificationTime()));
+OverriddenFiles[fileEntry->getUniqueID()] = PreambleFileHash::createForFile(
+fileEntry->getSize(),
+fileEntry->getModificationTime());
}
for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
if (AnyFileChanged)
break;
-vfs::Status Status;
-if (FileMgr->getNoncachedStatValue(RB.first, Status)) {
+const FileEntry *fileEntry = FileMgr->getFile(RB.first);
+if (!fileEntry) {
AnyFileChanged = true;
break;
}
-OverriddenFiles[Status.getUniqueID()] =
+OverriddenFiles[fileEntry->getUniqueID()] =
PreambleFileHash::createForMemoryBuffer(RB.second);
}
// Check whether anything has changed.
for (llvm::StringMap::iterator
F = FilesInPreamble.begin(), FEnd = FilesInPreamble.end();
!AnyFileChanged && F != FEnd;
++F) {
-vfs::Status Status;
-if (FileMgr->getNoncachedStatValue(F->first(), Status)) {
+const FileEntry *fileEntry = FileMgr->getFile(F->first());
+if (!fileEntry) {
// If we can't stat the file, assume that something horrible happened.
AnyFileChanged = true;
break;
}
std::map::iterator Overridden
- = OverriddenFiles.find(Status.getUniqueID());
+ = OverriddenFiles.find(fileEntry->getUniqueID());
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)
AnyFileChanged = true;
continue;
}
-
+
// The file was not remapped; check whether it has changed on disk.
-if (Status.getSize() != uint64_t(F->second.Size) ||
-llvm::sys::toTimeT(Status.getLastModificationTime()) !=
-F->second.ModTime)
+if (fileEntry->getSize() != F->second.Size ||
+fileEntry->getModificationTime() != F->second.ModTime)
AnyFileChanged = true;
}
@@ -1873,6 +1880,8 @@
getDiagnostics().Reset();
ProcessWarningOptions(getDiagnostics(), Invocation->getDiagnosticOpts());
+ recreateFileManager();
+
std::unique_ptr OverrideMainBuffer;
if (PrecompilePreambleAfterNParses > 0) {
PreambleRebuildCounter = PrecompilePreambleAfterNParses;
@@ -2040,15 +2049,16 @@
RemappedFile.second);
}
+ recreateFileManager();
+
// If we have a preamble file lying around, or if we might try to
// build a precompiled preamble, do so n
[PATCH] D33405: Mark all ARCMT test files as binary files in git, just like with svn
erikjv created this revision. Herald added a subscriber: javed.absar. This is a work-around for ARCMT's inability to handle Windows-style line terminators (\r\n) correctly. The underlying problem seems to be that line terminators are assumed to be 1 character long. So with files ending with \r\n the rewritten files end with \r\r\n. See also PR33124. https://reviews.llvm.org/D33405 Files: .gitattributes Index: .gitattributes === --- /dev/null +++ .gitattributes @@ -0,0 +1,6 @@ +# Default behaviour +* text=auto + +# ARCMT test files +test/ARCMT/* binary +test/ARCMT/*/* binary Index: .gitattributes === --- /dev/null +++ .gitattributes @@ -0,0 +1,6 @@ +# Default behaviour +* text=auto + +# ARCMT test files +test/ARCMT/* binary +test/ARCMT/*/* binary ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33395: [clangd] Split clangd into library+executable (mainly for unit tests).
krasimir added inline comments. Comment at: clangd/tool/ClangdMain.cpp:11 +#include "../ClangdLSPServer.h" +#include "../JSONRPCDispatcher.h" #include "llvm/Support/CommandLine.h" I'd suggest to not have parent directory includes, but add them to the cmake file as in other extra tools, like in: https://github.com/llvm-mirror/clang-tools-extra/blob/master/clang-query/tool/CMakeLists.txt https://reviews.llvm.org/D33395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33094: [ASTMatchers] Add clang-query support for equals matcher
aaron.ballman added inline comments.
Comment at: include/clang/ASTMatchers/ASTMatchers.h:3846
+ CXXBoolLiteralExpr,
+ IntegerLiteral),
+ unsigned, Value, 1) {
Lekensteyn wrote:
> aaron.ballman wrote:
> > Is there a reason to not allow the equals matcher to do something like
> > `floatingLiteral(equals(1))`? Sure, the user could always write `1.0`, but
> > it seems somewhat hostile to require it.
> The ValueMatcher for float does not accept integers at the moment, adding
> FloatingLiteral now results in a compile error. It can be added though (might
> do this as well in the next revision, either in the existing patches or a new
> one).
That's a good reason not to do it right now. Thanks!
https://reviews.llvm.org/D33094
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33135: [ASTMatchers] Add support for floatLiterals
aaron.ballman added inline comments. Comment at: include/clang/ASTMatchers/Dynamic/VariantValue.h:335 unsigned Unsigned; +double Double; bool Boolean; Lekensteyn wrote: > aaron.ballman wrote: > > Lekensteyn wrote: > > > aaron.ballman wrote: > > > > This may or may not be a good idea, but do we want to put the values > > > > into an APFloat rather than a double? My concern with double is that > > > > (0) it may be subtly different if the user wants a 16- or 32-bit float > > > > explicitly, (1) it won't be able to represent long double values, or > > > > quad double. > > > > > > > > I'm thinking this value could be passed directly from the C++ API as an > > > > APFloat, float, or double, or provided using a StringRef for the > > > > dynamic API. > > > (32-bit) double values are a superset of (16-bit) float values, that > > > should be OK. > > > Long doubles are possible in the AST (e.g. for `0.1L`), but neither C11 > > > nor C++14 seem to define a quad double literal type (so that should be of > > > a lesser concern). > > > > > > Reasons why I chose for double instead of APFloat: > > > - `strtod` is readily available and does not abort the program. By > > > contrast, `APFloat(StringRef)` trips on assertions if the input is > > > invalid. > > > - I was not sure if the APFloat class can be used in an union. > > The downside to using `strtod()` is that invalid input is silently > > accepted. However, assertions on invalid input is certainly not good > > either. It might be worth modifying `APFloat::convertFromString()` to > > accept invalid input and return an error. > > > > I think instead of an `APFloat`, maybe using an `APValue` for both the > > `Unsigned` and `Double` fields might work. At the very least, it should > > give you implementation ideas. > > > > There is a quad double literal suffix: `q`. It's only supported on some > > architectures, however. There are also imaginary numbers (`i`) and half > > (`h`). > The strtod conversion was based on parseDouble in > lib/Support/CommandLine.cpp, so any conversion issues also exist there. > > Same question, can APFloat/APValue be used in a union? > > float (or quad-double suffixes) are explicitly not supported now in this > matcher, maybe they can be added later but for now I decided to keep the > grammar simple (that is, do not express double/float data types via the > literal). > The strtod conversion was based on parseDouble in > lib/Support/CommandLine.cpp, so any conversion issues also exist there. Good to know. > Same question, can APFloat/APValue be used in a union? I believe so, but I've not tried it myself. Also, as I mentioned, `APValue` demonstrates another implementation strategy in case you cannot use a union directly. > float (or quad-double suffixes) are explicitly not supported now in this > matcher, maybe they can be added later but for now I decided to keep the > grammar simple (that is, do not express double/float data types via the > literal). That's reasonable for an initial implementation. https://reviews.llvm.org/D33135 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33406: PR28129 expand vector oparation to an IR constant.
RKSimon added a comment.
Test _mm256_cmp_pd as well?
Comment at: lib/CodeGen/CGBuiltin.cpp:7922
case X86::BI__builtin_ia32_cmpps256:
+ if (CC == 0xf) {
+ Value *Vec =
Builder.CreateVectorSplat(Ops[0]->getType()->getVectorNumElements(),
You need a comment here - explain what the constant represents and what the
transform does.
Comment at: test/CodeGen/avx-builtins.c:1434
+ // CHECK: store <8 x float> https://reviews.llvm.org/D33406
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33354: [clang-tidy] readability-braces-around-statements false positive with char literals
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG. Thank you for investigating this! https://reviews.llvm.org/D33354 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33354: [clang-tidy] readability-braces-around-statements false positive with char literals
alexfh added a comment. I'm going to commit the patch for you, but I guess, you might want to ask for commit access (http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access). https://reviews.llvm.org/D33354 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33285: clang-format: do not reflow bullet lists
krasimir accepted this revision. krasimir added a comment. This revision is now accepted and ready to land. Looks great! https://reviews.llvm.org/D33285 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303546 - [mips] Support `micromips` attribute
Author: atanasyan
Date: Mon May 22 07:47:43 2017
New Revision: 303546
URL: http://llvm.org/viewvc/llvm-project?rev=303546&view=rev
Log:
[mips] Support `micromips` attribute
This patch adds support for the `micromips` and `nomicromips` attributes
for MIPS targets.
Differential revision: https://reviews.llvm.org/D33363
Added:
cfe/trunk/test/CodeGen/micromips-attr.c
cfe/trunk/test/Sema/attr-micromips.c
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
Modified: cfe/trunk/include/clang/Basic/Attr.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=303546&r1=303545&r2=303546&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Mon May 22 07:47:43 2017
@@ -1179,6 +1179,12 @@ def MipsInterrupt : InheritableAttr, Tar
let Documentation = [MipsInterruptDocs];
}
+def MicroMips : InheritableAttr, TargetSpecificAttr {
+ let Spellings = [GCC<"micromips">];
+ let Subjects = SubjectList<[Function], ErrorDiag>;
+ let Documentation = [MicroMipsDocs];
+}
+
def Mode : Attr {
let Spellings = [GCC<"mode">];
let Subjects = SubjectList<[Var, Enum, TypedefName, Field], ErrorDiag,
@@ -1261,6 +1267,12 @@ def NoMips16 : InheritableAttr, TargetSp
let Documentation = [Undocumented];
}
+def NoMicroMips : InheritableAttr, TargetSpecificAttr {
+ let Spellings = [GCC<"nomicromips">];
+ let Subjects = SubjectList<[Function], ErrorDiag>;
+ let Documentation = [MicroMipsDocs];
+}
+
// This is not a TargetSpecificAttr so that is silently accepted and
// ignored on other targets as encouraged by the OpenCL spec.
//
Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=303546&r1=303545&r2=303546&view=diff
==
--- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
+++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon May 22 07:47:43 2017
@@ -1269,6 +1269,19 @@ The semantics are as follows:
}];
}
+def MicroMipsDocs : Documentation {
+ let Category = DocCatFunction;
+ let Content = [{
+Clang supports the GNU style ``__attribute__((micromips))`` and
+``__attribute__((nomicromips))`` attributes on MIPS targets. These attributes
+may be attached to a function definition and instructs the backend to generate
+or not to generate microMIPS code for that function.
+
+These attributes override the -mmicromips and -mno-micromips options
+on the command line.
+ }];
+}
+
def AVRInterruptDocs : Documentation {
let Category = DocCatFunction;
let Content = [{
Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=303546&r1=303545&r2=303546&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Mon May 22 07:47:43 2017
@@ -6557,6 +6557,11 @@ public:
Fn->addFnAttr("nomips16");
}
+if (FD->hasAttr())
+ Fn->addFnAttr("micromips");
+else if (FD->hasAttr())
+ Fn->addFnAttr("nomicromips");
+
const MipsInterruptAttr *Attr = FD->getAttr();
if (!Attr)
return;
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=303546&r1=303545&r2=303546&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Mon May 22 07:47:43 2017
@@ -5935,12 +5935,18 @@ static void ProcessDeclAttribute(Sema &S
handleDLLAttr(S, D, Attr);
break;
case AttributeList::AT_Mips16:
-handleSimpleAttributeWithExclusions(S, D,
- Attr);
+handleSimpleAttributeWithExclusions(S, D, Attr);
break;
case AttributeList::AT_NoMips16:
handleSimpleAttribute(S, D, Attr);
break;
+ case AttributeList::AT_MicroMips:
+handleSimpleAttributeWithExclusions(S, D, Attr);
+break;
+ case AttributeList::AT_NoMicroMips:
+handleSimpleAttribute(S, D, Attr);
+break;
case AttributeList::AT_AMDGPUFlatWorkGroupSize:
handleAMDGPUFlatWorkGroupSizeAttr(S, D, Attr);
break;
Added: cfe/trunk/test/CodeGen/micromips-attr.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/micromips-attr.c?rev=303546&view=auto
==
--- cfe/trunk/test/CodeGen/micromips-attr.c (added)
+++ cfe/trunk/test/CodeGen/micromips-attr.c Mo
[PATCH] D33363: [mips] Support `micromips` attribute
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303546: [mips] Support `micromips` attribute (authored by
atanasyan).
Changed prior to commit:
https://reviews.llvm.org/D33363?vs=99703&id=99748#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33363
Files:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/CodeGen/micromips-attr.c
cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
cfe/trunk/test/Sema/attr-micromips.c
Index: cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
+++ cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -2,7 +2,7 @@
// The number of supported attributes should never go down!
-// CHECK: #pragma clang attribute supports 60 attributes:
+// CHECK: #pragma clang attribute supports 62 attributes:
// CHECK-NEXT: AMDGPUFlatWorkGroupSize (SubjectMatchRule_function)
// CHECK-NEXT: AMDGPUNumSGPR (SubjectMatchRule_function)
// CHECK-NEXT: AMDGPUNumVGPR (SubjectMatchRule_function)
@@ -30,8 +30,10 @@
// CHECK-NEXT: IFunc (SubjectMatchRule_function)
// CHECK-NEXT: InternalLinkage (SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_record)
// CHECK-NEXT: LTOVisibilityPublic (SubjectMatchRule_record)
+// CHECK-NEXT: MicroMips (SubjectMatchRule_function)
// CHECK-NEXT: NoDebug (SubjectMatchRule_hasType_functionType, SubjectMatchRule_objc_method, SubjectMatchRule_variable_not_is_parameter)
// CHECK-NEXT: NoDuplicate (SubjectMatchRule_function)
+// CHECK-NEXT: NoMicroMips (SubjectMatchRule_function)
// CHECK-NEXT: NoSanitize (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
// CHECK-NEXT: NoSanitizeSpecific (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
// CHECK-NEXT: NoSplitStack (SubjectMatchRule_function)
Index: cfe/trunk/test/Sema/attr-micromips.c
===
--- cfe/trunk/test/Sema/attr-micromips.c
+++ cfe/trunk/test/Sema/attr-micromips.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple mips-linux-gnu -fsyntax-only -verify %s
+
+__attribute__((nomicromips(0))) void foo1(); // expected-error {{'nomicromips' attribute takes no arguments}}
+__attribute__((micromips(1))) void foo2();// expected-error {{'micromips' attribute takes no arguments}}
+
+__attribute((nomicromips)) int a; // expected-error {{attribute only applies to functions}}
+__attribute((micromips)) int b; // expected-error {{attribute only applies to functions}}
+
+__attribute__((micromips,mips16)) void foo5(); // expected-error {{'micromips' and 'mips16' attributes are not compatible}} \
+// expected-note {{conflicting attribute is here}}
+__attribute__((mips16,micromips)) void foo6(); // expected-error {{'mips16' and 'micromips' attributes are not compatible}} \
+// expected-note {{conflicting attribute is here}}
+
+__attribute((micromips)) void foo7();
+__attribute((nomicromips)) void foo8();
Index: cfe/trunk/test/CodeGen/micromips-attr.c
===
--- cfe/trunk/test/CodeGen/micromips-attr.c
+++ cfe/trunk/test/CodeGen/micromips-attr.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple mips-linux-gnu -emit-llvm -o - %s | FileCheck %s
+
+void __attribute__((micromips)) foo (void) {}
+
+// CHECK: define void @foo() [[MICROMIPS:#[0-9]+]]
+
+void __attribute__((nomicromips)) nofoo (void) {}
+
+// CHECK: define void @nofoo() [[NOMICROMIPS:#[0-9]+]]
+
+// CHECK: attributes [[MICROMIPS]] = { noinline nounwind {{.*}} "micromips" {{.*}} }
+// CHECK: attributes [[NOMICROMIPS]] = { noinline nounwind {{.*}} "nomicromips" {{.*}} }
Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -6557,6 +6557,11 @@
Fn->addFnAttr("nomips16");
}
+if (FD->hasAttr())
+ Fn->addFnAttr("micromips");
+else if (FD->hasAttr())
+ Fn->addFnAttr("nomicromips");
+
const MipsInterruptAttr *Attr = FD->getAttr();
if (!Attr)
return;
Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -5935,12 +5935,18 @@
handleDLLAttr(S, D, Attr);
break;
case AttributeList::AT_Mips16:
-handleSimpleAttributeWithExclusions(S, D,
- Attr);
+handleSimpleAttributeWithExclusions(S, D, Attr);
break;
[PATCH] D33364: [mips] Support micromips attribute passed by front-end
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303545: [mips] Support micromips attribute passed by
front-end (authored by atanasyan).
Changed prior to commit:
https://reviews.llvm.org/D33364?vs=99590&id=99747#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33364
Files:
llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp
llvm/trunk/test/CodeGen/Mips/micromips-attr.ll
Index: llvm/trunk/test/CodeGen/Mips/micromips-attr.ll
===
--- llvm/trunk/test/CodeGen/Mips/micromips-attr.ll
+++ llvm/trunk/test/CodeGen/Mips/micromips-attr.ll
@@ -0,0 +1,39 @@
+; RUN: llc -march=mips -mcpu=mips32 --mattr=-micromips < %s | FileCheck %s
+
+define void @foo() #0 {
+entry:
+ ret void
+}
+; CHECK:.setmicromips
+; CHECK-NEXT: .setnomips16
+; CHECK-NEXT: .entfoo
+; CHECK-NEXT: foo:
+
+define void @bar() #1 {
+entry:
+ ret void
+}
+; CHECK:.setnomicromips
+; CHECK-NEXT: .setnomips16
+; CHECK-NEXT: .entbar
+; CHECK-NEXT: bar:
+
+attributes #0 = {
+ nounwind "micromips"
+ "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false"
+ "less-precise-fpmad"="false" "no-frame-pointer-elim"="false"
+ "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false"
+ "no-signed-zeros-fp-math"="false" "no-trapping-math"="false"
+ "stack-protector-buffer-size"="8" "unsafe-fp-math"="false"
+ "use-soft-float"="false"
+}
+
+attributes #1 = {
+ nounwind
+ "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false"
+ "less-precise-fpmad"="false" "no-frame-pointer-elim"="false"
+ "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false"
+ "no-signed-zeros-fp-math"="false" "no-trapping-math"="false"
+ "stack-protector-buffer-size"="8" "unsafe-fp-math"="false"
+ "use-soft-float"="false"
+}
Index: llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp
===
--- llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp
+++ llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp
@@ -154,6 +154,11 @@
bool hasNoMips16Attr =
!F.getFnAttribute("nomips16").hasAttribute(Attribute::None);
+ bool HasMicroMipsAttr =
+ !F.getFnAttribute("micromips").hasAttribute(Attribute::None);
+ bool HasNoMicroMipsAttr =
+ !F.getFnAttribute("nomicromips").hasAttribute(Attribute::None);
+
// FIXME: This is related to the code below to reset the target options,
// we need to know whether or not the soft float flag is set on the
// function, so we can enable it as a subtarget feature.
@@ -165,6 +170,10 @@
FS += FS.empty() ? "+mips16" : ",+mips16";
else if (hasNoMips16Attr)
FS += FS.empty() ? "-mips16" : ",-mips16";
+ if (HasMicroMipsAttr)
+FS += FS.empty() ? "+micromips" : ",+micromips";
+ else if (HasNoMicroMipsAttr)
+FS += FS.empty() ? "-micromips" : ",-micromips";
if (softFloat)
FS += FS.empty() ? "+soft-float" : ",+soft-float";
Index: llvm/trunk/test/CodeGen/Mips/micromips-attr.ll
===
--- llvm/trunk/test/CodeGen/Mips/micromips-attr.ll
+++ llvm/trunk/test/CodeGen/Mips/micromips-attr.ll
@@ -0,0 +1,39 @@
+; RUN: llc -march=mips -mcpu=mips32 --mattr=-micromips < %s | FileCheck %s
+
+define void @foo() #0 {
+entry:
+ ret void
+}
+; CHECK:.setmicromips
+; CHECK-NEXT: .setnomips16
+; CHECK-NEXT: .entfoo
+; CHECK-NEXT: foo:
+
+define void @bar() #1 {
+entry:
+ ret void
+}
+; CHECK:.setnomicromips
+; CHECK-NEXT: .setnomips16
+; CHECK-NEXT: .entbar
+; CHECK-NEXT: bar:
+
+attributes #0 = {
+ nounwind "micromips"
+ "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false"
+ "less-precise-fpmad"="false" "no-frame-pointer-elim"="false"
+ "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false"
+ "no-signed-zeros-fp-math"="false" "no-trapping-math"="false"
+ "stack-protector-buffer-size"="8" "unsafe-fp-math"="false"
+ "use-soft-float"="false"
+}
+
+attributes #1 = {
+ nounwind
+ "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false"
+ "less-precise-fpmad"="false" "no-frame-pointer-elim"="false"
+ "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false"
+ "no-signed-zeros-fp-math"="false" "no-trapping-math"="false"
+ "stack-protector-buffer-size"="8" "unsafe-fp-math"="false"
+ "use-soft-float"="false"
+}
Index: llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp
===
--- llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp
+++ llvm/trunk/lib/Target/Mips/MipsTargetMachine.cpp
@@ -154,6 +154,11 @@
bool hasNoMips16Attr =
!F.getFnAttribute("nomips16").hasAttribute(Attribute::None);
+ bool HasMicroMipsAttr =
+ !F.getFnAttribute("micromips").hasAttribute(Attribute::
[clang-tools-extra] r303547 - [clangd] Switch to incomplete translation units
Author: krasimir Date: Mon May 22 07:49:08 2017 New Revision: 303547 URL: http://llvm.org/viewvc/llvm-project?rev=303547&view=rev Log: [clangd] Switch to incomplete translation units Summary: This speeds up code completion. All the cool kids (ycmd) are doing it. Reviewers: bkramer, ilya-biryukov Reviewed By: bkramer Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D33350 Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=303547&r1=303546&r2=303547&view=diff == --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Mon May 22 07:49:08 2017 @@ -45,7 +45,7 @@ ClangdUnit::ClangdUnit(PathRef FileName, ArgP, ArgP + ArgStrs.size(), PCHs, Diags, ResourceDir, /*OnlyLocalDecls=*/false, /*CaptureDiagnostics=*/true, RemappedSource, /*RemappedFilesKeepOriginalName=*/true, - /*PrecompilePreambleAfterNParses=*/1, /*TUKind=*/TU_Complete, + /*PrecompilePreambleAfterNParses=*/1, /*TUKind=*/TU_Prefix, /*CacheCodeCompletionResults=*/true, /*IncludeBriefCommentsInCodeCompletion=*/true, /*AllowPCHWithCompilerErrors=*/true)); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33350: [clangd] Switch to incomplete translation units
This revision was automatically updated to reflect the committed changes. Closed by commit rL303547: [clangd] Switch to incomplete translation units (authored by krasimir). Changed prior to commit: https://reviews.llvm.org/D33350?vs=99535&id=99749#toc Repository: rL LLVM https://reviews.llvm.org/D33350 Files: clang-tools-extra/trunk/clangd/ClangdUnit.cpp Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp === --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp @@ -45,7 +45,7 @@ ArgP, ArgP + ArgStrs.size(), PCHs, Diags, ResourceDir, /*OnlyLocalDecls=*/false, /*CaptureDiagnostics=*/true, RemappedSource, /*RemappedFilesKeepOriginalName=*/true, - /*PrecompilePreambleAfterNParses=*/1, /*TUKind=*/TU_Complete, + /*PrecompilePreambleAfterNParses=*/1, /*TUKind=*/TU_Prefix, /*CacheCodeCompletionResults=*/true, /*IncludeBriefCommentsInCodeCompletion=*/true, /*AllowPCHWithCompilerErrors=*/true)); Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp === --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp @@ -45,7 +45,7 @@ ArgP, ArgP + ArgStrs.size(), PCHs, Diags, ResourceDir, /*OnlyLocalDecls=*/false, /*CaptureDiagnostics=*/true, RemappedSource, /*RemappedFilesKeepOriginalName=*/true, - /*PrecompilePreambleAfterNParses=*/1, /*TUKind=*/TU_Complete, + /*PrecompilePreambleAfterNParses=*/1, /*TUKind=*/TU_Prefix, /*CacheCodeCompletionResults=*/true, /*IncludeBriefCommentsInCodeCompletion=*/true, /*AllowPCHWithCompilerErrors=*/true)); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32480: clang-format: Add CompactNamespaces option
Typz added a comment. Merging the 2 options is definitely a "safe" option, as it prevents ensures only the most obvious behavior is accessible. However, it has significant (IMO) drawbacks: - "Compact" is a not an namespace indentation type, this will make the option quite confusing - If indentation inside compact namespaces is needed, it cannot easily be added: we would need an extra mode NI_CompactWithIndent All in all, I think I prefer the current behavior of the patch (a separate CompactNamespace options, with consistent [if not useful] indentation settings); but it is your call, just let me know how to proceed. https://reviews.llvm.org/D32480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r303546 - [mips] Support `micromips` attribute
On Mon, May 22, 2017 at 8:47 AM, Simon Atanasyan via cfe-commits
wrote:
> Author: atanasyan
> Date: Mon May 22 07:47:43 2017
> New Revision: 303546
>
> URL: http://llvm.org/viewvc/llvm-project?rev=303546&view=rev
> Log:
> [mips] Support `micromips` attribute
>
> This patch adds support for the `micromips` and `nomicromips` attributes
> for MIPS targets.
>
> Differential revision: https://reviews.llvm.org/D33363
>
> Added:
> cfe/trunk/test/CodeGen/micromips-attr.c
> cfe/trunk/test/Sema/attr-micromips.c
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
> cfe/trunk/include/clang/Basic/AttrDocs.td
> cfe/trunk/lib/CodeGen/TargetInfo.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
>
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=303546&r1=303545&r2=303546&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Mon May 22 07:47:43 2017
> @@ -1179,6 +1179,12 @@ def MipsInterrupt : InheritableAttr, Tar
>let Documentation = [MipsInterruptDocs];
> }
>
> +def MicroMips : InheritableAttr, TargetSpecificAttr {
> + let Spellings = [GCC<"micromips">];
> + let Subjects = SubjectList<[Function], ErrorDiag>;
Why is this an error rather than a warning? Same question below.
> + let Documentation = [MicroMipsDocs];
> +}
> +
> def Mode : Attr {
>let Spellings = [GCC<"mode">];
>let Subjects = SubjectList<[Var, Enum, TypedefName, Field], ErrorDiag,
> @@ -1261,6 +1267,12 @@ def NoMips16 : InheritableAttr, TargetSp
>let Documentation = [Undocumented];
> }
>
> +def NoMicroMips : InheritableAttr, TargetSpecificAttr {
> + let Spellings = [GCC<"nomicromips">];
> + let Subjects = SubjectList<[Function], ErrorDiag>;
> + let Documentation = [MicroMipsDocs];
> +}
> +
> // This is not a TargetSpecificAttr so that is silently accepted and
> // ignored on other targets as encouraged by the OpenCL spec.
> //
>
> Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=303546&r1=303545&r2=303546&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
> +++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon May 22 07:47:43 2017
> @@ -1269,6 +1269,19 @@ The semantics are as follows:
>}];
> }
>
> +def MicroMipsDocs : Documentation {
> + let Category = DocCatFunction;
> + let Content = [{
> +Clang supports the GNU style ``__attribute__((micromips))`` and
> +``__attribute__((nomicromips))`` attributes on MIPS targets. These attributes
> +may be attached to a function definition and instructs the backend to
> generate
> +or not to generate microMIPS code for that function.
> +
> +These attributes override the -mmicromips and -mno-micromips options
Please quote the command line options with ``.
> +on the command line.
> + }];
> +}
> +
> def AVRInterruptDocs : Documentation {
>let Category = DocCatFunction;
>let Content = [{
>
> Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=303546&r1=303545&r2=303546&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Mon May 22 07:47:43 2017
> @@ -6557,6 +6557,11 @@ public:
>Fn->addFnAttr("nomips16");
> }
>
> +if (FD->hasAttr())
> + Fn->addFnAttr("micromips");
> +else if (FD->hasAttr())
> + Fn->addFnAttr("nomicromips");
> +
> const MipsInterruptAttr *Attr = FD->getAttr();
> if (!Attr)
>return;
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=303546&r1=303545&r2=303546&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Mon May 22 07:47:43 2017
> @@ -5935,12 +5935,18 @@ static void ProcessDeclAttribute(Sema &S
> handleDLLAttr(S, D, Attr);
> break;
>case AttributeList::AT_Mips16:
> -handleSimpleAttributeWithExclusions(S, D,
> - Attr);
> +handleSimpleAttributeWithExclusions +MipsInterruptAttr>(S, D, Attr);
> break;
>case AttributeList::AT_NoMips16:
> handleSimpleAttribute(S, D, Attr);
> break;
> + case AttributeList::AT_MicroMips:
> +handleSimpleAttributeWithExclusions(S, D,
> Attr);
> +break;
> + case AttributeList::AT_NoMicroMips:
> +han
[PATCH] D33304: [WIP][clang-tidy] Add a new module Android and a new check for file descriptors.
alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.
Comment at: clang-tidy/android/FileDescriptorCheck.cpp:25
+ callExpr(
+ callee(functionDecl(allOf(
+ isExpansionInFileMatching(HEADER_FILE), returns(asString("int")),
`functionDecl()` is implicitly `allOf`.
Comment at: clang-tidy/android/FileDescriptorCheck.cpp:26
+ callee(functionDecl(allOf(
+ isExpansionInFileMatching(HEADER_FILE), returns(asString("int")),
+ hasParameter(1, hasType(isInteger())),
`isExpansionInFileMatching(...)` is a dependency on the implementation of the
library. The fact that `#include ` provides this macro doesn't mean
the macro is defined in this header (it could be in another transitively
included header). Same below.
Comment at: clang-tidy/android/FileDescriptorCheck.cpp:26
+ callee(functionDecl(allOf(
+ isExpansionInFileMatching(HEADER_FILE), returns(asString("int")),
+ hasParameter(1, hasType(isInteger())),
alexfh wrote:
> `isExpansionInFileMatching(...)` is a dependency on the implementation of the
> library. The fact that `#include ` provides this macro doesn't mean
> the macro is defined in this header (it could be in another transitively
> included header). Same below.
`asString("int")` is wasteful, since it needs to allocate the string and print
the return type name there. `hasType(isInteger())` is a much better alternative.
Comment at: clang-tidy/android/FileDescriptorCheck.cpp:28
+ hasParameter(1, hasType(isInteger())),
+ anyOf(matchesName("open"),
matchesName("open64".bind("funcDecl")))
+ .bind("openFn"),
`matchesName` is wasteful, if you need just to compare the name. You probably
want to use `hasAnyName("open", "open64")`.
Comment at: clang-tidy/android/FileDescriptorCheck.cpp:90
+return checkFlag(cast(Flags));
+ } else if (isa(Flags)) {
+if (cast(Flags)->getOpcode() ==
http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return
Comment at: test/clang-tidy/android-file-descriptor.cpp:3
+
+#include
+
We don't use actual library headers in the tests. Instead, please add mock
declarations of the API you need (open, open64, O_ flags, etc.).
Repository:
rL LLVM
https://reviews.llvm.org/D33304
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32449: Modifying PthreadLockChecker.cpp to reduce false positives.
malhar1995 updated this revision to Diff 99750.
malhar1995 added a comment.
Addressed previous comments (removed Schrodinger from lock state names, changed
method name setAppropriateLockState to resolvePossiblyDestroyedMutex, added an
assert in resolvePossiblyDestroyedMutex, formatted the code using clang-format
and added some test-cases to test/Analysis/pthreadlock.c)
Repository:
rL LLVM
https://reviews.llvm.org/D32449
Files:
.gitignore
lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
test/Analysis/pthreadlock.c
Index: test/Analysis/pthreadlock.c
===
--- test/Analysis/pthreadlock.c
+++ test/Analysis/pthreadlock.c
@@ -176,6 +176,49 @@
pthread_mutex_unlock(pmtx); // no-warning
}
+void
+ok23(void) {
+ if(pthread_mutex_destroy(&mtx1) != 0) // no-warning
+ pthread_mutex_destroy(&mtx1); // no-warning
+}
+
+void
+ok24(void) {
+ if(pthread_mutex_destroy(&mtx1) != 0) // no-warning
+ pthread_mutex_lock(&mtx1); // no-warning
+}
+
+void
+ok25(void) {
+ if(pthread_mutex_destroy(&mtx1) != 0) // no-warning
+ pthread_mutex_unlock(&mtx1); // no-warning
+}
+
+void
+ok26(void) {
+ pthread_mutex_unlock(&mtx1); // no-warning
+ if(pthread_mutex_destroy(&mtx1) != 0) // no-warning
+ pthread_mutex_lock(&mtx1); // no-warning
+}
+
+void
+ok27(void) {
+ pthread_mutex_unlock(&mtx1); // no-warning
+ if(pthread_mutex_destroy(&mtx1) != 0) // no-warning
+ pthread_mutex_lock(&mtx1); // no-warning
+ else
+ pthread_mutex_init(&mtx1, NULL); // no-warning
+}
+
+void
+ok28() {
+ if(pthread_mutex_destroy(&mtx1)!=0) { // no-warning
+ pthread_mutex_lock(&mtx1); // no-warning
+ pthread_mutex_unlock(&mtx1); // no-warning
+ pthread_mutex_destroy(&mtx1); // no-warning
+ }
+}
+
void
bad1(void)
@@ -392,3 +435,56 @@
pthread_mutex_unlock(&mtx1); // no-warning
pthread_mutex_init(&mtx1, NULL); // expected-warning{{This lock has already been initialized}}
}
+
+void
+bad27(void)
+{
+ pthread_mutex_unlock(&mtx1); // no-warning
+ int ret = pthread_mutex_destroy(&mtx1); // no-warning
+ if(ret != 0) // no-warning
+ pthread_mutex_lock(&mtx1); // no-warning
+ else
+ pthread_mutex_unlock(&mtx1); // expected-warning{{This lock has already been destroyed}}
+}
+
+void
+bad28(void)
+{
+ pthread_mutex_unlock(&mtx1); // no-warning
+ int ret = pthread_mutex_destroy(&mtx1); // no-warning
+ if(ret != 0) // no-warning
+ pthread_mutex_lock(&mtx1); // no-warning
+ else
+ pthread_mutex_lock(&mtx1); // expected-warning{{This lock has already been destroyed}}
+}
+
+void
+bad29()
+{
+ pthread_mutex_lock(&mtx1); // no-warning
+ pthread_mutex_unlock(&mtx1); // no-warning
+ if(pthread_mutex_destroy(&mtx1) != 0) // no-warning
+ pthread_mutex_init(&mtx1, NULL); // expected-warning{{This lock has already been initialized}}
+ else
+ pthread_mutex_init(&mtx1, NULL); // no-warning
+}
+
+void
+bad30()
+{
+ pthread_mutex_lock(&mtx1); // no-warning
+ pthread_mutex_unlock(&mtx1); // no-warning
+ if(pthread_mutex_destroy(&mtx1) != 0) // no-warning
+ pthread_mutex_init(&mtx1, NULL); // expected-warning{{This lock has already been initialized}}
+ else
+ pthread_mutex_destroy(&mtx1); // expected-warning{{This lock has already been destroyed}}
+}
+
+void
+bad31()
+{
+ int ret = pthread_mutex_destroy(&mtx1); // no-warning
+ pthread_mutex_lock(&mtx1); // expected-warning{{This lock has already been destroyed}}
+ if(ret != 0)
+ pthread_mutex_lock(&mtx1);
+}
Index: lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -25,58 +25,79 @@
namespace {
struct LockState {
- enum Kind { Destroyed, Locked, Unlocked } K;
+ enum Kind {
+Destroyed,
+Locked,
+Unlocked,
+UntouchedAndPossiblyDestroyed,
+UnlockedAndPossiblyDestroyed
+ } K;
private:
LockState(Kind K) : K(K) {}
public:
static LockState getLocked() { return LockState(Locked); }
static LockState getUnlocked() { return LockState(Unlocked); }
static LockState getDestroyed() { return LockState(Destroyed); }
-
- bool operator==(const LockState &X) const {
-return K == X.K;
+ static LockState getUntouchedAndPossiblyDestroyed() {
+return LockState(UntouchedAndPossiblyDestroyed);
+ }
+ static LockState getUnlockedAndPossiblyDestroyed() {
+return LockState(UnlockedAndPossiblyDestroyed);
}
+ bool operator==(const LockState &X) const { return K == X.K; }
+
bool isLocked() const { return K == Locked; }
bool isUnlocked() const { return K == Unlocked; }
bool isDestroyed() const { return K == Destroyed; }
-
- void Profile(llvm::FoldingSetNodeID &ID) const {
-ID.AddInteger(K);
+ bool isUntouchedAndPossiblyDestroyed() const {
+return K == UntouchedAndPossiblyDestroyed;
}
+ bool isUnlockedAndPossiblyDestroyed() const {
+return K == UnlockedAndPossiblyDestroyed;
+ }
+
+ void Profile(llvm
[PATCH] D33272: Method loadFromCommandLine should be able to report errors
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
Thank you! This looks good with one nit (see the inline comment).
Comment at: lib/Tooling/CompilationDatabase.cpp:292
+ if (Argc == 0) {
+ErrorMsg = "error: no arguments specified\n";
+return nullptr;
The lack of the arguments (or the `--` below) shouldn't be treated as an error
(at least an error that is worth showing to the user). The caller will just
move on to trying the next kind of a compilation database. The only actual
errors we can get here may be coming from `stripPositionalArgs`.
The caller should be modified accordingly (i.e. not output anything when both
the return value is `nullptr` and `ErrorMsg` is empty).
https://reviews.llvm.org/D33272
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33272: Method loadFromCommandLine should be able to report errors
alexfh added inline comments. Comment at: lib/Tooling/CompilationDatabase.cpp:164 +// to user. +} else if (DiagLevel >= DiagnosticsEngine::Error) + Other.HandleDiagnostic(DiagLevel, Info); Another couple of nits: 1. the comment below would probably be better moved to the body of the second `if` 2. enclose the next statement in braces. https://reviews.llvm.org/D33272 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32346: [clang-tidy] New readability check for strlen argument
alexfh added inline comments.
Comment at: clang-tidy/readability/StrlenArgumentCheck.cpp:23-24
+ Finder->addMatcher(
+ callExpr(anyOf(callee(functionDecl(hasName("::strlen"))),
+ callee(functionDecl(hasName("std::strlen",
+ hasAnyArgument(ignoringParenImpCasts(
`anyOf(callee(functionDecl(hasName(x))), callee(functionDecl(hasName(y`
unnecessarily duplicates `callee` and `functionDecl`. A better option is
`callee(functionDecl(hasAnyName(x, y)))`.
Comment at: docs/clang-tidy/checks/readability-strlen-argument.rst:8
+
+In the example code below the developer probably wanted to make room for an
extra char in the allocation but misplaced the addition.
+
JonasToth wrote:
> danielmarjamaki wrote:
> > JonasToth wrote:
> > > when the intend was to allocate one more char, he would need to do
> > > `strlen(s) + 1`, why is it changed to subtraction then?
> > If I change it to strlen(s) + 1 then the logic of the program is changed.
> >
> > If I change it to subtraction then the logic is the same and the program is
> > still wrong, but imho it is easier to see.
> it should be made clear in the docs, that the code is bad, especially if its
> UB. the dangers of copy & paste ;) (even its unlikely that this will be
> copy&pasted).
If we have good reasons to think the original code is wrong, so it seems that
it's better to fix it, than to retain the behavior and just hide the bug?
Repository:
rL LLVM
https://reviews.llvm.org/D32346
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33406: PR28129 expand vector oparation to an IR constant.
spatel added inline comments.
Comment at: lib/CodeGen/CGBuiltin.cpp:7932
break;
case X86::BI__builtin_ia32_cmppd256:
ID = Intrinsic::x86_avx_cmp_pd_256;
1. Should we handle the 'pd256' version the same way?
2. How about the 0xb ('false') constant? It should produce a zero here?
3. Can or should we deal with the signalling versions (0x1b, 0x1f) too?
https://reviews.llvm.org/D33406
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32346: [clang-tidy] New readability check for strlen argument
alexfh requested changes to this revision. alexfh added inline comments. This revision now requires changes to proceed. Comment at: docs/clang-tidy/checks/readability-strlen-argument.rst:20 +char *p = new char[(strlen(s) - 1)] +strcpy(p, s); + JonasToth wrote: > danielmarjamaki wrote: > > JonasToth wrote: > > > isnt that an overflow? > > > an example: > > > `strlen(s) == 10` -> `p` will be 9 characters long, since its substracted > > > with `1`. > > > > > > the copy operation will then copy the content of `s` into `p`, therefore > > > copying 10 characters into a buffer of length 9. > > > > > > as i understand it `strcpy(p, s + 1)` would be correct with the sizes. > > yes it is overflow. My intention was to show that strlen(s+1) syntax is > > dangerous. > ok. please state that the overflow in a comment, its better to make that > explicit. BTW, `strlen(x) - N` is not only prone to overflows, but also less efficient (in case it's intentional). Did you run the check on real projects to see how likely this pattern is a bug? Repository: rL LLVM https://reviews.llvm.org/D32346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32425: [mips] Make checks in CodeGen/mips-varargs.c less fragile
sdardis accepted this revision. sdardis added a comment. This revision is now accepted and ready to land. LGTM,, https://reviews.llvm.org/D32425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33250: [Sema][ObjC] Fix a bug where -Wunguarded-availability was emitted at the wrong location
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.
LGTM with one change below:
Comment at: lib/Sema/SemaDeclAttr.cpp:7256
+ bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *PRE) {
+if (ObjCInterfaceDecl *ID = PRE->getClassReceiver())
+ DiagnoseDeclAvailability(ID, PRE->getReceiverLocation());
`getClassReceiver` calls `get` in a PointerUnion, which will trigger an
assertion when the PRE is not a class receiver. Please rewrite this `if` to
something like:
```
if (PRE->isClassReceiver())
DiagnoseDeclAvailability(PRE->getClassReceiver(),
PRE->getReceiverLocation());
```
https://reviews.llvm.org/D33250
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r303551 - [clang-tidy] readability-braces-around-statements false positive with char literals
Author: alexfh
Date: Mon May 22 08:58:16 2017
New Revision: 303551
URL: http://llvm.org/viewvc/llvm-project?rev=303551&view=rev
Log:
[clang-tidy] readability-braces-around-statements false positive with char
literals
Summary:
Single-line if statements cause a false positive when the last token in the
conditional statement is a char constant:
```
if (condition)
return 'a';
```
For some reason `findEndLocation` seems to skips too many (vertical)
whitespaces in this case. The same problem already occured with string literals
(https://reviews.llvm.org/D25558), and was fixed by adding a special check for
this very case. I just extended the condition to also include char constants.
No idea what really causes the issue though.
Reviewers: alexfh
Reviewed By: alexfh
Subscribers: xazax.hun, cfe-commits
Patch by Florian Gross!
Differential Revision: https://reviews.llvm.org/D33354
Modified:
clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp?rev=303551&r1=303550&r2=303551&view=diff
==
---
clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
(original)
+++
clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
Mon May 22 08:58:16 2017
@@ -54,14 +54,15 @@ SourceLocation forwardSkipWhitespaceAndC
SourceLocation findEndLocation(SourceLocation LastTokenLoc,
const SourceManager &SM,
const ASTContext *Context) {
- SourceLocation Loc = LastTokenLoc;
+ SourceLocation Loc =
+ Lexer::GetBeginningOfToken(LastTokenLoc, SM, Context->getLangOpts());
// Loc points to the beginning of the last (non-comment non-ws) token
// before end or ';'.
assert(Loc.isValid());
bool SkipEndWhitespaceAndComments = true;
tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
if (TokKind == tok::NUM_TOKENS || TokKind == tok::semi ||
- TokKind == tok::r_brace || isStringLiteral(TokKind)) {
+ TokKind == tok::r_brace) {
// If we are at ";" or "}", we found the last token. We could use as well
// `if (isa(S))`, but it wouldn't work for nested statements.
SkipEndWhitespaceAndComments = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r303552 - [clang-tidy] readability-redundant-declaration false positive for defaulted function
Author: alexfh
Date: Mon May 22 08:58:57 2017
New Revision: 303552
URL: http://llvm.org/viewvc/llvm-project?rev=303552&view=rev
Log:
[clang-tidy] readability-redundant-declaration false positive for defaulted
function
Summary:
```
template
struct C {
C();
};
template
C::C() = default;
```
Causes a readability-redundant-declaration diagnostic. This is caused by
`isDefinition` not matching defaulted functions.
Reviewers: alexfh, danielmarjamaki
Reviewed By: alexfh
Subscribers: xazax.hun, cfe-commits
Patch by Florian Gross!
Differential Revision: https://reviews.llvm.org/D33358
Modified:
clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp?rev=303552&r1=303551&r2=303552&view=diff
==
---
clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
(original)
+++
clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
Mon May 22 08:58:57 2017
@@ -19,11 +19,12 @@ namespace tidy {
namespace readability {
void RedundantDeclarationCheck::registerMatchers(MatchFinder *Finder) {
- auto UnlessDefinition = unless(isDefinition());
- Finder->addMatcher(namedDecl(anyOf(varDecl(UnlessDefinition),
- functionDecl(UnlessDefinition)))
- .bind("Decl"),
- this);
+ Finder->addMatcher(
+ namedDecl(
+ anyOf(varDecl(unless(isDefinition())),
+functionDecl(unless(anyOf(isDefinition(), isDefaulted())
+ .bind("Decl"),
+ this);
}
void RedundantDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
Modified:
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp?rev=303552&r1=303551&r2=303552&view=diff
==
---
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
(original)
+++
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
Mon May 22 08:58:57 2017
@@ -34,3 +34,11 @@ struct C {
static int I;
};
int C::I;
+
+template
+struct C2 {
+ C2();
+};
+
+template
+C2::C2() = default;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33354: [clang-tidy] readability-braces-around-statements false positive with char literals
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303551: [clang-tidy] readability-braces-around-statements
false positive with char… (authored by alexfh).
Changed prior to commit:
https://reviews.llvm.org/D33354?vs=99576&id=99756#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33354
Files:
clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
Index:
clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
===
---
clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++
clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -54,14 +54,15 @@
SourceLocation findEndLocation(SourceLocation LastTokenLoc,
const SourceManager &SM,
const ASTContext *Context) {
- SourceLocation Loc = LastTokenLoc;
+ SourceLocation Loc =
+ Lexer::GetBeginningOfToken(LastTokenLoc, SM, Context->getLangOpts());
// Loc points to the beginning of the last (non-comment non-ws) token
// before end or ';'.
assert(Loc.isValid());
bool SkipEndWhitespaceAndComments = true;
tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
if (TokKind == tok::NUM_TOKENS || TokKind == tok::semi ||
- TokKind == tok::r_brace || isStringLiteral(TokKind)) {
+ TokKind == tok::r_brace) {
// If we are at ";" or "}", we found the last token. We could use as well
// `if (isa(S))`, but it wouldn't work for nested statements.
SkipEndWhitespaceAndComments = false;
Index: clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -54,14 +54,15 @@
SourceLocation findEndLocation(SourceLocation LastTokenLoc,
const SourceManager &SM,
const ASTContext *Context) {
- SourceLocation Loc = LastTokenLoc;
+ SourceLocation Loc =
+ Lexer::GetBeginningOfToken(LastTokenLoc, SM, Context->getLangOpts());
// Loc points to the beginning of the last (non-comment non-ws) token
// before end or ';'.
assert(Loc.isValid());
bool SkipEndWhitespaceAndComments = true;
tok::TokenKind TokKind = getTokenKind(Loc, SM, Context);
if (TokKind == tok::NUM_TOKENS || TokKind == tok::semi ||
- TokKind == tok::r_brace || isStringLiteral(TokKind)) {
+ TokKind == tok::r_brace) {
// If we are at ";" or "}", we found the last token. We could use as well
// `if (isa(S))`, but it wouldn't work for nested statements.
SkipEndWhitespaceAndComments = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33358: [clang-tidy] readability-redundant-declaration false positive for defaulted function
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303552: [clang-tidy] readability-redundant-declaration false
positive for defaulted… (authored by alexfh).
Changed prior to commit:
https://reviews.llvm.org/D33358?vs=99570&id=99757#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33358
Files:
clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
Index:
clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
@@ -19,11 +19,12 @@
namespace readability {
void RedundantDeclarationCheck::registerMatchers(MatchFinder *Finder) {
- auto UnlessDefinition = unless(isDefinition());
- Finder->addMatcher(namedDecl(anyOf(varDecl(UnlessDefinition),
- functionDecl(UnlessDefinition)))
- .bind("Decl"),
- this);
+ Finder->addMatcher(
+ namedDecl(
+ anyOf(varDecl(unless(isDefinition())),
+functionDecl(unless(anyOf(isDefinition(), isDefaulted())
+ .bind("Decl"),
+ this);
}
void RedundantDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
Index:
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
===
---
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
+++
clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
@@ -34,3 +34,11 @@
static int I;
};
int C::I;
+
+template
+struct C2 {
+ C2();
+};
+
+template
+C2::C2() = default;
Index: clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/readability/RedundantDeclarationCheck.cpp
@@ -19,11 +19,12 @@
namespace readability {
void RedundantDeclarationCheck::registerMatchers(MatchFinder *Finder) {
- auto UnlessDefinition = unless(isDefinition());
- Finder->addMatcher(namedDecl(anyOf(varDecl(UnlessDefinition),
- functionDecl(UnlessDefinition)))
- .bind("Decl"),
- this);
+ Finder->addMatcher(
+ namedDecl(
+ anyOf(varDecl(unless(isDefinition())),
+functionDecl(unless(anyOf(isDefinition(), isDefaulted())
+ .bind("Decl"),
+ this);
}
void RedundantDeclarationCheck::check(const MatchFinder::MatchResult &Result) {
Index: clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/readability-redundant-declaration.cpp
@@ -34,3 +34,11 @@
static int I;
};
int C::I;
+
+template
+struct C2 {
+ C2();
+};
+
+template
+C2::C2() = default;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31745: [OpenCL] Added diagnostic for implicit declaration of function in OpenCL
echuraev added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:12449
// function declaration is going to be treated as an error.
- if (Diags.getDiagnosticLevel(diag_id, Loc) >= DiagnosticsEngine::Error) {
+ if (!getLangOpts().OpenCL &&
+ Diags.getDiagnosticLevel(diag_id, Loc) >= DiagnosticsEngine::Error) {
Anastasia wrote:
> This way prevents the typo corrections completely for OpenCL which is not
> very desirable. I was just wondering could we prevent adding the invalid
> builtin function identifiers instead to the correction candidate list.
>
> Like when `work_group_reserve_read_pipe` was first parsed it shouldn't have
> been added to the list of valid function identifiers to appear in the
> corrections of 'work_group_reserve_write_pipe'. I am guessing the identifier
> might be added when builtins are initialized...
Yes, sorry, I didn't think about it. I investigated the question how can I
remove invalid functions from correction candidate list. But I have a problem
and I hope that you will be able to help me.
I found that the correction is added in function `void
TypoCorrectionConsumer::addName` (in file //SemaLookup.cpp//) which called from
loop in function `std::unique_ptr
Sema::makeTypoCorrectionConsumer`. The loop you can see below:
```
// For unqualified lookup, look through all of the names that we have
// seen in this translation unit.
// FIXME: Re-add the ability to skip very unlikely potential corrections.
for (const auto &I : Context.Idents)
Consumer->FoundName(I.getKey());
```
But the map `Context.Idents` already contains names of implicit functions. So,
I found that names of functions were added to this dictionary during parsing
AST. After calling `ConsumeToken()` function in `void
Parser::ParseDeclarationSpecifiers` (in file //ParseDecl.cpp//) implicit
functions were added to the dictionary. But in this function I'm not able to
check is the OpenCL function implicit declared or not.
As a result I tried to remove names of implicit functions before calling
`CorrectTypo`. But unfortunately, we don't have an API for removing items from
`Context.Idents`. So, I don't know the good way for fixing these diagnostic
messages. Could you help me please? Do you have any suggestions?
Thank you in advance!
https://reviews.llvm.org/D31745
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31414: [NFC, Refactor] Modernize TemplateIdAnnotation using TrailingObjects
faisalv added a comment. *ping* https://reviews.llvm.org/D31414 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31414: [NFC, Refactor] Modernize TemplateIdAnnotation using TrailingObjects
aaron.ballman added inline comments.
Comment at: include/clang/Sema/ParsedTemplate.h:202
+ SmallVectorImpl &CleanupList) {
+ ;
+ TemplateIdAnnotation *TemplateId = new (std::malloc(
Spurious semi colon?
Comment at: include/clang/Sema/ParsedTemplate.h:215
+[](ParsedTemplateArgument &a) {
+ return a.~ParsedTemplateArgument();
+});
Why is this returning the result of an explicit destructor call?
Also `a` should be named `A` per usual naming conventions.
https://reviews.llvm.org/D31414
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r303554 - [clang-tidy] misc-move-const-arg shouldn't complain on std::move(lambda)
Author: alexfh
Date: Mon May 22 09:30:14 2017
New Revision: 303554
URL: http://llvm.org/viewvc/llvm-project?rev=303554&view=rev
Log:
[clang-tidy] misc-move-const-arg shouldn't complain on std::move(lambda)
Modified:
clang-tools-extra/trunk/clang-tidy/misc/MoveConstantArgumentCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-move-const-arg.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/MoveConstantArgumentCheck.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MoveConstantArgumentCheck.cpp?rev=303554&r1=303553&r2=303554&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/MoveConstantArgumentCheck.cpp
(original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MoveConstantArgumentCheck.cpp Mon
May 22 09:30:14 2017
@@ -74,6 +74,12 @@ void MoveConstantArgumentCheck::check(co
if (IsConstArg || IsTriviallyCopyable) {
if (const CXXRecordDecl *R = Arg->getType()->getAsCXXRecordDecl()) {
+ // According to [expr.prim.lambda]p3, "whether the closure type is
+ // trivially copyable" property can be changed by the implementation of
+ // the language, so we shouldn't rely on it when issuing diagnostics.
+ if (R->isLambda())
+return;
+ // Don't warn when the type is not copyable.
for (const auto *Ctor : R->ctors()) {
if (Ctor->isCopyConstructor() && Ctor->isDeleted())
return;
Modified: clang-tools-extra/trunk/test/clang-tidy/misc-move-const-arg.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-move-const-arg.cpp?rev=303554&r1=303553&r2=303554&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/misc-move-const-arg.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-move-const-arg.cpp Mon May 22
09:30:14 2017
@@ -157,6 +157,9 @@ void moveToConstReferenceNegatives() {
// No warning inside of macro expansion, even if the macro expansion is
inside
// a lambda that is, in turn, an argument to a macro.
CALL([no_move_semantics] { M3(NoMoveSemantics, no_move_semantics); });
+
+ auto lambda = [] {};
+ auto lambda2 = std::move(lambda);
}
class MoveOnly {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303555 - [index] 'using namespace' declarations in functions should record
Author: arphaman
Date: Mon May 22 09:39:39 2017
New Revision: 303555
URL: http://llvm.org/viewvc/llvm-project?rev=303555&view=rev
Log:
[index] 'using namespace' declarations in functions should record
the reference to the namespace
rdar://32323190
Modified:
cfe/trunk/lib/Index/IndexDecl.cpp
cfe/trunk/lib/Index/IndexSymbol.cpp
cfe/trunk/test/Index/Core/index-source.cpp
Modified: cfe/trunk/lib/Index/IndexDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=303555&r1=303554&r2=303555&view=diff
==
--- cfe/trunk/lib/Index/IndexDecl.cpp (original)
+++ cfe/trunk/lib/Index/IndexDecl.cpp Mon May 22 09:39:39 2017
@@ -568,8 +568,12 @@ public:
const DeclContext *DC = D->getDeclContext()->getRedeclContext();
const NamedDecl *Parent = dyn_cast(DC);
-IndexCtx.indexNestedNameSpecifierLoc(D->getQualifierLoc(), Parent,
- D->getLexicalDeclContext());
+// NNS for the local 'using namespace' directives is visited by the body
+// visitor.
+if (!D->getParentFunctionOrMethod())
+ IndexCtx.indexNestedNameSpecifierLoc(D->getQualifierLoc(), Parent,
+ D->getLexicalDeclContext());
+
return IndexCtx.handleReference(D->getNominatedNamespaceAsWritten(),
D->getLocation(), Parent,
D->getLexicalDeclContext(),
Modified: cfe/trunk/lib/Index/IndexSymbol.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexSymbol.cpp?rev=303555&r1=303554&r2=303555&view=diff
==
--- cfe/trunk/lib/Index/IndexSymbol.cpp (original)
+++ cfe/trunk/lib/Index/IndexSymbol.cpp Mon May 22 09:39:39 2017
@@ -61,6 +61,8 @@ bool index::isFunctionLocalSymbol(const
if (isa(D))
return true;
+ if (isa(D))
+return false;
if (!D->getParentFunctionOrMethod())
return false;
Modified: cfe/trunk/test/Index/Core/index-source.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.cpp?rev=303555&r1=303554&r2=303555&view=diff
==
--- cfe/trunk/test/Index/Core/index-source.cpp (original)
+++ cfe/trunk/test/Index/Core/index-source.cpp Mon May 22 09:39:39 2017
@@ -339,3 +339,14 @@ void ::ns::inner::func() {
// CHECK: [[@LINE-1]]:3 | namespace/C++ | ns | c:@N@ns | |
Ref,RelCont | rel: 1
// CHECK: [[@LINE-2]]:7 | namespace-alias/C++ | innerAlias |
c:@N@ns@NA@innerAlias | | Ref,RelCont | rel: 1
}
+
+void innerUsingNamespace() {
+ using namespace ns;
+// CHECK: [[@LINE-1]]:19 | namespace/C++ | ns | c:@N@ns | |
Ref,RelCont | rel: 1
+ {
+using namespace ns::innerAlias;
+// CHECK: [[@LINE-1]]:25 | namespace-alias/C++ | innerAlias |
c:@N@ns@NA@innerAlias | | Ref,RelCont | rel: 1
+// CHECK: [[@LINE-2]]:21 | namespace/C++ | ns | c:@N@ns | |
Ref,RelCont | rel: 1
+// CHECK-NOT: [[@LINE-3]]:21
+ }
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32480: clang-format: Add CompactNamespaces option
Typz updated this revision to Diff 99763.
Typz added a comment.
Remove dependency on https://reviews.llvm.org/D33314
https://reviews.llvm.org/D32480
Files:
include/clang/Format/Format.h
lib/Format/Format.cpp
lib/Format/NamespaceEndCommentsFixer.cpp
lib/Format/UnwrappedLineFormatter.cpp
unittests/Format/FormatTest.cpp
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1309,6 +1309,140 @@
Style));
}
+TEST_F(FormatTest, FormatsCompactNamespaces) {
+ FormatStyle Style = getLLVMStyle();
+ Style.CompactNamespaces = true;
+
+ verifyFormat("namespace A { namespace B {\n"
+ "}} // namespace A::B",
+ Style);
+
+ EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+ "namespace in {\n"
+ "} // namespace in\n"
+ "} // namespace out",
+ Style));
+
+ EXPECT_EQ("namespace out { namespace in1 {\n"
+"} // namespace in1\n"
+"namespace in2 {\n"
+"}} // namespace out::in2",
+format("namespace out {\n"
+ "namespace in1 {\n"
+ "} // namespace in1\n"
+ "namespace in2 {\n"
+ "} // namespace in2\n"
+ "} // namespace out",
+ Style));
+
+ EXPECT_EQ("namespace out {\n"
+"int i;\n"
+"namespace in {\n"
+"int j;\n"
+"} // namespace in\n"
+"int k;\n"
+"} // namespace out",
+format("namespace out { int i;\n"
+ "namespace in { int j; } // namespace in\n"
+ "int k; } // namespace out",
+ Style));
+
+ EXPECT_EQ("namespace A { namespace B { namespace C {\n"
+"}}} // namespace A::B::C\n",
+format("namespace A { namespace B {\n"
+ "namespace C {\n"
+ "}} // namespace B::C\n"
+ "} // namespace A\n",
+ Style));
+
+ EXPECT_EQ("namespace {\n"
+ "namespace {\n"
+"}} // namespace ::",
+format("namespace {\n"
+ "namespace {\n"
+ "} // namespace \n"
+ "} // namespace ",
+ Style));
+
+ EXPECT_EQ("namespace a { namespace b {\n"
+ "namespace c {\n"
+"}}} // namespace a::b::c",
+format("namespace a {\n"
+ "namespace b {\n"
+ "namespace c {\n"
+ "} // namespace c\n"
+ "} // namespace b\n"
+ "} // namespace a",
+ Style));
+
+ // Missing comments are added
+ EXPECT_EQ("namespace out { namespace in {\n"
+ "int i;\n"
+ "int j;\n"
+"}} // namespace out::in",
+format("namespace out {\n"
+ "namespace in {\n"
+ "int i;\n"
+ "int j;\n"
+ "}\n"
+ "}",
+ Style));
+
+ // Incorrect comments are fixed
+ EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out { namespace in {\n"
+ "}} // namespace out",
+ Style));
+ EXPECT_EQ("namespace out { namespace in {\n"
+"}} // namespace out::in",
+format("namespace out { namespace in {\n"
+ "}} // namespace in",
+ Style));
+
+ // Extra semicolon after 'inner' closing brace prevents merging
+ EXPECT_EQ("namespace out { namespace in {\n"
+"}; } // namespace out::in",
+format("namespace out {\n"
+ "namespace in {\n"
+ "}; // namespace in\n"
+ "} // namespace out",
+ Style));
+
+ // Extra semicolon after 'outer' closing brace is conserved
+ EXPECT_EQ("namespace out { namespace in {\n"
+"}}; // namespace out::in",
+format("namespace out {\n"
+ "namespace in {\n"
+ "} // namespace in\n"
+ "}; // namespace out",
+ Style));
+
+ Style.NamespaceIndentation = FormatStyle::NI_All;
+ EXPECT_EQ("namespace out { namespace in {\n"
+"int i;\n"
+"}} // namespace out::in",
+format("na
[PATCH] D32896: [OpenCL] Make CLK_NULL_RESERVE_ID invalid reserve id.
yaxunl added inline comments. Comment at: lib/Headers/opencl-c.h:16020 +// The macro CLK_NULL_RESERVE_ID refers to an invalid reservation ID. +#define CLK_NULL_RESERVE_ID (__builtin_astype((void *)0, reserve_id_t)) bool __ovld is_valid_reserve_id(reserve_id_t reserve_id); bader wrote: > yaxunl wrote: > > yaxunl wrote: > > > bader wrote: > > > > yaxunl wrote: > > > > > Anastasia wrote: > > > > > > echuraev wrote: > > > > > > > yaxunl wrote: > > > > > > > > Anastasia wrote: > > > > > > > > > yaxunl wrote: > > > > > > > > > > Anastasia wrote: > > > > > > > > > > > Looks good from my side. > > > > > > > > > > > > > > > > > > > > > > @yaxunl , since you originally committed this. Could you > > > > > > > > > > > please verify that changing from `SIZE_MAX` to `0` would > > > > > > > > > > > be fine. > > > > > > > > > > > > > > > > > > > > > > Btw, we have a similar definition for `CLK_NULL_EVENT`. > > > > > > > > > > `__PIPE_RESERVE_ID_VALID_BIT` is implementation detail and > > > > > > > > > > not part of the spec. I would suggest to remove it from > > > > > > > > > > this header file. > > > > > > > > > > > > > > > > > > > > The spec only requires CLK_NULL_RESERVE_ID to be defined > > > > > > > > > > but does not define its value. Naturally a valid id starts > > > > > > > > > > from 0 and increases. I don't see significant advantage to > > > > > > > > > > change CLK_NULL_RESERVE_ID from __SIZE_MAX to 0. > > > > > > > > > > > > > > > > > > > > Is there any reason that this change is needed? > > > > > > > > > I don't see issues to commit things outside of spec as soon > > > > > > > > > as they prefixed properly with "__". But I agree it would be > > > > > > > > > nice to see if it's any useful and what the motivation is for > > > > > > > > > having different implementation. > > > > > > > > For `__PIPE_RESERVE_ID_VALID_BIT`, it assumes that the > > > > > > > > implementation uses one specific bit of a reserve id to > > > > > > > > indicate that the reserve id is valid. Not all implementations > > > > > > > > assume that. Actually I am curious why that is needed too. > > > > > > > About `CLK_NULL_RESERVE_ID`: we check that reserve id is valid if > > > > > > > significant bit equal to one. `CLK_NULL_RESERVE_ID refers to an > > > > > > > invalid reservation, so if `CLK_NULL_RESERVE_ID equal to 0, we > > > > > > > can be sure that significant bit doesn't equal to 1 and it is > > > > > > > invalid reserve id. Also it is more obviously if > > > > > > > CLK_**NULL**_RESERVE_ID is equal to 0. > > > > > > > > > > > > > > What about `__PIPE_RESERVE_ID_VALID_BIT`: As I understand > > > > > > > previous implementation also assumes that one specific bit was of > > > > > > > a reverse id was used to indicate that the reserve id is valid. > > > > > > > So, we just increased reserve id size by one bit on 32-bit > > > > > > > platforms and by 33 bits on 64-bit platforms. > > > > > > It is more logical to me that `CLK_NULL_RESERVE_ID` is 0, but spec > > > > > > doesn't define it of course. > > > > > In our implementation, valid reserve id starts at 0 and increasing > > > > > linearly until `__SIZE_MAX-1`. This change will break our > > > > > implementation. > > > > > > > > > > However, we can modify our implementation to adopt this change since > > > > > it brings about benefits overall. > > > > Ideally it would be great to have unified implementation, but we can > > > > define device specific value for CLK_NULL_RESERVE_ID by using ifdef > > > > directive. > > > How about > > > > > > ``` > > > __attribute__((const)) size_t __clk_null_reserve_id(); > > > #define CLK_NULL_RESERVE_ID __clk_null_reserve_id() > > > > > > ``` > > > I think the spec does not require it to be compile time constant. Then > > > each library can implement its own __clk_null_reserve_id() whereas the IR > > > is target independent. > > Or we only do this for SPIR and define it as target specific value for > > other targets. > Defining CLK_NULL_RESERVE_ID as a function call should also work, but IMHO > compile time constant is preferable option. > I don't think making it compile time constant for SPIR only makes sense to me > - in this case we can use constant for all targets. > > How about following approach: use 0 by default and allow other targets > re-define CLK_NULL_RESERVE_ID value. > > ``` > #ifndef CLK_NULL_RESERVE_ID > #define CLK_NULL_RESERVE_ID 0 > #endif // CLK_NULL_RESERVE_ID > ``` > > If CLK_NULL_RESERVE_ID defined via -D command line option or included before > OpenCL C header file (via -include option), the defined value will be used, > otherwise 0. > > Will it work for you? No. That makes us unable to consume SPIR since CLK_NULL_RESERVE_ID is hardcoded as 0 when the source code is translated to SPIR whereas our target expects ~0U. To get a portable IR we need to represent CLK_NULL_RESERVE_ID as a function which can be lowered to a constant by a specific t
r303556 - clang-format: do not reflow bullet lists
Author: typz
Date: Mon May 22 09:47:17 2017
New Revision: 303556
URL: http://llvm.org/viewvc/llvm-project?rev=303556&view=rev
Log:
clang-format: do not reflow bullet lists
Summary:
This patch prevents reflowing bullet lists in block comments.
It handles all lists supported by doxygen and markdown, e.g. bullet
lists starting with '-', '*', '+', as well as numbered lists starting
with -# or a number followed by a dot.
Reviewers: krasimir
Reviewed By: krasimir
Subscribers: djasper, klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D33285
Modified:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/unittests/Format/FormatTestComments.cpp
Modified: cfe/trunk/lib/Format/BreakableToken.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/BreakableToken.cpp?rev=303556&r1=303555&r2=303556&view=diff
==
--- cfe/trunk/lib/Format/BreakableToken.cpp (original)
+++ cfe/trunk/lib/Format/BreakableToken.cpp Mon May 22 09:47:17 2017
@@ -78,6 +78,14 @@ static BreakableToken::Split getCommentS
}
StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
+
+ // Do not split before a number followed by a dot: this would be interpreted
+ // as a numbered list, which would prevent re-flowing in subsequent passes.
+ static llvm::Regex kNumberedListRegexp = llvm::Regex("^[1-9][0-9]?\\.");
+ if (SpaceOffset != StringRef::npos &&
+ kNumberedListRegexp.match(Text.substr(SpaceOffset).ltrim(Blanks)))
+SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+
if (SpaceOffset == StringRef::npos ||
// Don't break at leading whitespace.
Text.find_last_not_of(Blanks, SpaceOffset) == StringRef::npos) {
@@ -299,8 +307,9 @@ const FormatToken &BreakableComment::tok
static bool mayReflowContent(StringRef Content) {
Content = Content.trim(Blanks);
// Lines starting with '@' commonly have special meaning.
- static const SmallVector kSpecialMeaningPrefixes = {
- "@", "TODO", "FIXME", "XXX"};
+ // Lines starting with '-', '-#', '+' or '*' are bulleted/numbered lists.
+ static const SmallVector kSpecialMeaningPrefixes = {
+ "@", "TODO", "FIXME", "XXX", "-# ", "- ", "+ ", "* " };
bool hasSpecialMeaningPrefix = false;
for (StringRef Prefix : kSpecialMeaningPrefixes) {
if (Content.startswith(Prefix)) {
@@ -308,6 +317,14 @@ static bool mayReflowContent(StringRef C
break;
}
}
+
+ // Numbered lists may also start with a number followed by '.'
+ // To avoid issues if a line starts with a number which is actually the end
+ // of a previous line, we only consider numbers with up to 2 digits.
+ static llvm::Regex kNumberedListRegexp = llvm::Regex("^[1-9][0-9]?\\. ");
+ hasSpecialMeaningPrefix = hasSpecialMeaningPrefix ||
+kNumberedListRegexp.match(Content);
+
// Simple heuristic for what to reflow: content should contain at least two
// characters and either the first or second character must be
// non-punctuation.
Modified: cfe/trunk/unittests/Format/FormatTestComments.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestComments.cpp?rev=303556&r1=303555&r2=303556&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestComments.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestComments.cpp Mon May 22 09:47:17 2017
@@ -1577,7 +1577,7 @@ TEST_F(FormatTestComments, ReflowsCommen
" *\n"
" * long */",
getLLVMStyleWithColumns(20)));
-
+
// Don't reflow lines having content that is a single character.
EXPECT_EQ("// long long long\n"
"// long\n"
@@ -1602,7 +1602,7 @@ TEST_F(FormatTestComments, ReflowsCommen
format("// long long long long\n"
"// @param arg",
getLLVMStyleWithColumns(20)));
-
+
// Don't reflow lines starting with 'TODO'.
EXPECT_EQ("// long long long\n"
"// long\n"
@@ -1671,6 +1671,69 @@ TEST_F(FormatTestComments, ReflowsCommen
"// long",
getLLVMStyleWithColumns(20)));
+ // Don't reflow separate bullets in list
+ EXPECT_EQ("// - long long long\n"
+"// long\n"
+"// - long",
+format("// - long long long long\n"
+ "// - long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// * long long long\n"
+"// long\n"
+"// * long",
+format("// * long long long long\n"
+ "// * long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// + long long long\n"
+"// long\n"
+"// + long",
+format("// + long long long long\n"
+ "// + long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// 1. long long
[PATCH] D33285: clang-format: do not reflow bullet lists
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303556: clang-format: do not reflow bullet lists (authored
by Typz).
Changed prior to commit:
https://reviews.llvm.org/D33285?vs=99436&id=99764#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33285
Files:
cfe/trunk/lib/Format/BreakableToken.cpp
cfe/trunk/unittests/Format/FormatTestComments.cpp
Index: cfe/trunk/unittests/Format/FormatTestComments.cpp
===
--- cfe/trunk/unittests/Format/FormatTestComments.cpp
+++ cfe/trunk/unittests/Format/FormatTestComments.cpp
@@ -1577,7 +1577,7 @@
" *\n"
" * long */",
getLLVMStyleWithColumns(20)));
-
+
// Don't reflow lines having content that is a single character.
EXPECT_EQ("// long long long\n"
"// long\n"
@@ -1602,7 +1602,7 @@
format("// long long long long\n"
"// @param arg",
getLLVMStyleWithColumns(20)));
-
+
// Don't reflow lines starting with 'TODO'.
EXPECT_EQ("// long long long\n"
"// long\n"
@@ -1671,6 +1671,69 @@
"// long",
getLLVMStyleWithColumns(20)));
+ // Don't reflow separate bullets in list
+ EXPECT_EQ("// - long long long\n"
+"// long\n"
+"// - long",
+format("// - long long long long\n"
+ "// - long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// * long long long\n"
+"// long\n"
+"// * long",
+format("// * long long long long\n"
+ "// * long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// + long long long\n"
+"// long\n"
+"// + long",
+format("// + long long long long\n"
+ "// + long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// 1. long long long\n"
+"// long\n"
+"// 2. long",
+format("// 1. long long long long\n"
+ "// 2. long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// -# long long long\n"
+"// long\n"
+"// -# long",
+format("// -# long long long long\n"
+ "// -# long",
+ getLLVMStyleWithColumns(20)));
+
+ EXPECT_EQ("// - long long long\n"
+"// long long long\n"
+"// - long",
+format("// - long long long long\n"
+ "// long long\n"
+ "// - long",
+ getLLVMStyleWithColumns(20)));
+ EXPECT_EQ("// - long long long\n"
+"// long long long\n"
+"// long\n"
+"// - long",
+format("// - long long long long\n"
+ "// long long long\n"
+ "// - long",
+ getLLVMStyleWithColumns(20)));
+
+ // Large number (>2 digits) are not list items
+ EXPECT_EQ("// long long long\n"
+"// long 1024. long.",
+format("// long long long long\n"
+ "// 1024. long.",
+ getLLVMStyleWithColumns(20)));
+
+ // Do not break before number, to avoid introducing a non-reflowable doxygen
+ // list item.
+ EXPECT_EQ("// long long\n"
+"// long 10. long.",
+format("// long long long 10.\n"
+ "// long.",
+ getLLVMStyleWithColumns(20)));
+
// Don't break or reflow after implicit string literals.
verifyFormat("#include // l l l\n"
" // l",
Index: cfe/trunk/lib/Format/BreakableToken.cpp
===
--- cfe/trunk/lib/Format/BreakableToken.cpp
+++ cfe/trunk/lib/Format/BreakableToken.cpp
@@ -78,6 +78,14 @@
}
StringRef::size_type SpaceOffset = Text.find_last_of(Blanks, MaxSplitBytes);
+
+ // Do not split before a number followed by a dot: this would be interpreted
+ // as a numbered list, which would prevent re-flowing in subsequent passes.
+ static llvm::Regex kNumberedListRegexp = llvm::Regex("^[1-9][0-9]?\\.");
+ if (SpaceOffset != StringRef::npos &&
+ kNumberedListRegexp.match(Text.substr(SpaceOffset).ltrim(Blanks)))
+SpaceOffset = Text.find_last_of(Blanks, SpaceOffset);
+
if (SpaceOffset == StringRef::npos ||
// Don't break at leading whitespace.
Text.find_last_not_of(Blanks, SpaceOffset) == StringRef::npos) {
@@ -299,15 +307,24 @@
static bool mayReflowContent(StringRef Content) {
Content = Content.trim(Blanks);
// Lines starting with '@' commonly have special meaning.
- static const SmallVector kSpecialMeaningPrefixes = {
- "@", "TODO", "FIXME", "XXX"};
+ // Lines starting with '-', '-#', '+' or '*' are bulleted/numbered lists.
+ static const SmallVector kSpecialMeaningPrefixes = {
+ "@", "TODO", "FIXME"
r303557 - clang-format: [JS] avoid line breaks before unindented r_parens.
Author: mprobst
Date: Mon May 22 09:58:26 2017
New Revision: 303557
URL: http://llvm.org/viewvc/llvm-project?rev=303557&view=rev
Log:
clang-format: [JS] avoid line breaks before unindented r_parens.
The change that enabled wrapping at the previous scope's indentation had
unintended side-effects in that clang-format would prefer to wrap
closing parentheses to the next line if it avoided a wrap on the next
line (assuming very narrow lines):
fooObject
.someCall(barbazbam)
.then(bam);
Would get formatted as:
fooObject.someCall(barbazbam
).then(bam);
Because the ')' is now indented at the parent level (fooObject).
Normally formatting a builder pattern style call sequence like that is
outlawed in clang-format anyway. However for JavaScript this is special
cased to support trailing .bind calls.
This change disallows this special case when following a closing ')' to
avoid the problem.
Included are some random comment fixes.
Reviewers: djasper
Subscribers: klimek, cfe-commits
Differential Revision: https://reviews.llvm.org/D33399
Modified:
cfe/trunk/lib/Format/ContinuationIndenter.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp
Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=303557&r1=303556&r2=303557&view=diff
==
--- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
+++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Mon May 22 09:58:26 2017
@@ -207,7 +207,7 @@ bool ContinuationIndenter::mustBreak(con
// ...
// }.bind(...));
// FIXME: We should find a more generic solution to this problem.
- !(State.Column <= NewLineColumn &&
+ !(State.Column <= NewLineColumn && Previous.isNot(tok::r_paren) &&
Style.Language == FormatStyle::LK_JavaScript))
return true;
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=303557&r1=303556&r2=303557&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Mon May 22 09:58:26 2017
@@ -2570,7 +2570,7 @@ bool TokenAnnotator::canBreakBefore(cons
Keywords.kw_interface, Keywords.kw_type, tok::kw_static,
tok::kw_public, tok::kw_private, tok::kw_protected,
Keywords.kw_abstract, Keywords.kw_get, Keywords.kw_set))
- return false; // Otherwise a semicolon is inserted.
+ return false; // Otherwise automatic semicolon insertion would trigger.
if (Left.is(TT_JsFatArrow) && Right.is(tok::l_brace))
return false;
if (Left.is(TT_JsTypeColon))
Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=303557&r1=303556&r2=303557&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Mon May 22 09:58:26 2017
@@ -1823,6 +1823,11 @@ TEST_F(FormatTestJS, NonNullAssertionOpe
verifyFormat("let x = !foo;\n");
verifyFormat("let x = foo[0]!;\n");
verifyFormat("let x = (foo)!;\n");
+ verifyFormat("let x = x(foo!);\n");
+ verifyFormat(
+ "a.aa(a.a!).then(\n"
+ "x => x(x));\n",
+ getGoogleJSStyleWithColumns(20));
verifyFormat("let x = foo! - 1;\n");
verifyFormat("let x = {foo: 1}!;\n");
verifyFormat(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32896: [OpenCL] Make CLK_NULL_RESERVE_ID invalid reserve id.
bader added inline comments. Comment at: lib/Headers/opencl-c.h:16020 +// The macro CLK_NULL_RESERVE_ID refers to an invalid reservation ID. +#define CLK_NULL_RESERVE_ID (__builtin_astype((void *)0, reserve_id_t)) bool __ovld is_valid_reserve_id(reserve_id_t reserve_id); yaxunl wrote: > bader wrote: > > yaxunl wrote: > > > yaxunl wrote: > > > > bader wrote: > > > > > yaxunl wrote: > > > > > > Anastasia wrote: > > > > > > > echuraev wrote: > > > > > > > > yaxunl wrote: > > > > > > > > > Anastasia wrote: > > > > > > > > > > yaxunl wrote: > > > > > > > > > > > Anastasia wrote: > > > > > > > > > > > > Looks good from my side. > > > > > > > > > > > > > > > > > > > > > > > > @yaxunl , since you originally committed this. Could > > > > > > > > > > > > you please verify that changing from `SIZE_MAX` to `0` > > > > > > > > > > > > would be fine. > > > > > > > > > > > > > > > > > > > > > > > > Btw, we have a similar definition for `CLK_NULL_EVENT`. > > > > > > > > > > > `__PIPE_RESERVE_ID_VALID_BIT` is implementation detail > > > > > > > > > > > and not part of the spec. I would suggest to remove it > > > > > > > > > > > from this header file. > > > > > > > > > > > > > > > > > > > > > > The spec only requires CLK_NULL_RESERVE_ID to be defined > > > > > > > > > > > but does not define its value. Naturally a valid id > > > > > > > > > > > starts from 0 and increases. I don't see significant > > > > > > > > > > > advantage to change CLK_NULL_RESERVE_ID from __SIZE_MAX > > > > > > > > > > > to 0. > > > > > > > > > > > > > > > > > > > > > > Is there any reason that this change is needed? > > > > > > > > > > I don't see issues to commit things outside of spec as soon > > > > > > > > > > as they prefixed properly with "__". But I agree it would > > > > > > > > > > be nice to see if it's any useful and what the motivation > > > > > > > > > > is for having different implementation. > > > > > > > > > For `__PIPE_RESERVE_ID_VALID_BIT`, it assumes that the > > > > > > > > > implementation uses one specific bit of a reserve id to > > > > > > > > > indicate that the reserve id is valid. Not all > > > > > > > > > implementations assume that. Actually I am curious why that > > > > > > > > > is needed too. > > > > > > > > About `CLK_NULL_RESERVE_ID`: we check that reserve id is valid > > > > > > > > if significant bit equal to one. `CLK_NULL_RESERVE_ID refers to > > > > > > > > an invalid reservation, so if `CLK_NULL_RESERVE_ID equal to 0, > > > > > > > > we can be sure that significant bit doesn't equal to 1 and it > > > > > > > > is invalid reserve id. Also it is more obviously if > > > > > > > > CLK_**NULL**_RESERVE_ID is equal to 0. > > > > > > > > > > > > > > > > What about `__PIPE_RESERVE_ID_VALID_BIT`: As I understand > > > > > > > > previous implementation also assumes that one specific bit was > > > > > > > > of a reverse id was used to indicate that the reserve id is > > > > > > > > valid. So, we just increased reserve id size by one bit on > > > > > > > > 32-bit platforms and by 33 bits on 64-bit platforms. > > > > > > > It is more logical to me that `CLK_NULL_RESERVE_ID` is 0, but > > > > > > > spec doesn't define it of course. > > > > > > In our implementation, valid reserve id starts at 0 and increasing > > > > > > linearly until `__SIZE_MAX-1`. This change will break our > > > > > > implementation. > > > > > > > > > > > > However, we can modify our implementation to adopt this change > > > > > > since it brings about benefits overall. > > > > > Ideally it would be great to have unified implementation, but we can > > > > > define device specific value for CLK_NULL_RESERVE_ID by using ifdef > > > > > directive. > > > > How about > > > > > > > > ``` > > > > __attribute__((const)) size_t __clk_null_reserve_id(); > > > > #define CLK_NULL_RESERVE_ID __clk_null_reserve_id() > > > > > > > > ``` > > > > I think the spec does not require it to be compile time constant. Then > > > > each library can implement its own __clk_null_reserve_id() whereas the > > > > IR is target independent. > > > Or we only do this for SPIR and define it as target specific value for > > > other targets. > > Defining CLK_NULL_RESERVE_ID as a function call should also work, but IMHO > > compile time constant is preferable option. > > I don't think making it compile time constant for SPIR only makes sense to > > me - in this case we can use constant for all targets. > > > > How about following approach: use 0 by default and allow other targets > > re-define CLK_NULL_RESERVE_ID value. > > > > ``` > > #ifndef CLK_NULL_RESERVE_ID > > #define CLK_NULL_RESERVE_ID 0 > > #endif // CLK_NULL_RESERVE_ID > > ``` > > > > If CLK_NULL_RESERVE_ID defined via -D command line option or included > > before OpenCL C header file (via -include option), the defined value will > > be used, otherwise 0. > > > > Will it work for you? > No. That makes us unable to consume SPIR since CLK_N
r303559 - [index] Visit the default argument values in function definitions
Author: arphaman
Date: Mon May 22 10:17:44 2017
New Revision: 303559
URL: http://llvm.org/viewvc/llvm-project?rev=303559&view=rev
Log:
[index] Visit the default argument values in function definitions
rdar://32323315
Modified:
cfe/trunk/lib/Index/IndexDecl.cpp
cfe/trunk/test/Index/Core/index-source.cpp
Modified: cfe/trunk/lib/Index/IndexDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=303559&r1=303558&r2=303559&view=diff
==
--- cfe/trunk/lib/Index/IndexDecl.cpp (original)
+++ cfe/trunk/lib/Index/IndexDecl.cpp Mon May 22 10:17:44 2017
@@ -98,6 +98,17 @@ public:
}
}
}
+} else {
+ // Index the default parameter value for function definitions.
+ if (const auto *FD = dyn_cast(D)) {
+if (FD->isThisDeclarationADefinition()) {
+ for (const auto *PV : FD->parameters()) {
+if (PV->hasDefaultArg() && !PV->hasUninstantiatedDefaultArg() &&
+!PV->hasUnparsedDefaultArg())
+ IndexCtx.indexBody(PV->getDefaultArg(), D);
+ }
+}
+ }
}
}
Modified: cfe/trunk/test/Index/Core/index-source.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.cpp?rev=303559&r1=303558&r2=303559&view=diff
==
--- cfe/trunk/test/Index/Core/index-source.cpp (original)
+++ cfe/trunk/test/Index/Core/index-source.cpp Mon May 22 10:17:44 2017
@@ -1,7 +1,7 @@
// RUN: c-index-test core -print-source-symbols -- %s -std=c++14 -target
x86_64-apple-macosx10.7 | FileCheck %s
// CHECK: [[@LINE+1]]:7 | class/C++ | Cls | [[Cls_USR:.*]] | | Def
| rel: 0
-class Cls {
+class Cls { public:
// CHECK: [[@LINE+3]]:3 | constructor/C++ | Cls | c:@S@Cls@F@Cls#I# |
__ZN3ClsC1Ei | Decl,RelChild | rel: 1
// CHECK-NEXT: RelChild | Cls | c:@S@Cls
// CHECK: [[@LINE+1]]:3 | class/C++ | Cls | c:@S@Cls | |
Ref,RelCont | rel: 1
@@ -350,3 +350,19 @@ void innerUsingNamespace() {
// CHECK-NOT: [[@LINE-3]]:21
}
}
+
+void indexDefaultValueInDefn(Cls cls = Cls(gvi), Record param = Record()) {
+// CHECK: [[@LINE-1]]:40 | class/C++ | Cls | c:@S@Cls | |
Ref,RelCont | rel: 1
+// CHECK: [[@LINE-2]]:44 | variable/C | gvi | c:@gvi | _gvi | Ref,Read,RelCont
| rel: 1
+// CHECK-NOT: [[@LINE-3]]:44
+// CHECK: [[@LINE-4]]:65 | struct/C++ | Record | c:@S@Record | |
Ref,RelCont | rel: 1
+// CHECK-NOT: [[@LINE-5]]:65
+}
+
+template class T>
+struct IndexDefaultValue {
+ IndexDefaultValue(int k = Record::C) {
+// CHECK: [[@LINE-1]]:38 | static-property/C++ | C | c:@S@Record@C |
__ZN6Record1CE | Ref,Read,RelCont | rel: 1
+// CHECK: [[@LINE-2]]:30 | struct/C++ | Record | c:@S@Record | |
Ref,RelCont | rel: 1
+ }
+};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33412: Add support for #pragma clang section
javed.absar created this revision.
Herald added a subscriber: aemerson.
This patch adds support for a '#pragma clang section' directive in clang.
An RFC was sent out earlier by my colleague James Molloy:
http://lists.llvm.org/pipermail/cfe-dev/2017-March/053100.html
Purpose:
The purpose of this is to provide to developers a means to specify
section-names
for global variables, functions and static variables, using #pragma
directives.
This will provide a migration path towards clang for developers in the
embedded
and automotive domain. For example, AUTOSAR, an automotive standard, mandates
the use of a #pragma in header files to determine in which sections
initialized
and uninitialized data get put into.
This feature is implemented in our legacy ARM Compiler 5 toolchain and GCC
forks
used across the automotive space that have this feature implemented compatible
with the ARM Compiler 5 implementation. The documentation is here:
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0472m/chr1359124985290.html
User-Visible Behavior:
The developer can specify section names as:
#pragma clang section bss="myBSS" data="myData" rodata="myRodata"
text="myText"
The developer can "unspecify" a section name with empty string e.g.
#pragma clang section bss="" data="" text="" rodata=""
1. The pragma applies to all global variable, statics and function
declarations
from the pragma to the end of the translation unit.
2. The pragma clang section is enabled automatically, without need of any
flags.
3. This feature is only defined to work sensibly for ELF targets.
4. If section name is specified through _attribute_((section("myname"))), then
the attribute name gains precedence over any applicable section name via
pragma directives.
5. Global variables, including - basic types, arrays, struct - that are
initialized to zero
e.g. int x = 0; will be placed in the named bss section, if one is present.
6. The section type using '#pragma clang section' approach does not does NOT
try to infer section-kind from the name.
For example, assigning a section ".bss.mysec" does NOT mean it will be placed
in BSS.
Design:
The decision about which section-kind applies to each global is not taken by
this
implementation but is purely inquired from the default back-end
implementation in LLVM.
Once the section-kind is known, appropriate section name as specified by the
user
using pragma directive, is applied to that global.
Note that this is more effective approach than choosing the section name in
the front-end
when optimisation passes have not been run and the final proper section is
not known.
There is a llvm corresponding patch with this patch.
https://reviews.llvm.org/D33412
Files:
include/clang/Basic/Attr.td
include/clang/Basic/DiagnosticParseKinds.td
include/clang/Parse/Parser.h
include/clang/Sema/Sema.h
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CodeGenModule.cpp
lib/Parse/ParsePragma.cpp
lib/Sema/SemaAttr.cpp
lib/Sema/SemaDecl.cpp
test/CodeGenCXX/clang-sections-tentative.c
test/CodeGenCXX/clang-sections.cpp
test/Sema/pragma-clang-section.c
Index: test/Sema/pragma-clang-section.c
===
--- /dev/null
+++ test/Sema/pragma-clang-section.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple arm-none-eabi
+#pragma clang section bss="mybss.1" data="mydata.1" rodata="myrodata.1" text="mytext.1"
+#pragma clang section bss="" data="" rodata="" text=""
+#pragma clang section
+
+#pragma clang section dss="mybss.2" // expected-error {{expected one of [bss|data|rodata|text] section kind in '#pragma clang section'}}
+#pragma clang section deta="mydata.2" // expected-error {{expected one of [bss|data|rodata|text] section kind in '#pragma clang section'}}
+#pragma clang section rodeta="rodata.2" // expected-error {{expected one of [bss|data|rodata|text] section kind in '#pragma clang section'}}
+#pragma clang section taxt="text.2" // expected-error {{expected one of [bss|data|rodata|text] section kind in '#pragma clang section'}}
+
+#pragma clang section section bss="mybss.2" // expected-error {{expected one of [bss|data|rodata|text] section kind in '#pragma clang section'}}
+
+#pragma clang section bss "mybss.2" // expected-error {{expected '=' following '#pragma clang section bss'}}
+#pragma clang section data "mydata.2" // expected-error {{expected '=' following '#pragma clang section data'}}
+#pragma clang section rodata "myrodata.2" // expected-error {{expected '=' following '#pragma clang section rodata'}}
+#pragma clang section bss="" data="" rodata="" text="" more //expected-error {{expected one of [bss|data|rodata|text] section kind in '#pragma clang section'}}
+int a;
Index: test/CodeGenCXX/clang-sections.cpp
===
--- /dev/null
+++ test/C
[PATCH] D33304: [WIP][clang-tidy] Add a new module Android and a new check for file descriptors.
yawanng added a comment. I will make some major changes to this CL based on the current suggestions from reviewers and update it for further review later. Thank you for the valuable advice. Repository: rL LLVM https://reviews.llvm.org/D33304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303562 - [Sema][ObjC] Fix a bug where -Wunguarded-availability was emitted at the wrong location
Author: epilk
Date: Mon May 22 10:41:12 2017
New Revision: 303562
URL: http://llvm.org/viewvc/llvm-project?rev=303562&view=rev
Log:
[Sema][ObjC] Fix a bug where -Wunguarded-availability was emitted at the wrong
location
Differential revision: https://reviews.llvm.org/D33250
Modified:
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/SemaObjC/unguarded-availability.m
Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=303562&r1=303561&r2=303562&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Mon May 22 10:41:12 2017
@@ -7258,6 +7258,12 @@ public:
bool TraverseLambdaExpr(LambdaExpr *E) { return true; }
+ bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *PRE) {
+if (PRE->isClassReceiver())
+ DiagnoseDeclAvailability(PRE->getClassReceiver(),
PRE->getReceiverLocation());
+return true;
+ }
+
bool VisitObjCMessageExpr(ObjCMessageExpr *Msg) {
if (ObjCMethodDecl *D = Msg->getMethodDecl())
DiagnoseDeclAvailability(
@@ -7387,6 +7393,9 @@ bool DiagnoseUnguardedAvailability::Visi
const Type *TyPtr = Ty.getTypePtr();
SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
+ if (Range.isInvalid())
+return true;
+
if (const TagType *TT = dyn_cast(TyPtr)) {
TagDecl *TD = TT->getDecl();
DiagnoseDeclAvailability(TD, Range);
Modified: cfe/trunk/test/SemaObjC/unguarded-availability.m
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/unguarded-availability.m?rev=303562&r1=303561&r2=303562&view=diff
==
--- cfe/trunk/test/SemaObjC/unguarded-availability.m (original)
+++ cfe/trunk/test/SemaObjC/unguarded-availability.m Mon May 22 10:41:12 2017
@@ -135,6 +135,26 @@ void (^topLevelBlockDecl)() = ^ {
func_10_12();
};
+AVAILABLE_10_12
+__attribute__((objc_root_class))
+@interface InterWithProp // expected-note 2 {{marked partial here}}
+@property(class) int x;
++ (void) setX: (int)newX AVAILABLE_10_12; // expected-note{{marked partial
here}}
+@end
+void test_property(void) {
+ int y = InterWithProp.x; // expected-warning{{'InterWithProp' is only
available on macOS 10.12 or newer}} expected-note{{@available}}
+ InterWithProp.x = y; // expected-warning{{'InterWithProp' is only available
on macOS 10.12 or newer}} expected-note{{@available}} expected-warning{{'setX:'
is only available on macOS 10.12 or newer}} expected-note{{@available}}
+}
+
+__attribute__((objc_root_class))
+@interface Subscriptable
+- (id)objectAtIndexedSubscript:(int)sub AVAILABLE_10_12; //
expected-note{{marked partial here}}
+@end
+
+void test_at(Subscriptable *x) {
+ id y = x[42]; // expected-warning{{'objectAtIndexedSubscript:' is only
available on macOS 10.12 or newer}} expected-note{{@available}}
+}
+
#ifdef OBJCPP
int f(char) AVAILABLE_10_12;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33250: [Sema][ObjC] Fix a bug where -Wunguarded-availability was emitted at the wrong location
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303562: [Sema][ObjC] Fix a bug where
-Wunguarded-availability was emitted at the wrong… (authored by epilk).
Changed prior to commit:
https://reviews.llvm.org/D33250?vs=99605&id=99772#toc
Repository:
rL LLVM
https://reviews.llvm.org/D33250
Files:
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/SemaObjC/unguarded-availability.m
Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -7258,6 +7258,12 @@
bool TraverseLambdaExpr(LambdaExpr *E) { return true; }
+ bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *PRE) {
+if (PRE->isClassReceiver())
+ DiagnoseDeclAvailability(PRE->getClassReceiver(),
PRE->getReceiverLocation());
+return true;
+ }
+
bool VisitObjCMessageExpr(ObjCMessageExpr *Msg) {
if (ObjCMethodDecl *D = Msg->getMethodDecl())
DiagnoseDeclAvailability(
@@ -7387,6 +7393,9 @@
const Type *TyPtr = Ty.getTypePtr();
SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
+ if (Range.isInvalid())
+return true;
+
if (const TagType *TT = dyn_cast(TyPtr)) {
TagDecl *TD = TT->getDecl();
DiagnoseDeclAvailability(TD, Range);
Index: cfe/trunk/test/SemaObjC/unguarded-availability.m
===
--- cfe/trunk/test/SemaObjC/unguarded-availability.m
+++ cfe/trunk/test/SemaObjC/unguarded-availability.m
@@ -135,6 +135,26 @@
func_10_12();
};
+AVAILABLE_10_12
+__attribute__((objc_root_class))
+@interface InterWithProp // expected-note 2 {{marked partial here}}
+@property(class) int x;
++ (void) setX: (int)newX AVAILABLE_10_12; // expected-note{{marked partial
here}}
+@end
+void test_property(void) {
+ int y = InterWithProp.x; // expected-warning{{'InterWithProp' is only
available on macOS 10.12 or newer}} expected-note{{@available}}
+ InterWithProp.x = y; // expected-warning{{'InterWithProp' is only available
on macOS 10.12 or newer}} expected-note{{@available}} expected-warning{{'setX:'
is only available on macOS 10.12 or newer}} expected-note{{@available}}
+}
+
+__attribute__((objc_root_class))
+@interface Subscriptable
+- (id)objectAtIndexedSubscript:(int)sub AVAILABLE_10_12; //
expected-note{{marked partial here}}
+@end
+
+void test_at(Subscriptable *x) {
+ id y = x[42]; // expected-warning{{'objectAtIndexedSubscript:' is only
available on macOS 10.12 or newer}} expected-note{{@available}}
+}
+
#ifdef OBJCPP
int f(char) AVAILABLE_10_12;
Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -7258,6 +7258,12 @@
bool TraverseLambdaExpr(LambdaExpr *E) { return true; }
+ bool VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *PRE) {
+if (PRE->isClassReceiver())
+ DiagnoseDeclAvailability(PRE->getClassReceiver(), PRE->getReceiverLocation());
+return true;
+ }
+
bool VisitObjCMessageExpr(ObjCMessageExpr *Msg) {
if (ObjCMethodDecl *D = Msg->getMethodDecl())
DiagnoseDeclAvailability(
@@ -7387,6 +7393,9 @@
const Type *TyPtr = Ty.getTypePtr();
SourceRange Range{Ty.getBeginLoc(), Ty.getEndLoc()};
+ if (Range.isInvalid())
+return true;
+
if (const TagType *TT = dyn_cast(TyPtr)) {
TagDecl *TD = TT->getDecl();
DiagnoseDeclAvailability(TD, Range);
Index: cfe/trunk/test/SemaObjC/unguarded-availability.m
===
--- cfe/trunk/test/SemaObjC/unguarded-availability.m
+++ cfe/trunk/test/SemaObjC/unguarded-availability.m
@@ -135,6 +135,26 @@
func_10_12();
};
+AVAILABLE_10_12
+__attribute__((objc_root_class))
+@interface InterWithProp // expected-note 2 {{marked partial here}}
+@property(class) int x;
++ (void) setX: (int)newX AVAILABLE_10_12; // expected-note{{marked partial here}}
+@end
+void test_property(void) {
+ int y = InterWithProp.x; // expected-warning{{'InterWithProp' is only available on macOS 10.12 or newer}} expected-note{{@available}}
+ InterWithProp.x = y; // expected-warning{{'InterWithProp' is only available on macOS 10.12 or newer}} expected-note{{@available}} expected-warning{{'setX:' is only available on macOS 10.12 or newer}} expected-note{{@available}}
+}
+
+__attribute__((objc_root_class))
+@interface Subscriptable
+- (id)objectAtIndexedSubscript:(int)sub AVAILABLE_10_12; // expected-note{{marked partial here}}
+@end
+
+void test_at(Subscriptable *x) {
+ id y = x[42]; // expected-warning{{'objectAtIndexedSubscript:' is only available on macOS 10.12 or newer}} expected-note{{@available}}
+}
+
#ifdef OBJCPP
int f(char) AVAILABLE_10_12;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mail
r303563 - [index] Index the deleted functions
Author: arphaman
Date: Mon May 22 10:42:45 2017
New Revision: 303563
URL: http://llvm.org/viewvc/llvm-project?rev=303563&view=rev
Log:
[index] Index the deleted functions
rdar://32323386
Modified:
cfe/trunk/lib/Index/IndexDecl.cpp
cfe/trunk/test/Index/Core/index-source.cpp
Modified: cfe/trunk/lib/Index/IndexDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=303563&r1=303562&r2=303563&view=diff
==
--- cfe/trunk/lib/Index/IndexDecl.cpp (original)
+++ cfe/trunk/lib/Index/IndexDecl.cpp Mon May 22 10:42:45 2017
@@ -217,9 +217,6 @@ public:
}
bool VisitFunctionDecl(const FunctionDecl *D) {
-if (D->isDeleted())
- return true;
-
SymbolRoleSet Roles{};
SmallVector Relations;
if (auto *CXXMD = dyn_cast(D)) {
Modified: cfe/trunk/test/Index/Core/index-source.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.cpp?rev=303563&r1=303562&r2=303563&view=diff
==
--- cfe/trunk/test/Index/Core/index-source.cpp (original)
+++ cfe/trunk/test/Index/Core/index-source.cpp Mon May 22 10:42:45 2017
@@ -366,3 +366,11 @@ struct IndexDefaultValue {
// CHECK: [[@LINE-2]]:30 | struct/C++ | Record | c:@S@Record | |
Ref,RelCont | rel: 1
}
};
+
+struct DeletedMethods {
+ DeletedMethods(const DeletedMethods &) = delete;
+// CHECK: [[@LINE-1]]:3 | constructor/cxx-copy-ctor/C++ | DeletedMethods |
c:@S@DeletedMethods@F@DeletedMethods#&1$@S@DeletedMethods# |
__ZN14DeletedMethodsC1ERKS_ | Def,RelChild | rel: 1
+// CHECK: RelChild | DeletedMethods | c:@S@DeletedMethods
+// CHECK: [[@LINE-3]]:24 | struct/C++ | DeletedMethods | c:@S@DeletedMethods |
| Ref,RelCont | rel: 1
+// CHECK: [[@LINE-4]]:3 | struct/C++ | DeletedMethods | c:@S@DeletedMethods |
| Ref,RelCont | rel: 1
+};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33415: Replaced WorkerRequest with std::function...
ilya-biryukov created this revision.
And implemented a helper function to dump an AST of a file for
testing/debugging purposes.
https://reviews.llvm.org/D33415
Files:
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/ClangdUnitStore.h
Index: clangd/ClangdUnitStore.h
===
--- clangd/ClangdUnitStore.h
+++ clangd/ClangdUnitStore.h
@@ -50,6 +50,18 @@
std::forward(Action));
}
+ /// Run the specified \p Action on the ClangdUnit for \p File.
+ /// Unit for \p File should exist in the store.
+ template
+ void runOnExistingUnit(PathRef File, Func Action) {
+std::lock_guard Lock(Mutex);
+
+auto It = OpenedFiles.find(File);
+assert(It != OpenedFiles.end() && "File is not in OpenedFiles");
+
+Action(It->second);
+ }
+
/// Remove ClangdUnit for \p File, if any
void removeUnitIfPresent(PathRef File);
Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -16,6 +16,10 @@
#include "clang/Tooling/Core/Replacement.h"
#include
+namespace llvm {
+class raw_ostream;
+}
+
namespace clang {
class ASTUnit;
class PCHContainerOperations;
@@ -52,6 +56,10 @@
/// located in the current file.
std::vector getLocalDiagnostics() const;
+ /// For testing/debugging purposes. Note that this method deserializes all
+ /// unserialized Decls, so use with care.
+ void dumpAST(llvm::raw_ostream &OS) const;
+
private:
Path FileName;
std::unique_ptr Unit;
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -222,3 +222,7 @@
}
return Result;
}
+
+void ClangdUnit::dumpAST(llvm::raw_ostream &OS) const {
+ Unit->getASTContext().getTranslationUnitDecl()->dump(OS, true);
+}
Index: clangd/ClangdServer.h
===
--- clangd/ClangdServer.h
+++ clangd/ClangdServer.h
@@ -24,6 +24,7 @@
#include "Protocol.h"
#include
+#include
#include
#include
#include
@@ -49,30 +50,21 @@
std::vector Diagnostics) = 0;
};
-enum class WorkerRequestKind { ParseAndPublishDiagnostics, RemoveDocData };
-
-/// A request to the worker thread
-class WorkerRequest {
-public:
- WorkerRequest() = default;
- WorkerRequest(WorkerRequestKind Kind, Path File, DocVersion Version);
-
- WorkerRequestKind Kind;
- Path File;
- DocVersion Version;
-};
-
class ClangdServer;
/// Handles running WorkerRequests of ClangdServer on a separate threads.
/// Currently runs only one worker thread.
class ClangdScheduler {
public:
- ClangdScheduler(ClangdServer &Server, bool RunSynchronously);
+ ClangdScheduler(bool RunSynchronously);
~ClangdScheduler();
- /// Enqueue WorkerRequest to be run on a worker thread
- void enqueue(ClangdServer &Server, WorkerRequest Request);
+ /// Add \p Request to the start of the queue. \p Request will be run on a
+ /// separate worker thread.
+ void addToFront(std::function Request);
+ /// Add \p Request to the end of the queue. \p Request will be run on a
+ /// separate worker thread.
+ void addToEnd(std::function Request);
private:
bool RunSynchronously;
@@ -83,11 +75,10 @@
std::thread Worker;
/// Setting Done to true will make the worker thread terminate.
bool Done = false;
- /// A LIFO queue of requests. Note that requests are discarded if the
- /// `version` field is not equal to the one stored inside DraftStore.
+ /// A queue of requests.
/// FIXME(krasimir): code completion should always have priority over parsing
/// for diagnostics.
- std::deque RequestQueue;
+ std::deque> RequestQueue;
/// Condition variable to wake up the worker thread.
std::condition_variable RequestCV;
};
@@ -126,12 +117,12 @@
/// conversions in outside code, maybe there's a way to get rid of it.
std::string getDocument(PathRef File);
-private:
- friend class ClangdScheduler;
-
- /// This function is called on a worker thread.
- void handleRequest(WorkerRequest Request);
+ /// Only for testing purposes.
+ /// Waits until all requests to worker thread are finished and dumps AST for
+ /// \p File. \p File must be in the list of added documents.
+ std::string dumpAST(PathRef File);
+private:
std::unique_ptr CDB;
std::unique_ptr DiagConsumer;
DraftStore DraftMgr;
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -15,6 +15,8 @@
#include "clang/Tooling/CompilationDatabase.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/raw_ostream.h"
+#include
using namespace clang;
using namespace clang::clangd;
@@ -56,22 +58,18 @@
return {Lines, Cols};
}
-WorkerRequest:
r303564 - [mips] Quote command line options with `` in the micromips attribute description. NFC
Author: atanasyan Date: Mon May 22 10:53:31 2017 New Revision: 303564 URL: http://llvm.org/viewvc/llvm-project?rev=303564&view=rev Log: [mips] Quote command line options with `` in the micromips attribute description. NFC Modified: cfe/trunk/include/clang/Basic/AttrDocs.td Modified: cfe/trunk/include/clang/Basic/AttrDocs.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=303564&r1=303563&r2=303564&view=diff == --- cfe/trunk/include/clang/Basic/AttrDocs.td (original) +++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon May 22 10:53:31 2017 @@ -1277,7 +1277,7 @@ Clang supports the GNU style ``__attribu may be attached to a function definition and instructs the backend to generate or not to generate microMIPS code for that function. -These attributes override the -mmicromips and -mno-micromips options +These attributes override the `-mmicromips` and `-mno-micromips` options on the command line. }]; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303565 - [mips] Add one more check to the micromips attribute test case. NFC
Author: atanasyan
Date: Mon May 22 10:53:34 2017
New Revision: 303565
URL: http://llvm.org/viewvc/llvm-project?rev=303565&view=rev
Log:
[mips] Add one more check to the micromips attribute test case. NFC
Modified:
cfe/trunk/test/Sema/attr-micromips.c
Modified: cfe/trunk/test/Sema/attr-micromips.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-micromips.c?rev=303565&r1=303564&r2=303565&view=diff
==
--- cfe/trunk/test/Sema/attr-micromips.c (original)
+++ cfe/trunk/test/Sema/attr-micromips.c Mon May 22 10:53:34 2017
@@ -13,3 +13,5 @@ __attribute__((mips16,micromips)) void f
__attribute((micromips)) void foo7();
__attribute((nomicromips)) void foo8();
+__attribute__((mips16)) void foo9(void) __attribute__((micromips)); //
expected-error {{'micromips' and 'mips16' attributes are not compatible}} \
+//
expected-note {{conflicting attribute is here}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33395: [clangd] Split clangd into library+executable (mainly for unit tests).
ilya-biryukov updated this revision to Diff 99774.
ilya-biryukov added a comment.
Got rid of relative includes from parent dir in ClangdMain.cpp. (Addressed
krasimir's comments)
https://reviews.llvm.org/D33395
Files:
clangd/CMakeLists.txt
clangd/ClangdMain.cpp
clangd/tool/CMakeLists.txt
clangd/tool/ClangdMain.cpp
Index: clangd/ClangdMain.cpp
===
--- /dev/null
+++ clangd/ClangdMain.cpp
@@ -1,40 +0,0 @@
-//===--- ClangdMain.cpp - clangd server loop --===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "ClangdLSPServer.h"
-#include "JSONRPCDispatcher.h"
-#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/FileSystem.h"
-#include "llvm/Support/Program.h"
-
-#include
-#include
-#include
-
-using namespace clang;
-using namespace clang::clangd;
-
-static llvm::cl::opt
-RunSynchronously("run-synchronously",
- llvm::cl::desc("parse on main thread"),
- llvm::cl::init(false), llvm::cl::Hidden);
-
-int main(int argc, char *argv[]) {
- llvm::cl::ParseCommandLineOptions(argc, argv, "clangd");
-
- llvm::raw_ostream &Outs = llvm::outs();
- llvm::raw_ostream &Logs = llvm::errs();
- JSONOutput Out(Outs, Logs);
-
- // Change stdin to binary to not lose \r\n on windows.
- llvm::sys::ChangeStdinToBinary();
-
- ClangdLSPServer LSPServer(Out, RunSynchronously);
- LSPServer.run(std::cin);
-}
Index: clangd/tool/CMakeLists.txt
===
--- clangd/tool/CMakeLists.txt
+++ clangd/tool/CMakeLists.txt
@@ -1,20 +1,14 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
+
add_clang_executable(clangd
- ClangdLSPServer.cpp
ClangdMain.cpp
- ClangdServer.cpp
- ClangdUnit.cpp
- ClangdUnitStore.cpp
- DraftStore.cpp
- GlobalCompilationDatabase.cpp
- JSONRPCDispatcher.cpp
- Protocol.cpp
- ProtocolHandlers.cpp
)
install(TARGETS clangd RUNTIME DESTINATION bin)
target_link_libraries(clangd
clangBasic
+ clangDaemon
clangFormat
clangFrontend
clangSema
Index: clangd/CMakeLists.txt
===
--- clangd/CMakeLists.txt
+++ clangd/CMakeLists.txt
@@ -1,6 +1,5 @@
-add_clang_executable(clangd
+add_clang_library(clangDaemon
ClangdLSPServer.cpp
- ClangdMain.cpp
ClangdServer.cpp
ClangdUnit.cpp
ClangdUnitStore.cpp
@@ -11,14 +10,14 @@
ProtocolHandlers.cpp
)
-install(TARGETS clangd RUNTIME DESTINATION bin)
-
-target_link_libraries(clangd
+target_link_libraries(clangDaemon
clangBasic
clangFormat
clangFrontend
clangSema
clangTooling
clangToolingCore
LLVMSupport
)
+
+add_subdirectory(tool)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r303546 - [mips] Support `micromips` attribute
On Mon, May 22, 2017 at 3:57 PM, Aaron Ballman wrote:
> On Mon, May 22, 2017 at 8:47 AM, Simon Atanasyan via cfe-commits
> wrote:
>> Author: atanasyan
>> Date: Mon May 22 07:47:43 2017
>> New Revision: 303546
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=303546&view=rev
>> Log:
>> [mips] Support `micromips` attribute
>>
>> This patch adds support for the `micromips` and `nomicromips` attributes
>> for MIPS targets.
>>
>> Differential revision: https://reviews.llvm.org/D33363
[...]
>> Modified: cfe/trunk/include/clang/Basic/Attr.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=303546&r1=303545&r2=303546&view=diff
>> ==
>> --- cfe/trunk/include/clang/Basic/Attr.td (original)
>> +++ cfe/trunk/include/clang/Basic/Attr.td Mon May 22 07:47:43 2017
>> @@ -1179,6 +1179,12 @@ def MipsInterrupt : InheritableAttr, Tar
>>let Documentation = [MipsInterruptDocs];
>> }
>>
>> +def MicroMips : InheritableAttr, TargetSpecificAttr {
>> + let Spellings = [GCC<"micromips">];
>> + let Subjects = SubjectList<[Function], ErrorDiag>;
>
> Why is this an error rather than a warning? Same question below.
Because GCC shows the error in this case. And I cannot imagine any
reason why micromips attribute added to non-function declaration.
% mips-mti-linux-gnu-gcc --version
mips-mti-linux-gnu-gcc (Codescape GNU Tools 2016.05-01 for MIPS MTI Linux) 4.9.2
% mips-mti-linux-gnu-gcc -c attr-micromips.c
tools/clang/test/Sema/attr-micromips.c:6:1: error: ‘nomicromips’
attribute only applies to functions
__attribute((nomicromips)) int a; // expected-error {{attribute only
applies to functions}}
^
tools/clang/test/Sema/attr-micromips.c:7:1: error: ‘micromips’
attribute only applies to functions
__attribute((micromips)) int b; // expected-error {{attribute only
applies to functions}}
>> Modified: cfe/trunk/include/clang/Basic/AttrDocs.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrDocs.td?rev=303546&r1=303545&r2=303546&view=diff
>> ==
>> --- cfe/trunk/include/clang/Basic/AttrDocs.td (original)
>> +++ cfe/trunk/include/clang/Basic/AttrDocs.td Mon May 22 07:47:43 2017
>> @@ -1269,6 +1269,19 @@ The semantics are as follows:
>>}];
>> }
>>
>> +def MicroMipsDocs : Documentation {
>> + let Category = DocCatFunction;
>> + let Content = [{
>> +Clang supports the GNU style ``__attribute__((micromips))`` and
>> +``__attribute__((nomicromips))`` attributes on MIPS targets. These
>> attributes
>> +may be attached to a function definition and instructs the backend to
>> generate
>> +or not to generate microMIPS code for that function.
>> +
>> +These attributes override the -mmicromips and -mno-micromips options
>
> Please quote the command line options with ``.
Fixed at r303564.
>> Added: cfe/trunk/test/Sema/attr-micromips.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-micromips.c?rev=303546&view=auto
>> ==
>> --- cfe/trunk/test/Sema/attr-micromips.c (added)
>> +++ cfe/trunk/test/Sema/attr-micromips.c Mon May 22 07:47:43 2017
>> @@ -0,0 +1,15 @@
>> +// RUN: %clang_cc1 -triple mips-linux-gnu -fsyntax-only -verify %s
>> +
>> +__attribute__((nomicromips(0))) void foo1(); // expected-error
>> {{'nomicromips' attribute takes no arguments}}
>> +__attribute__((micromips(1))) void foo2();// expected-error
>> {{'micromips' attribute takes no arguments}}
>> +
>> +__attribute((nomicromips)) int a; // expected-error {{attribute only
>> applies to functions}}
>> +__attribute((micromips)) int b; // expected-error {{attribute only
>> applies to functions}}
>> +
>> +__attribute__((micromips,mips16)) void foo5(); // expected-error
>> {{'micromips' and 'mips16' attributes are not compatible}} \
>> +// expected-note
>> {{conflicting attribute is here}}
>> +__attribute__((mips16,micromips)) void foo6(); // expected-error
>> {{'mips16' and 'micromips' attributes are not compatible}} \
>> +// expected-note
>> {{conflicting attribute is here}}
>
> Can you also add a test like:
>
> __attribute__((mips16)) void foo9(void) __attribute__((micromips));
Fixed at r303565.
--
Simon Atanasyan
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33416: [clangd] Allow to use vfs::FileSystem for file accesses.
ilya-biryukov created this revision.
Herald added a subscriber: mgorny.
Custom vfs::FileSystem is currently used for unit tests.
This revision depends on https://reviews.llvm.org/D33397.
https://reviews.llvm.org/D33416
Files:
clangd/ClangdLSPServer.cpp
clangd/ClangdServer.cpp
clangd/ClangdServer.h
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/ClangdUnitStore.h
unittests/CMakeLists.txt
unittests/clangd/CMakeLists.txt
unittests/clangd/ClangdTests.cpp
Index: unittests/clangd/ClangdTests.cpp
===
--- /dev/null
+++ unittests/clangd/ClangdTests.cpp
@@ -0,0 +1,361 @@
+//===-- ClangdTes.cpp - Change namespace unit tests ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ClangdServer.h"
+#include "clang/Basic/VirtualFileSystem.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/Errc.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Regex.h"
+#include "gtest/gtest.h"
+#include
+#include
+#include
+#include
+
+namespace clang {
+namespace vfs {
+
+/// An implementation of vfs::FileSystem that only allows access to
+/// files and folders inside a set of whitelisted directories.
+///
+/// FIXME(ibiryukov): should it also emulate access to parents of whitelisted
+/// directories with only whitelisted contents?
+class FilteredFileSystem : public vfs::FileSystem {
+public:
+ /// The paths inside \p WhitelistedDirs should be absolute
+ FilteredFileSystem(std::vector WhitelistedDirs,
+ IntrusiveRefCntPtr InnerFS)
+ : WhitelistedDirs(std::move(WhitelistedDirs)), InnerFS(InnerFS) {
+assert(std::all_of(WhitelistedDirs.begin(), WhitelistedDirs.end(),
+ [](const std::string &Path) -> bool {
+ return llvm::sys::path::is_absolute(Path);
+ }) &&
+ "Not all WhitelistedDirs are absolute");
+ }
+
+ virtual llvm::ErrorOr status(const Twine &Path) {
+if (!isInsideWhitelistedDir(Path))
+ return llvm::errc::no_such_file_or_directory;
+return InnerFS->status(Path);
+ }
+
+ virtual llvm::ErrorOr>
+ openFileForRead(const Twine &Path) {
+if (!isInsideWhitelistedDir(Path))
+ return llvm::errc::no_such_file_or_directory;
+return InnerFS->openFileForRead(Path);
+ }
+
+ llvm::ErrorOr>
+ getBufferForFile(const Twine &Name, int64_t FileSize = -1,
+ bool RequiresNullTerminator = true,
+ bool IsVolatile = false) {
+if (!isInsideWhitelistedDir(Name))
+ return llvm::errc::no_such_file_or_directory;
+return InnerFS->getBufferForFile(Name, FileSize, RequiresNullTerminator,
+ IsVolatile);
+ }
+
+ virtual directory_iterator dir_begin(const Twine &Dir, std::error_code &EC) {
+if (!isInsideWhitelistedDir(Dir)) {
+ EC = llvm::errc::no_such_file_or_directory;
+ return directory_iterator();
+}
+return InnerFS->dir_begin(Dir, EC);
+ }
+
+ virtual std::error_code setCurrentWorkingDirectory(const Twine &Path) {
+return InnerFS->setCurrentWorkingDirectory(Path);
+ }
+
+ virtual llvm::ErrorOr getCurrentWorkingDirectory() const {
+return InnerFS->getCurrentWorkingDirectory();
+ }
+
+ bool exists(const Twine &Path) {
+if (!isInsideWhitelistedDir(Path))
+ return false;
+return InnerFS->exists(Path);
+ }
+
+ std::error_code makeAbsolute(SmallVectorImpl &Path) const {
+return InnerFS->makeAbsolute(Path);
+ }
+
+private:
+ bool isInsideWhitelistedDir(const Twine &InputPath) const {
+SmallString<128> Path;
+InputPath.toVector(Path);
+
+if (makeAbsolute(Path))
+ return false;
+
+for (const auto &Dir : WhitelistedDirs) {
+ if (Path.startswith(Dir))
+return true;
+}
+return false;
+ }
+
+ std::vector WhitelistedDirs;
+ IntrusiveRefCntPtr InnerFS;
+};
+
+/// Create a vfs::FileSystem that has access only to temporary directories
+/// (obtained by calling system_temp_directory).
+IntrusiveRefCntPtr getTempOnlyFS() {
+ llvm::SmallString<128> TmpDir1;
+ llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/false, TmpDir1);
+ llvm::SmallString<128> TmpDir2;
+ llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true, TmpDir2);
+
+ std::vector TmpDirs;
+ TmpDirs.push_back(TmpDir1.str());
+ if (TmpDir2 != TmpDir2)
+TmpDirs.push_back(TmpDir2.str());
+ return new vfs::FilteredFileSystem(std::move(TmpDirs),
+ vfs::getRealFileSystem());
+}
+} // namespace vfs
+
+namespace clangd {
+namespace {
+
+class ErrorCheckingDiagConsumer : public DiagnosticsConsumer {
+public:
+ void onDiagnosticsReady(PathRef File,
+
[PATCH] D31646: [coroutines] Build GRO declaration and return GRO statement
GorNishanov added a comment. @rsmith barely audible ping https://reviews.llvm.org/D31646 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33395: [clangd] Split clangd into library+executable (mainly for unit tests).
ilya-biryukov marked an inline comment as done. ilya-biryukov added inline comments. Comment at: clangd/tool/ClangdMain.cpp:11 +#include "../ClangdLSPServer.h" +#include "../JSONRPCDispatcher.h" #include "llvm/Support/CommandLine.h" krasimir wrote: > I'd suggest to not have parent directory includes, but add them to the cmake > file as in other extra tools, like in: > https://github.com/llvm-mirror/clang-tools-extra/blob/master/clang-query/tool/CMakeLists.txt Done. I've actually followed a pattern of CMakeLists.txt from tool-extra, albeit a different one :-) https://github.com/llvm-mirror/clang-tools-extra/blob/master/clang-tidy/tool/ClangTidyMain.cpp https://reviews.llvm.org/D33395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33333: Emit warning when throw exception in destruct or dealloc functions which has a (possible implicit) noexcept specifier
jyu2 added a comment. In https://reviews.llvm.org/D3#760431, @aaron.ballman wrote: > In https://reviews.llvm.org/D3#760419, @jyu2 wrote: > > > In https://reviews.llvm.org/D3#760149, @aaron.ballman wrote: > > > > > As an FYI, there is a related check currently under development in > > > clang-tidy; we probably should not duplicate this functionality in both > > > places. See https://reviews.llvm.org/D19201 for the other review. > > > > > > To my understanding, clang-tidy is kind of source check tool. It does more > > intensive checking than compiler. > > My purpose here is to emit minimum warning form compiler, in case, > > clang-tidy is not used. > > > Sort of correct. clang-tidy doesn't necessarily do more intensive checking > than the compiler. It's more an AST matching source checking tool for checks > that are either expensive or have a higher false-positive rate than we'd want > to see in the frontend. > > I think that this particular check should probably be in the frontend, like > you're doing. My biggest concern is that we do not duplicate functionality > between the two tools. https://reviews.llvm.org/D19201 has has a considerable > amount of review, so you should look at the test cases there and ensure you > are handling them (including the fixits). Hopefully your patch ends up > covering all of the functionality in the clang-tidy patch. > > > In https://reviews.llvm.org/D3#760353, @Prazek wrote: > > > >> Could you add similar tests as the ones that Stanislaw provied in his > >> patch? > >> Like the one with checking if throw is catched, or the conditional > >> noexcept (by a macro, etc) > > > > > > Good idea! Could add “marco” test for this. But I am not sure compiler > > want to add check for throw caught by different catch handler. Because at > > compile time, compiler can not statically determine which catch handler > > will be used to caught the exception on all time. I would think that is > > pragma's responsibility. > > > > For example: > > > > If (a) throw A else throw B; > > > > > > > > My main concern there is implicit noexcept gets set by compiler, which > > cause runtime to termination. As I said, I don't think checking throw type matching catch handler type is compiler's job. I'd like either silence all warning for that. What do you think? https://reviews.llvm.org/D3 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax
mharoush marked an inline comment as done. mharoush added inline comments. Comment at: include/llvm/MC/MCParser/MCAsmParser.h:64 unsigned &Offset) = 0; + virtual bool EvaluateLookupAsEnum(void *LookupResult,int64_t &Result) = 0; }; rnk wrote: > It would be cleaner if InlineAsmIdentifierInfo simply contained an APInt > indicating that the identifier in question was an enum, or other integral > constant expression. Less callbacks to implement. Done Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:722 + bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End, +bool &ReplaceEnumIdentifier); std::unique_ptr rnk wrote: > Please try to eliminate the need for this extra boolean out parameter. This flag is used in case we try to compile something like mov edx, A+6 ( when A is defined as ConstantEnum ) the original condition (Line:1905) will skip the rewrite since we have an identifier as the first Token, however if we try to compile 6+A it will be rewritten as expected. I tried to solve this in different ways, I got either the exact opposite (A+6 was replaced and 6+A wasn't) or a collision of rewrites when trying to preform other forms of replacements and leaving this section intact. I can perhaps change the way we handle general expressions to the same way we handle them under parseIntelBracExpression, meaning use the first iteration of X86AsmParser to reformat the string (write imm prefixes and replace identifiers when needed) then refactor the string to its canonical form on the second pass. In short this was the simplest solution that worked without regression, maybe you have a better idea? If the issue is the method signature pollution I can move this flag into the SM class as a member to indicate a rewrite is needed, however since this flag and method are limited to this context alone, I am not sure if the overhead is wanted in such case . Repository: rL LLVM https://reviews.llvm.org/D33278 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax
mharoush updated this revision to Diff 99778.
mharoush added a comment.
Using identifier info to pass enum information.
Repository:
rL LLVM
https://reviews.llvm.org/D33278
Files:
include/llvm/MC/MCParser/MCAsmParser.h
lib/Target/X86/AsmParser/X86AsmParser.cpp
Index: lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -718,7 +718,8 @@
ParseIntelSegmentOverride(unsigned SegReg, SMLoc Start, unsigned Size);
std::unique_ptr ParseRoundingModeOp(SMLoc Start, SMLoc End);
bool ParseIntelNamedOperator(StringRef Name, IntelExprStateMachine &SM);
- bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End);
+ bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End,
+bool &ReplaceEnumIdentifier);
std::unique_ptr
ParseIntelBracExpression(unsigned SegReg, SMLoc Start, int64_t ImmDisp,
bool isSymbol, unsigned Size);
@@ -1306,8 +1307,9 @@
return false;
return true;
}
-
-bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End) {
+bool X86AsmParser::ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End,
+bool &ReplaceEnumIdentifier) {
+ ReplaceEnumIdentifier = false;
MCAsmParser &Parser = getParser();
const AsmToken &Tok = Parser.getTok();
@@ -1368,11 +1370,40 @@
PrevTK == AsmToken::RBrac) {
return false;
} else {
-InlineAsmIdentifierInfo &Info = SM.getIdentifierInfo();
+InlineAsmIdentifierInfo Info;
+Info.clear();
if (ParseIntelIdentifier(Val, Identifier, Info,
/*Unevaluated=*/false, End))
return true;
-SM.onIdentifierExpr(Val, Identifier);
+// Check if the parsed identifier was a constant Integer. Here we
+// assume Val is of type MCConstantExpr only when it is safe to replace
+// the identifier with its constant value.
+if (const MCConstantExpr *CE =
+dyn_cast_or_null(Val)) {
+ StringRef ErrMsg;
+ // SM should treat the value as it would an explicit integer in the
+ // expression.
+ if(SM.onInteger(CE->getValue(), ErrMsg))
+return Error(IdentLoc, ErrMsg);
+ // In case we are called on a bracketed expression,
+ if (isParsingInlineAsm() && SM.getAddImmPrefix()) {
+// A single rewrite of the integer value is preformed for each enum
+// identifier. This is only done when we are inside a bracketed
+// expression in order to match the behavior for the equivalent
+// integer tokens.
+size_t Len = End.getPointer() - IdentLoc.getPointer();
+InstInfo->AsmRewrites->emplace_back(AOK_Imm, IdentLoc,Len,
+CE->getValue());
+break;
+ }
+ // Set force rewrite flag only when not bracketed expression.
+ ReplaceEnumIdentifier = true;
+} else {
+ // Notify the SM a variable identifier was found.
+ InlineAsmIdentifierInfo &SMInfo = SM.getIdentifierInfo();
+ SMInfo = Info;
+ SM.onIdentifierExpr(Val, Identifier);
+}
}
break;
}
@@ -1452,7 +1483,8 @@
// may have already parsed an immediate displacement before the bracketed
// expression.
IntelExprStateMachine SM(ImmDisp, /*StopOnLBrac=*/false, /*AddImmPrefix=*/true);
- if (ParseIntelExpression(SM, End))
+ bool ReplaceEnumIdentifier;
+ if (ParseIntelExpression(SM, End, ReplaceEnumIdentifier))
return nullptr;
const MCExpr *Disp = nullptr;
@@ -1559,7 +1591,15 @@
// failed parsing.
assert((End.getPointer() == EndPtr || !Result) &&
"frontend claimed part of a token?");
-
+
+ // Check if the search yielded a constant integer (enum identifier).
+ if (Result && Info.IsConstEnum) {
+// By creating MCConstantExpr we let the user of Val know it is safe
+// to use as an explicit constant with value = ConstVal.
+Val = MCConstantExpr::create(Info.ConstIntValue.getSExtValue(),
+ getParser().getContext());
+return false;
+ }
// If the identifier lookup was unsuccessful, assume that we are dealing with
// a label.
if (!Result) {
@@ -1852,18 +1892,21 @@
AsmToken StartTok = Tok;
IntelExprStateMachine SM(/*Imm=*/0, /*StopOnLBrac=*/true,
/*AddImmPrefix=*/false);
- if (ParseIntelExpression(SM, End))
+ // The parsed expression may contain enum identifier tokens which we must
+ // replace, ReplaceEnumIdentifier flag lets us know when to force rewrite.
+ bool ReplaceEnumIdentifier;
+ if (ParseIntelExpression(SM, End, ReplaceEnumIdentifier))
return nullptr;
-
bool isSymbol = SM.getSym() &&
[PATCH] D32896: [OpenCL] Make CLK_NULL_RESERVE_ID invalid reserve id.
yaxunl added inline comments. Comment at: lib/Headers/opencl-c.h:16020 +// The macro CLK_NULL_RESERVE_ID refers to an invalid reservation ID. +#define CLK_NULL_RESERVE_ID (__builtin_astype((void *)0, reserve_id_t)) bool __ovld is_valid_reserve_id(reserve_id_t reserve_id); bader wrote: > yaxunl wrote: > > bader wrote: > > > yaxunl wrote: > > > > yaxunl wrote: > > > > > bader wrote: > > > > > > yaxunl wrote: > > > > > > > Anastasia wrote: > > > > > > > > echuraev wrote: > > > > > > > > > yaxunl wrote: > > > > > > > > > > Anastasia wrote: > > > > > > > > > > > yaxunl wrote: > > > > > > > > > > > > Anastasia wrote: > > > > > > > > > > > > > Looks good from my side. > > > > > > > > > > > > > > > > > > > > > > > > > > @yaxunl , since you originally committed this. Could > > > > > > > > > > > > > you please verify that changing from `SIZE_MAX` to > > > > > > > > > > > > > `0` would be fine. > > > > > > > > > > > > > > > > > > > > > > > > > > Btw, we have a similar definition for > > > > > > > > > > > > > `CLK_NULL_EVENT`. > > > > > > > > > > > > `__PIPE_RESERVE_ID_VALID_BIT` is implementation detail > > > > > > > > > > > > and not part of the spec. I would suggest to remove it > > > > > > > > > > > > from this header file. > > > > > > > > > > > > > > > > > > > > > > > > The spec only requires CLK_NULL_RESERVE_ID to be > > > > > > > > > > > > defined but does not define its value. Naturally a > > > > > > > > > > > > valid id starts from 0 and increases. I don't see > > > > > > > > > > > > significant advantage to change CLK_NULL_RESERVE_ID > > > > > > > > > > > > from __SIZE_MAX to 0. > > > > > > > > > > > > > > > > > > > > > > > > Is there any reason that this change is needed? > > > > > > > > > > > I don't see issues to commit things outside of spec as > > > > > > > > > > > soon as they prefixed properly with "__". But I agree it > > > > > > > > > > > would be nice to see if it's any useful and what the > > > > > > > > > > > motivation is for having different implementation. > > > > > > > > > > For `__PIPE_RESERVE_ID_VALID_BIT`, it assumes that the > > > > > > > > > > implementation uses one specific bit of a reserve id to > > > > > > > > > > indicate that the reserve id is valid. Not all > > > > > > > > > > implementations assume that. Actually I am curious why that > > > > > > > > > > is needed too. > > > > > > > > > About `CLK_NULL_RESERVE_ID`: we check that reserve id is > > > > > > > > > valid if significant bit equal to one. `CLK_NULL_RESERVE_ID > > > > > > > > > refers to an invalid reservation, so if `CLK_NULL_RESERVE_ID > > > > > > > > > equal to 0, we can be sure that significant bit doesn't equal > > > > > > > > > to 1 and it is invalid reserve id. Also it is more obviously > > > > > > > > > if CLK_**NULL**_RESERVE_ID is equal to 0. > > > > > > > > > > > > > > > > > > What about `__PIPE_RESERVE_ID_VALID_BIT`: As I understand > > > > > > > > > previous implementation also assumes that one specific bit > > > > > > > > > was of a reverse id was used to indicate that the reserve id > > > > > > > > > is valid. So, we just increased reserve id size by one bit on > > > > > > > > > 32-bit platforms and by 33 bits on 64-bit platforms. > > > > > > > > It is more logical to me that `CLK_NULL_RESERVE_ID` is 0, but > > > > > > > > spec doesn't define it of course. > > > > > > > In our implementation, valid reserve id starts at 0 and > > > > > > > increasing linearly until `__SIZE_MAX-1`. This change will break > > > > > > > our implementation. > > > > > > > > > > > > > > However, we can modify our implementation to adopt this change > > > > > > > since it brings about benefits overall. > > > > > > Ideally it would be great to have unified implementation, but we > > > > > > can define device specific value for CLK_NULL_RESERVE_ID by using > > > > > > ifdef directive. > > > > > How about > > > > > > > > > > ``` > > > > > __attribute__((const)) size_t __clk_null_reserve_id(); > > > > > #define CLK_NULL_RESERVE_ID __clk_null_reserve_id() > > > > > > > > > > ``` > > > > > I think the spec does not require it to be compile time constant. > > > > > Then each library can implement its own __clk_null_reserve_id() > > > > > whereas the IR is target independent. > > > > Or we only do this for SPIR and define it as target specific value for > > > > other targets. > > > Defining CLK_NULL_RESERVE_ID as a function call should also work, but > > > IMHO compile time constant is preferable option. > > > I don't think making it compile time constant for SPIR only makes sense > > > to me - in this case we can use constant for all targets. > > > > > > How about following approach: use 0 by default and allow other targets > > > re-define CLK_NULL_RESERVE_ID value. > > > > > > ``` > > > #ifndef CLK_NULL_RESERVE_ID > > > #define CLK_NULL_RESERVE_ID 0 > > > #endif // CLK_NULL_RESERVE_ID > > > ``` > > > > > > If CLK_NULL_RESERVE_ID defined via -D comm
[PATCH] D33277: [Clang][x86][Inline Asm] - Enum support for MS syntax
mharoush updated this revision to Diff 99780.
mharoush added a comment.
removed functions and dropped an irrelevant test case.
Repository:
rL LLVM
https://reviews.llvm.org/D33277
Files:
lib/Sema/SemaStmtAsm.cpp
test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -662,11 +662,21 @@
}
fillInlineAsmTypeInfo(Context, T, Info);
-
+
+ Expr *Res = Result.get();
// We can work with the expression as long as it's not an r-value.
- if (!Result.get()->isRValue())
+ if (!Res->isRValue()) {
Info.IsVarDecl = true;
+return Result;
+ }
+ Expr::EvalResult EvlResult;
+ // Try to evaluate the identifier as enum constant
+ if (isa(Res->getType()) && Res->EvaluateAsRValue(EvlResult,
+ getASTContext())) {
+Info.ConstIntValue = EvlResult.Val.getInt();
+Info.IsConstEnum = true;
+ }
return Result;
}
Index: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
===
--- test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
+++ test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
@@ -0,0 +1,44 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -fasm-blocks -emit-llvm -o - | FileCheck %s
+namespace x {
+enum { A = 12 };
+namespace y {
+enum { A = 17 };
+}
+}
+
+void x86_enum_namespaces() {
+ enum { A = 1 };
+ // CHECK: mov eax, $$12
+ __asm mov eax, x::A
+ // CHECK: mov eax, $$17
+ __asm mov eax, x::y::A
+ // CHECK: mov eax, $$1
+ __asm mov eax, A
+}
+
+void x86_enum_arithmethic() {
+ enum { A = 1, B };
+ // CHECK: mov eax, $$21
+ __asm mov eax, (A + 9) * 2 + A
+ // CHECK: mov eax, $$4
+ __asm mov eax, A << 2
+ // CHECK: mov eax, $$2
+ __asm mov eax, B & 3
+ // CHECK: mov eax, $$5
+ __asm mov eax, 3 + (B & 3)
+ // CHECK: mov eax, $$8
+ __asm mov eax, 2 << A * B
+}
+
+void x86_enum_mem() {
+ int arr[4];
+ enum { A = 4, B };
+
+ // CHECK: mov eax, [($$12 + $$9) + $$4 * $$5 + $$3 + $$3 + eax]
+ __asm { mov eax, [(x::A + 9) + A * B + 3 + 3 + eax] }
+ // CHECK: mov eax, dword ptr $$4$0
+ __asm { mov eax, dword ptr [arr + A] }
+ // CHECK: mov eax, dword ptr $$8$0
+ __asm { mov eax, dword ptr A[arr + A] }
+}
\ No newline at end of file
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -662,11 +662,21 @@
}
fillInlineAsmTypeInfo(Context, T, Info);
-
+
+ Expr *Res = Result.get();
// We can work with the expression as long as it's not an r-value.
- if (!Result.get()->isRValue())
+ if (!Res->isRValue()) {
Info.IsVarDecl = true;
+return Result;
+ }
+ Expr::EvalResult EvlResult;
+ // Try to evaluate the identifier as enum constant
+ if (isa(Res->getType()) && Res->EvaluateAsRValue(EvlResult,
+ getASTContext())) {
+Info.ConstIntValue = EvlResult.Val.getInt();
+Info.IsConstEnum = true;
+ }
return Result;
}
Index: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
===
--- test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
+++ test/CodeGen/x86-ms-inline-asm-enum_feature.cpp
@@ -0,0 +1,44 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -fasm-blocks -emit-llvm -o - | FileCheck %s
+namespace x {
+enum { A = 12 };
+namespace y {
+enum { A = 17 };
+}
+}
+
+void x86_enum_namespaces() {
+ enum { A = 1 };
+ // CHECK: mov eax, $$12
+ __asm mov eax, x::A
+ // CHECK: mov eax, $$17
+ __asm mov eax, x::y::A
+ // CHECK: mov eax, $$1
+ __asm mov eax, A
+}
+
+void x86_enum_arithmethic() {
+ enum { A = 1, B };
+ // CHECK: mov eax, $$21
+ __asm mov eax, (A + 9) * 2 + A
+ // CHECK: mov eax, $$4
+ __asm mov eax, A << 2
+ // CHECK: mov eax, $$2
+ __asm mov eax, B & 3
+ // CHECK: mov eax, $$5
+ __asm mov eax, 3 + (B & 3)
+ // CHECK: mov eax, $$8
+ __asm mov eax, 2 << A * B
+}
+
+void x86_enum_mem() {
+ int arr[4];
+ enum { A = 4, B };
+
+ // CHECK: mov eax, [($$12 + $$9) + $$4 * $$5 + $$3 + $$3 + eax]
+ __asm { mov eax, [(x::A + 9) + A * B + 3 + 3 + eax] }
+ // CHECK: mov eax, dword ptr $$4$0
+ __asm { mov eax, dword ptr [arr + A] }
+ // CHECK: mov eax, dword ptr $$8$0
+ __asm { mov eax, dword ptr A[arr + A] }
+}
\ No newline at end of file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33395: [clangd] Split clangd into library+executable (mainly for unit tests).
bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. looks good to me https://reviews.llvm.org/D33395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33277: [Clang][x86][Inline Asm] - Enum support for MS syntax
mharoush marked an inline comment as done.
mharoush added inline comments.
Comment at: lib/Parse/ParseStmtAsm.cpp:100
+ // result of a call to LookupInlineAsmIdentifier.
+ bool EvaluateLookupAsEnum(void *LookupResult, int64_t &Result) {
+if (!LookupResult) return false;
rnk wrote:
> Please format this, clang-format will do the job
Revised
Comment at: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp:12
+ const int a = 0;
+ // CHECK-NOT: mov eax, [$$0]
+ __asm mov eax, [a]
rnk wrote:
> Use CHECK-LABEL, CHECK, and CHECK-SAME the way that the existing
> ms-inline-asm.c tests do so that this test is easier to debug when it fails.
This test case was just meant verify that other Integer constants are not
folded since we get a different behavior for statements such as mov eax, [a].
#---
In this example X86AsmParser regards the address of the variable 'a' and not
its value i.e. we end up with the value of 'a' in eax (loaded from the stack)
and not with the value pointed by the const int value of 'a' as its address.
---#
I can clarify the intention in a comment or completely remove the test case
since this isn't really required here.
Repository:
rL LLVM
https://reviews.llvm.org/D33277
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33368: [libcxxabi][demangler] Fix a crash in the demangler
erik.pilkington added inline comments. Comment at: src/cxa_demangle.cpp:3036 break; -if (db.names.size() < 2) +if (k1 <= k0) return first; compnerd wrote: > I'm not sure how `k1` can be `<` than `k0`. Isn't this effectively always > going down the `==` path? If I'm just mistaken, then I think that this needs > a comment. I agree that it should be impossible, but the previous version handled this case with the `if (db.names.size() < 2)` check, when really `db.names.size()` can only be 1 or greater. I think it makes more sense to be paranoid here, I trust this file about as far as I can throw it (And I can't throw it very far, its not a tangible object). https://reviews.llvm.org/D33368 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r303568 - [index] Index the default template parameter values
Author: arphaman
Date: Mon May 22 11:50:54 2017
New Revision: 303568
URL: http://llvm.org/viewvc/llvm-project?rev=303568&view=rev
Log:
[index] Index the default template parameter values
rdar://32323724
Modified:
cfe/trunk/lib/Index/IndexDecl.cpp
cfe/trunk/test/Index/Core/index-source.cpp
Modified: cfe/trunk/lib/Index/IndexDecl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexDecl.cpp?rev=303568&r1=303567&r2=303568&view=diff
==
--- cfe/trunk/lib/Index/IndexDecl.cpp (original)
+++ cfe/trunk/lib/Index/IndexDecl.cpp Mon May 22 11:50:54 2017
@@ -63,6 +63,17 @@ public:
case TemplateArgument::Type:
IndexCtx.indexTypeSourceInfo(LocInfo.getAsTypeSourceInfo(), Parent, DC);
break;
+case TemplateArgument::Template:
+case TemplateArgument::TemplateExpansion:
+ IndexCtx.indexNestedNameSpecifierLoc(TALoc.getTemplateQualifierLoc(),
+ Parent, DC);
+ if (const TemplateDecl *TD = TALoc.getArgument()
+ .getAsTemplateOrTemplatePattern()
+ .getAsTemplateDecl()) {
+if (const NamedDecl *TTD = TD->getTemplatedDecl())
+ IndexCtx.handleReference(TTD, TALoc.getTemplateNameLoc(), Parent,
DC);
+ }
+ break;
default:
break;
}
@@ -610,8 +621,43 @@ public:
return true;
}
+ static bool shouldIndexTemplateParameterDefaultValue(const NamedDecl *D) {
+if (!D)
+ return false;
+// We want to index the template parameters only once when indexing the
+// canonical declaration.
+if (const auto *FD = dyn_cast(D))
+ return FD->getCanonicalDecl() == FD;
+else if (const auto *TD = dyn_cast(D))
+ return TD->getCanonicalDecl() == TD;
+else if (const auto *VD = dyn_cast(D))
+ return VD->getCanonicalDecl() == VD;
+return true;
+ }
+
bool VisitTemplateDecl(const TemplateDecl *D) {
// FIXME: Template parameters.
+
+// Index the default values for the template parameters.
+const NamedDecl *Parent = D->getTemplatedDecl();
+if (D->getTemplateParameters() &&
+shouldIndexTemplateParameterDefaultValue(Parent)) {
+ const TemplateParameterList *Params = D->getTemplateParameters();
+ for (const NamedDecl *TP : *Params) {
+if (const auto *TTP = dyn_cast(TP)) {
+ if (TTP->hasDefaultArgument())
+IndexCtx.indexTypeSourceInfo(TTP->getDefaultArgumentInfo(),
Parent);
+} else if (const auto *NTTP = dyn_cast(TP)) {
+ if (NTTP->hasDefaultArgument())
+IndexCtx.indexBody(NTTP->getDefaultArgument(), Parent);
+} else if (const auto *TTPD = dyn_cast(TP)) {
+ if (TTPD->hasDefaultArgument())
+handleTemplateArgumentLoc(TTPD->getDefaultArgument(), Parent,
+ /*DC=*/nullptr);
+}
+ }
+}
+
return Visit(D->getTemplatedDecl());
}
Modified: cfe/trunk/test/Index/Core/index-source.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Core/index-source.cpp?rev=303568&r1=303567&r2=303568&view=diff
==
--- cfe/trunk/test/Index/Core/index-source.cpp (original)
+++ cfe/trunk/test/Index/Core/index-source.cpp Mon May 22 11:50:54 2017
@@ -374,3 +374,62 @@ struct DeletedMethods {
// CHECK: [[@LINE-3]]:24 | struct/C++ | DeletedMethods | c:@S@DeletedMethods |
| Ref,RelCont | rel: 1
// CHECK: [[@LINE-4]]:3 | struct/C++ | DeletedMethods | c:@S@DeletedMethods |
| Ref,RelCont | rel: 1
};
+
+namespace ns2 {
+template struct ACollectionDecl { };
+}
+
+template |
Ref,RelCont | rel: 1
+// CHECK-NEXT: RelCont | TemplateDefaultValues |
c:@ST>3#T#NI#t>1#T@TemplateDefaultValues
+ int x = Record::C,
+// CHECK: [[@LINE-1]]:26 | static-property/C++ | C | c:@S@Record@C |
__ZN6Record1CE | Ref,Read,RelCont | rel: 1
+// CHECK-NEXT: RelCont | TemplateDefaultValues |
c:@ST>3#T#NI#t>1#T@TemplateDefaultValues
+// CHECK: [[@LINE-3]]:18 | struct/C++ | Record | c:@S@Record | |
Ref,RelCont | rel: 1
+ template class Collection = ns2::ACollectionDecl>
+// CHECK: [[@LINE-1]]:49 | namespace/C++ | ns2 | c:@N@ns2 | |
Ref,RelCont | rel: 1
+// CHECK-NEXT: RelCont | TemplateDefaultValues |
c:@ST>3#T#NI#t>1#T@TemplateDefaultValues
+// CHECK: [[@LINE-3]]:54 | struct(Gen)/C++ | ACollectionDecl |
c:@N@ns2@ST>1#T@ACollectionDecl | | Ref,RelCont | rel: 1
+// CHECK-NEXT: RelCont | TemplateDefaultValues |
c:@ST>3#T#NI#t>1#T@TemplateDefaultValues
+struct TemplateDefaultValues { };
+
+template |
Ref,RelCont | rel: 1
+ int x = sizeof(Cls)>
+// CHECK: [[@LINE-1]]:25 | class/C++ | Cls | c:@S@Cls | |
Ref,RelCont | rel: 1
+void functionTemplateDefaultValues() { }
+
+namespace ensureDefaultTemplateParamsAreRecordedOnce {
+
+template
+// CHECK: [[@LINE-1]]:23 | class/C++ |
[PATCH] D33278: [LLVM][x86][Inline Asm] - Enum support for MS syntax
rnk added inline comments.
Comment at: include/llvm/MC/MCParser/MCAsmParser.h:64
unsigned &Offset) = 0;
+ virtual bool EvaluateLookupAsEnum(void *LookupResult,int64_t &Result) = 0;
};
mharoush wrote:
> rnk wrote:
> > It would be cleaner if InlineAsmIdentifierInfo simply contained an APInt
> > indicating that the identifier in question was an enum, or other integral
> > constant expression. Less callbacks to implement.
> Done
Thanks! This is better.
Comment at: include/llvm/MC/MCParser/MCAsmParser.h:40
void *OpDecl;
bool IsVarDecl;
unsigned Length, Size, Type;
Please group the booleans to reduce padding. Ideally, make this an enum
something like:
```
enum IdKind : uint8_t {
Variable,
Function,
ConstInt,
};
IdKind Kind;
```
This is smaller and prevents nonsense states.
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:1382
+if (const MCConstantExpr *CE =
+dyn_cast_or_null(Val)) {
+ StringRef ErrMsg;
Please use clang-format here and elsewhere
Comment at: lib/Target/X86/AsmParser/X86AsmParser.cpp:722
+ bool ParseIntelExpression(IntelExprStateMachine &SM, SMLoc &End,
+bool &ReplaceEnumIdentifier);
std::unique_ptr
mharoush wrote:
> rnk wrote:
> > Please try to eliminate the need for this extra boolean out parameter.
> This flag is used in case we try to compile something like mov edx, A+6 (
> when A is defined as ConstantEnum ) the original condition (Line:1905) will
> skip the rewrite since we have an identifier as the first Token, however if
> we try to compile 6+A it will be rewritten as expected.
>
> I tried to solve this in different ways, I got either the exact opposite (A+6
> was replaced and 6+A wasn't) or a collision of rewrites when trying to
> preform other forms of replacements and leaving this section intact.
>
> I can perhaps change the way we handle general expressions to the same way we
> handle them under parseIntelBracExpression, meaning use the first iteration
> of X86AsmParser to reformat the string (write imm prefixes and replace
> identifiers when needed) then refactor the string to its canonical form on
> the second pass.
>
> In short this was the simplest solution that worked without regression, maybe
> you have a better idea?
>
> If the issue is the method signature pollution I can move this flag into the
> SM class as a member to indicate a rewrite is needed, however since this flag
> and method are limited to this context alone, I am not sure if the overhead
> is wanted in such case .
I suspect there is a way to simplify the code so that this corner case no
longer exists. I don't really have time to dig through the test cases to come
up with it, but please make an effort.
Repository:
rL LLVM
https://reviews.llvm.org/D33278
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33277: [Clang][x86][Inline Asm] - Enum support for MS syntax
rnk added inline comments. Comment at: lib/Sema/SemaStmtAsm.cpp:674 + Expr::EvalResult EvlResult; + // Try to evaluate the identifier as enum constant + if (isa(Res->getType()) && Res->EvaluateAsRValue(EvlResult, Please add a comment explaining that non-enum integer constants should not be folded. I expected that MSVC would fold constexpr integers here, but it does not, so that's worth noting. Comment at: test/CodeGen/x86-ms-inline-asm-enum_feature.cpp:12 + const int a = 0; + // CHECK-NOT: mov eax, [$$0] + __asm mov eax, [a] mharoush wrote: > rnk wrote: > > Use CHECK-LABEL, CHECK, and CHECK-SAME the way that the existing > > ms-inline-asm.c tests do so that this test is easier to debug when it fails. > This test case was just meant verify that other Integer constants are not > folded since we get a different behavior for statements such as mov eax, [a]. > #--- > In this example X86AsmParser regards the address of the variable 'a' and not > its value i.e. we end up with the value of 'a' in eax (loaded from the stack) > and not with the value pointed by the const int value of 'a' as its address. > ---# > > I can clarify the intention in a comment or completely remove the test case > since this isn't really required here. The test case is fine and the intention is clear, I am just suggesting that you use more FileCheck features (CHECK-LABEL and CHECK-SAME) to make the test easier to debug when it fails in the future. Repository: rL LLVM https://reviews.llvm.org/D33277 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33170: [X86] Adding avx512_vpopcntdq feature set and its intrinsics
craig.topper accepted this revision. craig.topper added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/CodeGen/CGBuiltin.cpp:7526 +llvm::Type *ResultType = ConvertType(E->getType()); +llvm::Function *F = CGM.getIntrinsic(Intrinsic::ctpop, ResultType); +return Builder.CreateCall(F, Ops); oren_ben_simhon wrote: > oren_ben_simhon wrote: > > craig.topper wrote: > > > Why did the call to ConvertType come back? This code can just be > > > > > > > > > ``` > > > llvm::Function *F = CGM.getIntrinsic(Intrinsic::ctpop, Ops[0]->getType()); > > > return Builder.CreateCall(F, Ops); > > > ``` > > After checking it again, I reverted that change because AFAIK Ops[0] is the > > first argument and not the return type of the statement. > After checking it again, I reverted that change because AFAIK Ops[0] is the > first argument and not the return type of the statement. That's true, but for this intrinsic they are required to be the same type. But its not a big deal. Repository: rL LLVM https://reviews.llvm.org/D33170 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33272: Method loadFromCommandLine should be able to report errors
sepavloff updated this revision to Diff 99786.
sepavloff marked an inline comment as done.
sepavloff added a comment.
Addressed reviewer's notes.
https://reviews.llvm.org/D33272
Files:
include/clang/Tooling/CompilationDatabase.h
lib/Frontend/CreateInvocationFromCommandLine.cpp
lib/Tooling/CommonOptionsParser.cpp
lib/Tooling/CompilationDatabase.cpp
lib/Tooling/Tooling.cpp
unittests/Tooling/CompilationDatabaseTest.cpp
Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -504,29 +504,35 @@
TEST(ParseFixedCompilationDatabase, ReturnsNullOnEmptyArgumentList) {
int Argc = 0;
- std::unique_ptr Database(
- FixedCompilationDatabase::loadFromCommandLine(Argc, nullptr));
+ std::string ErrorMsg;
+ std::unique_ptr Database =
+ FixedCompilationDatabase::loadFromCommandLine(Argc, nullptr, ErrorMsg);
EXPECT_FALSE(Database);
+ EXPECT_EQ(ErrorMsg, "warning: no arguments specified\n");
EXPECT_EQ(0, Argc);
}
TEST(ParseFixedCompilationDatabase, ReturnsNullWithoutDoubleDash) {
int Argc = 2;
const char *Argv[] = { "1", "2" };
+ std::string ErrorMsg;
std::unique_ptr Database(
- FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
+ FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg));
EXPECT_FALSE(Database);
+ EXPECT_EQ(ErrorMsg, "warning: double dash not found\n");
EXPECT_EQ(2, Argc);
}
TEST(ParseFixedCompilationDatabase, ReturnsArgumentsAfterDoubleDash) {
int Argc = 5;
const char *Argv[] = {
"1", "2", "--\0no-constant-folding", "-DDEF3", "-DDEF4"
};
+ std::string ErrorMsg;
std::unique_ptr Database(
- FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
+ FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg));
ASSERT_TRUE((bool)Database);
+ ASSERT_TRUE(ErrorMsg.empty());
std::vector Result =
Database->getCompileCommands("source");
ASSERT_EQ(1ul, Result.size());
@@ -543,9 +549,11 @@
TEST(ParseFixedCompilationDatabase, ReturnsEmptyCommandLine) {
int Argc = 3;
const char *Argv[] = { "1", "2", "--\0no-constant-folding" };
- std::unique_ptr Database(
- FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
+ std::string ErrorMsg;
+ std::unique_ptr Database =
+ FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg);
ASSERT_TRUE((bool)Database);
+ ASSERT_TRUE(ErrorMsg.empty());
std::vector Result =
Database->getCompileCommands("source");
ASSERT_EQ(1ul, Result.size());
@@ -560,9 +568,11 @@
TEST(ParseFixedCompilationDatabase, HandlesPositionalArgs) {
const char *Argv[] = {"1", "2", "--", "-c", "somefile.cpp", "-DDEF3"};
int Argc = sizeof(Argv) / sizeof(char*);
- std::unique_ptr Database(
- FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
+ std::string ErrorMsg;
+ std::unique_ptr Database =
+ FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg);
ASSERT_TRUE((bool)Database);
+ ASSERT_TRUE(ErrorMsg.empty());
std::vector Result =
Database->getCompileCommands("source");
ASSERT_EQ(1ul, Result.size());
@@ -579,9 +589,11 @@
TEST(ParseFixedCompilationDatabase, HandlesArgv0) {
const char *Argv[] = {"1", "2", "--", "mytool", "somefile.cpp"};
int Argc = sizeof(Argv) / sizeof(char*);
- std::unique_ptr Database(
- FixedCompilationDatabase::loadFromCommandLine(Argc, Argv));
+ std::string ErrorMsg;
+ std::unique_ptr Database =
+ FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg);
ASSERT_TRUE((bool)Database);
+ ASSERT_TRUE(ErrorMsg.empty());
std::vector Result =
Database->getCompileCommands("source");
ASSERT_EQ(1ul, Result.size());
Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -260,6 +260,8 @@
Driver->setCheckInputsExist(false);
const std::unique_ptr Compilation(
Driver->BuildCompilation(llvm::makeArrayRef(Argv)));
+ if (!Compilation)
+return false;
const llvm::opt::ArgStringList *const CC1Args = getCC1Arguments(
&Diagnostics, Compilation.get());
if (!CC1Args) {
Index: lib/Tooling/CompilationDatabase.cpp
===
--- lib/Tooling/CompilationDatabase.cpp
+++ lib/Tooling/CompilationDatabase.cpp
@@ -27,6 +27,7 @@
#include "llvm/Option/Arg.h"
#include "llvm/Support/Host.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/raw_ostream.h"
#include
#include
using namespace clang;
@@ -150,23 +151,21 @@
// options.
class UnusedInputDiagConsumer : public DiagnosticConsumer {
public:
- UnusedInputDiagConsumer() : Other(nullptr) {}
-
- // Useful for debugging, chain diagnostics to another consumer after
- // recording for our own purposes.
- UnusedInputDiag
[PATCH] D33272: Method loadFromCommandLine should be able to report errors
sepavloff added a comment.
Thank you!
I put updated fix here. If it is OK, I'll commit it tomorrow.
Comment at: lib/Tooling/CompilationDatabase.cpp:292
+ if (Argc == 0) {
+ErrorMsg = "error: no arguments specified\n";
+return nullptr;
alexfh wrote:
> The lack of the arguments (or the `--` below) shouldn't be treated as an
> error (at least an error that is worth showing to the user). The caller will
> just move on to trying the next kind of a compilation database. The only
> actual errors we can get here may be coming from `stripPositionalArgs`.
>
> The caller should be modified accordingly (i.e. not output anything when both
> the return value is `nullptr` and `ErrorMsg` is empty).
What if error message contains a prefix that indicates if this is error or
warning? Errors could be printed and warnings ignored in
`CommonOptionsParser.cpp:122`. Such solution would allow clients to investigate
failures in more details.
https://reviews.llvm.org/D33272
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
