[PATCH] D41537: Optionally add code completion results for arrow instead of dot
yvvan marked an inline comment as done. yvvan added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:4148 + + CompletionSucceded |= DoCompletion(Base, IsArrow, None); + yvvan wrote: > ilya-biryukov wrote: > > NIT: maybe swap the two cases to do the non-fixit ones first? It is the > > most common case, so it should probably go first. > The actual completion should be the last because of the "Completion context". > I've put them in this order intentionally. I will add comment about that. I've rechecked this part - I can actually now move it without breaking anything https://reviews.llvm.org/D41537 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41537: Optionally add code completion results for arrow instead of dot
yvvan updated this revision to Diff 148553.
yvvan added a comment.
NIT-s addressed
https://reviews.llvm.org/D41537
Files:
include/clang/Driver/CC1Options.td
include/clang/Sema/CodeCompleteConsumer.h
include/clang/Sema/CodeCompleteOptions.h
include/clang/Sema/Sema.h
lib/Frontend/ASTUnit.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Parse/ParseExpr.cpp
lib/Sema/CodeCompleteConsumer.cpp
lib/Sema/SemaCodeComplete.cpp
test/CodeCompletion/member-access.cpp
Index: test/CodeCompletion/member-access.cpp
===
--- test/CodeCompletion/member-access.cpp
+++ test/CodeCompletion/member-access.cpp
@@ -166,3 +166,47 @@
typename Template::Nested m;
// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:166:25 %s -o - | FileCheck -check-prefix=CHECK-CC7 %s
}
+
+class Proxy2 {
+public:
+ Derived *operator->() const;
+ int member5;
+};
+
+void test2(const Proxy2 &p) {
+ p->
+}
+
+void test3(const Proxy2 &p) {
+ p.
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits -code-completion-at=%s:177:6 %s -o - | FileCheck -check-prefix=CHECK-CC8 --implicit-check-not="Derived : Derived(" %s
+// CHECK-CC8: Base1 : Base1::
+// CHECK-CC8: member1 : [#int#][#Base1::#]member1
+// CHECK-CC8: member1 : [#int#][#Base2::#]member1
+// CHECK-CC8: member2 : [#float#][#Base1::#]member2
+// CHECK-CC8: member3 : [#double#][#Base2::#]member3
+// CHECK-CC8: member4 : [#int#]member4
+// CHECK-CC8: member5 : [#int#]member5 (requires fix-it: {177:4-177:6} to ".")
+// CHECK-CC8: memfun1 : [#void#][#Base3::#]memfun1(<#float#>)
+// CHECK-CC8: memfun1 : [#void#][#Base3::#]memfun1(<#double#>)[# const#]
+// CHECK-CC8: memfun1 (Hidden) : [#void#]Base2::memfun1(<#int#>)
+// CHECK-CC8: memfun2 : [#void#][#Base3::#]memfun2(<#int#>)
+// CHECK-CC8: memfun3 : [#int#]memfun3(<#int#>)
+// CHECK-CC8: operator-> : [#Derived *#]operator->()[# const#] (requires fix-it: {177:4-177:6} to ".")
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-with-fixits -code-completion-at=%s:181:6 %s -o - | FileCheck -check-prefix=CHECK-CC9 --implicit-check-not="Derived : Derived(" %s
+// CHECK-CC9: Base1 : Base1::
+// CHECK-CC9: member1 : [#int#][#Base1::#]member1 (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: member1 : [#int#][#Base2::#]member1 (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: member2 : [#float#][#Base1::#]member2 (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: member3 : [#double#][#Base2::#]member3 (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: member4 : [#int#]member4 (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: member5 : [#int#]member5
+// CHECK-CC9: memfun1 : [#void#][#Base3::#]memfun1(<#float#>) (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: memfun1 : [#void#][#Base3::#]memfun1(<#double#>)[# const#] (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: memfun1 (Hidden) : [#void#]Base2::memfun1(<#int#>) (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: memfun2 : [#void#][#Base3::#]memfun2(<#int#>) (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: memfun3 : [#int#]memfun3(<#int#>) (requires fix-it: {181:4-181:5} to "->")
+// CHECK-CC9: operator-> : [#Derived *#]operator->()[# const#]
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -1291,19 +1291,22 @@
class CodeCompletionDeclConsumer : public VisibleDeclConsumer {
ResultBuilder &Results;
DeclContext *CurContext;
+std::vector FixIts;
public:
-CodeCompletionDeclConsumer(ResultBuilder &Results, DeclContext *CurContext)
- : Results(Results), CurContext(CurContext) { }
+CodeCompletionDeclConsumer(
+ResultBuilder &Results, DeclContext *CurContext,
+std::vector FixIts = std::vector())
+: Results(Results), CurContext(CurContext), FixIts(std::move(FixIts)) {}
void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, DeclContext *Ctx,
bool InBaseClass) override {
bool Accessible = true;
if (Ctx)
Accessible = Results.getSema().IsSimplyAccessible(ND, Ctx);
ResultBuilder::Result Result(ND, Results.getBasePriority(ND), nullptr,
- false, Accessible);
+ false, Accessible, FixIts);
Results.AddResult(Result, CurContext, Hiding, InBaseClass);
}
@@ -3979,14 +3982,18 @@
static void AddRecordMembersCompletionResults(Sema &SemaRef,
ResultBuilder &Results, Scope *S,
QualType BaseType,
- RecordDecl *RD) {
+ RecordDecl *RD,
+ Optional AccessOpFixIt) {
// Indicate that we are performing a member access, and the cv-qualifiers
[PATCH] D37442: [C++17] Disallow lambdas in template parameters (PR33696).
Rakete updated this revision to Diff 148555.
Rakete added a comment.
Rebase and friendly ping :)
https://reviews.llvm.org/D37442
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Parse/ParseDecl.cpp
lib/Parse/ParseDeclCXX.cpp
lib/Parse/ParseStmt.cpp
lib/Parse/ParseTemplate.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprCXX.cpp
lib/Sema/TreeTransform.h
test/CXX/expr/expr.prim/expr.prim.lambda/p2-template-parameter.cpp
Index: test/CXX/expr/expr.prim/expr.prim.lambda/p2-template-parameter.cpp
===
--- test/CXX/expr/expr.prim/expr.prim.lambda/p2-template-parameter.cpp
+++ test/CXX/expr/expr.prim/expr.prim.lambda/p2-template-parameter.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -std=c++17 %s -verify
+
+template struct Nothing {};
+
+void pr33696() {
+Nothing<[]() { return 0; }()> nothing; // expected-error{{a lambda expression cannot appear in this context}}
+}
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -5494,7 +5494,7 @@
// decltype expressions are not potentially evaluated contexts
EnterExpressionEvaluationContext Unevaluated(
SemaRef, Sema::ExpressionEvaluationContext::Unevaluated, nullptr,
- /*IsDecltype=*/true);
+ Sema::ExpressionEvaluationContextRecord::EK_Decltype);
ExprResult E = getDerived().TransformExpr(T->getUnderlyingExpr());
if (E.isInvalid())
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -6363,7 +6363,8 @@
if (RD->isInvalidDecl() || RD->isDependentContext())
return E;
- bool IsDecltype = ExprEvalContexts.back().IsDecltype;
+ bool IsDecltype = ExprEvalContexts.back().ExprContext ==
+ExpressionEvaluationContextRecord::EK_Decltype;
CXXDestructorDecl *Destructor = IsDecltype ? nullptr : LookupDestructor(RD);
if (Destructor) {
@@ -6445,7 +6446,9 @@
/// are omitted for the 'topmost' call in the decltype expression. If the
/// topmost call bound a temporary, strip that temporary off the expression.
ExprResult Sema::ActOnDecltypeExpression(Expr *E) {
- assert(ExprEvalContexts.back().IsDecltype && "not in a decltype expression");
+ assert(ExprEvalContexts.back().ExprContext ==
+ ExpressionEvaluationContextRecord::EK_Decltype &&
+ "not in a decltype expression");
// C++11 [expr.call]p11:
// If a function call is a prvalue of object type,
@@ -6487,7 +6490,8 @@
TopBind = nullptr;
// Disable the special decltype handling now.
- ExprEvalContexts.back().IsDecltype = false;
+ ExprEvalContexts.back().ExprContext =
+ ExpressionEvaluationContextRecord::EK_Other;
// In MS mode, don't perform any extra checking of call return types within a
// decltype expression.
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -14038,11 +14038,11 @@
}
void
-Sema::PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext,
- Decl *LambdaContextDecl,
- bool IsDecltype) {
+Sema::PushExpressionEvaluationContext(
+ExpressionEvaluationContext NewContext, Decl *LambdaContextDecl,
+ExpressionEvaluationContextRecord::ExpressionKind ExprContext) {
ExprEvalContexts.emplace_back(NewContext, ExprCleanupObjects.size(), Cleanup,
-LambdaContextDecl, IsDecltype);
+LambdaContextDecl, ExprContext);
Cleanup.reset();
if (!MaybeODRUseExprs.empty())
std::swap(MaybeODRUseExprs, ExprEvalContexts.back().SavedMaybeODRUseExprs);
@@ -14049,11 +14049,11 @@
}
void
-Sema::PushExpressionEvaluationContext(ExpressionEvaluationContext NewContext,
- ReuseLambdaContextDecl_t,
- bool IsDecltype) {
+Sema::PushExpressionEvaluationContext(
+ExpressionEvaluationContext NewContext, ReuseLambdaContextDecl_t,
+ExpressionEvaluationContextRecord::ExpressionKind ExprContext) {
Decl *ClosureContextDecl = ExprEvalContexts.back().ManglingContextDecl;
- PushExpressionEvaluationContext(NewContext, ClosureContextDecl, IsDecltype);
+ PushExpressionEvaluationContext(NewContext, ClosureContextDecl, ExprContext);
}
void Sema::PopExpressionEvaluationContext() {
@@ -14061,7 +14061,9 @@
unsigned NumTypos = Rec.NumTypos;
if (!Rec.Lambdas.empty()) {
-if (Rec.isUnevaluated() || Rec.isConstantEvaluated()) {
+using ExpressionKind = ExpressionEvaluationContextRecord::ExpressionKind;
+if (Rec.ExprContext == ExpressionKind::EK_TemplateParameter || Rec.isUnevaluated() ||
+(Rec.isConstantEvaluated() && !getLangO
[PATCH] D47331: [clangd] Remove accessors for top-level decls from preamble
sammccall added a comment.
Wow, nice!
Just unsure about the test helper. (We can rewrite it in another way if needed)
Comment at: clangd/ClangdUnit.h:88
- /// This function returns all top-level decls, including those that come
- /// from Preamble. Decls, coming from Preamble, have to be deserialized, so
- /// this call might be expensive.
- ArrayRef getTopLevelDecls();
+ /// This function returns top-level decls, present in the main file of the
+ /// AST. The result does not include the decls that come from the preamble.
nit: remove comma
Comment at: unittests/clangd/TestTU.cpp:77
const NamedDecl *Result = nullptr;
- for (const Decl *D : AST.getTopLevelDecls()) {
+ for (const Decl *D : AST.getLocalTopLevelDecls()) {
auto *ND = dyn_cast(D);
isn't this incorrect? Or should be "findDeclInMainFile" or similar?
I'd think this would conflict with your other patch, which uses this to test
the boost of things that come from the header.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47331: [clangd] Remove accessors for top-level decls from preamble
ioeric added a comment. Nice! This looks good to me. Just some nits. I'll let Sam stamp. Comment at: clangd/ClangdUnit.h:81 ASTContext &getASTContext(); const ASTContext &getASTContext() const; IIUC, `ASTContext` in a `ParsedAST` may not contain information from preambles and thus may give an incomplete AST. If so, I think we should make this more explicit in the class level to make sure this is not misused (e.g. in case the incomplete context is used to build dynamic index). Comment at: clangd/ClangdUnit.h:119 std::vector Diags; std::vector TopLevelDecls; std::vector Inclusions; nit: also rename this and add comment? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47331 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46862: [libclang] Optionally add code completion results for arrow instead of dot
yvvan updated this revision to Diff 148559.
yvvan added a comment.
Update according to c++ part changes
https://reviews.llvm.org/D46862
Files:
include/clang-c/Index.h
test/Index/complete-arrow-dot.cpp
tools/c-index-test/c-index-test.c
tools/libclang/CIndex.cpp
tools/libclang/CIndexCodeCompletion.cpp
tools/libclang/libclang.exports
Index: tools/libclang/libclang.exports
===
--- tools/libclang/libclang.exports
+++ tools/libclang/libclang.exports
@@ -171,6 +171,8 @@
clang_getCompletionChunkCompletionString
clang_getCompletionChunkKind
clang_getCompletionChunkText
+clang_getCompletionNumFixIts
+clang_getCompletionFixIt
clang_getCompletionNumAnnotations
clang_getCompletionParent
clang_getCompletionPriority
@@ -260,6 +262,7 @@
clang_getSpellingLocation
clang_getTUResourceUsageName
clang_getTemplateCursorKind
+clang_getToken
clang_getTokenExtent
clang_getTokenKind
clang_getTokenLocation
Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -16,6 +16,7 @@
#include "CIndexDiagnostic.h"
#include "CLog.h"
#include "CXCursor.h"
+#include "CXSourceLocation.h"
#include "CXString.h"
#include "CXTranslationUnit.h"
#include "clang/AST/Decl.h"
@@ -302,10 +303,53 @@
/// A string containing the Objective-C selector entered thus far for a
/// message send.
std::string Selector;
+
+ /// Vector of fix-its for each completion result that *must* be applied
+ /// before that result for the corresponding completion item.
+ std::vector> FixItsVector;
};
} // end anonymous namespace
+unsigned clang_getCompletionNumFixIts(CXCodeCompleteResults *results,
+ unsigned completion_index) {
+ AllocatedCXCodeCompleteResults *allocated_results = (AllocatedCXCodeCompleteResults *)results;
+
+ if (!allocated_results || allocated_results->FixItsVector.size() <= completion_index)
+return 0;
+
+ return static_cast(allocated_results->FixItsVector[completion_index].size());
+}
+
+CXString clang_getCompletionFixIt(CXCodeCompleteResults *results,
+ unsigned completion_index,
+ unsigned fixit_index,
+ CXSourceRange *replacement_range) {
+ AllocatedCXCodeCompleteResults *allocated_results = (AllocatedCXCodeCompleteResults *)results;
+
+ if (!allocated_results || allocated_results->FixItsVector.size() <= completion_index) {
+if (replacement_range)
+ *replacement_range = clang_getNullRange();
+return cxstring::createNull();
+ }
+
+ ArrayRef FixIts = allocated_results->FixItsVector[completion_index];
+ if (FixIts.size() <= fixit_index) {
+if (replacement_range)
+ *replacement_range = clang_getNullRange();
+return cxstring::createNull();
+ }
+
+ const FixItHint &FixIt = FixIts[fixit_index];
+ if (replacement_range) {
+*replacement_range = cxloc::translateSourceRange(
+*allocated_results->SourceMgr, allocated_results->LangOpts,
+FixIt.RemoveRange);
+ }
+
+ return cxstring::createRef(FixIt.CodeToInsert.c_str());
+}
+
/// Tracks the number of code-completion result objects that are
/// currently active.
///
@@ -531,18 +575,22 @@
CodeCompletionResult *Results,
unsigned NumResults) override {
StoredResults.reserve(StoredResults.size() + NumResults);
+ if (includeFixIts())
+AllocatedResults.FixItsVector.reserve(NumResults);
for (unsigned I = 0; I != NumResults; ++I) {
-CodeCompletionString *StoredCompletion
+CodeCompletionString *StoredCompletion
= Results[I].CreateCodeCompletionString(S, Context, getAllocator(),
getCodeCompletionTUInfo(),
includeBriefComments());
CXCompletionResult R;
R.CursorKind = Results[I].CursorKind;
R.CompletionString = StoredCompletion;
StoredResults.push_back(R);
+if (includeFixIts())
+ AllocatedResults.FixItsVector.emplace_back(std::move(Results[I].FixIts));
}
-
+
enum CodeCompletionContext::Kind contextKind = Context.getKind();
AllocatedResults.ContextKind = contextKind;
@@ -644,13 +692,13 @@
unsigned options) {
bool IncludeBriefComments = options & CXCodeComplete_IncludeBriefComments;
bool SkipPreamble = options & CXCodeComplete_SkipPreamble;
+ bool IncludeFixIts = options & CXCodeComplete_IncludeFixIts;
#ifdef UDP_CODE_COMPLETION_LOGGER
#ifdef UDP_CODE_COMPLETION_LOGGER_PORT
const llvm::TimeRecord &StartTime = llvm::TimeRecord::getCurrentTime();
#endif
#endif
-
bool EnableLogging = getenv("LIBCLAN
[PATCH] D39679: [C++11] Fix warning when dropping cv-qualifiers when assigning to a reference with a braced initializer list
Rakete updated this revision to Diff 148564.
Rakete added a comment.
Rebased + friendly ping :)
https://reviews.llvm.org/D39679
Files:
lib/Sema/SemaInit.cpp
test/SemaCXX/references.cpp
Index: test/SemaCXX/references.cpp
===
--- test/SemaCXX/references.cpp
+++ test/SemaCXX/references.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
int g(int);
void f() {
@@ -55,6 +55,24 @@
// const double& rcd2 = 2; // rcd2 refers to temporary with value 2.0
const volatile int cvi = 1;
const int& r = cvi; // expected-error{{binding value of type 'const volatile int' to reference to type 'const int' drops 'volatile' qualifier}}
+
+#if __cplusplus >= 201103L
+ const int& r2{cvi}; // expected-error{{binding value of type 'const volatile int' to reference to type 'const int' drops 'volatile' qualifier}}
+
+ const int a = 2;
+ int& r3{a}; // expected-error{{binding value of type 'const int' to reference to type 'int' drops 'const'}}
+
+ const int&& r4{a}; // expected-error{{rvalue reference to type 'const int' cannot bind to lvalue of type 'const int'}}
+
+ void func();
+ void func(int);
+ int &ptr1 = {func}; // expected-error{{address of overloaded function 'func' does not match required type 'int'}}
+ int &&ptr2{func}; // expected-error{{address of overloaded function 'func' does not match required type 'int'}}
+ // expected-note@-4{{candidate function}}
+ // expected-note@-4{{candidate function}}
+ // expected-note@-6{{candidate function}}
+ // expected-note@-6{{candidate function}}
+#endif
}
// C++ [dcl.init.ref]p3
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -7571,6 +7571,18 @@
if (!Failed())
return false;
+ // When we want to diagnose only one element of a braced-init-list, we need to factor it out.
+ Expr *OnlyArg;
+ if (Args.size() == 1) {
+auto *List = dyn_cast(Args[0]);
+if (List && List->getNumInits() == 1)
+ OnlyArg = List->getInit(0);
+else
+ OnlyArg = Args[0];
+ }
+ else
+OnlyArg = nullptr;
+
QualType DestType = Entity.getType();
switch (Failure) {
case FK_TooManyInitsForReference:
@@ -7631,7 +7643,7 @@
? diag::err_array_init_different_type
: diag::err_array_init_non_constant_array))
<< DestType.getNonReferenceType()
- << Args[0]->getType()
+ << OnlyArg->getType()
<< Args[0]->getSourceRange();
break;
@@ -7642,7 +7654,7 @@
case FK_AddressOfOverloadFailed: {
DeclAccessPair Found;
-S.ResolveAddressOfOverloadedFunction(Args[0],
+S.ResolveAddressOfOverloadedFunction(OnlyArg,
DestType.getNonReferenceType(),
true,
Found);
@@ -7662,11 +7674,11 @@
case OR_Ambiguous:
if (Failure == FK_UserConversionOverloadFailed)
S.Diag(Kind.getLocation(), diag::err_typecheck_ambiguous_condition)
- << Args[0]->getType() << DestType
+ << OnlyArg->getType() << DestType
<< Args[0]->getSourceRange();
else
S.Diag(Kind.getLocation(), diag::err_ref_init_ambiguous)
- << DestType << Args[0]->getType()
+ << DestType << OnlyArg->getType()
<< Args[0]->getSourceRange();
FailedCandidateSet.NoteCandidates(S, OCD_ViableCandidates, Args);
@@ -7676,10 +7688,10 @@
if (!S.RequireCompleteType(Kind.getLocation(),
DestType.getNonReferenceType(),
diag::err_typecheck_nonviable_condition_incomplete,
- Args[0]->getType(), Args[0]->getSourceRange()))
+ OnlyArg->getType(), Args[0]->getSourceRange()))
S.Diag(Kind.getLocation(), diag::err_typecheck_nonviable_condition)
<< (Entity.getKind() == InitializedEntity::EK_Result)
- << Args[0]->getType() << Args[0]->getSourceRange()
+ << OnlyArg->getType() << Args[0]->getSourceRange()
<< DestType.getNonReferenceType();
FailedCandidateSet.NoteCandidates(S, OCD_AllCandidates, Args);
@@ -7687,7 +7699,7 @@
case OR_Deleted: {
S.Diag(Kind.getLocation(), diag::err_typecheck_deleted_function)
-<< Args[0]->getType() << DestType.getNonReferenceType()
+<< OnlyArg->getType() << DestType.getNonReferenceType()
<< Args[0]->getSourceRange();
OverloadCandidateSet::iterator Best;
OverloadingResult Ovl
@@ -7723,7 +7735,7 @@
: diag::err_lvalue_reference_bind_to_unrelated)
<< DestType.getNonReferenceType().isVolatileQualified()
<< DestType.getNonReferenceType()
- << Args[0]->getType()
+ << OnlyArg->getType()
<< Args[0]->getSourceRange
[PATCH] D44954: [clangd] Add "member" symbols to the index
ioeric added a comment.
The change looks mostly good. Some nits and questions about the testing.
Comment at: clangd/index/Index.h:158
unsigned References = 0;
-
+ /// Whether or not this is symbol is meant to be used for the global
+ /// completion.
s/this is symbol/this symbol/?
Comment at: clangd/index/SymbolCollector.cpp:158
+ translationUnitDecl(), namespaceDecl(), linkageSpecDecl(), recordDecl(),
+ enumDecl(), objcProtocolDecl(), objcInterfaceDecl(), objcCategoryDecl(),
+ objcCategoryImplDecl(), objcImplementationDecl()));
(Disclaimer: I don't know obj-c...)
It seems that some objc contexts here are good for global code completion. If
we want to support objc symbols, it might also make sense to properly set
`SupportGlobalCompletion` for them.
Comment at: clangd/index/SymbolCollector.cpp:359
+
+ // For global completion, we only want:
+ // * symbols in namespaces or translation unit scopes (e.g. no class
Could we pull this into a helper function? Something like
`S.SupportGlobalCompletion = DeclSupportsGlobalCompletion(...)`?
Comment at: unittests/clangd/CodeCompleteTests.cpp:657
+TEST(CompletionTest, DynamicIndexMultiFileMembers) {
+ MockFSProvider FS;
IIUC, this tests the `SupportGlobalCompletion` filtering logic for index
completion? If so, I think it would be more straightforward to do something
like:
```
Sym NoGlobal(...);
NoGlobal.SupportGlobalCompletion = false;
...
completions (Code, {NoGlobal, func(...), cls(...)})
// Expect only global symbols in completion results.
```
This avoid setting up the ClangdServer manually.
Comment at: unittests/clangd/CodeCompleteTests.cpp:741
+TEST(CompletionTest, Enums) {
+ EXPECT_THAT(completions(R"cpp(
It's not clear to me what the following tests (`Enums`, `AnonymousNamespace`,
`InMainFile`) are testing. Do they test code completion or symbol collector?
If these test symbol collection, they should be moved int
SymbolCollectorTest.cpp
Comment at: unittests/clangd/CodeCompleteTests.cpp:785
+ .items,
+ IsEmpty());
+
I think the behaviors above are the same before this change, so I'm not quite
sure what they are testing. Note that symbols in main files are not collected
into dynamic index, and by default the tests do not use dynamic index.
Comment at: unittests/clangd/CodeCompleteTests.cpp:846
+ auto Results = cantFail(runCodeComplete(Server, File, Test.point(), {}));
+ EXPECT_THAT(Results.items, IsEmpty());
+}
I think this is expected to be empty even for symbols that are not in anonymous
namespace because symbols in main files are not indexed.
Comment at: unittests/clangd/SymbolCollectorTests.cpp:70
MATCHER_P(Refs, R, "") { return int(arg.References) == R; }
+MATCHER_P(SupportGlobalCompletion, SupportGlobalCompletion, "") {
+ return arg.SupportGlobalCompletion == SupportGlobalCompletion;
nit: I'd probably call this `Global` for convenience.
Comment at: unittests/clangd/SymbolCollectorTests.cpp:216
runSymbolCollector(Header, /*Main=*/"");
- EXPECT_THAT(Symbols,
- UnorderedElementsAreArray(
- {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"),
- QName("kStr"), QName("foo"), QName("foo::bar"),
- QName("foo::int32"), QName("foo::int32_t"),
QName("foo::v1"),
- QName("foo::bar::v2"), QName("foo::baz")}));
+ EXPECT_THAT(Symbols, UnorderedElementsAreArray(
+ {QName("Foo"), QName("Foo::Foo"),
Could you also add checkers for the `SupportGlobalCompletion` field?
Comment at: unittests/clangd/SymbolCollectorTests.cpp:404
runSymbolCollector(Header, /*Main=*/"");
- EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Red"), QName("Color"),
-QName("Green"), QName("Color2"),
-QName("ns"), QName("ns::Black")));
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("Red"), QName("Color"),
QName("Green"),
Could you please also add checkers for `GlobalCodeCompletion` for these enums?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44954
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47223: [clangd] Handle enumerators in named, unscoped enums similarly to scoped enums
ioeric added a comment.
In https://reviews.llvm.org/D47223#1110571, @malaperle wrote:
> In https://reviews.llvm.org/D47223#1109247, @ilya-biryukov wrote:
>
> > I'm not sure if we have tests for that, but I remember that we kept the
> > enumerators in the outer scope so that completion could find them..
> > Am I right that this patch will change the behavior and we won't get
> > enumerators in the following example:
> >
> > /// foo.h
> > enum Foo {
> > A, B, C
> > };
> >
> > /// foo.cpp
> > #include "foo.h"
> >
> > int a = ^ // <-- A, B, C should be in completion list here.
> >
>
>
> Not quite but still potentially problematic. With the patch, A, B, C would be
> found but not ::A, ::B, ::C.
>
> > It's one of those cases where code completion and workspace symbol search
> > seem to want different results :-(
> > I suggest to add an extra string field for containing unscoped enum name,
> > maybe into symbol details? And add a parameter to `Index::fuzzyFind` on
> > whether we need to match enum scopes or not.
> > +@ioeric, +@sammccall, WDYT?
>
> I'll wait to see what others think before changing it. But I feel it's a bit
> odd that completion and workspace symbols would be inconsistent. I'd rather
> have it that A, ::A, and Foo::A work for both completion and workspace. Maybe
> it would complicate things too much...
Having "wrong" names in workspaceSymbol (e.g. ::A instead of ::Foo::A) and
missing results in code completions (e.g. ::A missing in ::^) seem equally bad.
I think the fundamental problem here is that C++ symbols (esp. enum constants)
can have different names (e.g. ns::Foo::A and ns::A). We want `ns::Foo::A` for
workspaceSymbols, but we also want `ns::A` for index-based code completions
(sema completion would give us `ns::Foo::A` right? ). One possible solution
would be adding a short name in `Symbol` (e.g. `ns::A` is a short name for
`ns::Foo::A`), and index/fuzzyFind can match on both names.
Comment at: clangd/index/SymbolCollector.cpp:365
+
+ using namespace clang::ast_matchers;
+ // For enumerators in unscoped enums that have names, even if they are not
drive-by nit: please pull this to a helper function.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47223
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41537: Optionally add code completion results for arrow instead of dot
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. Thanks for addressing the NITs. LGTM! https://reviews.llvm.org/D41537 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47067: Update NRVO logic to support early return
tzik updated this revision to Diff 148570.
tzik marked 2 inline comments as done.
tzik added a comment.
style fix. -O0 IR test.
Repository:
rC Clang
https://reviews.llvm.org/D47067
Files:
include/clang/AST/Decl.h
include/clang/Sema/Scope.h
lib/Sema/Scope.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaStmt.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriterDecl.cpp
test/CodeGenCXX/nrvo-noopt.cpp
test/CodeGenCXX/nrvo.cpp
test/SemaCXX/nrvo-ast.cpp
Index: test/SemaCXX/nrvo-ast.cpp
===
--- /dev/null
+++ test/SemaCXX/nrvo-ast.cpp
@@ -0,0 +1,139 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -ast-dump -o - %s | FileCheck %s
+
+struct X {
+ X();
+ X(const X&);
+ X(X&&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_00
+X test_00() {
+ // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+ X x;
+ return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_01
+X test_01(bool b) {
+ // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+ X x;
+ if (b)
+return x;
+ return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_02
+X test_02(bool b) {
+ // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+ X x;
+ // CHECK-NOT: VarDecl {{.*}} y {{.*}} nrvo
+ X y;
+ if (b)
+return y;
+ return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_03
+X test_03(bool b) {
+ if (b) {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+ }
+ // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+ X x;
+ return x;
+}
+
+extern "C" _Noreturn void exit(int) throw();
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_04
+X test_04(bool b) {
+ {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+if (b)
+ return x;
+ }
+ exit(1);
+}
+
+void may_throw();
+// CHECK-LABEL: FunctionDecl {{.*}} test_05
+X test_05() {
+ try {
+may_throw();
+return X();
+ } catch (X x) {
+// CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+return x;
+ }
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_06
+X test_06() {
+ // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+ X x __attribute__((aligned(8)));
+ return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_07
+X test_07(bool b) {
+ if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+ }
+ return X();
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_08
+X test_08(bool b) {
+ if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+ } else {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+ }
+}
+
+template
+struct Y {
+ Y();
+ // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+ // CHECK: VarDecl {{.*}} y 'Y' nrvo
+
+ // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+ // CHECK: VarDecl {{.*}} y 'Y' nrvo
+ static Y test_09() {
+Y y;
+return y;
+ }
+};
+
+struct Z {
+ Z(const X&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'A ()'
+// CHECK: VarDecl {{.*}} b 'B' nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'X ()'
+// CHECK: VarDecl {{.*}} b {{.*}} nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'Z ()'
+// CHECK-NOT: VarDecl {{.*}} b {{.*}} nrvo
+template
+A test_10() {
+ B b;
+ return b;
+}
+
+void instantiate() {
+ Y::test_09();
+ test_10();
+ test_10();
+}
Index: test/CodeGenCXX/nrvo.cpp
===
--- test/CodeGenCXX/nrvo.cpp
+++ test/CodeGenCXX/nrvo.cpp
@@ -130,17 +130,13 @@
}
// CHECK-LABEL: define void @_Z5test3b
-X test3(bool B) {
+X test3(bool B, X x) {
// CHECK: tail call {{.*}} @_ZN1XC1Ev
- // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
- // CHECK: call {{.*}} @_ZN1XC1Ev
- // CHECK: call {{.*}} @_ZN1XC1ERKS_
if (B) {
X y;
return y;
}
- // FIXME: we should NRVO this variable too.
- X x;
+ // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
return x;
}
@@ -191,21 +187,29 @@
}
// CHECK-LABEL: define void @_Z5test7b
+// CHECK-EH-LABEL: define void @_Z5test7b
X test7(bool b) {
// CHECK: tail call {{.*}} @_ZN1XC1Ev
// CHECK-NEXT: ret
+
+ // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+ // CHECK-EH-NEXT: ret
if (b) {
X x;
return x;
}
return X();
}
// CHECK-LABEL: define void @_Z5test8b
+// CHECK-EH-LABEL: define void @_Z5test8b
X test8(bool b) {
// CHECK: tail call {{.*}} @_ZN1XC1Ev
// CHECK-NEXT: ret
- if (b) {
+
+ // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+ // CHECK-EH-NEXT: ret
+if (b) {
X x;
return x;
} else {
@@ -221,4 +225,37 @@
// CHECK-LABEL: define linkonce_odr void @_ZN1YIiE1fEv
// CHECK: tail call {{.*}} @_ZN1YIiEC1Ev
+// CHECK-LABEL: define void @_Z6test10b
+X test10(bool B, X x) {
+ if (B) {
+// CHECK: tail call {{.*}} @_ZN1XC1ERKS_
+// CHECK-EH: tail call {{.*}} @_ZN1XC1ERKS_
+return x;
+ }
+ // CHECK: tail call {{.*}} @_ZN1XC1Ev
+ // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
+
+ // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+ // CHECK-EH-NOT: call {{.*}} @_ZN1XC1ERKS_
+ X y;
+ return y;
+}
+
+// CHECK-LABEL
[PATCH] D43341: [clang-doc] Implement reducer portion of the frontend framework
ioeric added a comment.
The reduce logic seems to be in a good shape. Some nits and questions inlined.
Comment at: clang-doc/Reducer.cpp:19
+
+#define REDUCE(INFO)
\
+ {
\
It seems that you could use a template function?
Comment at: clang-doc/Reducer.cpp:24
+for (auto &I : Values) {
\
+ if (!Tmp->merge(std::move(*static_cast(I.get()
\
+return nullptr;
\
Do we really want to give up all infos when one bad info/merge is present?
Comment at: clang-doc/Representation.h:138
+ SymbolID USR =
+ SymbolID(); // Unique identifier for the decl described by this Info.
+ const InfoType IT = InfoType::IT_default; // InfoType of this particular
Info.
Shouldn't USR be `SymbolID()` by default?
Comment at: clang-doc/Representation.h:146
+protected:
+ bool mergeBase(Info &&I);
};
It's a bit awkward that users have to dispatch from info types to the
corresponding `merge` function (as in `Reducer.cpp`). I think it would make
users' life easier if the library handles the dispatching.
I wonder if something like the following would be better:
```
struct Info {
std::unique_ptr merge(const Indo& LHS, const Info& RHS);
};
// A standalone function.
std::unique_ptr mergeInfo(const Info &LHS, const Info& RHS) {
// merge base info.
...
// merge subclass infos.
assert(LHS.IT == RHS.IT); // or return nullptr
switch (LHS.IT) {
...
return Namespace::merge(LHS, RHS);
}
}
struct NamespaceInfo : public Info {
std::unique_ptr merge(LHS, RHS);
};
```
The unhandled switch case warning in compiler would help you catch
unimplemented `merge` when new info types are added.
Comment at: clang-doc/tool/ClangDocMain.cpp:60
+static llvm::cl::opt
+DumpResult("dump",
If this only dumps the intermediate results, should we call it something like
`dump-intermediate` instead, to avoid confusion with final results?
Comment at: clang-doc/tool/ClangDocMain.cpp:148
+});
+return Err;
+ }
Print an error message on error?
Comment at: clang-doc/tool/ClangDocMain.cpp:154
+ llvm::StringMap>> MapOutput;
+ Exec->get()->getToolResults()->forEachResult([&](StringRef Key,
+ StringRef Value) {
Could you add a brief comment explaining what key and value are?
Comment at: clang-doc/tool/ClangDocMain.cpp:161
+for (auto &I : Infos) {
+ llvm::errs() << "COLLECT: " << llvm::toHex(llvm::toStringRef(I->USR))
+ << ": " << I->Name << "\n";
nit: remove debug logging?
Comment at: clang-doc/tool/ClangDocMain.cpp:170
+ // Reducing phase
+ llvm::outs() << "Reducing infos...\n";
+ for (auto &Group : MapOutput) {
nit: maybe also output the number of entries collected in the map?
Comment at: clang-doc/tool/ClangDocMain.cpp:181
+doc::writeInfo(I.get(), Buffer);
+ if (DumpResultToFile("bc", Group.getKey() + ".bc", Buffer))
+return 1;
(Sorry that I might be missing context here.)
Could you please explain the incentive for dumping each info group to one bc
file? If the key identifies a symbol (e.g. USR), wouldn't this result in a
bitcode file being created for each symbol? This doesn't seem very scalable.
https://reviews.llvm.org/D43341
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects
dstenb updated this revision to Diff 148577.
dstenb retitled this revision from "[Tooling] Clean up tmp files when creating
a fixed compilation database" to "[Driver] Clean up tmp files when deleting
Compilation objects".
dstenb edited the summary of this revision.
dstenb added a comment.
I have now updated the patch so that the files are removed when deleting
Compilation objects.
Expect for the fact that we now also remove files created in
stripPositionalArgs, the intention is that the removal behavior remains as
before.
https://reviews.llvm.org/D45686
Files:
include/clang/Driver/Compilation.h
lib/Driver/Compilation.cpp
lib/Driver/Driver.cpp
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1243,9 +1243,6 @@
// If any of the preprocessing commands failed, clean up and exit.
if (!FailingCommands.empty()) {
-if (!isSaveTempsEnabled())
- C.CleanupFileList(C.getTempFiles(), true);
-
Diag(clang::diag::note_drv_command_failed_diag_msg)
<< "Error generating preprocessed source(s).";
return;
@@ -1362,9 +1359,6 @@
C.ExecuteJobs(C.getJobs(), FailingCommands);
- // Remove temp files.
- C.CleanupFileList(C.getTempFiles());
-
// If the command succeeded, we are done.
if (FailingCommands.empty())
return 0;
Index: lib/Driver/Compilation.cpp
===
--- lib/Driver/Compilation.cpp
+++ lib/Driver/Compilation.cpp
@@ -38,13 +38,19 @@
InputArgList *_Args, DerivedArgList *_TranslatedArgs,
bool ContainsError)
: TheDriver(D), DefaultToolChain(_DefaultToolChain), Args(_Args),
- TranslatedArgs(_TranslatedArgs), ContainsError(ContainsError) {
+ TranslatedArgs(_TranslatedArgs), ContainsError(ContainsError),
+ SaveTempsEnabled(D.isSaveTempsEnabled()) {
// The offloading host toolchain is the default toolchain.
OrderedOffloadingToolchains.insert(
std::make_pair(Action::OFK_Host, &DefaultToolChain));
}
Compilation::~Compilation() {
+ // Remove temporary files. This must be done before arguments are freed, as
+ // the file names might be derived from the input arguments.
+ if (!SaveTempsEnabled)
+CleanupFileList(TempFiles);
+
delete TranslatedArgs;
delete Args;
@@ -245,6 +251,10 @@
AllActions.clear();
Jobs.clear();
+ // Remove temporary files.
+ if (!SaveTempsEnabled)
+CleanupFileList(TempFiles);
+
// Clear temporary/results file lists.
TempFiles.clear();
ResultFiles.clear();
@@ -262,6 +272,9 @@
// Redirect stdout/stderr to /dev/null.
Redirects = {None, {""}, {""}};
+
+ // Temporary files added by diagnostics should be kept.
+ SaveTempsEnabled = true;
}
StringRef Compilation::getSysRoot() const {
Index: include/clang/Driver/Compilation.h
===
--- include/clang/Driver/Compilation.h
+++ include/clang/Driver/Compilation.h
@@ -122,6 +122,9 @@
/// Whether an error during the parsing of the input args.
bool ContainsError;
+ /// Whether to save temporary files.
+ bool SaveTempsEnabled;
+
public:
Compilation(const Driver &D, const ToolChain &DefaultToolChain,
llvm::opt::InputArgList *Args,
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1243,9 +1243,6 @@
// If any of the preprocessing commands failed, clean up and exit.
if (!FailingCommands.empty()) {
-if (!isSaveTempsEnabled())
- C.CleanupFileList(C.getTempFiles(), true);
-
Diag(clang::diag::note_drv_command_failed_diag_msg)
<< "Error generating preprocessed source(s).";
return;
@@ -1362,9 +1359,6 @@
C.ExecuteJobs(C.getJobs(), FailingCommands);
- // Remove temp files.
- C.CleanupFileList(C.getTempFiles());
-
// If the command succeeded, we are done.
if (FailingCommands.empty())
return 0;
Index: lib/Driver/Compilation.cpp
===
--- lib/Driver/Compilation.cpp
+++ lib/Driver/Compilation.cpp
@@ -38,13 +38,19 @@
InputArgList *_Args, DerivedArgList *_TranslatedArgs,
bool ContainsError)
: TheDriver(D), DefaultToolChain(_DefaultToolChain), Args(_Args),
- TranslatedArgs(_TranslatedArgs), ContainsError(ContainsError) {
+ TranslatedArgs(_TranslatedArgs), ContainsError(ContainsError),
+ SaveTempsEnabled(D.isSaveTempsEnabled()) {
// The offloading host toolchain is the default toolchain.
OrderedOffloadingToolchains.insert(
std::make_pair(Action::OFK_Host, &DefaultToolChain));
}
Compilation::~Compilation() {
+ // Remove temporary files. This must be done before arguments are freed, as
+ // the file names might be derived from the input
[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects
lebedev.ri added inline comments. Comment at: include/clang/Driver/Compilation.h:126 + /// Whether to save temporary files. + bool SaveTempsEnabled; + `KeepTempFiles`? Comment at: lib/Driver/Compilation.cpp:276-277 + + // Temporary files added by diagnostics should be kept. + SaveTempsEnabled = true; } Is there a test that breaks without this? https://reviews.llvm.org/D45686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.
CarlosAlbertoEnciso updated this revision to Diff 148578.
CarlosAlbertoEnciso edited the summary of this revision.
CarlosAlbertoEnciso added a comment.
This patch addresses the reviewers comments:
1. Merge the -Wunused-local-typedefs and -Wunused-usings implementations
2. Update the Release Notes
3. Review the test cases
https://reviews.llvm.org/D44826
Files:
docs/ReleaseNotes.rst
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/ExternalSemaSource.h
include/clang/Sema/MultiplexExternalSemaSource.h
include/clang/Sema/Sema.h
include/clang/Sema/SemaInternal.h
include/clang/Serialization/ASTBitCodes.h
include/clang/Serialization/ASTReader.h
lib/Sema/MultiplexExternalSemaSource.cpp
lib/Sema/Sema.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaDeclCXX.cpp
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTWriter.cpp
test/FixIt/fixit.cpp
test/Modules/Inputs/module.map
test/Modules/Inputs/warn-unused-using.h
test/Modules/warn-unused-using.cpp
test/PCH/cxx-templates.cpp
test/SemaCXX/coreturn.cpp
test/SemaCXX/referenced_alias_declaration_1.cpp
test/SemaCXX/referenced_alias_declaration_2.cpp
test/SemaCXX/referenced_using_all.cpp
test/SemaCXX/referenced_using_declaration_1.cpp
test/SemaCXX/referenced_using_declaration_2.cpp
test/SemaCXX/referenced_using_directive.cpp
Index: test/SemaCXX/referenced_using_directive.cpp
===
--- test/SemaCXX/referenced_using_directive.cpp
+++ test/SemaCXX/referenced_using_directive.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+ typedef int Integer;
+ int var;
+}
+
+void Fa() {
+ using namespace N; // Referenced
+ var = 1;
+}
+
+void Fb() {
+ using namespace N; // expected-warning {{unused using ''}}
+ N::var = 1;
+}
+
+void Fc() {
+ using namespace N; // Referenced
+ Integer var = 1;
+}
+
+void Fd() {
+ using namespace N; // expected-warning {{unused using ''}}
+ N::Integer var = 1;
+}
Index: test/SemaCXX/referenced_using_declaration_2.cpp
===
--- test/SemaCXX/referenced_using_declaration_2.cpp
+++ test/SemaCXX/referenced_using_declaration_2.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+ typedef int Integer;
+ typedef char Char;
+}
+
+using N::Integer; // expected-warning {{unused using 'Integer'}}
+using N::Char; // Referenced
+
+void Foo(int p1, N::Integer p2, Char p3) {
+ N::Integer var;
+ var = 0;
+}
+
+using N::Integer; // Referenced
+Integer Bar() {
+ using N::Char;// expected-warning {{unused using 'Char'}}
+ return 0;
+}
Index: test/SemaCXX/referenced_using_declaration_1.cpp
===
--- test/SemaCXX/referenced_using_declaration_1.cpp
+++ test/SemaCXX/referenced_using_declaration_1.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace N {
+ // Types.
+ typedef int Integer;
+ struct Record {
+int a;
+ };
+
+ // Variables.
+ int var1;
+ int var2;
+
+ // Functions.
+ void func1();
+ void func2();
+}
+
+using N::Integer; // expected-warning {{unused using 'Integer'}}
+using N::Record;// expected-warning {{unused using 'Record'}}
+using N::var1; // expected-warning {{unused using 'var1'}}
+using N::var2; // expected-warning {{unused using 'var2'}}
+using N::func1; // expected-warning {{unused using 'func1'}}
+using N::func2; // expected-warning {{unused using 'func2'}}
+
+void Foo() {
+ using N::Integer; // expected-warning {{unused using 'Integer'}}
+ N::Integer int_var;
+ int_var = 1;
+
+ using N::Record; // Referenced
+ Record rec_var;
+ rec_var.a = 2;
+
+ using N::var1;// expected-warning {{unused using 'var1'}}
+ N::var1 = 3;
+
+ using N::var2;// Referenced
+ var2 = 4;
+
+ using N::func1; // expected-warning {{unused using 'func1'}}
+ N::func1();
+
+ using N::func2; // Referenced
+ func2();
+}
Index: test/SemaCXX/referenced_using_all.cpp
===
--- test/SemaCXX/referenced_using_all.cpp
+++ test/SemaCXX/referenced_using_all.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s
+
+namespace A {
+ typedef char Char;
+ typedef int Integer;
+ typedef float Float;
+ int var;
+}
+
+using A::Char; // expected-warning {{unused using 'Char'}}
+using A::Integer; // Referenced
+using A::Float; // expected-warning {{unused using 'Float'}}
+
+namespace B {
+ using A::Char;// Referenced
+ template
+ T FuncTempl(T p1,Char p2) {
+using A::Float; // Referenced
+typedef Float Type;
+Integer I;
+return p1;
+ }
+}
+
+using A::Char;
[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.
CarlosAlbertoEnciso marked 2 inline comments as done. CarlosAlbertoEnciso added inline comments. Comment at: lib/Serialization/ASTReader.cpp:8101-8103 +UsingDecl *D = dyn_cast_or_null( +GetDecl(UnusedUsingCandidates[I])); +if (D) dblaikie wrote: > roll the declaration into the condition, perhaps: > > if (auto *D = dyn_cast_or_null...) > Decls.insert(D); Done. Comment at: test/Modules/warn-unused-using.cpp:5 + +// For modules, the warning should only fire the first time, when the module is +// built. dblaikie wrote: > This would only warn for the unused local using, but wouldn't fire for an > namespace-scoped unused using? Perhaps there should be a test for that? I have added a namespace-scoped unused using. https://reviews.llvm.org/D44826 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47223: [clangd] Handle enumerators in named, unscoped enums similarly to scoped enums
sammccall added a comment.
In https://reviews.llvm.org/D47223#1110571, @malaperle wrote:
> In https://reviews.llvm.org/D47223#1109247, @ilya-biryukov wrote:
>
> > I'm not sure if we have tests for that, but I remember that we kept the
> > enumerators in the outer scope so that completion could find them..
> > Am I right that this patch will change the behavior and we won't get
> > enumerators in the following example:
> >
> > /// foo.h
> > enum Foo {
> > A, B, C
> > };
> >
> > /// foo.cpp
> > #include "foo.h"
> >
> > int a = ^ // <-- A, B, C should be in completion list here.
> >
>
>
> Not quite but still potentially problematic. With the patch, A, B, C would be
> found but not ::A, ::B, ::C.
I don't understand how this would still work (at least when using the index).
The index call would have Scopes set to the enclosing scopes {""}, which is
equivalent to a search for ::A. Maybe I'm missing something.
In https://reviews.llvm.org/D47223#1112118, @ioeric wrote:
> I think the fundamental problem here is that C++ symbols (esp. enum
> constants) can have different names (e.g. ns::Foo::A and ns::A).
Yup. There are a few other instances of this:
1. decls aliased with using decls - we model these as duplicate symbols
2. decls aliased with using directives - we ask clang to resolve the namespaces
in scope when searching
3. inline namespaces - we mostly get away with ignoring them
AFAICS none of these are ideal for enums. (2 fails because the list of
namespaces would get too long).
This is messy :-(
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47223
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47067: Update NRVO logic to support early return
tzik added inline comments.
Comment at: lib/Sema/Scope.cpp:122-123
+void Scope::setNRVOCandidate(VarDecl *Candidate) {
+ for (auto* D : DeclsInScope) {
+VarDecl* VD = dyn_cast_or_null(D);
+if (VD && VD != Candidate && VD->isNRVOCandidate())
rsmith wrote:
> `*` on the right, please. Also `auto` -> `Decl` would be clearer and no
> longer. Is `dyn_cast_or_null` really necessary? (Can `DeclsInScope` contain
> `nullptr`?) I would expect that just `dyn_cast` would suffice.
Done! Right, contents in `DeclsInScope` should be non-null, and dyn_cast should
work well.
Comment at: lib/Sema/SemaDecl.cpp:12619-12620
void Sema::computeNRVO(Stmt *Body, FunctionScopeInfo *Scope) {
- ReturnStmt **Returns = Scope->Returns.data();
+ for (ReturnStmt* Return : Scope->Returns) {
+const VarDecl* Candidate = Return->getNRVOCandidate();
+if (!Candidate)
rsmith wrote:
> `*` on the right, please.
Done
Repository:
rC Clang
https://reviews.llvm.org/D47067
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44826: Add -Wunused-using, a warning that finds unused using declarations.
lebedev.ri added a comment. Please improve test coverage a bit. `-Wunused-using` is part of `-Wunused`. And `-Wunused` is part of `-Wall`. That needs to be tested. Also, one small test to show that it is not on by default, and `-Wno-unused-using` actually disables it is needed. Comment at: docs/ReleaseNotes.rst:117 + + -Wall now includes the new warning flag `-Wunused-using`, which emits a + warning on using statements that are not used. This may result in new ``` ``-Wall`` ``` Comment at: docs/ReleaseNotes.rst:117 + + -Wall now includes the new warning flag `-Wunused-using`, which emits a + warning on using statements that are not used. This may result in new lebedev.ri wrote: > ``` > ``-Wall`` > > ``` ``` ``-Wunused-using`` ``` Comment at: include/clang/Basic/DiagnosticGroups.td:828-829 // -Wunused-local-typedefs = -Wunused-local-typedef +def : DiagGroup<"unused-usings", [UnusedUsing]>; +// -Wunused-usings = -Wunused-using Why? gcc compatibility? Comment at: include/clang/Basic/DiagnosticSemaKinds.td:282-284 +def warn_unused_using : Warning< + "unused using %0">, + InGroup, DefaultIgnore; http://en.cppreference.com/w/cpp/keyword/using says that there are multiple flavors. It may be more friendlier to have more than one single message for all of them. Comment at: test/SemaCXX/referenced_alias_declaration_1.cpp:1 +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s + The svn attribute changes are likely unintentional. Comment at: test/SemaCXX/referenced_alias_declaration_2.cpp:1 +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s + The svn attribute changes are likely unintentional. Comment at: test/SemaCXX/referenced_using_all.cpp:1 +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s + The svn attribute changes are likely unintentional. Comment at: test/SemaCXX/referenced_using_declaration_1.cpp:1 +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s + The svn attribute changes are likely unintentional. Comment at: test/SemaCXX/referenced_using_declaration_2.cpp:1 +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s + The svn attribute changes are likely unintentional. Comment at: test/SemaCXX/referenced_using_directive.cpp:1 +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wunused-using -verify %s + The svn attribute changes are likely unintentional. https://reviews.llvm.org/D44826 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47058: [ASTImporter] Fix ClassTemplateSpecialization in wrong DC
This revision was automatically updated to reflect the committed changes.
Closed by commit rL333269: [ASTImporter] Fix ClassTemplateSpecialization in
wrong DC (authored by martong, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D47058
Files:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp
Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -4320,9 +4320,13 @@
D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind());
-// Add the specialization to this context.
+// Set the context of this specialization/instantiation.
D2->setLexicalDeclContext(LexicalDC);
-LexicalDC->addDeclInternal(D2);
+
+// Add to the DC only if it was an explicit specialization/instantiation.
+if (D2->isExplicitInstantiationOrSpecialization()) {
+ LexicalDC->addDeclInternal(D2);
+}
}
Importer.Imported(D, D2);
if (D->isCompleteDefinition() && ImportDefinition(D, D2))
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -1214,7 +1214,7 @@
TEST_P(
ASTImporterTestBase,
-
DISABLED_TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) {
+TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) {
Decl *From, *To;
std::tie(From, To) = getImportedDecl(
Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -4320,9 +4320,13 @@
D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind());
-// Add the specialization to this context.
+// Set the context of this specialization/instantiation.
D2->setLexicalDeclContext(LexicalDC);
-LexicalDC->addDeclInternal(D2);
+
+// Add to the DC only if it was an explicit specialization/instantiation.
+if (D2->isExplicitInstantiationOrSpecialization()) {
+ LexicalDC->addDeclInternal(D2);
+}
}
Importer.Imported(D, D2);
if (D->isCompleteDefinition() && ImportDefinition(D, D2))
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -1214,7 +1214,7 @@
TEST_P(
ASTImporterTestBase,
-DISABLED_TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) {
+TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) {
Decl *From, *To;
std::tie(From, To) = getImportedDecl(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333269 - [ASTImporter] Fix ClassTemplateSpecialization in wrong DC
Author: martong
Date: Fri May 25 04:21:24 2018
New Revision: 333269
URL: http://llvm.org/viewvc/llvm-project?rev=333269&view=rev
Log:
[ASTImporter] Fix ClassTemplateSpecialization in wrong DC
Summary:
ClassTemplateSpecialization is put in the wrong DeclContex if implicitly
instantiated. This patch fixes it.
Reviewers: a.sidorin, r.stahl, xazax.hun
Subscribers: rnkovacs, dkrupp, cfe-commits
Differential Revision: https://reviews.llvm.org/D47058
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=333269&r1=333268&r2=333269&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Fri May 25 04:21:24 2018
@@ -4320,9 +4320,13 @@ Decl *ASTNodeImporter::VisitClassTemplat
D2->setTemplateSpecializationKind(D->getTemplateSpecializationKind());
-// Add the specialization to this context.
+// Set the context of this specialization/instantiation.
D2->setLexicalDeclContext(LexicalDC);
-LexicalDC->addDeclInternal(D2);
+
+// Add to the DC only if it was an explicit specialization/instantiation.
+if (D2->isExplicitInstantiationOrSpecialization()) {
+ LexicalDC->addDeclInternal(D2);
+}
}
Importer.Imported(D, D2);
if (D->isCompleteDefinition() && ImportDefinition(D, D2))
Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=333269&r1=333268&r2=333269&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Fri May 25 04:21:24 2018
@@ -1214,7 +1214,7 @@ TEST_P(ASTImporterTestBase, TUshouldNotC
TEST_P(
ASTImporterTestBase,
-
DISABLED_TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) {
+TUshouldNotContainClassTemplateSpecializationOfImplicitInstantiation) {
Decl *From, *To;
std::tie(From, To) = getImportedDecl(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47067: Update NRVO logic to support early return
xbolva00 added inline comments. Comment at: lib/Sema/Scope.cpp:128 - if (getEntity()) -return; - - if (NRVO.getInt()) -getParent()->setNoNRVO(); - else if (NRVO.getPointer()) -getParent()->addNRVOCandidate(NRVO.getPointer()); + if (getParent()) +getParent()->setNRVOCandidate(Candidate); auto * Parent = getParent(); if (Parent) Parent>setNRVOCandidate(Candidate); ? Repository: rC Clang https://reviews.llvm.org/D47067 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47367: [ASTImporter] Add ms compatibility to tests which use the TestBase
martong created this revision.
martong added reviewers: a.sidorin, r.stahl, xazax.hun.
Herald added subscribers: cfe-commits, dkrupp, rnkovacs.
In order to avoid build failures on MS, we use -fms-compatibility too in the
tests which use the TestBase.
Repository:
rC Clang
https://reviews.llvm.org/D47367
Files:
unittests/AST/ASTImporterTest.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1563,10 +1563,6 @@
.match(ToTU, classTemplateSpecializationDecl()));
}
-INSTANTIATE_TEST_CASE_P(
-ParameterizedTests, ASTImporterTestBase,
-::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),);
-
struct ImportFunctions : ASTImporterTestBase {};
TEST_P(ImportFunctions,
@@ -1791,10 +1787,6 @@
EXPECT_TRUE(To->isVirtual());
}
-INSTANTIATE_TEST_CASE_P(
-ParameterizedTests, ImportFunctions,
-::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),);
-
AST_MATCHER_P(TagDecl, hasTypedefForAnonDecl, Matcher,
InnerMatcher) {
if (auto *Typedef = Node.getTypedefNameForAnonDecl())
@@ -1925,9 +1917,20 @@
EXPECT_FALSE(NS->containsDecl(Spec));
}
-INSTANTIATE_TEST_CASE_P(
-ParameterizedTests, DeclContextTest,
-::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),);
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest,
+::testing::Values(ArgVector()), );
+
+auto DefaultTestValuesForRunOptions = ::testing::Values(
+ArgVector(),
+ArgVector{"-fdelayed-template-parsing"},
+ArgVector{"-fms-compatibility"},
+ArgVector{"-fdelayed-template-parsing", "-fms-compatibility"});
+
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterTestBase,
+DefaultTestValuesForRunOptions, );
+
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctions,
+DefaultTestValuesForRunOptions, );
} // end namespace ast_matchers
} // end namespace clang
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1563,10 +1563,6 @@
.match(ToTU, classTemplateSpecializationDecl()));
}
-INSTANTIATE_TEST_CASE_P(
-ParameterizedTests, ASTImporterTestBase,
-::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),);
-
struct ImportFunctions : ASTImporterTestBase {};
TEST_P(ImportFunctions,
@@ -1791,10 +1787,6 @@
EXPECT_TRUE(To->isVirtual());
}
-INSTANTIATE_TEST_CASE_P(
-ParameterizedTests, ImportFunctions,
-::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),);
-
AST_MATCHER_P(TagDecl, hasTypedefForAnonDecl, Matcher,
InnerMatcher) {
if (auto *Typedef = Node.getTypedefNameForAnonDecl())
@@ -1925,9 +1917,20 @@
EXPECT_FALSE(NS->containsDecl(Spec));
}
-INSTANTIATE_TEST_CASE_P(
-ParameterizedTests, DeclContextTest,
-::testing::Values(ArgVector(), ArgVector{"-fdelayed-template-parsing"}),);
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, DeclContextTest,
+::testing::Values(ArgVector()), );
+
+auto DefaultTestValuesForRunOptions = ::testing::Values(
+ArgVector(),
+ArgVector{"-fdelayed-template-parsing"},
+ArgVector{"-fms-compatibility"},
+ArgVector{"-fdelayed-template-parsing", "-fms-compatibility"});
+
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterTestBase,
+DefaultTestValuesForRunOptions, );
+
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctions,
+DefaultTestValuesForRunOptions, );
} // end namespace ast_matchers
} // end namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46950: [ASTImporter] Fix duplicate class template definitions problem
martong added a comment. @a.sidorin Yes, that is a very good idea. Just created a new patch which adds that switch for the tests which use the TestBase. https://reviews.llvm.org/D47367 Repository: rC Clang https://reviews.llvm.org/D46950 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47367: [ASTImporter] Add ms compatibility to tests which use the TestBase
martong added a comment. This patch does not target `testImport` because I am not sure how to handle the options there: auto RunOptsFrom = getRunOptionsForLanguage(FromLang); auto RunOptsTo = getRunOptionsForLanguage(ToLang); for (const auto &FromArgs : RunOptsFrom) for (const auto &ToArgs : RunOptsTo) EXPECT_TRUE(testImport(FromCode, FromArgs, ToCode, ToArgs, Verifier, AMatcher)); I think, it is overkill to test all possible combinations of the options, we should come up with something better here. Perhaps we could exploit parameterized tests here as well. @a.sidorin, @xazax.hun What is your opinion? Repository: rC Clang https://reviews.llvm.org/D47367 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47097: [DebugInfo] Preserve scope in auto generated StoreInst
gramanas updated this revision to Diff 148588.
gramanas added a comment.
I added a test that illustrates what @vsk is talking about.
Sorry for not providing the relevant information but I thought this would
be a simple one liner like https://reviews.llvm.org/rL327800. I will update the
revision's
description to include everything I know about this.
Repository:
rC Clang
https://reviews.llvm.org/D47097
Files:
lib/CodeGen/CGDecl.cpp
test/CodeGen/debug-info-preserve-scope.c
Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+
+// RUN: %clang_cc1 -O1 -debug-info-kind=limited -emit-llvm -mllvm \
+// RUN: -opt-bisect-limit=2 -o - %s 2> /dev/null | FileCheck %s \
+// RUN: --check-prefix PHI
+
+extern int map[];
+// PHI-LABEL: define void @test1
+void test1(int a, int n) {
+ for (int i = 0; i < n; ++i)
+a = map[a];
+}
+
+// PHI: for.cond:
+// PHI-NEXT: {{.*}} = phi i32 {{.*}} !dbg ![[test1DbgLoc:[0-9]+]]
+
+// PHI: ![[test1DbgLoc]] = !DILocation(line: 0
+
+
+static int i;
+// CHECK-LABEL: define void @test2
+void test2(int b) {
+ i = b;
+}
+
+// CHECK: store i32 {{.*}} !dbg ![[test2DbgLoc:[0-9]+]]
+
+// CHECK: ![[test2DbgLoc]] = !DILocation(line: 0
+
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1946,6 +1946,9 @@
}
}
+ // Set artificial debug location in order to preserve the scope
+ auto DL = ApplyDebugLocation::CreateArtificial(*this);
+
Address DeclPtr = Address::invalid();
bool DoStore = false;
bool IsScalar = hasScalarEvaluationKind(Ty);
Index: test/CodeGen/debug-info-preserve-scope.c
===
--- /dev/null
+++ test/CodeGen/debug-info-preserve-scope.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -debug-info-kind=limited -emit-llvm -o - %s | FileCheck %s
+
+// RUN: %clang_cc1 -O1 -debug-info-kind=limited -emit-llvm -mllvm \
+// RUN: -opt-bisect-limit=2 -o - %s 2> /dev/null | FileCheck %s \
+// RUN: --check-prefix PHI
+
+extern int map[];
+// PHI-LABEL: define void @test1
+void test1(int a, int n) {
+ for (int i = 0; i < n; ++i)
+a = map[a];
+}
+
+// PHI: for.cond:
+// PHI-NEXT: {{.*}} = phi i32 {{.*}} !dbg ![[test1DbgLoc:[0-9]+]]
+
+// PHI: ![[test1DbgLoc]] = !DILocation(line: 0
+
+
+static int i;
+// CHECK-LABEL: define void @test2
+void test2(int b) {
+ i = b;
+}
+
+// CHECK: store i32 {{.*}} !dbg ![[test2DbgLoc:[0-9]+]]
+
+// CHECK: ![[test2DbgLoc]] = !DILocation(line: 0
+
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1946,6 +1946,9 @@
}
}
+ // Set artificial debug location in order to preserve the scope
+ auto DL = ApplyDebugLocation::CreateArtificial(*this);
+
Address DeclPtr = Address::invalid();
bool DoStore = false;
bool IsScalar = hasScalarEvaluationKind(Ty);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
ebevhan added a comment. With the exception of the two inline comments, this looks good to me now! Comment at: include/clang/Basic/LangOptions.def:301 +LANGOPT(FixedPointEnabled, 1, 0, "Fixed point types are enabled") + Just a minor nit... The 'Enabled' is superfluous. Comment at: include/clang/Driver/Options.td:882 +def enable_fixed_point : Flag<["-", "--"], "enable-fixed-point">, Group, + Flags<[CC1Option]>, HelpText<"Enable fixed point types">; Shouldn't this be an `-f` flag, like `-ffixed-point`? I'm not for or against either, just wondering what anyone else thinks. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333272 - Optionally add code completion results for arrow instead of dot
Author: yvvan Date: Fri May 25 05:56:26 2018 New Revision: 333272 URL: http://llvm.org/viewvc/llvm-project?rev=333272&view=rev Log: Optionally add code completion results for arrow instead of dot Currently getting such completions requires source correction, reparsing and calling completion again. And if it shows no results and rollback is required then it costs one more reparse. With this change it's possible to get all results which can be later filtered to split changes which require correction. Differential Revision: https://reviews.llvm.org/D41537 Modified: cfe/trunk/include/clang/Driver/CC1Options.td cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h cfe/trunk/include/clang/Sema/CodeCompleteOptions.h cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Frontend/ASTUnit.cpp cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/lib/Parse/ParseExpr.cpp cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp cfe/trunk/lib/Sema/SemaCodeComplete.cpp cfe/trunk/test/CodeCompletion/member-access.cpp Modified: cfe/trunk/include/clang/Driver/CC1Options.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=333272&r1=333271&r2=333272&view=diff == --- cfe/trunk/include/clang/Driver/CC1Options.td (original) +++ cfe/trunk/include/clang/Driver/CC1Options.td Fri May 25 05:56:26 2018 @@ -445,6 +445,8 @@ def no_code_completion_ns_level_decls : HelpText<"Do not include declarations inside namespaces (incl. global namespace) in the code-completion results.">; def code_completion_brief_comments : Flag<["-"], "code-completion-brief-comments">, HelpText<"Include brief documentation comments in code-completion results.">; +def code_completion_with_fixits : Flag<["-"], "code-completion-with-fixits">, + HelpText<"Include code completion results which require small fix-its.">; def disable_free : Flag<["-"], "disable-free">, HelpText<"Disable freeing of memory on exit">; def discard_value_names : Flag<["-"], "discard-value-names">, Modified: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h?rev=333272&r1=333271&r2=333272&view=diff == --- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h (original) +++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h Fri May 25 05:56:26 2018 @@ -783,6 +783,33 @@ public: /// The availability of this result. CXAvailabilityKind Availability = CXAvailability_Available; + /// FixIts that *must* be applied before inserting the text for the + /// corresponding completion item. + /// + /// Completion items with non-empty fixits will not be returned by default, + /// they should be explicitly requested by setting + /// CompletionOptions::IncludeFixIts. For the editors to be able to + /// compute position of the cursor for the completion item itself, the + /// following conditions are guaranteed to hold for RemoveRange of the stored + /// fixits: + /// - Ranges in the fixits are guaranteed to never contain the completion + /// point (or identifier under completion point, if any) inside them, except + /// at the start or at the end of the range. + /// - If a fixit range starts or ends with completion point (or starts or + /// ends after the identifier under completion point), it will contain at + /// least one character. It allows to unambiguously recompute completion + /// point after applying the fixit. + /// The intuition is that provided fixits change code around the identifier we + /// complete, but are not allowed to touch the identifier itself or the + /// completion point. One example of completion items with corrections are the + /// ones replacing '.' with '->' and vice versa: + /// std::unique_ptr> vec_ptr; + /// In 'vec_ptr.^', one of completion items is 'push_back', it requires + /// replacing '.' with '->'. + /// In 'vec_ptr->^', one of completion items is 'release', it requires + /// replacing '->' with '.'. + std::vector FixIts; + /// Whether this result is hidden by another name. bool Hidden : 1; @@ -807,15 +834,17 @@ public: NestedNameSpecifier *Qualifier = nullptr; /// Build a result that refers to a declaration. - CodeCompletionResult(const NamedDecl *Declaration, - unsigned Priority, + CodeCompletionResult(const NamedDecl *Declaration, unsigned Priority, NestedNameSpecifier *Qualifier = nullptr, bool QualifierIsInformative = false, - bool Accessible = true) + bool Accessible = true, + std::vector FixIts = std::vector()) : Declaration(Declaration), Priority(Priority), Kind(RK_Declaration), Hidden(false), QualifierIsInformative(QualifierIsInfo
[PATCH] D41537: Optionally add code completion results for arrow instead of dot
This revision was automatically updated to reflect the committed changes. Closed by commit rL333272: Optionally add code completion results for arrow instead of dot (authored by yvvan, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D41537?vs=148553&id=148591#toc Repository: rL LLVM https://reviews.llvm.org/D41537 Files: cfe/trunk/include/clang/Driver/CC1Options.td cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h cfe/trunk/include/clang/Sema/CodeCompleteOptions.h cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Frontend/ASTUnit.cpp cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/lib/Parse/ParseExpr.cpp cfe/trunk/lib/Sema/CodeCompleteConsumer.cpp cfe/trunk/lib/Sema/SemaCodeComplete.cpp cfe/trunk/test/CodeCompletion/member-access.cpp Index: cfe/trunk/include/clang/Driver/CC1Options.td === --- cfe/trunk/include/clang/Driver/CC1Options.td +++ cfe/trunk/include/clang/Driver/CC1Options.td @@ -445,6 +445,8 @@ HelpText<"Do not include declarations inside namespaces (incl. global namespace) in the code-completion results.">; def code_completion_brief_comments : Flag<["-"], "code-completion-brief-comments">, HelpText<"Include brief documentation comments in code-completion results.">; +def code_completion_with_fixits : Flag<["-"], "code-completion-with-fixits">, + HelpText<"Include code completion results which require small fix-its.">; def disable_free : Flag<["-"], "disable-free">, HelpText<"Disable freeing of memory on exit">; def discard_value_names : Flag<["-"], "discard-value-names">, Index: cfe/trunk/include/clang/Sema/Sema.h === --- cfe/trunk/include/clang/Sema/Sema.h +++ cfe/trunk/include/clang/Sema/Sema.h @@ -10231,7 +10231,7 @@ struct CodeCompleteExpressionData; void CodeCompleteExpression(Scope *S, const CodeCompleteExpressionData &Data); - void CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base, + void CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base, Expr *OtherOpBase, SourceLocation OpLoc, bool IsArrow, bool IsBaseExprStatement); void CodeCompletePostfixExpression(Scope *S, ExprResult LHS); Index: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h === --- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h +++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h @@ -783,6 +783,33 @@ /// The availability of this result. CXAvailabilityKind Availability = CXAvailability_Available; + /// FixIts that *must* be applied before inserting the text for the + /// corresponding completion item. + /// + /// Completion items with non-empty fixits will not be returned by default, + /// they should be explicitly requested by setting + /// CompletionOptions::IncludeFixIts. For the editors to be able to + /// compute position of the cursor for the completion item itself, the + /// following conditions are guaranteed to hold for RemoveRange of the stored + /// fixits: + /// - Ranges in the fixits are guaranteed to never contain the completion + /// point (or identifier under completion point, if any) inside them, except + /// at the start or at the end of the range. + /// - If a fixit range starts or ends with completion point (or starts or + /// ends after the identifier under completion point), it will contain at + /// least one character. It allows to unambiguously recompute completion + /// point after applying the fixit. + /// The intuition is that provided fixits change code around the identifier we + /// complete, but are not allowed to touch the identifier itself or the + /// completion point. One example of completion items with corrections are the + /// ones replacing '.' with '->' and vice versa: + /// std::unique_ptr> vec_ptr; + /// In 'vec_ptr.^', one of completion items is 'push_back', it requires + /// replacing '.' with '->'. + /// In 'vec_ptr->^', one of completion items is 'release', it requires + /// replacing '->' with '.'. + std::vector FixIts; + /// Whether this result is hidden by another name. bool Hidden : 1; @@ -807,15 +834,17 @@ NestedNameSpecifier *Qualifier = nullptr; /// Build a result that refers to a declaration. - CodeCompletionResult(const NamedDecl *Declaration, - unsigned Priority, + CodeCompletionResult(const NamedDecl *Declaration, unsigned Priority, NestedNameSpecifier *Qualifier = nullptr, bool QualifierIsInformative = false, - bool Accessible = true) + bool Accessible = true, + std::vector FixIts = std::vector()) : Declaration(Declaration), Priority(Priori
[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects
dstenb updated this revision to Diff 148592.
dstenb marked an inline comment as done.
dstenb added a comment.
Renamed SaveTempsEnabled field to KeepTempFiles.
https://reviews.llvm.org/D45686
Files:
include/clang/Driver/Compilation.h
lib/Driver/Compilation.cpp
lib/Driver/Driver.cpp
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1243,9 +1243,6 @@
// If any of the preprocessing commands failed, clean up and exit.
if (!FailingCommands.empty()) {
-if (!isSaveTempsEnabled())
- C.CleanupFileList(C.getTempFiles(), true);
-
Diag(clang::diag::note_drv_command_failed_diag_msg)
<< "Error generating preprocessed source(s).";
return;
@@ -1362,9 +1359,6 @@
C.ExecuteJobs(C.getJobs(), FailingCommands);
- // Remove temp files.
- C.CleanupFileList(C.getTempFiles());
-
// If the command succeeded, we are done.
if (FailingCommands.empty())
return 0;
Index: lib/Driver/Compilation.cpp
===
--- lib/Driver/Compilation.cpp
+++ lib/Driver/Compilation.cpp
@@ -38,13 +38,19 @@
InputArgList *_Args, DerivedArgList *_TranslatedArgs,
bool ContainsError)
: TheDriver(D), DefaultToolChain(_DefaultToolChain), Args(_Args),
- TranslatedArgs(_TranslatedArgs), ContainsError(ContainsError) {
+ TranslatedArgs(_TranslatedArgs), ContainsError(ContainsError),
+ KeepTempFiles(D.isSaveTempsEnabled()) {
// The offloading host toolchain is the default toolchain.
OrderedOffloadingToolchains.insert(
std::make_pair(Action::OFK_Host, &DefaultToolChain));
}
Compilation::~Compilation() {
+ // Remove temporary files. This must be done before arguments are freed, as
+ // the file names might be derived from the input arguments.
+ if (!KeepTempFiles)
+CleanupFileList(TempFiles);
+
delete TranslatedArgs;
delete Args;
@@ -245,6 +251,10 @@
AllActions.clear();
Jobs.clear();
+ // Remove temporary files.
+ if (!KeepTempFiles)
+CleanupFileList(TempFiles);
+
// Clear temporary/results file lists.
TempFiles.clear();
ResultFiles.clear();
@@ -262,6 +272,9 @@
// Redirect stdout/stderr to /dev/null.
Redirects = {None, {""}, {""}};
+
+ // Temporary files added by diagnostics should be kept.
+ KeepTempFiles = true;
}
StringRef Compilation::getSysRoot() const {
Index: include/clang/Driver/Compilation.h
===
--- include/clang/Driver/Compilation.h
+++ include/clang/Driver/Compilation.h
@@ -122,6 +122,9 @@
/// Whether an error during the parsing of the input args.
bool ContainsError;
+ /// Whether to keep temporary files.
+ bool KeepTempFiles;
+
public:
Compilation(const Driver &D, const ToolChain &DefaultToolChain,
llvm::opt::InputArgList *Args,
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1243,9 +1243,6 @@
// If any of the preprocessing commands failed, clean up and exit.
if (!FailingCommands.empty()) {
-if (!isSaveTempsEnabled())
- C.CleanupFileList(C.getTempFiles(), true);
-
Diag(clang::diag::note_drv_command_failed_diag_msg)
<< "Error generating preprocessed source(s).";
return;
@@ -1362,9 +1359,6 @@
C.ExecuteJobs(C.getJobs(), FailingCommands);
- // Remove temp files.
- C.CleanupFileList(C.getTempFiles());
-
// If the command succeeded, we are done.
if (FailingCommands.empty())
return 0;
Index: lib/Driver/Compilation.cpp
===
--- lib/Driver/Compilation.cpp
+++ lib/Driver/Compilation.cpp
@@ -38,13 +38,19 @@
InputArgList *_Args, DerivedArgList *_TranslatedArgs,
bool ContainsError)
: TheDriver(D), DefaultToolChain(_DefaultToolChain), Args(_Args),
- TranslatedArgs(_TranslatedArgs), ContainsError(ContainsError) {
+ TranslatedArgs(_TranslatedArgs), ContainsError(ContainsError),
+ KeepTempFiles(D.isSaveTempsEnabled()) {
// The offloading host toolchain is the default toolchain.
OrderedOffloadingToolchains.insert(
std::make_pair(Action::OFK_Host, &DefaultToolChain));
}
Compilation::~Compilation() {
+ // Remove temporary files. This must be done before arguments are freed, as
+ // the file names might be derived from the input arguments.
+ if (!KeepTempFiles)
+CleanupFileList(TempFiles);
+
delete TranslatedArgs;
delete Args;
@@ -245,6 +251,10 @@
AllActions.clear();
Jobs.clear();
+ // Remove temporary files.
+ if (!KeepTempFiles)
+CleanupFileList(TempFiles);
+
// Clear temporary/results file lists.
TempFiles.clear();
ResultFiles.clear();
@@ -262,6 +272,9 @@
// Redirect stdo
[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects
erichkeane added inline comments.
Comment at: lib/Driver/Compilation.cpp:42
+ TranslatedArgs(_TranslatedArgs), ContainsError(ContainsError),
+ KeepTempFiles(D.isSaveTempsEnabled()) {
// The offloading host toolchain is the default toolchain.
I'd prefer to just look this up from 'TheDriver' when it is needed. We
shouldn't store things we can look up easy enough.
https://reviews.llvm.org/D45686
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33537: [clang-tidy] Exception Escape Checker
baloghadamsoftware added inline comments.
Comment at: test/clang-tidy/bugprone-exception-escape.cpp:178
+void indirect_implicit() noexcept {
+ // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'indirect_implicit'
throws
+ implicit_int_thrower();
lebedev.ri wrote:
> baloghadamsoftware wrote:
> > dberris wrote:
> > > How deep does this go? Say we have a call to a function that's extern
> > > which doesn't have 'noexcept' nor a dynamic exception specifier -- do we
> > > assume that the call to an extern function may throw? Does that warn?
> > > What does the warning look like? Should it warn? How about when you call
> > > a function through a function pointer?
> > >
> > > The documentation should cover these cases and/or more explicitly say in
> > > the warning that an exception may throw in a noexcept function (rather
> > > than just "function <...> throws").
> > We take the most conservative way here. We assume that a function throws if
> > and only if its body has a throw statement or its header has the (old)
> > throw() exception specification. We do not follow function pointers.
> While i realize it may have more noise, it may be nice to have a more
> pedantic mode (guarded by an option?).
> E.g. `noexcept` propagation, much like `const` on methods propagation.
> Or at least a test that shows that it does not happen, unless i simply did
> not notice it :)
This could be an enhancement in the future, yes.
https://reviews.llvm.org/D33537
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333275 - [analyzer] Added template argument lists to the Pathdiagnostic output
Author: szelethus
Date: Fri May 25 06:18:38 2018
New Revision: 333275
URL: http://llvm.org/viewvc/llvm-project?rev=333275&view=rev
Log:
[analyzer] Added template argument lists to the Pathdiagnostic output
Because template parameter lists were not displayed
in the plist output, it was difficult to decide in
some cases whether a given checker found a true or a
false positive. This patch aims to correct this.
Differential Revision: https://reviews.llvm.org/D46933
Added:
cfe/trunk/test/Analysis/plist-diagnostics-template-function.cpp
cfe/trunk/test/Analysis/plist-diagnostics-template-record.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp?rev=333275&r1=333274&r2=333275&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp Fri May 25 06:18:38
2018
@@ -16,6 +16,7 @@
#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
+#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/OperationKinds.h"
@@ -1000,11 +1001,49 @@ void PathDiagnosticCallPiece::setCallee(
CalleeCtx->getAnalysisDeclContext()->isBodyAutosynthesized());
}
+static void describeTemplateParameters(raw_ostream &Out,
+ const ArrayRef TAList,
+ const LangOptions &LO,
+ StringRef Prefix = StringRef(),
+ StringRef Postfix = StringRef());
+
+static void describeTemplateParameter(raw_ostream &Out,
+ const TemplateArgument &TArg,
+ const LangOptions &LO) {
+
+ if (TArg.getKind() == TemplateArgument::ArgKind::Pack) {
+describeTemplateParameters(Out, TArg.getPackAsArray(), LO);
+ } else {
+TArg.print(PrintingPolicy(LO), Out);
+ }
+}
+
+static void describeTemplateParameters(raw_ostream &Out,
+ const ArrayRef TAList,
+ const LangOptions &LO,
+ StringRef Prefix, StringRef Postfix) {
+ if (TAList.empty())
+return;
+
+ Out << Prefix;
+ for (int I = 0, Last = TAList.size() - 1; I != Last; ++I) {
+describeTemplateParameter(Out, TAList[I], LO);
+Out << ", ";
+ }
+ describeTemplateParameter(Out, TAList[TAList.size() - 1], LO);
+ Out << Postfix;
+}
+
static void describeClass(raw_ostream &Out, const CXXRecordDecl *D,
StringRef Prefix = StringRef()) {
if (!D->getIdentifier())
return;
- Out << Prefix << '\'' << *D << '\'';
+ Out << Prefix << '\'' << *D;
+ if (const auto T = dyn_cast(D))
+describeTemplateParameters(Out, T->getTemplateArgs().asArray(),
+ D->getASTContext().getLangOpts(), "<", ">");
+
+ Out << '\'';
}
static bool describeCodeDecl(raw_ostream &Out, const Decl *D,
@@ -1062,7 +1101,16 @@ static bool describeCodeDecl(raw_ostream
return true;
}
- Out << Prefix << '\'' << cast(*D) << '\'';
+ Out << Prefix << '\'' << cast(*D);
+
+ // Adding template parameters.
+ if (const auto FD = dyn_cast(D))
+if (const TemplateArgumentList *TAList =
+FD->getTemplateSpecializationArgs())
+ describeTemplateParameters(Out, TAList->asArray(),
+ FD->getASTContext().getLangOpts(), "<", ">");
+
+ Out << '\'';
return true;
}
Added: cfe/trunk/test/Analysis/plist-diagnostics-template-function.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/plist-diagnostics-template-function.cpp?rev=333275&view=auto
==
--- cfe/trunk/test/Analysis/plist-diagnostics-template-function.cpp (added)
+++ cfe/trunk/test/Analysis/plist-diagnostics-template-function.cpp Fri May 25
06:18:38 2018
@@ -0,0 +1,41 @@
+// RUN: %clang_analyze_cc1 -analyzer-output=plist -o %t.plist -std=c++11
-analyzer-checker=core %s
+// RUN: FileCheck --input-file=%t.plist %s
+
+bool ret();
+
+template
+void f(int i) {
+ if (ret())
+i = i / (i - 5);
+}
+
+template <>
+void f(int i) {
+ if (ret())
+i = i / (i - 5);
+}
+
+template
+void defaultTemplateParameterFunction(int i) {
+ if (ret())
+int a = 10 / i;
+}
+
+template
+void variadicTemplateFunction(int i) {
+ if (ret())
+int a = 10 / i;
+}
+
+int main() {
+ f(5);
+ f(5);
+ defaultTemplateParameterFunction<>(0);
+ variadicTemplateFunction(0);
+}
+
+// CHECK: Calling 'f'
+// CHECK: Calling 'f'
+// CHECK: Calling
'defaultTemplateParamet
[PATCH] D46933: [analyzer] Added template argument lists to the Pathdiagnostic output
This revision was automatically updated to reflect the committed changes.
Closed by commit rC333275: [analyzer] Added template argument lists to the
Pathdiagnostic output (authored by Szelethus, committed by ).
Herald added a subscriber: cfe-commits.
Repository:
rC Clang
https://reviews.llvm.org/D46933
Files:
lib/StaticAnalyzer/Core/PathDiagnostic.cpp
test/Analysis/plist-diagnostics-template-function.cpp
test/Analysis/plist-diagnostics-template-record.cpp
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -16,6 +16,7 @@
#include "clang/AST/DeclBase.h"
#include "clang/AST/DeclCXX.h"
#include "clang/AST/DeclObjC.h"
+#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/OperationKinds.h"
@@ -1000,11 +1001,49 @@
CalleeCtx->getAnalysisDeclContext()->isBodyAutosynthesized());
}
+static void describeTemplateParameters(raw_ostream &Out,
+ const ArrayRef TAList,
+ const LangOptions &LO,
+ StringRef Prefix = StringRef(),
+ StringRef Postfix = StringRef());
+
+static void describeTemplateParameter(raw_ostream &Out,
+ const TemplateArgument &TArg,
+ const LangOptions &LO) {
+
+ if (TArg.getKind() == TemplateArgument::ArgKind::Pack) {
+describeTemplateParameters(Out, TArg.getPackAsArray(), LO);
+ } else {
+TArg.print(PrintingPolicy(LO), Out);
+ }
+}
+
+static void describeTemplateParameters(raw_ostream &Out,
+ const ArrayRef TAList,
+ const LangOptions &LO,
+ StringRef Prefix, StringRef Postfix) {
+ if (TAList.empty())
+return;
+
+ Out << Prefix;
+ for (int I = 0, Last = TAList.size() - 1; I != Last; ++I) {
+describeTemplateParameter(Out, TAList[I], LO);
+Out << ", ";
+ }
+ describeTemplateParameter(Out, TAList[TAList.size() - 1], LO);
+ Out << Postfix;
+}
+
static void describeClass(raw_ostream &Out, const CXXRecordDecl *D,
StringRef Prefix = StringRef()) {
if (!D->getIdentifier())
return;
- Out << Prefix << '\'' << *D << '\'';
+ Out << Prefix << '\'' << *D;
+ if (const auto T = dyn_cast(D))
+describeTemplateParameters(Out, T->getTemplateArgs().asArray(),
+ D->getASTContext().getLangOpts(), "<", ">");
+
+ Out << '\'';
}
static bool describeCodeDecl(raw_ostream &Out, const Decl *D,
@@ -1062,7 +1101,16 @@
return true;
}
- Out << Prefix << '\'' << cast(*D) << '\'';
+ Out << Prefix << '\'' << cast(*D);
+
+ // Adding template parameters.
+ if (const auto FD = dyn_cast(D))
+if (const TemplateArgumentList *TAList =
+FD->getTemplateSpecializationArgs())
+ describeTemplateParameters(Out, TAList->asArray(),
+ FD->getASTContext().getLangOpts(), "<", ">");
+
+ Out << '\'';
return true;
}
Index: test/Analysis/plist-diagnostics-template-function.cpp
===
--- test/Analysis/plist-diagnostics-template-function.cpp
+++ test/Analysis/plist-diagnostics-template-function.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_analyze_cc1 -analyzer-output=plist -o %t.plist -std=c++11 -analyzer-checker=core %s
+// RUN: FileCheck --input-file=%t.plist %s
+
+bool ret();
+
+template
+void f(int i) {
+ if (ret())
+i = i / (i - 5);
+}
+
+template <>
+void f(int i) {
+ if (ret())
+i = i / (i - 5);
+}
+
+template
+void defaultTemplateParameterFunction(int i) {
+ if (ret())
+int a = 10 / i;
+}
+
+template
+void variadicTemplateFunction(int i) {
+ if (ret())
+int a = 10 / i;
+}
+
+int main() {
+ f(5);
+ f(5);
+ defaultTemplateParameterFunction<>(0);
+ variadicTemplateFunction(0);
+}
+
+// CHECK: Calling 'f'
+// CHECK: Calling 'f'
+// CHECK: Calling 'defaultTemplateParameterFunction<0>'
+// CHECK: Calling 'variadicTemplateFunction'
+
Index: test/Analysis/plist-diagnostics-template-record.cpp
===
--- test/Analysis/plist-diagnostics-template-record.cpp
+++ test/Analysis/plist-diagnostics-template-record.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_analyze_cc1 -analyzer-output=plist -o %t.plist -std=c++11 -analyzer-checker=core %s
+// RUN: FileCheck --input-file=%t.plist %s
+
+bool ret();
+
+template
+struct DivByZero {
+ int i;
+ DivByZero(bool b) {
+if (ret())
+ i = 50 / (b - 1);
+ }
+};
+
+template
+struct DivByZero {
+ int i;
+ DivByZero(bool b) {
+if (ret())
+
[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects
dstenb added inline comments. Comment at: lib/Driver/Compilation.cpp:276-277 + + // Temporary files added by diagnostics should be kept. + SaveTempsEnabled = true; } lebedev.ri wrote: > Is there a test that breaks without this? Yes, the following tests fail: - Driver/crash-report-modules.m - Driver/crash-report-spaces.c - Driver/crash-report-header.h - Driver/crash-report.c - Modules/crash-vfs-path-emptydir-entries.m - Modules/crash-vfs-path-symlink-topheader.m - Modules/crash-vfs-path-symlink-component.m - Modules/crash-vfs-path-traversal.m - Modules/crash-vfs-relative-overlay.m - Modules/crash-vfs-umbrella-frameworks.m due to the preprocessed files for the crash diagnostics having been removed. https://reviews.llvm.org/D45686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects
lebedev.ri added a comment. LGTM, but i'm quite unfamiliar with this area of the code, so please wait for someone else to accept :) Comment at: lib/Driver/Compilation.cpp:276-277 + + // Temporary files added by diagnostics should be kept. + SaveTempsEnabled = true; } dstenb wrote: > lebedev.ri wrote: > > Is there a test that breaks without this? > Yes, the following tests fail: > > - Driver/crash-report-modules.m > - Driver/crash-report-spaces.c > - Driver/crash-report-header.h > - Driver/crash-report.c > - Modules/crash-vfs-path-emptydir-entries.m > - Modules/crash-vfs-path-symlink-topheader.m > - Modules/crash-vfs-path-symlink-component.m > - Modules/crash-vfs-path-traversal.m > - Modules/crash-vfs-relative-overlay.m > - Modules/crash-vfs-umbrella-frameworks.m > > due to the preprocessed files for the crash diagnostics having been removed. Ok, good. https://reviews.llvm.org/D45686 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory
ilya-biryukov updated this revision to Diff 148599.
ilya-biryukov added a comment.
- Rebase, fix merge conflicts
- Simpler implemenataion of the Cache
- s/IdleASTs/ASTCache
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47063
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/TUScheduler.cpp
clangd/TUScheduler.h
test/clangd/trace.test
unittests/clangd/FileIndexTests.cpp
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -11,6 +11,7 @@
#include "TestFS.h"
#include "TestTU.h"
#include "index/FileIndex.h"
+#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/CompilationDatabase.h"
@@ -210,15 +211,15 @@
auto FooH = testPath("foo.h");
FileIndex Index;
bool IndexUpdated = false;
- CppFile File("foo.cpp", /*StorePreambleInMemory=*/true,
- std::make_shared(),
- [&Index, &IndexUpdated](PathRef FilePath, ASTContext &Ctx,
- std::shared_ptr PP) {
- EXPECT_FALSE(IndexUpdated)
- << "Expected only a single index update";
- IndexUpdated = true;
- Index.update(FilePath, &Ctx, std::move(PP));
- });
+ ASTBuilder Builder("foo.cpp", /*StorePreambleInMemory=*/true,
+ std::make_shared(),
+ [&Index, &IndexUpdated](PathRef FilePath, ASTContext &Ctx,
+ std::shared_ptr PP) {
+ EXPECT_FALSE(IndexUpdated)
+ << "Expected only a single index update";
+ IndexUpdated = true;
+ Index.update(FilePath, &Ctx, std::move(PP));
+ });
// Preparse ParseInputs.
ParseInputs PI;
@@ -243,7 +244,9 @@
)cpp";
// Rebuild the file.
- File.rebuild(std::move(PI));
+ auto CI = Builder.buildCompilerInvocation(PI);
+ Builder.buildPreamble(*CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(),
+PI);
ASSERT_TRUE(IndexUpdated);
// Check the index contains symbols from the preamble, but not from the main
Index: test/clangd/trace.test
===
--- test/clangd/trace.test
+++ test/clangd/trace.test
@@ -8,14 +8,14 @@
# CHECK: "args": {
# CHECK: "File": "{{.*(/|\\)}}foo.c"
# CHECK: },
-# CHECK: "name": "Preamble",
+# CHECK: "name": "BuildPreamble",
# CHECK: "ph": "X",
# CHECK: }
# CHECK: {
# CHECK: "args": {
# CHECK: "File": "{{.*(/|\\)}}foo.c"
# CHECK: },
-# CHECK: "name": "Build",
+# CHECK: "name": "BuildAST",
# CHECK: "ph": "X",
# CHECK: }
# CHECK: },
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -42,6 +42,15 @@
/// within a bounded amount of time.
};
+/// Configuration of the AST retention policy. This only covers retention of
+/// *idle* ASTs. If queue has operations requiring the AST, they might be
+/// kept in memory.
+struct ASTRetentionParams {
+ /// Maximum number of ASTs to be retained in memory when there are no pending
+ /// requests for them.
+ unsigned MaxRetainedASTs = 3;
+};
+
/// Handles running tasks for ClangdServer and managing the resources (e.g.,
/// preambles and ASTs) for opened files.
/// TUScheduler is not thread-safe, only one thread should be providing updates
@@ -53,7 +62,8 @@
public:
TUScheduler(unsigned AsyncThreadsCount, bool StorePreamblesInMemory,
PreambleParsedCallback PreambleCallback,
- std::chrono::steady_clock::duration UpdateDebounce);
+ std::chrono::steady_clock::duration UpdateDebounce,
+ ASTRetentionParams RetentionConfig = {});
~TUScheduler();
/// Returns estimated memory usage for each of the currently open files.
@@ -99,11 +109,18 @@
/// This class stores per-file data in the Files map.
struct FileData;
+public:
+ /// Responsible for retaining and rebuilding idle ASTs. An implementation is
+ /// an LRU cache.
+ class ASTCache;
+
+private:
const bool StorePreamblesInMemory;
const std::shared_ptr PCHOps;
const PreambleParsedCallback PreambleCallback;
Semaphore Barrier;
llvm::StringMap> Files;
+ std::unique_ptr IdleASTs;
// None when running tasks synchronously and non-None when running tasks
// asynchronously.
llvm::Optional PreambleTasks;
Index: clangd/TUScheduler.cpp
===
--- clangd/TUScheduler.cpp
+++ clangd/TUScheduler.cpp
@@ -45,16 +45,82 @@
#include "TUScheduler.h"
#include "Logger.h"
#include "Trace.h"
+#include "clang/Frontend/CompilerInvocation.h"
[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory
ilya-biryukov added a comment.
I have addressed the comments regarding the cache implementation. ASTBuilder
ones are still pending, but I would appreciate the feedback on how
`TUScheduler.cpp` looks like.
Comment at: clangd/ClangdUnit.h:132
-/// Manages resources, required by clangd. Allows to rebuild file with new
-/// contents, and provides AST and Preamble for it.
-class CppFile {
+/// A helper class that handles building preambles and ASTs for a file. Also
+/// adds some logging.
sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > This may be change aversion, but I'm not sure this class does enough
> > > after this change - it doesn't store the inputs or the outputs/cache, so
> > > it kind of seems like it wants to be a function.
> > >
> > > I guess the motivation here is that storing the outputs means dealing
> > > with the cache, and the cache is local to TUScheduler.
> > > But `CppFile` is only used in TUScheduler, so we could move this there
> > > too? It feels like expanding the scope more than I'd like.
> > >
> > > The upside is that I think it's a more natural division of
> > > responsibility: `CppFile` could continue to be the "main" holder of the
> > > `shared_ptr` (which we don't limit, but share), and instead of
> > > `Optional` it'd have a `weak_ptr` which is
> > > controlled and can be refreshed through the cache.
> > >
> > > As discussed offline, the cache could look something like:
> > > ```
> > > class Cache {
> > >shared_ptr put(ParsedAST);
> > >void hintUsed(ParsedAST*); // optional, bumps LRU when client reads
> > >void hintUnused(ParsedAST*); // optional, releases when client abandons
> > > }
> > >
> > > shared_ptr CppFile::getAST() {
> > > shared_ptr AST = WeakAST.lock();
> > > if (AST)
> > > Cache.hintUsed(AST.get());
> > > else
> > > WeakAST = AST = Cache.put(build...);
> > > return AST;
> > > }
> > > ```
> > I've reimplemented the cache with weak_ptr/shared_ptr.
> > With a slightly different interface to hide more stuff in the cache API.
> > Please take a look and let me know what you think.
> >
> > Nevertheless, I still find `CppFile` refactoring useful. It unties
> > preambles from the ASTs and I believe this is a good thing, given that
> > their lifetimes are different.
> I'm not sure how much they were tangled before, they were computed in the
> same place, but accessed separately, and it's not sure you ever *want* to
> compute them separately? (possibly in unit tests?)
>
> I do think making ASTWorker maintain the old preamble and pass it in is
> confusing. The remaining members are trivial and unrelated enough that I do
> think if the references to the preamble/ast are removed, then moving the
> remaining members to ASTWorker or to a dumb struct, and making these free
> functions would make it easier to navigate the class structure.
> it's not sure you ever *want* to compute them separately?
I think that's exactly what we're doing now. The ASTs can now get built
separately from the preamble, because they can be evicted from the cache and
need to be rebuild while preamble is not changed.
> The remaining members are trivial and unrelated enough that I do think if the
> references to the preamble/ast are removed, then moving the remaining members
> to ASTWorker or to a dumb struct, and making these free functions would make
> it easier to navigate the class structure.
Dumb struct SG, it's essentially what ASTWorker is right now.
Comment at: clangd/TUScheduler.cpp:71
+
+ /// Update the function used to compute the value.
+ void update(std::function()> ComputeF);
sammccall wrote:
> I think I understand this more as "updates the value" but the value is lazy...
>
> I find this API somewhat hard to follow, maybe just because it's unfamiliar.
> I've mostly seen cache APIs look like one of:
> 1. `Cache(function Compute)`, `Value Cache::get(Input)`
> 2. `void Cache::put(Key, Value)`, `Optional Cache::get(Key)`
> 3. `Handle Cache::put(Value)`, `Optional Handle::get()`
>
> This one is `Slot Cache::create()`, `void Slot::update(function
> LazyV)`, `Value Slot::get()`.
>
> It's similar-ish to 3, but has 3 nontrivial operations instead of 2, and the
> slot object is externally mutable instead of immutable, so it seems more
> complex. What does it buy us in exchange?
>
> (1 & 2 work well with a natural key or inputs that are easy to compare, which
> we don't particularly have)
As discussed offline, now we have a simpler version that keeps `unique_ptr`s to
idle ASTs and the clients are responsible for building the ASTs.
Note that it's not a "cache" per se, so we might want a different name for that.
@sammccall, you suggested to call it a pool, I find it reasonable. Should we
name it `ASTPool` instead of `ASTCache`?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47063
_
r333276 - Fix members initialization order in constructor (fails with -Werror)
Author: yvvan
Date: Fri May 25 06:46:07 2018
New Revision: 333276
URL: http://llvm.org/viewvc/llvm-project?rev=333276&view=rev
Log:
Fix members initialization order in constructor (fails with -Werror)
Modified:
cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
Modified: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h?rev=333276&r1=333275&r2=333276&view=diff
==
--- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h (original)
+++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h Fri May 25 06:46:07 2018
@@ -840,10 +840,10 @@ public:
bool Accessible = true,
std::vector FixIts =
std::vector())
: Declaration(Declaration), Priority(Priority), Kind(RK_Declaration),
-Hidden(false), QualifierIsInformative(QualifierIsInformative),
+FixIts(std::move(FixIts)), Hidden(false),
+QualifierIsInformative(QualifierIsInformative),
StartsNestedNameSpecifier(false), AllParametersAreInformative(false),
-DeclaringEntity(false), Qualifier(Qualifier),
-FixIts(std::move(FixIts)) {
+DeclaringEntity(false), Qualifier(Qualifier) {
//FIXME: Add assert to check FixIts range requirements.
computeCursorKindAndAvailability(Accessible);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47291: Proposal to make rtti errors more generic.
filcab added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6729 def err_no_dynamic_cast_with_fno_rtti : Error< - "cannot use dynamic_cast with -fno-rtti">; + "use of dynamic_cast requires enabling RTTI">; I'd prefer to have the way to enable RTTI mentioned in the message. Could we have something like `ToolChain::getRTTIMode()` and have a "RTTI was on/of" or "RTTI defaulted to on/off"? That way we'd be able to have a message similar to the current one (mentioning "you passed -fno-rtti") on platforms that default to RTTI=on, and have your updated message (possibly with a mention of "use -frtti") on platforms that default to RTTI=off. (This is a minor usability comment about this patch, I don't consider it a blocker or anything) Repository: rC Clang https://reviews.llvm.org/D47291 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47063: [clangd] Keep only a limited number of idle ASTs in memory
sammccall added a comment.
The cache looks way simpler now, thank you!
As discussed offline, flattening ASTBuilder right into ASTWorker still seems
like a good idea to me, but happy with what you come up with there.
Comment at: clangd/TUScheduler.cpp:71
+
+ /// Update the function used to compute the value.
+ void update(std::function()> ComputeF);
ilya-biryukov wrote:
> sammccall wrote:
> > I think I understand this more as "updates the value" but the value is
> > lazy...
> >
> > I find this API somewhat hard to follow, maybe just because it's unfamiliar.
> > I've mostly seen cache APIs look like one of:
> > 1. `Cache(function Compute)`, `Value Cache::get(Input)`
> > 2. `void Cache::put(Key, Value)`, `Optional Cache::get(Key)`
> > 3. `Handle Cache::put(Value)`, `Optional Handle::get()`
> >
> > This one is `Slot Cache::create()`, `void Slot::update(function
> > LazyV)`, `Value Slot::get()`.
> >
> > It's similar-ish to 3, but has 3 nontrivial operations instead of 2, and
> > the slot object is externally mutable instead of immutable, so it seems
> > more complex. What does it buy us in exchange?
> >
> > (1 & 2 work well with a natural key or inputs that are easy to compare,
> > which we don't particularly have)
> As discussed offline, now we have a simpler version that keeps `unique_ptr`s
> to idle ASTs and the clients are responsible for building the ASTs.
> Note that it's not a "cache" per se, so we might want a different name for
> that.
> @sammccall, you suggested to call it a pool, I find it reasonable. Should we
> name it `ASTPool` instead of `ASTCache`?
I think the name is actually fine, it's still mostly a cache.
It does have things in common with a pool, but unrelated consumers can't share
a resource, so I think that name is at least as misleading.
Comment at: clangd/TUScheduler.cpp:66
+
+/// Provides an LRU cache of ASTs.
+class TUScheduler::ASTCache {
I'd say a little more about the interaction here. e.g.
```
/// An LRU cache of idle ASTs.
/// Because we want to limit the overall number of these we retain, the cache
/// owns ASTs (and may evict them) while their workers are idle.
/// Workers borrow them when active, and return them when done.
Comment at: clangd/TUScheduler.cpp:84
+ /// Store the value in the pool, possibly removing the last used AST.
+ void put(Key K, std::unique_ptr V) {
+std::unique_lock Lock(Mut);
consider assert(findByKey(K) == LRU.end()) as a precondition
Comment at: clangd/TUScheduler.cpp:92
+LRU.pop_back();
+// AST destructor may need to run, make sure it happens outside the lock.
+Lock.unlock();
Just "run the expensive destructor outside the lock"?
the "may not" case seems unimportant and slightly confusing here
Comment at: clangd/TUScheduler.cpp:94
+Lock.unlock();
+ForCleanup.reset();
+ }
this line isn't actually needed right?
Comment at: clangd/TUScheduler.cpp:342
+if (!AST)
+ return Action(llvm::make_error(
+ "invalid AST", llvm::errc::invalid_argument));
This failure doesn't get cached, correct? That's bad for performance.
But if we think this is always a clangd bug, it's probably fine. (and certainly
simplifies things)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47063
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47331: [clangd] Remove accessors for top-level decls from preamble
ilya-biryukov added inline comments.
Comment at: clangd/ClangdUnit.h:81
ASTContext &getASTContext();
const ASTContext &getASTContext() const;
ioeric wrote:
> IIUC, `ASTContext` in a `ParsedAST` may not contain information from
> preambles and thus may give an incomplete AST. If so, I think we should make
> this more explicit in the class level to make sure this is not misused (e.g.
> in case the incomplete context is used to build dynamic index).
Added a small doc comment about it. Let me know if you feel we need to
elaborate more on this.
I don't think anything changed wrt to this behavior in this patch, apart from
the fact that we used to deserialize more stuff most of the time before so this
wan't as visible.
Comment at: unittests/clangd/TestTU.cpp:77
const NamedDecl *Result = nullptr;
- for (const Decl *D : AST.getTopLevelDecls()) {
+ for (const Decl *D : AST.getLocalTopLevelDecls()) {
auto *ND = dyn_cast(D);
sammccall wrote:
> isn't this incorrect? Or should be "findDeclInMainFile" or similar?
>
> I'd think this would conflict with your other patch, which uses this to test
> the boost of things that come from the header.
Thanks for spotting that, I would also expect this to fail in the new patch.
I'll look into rewriting this helper.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47331: [clangd] Remove accessors for top-level decls from preamble
ilya-biryukov updated this revision to Diff 148602.
ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added a comment.
- Rename the field in addition to the getter
- Address review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47331
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/XRefs.cpp
unittests/clangd/TestTU.cpp
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -74,7 +74,7 @@
const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName) {
const NamedDecl *Result = nullptr;
- for (const Decl *D : AST.getTopLevelDecls()) {
+ for (const Decl *D : AST.getLocalTopLevelDecls()) {
auto *ND = dyn_cast(D);
if (!ND || ND->getNameAsString() != QName)
continue;
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -168,7 +168,7 @@
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
IndexOpts.IndexFunctionLocals = true;
- indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
+ indexTopLevelDecls(AST.getASTContext(), AST.getLocalTopLevelDecls(),
DeclMacrosFinder, IndexOpts);
return {DeclMacrosFinder.takeDecls(), DeclMacrosFinder.takeMacroInfos()};
@@ -418,7 +418,7 @@
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
IndexOpts.IndexFunctionLocals = true;
- indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
+ indexTopLevelDecls(AST.getASTContext(), AST.getLocalTopLevelDecls(),
DocHighlightsFinder, IndexOpts);
return DocHighlightsFinder.takeHighlights();
Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -44,12 +44,10 @@
// Stores Preamble and associated data.
struct PreambleData {
- PreambleData(PrecompiledPreamble Preamble,
- std::vector TopLevelDeclIDs,
- std::vector Diags, std::vector Inclusions);
+ PreambleData(PrecompiledPreamble Preamble, std::vector Diags,
+ std::vector Inclusions);
PrecompiledPreamble Preamble;
- std::vector TopLevelDeclIDs;
std::vector Diags;
// Processes like code completions and go-to-definitions will need #include
// information, and their compile action skips preamble range.
@@ -80,17 +78,19 @@
~ParsedAST();
+ /// Note that the returned ast will not contain decls from the preamble that
+ /// were not deserialized during parsing. Clients should expect only decls
+ /// from the main file to be in the AST.
ASTContext &getASTContext();
const ASTContext &getASTContext() const;
Preprocessor &getPreprocessor();
std::shared_ptr getPreprocessorPtr();
const Preprocessor &getPreprocessor() const;
- /// This function returns all top-level decls, including those that come
- /// from Preamble. Decls, coming from Preamble, have to be deserialized, so
- /// this call might be expensive.
- ArrayRef getTopLevelDecls();
+ /// This function returns top-level decls present in the main file of the AST.
+ /// The result does not include the decls that come from the preamble.
+ ArrayRef getLocalTopLevelDecls();
const std::vector &getDiagnostics() const;
@@ -103,11 +103,8 @@
ParsedAST(std::shared_ptr Preamble,
std::unique_ptr Clang,
std::unique_ptr Action,
-std::vector TopLevelDecls, std::vector Diags,
-std::vector Inclusions);
-
-private:
- void ensurePreambleDeclsDeserialized();
+std::vector LocalTopLevelDecls,
+std::vector Diags, std::vector Inclusions);
// In-memory preambles must outlive the AST, it is important that this member
// goes before Clang and Action.
@@ -122,8 +119,9 @@
// Data, stored after parsing.
std::vector Diags;
- std::vector TopLevelDecls;
- bool PreambleDeclsDeserialized;
+ // Top-level decls inside the current file. Not that this does not include
+ // top-level decls from the preamble.
+ std::vector LocalTopLevelDecls;
std::vector Inclusions;
};
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -90,37 +90,15 @@
CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
: File(File), ParsedCallback(ParsedCallback) {}
- std::vector takeTopLevelDeclIDs() {
-return std::move(TopLevelDeclIDs);
- }
-
std::vector takeInclusions() { return std::move(Inclusions); }
- void AfterPCHEmitted(ASTWriter &Writer) override {
-TopLevelDeclIDs.reserve(TopLevelDecls.size());
-for (Decl *D : TopLevelDecls) {
- // Invalid top-level decls may not have been serialized.
- if (D->isInvalidDecl())
[PATCH] D47067: Update NRVO logic to support early return
tzik updated this revision to Diff 148603.
tzik added a comment.
reuse getParent() result
Repository:
rC Clang
https://reviews.llvm.org/D47067
Files:
include/clang/AST/Decl.h
include/clang/Sema/Scope.h
lib/Sema/Scope.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaStmt.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriterDecl.cpp
test/CodeGenCXX/nrvo-noopt.cpp
test/CodeGenCXX/nrvo.cpp
test/SemaCXX/nrvo-ast.cpp
Index: test/SemaCXX/nrvo-ast.cpp
===
--- /dev/null
+++ test/SemaCXX/nrvo-ast.cpp
@@ -0,0 +1,139 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -ast-dump -o - %s | FileCheck %s
+
+struct X {
+ X();
+ X(const X&);
+ X(X&&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_00
+X test_00() {
+ // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+ X x;
+ return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_01
+X test_01(bool b) {
+ // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+ X x;
+ if (b)
+return x;
+ return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_02
+X test_02(bool b) {
+ // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+ X x;
+ // CHECK-NOT: VarDecl {{.*}} y {{.*}} nrvo
+ X y;
+ if (b)
+return y;
+ return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_03
+X test_03(bool b) {
+ if (b) {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+ }
+ // CHECK: VarDecl {{.*}} x {{.*}} nrvo
+ X x;
+ return x;
+}
+
+extern "C" _Noreturn void exit(int) throw();
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_04
+X test_04(bool b) {
+ {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+if (b)
+ return x;
+ }
+ exit(1);
+}
+
+void may_throw();
+// CHECK-LABEL: FunctionDecl {{.*}} test_05
+X test_05() {
+ try {
+may_throw();
+return X();
+ } catch (X x) {
+// CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+return x;
+ }
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_06
+X test_06() {
+ // CHECK-NOT: VarDecl {{.*}} x {{.*}} nrvo
+ X x __attribute__((aligned(8)));
+ return x;
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_07
+X test_07(bool b) {
+ if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+ }
+ return X();
+}
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_08
+X test_08(bool b) {
+ if (b) {
+// CHECK: VarDecl {{.*}} x {{.*}} nrvo
+X x;
+return x;
+ } else {
+// CHECK: VarDecl {{.*}} y {{.*}} nrvo
+X y;
+return y;
+ }
+}
+
+template
+struct Y {
+ Y();
+ // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+ // CHECK: VarDecl {{.*}} y 'Y' nrvo
+
+ // CHECK-LABEL: CXXMethodDecl {{.*}} test_09 'Y ()'
+ // CHECK: VarDecl {{.*}} y 'Y' nrvo
+ static Y test_09() {
+Y y;
+return y;
+ }
+};
+
+struct Z {
+ Z(const X&);
+};
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'A ()'
+// CHECK: VarDecl {{.*}} b 'B' nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'X ()'
+// CHECK: VarDecl {{.*}} b {{.*}} nrvo
+
+// CHECK-LABEL: FunctionDecl {{.*}} test_10 'Z ()'
+// CHECK-NOT: VarDecl {{.*}} b {{.*}} nrvo
+template
+A test_10() {
+ B b;
+ return b;
+}
+
+void instantiate() {
+ Y::test_09();
+ test_10();
+ test_10();
+}
Index: test/CodeGenCXX/nrvo.cpp
===
--- test/CodeGenCXX/nrvo.cpp
+++ test/CodeGenCXX/nrvo.cpp
@@ -130,17 +130,13 @@
}
// CHECK-LABEL: define void @_Z5test3b
-X test3(bool B) {
+X test3(bool B, X x) {
// CHECK: tail call {{.*}} @_ZN1XC1Ev
- // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
- // CHECK: call {{.*}} @_ZN1XC1Ev
- // CHECK: call {{.*}} @_ZN1XC1ERKS_
if (B) {
X y;
return y;
}
- // FIXME: we should NRVO this variable too.
- X x;
+ // CHECK: tail call {{.*}} @_ZN1XC1ERKS_
return x;
}
@@ -191,21 +187,29 @@
}
// CHECK-LABEL: define void @_Z5test7b
+// CHECK-EH-LABEL: define void @_Z5test7b
X test7(bool b) {
// CHECK: tail call {{.*}} @_ZN1XC1Ev
// CHECK-NEXT: ret
+
+ // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+ // CHECK-EH-NEXT: ret
if (b) {
X x;
return x;
}
return X();
}
// CHECK-LABEL: define void @_Z5test8b
+// CHECK-EH-LABEL: define void @_Z5test8b
X test8(bool b) {
// CHECK: tail call {{.*}} @_ZN1XC1Ev
// CHECK-NEXT: ret
- if (b) {
+
+ // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+ // CHECK-EH-NEXT: ret
+if (b) {
X x;
return x;
} else {
@@ -221,4 +225,37 @@
// CHECK-LABEL: define linkonce_odr void @_ZN1YIiE1fEv
// CHECK: tail call {{.*}} @_ZN1YIiEC1Ev
+// CHECK-LABEL: define void @_Z6test10b
+X test10(bool B, X x) {
+ if (B) {
+// CHECK: tail call {{.*}} @_ZN1XC1ERKS_
+// CHECK-EH: tail call {{.*}} @_ZN1XC1ERKS_
+return x;
+ }
+ // CHECK: tail call {{.*}} @_ZN1XC1Ev
+ // CHECK-NOT: call {{.*}} @_ZN1XC1ERKS_
+
+ // CHECK-EH: tail call {{.*}} @_ZN1XC1Ev
+ // CHECK-EH-NOT: call {{.*}} @_ZN1XC1ERKS_
+ X y;
+ return y;
+}
+
+// CHECK-LABEL: define {{.*}} void @_Z6test11I1XET_v
[PATCH] D47067: Update NRVO logic to support early return
tzik added inline comments. Comment at: lib/Sema/Scope.cpp:128 - if (getEntity()) -return; - - if (NRVO.getInt()) -getParent()->setNoNRVO(); - else if (NRVO.getPointer()) -getParent()->addNRVOCandidate(NRVO.getPointer()); + if (getParent()) +getParent()->setNRVOCandidate(Candidate); xbolva00 wrote: > auto * Parent = getParent(); > if (Parent) > Parent>setNRVOCandidate(Candidate); > > ? Updated! Repository: rC Clang https://reviews.llvm.org/D47067 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45686: [Driver] Clean up tmp files when deleting Compilation objects
dstenb updated this revision to Diff 148604.
dstenb added a comment.
Query TheDriver.isSaveTempsEnabled() at uses instead of storing the value in
the constructor.
https://reviews.llvm.org/D45686
Files:
include/clang/Driver/Compilation.h
lib/Driver/Compilation.cpp
lib/Driver/Driver.cpp
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1243,9 +1243,6 @@
// If any of the preprocessing commands failed, clean up and exit.
if (!FailingCommands.empty()) {
-if (!isSaveTempsEnabled())
- C.CleanupFileList(C.getTempFiles(), true);
-
Diag(clang::diag::note_drv_command_failed_diag_msg)
<< "Error generating preprocessed source(s).";
return;
@@ -1362,9 +1359,6 @@
C.ExecuteJobs(C.getJobs(), FailingCommands);
- // Remove temp files.
- C.CleanupFileList(C.getTempFiles());
-
// If the command succeeded, we are done.
if (FailingCommands.empty())
return 0;
Index: lib/Driver/Compilation.cpp
===
--- lib/Driver/Compilation.cpp
+++ lib/Driver/Compilation.cpp
@@ -45,6 +45,11 @@
}
Compilation::~Compilation() {
+ // Remove temporary files. This must be done before arguments are freed, as
+ // the file names might be derived from the input arguments.
+ if (!TheDriver.isSaveTempsEnabled() && !ForceKeepTempFiles)
+CleanupFileList(TempFiles);
+
delete TranslatedArgs;
delete Args;
@@ -245,6 +250,10 @@
AllActions.clear();
Jobs.clear();
+ // Remove temporary files.
+ if (!TheDriver.isSaveTempsEnabled() && !ForceKeepTempFiles)
+CleanupFileList(TempFiles);
+
// Clear temporary/results file lists.
TempFiles.clear();
ResultFiles.clear();
@@ -262,6 +271,9 @@
// Redirect stdout/stderr to /dev/null.
Redirects = {None, {""}, {""}};
+
+ // Temporary files added by diagnostics should be kept.
+ ForceKeepTempFiles = true;
}
StringRef Compilation::getSysRoot() const {
Index: include/clang/Driver/Compilation.h
===
--- include/clang/Driver/Compilation.h
+++ include/clang/Driver/Compilation.h
@@ -122,6 +122,9 @@
/// Whether an error during the parsing of the input args.
bool ContainsError;
+ /// Whether to keep temporary files regardless of -save-temps.
+ bool ForceKeepTempFiles = false;
+
public:
Compilation(const Driver &D, const ToolChain &DefaultToolChain,
llvm::opt::InputArgList *Args,
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1243,9 +1243,6 @@
// If any of the preprocessing commands failed, clean up and exit.
if (!FailingCommands.empty()) {
-if (!isSaveTempsEnabled())
- C.CleanupFileList(C.getTempFiles(), true);
-
Diag(clang::diag::note_drv_command_failed_diag_msg)
<< "Error generating preprocessed source(s).";
return;
@@ -1362,9 +1359,6 @@
C.ExecuteJobs(C.getJobs(), FailingCommands);
- // Remove temp files.
- C.CleanupFileList(C.getTempFiles());
-
// If the command succeeded, we are done.
if (FailingCommands.empty())
return 0;
Index: lib/Driver/Compilation.cpp
===
--- lib/Driver/Compilation.cpp
+++ lib/Driver/Compilation.cpp
@@ -45,6 +45,11 @@
}
Compilation::~Compilation() {
+ // Remove temporary files. This must be done before arguments are freed, as
+ // the file names might be derived from the input arguments.
+ if (!TheDriver.isSaveTempsEnabled() && !ForceKeepTempFiles)
+CleanupFileList(TempFiles);
+
delete TranslatedArgs;
delete Args;
@@ -245,6 +250,10 @@
AllActions.clear();
Jobs.clear();
+ // Remove temporary files.
+ if (!TheDriver.isSaveTempsEnabled() && !ForceKeepTempFiles)
+CleanupFileList(TempFiles);
+
// Clear temporary/results file lists.
TempFiles.clear();
ResultFiles.clear();
@@ -262,6 +271,9 @@
// Redirect stdout/stderr to /dev/null.
Redirects = {None, {""}, {""}};
+
+ // Temporary files added by diagnostics should be kept.
+ ForceKeepTempFiles = true;
}
StringRef Compilation::getSysRoot() const {
Index: include/clang/Driver/Compilation.h
===
--- include/clang/Driver/Compilation.h
+++ include/clang/Driver/Compilation.h
@@ -122,6 +122,9 @@
/// Whether an error during the parsing of the input args.
bool ContainsError;
+ /// Whether to keep temporary files regardless of -save-temps.
+ bool ForceKeepTempFiles = false;
+
public:
Compilation(const Driver &D, const ToolChain &DefaultToolChain,
llvm::opt::InputArgList *Args,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47331: [clangd] Remove accessors for top-level decls from preamble
ilya-biryukov updated this revision to Diff 148605.
ilya-biryukov added a comment.
- Rewrote findDecl, it will deserialize decls if needed now
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D47331
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/XRefs.cpp
unittests/clangd/TestTU.cpp
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -1,5 +1,4 @@
-//===--- TestTU.cpp - Scratch source files for testing *-
-//C++-*-===//
+//===--- TestTU.cpp - Scratch source files for testing ===//
//
// The LLVM Compiler Infrastructure
//
@@ -73,22 +72,24 @@
}
const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName) {
- const NamedDecl *Result = nullptr;
- for (const Decl *D : AST.getTopLevelDecls()) {
-auto *ND = dyn_cast(D);
-if (!ND || ND->getNameAsString() != QName)
- continue;
-if (Result) {
- ADD_FAILURE() << "Multiple Decls named " << QName;
- assert(false && "QName is not unique");
-}
-Result = ND;
- }
- if (!Result) {
-ADD_FAILURE() << "No Decl named " << QName << " in AST";
-assert(false && "No Decl with QName");
+ llvm::SmallVector Components;
+ QName.split(Components, "::");
+
+ auto &Ctx = AST.getASTContext();
+ auto LookupDecl = [&Ctx](const DeclContext &Scope,
+ llvm::StringRef Name) -> const NamedDecl & {
+auto LookupRes = Scope.lookup(DeclarationName(&Ctx.Idents.get(Name)));
+assert(!LookupRes.empty() && "Lookup failed");
+assert(LookupRes.size() == 1 && "Lookup returned multiple results");
+return *LookupRes.front();
+ };
+
+ const DeclContext *Scope = Ctx.getTranslationUnitDecl();
+ for (auto NameIt = Components.begin(), End = Components.end() - 1;
+ NameIt != End; ++NameIt) {
+Scope = &cast(LookupDecl(*Scope, *NameIt));
}
- return *Result;
+ return LookupDecl(*Scope, Components.back());
}
} // namespace clangd
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -168,7 +168,7 @@
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
IndexOpts.IndexFunctionLocals = true;
- indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
+ indexTopLevelDecls(AST.getASTContext(), AST.getLocalTopLevelDecls(),
DeclMacrosFinder, IndexOpts);
return {DeclMacrosFinder.takeDecls(), DeclMacrosFinder.takeMacroInfos()};
@@ -418,7 +418,7 @@
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
IndexOpts.IndexFunctionLocals = true;
- indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
+ indexTopLevelDecls(AST.getASTContext(), AST.getLocalTopLevelDecls(),
DocHighlightsFinder, IndexOpts);
return DocHighlightsFinder.takeHighlights();
Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -44,12 +44,10 @@
// Stores Preamble and associated data.
struct PreambleData {
- PreambleData(PrecompiledPreamble Preamble,
- std::vector TopLevelDeclIDs,
- std::vector Diags, std::vector Inclusions);
+ PreambleData(PrecompiledPreamble Preamble, std::vector Diags,
+ std::vector Inclusions);
PrecompiledPreamble Preamble;
- std::vector TopLevelDeclIDs;
std::vector Diags;
// Processes like code completions and go-to-definitions will need #include
// information, and their compile action skips preamble range.
@@ -80,17 +78,19 @@
~ParsedAST();
+ /// Note that the returned ast will not contain decls from the preamble that
+ /// were not deserialized during parsing. Clients should expect only decls
+ /// from the main file to be in the AST.
ASTContext &getASTContext();
const ASTContext &getASTContext() const;
Preprocessor &getPreprocessor();
std::shared_ptr getPreprocessorPtr();
const Preprocessor &getPreprocessor() const;
- /// This function returns all top-level decls, including those that come
- /// from Preamble. Decls, coming from Preamble, have to be deserialized, so
- /// this call might be expensive.
- ArrayRef getTopLevelDecls();
+ /// This function returns top-level decls present in the main file of the AST.
+ /// The result does not include the decls that come from the preamble.
+ ArrayRef getLocalTopLevelDecls();
const std::vector &getDiagnostics() const;
@@ -103,11 +103,8 @@
ParsedAST(std::shared_ptr Preamble,
std::unique_ptr Clang,
std::unique_ptr Action,
-std::vector TopLevelDecls, std::vector Diags,
-std::vector Inclusions);
-
-private:
- void ensurePreambleDeclsDeserialized();
+std::vector LocalTopLevelDecls
[PATCH] D46944: [analyzer] Use sufficiently large types for index/size calculation.
a.sidorin added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:89
SymMgr(context, BasicVals, alloc), MemMgr(context, alloc),
-StateMgr(stateMgr), ArrayIndexTy(context.LongLongTy),
+StateMgr(stateMgr), ArrayIndexTy(context.getSignedSizeType()),
ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}
ebevhan wrote:
> a.sidorin wrote:
> > As you correctly pointed, ssize_t is 32-bit on 32-bit systems. Therefore,
> > it is too short. So, we can leave this line as-is.
> But if it's hardcoded to LongLongTy, you have the same problem on 64-bit
> systems.
Some reasons why LongLongTy is used here are listed in D16063. In brief, you
just cannot create an array of size greater than SIZE_MAX/2 on 64-bit
platforms.
https://reviews.llvm.org/D46944
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46944: [analyzer] Use sufficiently large types for index/size calculation.
ebevhan added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:89
SymMgr(context, BasicVals, alloc), MemMgr(context, alloc),
-StateMgr(stateMgr), ArrayIndexTy(context.LongLongTy),
+StateMgr(stateMgr), ArrayIndexTy(context.getSignedSizeType()),
ArrayIndexWidth(context.getTypeSize(ArrayIndexTy)) {}
a.sidorin wrote:
> ebevhan wrote:
> > a.sidorin wrote:
> > > As you correctly pointed, ssize_t is 32-bit on 32-bit systems. Therefore,
> > > it is too short. So, we can leave this line as-is.
> > But if it's hardcoded to LongLongTy, you have the same problem on 64-bit
> > systems.
> Some reasons why LongLongTy is used here are listed in D16063. In brief, you
> just cannot create an array of size greater than SIZE_MAX/2 on 64-bit
> platforms.
I don't think that's limited to 64-bit platforms, it applies to 32-bit ones as
well. I know that LLVM has issues with indexing arrays that are larger than
half of the address space in general due to limitations of GEP.
https://reviews.llvm.org/D46944
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333278 - [analyzer] Added a getLValue method to ProgramState for bases
Author: szelethus
Date: Fri May 25 07:48:33 2018
New Revision: 333278
URL: http://llvm.org/viewvc/llvm-project?rev=333278&view=rev
Log:
[analyzer] Added a getLValue method to ProgramState for bases
Differential Revision: https://reviews.llvm.org/D46891
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h?rev=333278&r1=333277&r2=333278&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
Fri May 25 07:48:33 2018
@@ -294,6 +294,13 @@ public:
ProgramStateRef enterStackFrame(const CallEvent &Call,
const StackFrameContext *CalleeCtx) const;
+ /// Get the lvalue for a base class object reference.
+ Loc getLValue(const CXXBaseSpecifier &BaseSpec, const SubRegion *Super)
const;
+
+ /// Get the lvalue for a base class object reference.
+ Loc getLValue(const CXXRecordDecl *BaseClass, const SubRegion *Super,
+bool IsVirtual) const;
+
/// Get the lvalue for a variable reference.
Loc getLValue(const VarDecl *D, const LocationContext *LC) const;
@@ -724,6 +731,22 @@ inline ProgramStateRef ProgramState::bin
return this;
}
+inline Loc ProgramState::getLValue(const CXXBaseSpecifier &BaseSpec,
+ const SubRegion *Super) const {
+ const auto *Base = BaseSpec.getType()->getAsCXXRecordDecl();
+ return loc::MemRegionVal(
+ getStateManager().getRegionManager().getCXXBaseObjectRegion(
+Base, Super,
BaseSpec.isVirtual()));
+}
+
+inline Loc ProgramState::getLValue(const CXXRecordDecl *BaseClass,
+ const SubRegion *Super,
+ bool IsVirtual) const {
+ return loc::MemRegionVal(
+ getStateManager().getRegionManager().getCXXBaseObjectRegion(
+ BaseClass, Super,
IsVirtual));
+}
+
inline Loc ProgramState::getLValue(const VarDecl *VD,
const LocationContext *LC) const {
return getStateManager().StoreMgr->getLValueVar(VD, LC);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46891: [StaticAnalyzer] Added a getLValue method to ProgramState for bases
This revision was automatically updated to reflect the committed changes.
Closed by commit rC333278: [analyzer] Added a getLValue method to ProgramState
for bases (authored by Szelethus, committed by ).
Repository:
rC Clang
https://reviews.llvm.org/D46891
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -294,6 +294,13 @@
ProgramStateRef enterStackFrame(const CallEvent &Call,
const StackFrameContext *CalleeCtx) const;
+ /// Get the lvalue for a base class object reference.
+ Loc getLValue(const CXXBaseSpecifier &BaseSpec, const SubRegion *Super)
const;
+
+ /// Get the lvalue for a base class object reference.
+ Loc getLValue(const CXXRecordDecl *BaseClass, const SubRegion *Super,
+bool IsVirtual) const;
+
/// Get the lvalue for a variable reference.
Loc getLValue(const VarDecl *D, const LocationContext *LC) const;
@@ -724,6 +731,22 @@
return this;
}
+inline Loc ProgramState::getLValue(const CXXBaseSpecifier &BaseSpec,
+ const SubRegion *Super) const {
+ const auto *Base = BaseSpec.getType()->getAsCXXRecordDecl();
+ return loc::MemRegionVal(
+ getStateManager().getRegionManager().getCXXBaseObjectRegion(
+Base, Super,
BaseSpec.isVirtual()));
+}
+
+inline Loc ProgramState::getLValue(const CXXRecordDecl *BaseClass,
+ const SubRegion *Super,
+ bool IsVirtual) const {
+ return loc::MemRegionVal(
+ getStateManager().getRegionManager().getCXXBaseObjectRegion(
+ BaseClass, Super,
IsVirtual));
+}
+
inline Loc ProgramState::getLValue(const VarDecl *VD,
const LocationContext *LC) const {
return getStateManager().StoreMgr->getLValueVar(VD, LC);
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -294,6 +294,13 @@
ProgramStateRef enterStackFrame(const CallEvent &Call,
const StackFrameContext *CalleeCtx) const;
+ /// Get the lvalue for a base class object reference.
+ Loc getLValue(const CXXBaseSpecifier &BaseSpec, const SubRegion *Super) const;
+
+ /// Get the lvalue for a base class object reference.
+ Loc getLValue(const CXXRecordDecl *BaseClass, const SubRegion *Super,
+bool IsVirtual) const;
+
/// Get the lvalue for a variable reference.
Loc getLValue(const VarDecl *D, const LocationContext *LC) const;
@@ -724,6 +731,22 @@
return this;
}
+inline Loc ProgramState::getLValue(const CXXBaseSpecifier &BaseSpec,
+ const SubRegion *Super) const {
+ const auto *Base = BaseSpec.getType()->getAsCXXRecordDecl();
+ return loc::MemRegionVal(
+ getStateManager().getRegionManager().getCXXBaseObjectRegion(
+Base, Super, BaseSpec.isVirtual()));
+}
+
+inline Loc ProgramState::getLValue(const CXXRecordDecl *BaseClass,
+ const SubRegion *Super,
+ bool IsVirtual) const {
+ return loc::MemRegionVal(
+ getStateManager().getRegionManager().getCXXBaseObjectRegion(
+ BaseClass, Super, IsVirtual));
+}
+
inline Loc ProgramState::getLValue(const VarDecl *VD,
const LocationContext *LC) const {
return getStateManager().StoreMgr->getLValueVar(VD, LC);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r333280 - [clangd] Temporarily disable the test that crashes under asan.
Author: ibiryukov
Date: Fri May 25 07:55:18 2018
New Revision: 333280
URL: http://llvm.org/viewvc/llvm-project?rev=333280&view=rev
Log:
[clangd] Temporarily disable the test that crashes under asan.
It turns out that our fix did not solve the problem completely and the
crash due to stale preamble is still there under asan.
Disabling the test for now, will reenable it when landing a proper fix
for the problem.
Modified:
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL:
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=333280&r1=333279&r2=333280&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Fri May 25
07:55:18 2018
@@ -999,7 +999,8 @@ TEST(CompletionTest, NoIndexCompletionsI
}
}
-TEST(CompletionTest, DocumentationFromChangedFileCrash) {
+// FIXME: This still crashes under asan. Fix it and reenable the test.
+TEST(CompletionTest, DISABLED_DocumentationFromChangedFileCrash) {
MockFSProvider FS;
auto FooH = testPath("foo.h");
auto FooCpp = testPath("foo.cpp");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47375: [Driver] Add flag "--dependent-lib=..." when enabling asan or ubsan on PS4.
pgousseau created this revision.
pgousseau added reviewers: rsmith, filcab, probinson, gbedwell.
Herald added a subscriber: cfe-commits.
NFC for targets other than PS4.
Simplify users' workflow when enabling asan or ubsan and calling the linker
separately.
Repository:
rC Clang
https://reviews.llvm.org/D47375
Files:
lib/Driver/ToolChains/Clang.cpp
lib/Driver/ToolChains/PS4CPU.cpp
lib/Driver/ToolChains/PS4CPU.h
test/Driver/fsanitize.c
test/Driver/sanitizer-ld.c
Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -646,20 +646,25 @@
// RUN: -target x86_64-scei-ps4 -fuse-ld=ld \
// RUN: -shared \
// RUN: | FileCheck --check-prefix=CHECK-UBSAN-PS4 %s
+// CHECK-UBSAN-PS4: --dependent-lib=libSceDbgUBSanitizer_stub_weak.a
// CHECK-UBSAN-PS4: "{{.*}}ld{{(.gold)?(.exe)?}}"
// CHECK-UBSAN-PS4: -lSceDbgUBSanitizer_stub_weak
// RUN: %clang -fsanitize=address %s -### -o %t.o 2>&1 \
// RUN: -target x86_64-scei-ps4 -fuse-ld=ld \
// RUN: -shared \
// RUN: | FileCheck --check-prefix=CHECK-ASAN-PS4 %s
+// CHECK-ASAN-PS4: --dependent-lib=libSceDbgAddressSanitizer_stub_weak.a
// CHECK-ASAN-PS4: "{{.*}}ld{{(.gold)?(.exe)?}}"
// CHECK-ASAN-PS4: -lSceDbgAddressSanitizer_stub_weak
// RUN: %clang -fsanitize=address,undefined %s -### -o %t.o 2>&1 \
// RUN: -target x86_64-scei-ps4 -fuse-ld=ld \
// RUN: -shared \
// RUN: | FileCheck --check-prefix=CHECK-AUBSAN-PS4 %s
+// CHECK-AUBSAN-PS4-NOT: --dependent-lib=libSceDbgUBSanitizer_stub_weak.a
+// CHECK-AUBSAN-PS4: --dependent-lib=libSceDbgAddressSanitizer_stub_weak.a
+// CHECK-AUBSAN-PS4-NOT: --dependent-lib=libSceDbgUBSanitizer_stub_weak.a
// CHECK-AUBSAN-PS4: "{{.*}}ld{{(.gold)?(.exe)?}}"
// CHECK-AUBSAN-PS4: -lSceDbgAddressSanitizer_stub_weak
Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -620,6 +620,7 @@
// CHECK-ESAN-PS4: unsupported option '-fsanitize=efficiency-{{.*}}' for
target 'x86_64-scei-ps4'
// RUN: %clang -target x86_64-scei-ps4 -fsanitize=address %s -### 2>&1 |
FileCheck %s --check-prefix=CHECK-ASAN-PS4
// Make sure there are no *.{o,bc} or -l passed before the ASan library.
+// CHECK-ASAN-PS4: --dependent-lib=libSceDbgAddressSanitizer_stub_weak.a
// CHECK-ASAN-PS4-NOT: {{(\.(o|bc)"? |-l).*-lSceDbgAddressSanitizer_stub_weak}}
// CHECK-ASAN-PS4: -lSceDbgAddressSanitizer_stub_weak
Index: lib/Driver/ToolChains/PS4CPU.h
===
--- lib/Driver/ToolChains/PS4CPU.h
+++ lib/Driver/ToolChains/PS4CPU.h
@@ -23,6 +23,8 @@
void addProfileRTArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
llvm::opt::ArgStringList &CmdArgs);
+void addSanitizerArgs(const ToolChain &TC, llvm::opt::ArgStringList &CmdArgs);
+
class LLVM_LIBRARY_VISIBILITY Assemble : public Tool {
public:
Assemble(const ToolChain &TC)
Index: lib/Driver/ToolChains/PS4CPU.cpp
===
--- lib/Driver/ToolChains/PS4CPU.cpp
+++ lib/Driver/ToolChains/PS4CPU.cpp
@@ -76,6 +76,17 @@
}
}
+void tools::PS4cpu::addSanitizerArgs(const ToolChain &TC,
+ ArgStringList &CmdArgs) {
+ const SanitizerArgs &SanArgs = TC.getSanitizerArgs();
+ if (SanArgs.needsUbsanRt()) {
+CmdArgs.push_back("--dependent-lib=libSceDbgUBSanitizer_stub_weak.a");
+ }
+ if (SanArgs.needsAsanRt()) {
+CmdArgs.push_back("--dependent-lib=libSceDbgAddressSanitizer_stub_weak.a");
+ }
+}
+
static void ConstructPS4LinkJob(const Tool &T, Compilation &C,
const JobAction &JA, const InputInfo &Output,
const InputInfoList &Inputs,
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3687,9 +3687,11 @@
if (auto *ABICompatArg = Args.getLastArg(options::OPT_fclang_abi_compat_EQ))
ABICompatArg->render(Args, CmdArgs);
- // Add runtime flag for PS4 when PGO or Coverage are enabled.
- if (RawTriple.isPS4CPU())
+ // Add runtime flag for PS4 when PGO or Coverage or sanitizers are enabled.
+ if (RawTriple.isPS4CPU()) {
PS4cpu::addProfileRTArgs(getToolChain(), Args, CmdArgs);
+PS4cpu::addSanitizerArgs(getToolChain(), CmdArgs);
+ }
// Pass options for controlling the default header search paths.
if (Args.hasArg(options::OPT_nostdinc)) {
Index: test/Driver/sanitizer-ld.c
===
--- test/Driver/sanitizer-ld.c
+++ test/Driver/sanitizer-ld.c
@@ -646,20 +646,25 @@
// RUN: -target x86_64-scei-ps4 -fuse-ld=ld \
// RUN: -shared \
// RUN: | FileCheck --check-prefix=CHECK-UBSAN-PS4 %s
+// CHE
[PATCH] D47376: [CUDA][HIP] Do not offload for -M
yaxunl created this revision.
yaxunl added a reviewer: tra.
CUDA and HIP action builder currently tries to do offloading for -M, which
causes dependency file
not generated.
This patch changes action builder so that only host compilation is performed to
generate dependency
file.
This assumes that the header files do not depend on whether it is device
compilation or host
compilation. This is not ideal, but at least let projects using -M compile.
Ideally, we should create an offloading action for host dependency file and
device dependency file
and merge them to be on dependency file, which will be done in a separate patch.
https://reviews.llvm.org/D47376
Files:
lib/Driver/Driver.cpp
test/Driver/cuda-phases.cu
Index: test/Driver/cuda-phases.cu
===
--- test/Driver/cuda-phases.cu
+++ test/Driver/cuda-phases.cu
@@ -246,3 +246,14 @@
// DASM2-DAG: [[P7:[0-9]+]]: compiler, {[[P6]]}, ir, (device-[[T]], [[ARCH2]])
// DASM2_NV-DAG: [[P8:[0-9]+]]: backend, {[[P7]]}, assembler, (device-[[T]],
[[ARCH2]])
// DASM2_NV-DAG: [[P9:[0-9]+]]: offload, "device-[[T]] ([[TRIPLE]]:[[ARCH2]])"
{[[P8]]}, assembler
+
+//
+// Test -M does not cause device input.
+//
+// RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases
--cuda-gpu-arch=sm_30 --cuda-gpu-arch=sm_35 %s -M 2>&1 \
+// RUN: | FileCheck -check-prefixes=DEP %s
+// RUN: %clang -x hip -target powerpc64le-ibm-linux-gnu -ccc-print-phases
--cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s -M 2>&1 \
+// RUN: | FileCheck -check-prefixes=DEP %s
+// DEP-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", [[T:.*]],
(host-[[T]])
+// DEP-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, dependencies, (host-[[T]])
+
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2210,7 +2210,10 @@
// Set the flag to true, so that the builder acts on the current input.
IsActive = true;
-if (CompileHostOnly)
+// ToDo: Handle situations where device compilation and host
+// compilation have different dependencies. Currently we assume they
+// are the same therefore device compilation is not performed for -M.
+if (CompileHostOnly || Args.getLastArg(options::OPT_M))
return ABRT_Success;
// Replicate inputs for each GPU architecture.
Index: test/Driver/cuda-phases.cu
===
--- test/Driver/cuda-phases.cu
+++ test/Driver/cuda-phases.cu
@@ -246,3 +246,14 @@
// DASM2-DAG: [[P7:[0-9]+]]: compiler, {[[P6]]}, ir, (device-[[T]], [[ARCH2]])
// DASM2_NV-DAG: [[P8:[0-9]+]]: backend, {[[P7]]}, assembler, (device-[[T]], [[ARCH2]])
// DASM2_NV-DAG: [[P9:[0-9]+]]: offload, "device-[[T]] ([[TRIPLE]]:[[ARCH2]])" {[[P8]]}, assembler
+
+//
+// Test -M does not cause device input.
+//
+// RUN: %clang -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=sm_30 --cuda-gpu-arch=sm_35 %s -M 2>&1 \
+// RUN: | FileCheck -check-prefixes=DEP %s
+// RUN: %clang -x hip -target powerpc64le-ibm-linux-gnu -ccc-print-phases --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %s -M 2>&1 \
+// RUN: | FileCheck -check-prefixes=DEP %s
+// DEP-DAG: [[P0:[0-9]+]]: input, "{{.*}}cuda-phases.cu", [[T:.*]], (host-[[T]])
+// DEP-DAG: [[P1:[0-9]+]]: preprocessor, {[[P0]]}, dependencies, (host-[[T]])
+
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -2210,7 +2210,10 @@
// Set the flag to true, so that the builder acts on the current input.
IsActive = true;
-if (CompileHostOnly)
+// ToDo: Handle situations where device compilation and host
+// compilation have different dependencies. Currently we assume they
+// are the same therefore device compilation is not performed for -M.
+if (CompileHostOnly || Args.getLastArg(options::OPT_M))
return ABRT_Success;
// Replicate inputs for each GPU architecture.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47375: [Driver] Add flag "--dependent-lib=..." when enabling asan or ubsan on PS4.
probinson added a comment.
LGTM with the indicated test tweak, but best if @filcab also takes a look.
Comment at: lib/Driver/ToolChains/PS4CPU.cpp:87
+CmdArgs.push_back("--dependent-lib=libSceDbgAddressSanitizer_stub_weak.a");
+ }
+}
Don't bother with braces for a one-line `if` body.
Comment at: test/Driver/fsanitize.c:624
+// CHECK-ASAN-PS4: --dependent-lib=libSceDbgAddressSanitizer_stub_weak.a
// CHECK-ASAN-PS4-NOT: {{(\.(o|bc)"? |-l).*-lSceDbgAddressSanitizer_stub_weak}}
// CHECK-ASAN-PS4: -lSceDbgAddressSanitizer_stub_weak
Repeat this NOT line before the new check? to preserve the property described
in the comment.
Repository:
rC Clang
https://reviews.llvm.org/D47375
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47260: Testcase for dwarf 5 crash/assert when calculating a checksum for an expansion
probinson added a comment. @trixirt do you mind if I commandeer this review? I think I have a patch. Repository: rC Clang https://reviews.llvm.org/D47260 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333283 - [Sema] Add tests for weak functions
Author: hahnfeld
Date: Fri May 25 08:56:12 2018
New Revision: 333283
URL: http://llvm.org/viewvc/llvm-project?rev=333283&view=rev
Log:
[Sema] Add tests for weak functions
I found these checks to be missing, just add some simple cases.
Differential Revision: https://reviews.llvm.org/D47200
Modified:
cfe/trunk/test/Sema/attr-weak.c
Modified: cfe/trunk/test/Sema/attr-weak.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-weak.c?rev=333283&r1=333282&r2=333283&view=diff
==
--- cfe/trunk/test/Sema/attr-weak.c (original)
+++ cfe/trunk/test/Sema/attr-weak.c Fri May 25 08:56:12 2018
@@ -1,7 +1,9 @@
// RUN: %clang_cc1 -verify -fsyntax-only %s
+extern int f0() __attribute__((weak));
extern int g0 __attribute__((weak));
extern int g1 __attribute__((weak_import));
+int f2() __attribute__((weak));
int g2 __attribute__((weak));
int g3 __attribute__((weak_import)); // expected-warning {{'weak_import'
attribute cannot be specified on a definition}}
int __attribute__((weak_import)) g4(void);
@@ -11,6 +13,7 @@ void __attribute__((weak_import)) g5(voi
struct __attribute__((weak)) s0 {}; // expected-warning {{'weak' attribute
only applies to variables, functions, and classes}}
struct __attribute__((weak_import)) s1 {}; // expected-warning {{'weak_import'
attribute only applies to variables and functions}}
+static int f() __attribute__((weak)); // expected-error {{weak declaration
cannot have internal linkage}}
static int x __attribute__((weak)); // expected-error {{weak declaration
cannot have internal linkage}}
// rdar://9538608
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47200: [Sema] Add tests for weak functions
This revision was automatically updated to reflect the committed changes.
Closed by commit rL333283: [Sema] Add tests for weak functions (authored by
Hahnfeld, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D47200?vs=148021&id=148616#toc
Repository:
rL LLVM
https://reviews.llvm.org/D47200
Files:
cfe/trunk/test/Sema/attr-weak.c
Index: cfe/trunk/test/Sema/attr-weak.c
===
--- cfe/trunk/test/Sema/attr-weak.c
+++ cfe/trunk/test/Sema/attr-weak.c
@@ -1,7 +1,9 @@
// RUN: %clang_cc1 -verify -fsyntax-only %s
+extern int f0() __attribute__((weak));
extern int g0 __attribute__((weak));
extern int g1 __attribute__((weak_import));
+int f2() __attribute__((weak));
int g2 __attribute__((weak));
int g3 __attribute__((weak_import)); // expected-warning {{'weak_import'
attribute cannot be specified on a definition}}
int __attribute__((weak_import)) g4(void);
@@ -11,6 +13,7 @@
struct __attribute__((weak)) s0 {}; // expected-warning {{'weak' attribute
only applies to variables, functions, and classes}}
struct __attribute__((weak_import)) s1 {}; // expected-warning {{'weak_import'
attribute only applies to variables and functions}}
+static int f() __attribute__((weak)); // expected-error {{weak declaration
cannot have internal linkage}}
static int x __attribute__((weak)); // expected-error {{weak declaration
cannot have internal linkage}}
// rdar://9538608
Index: cfe/trunk/test/Sema/attr-weak.c
===
--- cfe/trunk/test/Sema/attr-weak.c
+++ cfe/trunk/test/Sema/attr-weak.c
@@ -1,7 +1,9 @@
// RUN: %clang_cc1 -verify -fsyntax-only %s
+extern int f0() __attribute__((weak));
extern int g0 __attribute__((weak));
extern int g1 __attribute__((weak_import));
+int f2() __attribute__((weak));
int g2 __attribute__((weak));
int g3 __attribute__((weak_import)); // expected-warning {{'weak_import' attribute cannot be specified on a definition}}
int __attribute__((weak_import)) g4(void);
@@ -11,6 +13,7 @@
struct __attribute__((weak)) s0 {}; // expected-warning {{'weak' attribute only applies to variables, functions, and classes}}
struct __attribute__((weak_import)) s1 {}; // expected-warning {{'weak_import' attribute only applies to variables and functions}}
+static int f() __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}}
static int x __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}}
// rdar://9538608
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47229: Make atomic non-member functions as nonnull
jakehehrlich added a comment.
This is causing breaks in fuchsia,
Code that looks like this
uintptr_t last_unlogged =
atomic_load_explicit(&unlogged_tail, memory_order_acquire);
do {
if (last_unlogged == 0)
return;
} while (!atomic_compare_exchange_weak_explicit(&unlogged_tail,
&last_unlogged, 0,
memory_order_acq_rel,
memory_order_relaxed));
Where unlogged_tail is somewhere on the stack. And
'atomic_compare_exchange_weak_explicit' is an #define alias for
'__c11_atomic_compare_exchange_weak'
Full context here:
https://fuchsia.googlesource.com/zircon/+/master/third_party/ulib/musl/ldso/dynlink.c#822
Repository:
rL LLVM
https://reviews.llvm.org/D47229
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47357: [Driver] Rename DefaultTargetTriple to TargetTriple
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm Repository: rC Clang https://reviews.llvm.org/D47357 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47229: Make atomic non-member functions as nonnull
jfb added a comment.
In https://reviews.llvm.org/D47229#1112549, @jakehehrlich wrote:
> This is causing breaks in fuchsia,
>
> Code that looks like this
>
> uintptr_t last_unlogged =
>atomic_load_explicit(&unlogged_tail, memory_order_acquire);
>do {
>if (last_unlogged == 0)
>return;
>} while (!atomic_compare_exchange_weak_explicit(&unlogged_tail,
>&last_unlogged, 0,
>memory_order_acq_rel,
>memory_order_relaxed));
>
>
> Where unlogged_tail is somewhere on the stack. And
> 'atomic_compare_exchange_weak_explicit' is an #define alias for
> '__c11_atomic_compare_exchange_weak'
>
> Full context here:
> https://fuchsia.googlesource.com/zircon/+/master/third_party/ulib/musl/ldso/dynlink.c#822
Here's a repro for what seems to be happening:
#include
void foo() {
atomic_int s;
int a;
atomic_compare_exchange_weak_explicit(&s, &a, 0, memory_order_acq_rel,
memory_order_relaxed);
}
Yields:
./foo.c:5:3: warning: null passed to a callee that requires a non-null
argument [-Wnonnull]
atomic_compare_exchange_weak_explicit(&s, &a, 0, memory_order_acq_rel,
memory_order_relaxed);
^ ~
/s/llvm1/llvm/debug/lib/clang/7.0.0/include/stdatomic.h:144:47: note:
expanded from macro 'atomic_compare_exchange_weak_explicit'
#define atomic_compare_exchange_weak_explicit
__c11_atomic_compare_exchange_weak
^
The problem is that my patch checks that the "desired" value is non-null, but
that's incorrect because it's not a pointer. I'll do a follow-up fix. Thanks
for catching this!
Repository:
rL LLVM
https://reviews.llvm.org/D47229
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333290 - Follow-up fix for nonnull atomic non-member functions
Author: jfb
Date: Fri May 25 10:36:49 2018
New Revision: 333290
URL: http://llvm.org/viewvc/llvm-project?rev=333290&view=rev
Log:
Follow-up fix for nonnull atomic non-member functions
Handling of the third parameter was only checking for *_n and not for the C11
variant, which means that cmpxchg of a 'desired' 0 value was erroneously
warning. Handle C11 properly, and add extgensive tests for this as well as NULL
pointers in a bunch of places.
Fixes r333246 from D47229.
Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Sema/atomic-ops.c
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=333290&r1=333289&r2=333290&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri May 25 10:36:49 2018
@@ -3519,8 +3519,8 @@ ExprResult Sema::SemaAtomicOpsOverloaded
break;
case 2:
// The third argument to compare_exchange / GNU exchange is the desired
-// value, either by-value (for the *_n variant) or as a pointer.
-if (!IsN)
+// value, either by-value (for the C11 and *_n variant) or as a
pointer.
+if (IsPassedByAddress)
CheckNonNullArgument(*this, TheCall->getArg(i), DRE->getLocStart());
Ty = ByValType;
break;
Modified: cfe/trunk/test/Sema/atomic-ops.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/atomic-ops.c?rev=333290&r1=333289&r2=333290&view=diff
==
--- cfe/trunk/test/Sema/atomic-ops.c (original)
+++ cfe/trunk/test/Sema/atomic-ops.c Fri May 25 10:36:49 2018
@@ -530,11 +530,15 @@ void memory_checks(_Atomic(int) *Ap, int
(void)__atomic_compare_exchange_n(p, p, val, 0, memory_order_seq_cst,
memory_order_relaxed);
}
-void nullPointerWarning(_Atomic(int) *Ap, int *p, int val) {
+void nullPointerWarning() {
volatile _Atomic(int) vai;
_Atomic(int) ai;
volatile int vi = 42;
int i = 42;
+ volatile _Atomic(int*) vap;
+ _Atomic(int*) ap;
+ volatile int* vp = NULL;
+ int* p = NULL;
__c11_atomic_init((volatile _Atomic(int)*)0, 42); // expected-warning {{null
passed to a callee that requires a non-null argument}}
__c11_atomic_init((_Atomic(int)*)0, 42); // expected-warning {{null passed
to a callee that requires a non-null argument}}
@@ -607,4 +611,65 @@ void nullPointerWarning(_Atomic(int) *Ap
(void)__atomic_fetch_min((int*)0, 42, memory_order_relaxed); //
expected-warning {{null passed to a callee that requires a non-null argument}}
(void)__atomic_fetch_max((volatile int*)0, 42, memory_order_relaxed); //
expected-warning {{null passed to a callee that requires a non-null argument}}
(void)__atomic_fetch_max((int*)0, 42, memory_order_relaxed); //
expected-warning {{null passed to a callee that requires a non-null argument}}
+
+ // These don't warn: the "desired" parameter is passed by value. Even for
+ // atomic pointers the "desired" result can be NULL.
+ __c11_atomic_init(&vai, 0);
+ __c11_atomic_init(&ai, 0);
+ __c11_atomic_init(&vap, NULL);
+ __c11_atomic_init(&ap, NULL);
+ __c11_atomic_store(&vai, 0, memory_order_relaxed);
+ __c11_atomic_store(&ai, 0, memory_order_relaxed);
+ __c11_atomic_store(&vap, NULL, memory_order_relaxed);
+ __c11_atomic_store(&ap, NULL, memory_order_relaxed);
+ (void)__c11_atomic_exchange(&vai, 0, memory_order_relaxed);
+ (void)__c11_atomic_exchange(&ai, 0, memory_order_relaxed);
+ (void)__c11_atomic_exchange(&vap, NULL, memory_order_relaxed);
+ (void)__c11_atomic_exchange(&ap, NULL, memory_order_relaxed);
+ (void)__c11_atomic_compare_exchange_weak(&vai, &i, 0, memory_order_relaxed,
memory_order_relaxed);
+ (void)__c11_atomic_compare_exchange_weak(&ai, &i, 0, memory_order_relaxed,
memory_order_relaxed);
+ (void)__c11_atomic_compare_exchange_weak(&vap, &p, NULL,
memory_order_relaxed, memory_order_relaxed);
+ (void)__c11_atomic_compare_exchange_weak(&ap, &p, NULL,
memory_order_relaxed, memory_order_relaxed);
+ (void)__c11_atomic_compare_exchange_strong(&vai, &i, 0,
memory_order_relaxed, memory_order_relaxed);
+ (void)__c11_atomic_compare_exchange_strong(&ai, &i, 0, memory_order_relaxed,
memory_order_relaxed);
+ (void)__c11_atomic_compare_exchange_strong(&vap, &p, NULL,
memory_order_relaxed, memory_order_relaxed);
+ (void)__c11_atomic_compare_exchange_strong(&ap, &p, NULL,
memory_order_relaxed, memory_order_relaxed);
+ (void)__c11_atomic_fetch_add(&vai, 0, memory_order_relaxed);
+ (void)__c11_atomic_fetch_add(&ai, 0, memory_order_relaxed);
+ (void)__c11_atomic_fetch_sub(&vai, 0, memory_order_relaxed);
+ (void)__c11_atomic_fetch_sub(&ai, 0, memory_order_relaxed);
+ (void)__c11_atomic_fetch_and(&vai, 0, memory_order_relaxed);
+ (void)__c11_atomic_fetch_and(&ai, 0, memory_order_relaxed);
+ (void)__c11_atomic_fetch_or(&vai,
[PATCH] D47229: Make atomic non-member functions as nonnull
jfb added a comment. Fixed by r333290. Repository: rL LLVM https://reviews.llvm.org/D47229 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types
rnk added a comment.
I think the approach makes sense.
Comment at: lib/CodeGen/CGObjCMac.cpp:7457-7460
CGObjCNonFragileABIMac::GetEHType(QualType T) {
// There's a particular fixed type info for 'id'.
if (T->isObjCIdType() || T->isObjCQualifiedIdType()) {
+if (CGM.getTriple().isWindowsMSVCEnvironment())
@rjmccall how should this be organized in the long run? At this point, the
naming seems totally wrong. Is the non-fragile ABI sort of the canonical way
forward for Obj-C, i.e. it's what a new platform would want to use to best stay
in sync with the future of obj-c?
Repository:
rC Clang
https://reviews.llvm.org/D47233
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47291: Proposal to make rtti errors more generic.
probinson added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:6729 def err_no_dynamic_cast_with_fno_rtti : Error< - "cannot use dynamic_cast with -fno-rtti">; + "use of dynamic_cast requires enabling RTTI">; filcab wrote: > I'd prefer to have the way to enable RTTI mentioned in the message. Could we > have something like `ToolChain::getRTTIMode()` and have a "RTTI was on/of" or > "RTTI defaulted to on/off"? That way we'd be able to have a message similar > to the current one (mentioning "you passed -fno-rtti") on platforms that > default to RTTI=on, and have your updated message (possibly with a mention of > "use -frtti") on platforms that default to RTTI=off. > > (This is a minor usability comment about this patch, I don't consider it a > blocker or anything) If the options are spelled differently for clang-cl and we had a way to retrieve the appropriate spellings, providing the option to use in the diagnostic does seem like a nice touch. Repository: rC Clang https://reviews.llvm.org/D47291 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47067: Update NRVO logic to support early return
Quuxplusone added a comment. Just commenting to say that this LGTM and I have no further nitpicks. I have verified that I cannot detect any change in the behavior of `-Wpessimizing-move` or `-Wreturn-std-move` due to this change (and I //can// successfully detect the additional copy-elision due to this change, hooray). Repository: rC Clang https://reviews.llvm.org/D47067 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan updated this revision to Diff 148637.
leonardchan added a comment.
Changed flag names
Repository:
rC Clang
https://reviews.llvm.org/D46084
Files:
include/clang-c/Index.h
include/clang/AST/ASTContext.h
include/clang/AST/BuiltinTypes.def
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/LangOptions.def
include/clang/Basic/Specifiers.h
include/clang/Basic/TargetInfo.h
include/clang/Basic/TokenKinds.def
include/clang/Driver/Options.td
include/clang/Sema/DeclSpec.h
include/clang/Serialization/ASTBitCodes.h
lib/AST/ASTContext.cpp
lib/AST/ExprConstant.cpp
lib/AST/ItaniumMangle.cpp
lib/AST/MicrosoftMangle.cpp
lib/AST/NSAPI.cpp
lib/AST/Type.cpp
lib/AST/TypeLoc.cpp
lib/Analysis/PrintfFormatString.cpp
lib/Basic/TargetInfo.cpp
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CodeGenTypes.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Index/USRGeneration.cpp
lib/Parse/ParseDecl.cpp
lib/Sema/DeclSpec.cpp
lib/Sema/SemaTemplateVariadic.cpp
lib/Sema/SemaType.cpp
lib/Serialization/ASTCommon.cpp
lib/Serialization/ASTReader.cpp
test/Frontend/fixed_point.c
test/Frontend/fixed_point_bit_widths.c
test/Frontend/fixed_point_errors.c
test/Frontend/fixed_point_errors.cpp
test/Frontend/fixed_point_not_enabled.c
tools/libclang/CXType.cpp
Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -53,6 +53,12 @@
BTCASE(Float);
BTCASE(Double);
BTCASE(LongDouble);
+BTCASE(ShortAccum);
+BTCASE(Accum);
+BTCASE(LongAccum);
+BTCASE(UShortAccum);
+BTCASE(UAccum);
+BTCASE(ULongAccum);
BTCASE(Float16);
BTCASE(Float128);
BTCASE(NullPtr);
@@ -546,6 +552,12 @@
TKIND(Float);
TKIND(Double);
TKIND(LongDouble);
+TKIND(ShortAccum);
+TKIND(Accum);
+TKIND(LongAccum);
+TKIND(UShortAccum);
+TKIND(UAccum);
+TKIND(ULongAccum);
TKIND(Float16);
TKIND(Float128);
TKIND(NullPtr);
Index: test/Frontend/fixed_point_not_enabled.c
===
--- /dev/null
+++ test/Frontend/fixed_point_not_enabled.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -x c -verify %s
+// RUN: %clang_cc1 -x c -fno-fixed-point -verify %s
+
+// Primary fixed point types
+signed short _Accum s_short_accum;// expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+signed _Accum s_accum;// expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+signed long _Accum s_long_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+unsigned short _Accum u_short_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+unsigned _Accum u_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+unsigned long _Accum u_long_accum;// expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+
+// Aliased fixed point types
+short _Accum short_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+_Accum accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
+ // expected-warning@-1{{type specifier missing, defaults to 'int'}}
+long _Accum long_accum; // expected-error{{compile with '-ffixed-point' to enable fixed point types}}
Index: test/Frontend/fixed_point_errors.cpp
===
--- /dev/null
+++ test/Frontend/fixed_point_errors.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -ffixed-point %s -verify
+
+// Name namgling is not provided for fixed point types in c++
+
+signed short _Accum s_short_accum; // expected-error{{fixed point types are only allowed in C}}
+signed _Accum s_accum; // expected-error{{fixed point types are only allowed in C}}
+signed long _Accum s_long_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned short _Accum u_short_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned _Accum u_accum;// expected-error{{fixed point types are only allowed in C}}
+unsigned long _Accum u_long_accum; // expected-error{{fixed point types are only allowed in C}}
+
+short _Accum short_accum; // expected-error{{fixed point types are only allowed in C}}
+_Accum accum; // expected-error{{fixed point types are only allowed in C}}
+// expected-error@-1{{C++ requires a type specifier for all declarations}}
+long _Accum long_accum; // expected-error{{fixed point types are only allowed in C}}
Index: test/Frontend/fixed_poi
[PATCH] D46084: [Fixed Point Arithmetic] Addition of the Fixed Point _Accum type
leonardchan marked 2 inline comments as done. leonardchan added inline comments. Comment at: include/clang/Driver/Options.td:882 +def enable_fixed_point : Flag<["-", "--"], "enable-fixed-point">, Group, + Flags<[CC1Option]>, HelpText<"Enable fixed point types">; ebevhan wrote: > Shouldn't this be an `-f` flag, like `-ffixed-point`? I'm not for or against > either, just wondering what anyone else thinks. Makes sense since this flag is target independent. Repository: rC Clang https://reviews.llvm.org/D46084 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46823: [analyzer] const init: handle non-explicit cases more accurately
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Looks good!
Do you have commit access? I think you should get commit access.
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1650
+
+// If there is a list, but no init, it must be zero.
+if (i >= InitList->getNumInits())
r.stahl wrote:
> r.stahl wrote:
> > NoQ wrote:
> > > NoQ wrote:
> > > > Would this work correctly if the element is not of an integral or
> > > > enumeration type? I think this needs an explicit check.
> > > What if we have an out-of-bounds access to a variable-length array? I
> > > don't think it'd yield zero.
> > I'm getting "variable-sized object may not be initialized", so this case
> > should not be possible.
> I'm having a hard time reproducing this either.
>
>
> ```
> struct S {
> int a = 3;
> };
> S const sarr[2] = {};
> void definit() {
> int i = 1;
> clang_analyzer_dump(sarr[i].a); // expected-warning{{test}}
> }
> ```
>
> results in a symbolic value, because makeZeroVal returns an empty SVal list
> for arrays, records, vectors and complex types. Otherwise it just returns
> UnknownVal (for example for not-yet-implemented floats).
>
> Can you think of a case where this would be an issue?
Yup, sounds reasonable.
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1650-1652
+// If there is a list, but no init, it must be zero.
+if (i >= InitList->getNumInits())
+ return svalBuilder.makeZeroVal(R->getElementType());
NoQ wrote:
> r.stahl wrote:
> > r.stahl wrote:
> > > NoQ wrote:
> > > > NoQ wrote:
> > > > > Would this work correctly if the element is not of an integral or
> > > > > enumeration type? I think this needs an explicit check.
> > > > What if we have an out-of-bounds access to a variable-length array? I
> > > > don't think it'd yield zero.
> > > I'm getting "variable-sized object may not be initialized", so this case
> > > should not be possible.
> > I'm having a hard time reproducing this either.
> >
> >
> > ```
> > struct S {
> > int a = 3;
> > };
> > S const sarr[2] = {};
> > void definit() {
> > int i = 1;
> > clang_analyzer_dump(sarr[i].a); // expected-warning{{test}}
> > }
> > ```
> >
> > results in a symbolic value, because makeZeroVal returns an empty SVal list
> > for arrays, records, vectors and complex types. Otherwise it just returns
> > UnknownVal (for example for not-yet-implemented floats).
> >
> > Can you think of a case where this would be an issue?
> Yup, sounds reasonable.
Had a look. This indeed looks fine, but for a completely different reason. In
fact structs don't appear in `getBindingForElement()` because they all go
through `getBindingForStruct()` instead. Hopefully this does take care of other
cornercases.
So your test case doesn't even trigger your code to be executed, neither would
`S s = sarr[i]; clang_analyzer_dump(s.a);`.
Still, as far as i understand, `sarr` here should be zero-initialized, so i
think the symbolic value can be improved upon, so we might as well add this as
a FIXME test.
https://reviews.llvm.org/D46823
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46845: [libcxx][c++17] P0083R5: Splicing Maps and Sets
rsmith added a comment. In https://reviews.llvm.org/D46845#1105638, @erik.pilkington wrote: > make __map_node_handle use a const_cast on the key instead of type-punning > between pair and pair. Add calls to std::launder in key() > and before inserting a node handle. Can you do this as a separate change, prior to adding node_handle? I would in particular expect this to result in the removal of the `union __value_type` from . https://reviews.llvm.org/D46845 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47260: Testcase for dwarf 5 crash/assert when calculating a checksum for an expansion
trixirt added a comment. commandeer away! Sure that is fine. Repository: rC Clang https://reviews.llvm.org/D47260 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46918: [Coverage] Discard the last uncompleted deferred region in a decl
vsk added a comment. Ping. https://reviews.llvm.org/D46918 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333301 - [OPENMP, NVPTX] Fixed codegen for orphaned parallel region.
Author: abataev
Date: Fri May 25 13:16:03 2018
New Revision: 01
URL: http://llvm.org/viewvc/llvm-project?rev=01&view=rev
Log:
[OPENMP, NVPTX] Fixed codegen for orphaned parallel region.
If orphaned parallel region is found, the next code must be emitted:
```
if(__kmpc_is_spmd_exec_mode() || __kmpc_parallel_level(loc, gtid))
Serialized execution.
else if (IsMasterThread())
Prepare and signal worker.
else
Outined function call.
```
Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp
Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=01&r1=00&r2=01&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Fri May 25 13:16:03 2018
@@ -1845,35 +1845,21 @@ void CGOpenMPRuntimeNVPTX::emitNonSPMDPa
RCG(CGF);
} else {
// Check for master and then parallelism:
- // if (__kmpc_is_spmd_exec_mode()) {
+ // if (__kmpc_is_spmd_exec_mode() || __kmpc_parallel_level(loc, gtid)) {
// Serialized execution.
- // } else if (is_master) {
+ // } else if (master) {
// Worker call.
- // } else if (__kmpc_parallel_level(loc, gtid)) {
- // Serialized execution.
// } else {
// Outlined function call.
// }
CGBuilderTy &Bld = CGF.Builder;
llvm::BasicBlock *ExitBB = CGF.createBasicBlock(".exit");
- llvm::BasicBlock *SPMDCheckBB = CGF.createBasicBlock(".spmdcheck");
+ llvm::BasicBlock *SeqBB = CGF.createBasicBlock(".sequential");
+ llvm::BasicBlock *ParallelCheckBB = CGF.createBasicBlock(".parcheck");
llvm::BasicBlock *MasterCheckBB = CGF.createBasicBlock(".mastercheck");
- llvm::BasicBlock *ParallelCheckBB =
- CGF.createBasicBlock(".parallelcheck");
llvm::Value *IsSPMD = Bld.CreateIsNotNull(CGF.EmitNounwindRuntimeCall(
createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_is_spmd_exec_mode)));
- Bld.CreateCondBr(IsSPMD, SPMDCheckBB, MasterCheckBB);
- CGF.EmitBlock(SPMDCheckBB);
- SeqGen(CGF, Action);
- CGF.EmitBranch(ExitBB);
- CGF.EmitBlock(MasterCheckBB);
- llvm::BasicBlock *MasterThenBB = CGF.createBasicBlock("master.then");
- llvm::Value *IsMaster =
- Bld.CreateICmpEQ(getNVPTXThreadID(CGF), getMasterThreadID(CGF));
- Bld.CreateCondBr(IsMaster, MasterThenBB, ParallelCheckBB);
- CGF.EmitBlock(MasterThenBB);
- L0ParallelGen(CGF, Action);
- CGF.EmitBranch(ExitBB);
+ Bld.CreateCondBr(IsSPMD, SeqBB, ParallelCheckBB);
// There is no need to emit line number for unconditional branch.
(void)ApplyDebugLocation::CreateEmpty(CGF);
CGF.EmitBlock(ParallelCheckBB);
@@ -1883,15 +1869,23 @@ void CGOpenMPRuntimeNVPTX::emitNonSPMDPa
createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_parallel_level),
{RTLoc, ThreadID});
llvm::Value *Res = Bld.CreateIsNotNull(PL);
- llvm::BasicBlock *ThenBlock = CGF.createBasicBlock("omp_if.then");
- llvm::BasicBlock *ElseBlock = CGF.createBasicBlock("omp_if.else");
- Bld.CreateCondBr(Res, ThenBlock, ElseBlock);
- // Emit the 'then' code.
- CGF.EmitBlock(ThenBlock);
+ Bld.CreateCondBr(Res, SeqBB, MasterCheckBB);
+ CGF.EmitBlock(SeqBB);
SeqGen(CGF, Action);
+ CGF.EmitBranch(ExitBB);
+ // There is no need to emit line number for unconditional branch.
+ (void)ApplyDebugLocation::CreateEmpty(CGF);
+ CGF.EmitBlock(MasterCheckBB);
+ llvm::BasicBlock *MasterThenBB = CGF.createBasicBlock("master.then");
+ llvm::BasicBlock *ElseBlock = CGF.createBasicBlock("omp_if.else");
+ llvm::Value *IsMaster =
+ Bld.CreateICmpEQ(getNVPTXThreadID(CGF), getMasterThreadID(CGF));
+ Bld.CreateCondBr(IsMaster, MasterThenBB, ElseBlock);
+ CGF.EmitBlock(MasterThenBB);
+ L0ParallelGen(CGF, Action);
+ CGF.EmitBranch(ExitBB);
// There is no need to emit line number for unconditional branch.
(void)ApplyDebugLocation::CreateEmpty(CGF);
- // Emit the 'else' code.
CGF.EmitBlock(ElseBlock);
RCG(CGF);
// There is no need to emit line number for unconditional branch.
Modified: cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp?rev=01&r1=00&r2=01&view=diff
==
--- cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp Fri May 25 13:16:03 2018
@@ -566,6 +566,10 @@ int baz(int f, double &a) {
// CHECK: icmp ne i8 [[RES]], 0
// CHECK: br i1
+ // CHECK: [[RES:%.+]] = call i16 @__kmpc_parallel_level(%stru
[PATCH] D45897: Convert clang-interpreter to ORC JIT API
This revision was automatically updated to reflect the committed changes.
Closed by commit rL02: Convert clang-interpreter to ORC JIT API (authored
by sas, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D45897?vs=143376&id=148656#toc
Repository:
rL LLVM
https://reviews.llvm.org/D45897
Files:
cfe/trunk/examples/clang-interpreter/CMakeLists.txt
cfe/trunk/examples/clang-interpreter/Invoke.cpp
cfe/trunk/examples/clang-interpreter/Invoke.h
cfe/trunk/examples/clang-interpreter/Manager.cpp
cfe/trunk/examples/clang-interpreter/Manager.h
cfe/trunk/examples/clang-interpreter/main.cpp
Index: cfe/trunk/examples/clang-interpreter/CMakeLists.txt
===
--- cfe/trunk/examples/clang-interpreter/CMakeLists.txt
+++ cfe/trunk/examples/clang-interpreter/CMakeLists.txt
@@ -12,8 +12,6 @@
add_clang_executable(clang-interpreter
main.cpp
- Invoke.cpp
- Manager.cpp
)
add_dependencies(clang-interpreter
Index: cfe/trunk/examples/clang-interpreter/main.cpp
===
--- cfe/trunk/examples/clang-interpreter/main.cpp
+++ cfe/trunk/examples/clang-interpreter/main.cpp
@@ -7,11 +7,8 @@
//
//===--===//
-#include "Invoke.h"
-#include "Manager.h"
-
-#include "clang/CodeGen/CodeGenAction.h"
#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/CodeGen/CodeGenAction.h"
#include "clang/Driver/Compilation.h"
#include "clang/Driver/Driver.h"
#include "clang/Driver/Tool.h"
@@ -21,37 +18,24 @@
#include "clang/Frontend/TextDiagnosticPrinter.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ExecutionEngine/ExecutionEngine.h"
-#include "llvm/ExecutionEngine/MCJIT.h"
+#include "llvm/ExecutionEngine/Orc/CompileUtils.h"
+#include "llvm/ExecutionEngine/Orc/IRCompileLayer.h"
+#include "llvm/ExecutionEngine/Orc/RTDyldObjectLinkingLayer.h"
+#include "llvm/ExecutionEngine/SectionMemoryManager.h"
+#include "llvm/IR/DataLayout.h"
+#include "llvm/IR/Mangler.h"
#include "llvm/IR/Module.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Host.h"
#include "llvm/Support/ManagedStatic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/TargetSelect.h"
#include "llvm/Support/raw_ostream.h"
+#include "llvm/Target/TargetMachine.h"
using namespace clang;
using namespace clang::driver;
-namespace interpreter {
-
-static llvm::ExecutionEngine *
-createExecutionEngine(std::unique_ptr M, std::string *ErrorStr) {
- llvm::EngineBuilder EB(std::move(M));
- EB.setErrorStr(ErrorStr);
- EB.setMemoryManager(llvm::make_unique());
- llvm::ExecutionEngine *EE = EB.create();
- EE->finalizeObject();
- return EE;
-}
-
-// Invoked from a try/catch block in invoke.cpp.
-//
-static int Invoke(llvm::ExecutionEngine *EE, llvm::Function *EntryFn,
- const std::vector &Args, char *const *EnvP) {
- return EE->runFunctionAsMain(EntryFn, Args, EnvP);
-}
-
// This function isn't referenced outside its translation unit, but it
// can't use the "static" keyword because its address is used for
// GetMainExecutable (since some platforms don't support taking the
@@ -61,13 +45,76 @@
return llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
}
-} // namespace interpreter
+namespace llvm {
+namespace orc {
+
+class SimpleJIT {
+private:
+ ExecutionSession ES;
+ std::shared_ptr Resolver;
+ std::unique_ptr TM;
+ const DataLayout DL;
+ RTDyldObjectLinkingLayer ObjectLayer;
+ IRCompileLayer CompileLayer;
+
+public:
+ SimpleJIT()
+ : Resolver(createLegacyLookupResolver(
+ES,
+[this](const std::string &Name) -> JITSymbol {
+ if (auto Sym = CompileLayer.findSymbol(Name, false))
+return Sym;
+ else if (auto Err = Sym.takeError())
+return std::move(Err);
+ if (auto SymAddr =
+ RTDyldMemoryManager::getSymbolAddressInProcess(Name))
+return JITSymbol(SymAddr, JITSymbolFlags::Exported);
+ return nullptr;
+},
+[](Error Err) { cantFail(std::move(Err), "lookupFlags failed"); })),
+TM(EngineBuilder().selectTarget()), DL(TM->createDataLayout()),
+ObjectLayer(ES,
+[this](VModuleKey) {
+ return RTDyldObjectLinkingLayer::Resources{
+ std::make_shared(), Resolver};
+}),
+CompileLayer(ObjectLayer, SimpleCompiler(*TM)) {
+llvm::sys::DynamicLibrary::LoadLibraryPermanently(nullptr);
+ }
+
+ const TargetMachine &getTargetMachine() const { return *TM; }
-int main(int argc, const char **argv, char * const *envp) {
+ VModuleKey addModule(std::unique_ptr M) {
+// Add the module to the JIT with a new VModuleKey.
+auto K = ES.allocateVModule();
+cantFail(CompileLayer.addModule(K, st
[PATCH] D41039: Add support for attribute "trivial_abi"
rsmith added a comment. In https://reviews.llvm.org/D41039#984009, @ahatanak wrote: > Yes, please document this in itanium-cxx-abi. Thanks! Looks like this hasn't happened yet. Repository: rL LLVM https://reviews.llvm.org/D41039 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333307 - [Support] Avoid normalization in sys::getDefaultTargetTriple
Author: phosek Date: Fri May 25 13:39:37 2018 New Revision: 07 URL: http://llvm.org/viewvc/llvm-project?rev=07&view=rev Log: [Support] Avoid normalization in sys::getDefaultTargetTriple The return value of sys::getDefaultTargetTriple, which is derived from -DLLVM_DEFAULT_TRIPLE, is used to construct tool names, default target, and in the future also to control the search path directly; as such it should be used textually, without interpretation by LLVM. Normalization of this value may lead to unexpected results, for example if we configure LLVM with -DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-linux-gnu, normalization will transform that value to x86_64--linux-gnu. Driver will use that value to search for tools prefixed with x86_64--linux-gnu- which may be confusing. This is also inconsistent with the behavior of the --target flag which is taken as-is without any normalization and overrides the value of LLVM_DEFAULT_TARGET_TRIPLE. Users of sys::getDefaultTargetTriple already perform their own normalization as needed, so this change shouldn't impact existing logic. Differential Revision: https://reviews.llvm.org/D47153 Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=07&r1=06&r2=07&view=diff == --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original) +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Fri May 25 13:39:37 2018 @@ -2930,10 +2930,11 @@ static void ParseTargetArgs(TargetOption Opts.FPMath = Args.getLastArgValue(OPT_mfpmath); Opts.FeaturesAsWritten = Args.getAllArgValues(OPT_target_feature); Opts.LinkerVersion = Args.getLastArgValue(OPT_target_linker_version); - Opts.Triple = llvm::Triple::normalize(Args.getLastArgValue(OPT_triple)); + Opts.Triple = Args.getLastArgValue(OPT_triple); // Use the default target triple if unspecified. if (Opts.Triple.empty()) Opts.Triple = llvm::sys::getDefaultTargetTriple(); + Opts.Triple = llvm::Triple::normalize(Opts.Triple); Opts.OpenCLExtensionsAsWritten = Args.getAllArgValues(OPT_cl_ext_EQ); Opts.ForceEnableInt128 = Args.hasArg(OPT_fforce_enable_int128); Opts.NVPTXUseShortPointers = Args.hasFlag( ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47260: [DebugInfo] Skip MD5 checksums of preprocessed files
probinson updated this revision to Diff 148663.
probinson added a comment.
Upload patch to suppress checksums when we see a preprocessed file.
https://reviews.llvm.org/D47260
Files:
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDebugInfo.h
clang/test/CodeGen/md5-checksum-crash.c
Index: clang/test/CodeGen/md5-checksum-crash.c
===
--- /dev/null
+++ clang/test/CodeGen/md5-checksum-crash.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited
-dwarf-version=5 %s -emit-llvm -o- | FileCheck %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -gcodeview -debug-info-kind=limited
%s -emit-llvm -o- | FileCheck %s
+
+// This had been crashing, no MD5 checksum for string.h.
+// Now if there are #line directives, don't bother with checksums
+// as a preprocessed file won't properly reflect the original source.
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+void __NTH() {}
+// Verify no checksum attributes on these files.
+// CHECK-DAG: DIFile(filename: "{{.*}}.c", directory: "{{[^"]*}}")
+// CHECK-DAG: DIFile(filename: "{{.*}}string.h", directory: "{{[^"]*}}")
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -57,6 +57,7 @@
CodeGenModule &CGM;
const codegenoptions::DebugInfoKind DebugKind;
bool DebugTypeExtRefs;
+ mutable bool EmitFileChecksums;
llvm::DIBuilder DBuilder;
llvm::DICompileUnit *TheCU = nullptr;
ModuleMap *ClangModuleMap = nullptr;
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -67,6 +67,8 @@
DBuilder(CGM.getModule()) {
for (const auto &KV : CGM.getCodeGenOpts().DebugPrefixMap)
DebugPrefixMap[KV.first] = KV.second;
+ EmitFileChecksums = CGM.getCodeGenOpts().EmitCodeView ||
+ CGM.getCodeGenOpts().DwarfVersion >= 5;
CreateCompileUnit();
}
@@ -365,15 +367,19 @@
CGDebugInfo::computeChecksum(FileID FID, SmallString<32> &Checksum) const {
Checksum.clear();
- if (!CGM.getCodeGenOpts().EmitCodeView &&
- CGM.getCodeGenOpts().DwarfVersion < 5)
+ if (!EmitFileChecksums)
return None;
SourceManager &SM = CGM.getContext().getSourceManager();
bool Invalid;
- llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID, &Invalid);
- if (Invalid)
+ const SrcMgr::SLocEntry &Entry = SM.getSLocEntry(FID, &Invalid);
+ if (Invalid || !Entry.isFile())
return None;
+ if (Entry.getFile().hasLineDirectives()) {
+EmitFileChecksums = false;
+return None;
+ }
+ llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID);
llvm::MD5 Hash;
llvm::MD5::MD5Result Result;
Index: clang/test/CodeGen/md5-checksum-crash.c
===
--- /dev/null
+++ clang/test/CodeGen/md5-checksum-crash.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited -dwarf-version=5 %s -emit-llvm -o- | FileCheck %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -gcodeview -debug-info-kind=limited %s -emit-llvm -o- | FileCheck %s
+
+// This had been crashing, no MD5 checksum for string.h.
+// Now if there are #line directives, don't bother with checksums
+// as a preprocessed file won't properly reflect the original source.
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+void __NTH() {}
+// Verify no checksum attributes on these files.
+// CHECK-DAG: DIFile(filename: "{{.*}}.c", directory: "{{[^"]*}}")
+// CHECK-DAG: DIFile(filename: "{{.*}}string.h", directory: "{{[^"]*}}")
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -57,6 +57,7 @@
CodeGenModule &CGM;
const codegenoptions::DebugInfoKind DebugKind;
bool DebugTypeExtRefs;
+ mutable bool EmitFileChecksums;
llvm::DIBuilder DBuilder;
llvm::DICompileUnit *TheCU = nullptr;
ModuleMap *ClangModuleMap = nullptr;
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -67,6 +67,8 @@
DBuilder(CGM.getModule()) {
for (const auto &KV : CGM.getCodeGenOpts().DebugPrefixMap)
DebugPrefixMap[KV.first] = KV.second;
+ EmitFileChecksums = CGM.getCodeGenOpts().EmitCodeView ||
+ CGM.getCodeGenOpts().DwarfVersion >= 5;
CreateCompileUnit();
}
@@ -365,15 +367,19 @@
CGDebugInfo::computeChecksum(FileID FID, SmallString<32> &Checksum) const {
Checksum.clear();
- if (!CGM.getCodeGenOpts().EmitCodeView &&
- CGM.getCodeGenOpts().DwarfVersion < 5)
+ if (!EmitFileChecksums)
return None;
SourceManager &SM = CGM.
[libcxx] r333308 - Fix optional deduction guide test breakage
Author: jfb
Date: Fri May 25 13:43:57 2018
New Revision: 08
URL: http://llvm.org/viewvc/llvm-project?rev=08&view=rev
Log:
Fix optional deduction guide test breakage
Modified:
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
Modified:
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp?rev=08&r1=07&r2=08&view=diff
==
---
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp
(original)
+++
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.fail.cpp
Fri May 25 13:43:57 2018
@@ -28,7 +28,7 @@ int main()
// Test the implicit deduction guides
{
// optional()
-std::optional opt; // expected-error {{no viable constructor or
deduction guide for deduction of template arguments of 'optional'}}
+std::optional opt; // expected-error {{declaration of variable 'opt'
with deduced type 'std::optional' requires an initializer}}
}
{
Modified:
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp?rev=08&r1=07&r2=08&view=diff
==
---
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
(original)
+++
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
Fri May 25 13:43:57 2018
@@ -45,7 +45,7 @@ int main()
// optional(const optional &);
std::optional source('A');
std::optional opt(source);
-static_assert(std::is_same_v>, "");
+static_assert(std::is_same_v>>, "");
assert(static_cast(opt) == static_cast(source));
assert(*opt == *source);
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47260: [DebugInfo] Skip MD5 checksums of preprocessed files
aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.
Minor comment inline.
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:378
return None;
+ if (Entry.getFile().hasLineDirectives()) {
+EmitFileChecksums = false;
Can you add a comment explaining why we are doing this here?
https://reviews.llvm.org/D47260
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47260: [DebugInfo] Skip MD5 checksums of preprocessed files
probinson marked an inline comment as done.
probinson added inline comments.
Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:378
return None;
+ if (Entry.getFile().hasLineDirectives()) {
+EmitFileChecksums = false;
aprantl wrote:
> Can you add a comment explaining why we are doing this here?
Of course.
https://reviews.llvm.org/D47260
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47260: [DebugInfo] Skip MD5 checksums of preprocessed files
This revision was automatically updated to reflect the committed changes.
probinson marked an inline comment as done.
Closed by commit rC11: [DebugInfo] Don't bother with MD5 checksums of
preprocessed files. (authored by probinson, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D47260?vs=148663&id=148667#toc
Repository:
rL LLVM
https://reviews.llvm.org/D47260
Files:
lib/CodeGen/CGDebugInfo.cpp
lib/CodeGen/CGDebugInfo.h
test/CodeGen/md5-checksum-crash.c
Index: test/CodeGen/md5-checksum-crash.c
===
--- test/CodeGen/md5-checksum-crash.c
+++ test/CodeGen/md5-checksum-crash.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited
-dwarf-version=5 %s -emit-llvm -o- | FileCheck %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -gcodeview -debug-info-kind=limited
%s -emit-llvm -o- | FileCheck %s
+
+// This had been crashing, no MD5 checksum for string.h.
+// Now if there are #line directives, don't bother with checksums
+// as a preprocessed file won't properly reflect the original source.
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+void __NTH() {}
+// Verify no checksum attributes on these files.
+// CHECK-DAG: DIFile(filename: "{{.*}}.c", directory: "{{[^"]*}}")
+// CHECK-DAG: DIFile(filename: "{{.*}}string.h", directory: "{{[^"]*}}")
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -67,6 +67,8 @@
DBuilder(CGM.getModule()) {
for (const auto &KV : CGM.getCodeGenOpts().DebugPrefixMap)
DebugPrefixMap[KV.first] = KV.second;
+ EmitFileChecksums = CGM.getCodeGenOpts().EmitCodeView ||
+ CGM.getCodeGenOpts().DwarfVersion >= 5;
CreateCompileUnit();
}
@@ -365,15 +367,21 @@
CGDebugInfo::computeChecksum(FileID FID, SmallString<32> &Checksum) const {
Checksum.clear();
- if (!CGM.getCodeGenOpts().EmitCodeView &&
- CGM.getCodeGenOpts().DwarfVersion < 5)
+ if (!EmitFileChecksums)
return None;
SourceManager &SM = CGM.getContext().getSourceManager();
bool Invalid;
- llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID, &Invalid);
- if (Invalid)
+ const SrcMgr::SLocEntry &Entry = SM.getSLocEntry(FID, &Invalid);
+ if (Invalid || !Entry.isFile())
return None;
+ if (Entry.getFile().hasLineDirectives()) {
+// This must be a preprocessed file; its content won't match the original
+// source; therefore checksumming the text we have is pointless or wrong.
+EmitFileChecksums = false;
+return None;
+ }
+ llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID);
llvm::MD5 Hash;
llvm::MD5::MD5Result Result;
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -57,6 +57,7 @@
CodeGenModule &CGM;
const codegenoptions::DebugInfoKind DebugKind;
bool DebugTypeExtRefs;
+ mutable bool EmitFileChecksums;
llvm::DIBuilder DBuilder;
llvm::DICompileUnit *TheCU = nullptr;
ModuleMap *ClangModuleMap = nullptr;
Index: test/CodeGen/md5-checksum-crash.c
===
--- test/CodeGen/md5-checksum-crash.c
+++ test/CodeGen/md5-checksum-crash.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited -dwarf-version=5 %s -emit-llvm -o- | FileCheck %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -gcodeview -debug-info-kind=limited %s -emit-llvm -o- | FileCheck %s
+
+// This had been crashing, no MD5 checksum for string.h.
+// Now if there are #line directives, don't bother with checksums
+// as a preprocessed file won't properly reflect the original source.
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+void __NTH() {}
+// Verify no checksum attributes on these files.
+// CHECK-DAG: DIFile(filename: "{{.*}}.c", directory: "{{[^"]*}}")
+// CHECK-DAG: DIFile(filename: "{{.*}}string.h", directory: "{{[^"]*}}")
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -67,6 +67,8 @@
DBuilder(CGM.getModule()) {
for (const auto &KV : CGM.getCodeGenOpts().DebugPrefixMap)
DebugPrefixMap[KV.first] = KV.second;
+ EmitFileChecksums = CGM.getCodeGenOpts().EmitCodeView ||
+ CGM.getCodeGenOpts().DwarfVersion >= 5;
CreateCompileUnit();
}
@@ -365,15 +367,21 @@
CGDebugInfo::computeChecksum(FileID FID, SmallString<32> &Checksum) const {
Checksum.clear();
- if (!CGM.getCodeGenOpts().EmitCodeView &&
- CGM.getCodeGenOpts().DwarfVersion < 5)
+ if (!EmitFileChecksums)
return None;
SourceManager &SM = CGM.getContext().getSourceManager();
bool Invalid;
- llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID, &Invalid);
-
r333311 - [DebugInfo] Don't bother with MD5 checksums of preprocessed files.
Author: probinson
Date: Fri May 25 13:59:29 2018
New Revision: 11
URL: http://llvm.org/viewvc/llvm-project?rev=11&view=rev
Log:
[DebugInfo] Don't bother with MD5 checksums of preprocessed files.
The checksum will not reflect the real source, so there's no clear
reason to include them in the debug info. Also this was causing a
crash on the DWARF side.
Differential Revision: https://reviews.llvm.org/D47260
Added:
cfe/trunk/test/CodeGen/md5-checksum-crash.c
Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h
Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=11&r1=10&r2=11&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri May 25 13:59:29 2018
@@ -67,6 +67,8 @@ CGDebugInfo::CGDebugInfo(CodeGenModule &
DBuilder(CGM.getModule()) {
for (const auto &KV : CGM.getCodeGenOpts().DebugPrefixMap)
DebugPrefixMap[KV.first] = KV.second;
+ EmitFileChecksums = CGM.getCodeGenOpts().EmitCodeView ||
+ CGM.getCodeGenOpts().DwarfVersion >= 5;
CreateCompileUnit();
}
@@ -365,15 +367,21 @@ Optional
CGDebugInfo::computeChecksum(FileID FID, SmallString<32> &Checksum) const {
Checksum.clear();
- if (!CGM.getCodeGenOpts().EmitCodeView &&
- CGM.getCodeGenOpts().DwarfVersion < 5)
+ if (!EmitFileChecksums)
return None;
SourceManager &SM = CGM.getContext().getSourceManager();
bool Invalid;
- llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID, &Invalid);
- if (Invalid)
+ const SrcMgr::SLocEntry &Entry = SM.getSLocEntry(FID, &Invalid);
+ if (Invalid || !Entry.isFile())
return None;
+ if (Entry.getFile().hasLineDirectives()) {
+// This must be a preprocessed file; its content won't match the original
+// source; therefore checksumming the text we have is pointless or wrong.
+EmitFileChecksums = false;
+return None;
+ }
+ llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID);
llvm::MD5 Hash;
llvm::MD5::MD5Result Result;
Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=11&r1=10&r2=11&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Fri May 25 13:59:29 2018
@@ -57,6 +57,7 @@ class CGDebugInfo {
CodeGenModule &CGM;
const codegenoptions::DebugInfoKind DebugKind;
bool DebugTypeExtRefs;
+ mutable bool EmitFileChecksums;
llvm::DIBuilder DBuilder;
llvm::DICompileUnit *TheCU = nullptr;
ModuleMap *ClangModuleMap = nullptr;
Added: cfe/trunk/test/CodeGen/md5-checksum-crash.c
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/md5-checksum-crash.c?rev=11&view=auto
==
--- cfe/trunk/test/CodeGen/md5-checksum-crash.c (added)
+++ cfe/trunk/test/CodeGen/md5-checksum-crash.c Fri May 25 13:59:29 2018
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited
-dwarf-version=5 %s -emit-llvm -o- | FileCheck %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -gcodeview -debug-info-kind=limited
%s -emit-llvm -o- | FileCheck %s
+
+// This had been crashing, no MD5 checksum for string.h.
+// Now if there are #line directives, don't bother with checksums
+// as a preprocessed file won't properly reflect the original source.
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+void __NTH() {}
+// Verify no checksum attributes on these files.
+// CHECK-DAG: DIFile(filename: "{{.*}}.c", directory: "{{[^"]*}}")
+// CHECK-DAG: DIFile(filename: "{{.*}}string.h", directory: "{{[^"]*}}")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47260: [DebugInfo] Skip MD5 checksums of preprocessed files
This revision was automatically updated to reflect the committed changes.
probinson marked an inline comment as done.
Closed by commit rL11: [DebugInfo] Don't bother with MD5 checksums of
preprocessed files. (authored by probinson, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D47260?vs=148663&id=148668#toc
Repository:
rL LLVM
https://reviews.llvm.org/D47260
Files:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h
cfe/trunk/test/CodeGen/md5-checksum-crash.c
Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -67,6 +67,8 @@
DBuilder(CGM.getModule()) {
for (const auto &KV : CGM.getCodeGenOpts().DebugPrefixMap)
DebugPrefixMap[KV.first] = KV.second;
+ EmitFileChecksums = CGM.getCodeGenOpts().EmitCodeView ||
+ CGM.getCodeGenOpts().DwarfVersion >= 5;
CreateCompileUnit();
}
@@ -365,15 +367,21 @@
CGDebugInfo::computeChecksum(FileID FID, SmallString<32> &Checksum) const {
Checksum.clear();
- if (!CGM.getCodeGenOpts().EmitCodeView &&
- CGM.getCodeGenOpts().DwarfVersion < 5)
+ if (!EmitFileChecksums)
return None;
SourceManager &SM = CGM.getContext().getSourceManager();
bool Invalid;
- llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID, &Invalid);
- if (Invalid)
+ const SrcMgr::SLocEntry &Entry = SM.getSLocEntry(FID, &Invalid);
+ if (Invalid || !Entry.isFile())
return None;
+ if (Entry.getFile().hasLineDirectives()) {
+// This must be a preprocessed file; its content won't match the original
+// source; therefore checksumming the text we have is pointless or wrong.
+EmitFileChecksums = false;
+return None;
+ }
+ llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID);
llvm::MD5 Hash;
llvm::MD5::MD5Result Result;
Index: cfe/trunk/lib/CodeGen/CGDebugInfo.h
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h
@@ -57,6 +57,7 @@
CodeGenModule &CGM;
const codegenoptions::DebugInfoKind DebugKind;
bool DebugTypeExtRefs;
+ mutable bool EmitFileChecksums;
llvm::DIBuilder DBuilder;
llvm::DICompileUnit *TheCU = nullptr;
ModuleMap *ClangModuleMap = nullptr;
Index: cfe/trunk/test/CodeGen/md5-checksum-crash.c
===
--- cfe/trunk/test/CodeGen/md5-checksum-crash.c
+++ cfe/trunk/test/CodeGen/md5-checksum-crash.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited
-dwarf-version=5 %s -emit-llvm -o- | FileCheck %s
+// RUN: %clang_cc1 -triple %ms_abi_triple -gcodeview -debug-info-kind=limited
%s -emit-llvm -o- | FileCheck %s
+
+// This had been crashing, no MD5 checksum for string.h.
+// Now if there are #line directives, don't bother with checksums
+// as a preprocessed file won't properly reflect the original source.
+#define __NTH fct
+void fn1() {}
+# 7 "/usr/include/string.h"
+void __NTH() {}
+// Verify no checksum attributes on these files.
+// CHECK-DAG: DIFile(filename: "{{.*}}.c", directory: "{{[^"]*}}")
+// CHECK-DAG: DIFile(filename: "{{.*}}string.h", directory: "{{[^"]*}}")
Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -67,6 +67,8 @@
DBuilder(CGM.getModule()) {
for (const auto &KV : CGM.getCodeGenOpts().DebugPrefixMap)
DebugPrefixMap[KV.first] = KV.second;
+ EmitFileChecksums = CGM.getCodeGenOpts().EmitCodeView ||
+ CGM.getCodeGenOpts().DwarfVersion >= 5;
CreateCompileUnit();
}
@@ -365,15 +367,21 @@
CGDebugInfo::computeChecksum(FileID FID, SmallString<32> &Checksum) const {
Checksum.clear();
- if (!CGM.getCodeGenOpts().EmitCodeView &&
- CGM.getCodeGenOpts().DwarfVersion < 5)
+ if (!EmitFileChecksums)
return None;
SourceManager &SM = CGM.getContext().getSourceManager();
bool Invalid;
- llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID, &Invalid);
- if (Invalid)
+ const SrcMgr::SLocEntry &Entry = SM.getSLocEntry(FID, &Invalid);
+ if (Invalid || !Entry.isFile())
return None;
+ if (Entry.getFile().hasLineDirectives()) {
+// This must be a preprocessed file; its content won't match the original
+// source; therefore checksumming the text we have is pointless or wrong.
+EmitFileChecksums = false;
+return None;
+ }
+ llvm::MemoryBuffer *MemBuffer = SM.getBuffer(FID);
llvm::MD5 Hash;
llvm::MD5::MD5Result Result;
Index: cfe/trunk/lib/CodeGen/CGDebugInfo.h
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.h
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.h
@@ -57,6 +57,7 @@
Cod
[PATCH] D47340: [ClangDiagnostics] Silence warning about fallthrough after PrintFatalError
xbolva00 added a comment. Ok? https://reviews.llvm.org/D47340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call
NoQ added a comment. Ok! Putting any remaining work into follow-up patches would be great indeed. Let's land this into `alpha.cplusplus` for now because i still haven't made up my mind for how to organize the proposed `bugprone` category (sorry!). Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:223-225 + ExplodedNode *Node = Context.generateNonFatalErrorNode(Context.getState()); + if (!Node) +return; Szelethus wrote: > NoQ wrote: > > I suspect that a fatal error is better here. We don't want the user to > > receive duplicate report from other checkers that catch uninitialized > > values; just one report is enough. > I think that would be a bad idea. For example, this checker shouts so loudly > while checking the LLVM project, it would practically halt the analysis of > the code, reducing the coverage, which means that checkers other then uninit > value checkers would "suffer" from it. > > However, I also think that having multiple uninit reports for the same object > might be good, especially with this checker, as it would be very easy to see > where the problem originated from. > > What do you think? Well, i guess that's the reason to not use the checker on LLVM. Regardless of fatal/nonfatal warnings, enabling this checker on LLVM regularly would be a bad idea because it's unlikely that anybody will be able to fix all the false positives to make it usable. And for other projects that don't demonstrate many false positives, this shouldn't be a problem. In order to indicate where the problem originates from, we have our bug reporter visitors that try their best to add such info directly to the report. In particular, @george.karpenkov's recent `NoStoreFuncVisitor` highlights functions in which a variable was //not// initialized but was probably expected to be. Not sure if it highlights constructors in its current shape, but that's definitely a better way to give this piece of information to the user because it doesn't make the user look for a different report to understand the current report. Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:258-260 +Report->addNote(NoteBuf, +PathDiagnosticLocation::create(FieldChain.getEndOfChain(), + Context.getSourceManager())); Szelethus wrote: > NoQ wrote: > > NoQ wrote: > > > Szelethus wrote: > > > > NoQ wrote: > > > > > Aha, ok, got it, so you're putting the warning at the end of the > > > > > constructor and squeezing all uninitialized fields into a single > > > > > warning by presenting them as notes. > > > > > > > > > > Because for now notes aren't supported everywhere (and aren't always > > > > > supported really well), i guess it'd be necessary to at least provide > > > > > an option to make note-less reports before the checker is enabled by > > > > > default everywhere. In this case notes contain critical pieces of > > > > > information and as such cannot be really discarded. > > > > > > > > > > I'm not sure that notes are providing a better user experience here > > > > > than simply saying that "field x.y->z is never initialized". With > > > > > notes, the user will have to scroll around (at least in scan-build) > > > > > to find which field is uninitialized. > > > > > > > > > > Notes will probably also not decrease the number of reports too much > > > > > because in most cases there will be just one uninitialized field. > > > > > Though i agree that when there is more than one report, the user will > > > > > be likely to deal with all fields at once rather than separately. > > > > > > > > > > So it's a bit wonky. > > > > > > > > > > We might want to enable one mode or another in the checker depending > > > > > on the analyzer output flag. Probably in the driver > > > > > (`RenderAnalyzerOptions()`). > > > > > > > > > > It'd be a good idea to land the checker into alpha first and fix this > > > > > in a follow-up patch because this review is already overweight. > > > > > [...]i guess it'd be necessary to at least provide an option to make > > > > > note-less reports before the checker is enabled by default > > > > > everywhere. In this case notes contain critical pieces of information > > > > > and as such cannot be really discarded. > > > > This can already be achieved with `-analyzer-config > > > > notes-as-events=true`. There no good reason however not to make an > > > > additional flag that would emit warnings instead of notes. > > > > > I'm not sure that notes are providing a better user experience here > > > > > than simply saying that "field x.y->z is never initialized". With > > > > > notes, the user will have to scroll around (at least in scan-build) > > > > > to find which field is uninitialized. > > > > This checker had a previous implementation that emitted a warning for > > > > each uninitialized fiel
[libcxx] r333315 - Fix array deduction guide test breakage
Author: jfb
Date: Fri May 25 14:17:43 2018
New Revision: 15
URL: http://llvm.org/viewvc/llvm-project?rev=15&view=rev
Log:
Fix array deduction guide test breakage
No matching constructor
Modified:
libcxx/trunk/test/std/containers/sequences/array/array.cons/deduct.pass.cpp
Modified:
libcxx/trunk/test/std/containers/sequences/array/array.cons/deduct.pass.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/containers/sequences/array/array.cons/deduct.pass.cpp?rev=15&r1=14&r2=15&view=diff
==
--- libcxx/trunk/test/std/containers/sequences/array/array.cons/deduct.pass.cpp
(original)
+++ libcxx/trunk/test/std/containers/sequences/array/array.cons/deduct.pass.cpp
Fri May 25 14:17:43 2018
@@ -51,6 +51,8 @@ int main()
}
// Test the implicit deduction guides
+// FIXME broken: no matching constructor
+#if 0
{
std::array source = {4.0, 5.0};
std::array arr(source); // array(array)
@@ -59,4 +61,5 @@ int main()
assert(arr[0] == 4.0);
assert(arr[1] == 5.0);
}
+#endif
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46918: [Coverage] Discard the last uncompleted deferred region in a decl
arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D46918 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47393: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for Objective-C 📜
stephanemoore created this revision.
Herald added subscribers: cfe-commits, klimek.
Repository:
rC Clang
https://reviews.llvm.org/D47393
Files:
lib/Format/Format.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1193,6 +1193,22 @@
"}");
}
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+ FormatStyle Style = getGoogleStyle();
+ verifyFormat(" = @\"\"\n"
+ " @\"\";",
+ Style);
+ verifyFormat("(@\"\"\n"
+ " @\"\");",
+ Style);
+ verifyFormat("(qqq, @\"\"\n"
+ " @\"\");",
+ Style);
+ verifyFormat("NSString *const kString = @\"\"\n"
+ " @\"\");",
+ Style);
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -799,6 +799,7 @@
// has been implemented.
GoogleStyle.BreakStringLiterals = false;
} else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
GoogleStyle.ColumnLimit = 100;
}
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1193,6 +1193,22 @@
"}");
}
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+ FormatStyle Style = getGoogleStyle();
+ verifyFormat(" = @\"\"\n"
+ " @\"\";",
+ Style);
+ verifyFormat("(@\"\"\n"
+ " @\"\");",
+ Style);
+ verifyFormat("(qqq, @\"\"\n"
+ " @\"\");",
+ Style);
+ verifyFormat("NSString *const kString = @\"\"\n"
+ " @\"\");",
+ Style);
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -799,6 +799,7 @@
// has been implemented.
GoogleStyle.BreakStringLiterals = false;
} else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
GoogleStyle.ColumnLimit = 100;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r333316 - Support Swift calling convention for PPC64 targets
Author: bwilson
Date: Fri May 25 14:26:03 2018
New Revision: 16
URL: http://llvm.org/viewvc/llvm-project?rev=16&view=rev
Log:
Support Swift calling convention for PPC64 targets
This adds basic support for the Swift calling convention with PPC64 targets.
Patch provided by Atul Sowani in bug report #37223
Modified:
cfe/trunk/lib/Basic/Targets/PPC.h
cfe/trunk/lib/CodeGen/TargetInfo.cpp
Modified: cfe/trunk/lib/Basic/Targets/PPC.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/PPC.h?rev=16&r1=15&r2=16&view=diff
==
--- cfe/trunk/lib/Basic/Targets/PPC.h (original)
+++ cfe/trunk/lib/Basic/Targets/PPC.h Fri May 25 14:26:03 2018
@@ -335,6 +335,15 @@ public:
}
return false;
}
+
+ CallingConvCheckResult checkCallingConvention(CallingConv CC) const override
{
+switch (CC) {
+case CC_Swift:
+ return CCCR_OK;
+default:
+ return CCCR_Warning;
+}
+ }
};
class LLVM_LIBRARY_VISIBILITY DarwinPPC32TargetInfo
Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=16&r1=15&r2=16&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Fri May 25 14:26:03 2018
@@ -4287,7 +4287,7 @@ PPC32TargetCodeGenInfo::initDwarfEHRegSi
namespace {
/// PPC64_SVR4_ABIInfo - The 64-bit PowerPC ELF (SVR4) ABI information.
-class PPC64_SVR4_ABIInfo : public ABIInfo {
+class PPC64_SVR4_ABIInfo : public SwiftABIInfo {
public:
enum ABIKind {
ELFv1 = 0,
@@ -4331,7 +4331,7 @@ private:
public:
PPC64_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT, ABIKind Kind, bool HasQPX,
bool SoftFloatABI)
- : ABIInfo(CGT), Kind(Kind), HasQPX(HasQPX),
+ : SwiftABIInfo(CGT), Kind(Kind), HasQPX(HasQPX),
IsSoftFloatABI(SoftFloatABI) {}
bool isPromotableTypeForABI(QualType Ty) const;
@@ -4374,6 +4374,15 @@ public:
Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
QualType Ty) const override;
+
+ bool shouldPassIndirectlyForSwift(ArrayRef scalars,
+bool asReturnValue) const override {
+return occupiesMoreThan(CGT, scalars, /*total*/ 4);
+ }
+
+ bool isSwiftErrorInRegister() const override {
+return false;
+ }
};
class PPC64_SVR4_TargetCodeGenInfo : public TargetCodeGenInfo {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46550: Support Swift calling convention for PPC64 targets
bob.wilson closed this revision. bob.wilson added a comment. Committed in Clang r16 Repository: rC Clang https://reviews.llvm.org/D46550 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47393: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for Objective-C 📜
stephanemoore updated this revision to Diff 148672.
Repository:
rC Clang
https://reviews.llvm.org/D47393
Files:
lib/Format/Format.cpp
unittests/Format/FormatTestObjC.cpp
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1193,6 +1193,18 @@
"}");
}
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+ Style = getGoogleStyle();
+ verifyFormat(" = @\"\"\n"
+ " @\"\";");
+ verifyFormat("(@\"\"\n"
+ " @\"\");");
+ verifyFormat("(qqq, @\"\"\n"
+ " @\"\");");
+ verifyFormat("NSString *const kString = @\"\"\n"
+ " @\"\");");
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -799,6 +799,7 @@
// has been implemented.
GoogleStyle.BreakStringLiterals = false;
} else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
GoogleStyle.ColumnLimit = 100;
}
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -1193,6 +1193,18 @@
"}");
}
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+ Style = getGoogleStyle();
+ verifyFormat(" = @\"\"\n"
+ " @\"\";");
+ verifyFormat("(@\"\"\n"
+ " @\"\");");
+ verifyFormat("(qqq, @\"\"\n"
+ " @\"\");");
+ verifyFormat("NSString *const kString = @\"\"\n"
+ " @\"\");");
+}
+
} // end namespace
} // end namespace format
} // end namespace clang
Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -799,6 +799,7 @@
// has been implemented.
GoogleStyle.BreakStringLiterals = false;
} else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
GoogleStyle.ColumnLimit = 100;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47225: Add nonnull; use it for atomics
arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM (FYI, Please use a weekly frequency for Pings). Repository: rCXX libc++ https://reviews.llvm.org/D47225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47394: [OpenMP][Clang][NVPTX] Replace bundling with partial linking for the OpenMP NVPTX device offloading toolchain
gtbercea created this revision.
gtbercea added reviewers: Hahnfeld, hfinkel, caomhin, carlo.bertolli, tra.
Herald added subscribers: cfe-commits, guansong.
So far, the clang-offload-bundler has been the default tool for bundling
together various files types produced by the different OpenMP offloading
toolchains supported by Clang. It does a great job for file types such as .bc,
.ll, .ii, .ast. It is also used for bundling object files. Object files are
special, in this case object files which contain sections meant to be executed
on devices other than the host (such is the case of the OpenMP NVPTX
toolchain). The bundling of object files prevents:
- STATIC LINKING: These bundled object files can be part of static libraries
which means that the object file requires an unbundling step. If an object file
in a static library requires "unbundling" then we need to know the whereabouts
of that library and of the files before the actual link step which makes it
impossible to do static linking using the "-L/path/to/lib/folder -labc" flag.
- INTEROPERABILITY WITH OTHER COMPILERS: These bundled object files can end up
being passed between Clang and other compilers which may lead to
incompatibilities: passing a bundled file from Clang to another compiler would
lead to that compiler not being able to unbundle it. Passing an unbundled
object file to Clang and therefore Clang not knowing that it doesn't need to
unbundle it.
**Goal:**
Disable the use of the clang-offload-bundler for bundling/unbundling object
files which contain OpenMP NVPTX device offloaded code. This applies to the
case where the following set of flags are passed to Clang:
-fopenmp -fopenmp-targets=nvptx64-nvidia-cuda
When the above condition is not met the compiler works as it does today by
invoking the clang-offload-bundler for bundling/unbundling object files (at the
cost of static linking and interoperability).
The clang-offload-bundler usage on files other than object files is not
affected by this patch.
**Extensibility**
Although this patch disables bundling/unbundling of object files via the
clang-offload-bundler for the OpenMP NVPTX device offloading toolchain ONLY,
this functionality can be extended to other platforms/system where:
- the device toolchain can produce a host-compatible object AND
- partial linking of host objects is supported.
**The solution:**
The solution enables the OpenMP NVPTX toolchain to produce an object file which
is host-compatible (when compiling with -c). The host-compatible file is
produced using several steps:
Step 1 (already exists): invoke PTXAS on the .s file to obtain a .cubin.
Step 2 (new step): invoke the FATBIN tool (this tool comes with every standard
CUDA installation) which creates a CUDA fatbinary that contains both the PTX
code (the .s file) and the .cubin file. This same tool can wrap the resulting
.fatbin file in a C/C++ wrapper thus creating a .fatbin.c file.
Step 3 (new step): call clang++ on the .fatbin.c file to create a .o file which
is host-compatible.
Once this device side host-compatible file is produced for the NVPTX toolchain
then one further step is needed:
Step 4 (new step): invoke a linker supporting partial linking (currently using
"ld -r") to link host-compatible object file against the original host file and
end up with one single object file which I can now safely pass to another
compiler or include in a static library (new step).
**Passing final object file to clang:**
This file doesn't require unbundling so call to "clang-offload-bundler
--unbundle" is NOT required.
The compiler needs to be notified that the object file contains an "offloaded
device part" by using: "-fopenmp -fopenmp-targets=nvptx64-nvidia-cuda". This
will invoke the OpenMP NVPTX toolchain and it will call only NVLINK on this
file.
**Passing final object file to clang inside a static lib "libabc.a" passed to
clang via: "-L/path/to/lib/folder -labc":**
Call clang with "-fopenmp -fopenmp-targets=nvptx64-nvidia-cuda" to trigger
NVPTX toolchain.
The -L path along with the -labc will be passed to NVLINK which will perform
the "static linking".
Repository:
rC Clang
https://reviews.llvm.org/D47394
Files:
include/clang/Driver/Action.h
include/clang/Driver/Compilation.h
include/clang/Driver/Driver.h
include/clang/Driver/ToolChain.h
lib/Driver/Action.cpp
lib/Driver/Compilation.cpp
lib/Driver/Driver.cpp
lib/Driver/ToolChain.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Driver/ToolChains/Clang.h
lib/Driver/ToolChains/Cuda.cpp
test/Driver/openmp-offload-gpu.c
test/Driver/openmp-offload.c
Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -480,13 +480,13 @@
// Create host object and bundle.
// CHK-BUJOBS: clang{{.*}}" "-cc1" "-triple" "powerpc64le--linux" "-emit-obj" {{.*}}"-fopenmp" {{.*}}"-o" "
// CHK-BUJOBS-SAME: [[HOSTOBJ:[^\\/]+\
[libcxx] r333317 - Fix optional test breakage
Author: jfb
Date: Fri May 25 14:32:27 2018
New Revision: 17
URL: http://llvm.org/viewvc/llvm-project?rev=17&view=rev
Log:
Fix optional test breakage
It seems GCC and clang disagree. Talked to mclow on IRC, disabling for now.
Modified:
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
Modified:
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
URL:
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp?rev=17&r1=16&r2=17&view=diff
==
---
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
(original)
+++
libcxx/trunk/test/std/utilities/optional/optional.object/optional.object.ctor/deduct.pass.cpp
Fri May 25 14:32:27 2018
@@ -43,11 +43,15 @@ int main()
{
// optional(const optional &);
+ // FIXME clang and GCC disagree about this!
+ // clang thinks opt is optional>, GCC thinks it's
optional.
+#if 0
std::optional source('A');
std::optional opt(source);
static_assert(std::is_same_v>>, "");
assert(static_cast(opt) == static_cast(source));
assert(*opt == *source);
+#endif
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47395: [libcxx] [test] Remove nonportable locale assumption in basic.ios.members/narrow.pass.cpp
BillyONeal created this revision.
BillyONeal added reviewers: EricWF, mclow.lists.
I'm not sure if libcxx is asserting UTF-8 here; but on Windows the full char
value is always passed through in its entirety, since the default codepage is
something like Windows-1252. The replacement character is only used for
non-chars there; and that should be a more portable test everywhere.
https://reviews.llvm.org/D47395
Files:
test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
Index:
test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
===
--- test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
+++ test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
@@ -18,7 +18,7 @@
int main()
{
-const std::ios ios(0);
-assert(ios.narrow('c', '*') == 'c');
-assert(ios.narrow('\xFE', '*') == '*');
+const std::wios ios(0);
+assert(ios.narrow(L'c', '*') == 'c');
+assert(ios.narrow(L'\u203C', '*') == '*');
}
Index: test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
===
--- test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
+++ test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
@@ -18,7 +18,7 @@
int main()
{
-const std::ios ios(0);
-assert(ios.narrow('c', '*') == 'c');
-assert(ios.narrow('\xFE', '*') == '*');
+const std::wios ios(0);
+assert(ios.narrow(L'c', '*') == 'c');
+assert(ios.narrow(L'\u203C', '*') == '*');
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference
NoQ added inline comments. Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:265-276 + const llvm::APSInt &from = i->From(), &to = i->To(); + const llvm::APSInt &newFrom = (to.isMinSignedValue() ? + BV.getMaxValue(to) : + (to.isMaxSignedValue() ? + BV.getMinValue(to) : + BV.getValue(- to))); + const llvm::APSInt &newTo = (from.isMinSignedValue() ? baloghadamsoftware wrote: > NoQ wrote: > > baloghadamsoftware wrote: > > > NoQ wrote: > > > > Hmm, wait a minute, is this actually correct? > > > > > > > > For the range [-2³¹, -2³¹ + 1] over a 32-bit integer, the negated range > > > > will be [-2³¹, -2³¹] U [2³¹ - 1, 2³¹ - 1]. > > > > > > > > So there must be a place in the code where we take one range and add > > > > two ranges. > > > The two ends of the range of the type usually stands for +/- infinity. If > > > we add the minimum of the type when negating a negative range, then we > > > lose the whole point of this transformation. > > > > > > Example: If `A - B < 0`, then the range of `A - B` is `[-2³¹, -1]`, If we > > > negate this, and keep the `-2³¹` range end, then we get `[-2³¹, -2³¹]U[1, > > > 2³¹-1]`. However, this does not mean `B - A > 0`. If we make assumption > > > about this, we get two states instead of one, in the true state `A - B`'s > > > range is `[1, 2³¹-1]` and in the false state it is `[-2³¹, -2³¹]`. This > > > is surely not what we want. > > Well, we can't turn math into something we want, it is what it is. > > > > Iterator-related symbols are all planned to be within range [-2²⁹, -2²⁹], > > right? So if we subtract one such symbol from another, it's going to be in > > range [-2³⁰, 2³⁰]. Can we currently infer that? Or maybe we should make the > > iterator checker to enforce that separately? Because this range doesn't > > include -2³¹, so we won't have any problems with doing negation correctly. > > > > So as usual i propose to get this code mathematically correct and then see > > if we can ensure correct behavior by enforcing reasonable constraints on > > our symbols. > I agree that the code should do mathematically correct things, but what I > argue here is what math here means. Computer science is based on math, but it > is somewhat different because of finite ranges and overflows. So I initially > regarded the minimal and maximal values as infinity. Maybe that is not > correct. However, I am sure that negating `-2³¹` should never be `-2³¹`. This > is mathematically incorrect, and renders the whole calculation useless, since > the union of a positive range and a negative range is unsuitable for any > reasoning. I see two options here: > > 1. Remove the extension when negating a range which ends at the maximal value > of the type. So the negated range begins at the minimal value plus one. > However, cut the range which begins at the minimal value of the type by one. > So the negated range ends at the maximal value, as in the current version in > the patch. > > 2. Remove the extension as in 1. and disable the whole negation if we have > the range begins at the minimal value. > > Iterator checkers are of course not affected because of the max/4 constraints. > However, I am sure that negating `-2³¹` should never be `-2³¹`. This is > mathematically incorrect, and renders the whole calculation useless, since > the union of a positive range and a negative range is unsuitable for any > reasoning. Well, that's how computers already work. And that's how all sorts of abstract algebra work as well, so this is totally mathematically correct. We promise to support the [[ https://en.wikipedia.org/wiki/Two's_complement | two's complement ]] semantics in the analyzer when it comes to signed integer overflows. Even though it's technically UB, most implementations follow this semantics and a lot of real-world applications explicitly rely on that. Also we cannot simply drop values from our constraint ranges in the general case because the values we drop might be the only valid values, and the assumption that at least one non-dropped value can definitely be taken is generally incorrect. Finding cornercases like this one is one of the strong sides of any static analysis: it is in fact our job to make the user aware of it if he doesn't understand overflow rules. If it cannot be said that the variable on a certain path is non-negative because it might as well be -2³¹, we should totally explore this possibility. If for a certain checker it brings no benefit because such value would be unlikely in certain circumstances, that checker is free to cut off the respective paths, but the core should perform operations precisely. I don't think we have much room for personal preferences here. https://reviews.llvm.org/D35110 ___
[PATCH] D47394: [OpenMP][Clang][NVPTX] Replace bundling with partial linking for the OpenMP NVPTX device offloading toolchain
gtbercea updated this revision to Diff 148677.
Repository:
rC Clang
https://reviews.llvm.org/D47394
Files:
include/clang/Driver/Action.h
include/clang/Driver/Compilation.h
include/clang/Driver/Driver.h
include/clang/Driver/ToolChain.h
lib/Driver/Action.cpp
lib/Driver/Compilation.cpp
lib/Driver/Driver.cpp
lib/Driver/ToolChain.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Driver/ToolChains/Clang.h
lib/Driver/ToolChains/Cuda.cpp
test/Driver/openmp-offload-gpu.c
test/Driver/openmp-offload.c
Index: test/Driver/openmp-offload.c
===
--- test/Driver/openmp-offload.c
+++ test/Driver/openmp-offload.c
@@ -480,13 +480,13 @@
// Create host object and bundle.
// CHK-BUJOBS: clang{{.*}}" "-cc1" "-triple" "powerpc64le--linux" "-emit-obj" {{.*}}"-fopenmp" {{.*}}"-o" "
// CHK-BUJOBS-SAME: [[HOSTOBJ:[^\\/]+\.o]]" "-x" "ir" "{{.*}}[[HOSTBC]]"
-// CHK-BUJOBS: clang-offload-bundler{{.*}}" "-type=o" "-targets=openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu,host-powerpc64le--linux" "-outputs=
+// CHK-BUJOBS: clang-offload-bundler{{.*}}" "-type=o"{{.*}}"-targets=openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu,host-powerpc64le--linux" "-outputs=
// CHK-BUJOBS-SAME: [[RES:[^\\/]+\.o]]" "-inputs={{.*}}[[T1OBJ]],{{.*}}[[T2OBJ]],{{.*}}[[HOSTOBJ]]"
// CHK-BUJOBS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le--linux" "-S" {{.*}}"-fopenmp" {{.*}}"-o" "
// CHK-BUJOBS-ST-SAME: [[HOSTASM:[^\\/]+\.s]]" "-x" "ir" "{{.*}}[[HOSTBC]]"
// CHK-BUJOBS-ST: clang{{.*}}" "-cc1as" "-triple" "powerpc64le--linux" "-filetype" "obj" {{.*}}"-o" "
// CHK-BUJOBS-ST-SAME: [[HOSTOBJ:[^\\/]+\.o]]" "{{.*}}[[HOSTASM]]"
-// CHK-BUJOBS-ST: clang-offload-bundler{{.*}}" "-type=o" "-targets=openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu,host-powerpc64le--linux" "-outputs=
+// CHK-BUJOBS-ST: clang-offload-bundler{{.*}}" "-type=o"{{.*}}"-targets=openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu,host-powerpc64le--linux" "-outputs=
// CHK-BUJOBS-ST-SAME: [[RES:[^\\/]+\.o]]" "-inputs={{.*}}[[T1OBJ]],{{.*}}[[T2OBJ]],{{.*}}[[HOSTOBJ]]"
/// ###
Index: test/Driver/openmp-offload-gpu.c
===
--- test/Driver/openmp-offload-gpu.c
+++ test/Driver/openmp-offload-gpu.c
@@ -66,24 +66,29 @@
// CHK-PTXAS-CUBIN-BUNDLING: clang{{.*}}" "-o" "[[PTX:.*\.s]]"
// CHK-PTXAS-CUBIN-BUNDLING-NEXT: ptxas{{.*}}" "--output-file" "[[CUBIN:.*\.cubin]]" {{.*}}"[[PTX]]"
-// CHK-PTXAS-CUBIN-BUNDLING: clang-offload-bundler{{.*}}" "-type=o" {{.*}}"-inputs={{.*}}[[CUBIN]]
+// CHK-PTXAS-CUBIN-BUNDLING: fatbinary{{.*}}" "--create=[[FATBIN:.*\.fatbin]]" "
+// CHK-PTXAS-CUBIN-BUNDLING-SAME: --embedded-fatbin=[[FATBINC:.*\.fatbin.c]]" "
+// CHK-PTXAS-CUBIN-BUNDLING-SAME: --cmdline=--compile-only" "--image=profile={{.*}}[[PTX]]" "
+// CHK-PTXAS-CUBIN-BUNDLING-SAME: --image=profile={{.*}}file=[[CUBIN]]" "--cuda" "--device-c"
+// CHK-PTXAS-CUBIN-BUNDLING: clang++{{.*}}" "-c" "-o" "[[HOSTDEV:.*\.o]]"{{.*}}" "[[FATBINC]]" "-D__NV_MODULE_ID=
+// CHK-PTXAS-CUBIN-BUNDLING-NOT: clang-offload-bundler{{.*}}" "-type=o" {{.*}}"-inputs={{.*}}[[CUBIN]]
+// CHK-PTXAS-CUBIN-BUNDLING: ld" "-r" "[[HOSTDEV]]" "{{.*}}.o" "-o" "{{.*}}.o"
/// ###
-/// Check cubin file unbundling and usage by nvlink
+/// Check object file unbundling is not happening when skipping bundler
// RUN: touch %t.o
// RUN: %clang -### -target powerpc64le-unknown-linux-gnu -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda \
// RUN: -no-canonical-prefixes -save-temps %t.o 2>&1 \
// RUN: | FileCheck -check-prefix=CHK-CUBIN-UNBUNDLING-NVLINK %s
-/// Use DAG to ensure that cubin file has been unbundled.
-// CHK-CUBIN-UNBUNDLING-NVLINK-DAG: nvlink{{.*}}" {{.*}}"[[CUBIN:.*\.cubin]]"
-// CHK-CUBIN-UNBUNDLING-NVLINK-DAG: clang-offload-bundler{{.*}}" "-type=o" {{.*}}"-outputs={{.*}}[[CUBIN]]
-// CHK-CUBIN-UNBUNDLING-NVLINK-DAG-SAME: "-unbundle"
+/// Use DAG to ensure that object file has not been unbundled.
+// CHK-CUBIN-UNBUNDLING-NVLINK-DAG: nvlink{{.*}}" {{.*}}"[[OBJ:.*\.o]]"
+// CHK-CUBIN-UNBUNDLING-NVLINK-DAG: ld{{.*}}" {{.*}}"[[OBJ]]"
/// ###
-/// Check cubin file generation and usage by nvlink
+/// Check object file generation is not happening when skipping bundler
// RUN: touch %t1.o
// RUN: touch %t2.o
// RUN: %clang -### -no-canonical-prefixes -target powerpc64le-unknown-linux-gnu -fopenmp=libomp \
@@ -94,7 +99,7 @@
// RUN: -fopenmp-targets=nvptx64-nvidia-cuda %t1.o %t2.o 2>&1 \
// RUN: | FileCheck -check-prefix=CHK-TWOCUBIN %s
-// CHK-TWOCUBIN: nvlink{{.*}}openmp-offload-{{.*}}.cubin" "{{.*}}openmp-offload-{{.*}}.cubin"
+// CHK-TWOCUBIN: nvlink{{.*}}openmp-offload-{{.*}}.o" "{{.*}}o
r333318 - [X86] Mark a few more builtins const that were missed in r331814.
Author: ctopper Date: Fri May 25 15:07:43 2018 New Revision: 18 URL: http://llvm.org/viewvc/llvm-project?rev=18&view=rev Log: [X86] Mark a few more builtins const that were missed in r331814. Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=18&r1=17&r2=18&view=diff == --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original) +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Fri May 25 15:07:43 2018 @@ -1237,9 +1237,9 @@ TARGET_BUILTIN(__builtin_ia32_pmaxsq256, TARGET_BUILTIN(__builtin_ia32_pmaxuq128, "V2LLiV2LLiV2LLi", "nc", "avx512vl") TARGET_BUILTIN(__builtin_ia32_pmaxuq256, "V4LLiV4LLiV4LLi", "nc", "avx512vl") TARGET_BUILTIN(__builtin_ia32_pminsq128, "V2LLiV2LLiV2LLi", "nc", "avx512vl") -TARGET_BUILTIN(__builtin_ia32_pminsq256, "V4LLiV4LLiV4LLi", "n", "avx512vl") -TARGET_BUILTIN(__builtin_ia32_pminuq128, "V2LLiV2LLiV2LLi", "n", "avx512vl") -TARGET_BUILTIN(__builtin_ia32_pminuq256, "V4LLiV4LLiV4LLi", "n", "avx512vl") +TARGET_BUILTIN(__builtin_ia32_pminsq256, "V4LLiV4LLiV4LLi", "nc", "avx512vl") +TARGET_BUILTIN(__builtin_ia32_pminuq128, "V2LLiV2LLiV2LLi", "nc", "avx512vl") +TARGET_BUILTIN(__builtin_ia32_pminuq256, "V4LLiV4LLiV4LLi", "nc", "avx512vl") TARGET_BUILTIN(__builtin_ia32_rndscalepd_128_mask, "V2dV2dIiV2dUc", "nc", "avx512vl") TARGET_BUILTIN(__builtin_ia32_rndscalepd_256_mask, "V4dV4dIiV4dUc", "nc", "avx512vl") TARGET_BUILTIN(__builtin_ia32_rndscaleps_128_mask, "V4fV4fIiV4fUc", "nc", "avx512vl") ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41881: [analyzer] Flag bcmp, bcopy and bzero as obsolete
trixirt added a comment. Herald added a reviewer: george.karpenkov. I need someone to commit this.. Repository: rC Clang https://reviews.llvm.org/D41881 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D43341: [clang-doc] Implement reducer portion of the frontend framework
juliehockett updated this revision to Diff 148678.
juliehockett marked 11 inline comments as done.
juliehockett added a comment.
Reworking the reducer interface a bit to address comments.
https://reviews.llvm.org/D43341
Files:
clang-doc/BitcodeReader.cpp
clang-doc/BitcodeReader.h
clang-doc/BitcodeWriter.cpp
clang-doc/BitcodeWriter.h
clang-doc/CMakeLists.txt
clang-doc/Reducer.cpp
clang-doc/Reducer.h
clang-doc/Representation.cpp
clang-doc/Representation.h
clang-doc/tool/ClangDocMain.cpp
docs/ReleaseNotes.rst
test/clang-doc/bc-comment.cpp
test/clang-doc/bc-namespace.cpp
test/clang-doc/bc-record.cpp
Index: test/clang-doc/bc-record.cpp
===
--- /dev/null
+++ test/clang-doc/bc-record.cpp
@@ -0,0 +1,254 @@
+// This test requires Linux due to the system-dependent USR for the
+// inner class in function H.
+// REQUIRES: system-linux
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump-intermediate -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/bc/ACE81AFA6627B4CEF2B456FB6E1252925674AF7E.bc --dump | FileCheck %s --check-prefix CHECK-A
+// RUN: llvm-bcanalyzer %t/docs/bc/FC07BD34D5E77782C263FA97929EA8753740.bc --dump | FileCheck %s --check-prefix CHECK-B
+// RUN: llvm-bcanalyzer %t/docs/bc/1E3438A08BA22025C0B46289FF0686F92C8924C5.bc --dump | FileCheck %s --check-prefix CHECK-BC
+// RUN: llvm-bcanalyzer %t/docs/bc/06B5F6A19BA9F6A832E127C9968282B94619B210.bc --dump | FileCheck %s --check-prefix CHECK-C
+// RUN: llvm-bcanalyzer %t/docs/bc/0921737541208B8FA9BB42B60F78AC1D779AA054.bc --dump | FileCheck %s --check-prefix CHECK-D
+// RUN: llvm-bcanalyzer %t/docs/bc/289584A8E0FF4178A794622A547AA622503967A1.bc --dump | FileCheck %s --check-prefix CHECK-E
+// RUN: llvm-bcanalyzer %t/docs/bc/DEB4AC1CD9253CD9EF7FBE6BCAC506D77984ABD4.bc --dump | FileCheck %s --check-prefix CHECK-ECON
+// RUN: llvm-bcanalyzer %t/docs/bc/BD2BDEBD423F80BACCEA75DE6D6622D355FC2D17.bc --dump | FileCheck %s --check-prefix CHECK-EDES
+// RUN: llvm-bcanalyzer %t/docs/bc/E3B54702FABFF4037025BA194FC27C47006330B5.bc --dump | FileCheck %s --check-prefix CHECK-F
+// RUN: llvm-bcanalyzer %t/docs/bc/B6AC4C5C9F2EA3F2B3ECE1A33D349F4EE502B24E.bc --dump | FileCheck %s --check-prefix CHECK-H
+// RUN: llvm-bcanalyzer %t/docs/bc/6BA1EE2B3DAEACF6E4306F10AF44908F4807927C.bc --dump | FileCheck %s --check-prefix CHECK-I
+// RUN: llvm-bcanalyzer %t/docs/bc/5093D428CDC62096A67547BA52566E4FB9404EEE.bc --dump | FileCheck %s --check-prefix CHECK-PM
+// RUN: llvm-bcanalyzer %t/docs/bc/CA7C7935730B5EACD25F080E9C83FA087CCDC75E.bc --dump | FileCheck %s --check-prefix CHECK-X
+// RUN: llvm-bcanalyzer %t/docs/bc/641AB4A3D36399954ACDE29C7A8833032BF40472.bc --dump | FileCheck %s --check-prefix CHECK-Y
+
+void H() {
+ class I {};
+}
+// CHECK-H:
+ // CHECK-H-NEXT:
+ // CHECK-H-NEXT: blob data = 'H'
+ // CHECK-H-NEXT: blob data = '{{.*}}'
+ // CHECK-H-NEXT:
+// CHECK-H-NEXT:
+ // CHECK-H-NEXT: blob data = 'void'
+ // CHECK-H-NEXT:
+// CHECK-H-NEXT:
+ // CHECK-H-NEXT:
+// CHECK-H-NEXT:
+
+
+// CHECK-I:
+ // CHECK-I-NEXT:
+ // CHECK-I-NEXT: blob data = 'I'
+ // CHECK-I-NEXT:
+// CHECK-I-NEXT:
+// CHECK-I-NEXT: blob data = 'H'
+// CHECK-I-NEXT:
+// CHECK-I-NEXT:
+ // CHECK-I-NEXT:
+ // CHECK-I-NEXT: blob data = '{{.*}}'
+ // CHECK-I-NEXT:
+// CHECK-I-NEXT:
+
+union A { int X; int Y; };
+// CHECK-A:
+ // CHECK-A-NEXT:
+ // CHECK-A-NEXT: blob data = 'A'
+ // CHECK-A-NEXT: blob data = '{{.*}}'
+ // CHECK-A-NEXT:
+ // CHECK-A-NEXT:
+// CHECK-A-NEXT:
+ // CHECK-A-NEXT: blob data = 'int'
+ // CHECK-A-NEXT:
+// CHECK-A-NEXT:
+// CHECK-A-NEXT: blob data = 'X'
+// CHECK-A-NEXT:
+ // CHECK-A-NEXT:
+ // CHECK-A-NEXT:
+// CHECK-A-NEXT:
+ // CHECK-A-NEXT: blob data = 'int'
+ // CHECK-A-NEXT:
+// CHECK-A-NEXT:
+// CHECK-A-NEXT: blob data = 'Y'
+// CHECK-A-NEXT:
+ // CHECK-A-NEXT:
+// CHECK-A-NEXT:
+
+enum B { X, Y };
+// CHECK-B:
+ // CHECK-B-NEXT:
+ // CHECK-B-NEXT: blob data = 'B'
+ // CHECK-B-NEXT: blob data = '{{.*}}'
+ // CHECK-B-NEXT: blob data = 'X'
+ // CHECK-B-NEXT: blob data = 'Y'
+// CHECK-B-NEXT:
+
+enum class Bc { A, B };
+// CHECK-BC:
+ // CHECK-BC-NEXT:
+ // CHECK-BC-NEXT: blob data = 'Bc'
+ // CHECK-BC-NEXT: blob data = '{{.*}}'
+ // CHECK-BC-NEXT:
+ // CHECK-BC-NEXT: blob data = 'A'
+ // CHECK-BC-NEXT: blob data = 'B'
+// CHECK-BC-NEXT:
+
+struct C { int i; };
+// CHECK-C:
+ // CHECK-C-NEXT:
+ // CHECK-C-NEXT: blob data = 'C'
+ // CHECK-C-NEXT: blob data = '{{.*}}'
+ // CHECK-C-NEXT:
+// CHECK-C-NEXT:
+ // CHECK-C-NEXT: blob data = 'int'
+ // CHECK-C-NEXT:
+// CHECK-C-NEXT:
+// CHECK-C-NEXT: blob data = 'i'
+// CHECK-C-NEXT:
+ // CHECK-C-NEXT:
+// CHECK-C-NEXT:
+
+
