[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment
xgsa updated this revision to Diff 127267.
xgsa added a comment.
Review comments were applied.
https://reviews.llvm.org/D41326
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
test/clang-tidy/nolint-usage.cpp
test/clang-tidy/nolint.cpp
test/clang-tidy/nolintnextline.cpp
Index: test/clang-tidy/nolintnextline.cpp
===
--- test/clang-tidy/nolintnextline.cpp
+++ test/clang-tidy/nolintnextline.cpp
@@ -24,6 +24,18 @@
class C5 { C5(int i); };
+void f() {
+ int i;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'i' [clang-diagnostic-unused-variable]
+ // NOLINTNEXTLINE
+ int j;
+ // NOLINTNEXTLINE(clang-diagnostic-unused-variable)
+ int k;
+ // NOLINTNEXTLINE(clang-diagnostic-literal-conversion)
+ int l;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'l' [clang-diagnostic-unused-variable]
+}
+
// NOLINTNEXTLINE
class D { D(int i); };
@@ -44,6 +56,6 @@
// NOLINTNEXTLINE
MACRO_NOARG
-// CHECK-MESSAGES: Suppressed 8 warnings (8 NOLINT)
+// CHECK-MESSAGES: Suppressed 10 warnings (10 NOLINT)
-// RUN: %check_clang_tidy %s google-explicit-constructor %t --
+// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable %t -- -extra-arg=-Wunused-variable --
Index: test/clang-tidy/nolint.cpp
===
--- test/clang-tidy/nolint.cpp
+++ test/clang-tidy/nolint.cpp
@@ -30,6 +30,9 @@
int i;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'i' [clang-diagnostic-unused-variable]
int j; // NOLINT
+ int k; // NOLINT(clang-diagnostic-unused-variable)
+ int l; // NOLINT(clang-diagnostic-literal-conversion)
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'l' [clang-diagnostic-unused-variable]
}
#define MACRO(X) class X { X(int i); };
@@ -46,4 +49,4 @@
#define DOUBLE_MACRO MACRO(H) // NOLINT
DOUBLE_MACRO
-// CHECK-MESSAGES: Suppressed 12 warnings (12 NOLINT)
+// CHECK-MESSAGES: Suppressed 13 warnings (13 NOLINT)
Index: test/clang-tidy/nolint-usage.cpp
===
--- /dev/null
+++ test/clang-tidy/nolint-usage.cpp
@@ -0,0 +1,171 @@
+// RUN: %check_clang_tidy %s nolint-usage,google-explicit-constructor,misc-unused-parameters %t --
+
+// Case: NO_LINT on line with an error.
+class A1 { A1(int i); }; // NOLINT
+
+// Case: NO_LINT on line without errors.
+class A2 { explicit A2(int i); }; // NOLINT
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: there is no diagnostics on this line, the NOLINT comment is redundant
+
+// Case: NO_LINT for the specific check on line with an error on it.
+class A3 { A3(int i); }; // NOLINT(google-explicit-constructor)
+
+// Case: NO_LINT for the specific check on line with an error on another check.
+class A4 { A4(int i); }; // NOLINT(misc-unused-parameters)
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: there is no diagnostics for the 'misc-unused-parameters' check on this line, the NOLINT comment is redundant
+
+// Case: NO_LINT for the specific check on line without errors.
+class A5 { explicit A5(int i); }; // NOLINT(google-explicit-constructor)
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: there is no diagnostics for the 'google-explicit-constructor' check on this line, the NOLINT comment is redundant
+
+// Case: NO_LINT for unknown check on line with an error on some check.
+class A6 { A6(int i); }; // NOLINT(unknown-check)
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: unknown check name 'unknown-check' is specified for NOLINT comment
+
+// Case: NO_LINT for unknown check on line without errors.
+class A7 { explicit A7(int i); }; // NOLINT(unknown-check)
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: unknown check name 'unknown-check' is specified for NOLINT comment
+
+// Case: NO_LINT with not closed parenthesis without check names.
+class A8 { A8(int i); }; // NOLINT(
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: NOLINT comment has an opening parenthesis and no closing one, so all the diagnostics will be silenced for the line
+
+// Case: NO_LINT with not closed parenthesis with check names.
+class A9 { A9(int i); }; // NOLINT(unknown-check
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: NOLINT comment has an opening parenthesis and no closing one, so all the diagnostics will be silenced for the line
+
+// Case: NO_LINT with clang diagnostics
+int f() {
+ int i = 0; // NOLINT
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: there is no diagnostics on this line, the NOLINT comment is redundant
+ int j = 0; // NOLINT(clang-diagnostic-unused-variable)
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: there is no diagnostics for the 'clang-diagnostic-u
[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment
xgsa added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:339 std::unique_ptr OptionsProvider) : DiagEngine(nullptr), OptionsProvider(std::move(OptionsProvider)), + Profile(nullptr), xgsa wrote: > Eugene.Zelenko wrote: > > Please use default members initialization for DiagEngine and Profile. > Actually, it is not necessary, as unique_ptr is initialized with nullptr by > default. I have just removed initializing them here. Sorry, you are right, just ignore my previous comment. https://reviews.llvm.org/D41326 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment
xgsa updated this revision to Diff 127268.
xgsa added a comment.
Review comments applied.
https://reviews.llvm.org/D41326
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
test/clang-tidy/nolint-usage.cpp
test/clang-tidy/nolint.cpp
test/clang-tidy/nolintnextline.cpp
Index: test/clang-tidy/nolintnextline.cpp
===
--- test/clang-tidy/nolintnextline.cpp
+++ test/clang-tidy/nolintnextline.cpp
@@ -24,6 +24,18 @@
class C5 { C5(int i); };
+void f() {
+ int i;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'i' [clang-diagnostic-unused-variable]
+ // NOLINTNEXTLINE
+ int j;
+ // NOLINTNEXTLINE(clang-diagnostic-unused-variable)
+ int k;
+ // NOLINTNEXTLINE(clang-diagnostic-literal-conversion)
+ int l;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'l' [clang-diagnostic-unused-variable]
+}
+
// NOLINTNEXTLINE
class D { D(int i); };
@@ -44,6 +56,6 @@
// NOLINTNEXTLINE
MACRO_NOARG
-// CHECK-MESSAGES: Suppressed 8 warnings (8 NOLINT)
+// CHECK-MESSAGES: Suppressed 10 warnings (10 NOLINT)
-// RUN: %check_clang_tidy %s google-explicit-constructor %t --
+// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable %t -- -extra-arg=-Wunused-variable --
Index: test/clang-tidy/nolint.cpp
===
--- test/clang-tidy/nolint.cpp
+++ test/clang-tidy/nolint.cpp
@@ -30,6 +30,9 @@
int i;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'i' [clang-diagnostic-unused-variable]
int j; // NOLINT
+ int k; // NOLINT(clang-diagnostic-unused-variable)
+ int l; // NOLINT(clang-diagnostic-literal-conversion)
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unused variable 'l' [clang-diagnostic-unused-variable]
}
#define MACRO(X) class X { X(int i); };
@@ -46,4 +49,4 @@
#define DOUBLE_MACRO MACRO(H) // NOLINT
DOUBLE_MACRO
-// CHECK-MESSAGES: Suppressed 12 warnings (12 NOLINT)
+// CHECK-MESSAGES: Suppressed 13 warnings (13 NOLINT)
Index: test/clang-tidy/nolint-usage.cpp
===
--- /dev/null
+++ test/clang-tidy/nolint-usage.cpp
@@ -0,0 +1,171 @@
+// RUN: %check_clang_tidy %s nolint-usage,google-explicit-constructor,misc-unused-parameters %t --
+
+// Case: NO_LINT on line with an error.
+class A1 { A1(int i); }; // NOLINT
+
+// Case: NO_LINT on line without errors.
+class A2 { explicit A2(int i); }; // NOLINT
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: there is no diagnostics on this line, the NOLINT comment is redundant
+
+// Case: NO_LINT for the specific check on line with an error on it.
+class A3 { A3(int i); }; // NOLINT(google-explicit-constructor)
+
+// Case: NO_LINT for the specific check on line with an error on another check.
+class A4 { A4(int i); }; // NOLINT(misc-unused-parameters)
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: there is no diagnostics for the 'misc-unused-parameters' check on this line, the NOLINT comment is redundant
+
+// Case: NO_LINT for the specific check on line without errors.
+class A5 { explicit A5(int i); }; // NOLINT(google-explicit-constructor)
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: there is no diagnostics for the 'google-explicit-constructor' check on this line, the NOLINT comment is redundant
+
+// Case: NO_LINT for unknown check on line with an error on some check.
+class A6 { A6(int i); }; // NOLINT(unknown-check)
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: unknown check name 'unknown-check' is specified for NOLINT comment
+
+// Case: NO_LINT for unknown check on line without errors.
+class A7 { explicit A7(int i); }; // NOLINT(unknown-check)
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: unknown check name 'unknown-check' is specified for NOLINT comment
+
+// Case: NO_LINT with not closed parenthesis without check names.
+class A8 { A8(int i); }; // NOLINT(
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: NOLINT comment has an opening parenthesis and no closing one, so all the diagnostics will be silenced for the line
+
+// Case: NO_LINT with not closed parenthesis with check names.
+class A9 { A9(int i); }; // NOLINT(unknown-check
+// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: NOLINT comment has an opening parenthesis and no closing one, so all the diagnostics will be silenced for the line
+
+// Case: NO_LINT with clang diagnostics
+int f() {
+ int i = 0; // NOLINT
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: there is no diagnostics on this line, the NOLINT comment is redundant
+ int j = 0; // NOLINT(clang-diagnostic-unused-variable)
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: there is no diagnostics for the 'clang-diagnostic-unused
[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment
JVApen added a comment. I'm missing some documentation to understand the corner cases. How does this check behave with suppressed warnings for checks which ain't currently checked. (Using -no-... on a code base or suppressing the warnings via the pragmas) https://reviews.llvm.org/D41326 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41326: [clang-tidy] Added diagnostics about incorrect usage of NOLINT comment
xgsa added a comment. In https://reviews.llvm.org/D41326#957749, @JVApen wrote: > I'm missing some documentation to understand the corner cases. How does this > check behave with suppressed warnings for checks which ain't currently > checked. (Using -no-... on a code base or suppressing the warnings via the > pragmas) The check just gathers NOLINT comments and compares them with a list of reported diagnostics for the current configuration. Thus, it will report diagnostics on the NOLINT comments, which are added for the checks, which are disabled in current configuration (e.g. with option -check=-some_check). Note also that in spite NOLINT for clang warnings now works for clang-tidy, it doesn't work for clang itself, so I wouldn't recommended using something like "// NOLINT(clang-diagnostic-unused-variable)". As you mentioned, pragmas or -no-... options should be used instead. The check doesn't perform any processing of those things. https://reviews.llvm.org/D41326 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41217: [Concepts] Concept Specialization Expressions
saar.raz added inline comments.
Comment at: lib/Parse/ParseExpr.cpp:223
ExprResult Parser::ParseConstraintExpression() {
- // FIXME: this may erroneously consume a function-body as the braced
- // initializer list of a compound literal
- //
- // FIXME: this may erroneously consume a parenthesized rvalue reference
- // declarator as a parenthesized address-of-label expression
- ExprResult LHS(ParseCastExpression(/*isUnaryExpression=*/false));
- ExprResult Res(ParseRHSOfBinaryExpression(LHS, prec::LogicalOr));
-
+ ExprResult Res(ParseExpression());
+ if (Res.isUsable() && !Actions.CheckConstraintExpression(Res.get())) {
faisalv wrote:
> Are you sure this only accepts logical-or-epxressions? Unlike Hubert's
> initial implementation here, this seems to parse any expression (i.e
> including unparenthesized commas and assignments)? What am I missing?
>
Mm.. Good point, I was under the false assumption that this was equivalent -
I'll fix it back, thanks :)
Repository:
rC Clang
https://reviews.llvm.org/D41217
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40478: Added control flow architecture protection Flag
oren_ben_simhon added a comment. -mibt is currently in discussions with other compilers, any change will be uploaded to a different review. If you have more comments i will appreciate it. Repository: rL LLVM https://reviews.llvm.org/D40478 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r320942 - [ASTImporter] Support importing FunctionTemplateDecl and CXXDependentScopeMemberExpr
Author: a.sidorin
Date: Sun Dec 17 06:16:17 2017
New Revision: 320942
URL: http://llvm.org/viewvc/llvm-project?rev=320942&view=rev
Log:
[ASTImporter] Support importing FunctionTemplateDecl and
CXXDependentScopeMemberExpr
* Also introduces ImportTemplateArgumentListInfo facility (A. Sidorin)
Patch by Peter Szecsi!
Differential Revision: https://reviews.llvm.org/D38692
Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp
Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=320942&r1=320941&r2=320942&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Sun Dec 17 06:16:17 2017
@@ -134,12 +134,17 @@ namespace clang {
bool ImportTemplateArguments(const TemplateArgument *FromArgs,
unsigned NumFromArgs,
SmallVectorImpl &ToArgs);
+template
+bool ImportTemplateArgumentListInfo(const InContainerTy &Container,
+TemplateArgumentListInfo &ToTAInfo);
bool IsStructuralMatch(RecordDecl *FromRecord, RecordDecl *ToRecord,
bool Complain = true);
bool IsStructuralMatch(VarDecl *FromVar, VarDecl *ToVar,
bool Complain = true);
bool IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToRecord);
bool IsStructuralMatch(EnumConstantDecl *FromEC, EnumConstantDecl *ToEC);
+bool IsStructuralMatch(FunctionTemplateDecl *From,
+ FunctionTemplateDecl *To);
bool IsStructuralMatch(ClassTemplateDecl *From, ClassTemplateDecl *To);
bool IsStructuralMatch(VarTemplateDecl *From, VarTemplateDecl *To);
Decl *VisitDecl(Decl *D);
@@ -195,6 +200,7 @@ namespace clang {
ClassTemplateSpecializationDecl
*D);
Decl *VisitVarTemplateDecl(VarTemplateDecl *D);
Decl *VisitVarTemplateSpecializationDecl(VarTemplateSpecializationDecl *D);
+Decl *VisitFunctionTemplateDecl(FunctionTemplateDecl *D);
// Importing statements
DeclGroupRef ImportDeclGroup(DeclGroupRef DG);
@@ -280,6 +286,7 @@ namespace clang {
Expr *VisitCXXDeleteExpr(CXXDeleteExpr *E);
Expr *VisitCXXConstructExpr(CXXConstructExpr *E);
Expr *VisitCXXMemberCallExpr(CXXMemberCallExpr *E);
+Expr *VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E);
Expr *VisitExprWithCleanups(ExprWithCleanups *EWC);
Expr *VisitCXXThisExpr(CXXThisExpr *E);
Expr *VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr *E);
@@ -1247,6 +1254,18 @@ bool ASTNodeImporter::ImportTemplateArgu
return false;
}
+template
+bool ASTNodeImporter::ImportTemplateArgumentListInfo(
+const InContainerTy &Container, TemplateArgumentListInfo &ToTAInfo) {
+ for (const auto &FromLoc : Container) {
+if (auto ToLoc = ImportTemplateArgumentLoc(FromLoc))
+ ToTAInfo.addArgument(*ToLoc);
+else
+ return true;
+ }
+ return false;
+}
+
bool ASTNodeImporter::IsStructuralMatch(RecordDecl *FromRecord,
RecordDecl *ToRecord, bool Complain) {
// Eliminate a potential failure point where we attempt to re-import
@@ -1280,6 +1299,14 @@ bool ASTNodeImporter::IsStructuralMatch(
return Ctx.IsStructurallyEquivalent(FromEnum, ToEnum);
}
+bool ASTNodeImporter::IsStructuralMatch(FunctionTemplateDecl *From,
+FunctionTemplateDecl *To) {
+ StructuralEquivalenceContext Ctx(
+ Importer.getFromContext(), Importer.getToContext(),
+ Importer.getNonEquivalentDecls(), false, false);
+ return Ctx.IsStructurallyEquivalent(From, To);
+}
+
bool ASTNodeImporter::IsStructuralMatch(EnumConstantDecl *FromEC,
EnumConstantDecl *ToEC)
{
@@ -4197,6 +4224,64 @@ Decl *ASTNodeImporter::VisitVarTemplateS
return D2;
}
+Decl *ASTNodeImporter::VisitFunctionTemplateDecl(FunctionTemplateDecl *D) {
+ DeclContext *DC, *LexicalDC;
+ DeclarationName Name;
+ SourceLocation Loc;
+ NamedDecl *ToD;
+
+ if (ImportDeclParts(D, DC, LexicalDC, Name, ToD, Loc))
+return nullptr;
+
+ if (ToD)
+return ToD;
+
+ // Try to find a function in our own ("to") context with the same name, same
+ // type, and in the same context as the function we're importing.
+ if (!LexicalDC->isFunctionOrMethod()) {
+unsigned IDNS = Decl::IDNS_Ordinary;
+SmallVector FoundDecls;
+DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+for (unsigned I = 0, N = FoundDecls.size(); I != N; ++I) {
+ if (!FoundDecls[I]->isInIdentifierNamespace(IDNS))
+continue;
+
+ if (FunctionTemplateDecl *FoundFunction =
+ dyn_cast(FoundDecls[I])) {
+if (FoundFunction->hasExternalFormalLinkage() &&
+D->hasExternal
[PATCH] D38694: [ASTImporter] Support importing CXXUnresolvedConstructExpr and UnresolvedLookupExpr
a.sidorin updated this revision to Diff 127283.
a.sidorin added a comment.
Fixed sanity check.
Repository:
rC Clang
https://reviews.llvm.org/D38694
Files:
lib/AST/ASTImporter.cpp
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -653,5 +653,39 @@
usingShadowDecl();
}
+TEST(ImportExpr, ImportUnresolvedLookupExpr) {
+ MatchVerifier Verifier;
+ EXPECT_TRUE(testImport("template int foo();"
+ "template void declToImport() {"
+ " ::foo;"
+ " ::template foo;"
+ "}",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionTemplateDecl(has(functionDecl(has(
+ compoundStmt(has(unresolvedLookupExpr();
+}
+
+TEST(ImportExpr, ImportCXXUnresolvedConstructExpr) {
+ MatchVerifier Verifier;
+ EXPECT_TRUE(
+ testImport("template class C { T t; };"
+ "template void declToImport() {"
+ " C d;"
+ " d.t = T();"
+ "}",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionTemplateDecl(has(functionDecl(has(compoundStmt(has(
+ binaryOperator(has(cxxUnresolvedConstructExpr()));
+ EXPECT_TRUE(
+ testImport("template class C { T t; };"
+ "template void declToImport() {"
+ " C d;"
+ " (&d)->t = T();"
+ "}",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionTemplateDecl(has(functionDecl(has(compoundStmt(has(
+ binaryOperator(has(cxxUnresolvedConstructExpr()));
+}
+
} // end namespace ast_matchers
} // end namespace clang
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -287,6 +287,8 @@
Expr *VisitCXXConstructExpr(CXXConstructExpr *E);
Expr *VisitCXXMemberCallExpr(CXXMemberCallExpr *E);
Expr *VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E);
+Expr *VisitCXXUnresolvedConstructExpr(CXXUnresolvedConstructExpr *CE);
+Expr *VisitUnresolvedLookupExpr(UnresolvedLookupExpr *E);
Expr *VisitExprWithCleanups(ExprWithCleanups *EWC);
Expr *VisitCXXThisExpr(CXXThisExpr *E);
Expr *VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr *E);
@@ -5885,6 +5887,65 @@
cast_or_null(ToFQ), MemberNameInfo, ResInfo);
}
+Expr *ASTNodeImporter::VisitCXXUnresolvedConstructExpr(
+CXXUnresolvedConstructExpr *CE) {
+
+ unsigned NumArgs = CE->arg_size();
+
+ llvm::SmallVector ToArgs(NumArgs);
+ if (ImportArrayChecked(CE->arg_begin(), CE->arg_end(), ToArgs.begin()))
+return nullptr;
+
+ return CXXUnresolvedConstructExpr::Create(
+ Importer.getToContext(), Importer.Import(CE->getTypeSourceInfo()),
+ Importer.Import(CE->getLParenLoc()), llvm::makeArrayRef(ToArgs),
+ Importer.Import(CE->getRParenLoc()));
+}
+
+Expr *ASTNodeImporter::VisitUnresolvedLookupExpr(UnresolvedLookupExpr *E) {
+ CXXRecordDecl *NamingClass =
+ cast_or_null(Importer.Import(E->getNamingClass()));
+ if (E->getNamingClass() && !NamingClass)
+return nullptr;
+
+ DeclarationName Name = Importer.Import(E->getName());
+ if (E->getName() && !Name)
+return nullptr;
+
+ DeclarationNameInfo NameInfo(Name, Importer.Import(E->getNameLoc()));
+ // Import additional name location/type info.
+ ImportDeclarationNameLoc(E->getNameInfo(), NameInfo);
+
+ UnresolvedSet<8> ToDecls;
+ for (Decl *D : E->decls()) {
+if (NamedDecl *To = cast_or_null(Importer.Import(D)))
+ ToDecls.addDecl(To);
+else
+ return nullptr;
+ }
+
+ TemplateArgumentListInfo ToTAInfo(Importer.Import(E->getLAngleLoc()),
+Importer.Import(E->getRAngleLoc()));
+ TemplateArgumentListInfo *ResInfo = nullptr;
+ if (E->hasExplicitTemplateArgs()) {
+if (ImportTemplateArgumentListInfo(E->template_arguments(), ToTAInfo))
+ return nullptr;
+ResInfo = &ToTAInfo;
+ }
+
+ if (ResInfo || E->getTemplateKeywordLoc().isValid())
+return UnresolvedLookupExpr::Create(
+Importer.getToContext(), NamingClass,
+Importer.Import(E->getQualifierLoc()),
+Importer.Import(E->getTemplateKeywordLoc()), NameInfo, E->requiresADL(),
+ResInfo, ToDecls.begin(), ToDecls.end());
+
+ return UnresolvedLookupExpr::Create(
+ Importer.getToContext(), NamingClass,
+ Importer.Import(E->getQualifierLoc()), NameInfo, E->requiresADL(),
+ E->isOverloaded(), ToDecls.begin(), ToDecls.end());
+}
+
Expr *ASTNodeImporter::VisitCallExpr(CallExpr *E) {
QualType T = Importer.Import(E->getType());
if (T.isNull())
___
[PATCH] D40381: Parse concept definition
faisalv added a comment.
Thanks for working on this! :)
Comment at: include/clang/AST/DeclTemplate.h:3035
+ SourceRange getSourceRange() const override LLVM_READONLY {
+return SourceRange(getLocation(), getLocation());
+ }
why not just fix it now?
return SourceRange(getTemplateParameters()->getTemplateLoc(),
ConstraintExpr->getLocEnd();
?
Comment at: include/clang/AST/RecursiveASTVisitor.h:1725
+DEF_TRAVERSE_DECL(ConceptDecl, {})
+
Perhaps something along these lines?
DEF_TRAVERSE_DECL(ConceptDecl, {
TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters()));
TRY_TO(TraverseStmt(D->getConstraintExpr()));
// FIXME: Traverse all the concept specializations (one we implement
forming template-ids with them).
})
Comment at: include/clang/Sema/Sema.h:6196
+ const TemplateArgumentListInfo *TemplateArgs);
+
ExprResult BuildTemplateIdExpr(const CXXScopeSpec &SS,
Have you considered just deferring formation of concept template-ids to the
next patch (especially since it might require introduction of another AST node
ConceptSpecializationDecl. We could just focus on handling concept
declarations/definitions in this one (and emit an unimplemented error if
someone tries to form a template-id with a concept within BuildTemplateId etc.)
- but perhaps consider making sure we handle name clashes/redeclarations with
any other parsed names in the same namespace (within this patch)?
Comment at: lib/AST/DeclBase.cpp:773
+ return IDNS_Ordinary | IDNS_Tag;
+
// Never have names.
Pls consider just lumping this case with 'Binding/VarTemplate' (which by the
way also includes a comment for why we have to add the ugly IDNS_Tag hack here
(personally i think we should refactor this functionality, currently it seems
split between this and SemaLookup.cpp:getIDNS and would benefit from some sort
of well-designed cohesion - but that's another (low-priority) patch)
Comment at: lib/CodeGen/CGDecl.cpp:108
case Decl::Empty:
+ case Decl::Concept:
// None of these decls require codegen support.
You would need to add this to EmitTopLevelDecl too to handle global-namespace
concept declarations.
Comment at: lib/Parse/ParseTemplate.cpp:368
+
+ if (!TryConsumeToken(tok::equal)) {
+Diag(Tok.getLocation(), diag::err_expected) << "equal";
I think we should diagnose qualified-name errors here much more gracefully than
we do.
i.e. template concept N::C = ...
- perhaps try and parse a scope-specifier, and if found emit a diagnostic such
as concepts must be defined within their namespace ...
Comment at: lib/Sema/SemaTemplate.cpp:3899
+ // constraint expressions right now.
+ return Template->getConstraintExpr();
+}
I don't think we want to just return the constraint expr? I think we would need
to create another conceptspecializationDecl node and nest it within a
DeclRefExpr? But once again, I think we should defer this to another patch -
no?
Comment at: lib/Sema/SemaTemplate.cpp:3923
bool InstantiationDependent;
- if (R.getAsSingle() &&
- !TemplateSpecializationType::anyDependentTemplateArguments(
- *TemplateArgs, InstantiationDependent)) {
+ bool DependentArguments =
+TemplateSpecializationType::anyDependentTemplateArguments(
const bool, no?
Comment at: lib/Sema/SemaTemplate.cpp:7692
+ MultiTemplateParamsArg TemplateParameterLists,
+ IdentifierInfo *Name, SourceLocation L,
+ Expr *ConstraintExpr) {
Rename L as NameLoc?
Comment at: lib/Sema/SemaTemplate.cpp:7706
+ }
+
+ ConceptDecl *NewDecl = ConceptDecl::Create(Context, DC, L, Name,
Perhaps add a lookup check here? (or a fixme that we are deferring to the next
patch), something along these *pseudo-code* lines:
LookupResult Previous(*this,
DeclarationNameInfo(DeclarationName(Name),NameLoc), LookupOrdinaryName);
LookupName(Previous, S);
if (Previous.getRepresentativeDecl())... if the decl is a concept - give
error regarding only one definition allowed in a TU, if a different kind of
decl then declared w a different symbol type of error?
Comment at: lib/Sema/SemaTemplate.cpp:7713
+
+ if (!ConstraintExpr->isTypeDependent() &&
+ ConstraintExpr->getType() != Context.BoolTy) {
Consider refactoring these checks on constraint expressions into a separate
function checkConstraintExpression (that we can also call from other contexts
such as requires-clauses and nested requires expressions)?
Comment at: lib/Sema/SemaTemplate.cpp:7735
+ Act
[PATCH] D40705: [Parser] Diagnose storage classes in template parameter declarations
faisalv accepted this revision.
faisalv added a comment.
This revision is now accepted and ready to land.
Otherwise, I think this looks good enough to commit.
Do you have commit access? If not, let me know when you're ready for me to
commit it on your behalf ...
Thank you for fixing this!
Comment at: include/clang/Basic/DiagnosticParseKinds.td:671
+def err_storage_class_template_parameter : Error<
+ "storage class specified for template parameter %0">;
What about folding both of these diagnostics into one with something along
these lines:
def err_storage_class_template_parameter : Error<
"storage class specified for template parameter %select{|%1}0">;
And for no identifier, do << 0, and for the valid identifier case: do << 1 <<
ParamDecl.getIdentifier()
https://reviews.llvm.org/D40705
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40381: Parse concept definition
saar.raz added inline comments.
Comment at: lib/Sema/SemaTemplate.cpp:3899
+ // constraint expressions right now.
+ return Template->getConstraintExpr();
+}
faisalv wrote:
> I don't think we want to just return the constraint expr? I think we would
> need to create another conceptspecializationDecl node and nest it within a
> DeclRefExpr? But once again, I think we should defer this to another patch -
> no?
This was meant as a placeholder. The next patch (D41217) replaces this
function, I don't think it is that much of a big deal what happens here in this
patch.
Comment at: lib/Sema/SemaTemplate.cpp:7713
+
+ if (!ConstraintExpr->isTypeDependent() &&
+ ConstraintExpr->getType() != Context.BoolTy) {
faisalv wrote:
> Consider refactoring these checks on constraint expressions into a separate
> function checkConstraintExpression (that we can also call from other contexts
> such as requires-clauses and nested requires expressions)?
I did that in the upcoming patches, no need to do it here as well.
https://reviews.llvm.org/D40381
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40381: Parse concept definition
faisalv requested changes to this revision.
faisalv added inline comments.
This revision now requires changes to proceed.
Comment at: lib/Sema/SemaTemplate.cpp:3899
+ // constraint expressions right now.
+ return Template->getConstraintExpr();
+}
saar.raz wrote:
> faisalv wrote:
> > I don't think we want to just return the constraint expr? I think we would
> > need to create another conceptspecializationDecl node and nest it within a
> > DeclRefExpr? But once again, I think we should defer this to another patch
> > - no?
> This was meant as a placeholder. The next patch (D41217) replaces this
> function, I don't think it is that much of a big deal what happens here in
> this patch.
This: " I don't think it is that much of a big deal what happens here in this
patch." I think is poor practice in an open source project when you're not sure
when the next patch will land.
I think, when features are implemented incrementally - each patch should
diagnose the yet to be implemented features - and error out as gracefully as
possible. So for instance replacing the body of this function with an
appropriate diagnostic (that is then removed in future patches) would be the
better thing to do here.
Comment at: lib/Sema/SemaTemplate.cpp:7713
+
+ if (!ConstraintExpr->isTypeDependent() &&
+ ConstraintExpr->getType() != Context.BoolTy) {
saar.raz wrote:
> faisalv wrote:
> > Consider refactoring these checks on constraint expressions into a separate
> > function checkConstraintExpression (that we can also call from other
> > contexts such as requires-clauses and nested requires expressions)?
> I did that in the upcoming patches, no need to do it here as well.
Once again - relying on a future patch (especially without a clear FIXME) to
tweak the architectural/factorization issues that you have been made aware of
(and especially those, such as this, that do not require too much effort), is
not practice I would generally encourage.
So changyu, if you agree that these suggestions would improve the quality of
the patch and leave clang in a more robust state (maintenance-wise or
execution-wise), then please make the changes. If not, please share your
thoughts as to why these suggestions would either not be an improvement or be
inconsistent with the theme of this patch.
Thanks!
https://reviews.llvm.org/D40381
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r320954 - Refactor overridden methods iteration to avoid double lookups.
Author: d0k
Date: Sun Dec 17 15:52:45 2017
New Revision: 320954
URL: http://llvm.org/viewvc/llvm-project?rev=320954&view=rev
Log:
Refactor overridden methods iteration to avoid double lookups.
Convert most uses to range-for loops. No functionality change intended.
Modified:
cfe/trunk/include/clang/AST/DeclCXX.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/ASTDumper.cpp
cfe/trunk/lib/AST/CXXInheritance.cpp
cfe/trunk/lib/AST/DeclCXX.cpp
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
cfe/trunk/lib/AST/VTableBuilder.cpp
cfe/trunk/lib/Analysis/ThreadSafetyCommon.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/Index/IndexDecl.cpp
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=320954&r1=320953&r2=320954&view=diff
==
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Sun Dec 17 15:52:45 2017
@@ -2015,7 +2015,7 @@ public:
if (CD->isVirtualAsWritten() || CD->isPure())
return true;
-return (CD->begin_overridden_methods() != CD->end_overridden_methods());
+return CD->size_overridden_methods() != 0;
}
/// If it's possible to devirtualize a call to this method, return the called
Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=320954&r1=320953&r2=320954&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Sun Dec 17 15:52:45 2017
@@ -1378,35 +1378,27 @@ void ASTContext::setInstantiatedFromUnna
ASTContext::overridden_cxx_method_iterator
ASTContext::overridden_methods_begin(const CXXMethodDecl *Method) const {
- llvm::DenseMap::const_iterator Pos =
- OverriddenMethods.find(Method->getCanonicalDecl());
- if (Pos == OverriddenMethods.end())
-return nullptr;
- return Pos->second.begin();
+ return overridden_methods(Method).begin();
}
ASTContext::overridden_cxx_method_iterator
ASTContext::overridden_methods_end(const CXXMethodDecl *Method) const {
- llvm::DenseMap::const_iterator Pos =
- OverriddenMethods.find(Method->getCanonicalDecl());
- if (Pos == OverriddenMethods.end())
-return nullptr;
- return Pos->second.end();
+ return overridden_methods(Method).end();
}
unsigned
ASTContext::overridden_methods_size(const CXXMethodDecl *Method) const {
- llvm::DenseMap::const_iterator Pos =
- OverriddenMethods.find(Method->getCanonicalDecl());
- if (Pos == OverriddenMethods.end())
-return 0;
- return Pos->second.size();
+ auto Range = overridden_methods(Method);
+ return Range.end() - Range.begin();
}
ASTContext::overridden_method_range
ASTContext::overridden_methods(const CXXMethodDecl *Method) const {
- return overridden_method_range(overridden_methods_begin(Method),
- overridden_methods_end(Method));
+ llvm::DenseMap::const_iterator Pos =
+ OverriddenMethods.find(Method->getCanonicalDecl());
+ if (Pos == OverriddenMethods.end())
+return overridden_method_range(nullptr, nullptr);
+ return overridden_method_range(Pos->second.begin(), Pos->second.end());
}
void ASTContext::addOverriddenMethod(const CXXMethodDecl *Method,
Modified: cfe/trunk/lib/AST/ASTDumper.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTDumper.cpp?rev=320954&r1=320953&r2=320954&view=diff
==
--- cfe/trunk/lib/AST/ASTDumper.cpp (original)
+++ cfe/trunk/lib/AST/ASTDumper.cpp Sun Dec 17 15:52:45 2017
@@ -1195,12 +1195,11 @@ void ASTDumper::VisitFunctionDecl(const
};
dumpChild([=] {
-auto FirstOverrideItr = MD->begin_overridden_methods();
+auto Overrides = MD->overridden_methods();
OS << "Overrides: [ ";
-dumpOverride(*FirstOverrideItr);
+dumpOverride(*Overrides.begin());
for (const auto *Override :
- llvm::make_range(FirstOverrideItr + 1,
-MD->end_overridden_methods())) {
+ llvm::make_range(Overrides.begin() + 1, Overrides.end())) {
OS << ", ";
dumpOverride(Override);
}
Modified: cfe/trunk/lib/AST/CXXInheritance.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CXXInheritance.cpp?rev=320954&r1=320953&r2=320954&view=diff
==
--- cfe/trunk/lib/AST/CXXInheritance.cpp (original)
+++ cfe/trunk/lib/AST/CXXInheritance.cpp Sun Dec 17 15:52:45 2017
@@ -650,9 +650,11 @@ void FinalOverriderCollect
[PATCH] D41258: [analyzer] trackNullOrUndefValue: deduplicate path pieces for each node.
dcoughlin accepted this revision. dcoughlin added a comment. This revision is now accepted and ready to land. Looks good to me. Repository: rC Clang https://reviews.llvm.org/D41258 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41054: Teach clang/NetBSD about additional dependencies for sanitizers
krytarowski added a comment. Ping? Repository: rL LLVM https://reviews.llvm.org/D41054 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41179: [Sema] Diagnose template specializations with C linkage
faisalv added inline comments. Comment at: lib/Sema/SemaTemplate.cpp:7029 + } + // C++ [temp.expl.spec]p2: What do you think about factoring this out into a separate function that is then called both from here and CheckTemplateDeclScope (since it is exactly the same code?) https://reviews.llvm.org/D41179 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41179: [Sema] Diagnose template specializations with C linkage
sepavloff added a comment.
Is it correct to emit error in this case?
According to the Standard 10.5p1 "All function types, function names with
external linkage, and variable names with external linkage have a language
linkage". So templates do not have a language linkage.
The next paragraph, which introduces inkage-specification, states: "The
string-literal indicates the required language linkage". So the construct
`extern "C"` specifies just language linkage, which is not pertinent to
templates.
In 10.5p4 there is an example of a class defined in extern "C" construct:
extern "C" {
class X {
void mf(); // the name of the function mf and the member function’s type
have
// C++ language linkage
void mf2(void(*)()); // the name of the function mf2 has C++ language
linkage;
// the parameter has type “pointer to C function”
};
}
Classes do not have language linkage according to 10.5p1, just as templates, so
this code is valid.
It looks like defining templates inside extern "C" blocks is OK.
https://reviews.llvm.org/D41179
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
