Re: [PATCH] [ms-cxxabi] Fix the calling convention for operator new in records
Would it make sense to define Declarator::isMember(), which checks `getContext() == MemberContext DS.getStorageClassSpec() != DeclSpec::SCS_typedef !DS.isFriendSpecified()` or something like that? Also, should isStaticMember() check `DS.getStorageClassSpec() != DeclSpec::SCS_typedef !DS.isFriendSpecified()`? Sorry for going through so many review cycles on this; it's sort of confusing stuff, so I want it to be as easy to read as possible. http://llvm-reviews.chandlerc.com/D1761 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r191719 - Tweak changes in r186464 to avoid a crash.
Author: efriedma Date: Mon Sep 30 19:28:29 2013 New Revision: 191719 URL: http://llvm.org/viewvc/llvm-project?rev=191719view=rev Log: Tweak changes in r186464 to avoid a crash. Currently, IR generation can't handle file-scope compound literals with non-constant initializers in C++. Fixes PR17415 (the first crash in the bug). (We should probably change (T){1,2,3} to use the same codepath as T{1,2,3} in C++ eventually, given that the semantics of the latter are actually defined by the standard.) Modified: cfe/trunk/lib/AST/Expr.cpp cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/test/SemaCXX/compound-literal.cpp Modified: cfe/trunk/lib/AST/Expr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=191719r1=191718r2=191719view=diff == --- cfe/trunk/lib/AST/Expr.cpp (original) +++ cfe/trunk/lib/AST/Expr.cpp Mon Sep 30 19:28:29 2013 @@ -2654,9 +2654,6 @@ bool Expr::isConstantInitializer(ASTCont return Exp-isConstantInitializer(Ctx, false); } case InitListExprClass: { -// FIXME: This doesn't deal with fields with reference types correctly. -// FIXME: This incorrectly allows pointers cast to integers to be assigned -// to bitfields. const InitListExpr *ILE = castInitListExpr(this); if (ILE-getType()-isArrayType()) { unsigned numInits = ILE-getNumInits(); Modified: cfe/trunk/lib/Sema/SemaExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=191719r1=191718r2=191719view=diff == --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) +++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Sep 30 19:28:29 2013 @@ -4717,7 +4717,10 @@ Sema::BuildCompoundLiteralExpr(SourceLoc LiteralExpr = Result.get(); bool isFileScope = getCurFunctionOrMethodDecl() == 0; - if (!getLangOpts().CPlusPlus isFileScope) { // 6.5.2.5p3 + if (isFileScope + !LiteralExpr-isTypeDependent() + !LiteralExpr-isValueDependent() + !literalType-isDependentType()) { // 6.5.2.5p3 if (CheckForConstantInitializer(LiteralExpr, literalType)) return ExprError(); } Modified: cfe/trunk/test/SemaCXX/compound-literal.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/compound-literal.cpp?rev=191719r1=191718r2=191719view=diff == --- cfe/trunk/test/SemaCXX/compound-literal.cpp (original) +++ cfe/trunk/test/SemaCXX/compound-literal.cpp Mon Sep 30 19:28:29 2013 @@ -76,3 +76,13 @@ namespace brace_initializers { (void)(PrivateDtor){1, 2}; // expected-error {{temporary of type 'brace_initializers::PrivateDtor' has private destructor}} } } + +// This doesn't necessarily need to be an error, but CodeGen can't handle it +// at the moment. +int PR17415 = (int){PR17415}; // expected-error {{initializer element is not a compile-time constant}} + +// Make sure we accept this. (Not sure if we actually should... but we do +// at the moment.) +templateunsigned struct Value { }; +templatetypename T +int check_narrowed(Valuesizeof((T){1.1})); ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r191726 - Fix typo correction usage of SemaAccess.cpp.
Author: efriedma Date: Mon Sep 30 21:44:48 2013 New Revision: 191726 URL: http://llvm.org/viewvc/llvm-project?rev=191726view=rev Log: Fix typo correction usage of SemaAccess.cpp. When we check access for lookup results, make sure we propagate the result's access to the access control APIs; this can be different from the natural access of the declaration depending on the path used by the lookup. PR17394. Modified: cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Sema/SemaAccess.cpp cfe/trunk/lib/Sema/SemaLookup.cpp cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=191726r1=191725r2=191726view=diff == --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Mon Sep 30 21:44:48 2013 @@ -4896,7 +4896,7 @@ public: AccessResult CheckFriendAccess(NamedDecl *D); AccessResult CheckMemberAccess(SourceLocation UseLoc, CXXRecordDecl *NamingClass, - NamedDecl *D); + DeclAccessPair Found); AccessResult CheckMemberOperatorAccess(SourceLocation Loc, Expr *ObjectExpr, Expr *ArgExpr, Modified: cfe/trunk/lib/Sema/SemaAccess.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaAccess.cpp?rev=191726r1=191725r2=191726view=diff == --- cfe/trunk/lib/Sema/SemaAccess.cpp (original) +++ cfe/trunk/lib/Sema/SemaAccess.cpp Mon Sep 30 21:44:48 2013 @@ -1390,8 +1390,6 @@ static AccessResult IsAccessible(Sema S CXXBasePath *Path = FindBestPath(S, EC, Entity, FinalAccess, Paths); if (!Path) return AR_dependent; - if (Path-Access == AS_none) // This can happen during typo correction. -return AR_inaccessible; assert(Path-Access = UnprivilegedAccess access along best path worse than direct?); @@ -1716,14 +1714,14 @@ Sema::AccessResult Sema::CheckAllocation /// \brief Checks access to a member. Sema::AccessResult Sema::CheckMemberAccess(SourceLocation UseLoc, CXXRecordDecl *NamingClass, - NamedDecl *D) { + DeclAccessPair Found) { if (!getLangOpts().AccessControl || !NamingClass || - D-getAccess() == AS_public) + Found.getAccess() == AS_public) return AR_accessible; AccessTarget Entity(Context, AccessTarget::Member, NamingClass, - DeclAccessPair::make(D, D-getAccess()), QualType()); + Found, QualType()); return CheckAccess(*this, UseLoc, Entity); } Modified: cfe/trunk/lib/Sema/SemaLookup.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=191726r1=191725r2=191726view=diff == --- cfe/trunk/lib/Sema/SemaLookup.cpp (original) +++ cfe/trunk/lib/Sema/SemaLookup.cpp Mon Sep 30 21:44:48 2013 @@ -4409,7 +4409,7 @@ retry_lookup: TRD != TRDEnd; ++TRD) { if (CheckMemberAccess(TC.getCorrectionRange().getBegin(), NSType ? NSType-getAsCXXRecordDecl() : 0, -*TRD) == AR_accessible) +TRD.getPair()) == AR_accessible) TC.addCorrectionDecl(*TRD); } if (TC.isResolved()) Modified: cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp?rev=191726r1=191725r2=191726view=diff == --- cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp (original) +++ cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp Mon Sep 30 21:44:48 2013 @@ -135,3 +135,12 @@ void test() { req.set_check(false); // expected-error-re {{use of undeclared identifier 'req'$}} } } + +namespace PR17394 { + class A { + protected: +long zz; + }; + class B : private A {}; + B zy; // expected-error {{expected ';' after top level declarator}}{} +} ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Implements Parts of Intrin.h
The #pragma clang system_header shouldn't be necessary. http://llvm-reviews.chandlerc.com/D1766 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Implements Parts of Intrin.h
On Thu, Sep 26, 2013 at 6:10 PM, Reid Kleckner r...@google.com wrote: Because it's already considered a system header, or because it shouldn't have warnings? Because it should already be considered a system header. -Eli The header does have warnings, because other system headers provide prototypes for these intrinsics, and they don't apply static, inline, or __forceinline / always_inline. We may have only ween warnings as a result of including this file directly. On Thu, Sep 26, 2013 at 6:01 PM, Eli Friedman eli.fried...@gmail.comwrote: The #pragma clang system_header shouldn't be necessary. http://llvm-reviews.chandlerc.com/D1766 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [patch] Produce an error on unknown -f options
On Wed, Sep 25, 2013 at 9:03 AM, Rafael Espíndola rafael.espind...@gmail.com wrote: The attached patch changes the driver to produce an error on unknown -f options. This matches gcc's behavior. LGTM. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] [ms-cxxabi] Fix the calling convention for operator new in records
Please refactor the declarator declares a static member check so the code is only in one place. -Eli On Wed, Sep 25, 2013 at 4:53 PM, Reid Kleckner r...@google.com wrote: Hi rsmith, Operator new, new[], delete, and delete[] are all implicitly static when declared inside a record. CXXMethodDecl already knows this, but we need to account for that before we pick the calling convention for the function type. Fixes PR17371. http://llvm-reviews.chandlerc.com/D1761 Files: include/clang/AST/DeclCXX.h include/clang/Sema/Sema.h lib/AST/DeclCXX.cpp lib/AST/MicrosoftMangle.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaType.cpp test/CodeGenCXX/mangle-ms.cpp ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: Re: Fix for ICE in clang due to infinite loop.
On Tue, Sep 24, 2013 at 11:48 PM, MAYUR PANDEY mayu...@samsung.com wrote: This is the similar way it has been fixed in gcc. They have kept a note of parent and they break the loop when it is again noticed. With the usage of global map there are no performance issues as such. Also please suggest the alternate way of fixing this issue. I'm not an expert on overload resolution rules, so if you're certain we actually need a map to catch this case, that's fine, I guess. (Richard, can you chime in?) The rest of my feedback still applies. -Eli --- *Original Message* --- *Sender* : Eli Friedmaneli.fried...@gmail.com *Date* : Sep 10, 2013 06:53 (GMT+09:00) *Title* : Re: Fix for ICE in clang due to infinite loop. On Mon, Sep 9, 2013 at 5:08 AM, MAYUR PANDEY mayu...@samsung.com wrote: Hi, Attached is the fix for Compiler crash caused by infinite loop when we try to compile the following code. struct A; struct B { B (A const ); B (B ); }; struct A { A (B); }; B f (B const b) { return b; } ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: r191394 - Produce an error for unknown -f options.
On Wed, Sep 25, 2013 at 5:42 PM, Nick Lewycky nlewy...@google.com wrote: On 25 September 2013 12:07, Rafael Espindola rafael.espind...@gmail.comwrote: Author: rafael Date: Wed Sep 25 14:07:08 2013 New Revision: 191394 URL: http://llvm.org/viewvc/llvm-project?rev=191394view=rev Log: Produce an error for unknown -f options. Err, we aren't ready for this. If we're going to do this, could you stage it such that the gcc flags are added but ignored, then we error on unknown options? This is already how it works. If our list of ignored gcc -f flags isn't complete enough, that's simple to fix. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] [ms-cxxabi] Fix the calling convention for operator new in records
Comment at: lib/Sema/SemaDecl.cpp:6497 @@ -6497,1 +6496,3 @@ + } + FunctionTemplateDecl *FunctionTemplate = 0; Did you mean to remove the other code which sets isFriend? Comment at: lib/Sema/SemaType.cpp:2452 @@ -2449,2 +2451,3 @@ IsCXXInstanceMethod = (D.getContext() == Declarator::MemberContext + DS.getStorageClassSpec() != DeclSpec::SCS_typedef The MemberContext check is redundant. Comment at: include/clang/Basic/OperatorKinds.h:34-44 @@ +33,13 @@ + +/// Returns true if the given operator is implicitly static in a record +/// context. +inline bool isImplicitlyStaticOperator(OverloadedOperatorKind OOK) { + // [class.free]p1: + // Any allocation function for a class T is a static member + // (even if not explicitly declared static). + // [class.free]p6 Any deallocation function for a class X is a static member + // (even if not explicitly declared static). + return OOK == OO_New || OOK == OO_Array_New || OOK == OO_Delete || + OOK == OO_Array_Delete; +} + Reid Kleckner wrote: Doing this here seems wrong. Any better ideas? I didn't want to include DeclCXX.h from DeclSpec.h. You could sink the isStaticMember() implementation into DeclSpec.cpp. http://llvm-reviews.chandlerc.com/D1761 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [patch] Produce an error for -munknown-to-clang-option
On Tue, Sep 24, 2013 at 6:46 AM, Rafael Espíndola rafael.espind...@gmail.com wrote: The attached patch makes the driver produce an error for unknown -m options. Since the --machine= alias was already broken, I just reported llvm.org/pr17342 and removed it in this patch. LGTM. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Gracefully (and correctly) handle init of multiple union members
On Tue, Sep 24, 2013 at 8:04 AM, Matthew Curtis mcur...@codeaurora.orgwrote: We now emit warnings when doing so and code generation is consistent with GCC. Note that the C99 spec is unclear as to the precise behavior. See also ... Bug: http://llvm.org/bugs/show_bug.**cgi?id=16644http://llvm.org/bugs/show_bug.cgi?id=16644and cfe-dev discussion: http://lists.cs.uiuc.edu/**pipermail/cfe-dev/2013-**September/031918.htmlhttp://lists.cs.uiuc.edu/pipermail/cfe-dev/2013-September/031918.html +FieldDecl *CFD = ArrayFillerOrUnionFieldInit.dyn_castFieldDecl *(); +assert((FD == 0 || CFD == 0 || CFD == FD) +Only one field of a union may be initiazed at a time!); This will cause an unused variable warning in release builds. Also, spelling. diff --git a/test/Sema/designated-initializers.c b/test/Sema/designated-initializers.c index 36fa559..be365a0 100644 --- a/test/Sema/designated-initializers.c +++ b/test/Sema/designated-initializers.c @@ -1,5 +1,8 @@ // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-unknown-unknown %s +// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -DCHECK_CODEGEN=1 \ +// RUN: -S -emit-llvm -o - 21 | FileCheck %s Please put a separate code generation test into test/CodeGen. It's okay if you duplicate the test code. The approach looks fine. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] [OPENMP] Bug fixes and improvements, part 2
On Mon, Sep 23, 2013 at 9:01 PM, Alexey Bataev a.bat...@hotmail.com wrote: Hi doug.gregor, eli.friedman, gribozavr, cbergstrom, hfinkel, pan, Improved variable lookup procedure for threadprivate variables. http://llvm-reviews.chandlerc.com/D1746 Files: test/OpenMP/threadprivate_ast_print.cpp test/OpenMP/threadprivate_messages.cpp lib/Sema/SemaOpenMP.cpp lib/AST/DeclPrinter.cpp Index: test/OpenMP/threadprivate_ast_print.cpp === --- test/OpenMP/threadprivate_ast_print.cpp +++ test/OpenMP/threadprivate_ast_print.cpp @@ -15,7 +15,7 @@ static int b; // CHECK: static int b; #pragma omp threadprivate(b) -// CHECK-NEXT: #pragma omp threadprivate(b) +// CHECK-NEXT: #pragma omp threadprivate(St1::b) } d; int a, b; @@ -38,6 +38,15 @@ //CHECK-NEXT: static T v; //CHECK-NEXT: #pragma omp threadprivate(v) +namespace ns{ + int a; +} +// CHECK: namespace ns { +// CHECK-NEXT: int a; +// CHECK-NEXT: } +#pragma omp threadprivate(ns::a) +// CHECK-NEXT: #pragma omp threadprivate(ns::a) + int main () { static int a; // CHECK: static int a; Index: test/OpenMP/threadprivate_messages.cpp === --- test/OpenMP/threadprivate_messages.cpp +++ test/OpenMP/threadprivate_messages.cpp @@ -60,12 +60,12 @@ #pragma omp threadprivate (g) namespace ns { - int m; // expected-note 2 {{'m' defined here}} + int m; #pragma omp threadprivate (m) } #pragma omp threadprivate (m) // expected-error {{use of undeclared identifier 'm'}} -#pragma omp threadprivate (ns::m) // expected-error {{'#pragma omp threadprivate' must appear in the scope of the 'ns::m' variable declaration}} -#pragma omp threadprivate (ns:m) // expected-error {{unexpected ':' in nested name specifier; did you mean '::'?}} expected-error {{'#pragma omp threadprivate' must appear in the scope of the 'ns::m' variable declaration}} +#pragma omp threadprivate (ns::m) // expected-error {{'#pragma omp threadprivate' must precede all references to variable 'ns::m'}} +#pragma omp threadprivate (ns:m) // expected-error {{unexpected ':' in nested name specifier; did you mean '::'?}} expected-error {{'#pragma omp threadprivate' must precede all references to variable 'ns::m'}} const int h = 12; const volatile int i = 10; Index: lib/Sema/SemaOpenMP.cpp === --- lib/Sema/SemaOpenMP.cpp +++ lib/Sema/SemaOpenMP.cpp @@ -428,7 +428,9 @@ // A threadprivate directive for static block-scope variables must appear // in the scope of the variable and not in a nested scope. NamedDecl *ND = castNamedDecl(VD); - if (!isDeclInScope(ND, getCurLexicalContext(), CurScope)) { + if ((!getCurLexicalContext()-isFileContext() || + !VD-getDeclContext()-isFileContext()) + !isDeclInScope(ND, getCurLexicalContext(), CurScope)) { Diag(Id.getLoc(), diag::err_omp_var_scope) getOpenMPDirectiveName(OMPD_threadprivate) VD; bool IsDecl = VD-isThisDeclarationADefinition(Context) == The comments before this check list four distinct rules for different kinds of variables. Please implement them explicitly. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r191339 - Fix -Wmissing-variable-declarations regression.
Author: efriedma Date: Tue Sep 24 18:10:08 2013 New Revision: 191339 URL: http://llvm.org/viewvc/llvm-project?rev=191339view=rev Log: Fix -Wmissing-variable-declarations regression. This issue was introduced in r181677. PR17349. Modified: cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/test/SemaCXX/warn-missing-variable-declarations.cpp Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=191339r1=191338r2=191339view=diff == --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Sep 24 18:10:08 2013 @@ -8683,7 +8683,7 @@ void Sema::CheckCompleteVariableDeclarat } if (var-isThisDeclarationADefinition() - var-isExternallyVisible() + var-isExternallyVisible() var-hasLinkage() getDiagnostics().getDiagnosticLevel( diag::warn_missing_variable_declarations, var-getLocation())) { Modified: cfe/trunk/test/SemaCXX/warn-missing-variable-declarations.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-missing-variable-declarations.cpp?rev=191339r1=191338r2=191339view=diff == --- cfe/trunk/test/SemaCXX/warn-missing-variable-declarations.cpp (original) +++ cfe/trunk/test/SemaCXX/warn-missing-variable-declarations.cpp Tue Sep 24 18:10:08 2013 @@ -41,3 +41,9 @@ int CGood1::MGood1; namespace { int mgood4; } + +class C { + void test() { +static int x = 0; // no-warn + } +}; ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r191340 - Allow dynamic_cast to void* even with -fno-rtti.
Author: efriedma Date: Tue Sep 24 18:21:41 2013 New Revision: 191340 URL: http://llvm.org/viewvc/llvm-project?rev=191340view=rev Log: Allow dynamic_cast to void* even with -fno-rtti. PR17346. Modified: cfe/trunk/lib/Sema/SemaCast.cpp cfe/trunk/test/SemaCXX/no-rtti.cpp Modified: cfe/trunk/lib/Sema/SemaCast.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=191340r1=191339r2=191340view=diff == --- cfe/trunk/lib/Sema/SemaCast.cpp (original) +++ cfe/trunk/lib/Sema/SemaCast.cpp Tue Sep 24 18:21:41 2013 @@ -669,8 +669,10 @@ void CastOperation::CheckDynamicCast() { Self.MarkVTableUsed(OpRange.getBegin(), castCXXRecordDecl(SrcRecord-getDecl())); - // dynamic_cast is not available with fno-rtti - if (!Self.getLangOpts().RTTI) { + // dynamic_cast is not available with -fno-rtti. + // As an exception, dynamic_cast to void* is available because it doesn't + // use RTTI. + if (!Self.getLangOpts().RTTI !DestPointee-isVoidType()) { Self.Diag(OpRange.getBegin(), diag::err_no_dynamic_cast_with_fno_rtti); SrcExpr = ExprError(); return; Modified: cfe/trunk/test/SemaCXX/no-rtti.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/no-rtti.cpp?rev=191340r1=191339r2=191340view=diff == --- cfe/trunk/test/SemaCXX/no-rtti.cpp (original) +++ cfe/trunk/test/SemaCXX/no-rtti.cpp Tue Sep 24 18:21:41 2013 @@ -22,3 +22,8 @@ struct B : public A { bool isa_B(A *a) { return dynamic_castB *(a) != 0; // expected-error {{cannot use dynamic_cast with -fno-rtti}} } + +void* getMostDerived(A* a) { + // This cast does not use RTTI. + return dynamic_castvoid *(a); +} ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Gracefully (and correctly) handle init of multiple union members
One more thing I didn't catch the first time: please add a test for something like the following: typedef union { struct { int zero; int one; int two; int three; } a; int b[4]; } union_16644_t; union_16644_t x[2] = { [0].a.one = 2, [1].a.zero = 3, [0].a.zero = 5 }; -Eli On Tue, Sep 24, 2013 at 2:06 PM, Matthew Curtis mcur...@codeaurora.orgwrote: New patch incorporating Eli's feedback. Thanks Eli! Matthew Curtis On 9/24/2013 2:07 PM, Eli Friedman wrote: On Tue, Sep 24, 2013 at 8:04 AM, Matthew Curtis mcur...@codeaurora.orgwrote: We now emit warnings when doing so and code generation is consistent with GCC. Note that the C99 spec is unclear as to the precise behavior. See also ... Bug: http://llvm.org/bugs/show_bug.cgi?id=16644 and cfe-dev discussion: http://lists.cs.uiuc.edu/pipermail/cfe-dev/2013-September/031918.html +FieldDecl *CFD = ArrayFillerOrUnionFieldInit.dyn_castFieldDecl *(); +assert((FD == 0 || CFD == 0 || CFD == FD) +Only one field of a union may be initiazed at a time!); This will cause an unused variable warning in release builds. Also, spelling. diff --git a/test/Sema/designated-initializers.c b/test/Sema/designated-initializers.c index 36fa559..be365a0 100644 --- a/test/Sema/designated-initializers.c +++ b/test/Sema/designated-initializers.c @@ -1,5 +1,8 @@ // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-unknown-unknown %s +// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -DCHECK_CODEGEN=1 \ +// RUN: -S -emit-llvm -o - 21 | FileCheck %s Please put a separate code generation test into test/CodeGen. It's okay if you duplicate the test code. The approach looks fine. -Eli -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: r191150 - PR17295: Do not allow explicit conversion functions to be used in cases where
On Sat, Sep 21, 2013 at 2:55 PM, Richard Smith richard-l...@metafoo.co.ukwrote: Author: rsmith Date: Sat Sep 21 16:55:46 2013 New Revision: 191150 URL: http://llvm.org/viewvc/llvm-project?rev=191150view=rev Log: PR17295: Do not allow explicit conversion functions to be used in cases where an additional conversion (other than a qualification conversion) would be required after the explicit conversion. Conversely, do allow explicit conversion functions to be used when initializing a temporary for a reference binding in direct-list-initialization. Modified: cfe/trunk/include/clang/Sema/Initialization.h cfe/trunk/lib/Sema/SemaInit.cpp cfe/trunk/lib/Sema/SemaOverload.cpp cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp cfe/trunk/test/SemaCXX/explicit.cpp Modified: cfe/trunk/include/clang/Sema/Initialization.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Initialization.h?rev=191150r1=191149r2=191150view=diff == --- cfe/trunk/include/clang/Sema/Initialization.h (original) +++ cfe/trunk/include/clang/Sema/Initialization.h Sat Sep 21 16:55:46 2013 @@ -853,6 +853,10 @@ public: const InitializationKind Kind, MultiExprArg Args, bool InInitList = false); + void InitializeFrom(Sema S, const InitializedEntity Entity, + const InitializationKind Kind, MultiExprArg Args, + bool InInitList); + ~InitializationSequence(); /// \brief Perform the actual initialization of the given entity based on Modified: cfe/trunk/lib/Sema/SemaInit.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=191150r1=191149r2=191150view=diff == --- cfe/trunk/lib/Sema/SemaInit.cpp (original) +++ cfe/trunk/lib/Sema/SemaInit.cpp Sat Sep 21 16:55:46 2013 @@ -3346,6 +3346,33 @@ static void TryListInitialization(Sema return; } } + if (S.getLangOpts().CPlusPlus !DestType-isAggregateType() + InitList-getNumInits() == 1 + InitList-getInit(0)-getType()-isRecordType()) { +// - Otherwise, if the initializer list has a single element of type E +// [...references are handled above...], the object or reference is +// initialized from that element; if a narrowing conversion is required +// to convert the element to T, the program is ill-formed. +// +// Per core-24034, this is direct-initialization if we were performing +// direct-list-initialization and copy-initialization otherwise. +// We can't use InitListChecker for this, because it always performs +// copy-initialization. This only matters if we might use an 'explicit' +// conversion operator, so we only need to handle the cases where the source +// is of record type. +InitializationKind SubKind = +Kind.getKind() == InitializationKind::IK_DirectList +? InitializationKind::CreateDirect(Kind.getLocation(), + InitList-getLBraceLoc(), + InitList-getRBraceLoc()) +: Kind; +Expr *SubInit[1] = { InitList-getInit(0) }; +Sequence.InitializeFrom(S, Entity, SubKind, SubInit, +/*TopLevelOfInitList*/true); +if (Sequence) + Sequence.RewrapReferenceInitList(Entity.getType(), InitList); +return; + } InitListChecker CheckInitList(S, Entity, InitList, DestType, /*VerifyOnly=*/true); @@ -4366,6 +4393,14 @@ InitializationSequence::InitializationSe MultiExprArg Args, bool TopLevelOfInitList) : FailedCandidateSet(Kind.getLocation()) { + InitializeFrom(S, Entity, Kind, Args, TopLevelOfInitList); +} + +void InitializationSequence::InitializeFrom(Sema S, +const InitializedEntity Entity, +const InitializationKind Kind, +MultiExprArg Args, +bool TopLevelOfInitList) { ASTContext Context = S.Context; // Eliminate non-overload placeholder types in the arguments. We Modified: cfe/trunk/lib/Sema/SemaOverload.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=191150r1=191149r2=191150view=diff == --- cfe/trunk/lib/Sema/SemaOverload.cpp (original) +++ cfe/trunk/lib/Sema/SemaOverload.cpp Sat Sep 21 16:55:46 2013 @@ -5829,6 +5829,17 @@ Sema::AddConversionCandidate(CXXConversi ConvType =
Re: [patch] Fix pr9701
On Mon, Sep 23, 2013 at 2:17 PM, Rafael Espíndola rafael.espind...@gmail.com wrote: On 5 September 2013 14:27, Rafael Espíndola rafael.espind...@gmail.com wrote: On 3 September 2013 21:11, Richard Smith rich...@metafoo.co.uk wrote: What about -munknown-to-clang-option? We really want an error on those ones, since they can change the ABI. The behavior of -funknown-to-clang-option, -Wunknown-to-clang-option and -munknown-to-clang-option is unchanged (we just warn). I think we should be more strict about these too, but since they are the most common options I would like to change them in a follow up patch. Ping. Please add tests for the current behavior anyway, so it's clear what the patch does and does not do. Otherwise LGTM. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r191243 - Add _mm_stream_si64 intrinsic.
Author: efriedma Date: Mon Sep 23 18:38:39 2013 New Revision: 191243 URL: http://llvm.org/viewvc/llvm-project?rev=191243view=rev Log: Add _mm_stream_si64 intrinsic. While I'm here, also fix the alignment computation for the whole family of intrinsics. PR17298. Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def cfe/trunk/lib/CodeGen/CGBuiltin.cpp cfe/trunk/lib/Headers/emmintrin.h cfe/trunk/test/CodeGen/builtins-x86.c cfe/trunk/test/CodeGen/sse-builtins.c Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=191243r1=191242r2=191243view=diff == --- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original) +++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Mon Sep 23 18:38:39 2013 @@ -258,6 +258,7 @@ BUILTIN(__builtin_ia32_storeupd, vd*V2d BUILTIN(__builtin_ia32_movmskpd, iV2d, ) BUILTIN(__builtin_ia32_pmovmskb128, iV16c, ) BUILTIN(__builtin_ia32_movnti, vi*i, ) +BUILTIN(__builtin_ia32_movnti64, vLLi*LLi, ) BUILTIN(__builtin_ia32_movntpd, vd*V2d, ) BUILTIN(__builtin_ia32_movntdq, vV2LLi*V2LLi, ) BUILTIN(__builtin_ia32_psadbw128, V2LLiV16cV16c, ) Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=191243r1=191242r2=191243view=diff == --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original) +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Mon Sep 23 18:38:39 2013 @@ -3249,7 +3249,8 @@ Value *CodeGenFunction::EmitX86BuiltinEx case X86::BI__builtin_ia32_movntpd256: case X86::BI__builtin_ia32_movntdq: case X86::BI__builtin_ia32_movntdq256: - case X86::BI__builtin_ia32_movnti: { + case X86::BI__builtin_ia32_movnti: + case X86::BI__builtin_ia32_movnti64: { llvm::MDNode *Node = llvm::MDNode::get(getLLVMContext(), Builder.getInt32(1)); @@ -3259,7 +3260,16 @@ Value *CodeGenFunction::EmitX86BuiltinEx cast); StoreInst *SI = Builder.CreateStore(Ops[1], BC); SI-setMetadata(CGM.getModule().getMDKindID(nontemporal), Node); -SI-setAlignment(16); + +// If the operand is an integer, we can't assume alignment. Otherwise, +// assume natural alignment. +QualType ArgTy = E-getArg(1)-getType(); +unsigned Align; +if (ArgTy-isIntegerType()) + Align = 1; +else + Align = getContext().getTypeSizeInChars(ArgTy).getQuantity(); +SI-setAlignment(Align); return SI; } // 3DNow! Modified: cfe/trunk/lib/Headers/emmintrin.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/emmintrin.h?rev=191243r1=191242r2=191243view=diff == --- cfe/trunk/lib/Headers/emmintrin.h (original) +++ cfe/trunk/lib/Headers/emmintrin.h Mon Sep 23 18:38:39 2013 @@ -1214,6 +1214,14 @@ _mm_stream_si32(int *__p, int __a) __builtin_ia32_movnti(__p, __a); } +#ifdef __x86_64__ +static __inline__ void __attribute__((__always_inline__, __nodebug__)) +_mm_stream_si64(long long *__p, long long __a) +{ + __builtin_ia32_movnti64(__p, __a); +} +#endif + static __inline__ void __attribute__((__always_inline__, __nodebug__)) _mm_clflush(void const *__p) { Modified: cfe/trunk/test/CodeGen/builtins-x86.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins-x86.c?rev=191243r1=191242r2=191243view=diff == --- cfe/trunk/test/CodeGen/builtins-x86.c (original) +++ cfe/trunk/test/CodeGen/builtins-x86.c Mon Sep 23 18:38:39 2013 @@ -55,6 +55,7 @@ void f0() { const float* tmp_fCp; double*tmp_dp; const double* tmp_dCp; + long long* tmp_LLip; #define imm_i 32 #define imm_i_0_2 0 @@ -288,6 +289,9 @@ void f0() { tmp_i = __builtin_ia32_movmskpd(tmp_V2d); tmp_i = __builtin_ia32_pmovmskb128(tmp_V16c); (void) __builtin_ia32_movnti(tmp_ip, tmp_i); +#ifdef USE_64 + (void) __builtin_ia32_movnti64(tmp_LLip, tmp_LLi); +#endif (void) __builtin_ia32_movntpd(tmp_dp, tmp_V2d); (void) __builtin_ia32_movntdq(tmp_V2LLip, tmp_V2LLi); tmp_V2LLi = __builtin_ia32_psadbw128(tmp_V16c, tmp_V16c); Modified: cfe/trunk/test/CodeGen/sse-builtins.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/sse-builtins.c?rev=191243r1=191242r2=191243view=diff == --- cfe/trunk/test/CodeGen/sse-builtins.c (original) +++ cfe/trunk/test/CodeGen/sse-builtins.c Mon Sep 23 18:38:39 2013 @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -ffreestanding -triple i386-apple-darwin9 -target-cpu pentium4 -target-feature +sse4.1 -g -emit-llvm %s -o - | FileCheck %s +// RUN: %clang_cc1 -ffreestanding -triple x86_64-apple-macosx10.8.0
r191248 - Fix argument types of some AVX2 intrinsics.
Author: efriedma Date: Mon Sep 23 18:52:04 2013 New Revision: 191248 URL: http://llvm.org/viewvc/llvm-project?rev=191248view=rev Log: Fix argument types of some AVX2 intrinsics. This fix makes our headers consistent with gcc. PR17312. Modified: cfe/trunk/lib/Headers/avx2intrin.h cfe/trunk/test/CodeGen/avx2-builtins.c Modified: cfe/trunk/lib/Headers/avx2intrin.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx2intrin.h?rev=191248r1=191247r2=191248view=diff == --- cfe/trunk/lib/Headers/avx2intrin.h (original) +++ cfe/trunk/lib/Headers/avx2intrin.h Mon Sep 23 18:52:04 2013 @@ -1061,7 +1061,7 @@ _mm_srlv_epi64(__m128i __X, __m128i __Y) #define _mm_mask_i32gather_epi64(a, m, i, mask, s) __extension__ ({ \ __m128i __a = (a); \ - int const *__m = (m); \ + long long const *__m = (m); \ __m128i __i = (i); \ __m128i __mask = (mask); \ (__m128i)__builtin_ia32_gatherd_q((__v2di)__a, (const __v2di *)__m, \ @@ -1069,7 +1069,7 @@ _mm_srlv_epi64(__m128i __X, __m128i __Y) #define _mm256_mask_i32gather_epi64(a, m, i, mask, s) __extension__ ({ \ __m256i __a = (a); \ - int const *__m = (m); \ + long long const *__m = (m); \ __m128i __i = (i); \ __m256i __mask = (mask); \ (__m256i)__builtin_ia32_gatherd_q256((__v4di)__a, (const __v4di *)__m, \ @@ -1077,7 +1077,7 @@ _mm_srlv_epi64(__m128i __X, __m128i __Y) #define _mm_mask_i64gather_epi64(a, m, i, mask, s) __extension__ ({ \ __m128i __a = (a); \ - int const *__m = (m); \ + long long const *__m = (m); \ __m128i __i = (i); \ __m128i __mask = (mask); \ (__m128i)__builtin_ia32_gatherq_q((__v2di)__a, (const __v2di *)__m, \ @@ -1085,7 +1085,7 @@ _mm_srlv_epi64(__m128i __X, __m128i __Y) #define _mm256_mask_i64gather_epi64(a, m, i, mask, s) __extension__ ({ \ __m256i __a = (a); \ - int const *__m = (m); \ + long long const *__m = (m); \ __m256i __i = (i); \ __m256i __mask = (mask); \ (__m256i)__builtin_ia32_gatherq_q256((__v4di)__a, (const __v4di *)__m, \ @@ -1176,28 +1176,28 @@ _mm_srlv_epi64(__m128i __X, __m128i __Y) (__v4si)_mm_set1_epi32(-1), (s)); }) #define _mm_i32gather_epi64(m, i, s) __extension__ ({ \ - int const *__m = (m); \ + long long const *__m = (m); \ __m128i __i = (i); \ (__m128i)__builtin_ia32_gatherd_q((__v2di)_mm_setzero_si128(), \ (const __v2di *)__m, (__v4si)__i, \ (__v2di)_mm_set1_epi64x(-1), (s)); }) #define _mm256_i32gather_epi64(m, i, s) __extension__ ({ \ - int const *__m = (m); \ + long long const *__m = (m); \ __m128i __i = (i); \ (__m256i)__builtin_ia32_gatherd_q256((__v4di)_mm256_setzero_si256(), \ (const __v4di *)__m, (__v4si)__i, \ (__v4di)_mm256_set1_epi64x(-1), (s)); }) #define _mm_i64gather_epi64(m, i, s) __extension__ ({ \ - int const *__m = (m); \ + long long const *__m = (m); \ __m128i __i = (i); \ (__m128i)__builtin_ia32_gatherq_q((__v2di)_mm_setzero_si128(), \ (const __v2di *)__m, (__v2di)__i, \ (__v2di)_mm_set1_epi64x(-1), (s)); }) #define _mm256_i64gather_epi64(m, i, s) __extension__ ({ \ - int const *__m = (m); \ + long long const *__m = (m); \ __m256i __i = (i); \ (__m256i)__builtin_ia32_gatherq_q256((__v4di)_mm256_setzero_si256(), \ (const __v4di *)__m, (__v4di)__i, \ Modified: cfe/trunk/test/CodeGen/avx2-builtins.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/avx2-builtins.c?rev=191248r1=191247r2=191248view=diff == --- cfe/trunk/test/CodeGen/avx2-builtins.c (original) +++ cfe/trunk/test/CodeGen/avx2-builtins.c Mon Sep 23 18:52:04 2013 @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 %s -O3 -triple=x86_64-apple-darwin -target-feature +avx2 -emit-llvm -o - | FileCheck %s +// RUN: %clang_cc1 %s -O3 -triple=x86_64-apple-darwin -target-feature +avx2 -emit-llvm -o - -Werror | FileCheck %s // Don't include mm_malloc.h, it's system specific. #define __MM_MALLOC_H @@ -850,22 +850,22 @@ __m128i test_mm256_mask_i64gather_epi32( return _mm256_mask_i64gather_epi32(a, b, c, d, 2); } -__m128i test_mm_mask_i32gather_epi64(__m128i a, int const *b, __m128i c, +__m128i test_mm_mask_i32gather_epi64(__m128i a, long long const *b, __m128i c, __m128i d) { // CHECK: @llvm.x86.avx2.gather.d.q return _mm_mask_i32gather_epi64(a, b, c, d, 2); } -__m256i test_mm256_mask_i32gather_epi64(__m256i a, int const *b, __m128i c, +__m256i test_mm256_mask_i32gather_epi64(__m256i a, long long const *b, __m128i c, __m256i d) { // CHECK: @llvm.x86.avx2.gather.d.q.256 return _mm256_mask_i32gather_epi64(a, b, c, d, 2); } -__m128i test_mm_mask_i64gather_epi64(__m128i a, int const *b, __m128i c, +__m128i test_mm_mask_i64gather_epi64(__m128i a, long long const *b, __m128i
r191120 - Fix return type of _mm_extract_epi8 etc.
Author: efriedma Date: Fri Sep 20 19:05:25 2013 New Revision: 191120 URL: http://llvm.org/viewvc/llvm-project?rev=191120view=rev Log: Fix return type of _mm_extract_epi8 etc. PR17300. Modified: cfe/trunk/lib/Headers/smmintrin.h cfe/trunk/test/CodeGen/vector.c Modified: cfe/trunk/lib/Headers/smmintrin.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/smmintrin.h?rev=191120r1=191119r2=191120view=diff == --- cfe/trunk/lib/Headers/smmintrin.h (original) +++ cfe/trunk/lib/Headers/smmintrin.h Fri Sep 20 19:05:25 2013 @@ -230,9 +230,10 @@ _mm_max_epu32 (__m128i __V1, __m128i __V * as a zero extended value, so it is unsigned. */ #define _mm_extract_epi8(X, N) (__extension__ ({ __v16qi __a = (__v16qi)(X); \ - (unsigned char)__a[(N)];})) + (int)(unsigned char) \ + __a[(N)];})) #define _mm_extract_epi32(X, N) (__extension__ ({ __v4si __a = (__v4si)(X); \ - (unsigned)__a[(N)];})) + __a[(N)];})) #ifdef __x86_64__ #define _mm_extract_epi64(X, N) (__extension__ ({ __v2di __a = (__v2di)(X); \ __a[(N)];})) Modified: cfe/trunk/test/CodeGen/vector.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/vector.c?rev=191120r1=191119r2=191120view=diff == --- cfe/trunk/test/CodeGen/vector.c (original) +++ cfe/trunk/test/CodeGen/vector.c Fri Sep 20 19:05:25 2013 @@ -55,3 +55,10 @@ unsigned long test_epi16(__m128i x) { re // CHECK: @test_epi16 // CHECK: extractelement 8 x i16 {{.*}}, i32 3 // CHECK: zext i16 {{.*}} to i32 + +void extractinttypes() { + extern int check_extract_result_int; + extern __typeof(_mm_extract_epi8(_mm_setzero_si128(), 3)) check_result_int; + extern __typeof(_mm_extract_epi16(_mm_setzero_si128(), 3)) check_result_int; + extern __typeof(_mm_extract_epi32(_mm_setzero_si128(), 3)) check_result_int; +} ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Recognize llvm::Triple::NaCl for MIPSEL
LGTM. -Eli On Fri, Sep 20, 2013 at 9:45 AM, Petar Jovanovic petar.jovano...@imgtec.com wrote: Any additional comments? Ok to commit? Regards, Petar -- *From:* Petar Jovanovic *Sent:* Friday, September 20, 2013 12:47 AM *To:* Eli Friedman *Cc:* cfe-commits@cs.uiuc.edu; llvm-comm...@cs.uiuc.edu; Eli Bendersky *Subject:* RE: [PATCH] Recognize llvm::Triple::NaCl for MIPSEL The patch has been updated with the test case. Take a look. Regards, Petar -- *From:* Eli Friedman [eli.fried...@gmail.com] *Sent:* Thursday, September 19, 2013 12:38 AM *To:* Petar Jovanovic *Cc:* cfe-commits@cs.uiuc.edu; llvm-comm...@cs.uiuc.edu; Eli Bendersky *Subject:* Re: [PATCH] Recognize llvm::Triple::NaCl for MIPSEL On Wed, Sep 18, 2013 at 11:28 AM, Petar Jovanovic petar.jovano...@imgtec.com wrote: A patch to AllocateTarget to recognize llvm::Triple::NaCl for MIPSEL and return NaClTargetInfo. Same as what other architectures in NaCl already do. Missing testcase? -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] [OPENMP] Bug fixes and improvements
LGTM. http://llvm-reviews.chandlerc.com/D1725 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] [OPENMP] Bug fixes and improvements
Please separate the functional changes and code cleanups into separate patches. -Eli On Wed, Sep 18, 2013 at 11:19 PM, Alexey Bataev a.bat...@hotmail.comwrote: Hi doug.gregor, gribozavr, cbergstrom, hfinkel, pan, 1. Fixed constructor of shared clause. 2. Some macros for clauses processing are replaced by private template methods. 3. Additional checks in sema analysis of OpenMP clauses. 4. Fixed rules in sema for scope analysis in OpenMP threadprivate directive. http://llvm-reviews.chandlerc.com/D1725 Files: test/OpenMP/threadprivate_ast_print.cpp test/OpenMP/threadprivate_messages.cpp include/clang/AST/StmtOpenMP.h include/clang/AST/RecursiveASTVisitor.h tools/libclang/RecursiveASTVisitor.h tools/libclang/CIndex.cpp lib/Sema/TreeTransform.h lib/Sema/SemaOpenMP.cpp lib/AST/DeclPrinter.cpp lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/Serialization/ASTWriterStmt.cpp ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] [Patch] Fix for assertion when incomplete array type is used as template param
This patch isn't right; we're not building the correct AST. -ast-dump shows the following, which is clearly wrong: | `-CompoundStmt 0x7fddf282f888 line:5:13, line:9:1 | |-DeclStmt 0x7fddf282f820 line:7:1, col:23 | | `-VarDecl 0x7fddf282f790 col:1, col:3 t 'int []':'int []' | | `-ImplicitValueInitExpr 0x7fddf282f810 invalid sloc 'int []':'int []' Also, when you submit a patch, please include the testcase as a regression test in clang/test/. http://llvm-reviews.chandlerc.com/D1700 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: r191046 - Don't correct typos in Sema::BuildCXXNestedNameSpecifier with -fms-extensions
On Thu, Sep 19, 2013 at 3:38 PM, Kaelyn Uhrain ri...@google.com wrote: Author: rikka Date: Thu Sep 19 17:38:48 2013 New Revision: 191046 URL: http://llvm.org/viewvc/llvm-project?rev=191046view=rev Log: Don't correct typos in Sema::BuildCXXNestedNameSpecifier with -fms-extensions When -fms-extensions is enabled, the typo correction was being called here on non-error paths (as in test/SemaTemplate/lookup-dependent-bases.cpp) and correct compilation depended on Sema::CorrectTypo not finding a viable candidate. Modified: cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp Modified: cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp?rev=191046r1=191045r2=191046view=diff == --- cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp (original) +++ cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp Thu Sep 19 17:38:48 2013 @@ -484,7 +484,7 @@ bool Sema::BuildCXXNestedNameSpecifier(S // FIXME: Deal with ambiguities cleanly. - if (Found.empty() !ErrorRecoveryLookup) { + if (Found.empty() !ErrorRecoveryLookup !getLangOpts().MicrosoftExt) { // We haven't found anything, and we're not recovering from Are you sure you don't mean MicrosoftMode? Also, missing testcase. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [patch] [llvm] Fix a trailing comma related pedantic warning
On Thu, Sep 19, 2013 at 3:18 PM, G M gmiso...@gmail.com wrote: Fix a small warning in Dwarf.h:801:15: warning: comma at end of enumerator list [-Wpedantic This was fixed in r191053. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: r190912 - Fix ObjC @encode for C++ classes w/virtual bases.
On Tue, Sep 17, 2013 at 8:08 PM, jahanian fjahan...@apple.com wrote: Can you elaborate what got fixed. Are we matching gcc's behavior? I see that it is currently {E=^^?i^^?ii^^?} for this test case. The patch fixes an assertion failure, and also nonsensical output when assertions are turned off. (The encoding is supposed to match the layout of the class, and the class has two vptrs, not three.) -Eli - Fariborz On Sep 17, 2013, at 6:59 PM, Eli Friedman eli.fried...@gmail.com wrote: Author: efriedma Date: Tue Sep 17 20:59:16 2013 New Revision: 190912 URL: http://llvm.org/viewvc/llvm-project?rev=190912view=rev Log: Fix ObjC @encode for C++ classes w/virtual bases. PR17142. Modified: cfe/trunk/lib/AST/ASTContext.cpp cfe/trunk/test/CodeGenObjCXX/encode.mm Modified: cfe/trunk/lib/AST/ASTContext.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=190912r1=190911r2=190912view=diff == --- cfe/trunk/lib/AST/ASTContext.cpp (original) +++ cfe/trunk/lib/AST/ASTContext.cpp Tue Sep 17 20:59:16 2013 @@ -5552,7 +5552,8 @@ void ASTContext::getObjCEncodingForStruc if (base-isEmpty()) continue; uint64_t offs = toBits(layout.getVBaseClassOffset(base)); - if (FieldOrBaseOffsets.find(offs) == FieldOrBaseOffsets.end()) + if (offs = uint64_t(toBits(layout.getNonVirtualSize())) + FieldOrBaseOffsets.find(offs) == FieldOrBaseOffsets.end()) FieldOrBaseOffsets.insert(FieldOrBaseOffsets.end(), std::make_pair(offs, base)); } Modified: cfe/trunk/test/CodeGenObjCXX/encode.mm URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/encode.mm?rev=190912r1=190911r2=190912view=diff == --- cfe/trunk/test/CodeGenObjCXX/encode.mm (original) +++ cfe/trunk/test/CodeGenObjCXX/encode.mm Tue Sep 17 20:59:16 2013 @@ -214,3 +214,13 @@ public: } @end // CHECK: internal global [41 x i8] c{dynamic_class=\22_vptr$dynamic_class\22^^?}\00 + +namespace PR17142 { + struct A { virtual ~A(); }; + struct B : virtual A { int y; }; + struct C { virtual ~C(); int z; }; + struct D : C, B { int a; }; + struct E : D {}; + // CHECK: @_ZN7PR171421xE = constant [14 x i8] c{E=^^?i^^?ii}\00 + extern const char x[] = @encode(E); +} ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Recognize llvm::Triple::NaCl for MIPSEL
On Wed, Sep 18, 2013 at 11:28 AM, Petar Jovanovic petar.jovano...@imgtec.com wrote: A patch to AllocateTarget to recognize llvm::Triple::NaCl for MIPSEL and return NaClTargetInfo. Same as what other architectures in NaCl already do. Missing testcase? -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Use correct semantic DeclContext for local declarations of variables and functions with linkage
LGTM. http://llvm-reviews.chandlerc.com/D1499 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Make Preprocessor::Lex non-recursive
On Wed, Sep 18, 2013 at 3:45 PM, Richard Smith rich...@metafoo.co.ukwrote: On Wed, Sep 18, 2013 at 3:44 PM, Richard Smith rich...@metafoo.co.ukwrote: A modules-related change seems to have slipped into lib/Frontend/CompilerInstance.cpp Ah, yes, thanks; fixed in my local copy. The IsAtStartOfLine FIXME concerns me a little. Token.h says: StartOfLine = 0x01, // At start of line or only after whitespace. ... which implies to me that the intent was that it be false if there's a preceding EMPTY macro. I'm not sure if that's what the existing users of the macro want, though. Um, users of the flag. The current users of the flag expect the current behavior, which ignores empty macros. The canonical example is PrintPreprocessedOutput, which uses it to indicate whether it needs to insert a newline between two tokens, the behavior being tested in test/Preprocessor/hash_line.c. This patch doesn't change our behavior here outside of the IDENTITY() case. I'll update the comment. What happens if the Lex call in isNextPPTokenLParen returns false? Lex never returns false if we're lexing in raw mode. I'll add an assertion to that effect. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Add specific warning flags for GNU ext in Sema
On Tue, Sep 17, 2013 at 7:28 PM, Peter N Lewis pe...@stairways.com.auwrote: On 18/09/2013, at 6:38 , Eli Friedman eli.fried...@gmail.com wrote: Sorry about the delay. LGTM. No problem. Eli, if you're happy, unless it needs a seconder, please commit it for me. r190972. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190971 - Fix CharByteWidth assertion in LiteralSupport.
Author: efriedma Date: Wed Sep 18 18:23:13 2013 New Revision: 190971 URL: http://llvm.org/viewvc/llvm-project?rev=190971view=rev Log: Fix CharByteWidth assertion in LiteralSupport. Patch by Eelis van der Weegen. Modified: cfe/trunk/lib/Lex/LiteralSupport.cpp Modified: cfe/trunk/lib/Lex/LiteralSupport.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/LiteralSupport.cpp?rev=190971r1=190970r2=190971view=diff == --- cfe/trunk/lib/Lex/LiteralSupport.cpp (original) +++ cfe/trunk/lib/Lex/LiteralSupport.cpp Wed Sep 18 18:23:13 2013 @@ -336,7 +336,7 @@ static void EncodeUCNEscape(const char * return; } - assert((CharByteWidth == 1 || CharByteWidth == 2 || CharByteWidth) + assert((CharByteWidth == 1 || CharByteWidth == 2 || CharByteWidth == 4) only character widths of 1, 2, or 4 bytes supported); (void)UcnLen; ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Check for the intended values in CharByteWidth assertion
r190971. -Eli On Wed, Sep 18, 2013 at 5:08 AM, Eelis ee...@eelis.net wrote: See patch. ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190972 - Add specific warning flags for GNU ext in Sema.
Author: efriedma Date: Wed Sep 18 18:23:17 2013 New Revision: 190972 URL: http://llvm.org/viewvc/llvm-project?rev=190972view=rev Log: Add specific warning flags for GNU ext in Sema. This patch adds the following, more specific warning flags: gnu-anonymous-struct gnu-compound-literal-initializer gnu-empty-struct gnu-flexible-array-initializer gnu-flexible-array-union-member gnu-folding-constant redeclared-class-member gnu-redeclared-enum gnu-union-cast gnu-variable-sized-type-not-at-end Patch by Peter Lewis. Added: cfe/trunk/test/SemaCXX/gnu-flags.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/test/Misc/warning-flags-tree.c cfe/trunk/test/Misc/warning-flags.c cfe/trunk/test/Sema/gnu-flags.c Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=190972r1=190971r2=190972view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Wed Sep 18 18:23:17 2013 @@ -23,12 +23,14 @@ def AddressOfTemporary : DiagGroupaddr def : DiagGroupaggregate-return; def GNUAlignofExpression : DiagGroupgnu-alignof-expression; def AmbigMemberTemplate : DiagGroupambiguous-member-template; +def GNUAnonymousStruct : DiagGroupgnu-anonymous-struct; def ArrayBounds : DiagGrouparray-bounds; def ArrayBoundsPointerArithmetic : DiagGrouparray-bounds-pointer-arithmetic; def Availability : DiagGroupavailability; def Section : DiagGroupsection; def AutoImport : DiagGroupauto-import; def GNUBinaryLiteral : DiagGroupgnu-binary-literal; +def GNUCompoundLiteralInitializer : DiagGroupgnu-compound-literal-initializer; def BitFieldConstantConversion : DiagGroupbitfield-constant-conversion; def ConstantConversion : DiagGroupconstant-conversion, [ BitFieldConstantConversion ] ; @@ -88,10 +90,14 @@ def Documentation : DiagGroupdocumenta def EmptyBody : DiagGroupempty-body; def GNUEmptyInitializer : DiagGroupgnu-empty-initializer; +def GNUEmptyStruct : DiagGroupgnu-empty-struct; def ExtraTokens : DiagGroupextra-tokens; def CXX11ExtraSemi : DiagGroupc++11-extra-semi; def ExtraSemi : DiagGroupextra-semi, [CXX11ExtraSemi]; +def GNUFlexibleArrayInitializer : DiagGroupgnu-flexible-array-initializer; +def GNUFlexibleArrayUnionMember : DiagGroupgnu-flexible-array-union-member; +def GNUFoldingConstant : DiagGroupgnu-folding-constant; def FormatExtraArgs : DiagGroupformat-extra-args; def FormatZeroLength : DiagGroupformat-zero-length; @@ -233,6 +239,8 @@ def PoundPragmaMessage : DiagGroup#pra DiagCategory#pragma message Directive; def : DiagGrouppointer-to-int-cast; def : DiagGroupredundant-decls; +def RedeclaredClassMember : DiagGroupredeclared-class-member; +def GNURedeclaredEnum : DiagGroupgnu-redeclared-enum; def ReturnStackAddress : DiagGroupreturn-stack-address; def ReturnTypeCLinkage : DiagGroupreturn-type-c-linkage; def ReturnType : DiagGroupreturn-type, [ReturnTypeCLinkage]; @@ -266,6 +274,8 @@ def TautologicalCompare : DiagGrouptau def HeaderHygiene : DiagGroupheader-hygiene; def DuplicateDeclSpecifier : DiagGroupduplicate-decl-specifier; def CompareDistinctPointerType : DiagGroupcompare-distinct-pointer-types; +def GNUUnionCast : DiagGroupgnu-union-cast; +def GNUVariableSizedTypeNotAtEnd : DiagGroupgnu-variable-sized-type-not-at-end; def Varargs : DiagGroupvarargs; def Unsequenced : DiagGroupunsequenced; @@ -541,15 +551,19 @@ def C11 : DiagGroupc11-extensions; def C99 : DiagGroupc99-extensions; // A warning group for warnings about GCC extensions. -def GNU : DiagGroupgnu, [GNUAlignofExpression, GNUBinaryLiteral, -GNUCaseRange, GNUComplexInteger, -GNUConditionalOmittedOperand, -GNUDesignator, GNUEmptyInitializer, -VLAExtension, +def GNU : DiagGroupgnu, [GNUAlignofExpression, GNUAnonymousStruct, +GNUBinaryLiteral, GNUCaseRange, +GNUComplexInteger, GNUCompoundLiteralInitializer, +GNUConditionalOmittedOperand, GNUDesignator, +GNUEmptyInitializer, GNUEmptyStruct, +VLAExtension, GNUFlexibleArrayInitializer, +GNUFlexibleArrayUnionMember, GNUFoldingConstant, GNUImaginaryConstant, GNULabelsAsValue, +RedeclaredClassMember, GNURedeclaredEnum, GNUStatementExpression, GNUStaticFloatInit, -ZeroLengthArray, -GNUZeroLineDirective, GNUZeroVariadicMacroArguments]; +GNUUnionCast,
r190980 - Make Preprocessor::Lex non-recursive.
Author: efriedma Date: Wed Sep 18 19:41:32 2013 New Revision: 190980 URL: http://llvm.org/viewvc/llvm-project?rev=190980view=rev Log: Make Preprocessor::Lex non-recursive. Before this patch, Lex() would recurse whenever the current lexer changed (e.g. upon entry into a macro). This patch turns the recursion into a loop: the various lex routines now don't return a token when the current lexer changes, and at the top level Preprocessor::Lex() now loops until it finds a token. Normally, the recursion wouldn't end up being very deep, but the recursion depth can explode in edge cases like a bunch of consecutive macros which expand to nothing (like in the testcase test/Preprocessor/macro_expand_empty.c in this patch). rdar://problem/14569770 Added: cfe/trunk/test/Preprocessor/macro_expand_empty.c Modified: cfe/trunk/include/clang/Lex/Lexer.h cfe/trunk/include/clang/Lex/PTHLexer.h cfe/trunk/include/clang/Lex/Preprocessor.h cfe/trunk/include/clang/Lex/Token.h cfe/trunk/include/clang/Lex/TokenLexer.h cfe/trunk/lib/Frontend/VerifyDiagnosticConsumer.cpp cfe/trunk/lib/Lex/Lexer.cpp cfe/trunk/lib/Lex/PPLexerChange.cpp cfe/trunk/lib/Lex/PPMacroExpansion.cpp cfe/trunk/lib/Lex/PTHLexer.cpp cfe/trunk/lib/Lex/Preprocessor.cpp cfe/trunk/lib/Lex/PreprocessorLexer.cpp cfe/trunk/lib/Lex/TokenLexer.cpp cfe/trunk/test/Preprocessor/hash_line.c cfe/trunk/test/SemaCXX/warn-empty-body.cpp Modified: cfe/trunk/include/clang/Lex/Lexer.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Lexer.h?rev=190980r1=190979r2=190980view=diff == --- cfe/trunk/include/clang/Lex/Lexer.h (original) +++ cfe/trunk/include/clang/Lex/Lexer.h Wed Sep 18 19:41:32 2013 @@ -80,6 +80,12 @@ class Lexer : public PreprocessorLexer { // line flag set on it. bool IsAtStartOfLine; + bool IsAtPhysicalStartOfLine; + + bool HasLeadingSpace; + + bool HasLeadingEmptyMacro; + // CurrentConflictMarkerState - The kind of conflict marker we are handling. ConflictMarkerKind CurrentConflictMarkerState; @@ -127,31 +133,21 @@ public: /// from. Currently this is only used by _Pragma handling. SourceLocation getFileLoc() const { return FileLoc; } +private: /// Lex - Return the next token in the file. If this is the end of file, it /// return the tok::eof token. This implicitly involves the preprocessor. - void Lex(Token Result) { -// Start a new token. -Result.startToken(); - -// NOTE, any changes here should also change code after calls to -// Preprocessor::HandleDirective -if (IsAtStartOfLine) { - Result.setFlag(Token::StartOfLine); - IsAtStartOfLine = false; -} - -// Get a token. Note that this may delete the current lexer if the end of -// file is reached. -LexTokenInternal(Result); - } + bool Lex(Token Result); +public: /// isPragmaLexer - Returns true if this Lexer is being used to lex a pragma. bool isPragmaLexer() const { return Is_PragmaLexer; } +private: /// IndirectLex - An indirect call to 'Lex' that can be invoked via /// the PreprocessorLexer interface. void IndirectLex(Token Result) { Lex(Result); } +public: /// LexFromRawLexer - Lex a token from a designated raw lexer (one with no /// associated preprocessor object. Return true if the 'next character to /// read' pointer points at the end of the lexer buffer, false otherwise. @@ -447,12 +443,14 @@ private: /// LexTokenInternal - Internal interface to lex a preprocessing token. Called /// by Lex. /// - void LexTokenInternal(Token Result); + bool LexTokenInternal(Token Result, bool TokAtPhysicalStartOfLine); + + bool CheckUnicodeWhitespace(Token Result, uint32_t C, const char *CurPtr); /// Given that a token begins with the Unicode character \p C, figure out /// what kind of token it is and dispatch to the appropriate lexing helper /// function. - void LexUnicode(Token Result, uint32_t C, const char *CurPtr); + bool LexUnicode(Token Result, uint32_t C, const char *CurPtr); /// FormTokenWithChars - When we lex a token, we have identified a span /// starting at BufferPtr, going to TokEnd that forms the token. This method @@ -570,24 +568,28 @@ private: void SkipBytes(unsigned Bytes, bool StartOfLine); + void PropagateLineStartLeadingSpaceInfo(Token Result); + const char *LexUDSuffix(Token Result, const char *CurPtr, bool IsStringLiteral); // Helper functions to lex the remainder of a token of the specific type. - void LexIdentifier (Token Result, const char *CurPtr); - void LexNumericConstant(Token Result, const char *CurPtr); - void LexStringLiteral (Token Result, const char *CurPtr, + bool LexIdentifier (Token Result, const char *CurPtr); + bool LexNumericConstant(Token Result, const char *CurPtr); + bool LexStringLiteral
Re: r190979 - Avoid including stdlib.h in the intrin.h test
On Wed, Sep 18, 2013 at 5:32 PM, Reid Kleckner r...@kleckner.net wrote: Author: rnk Date: Wed Sep 18 19:32:11 2013 New Revision: 190979 URL: http://llvm.org/viewvc/llvm-project?rev=190979view=rev Log: Avoid including stdlib.h in the intrin.h test Modified: cfe/trunk/test/Headers/ms-intrin.cpp Modified: cfe/trunk/test/Headers/ms-intrin.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/ms-intrin.cpp?rev=190979r1=190978r2=190979view=diff == --- cfe/trunk/test/Headers/ms-intrin.cpp (original) +++ cfe/trunk/test/Headers/ms-intrin.cpp Wed Sep 18 19:32:11 2013 @@ -1,5 +1,10 @@ // RUN: %clang -target i386-pc-win32 -fms-extensions -fsyntax-only %s +// Get size_t, but avoid including mm_malloc.h which includes stdlib.h which may +// not exist. +#include stdint.h +#undef __STDC_HOSTED__ + #include Intrin.h It's more conventional to use -ffreestanding rather than write #undef __STDC_HOSTED__... -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190981 - Fix crash with cast of value-dependent expr.
Author: efriedma Date: Wed Sep 18 20:12:33 2013 New Revision: 190981 URL: http://llvm.org/viewvc/llvm-project?rev=190981view=rev Log: Fix crash with cast of value-dependent expr. We don't really need to perform semantic analysis on the dependent expression anyway, so just call the cast dependent. rdar://problem/15012610 Modified: cfe/trunk/lib/Sema/SemaCast.cpp cfe/trunk/test/SemaTemplate/dependent-expr.cpp Modified: cfe/trunk/lib/Sema/SemaCast.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCast.cpp?rev=190981r1=190980r2=190981view=diff == --- cfe/trunk/lib/Sema/SemaCast.cpp (original) +++ cfe/trunk/lib/Sema/SemaCast.cpp Wed Sep 18 20:12:33 2013 @@ -243,7 +243,9 @@ Sema::BuildCXXNamedCast(SourceLocation O // If the type is dependent, we won't do the semantic analysis now. // FIXME: should we check this in a more fine-grained manner? - bool TypeDependent = DestType-isDependentType() || Ex.get()-isTypeDependent(); + bool TypeDependent = DestType-isDependentType() || + Ex.get()-isTypeDependent() || + Ex.get()-isValueDependent(); CastOperation Op(*this, DestType, E); Op.OpRange = SourceRange(OpLoc, Parens.getEnd()); @@ -2008,7 +2010,8 @@ void CastOperation::CheckCXXCStyleCast(b } // If the type is dependent, we won't do any other semantic analysis now. - if (DestType-isDependentType() || SrcExpr.get()-isTypeDependent()) { + if (DestType-isDependentType() || SrcExpr.get()-isTypeDependent() || + SrcExpr.get()-isValueDependent()) { assert(Kind == CK_Dependent); return; } Modified: cfe/trunk/test/SemaTemplate/dependent-expr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/dependent-expr.cpp?rev=190981r1=190980r2=190981view=diff == --- cfe/trunk/test/SemaTemplate/dependent-expr.cpp (original) +++ cfe/trunk/test/SemaTemplate/dependent-expr.cpp Wed Sep 18 20:12:33 2013 @@ -72,3 +72,10 @@ namespace PR8795 { return data[0]; } } + +templatetypename T struct CastDependentIntToPointer { + static void* f() { +T *x; +return ((void*)(((unsigned long)(x)|0x1ul))); + } +}; ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: r190979 - Avoid including stdlib.h in the intrin.h test
On Wed, Sep 18, 2013 at 6:12 PM, Reid Kleckner r...@google.com wrote: On Wed, Sep 18, 2013 at 6:05 PM, Eli Friedman eli.fried...@gmail.comwrote: On Wed, Sep 18, 2013 at 5:32 PM, Reid Kleckner r...@kleckner.net wrote: Author: rnk Date: Wed Sep 18 19:32:11 2013 New Revision: 190979 URL: http://llvm.org/viewvc/llvm-project?rev=190979view=rev Log: Avoid including stdlib.h in the intrin.h test Modified: cfe/trunk/test/Headers/ms-intrin.cpp Modified: cfe/trunk/test/Headers/ms-intrin.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/ms-intrin.cpp?rev=190979r1=190978r2=190979view=diff == --- cfe/trunk/test/Headers/ms-intrin.cpp (original) +++ cfe/trunk/test/Headers/ms-intrin.cpp Wed Sep 18 19:32:11 2013 @@ -1,5 +1,10 @@ // RUN: %clang -target i386-pc-win32 -fms-extensions -fsyntax-only %s +// Get size_t, but avoid including mm_malloc.h which includes stdlib.h which may +// not exist. +#include stdint.h +#undef __STDC_HOSTED__ + #include Intrin.h It's more conventional to use -ffreestanding rather than write #undef __STDC_HOSTED__... That doesn't work because we need size_t from stdint.h. ;_; I recall this is why Eric gave up testing it in the first place. Umm, the C standard doesn't specify that size_t comes from stdint.h. Try stddef.h. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190984 - Fix use-after-free in r190980.
Author: efriedma Date: Wed Sep 18 20:51:23 2013 New Revision: 190984 URL: http://llvm.org/viewvc/llvm-project?rev=190984view=rev Log: Fix use-after-free in r190980. Modified: cfe/trunk/lib/Lex/Lexer.cpp Modified: cfe/trunk/lib/Lex/Lexer.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Lexer.cpp?rev=190984r1=190983r2=190984view=diff == --- cfe/trunk/lib/Lex/Lexer.cpp (original) +++ cfe/trunk/lib/Lex/Lexer.cpp Wed Sep 18 20:51:23 2013 @@ -2823,9 +2823,12 @@ bool Lexer::Lex(Token Result) { bool atPhysicalStartOfLine = IsAtPhysicalStartOfLine; IsAtPhysicalStartOfLine = false; - bool result = LexTokenInternal(Result, atPhysicalStartOfLine); - assert((result || !isLexingRawMode()) Raw lex must succeed); - return result; + bool isRawLex = isLexingRawMode(); + (void) isRawLex; + bool returnedToken = LexTokenInternal(Result, atPhysicalStartOfLine); + // (After the LexTokenInternal call, the lexer might be destroyed.) + assert((returnedToken || !isRawLex) Raw lex must succeed); + return returnedToken; } /// LexTokenInternal - This implements a simple C family lexer. It is an ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: PR16752[PATCH]: 'mode' attribute for unusual targets doesn't work properly.
Sorry about the delayed response. XC mode is not valid on PPC; if there's a test trying to use it, the test is wrong. Please just fix the test. -Eli On Tue, Sep 17, 2013 at 11:54 AM, Stepan Dyatkovskiy stpwo...@narod.ruwrote: Hi Eli, I have to resend you PR16752 fix. Since I had changed it. The previous one causes failures on some buildbots. This fix also contains customization for PPC case: For 'XC' mode (96 bit float) PPC uses 128 bit fp type. -Stepan. Eli Friedman wrote: LGTM. -Eli On Mon, Sep 9, 2013 at 5:04 AM, Stepan Dyatkovskiy stpwo...@narod.ru mailto:stpwo...@narod.ru wrote: Since I had split the patch and committed only ASTContext and TargetInfo getIntTypeByWidth and friends, there is the second patch, that fixes PR16752 itself. Please, find it in attachment for review. -Stepan. Eli Friedman wrote: On Tue, Sep 3, 2013 at 2:16 AM, Stepan Dyatkovskiy stpwo...@narod.ru mailto:stpwo...@narod.ru mailto:stpwo...@narod.ru mailto:stpwo...@narod.ru wrote: Hi Eli, Sorry for latency. As you remember this patch should correct 'mode' attr implementation. You proposed to use generic way of type detection for each target: just scan for target types and select one with suitable width. Unfortunately I can't commit patch with generic implementation. I got test failure for clang. And I expect more failures for another targets. The reason is next. The target could still keep mode unsupported even if it has type of suitable width. I mean the next. For example this string should cause error for i686 machines (128 bit float): typedef float f128ibm __attribute__ ((mode (TF))); But in case of generic approach for all targets it would be passed. In this case virtual functions allows to implement exceptions. The generic implementation is still essentially correct; the the issue is that getLongDoubleWidth() isn't actually the right value to check. It returns sizeof(long double) * 8, not the actual width of the underlying float format. You should be able to work around this by checking getLongDoubleFormat() instead of getLongDoubleWidth(). In this case 'getIntTypeByWidth' may be a bit confusing name. Instead it is supposed to return type for some particular mode, not by width. Something like getInt/RealTypeForMode(width, sign). Though, currently I kept the original name. If you want to change it, that's fine. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Add __builtin_convertvector
On Tue, Sep 17, 2013 at 2:02 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Mon, Sep 16, 2013 at 8:41 PM, hfin...@anl.gov hfin...@anl.gov wrote: Comment at: include/clang/AST/Expr.h:3474 @@ +3473,3 @@ +/// vector type of the same arity. +class ConvertVectorExpr : public Expr { // Should this be an ExplicitCastExpr? +private: Eli Friedman wrote: No, this should not be a CastExpr. Okay; I copied this comment from the AsTypeExpr node, so we should probably remove it from there too. Okay. Comment at: lib/CodeGen/CGExprScalar.cpp:1011 @@ +1010,3 @@ + Res = Builder.CreateIntCast(Src, DstTy, InputSigned, conv); + else if (InputSigned) + Res = Builder.CreateSIToFP(Src, DstTy, conv); Eli Friedman wrote: What if the destination type is a bool vector? I think bool vectors are explicitly forbidden (in r190721). You can still use the ext_vector_type attribute to get one. Ah, thanks! The code generator seems to quite happily produce (and the IR validator accepts): %conv = fptoui 8 x float %0 to 8 x i1 and: %conv = trunc 8 x i16 %0 to 8 x i1 will that be sufficient, do we need to do something special? Would it be better to explicitly compare against zero? Your documentation says it will do what a C cast would do (which is compare against zero, not truncate). -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Trapping math and libfuncs improvements [Part 3 - new definitions]
LGTM. -Eli On Fri, Sep 13, 2013 at 10:05 AM, Hal Finkel hfin...@anl.gov wrote: Ping. Thanks again, Hal - Original Message - Eli, I've revised the patch, adding back for 'e' flags where POSIX says that errno might be modified (even if the Linux man pages claim otherwise); with the following exceptions: For ceil, floor, nearbyint, rint - The Linux man page explains that the conditions under which the documented errno values might be set are impossible on all known IEEE systems [this does not represent a functionality change from the current definitions, I'm just noting it here for completeness]. Note that this changes the current definitions for lrint and fma (unfortunately). The Linux man page documents that these don't set errno, but the POSIX standard says that they should. Thanks again, Hal - Original Message - Eli, Here's the third (and, for now, final) part: adding new LIBBUILTIN definitions to match the existing libm __builtin_* definitions. As it turns out, I think that you were right: separating out the changes to the existing definitions does make it easier to see what is changing :) Thanks again, Hal - Original Message - - Original Message - - Original Message - Looks fine. r190217, thanks! [I also committed, in r190218, a trivial reordering of the existing definitions.] And in r190222, I reordered the existing definitions to match the order of the __builtin_* definitions (which is a small change, but should make comparing to the new list easier). -Hal -Hal -Eli On Fri, Sep 6, 2013 at 4:10 PM, Hal Finkel hfin...@anl.gov wrote: Eli, Here's the first patch -- adding of the missing 'n' (nothrow) flag to the existing libm LIBBUILTIN definitions (plus a test case -- unfortunately making the source also valid C++ seems to make the test case uglier). Thanks again, Hal - Original Message - On Fri, Sep 6, 2013 at 3:53 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Fri, Sep 6, 2013 at 3:32 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Fri, Sep 6, 2013 at 8:27 AM, Hal Finkel hfin...@anl.gov wrote: Ben, Chad, et al., The attached patch cleans up the LIBBUILTIN libm function definitions, and improves the way in which -ftrapping-math is handled in a complementary way. Specifically, this patch: 1. The libm LIBBUILTIN definitions are synchronized with the __builtin_* definitions for libm functions. We currently have __builtin_* definitions for many more functions than those for which we have LIBBUILTIN definitions. This unnecessarily pessimizes code generation around many of the remaining libm functions. 2. Many of these functions can be marked as readnone so long as we can neglect floating point exceptions. I've added a new specifier 'T', which like 'e' for errno, marks the builtin as const so long as trapping math is disabled. I've added a TrappingMath lang option to track this. 3. Disables -ftrapping-math by default. This is currently enabled by default (following gcc), but this is silly. No system of which I'm aware actually traps on floating-point exceptions by default (some system-specific mechanism is necessary in order to enable SIGFPE generation). In addition, in Clang, we currently don't support fenv access. As a result, we can turn this off by default (and enable better code generation around the functions for which we have LIBBUILTIN definitions). 4. Updates the CodeGen/libcall-declarations.c test case. As a side effect, the existing libm LIBBUILTIN definitions that were missing the 'n' (nothrow) specifier now have it. For clarity, please split the patch into four separate patches: reformatting,
Re: [PATCH] Add specific warning flags for GNU ext in Sema
Sorry about the delay. LGTM. -Eli On Mon, Sep 16, 2013 at 7:16 PM, Peter N Lewis pe...@stairways.com.auwrote: Ping x 2. On 05/09/2013, at 18:33 , Peter Lewis pe...@stairways.com.au wrote: This patch adds the following specific warning flags to -Wgnu: gnu-anonymous-struct gnu-compound-literal-initializer gnu-empty-struct gnu-flexible-array-initializer gnu-flexible-array-union-member gnu-folding-constant redeclared-class-member gnu-redeclared-enum gnu-union-cast gnu-variable-sized-type-not-at-end At Eli's suggestion, redeclared-class-member does not have the gnu prefix, and this patch subsumes ext_vla_folded_to_constant into gnu-folding-constant (a visible effect of this is that -Wgnu enables gnu-folding-constant which enabled ext_vla_folded_to_constant where previously it would not have). http://llvm-reviews.chandlerc.com/D1605 Files: test/Sema/gnu-flags.c test/Misc/warning-flags-tree.c test/Misc/warning-flags.c test/SemaCXX/gnu-flags.cpp include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td D1605.1.patch -- Keyboard Maestro 6.2 now out - control Mail, reveal a file, format AppleScripts and more. Keyboard Maestro http://www.keyboardmaestro.com/ Macros for your Mac http://www.stairways.com/ http://download.keyboardmaestro.com/ ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Add __builtin_convertvector
You're missing a testcase for template instantiation. http://llvm-reviews.chandlerc.com/D1677 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190912 - Fix ObjC @encode for C++ classes w/virtual bases.
Author: efriedma Date: Tue Sep 17 20:59:16 2013 New Revision: 190912 URL: http://llvm.org/viewvc/llvm-project?rev=190912view=rev Log: Fix ObjC @encode for C++ classes w/virtual bases. PR17142. Modified: cfe/trunk/lib/AST/ASTContext.cpp cfe/trunk/test/CodeGenObjCXX/encode.mm Modified: cfe/trunk/lib/AST/ASTContext.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=190912r1=190911r2=190912view=diff == --- cfe/trunk/lib/AST/ASTContext.cpp (original) +++ cfe/trunk/lib/AST/ASTContext.cpp Tue Sep 17 20:59:16 2013 @@ -5552,7 +5552,8 @@ void ASTContext::getObjCEncodingForStruc if (base-isEmpty()) continue; uint64_t offs = toBits(layout.getVBaseClassOffset(base)); - if (FieldOrBaseOffsets.find(offs) == FieldOrBaseOffsets.end()) + if (offs = uint64_t(toBits(layout.getNonVirtualSize())) + FieldOrBaseOffsets.find(offs) == FieldOrBaseOffsets.end()) FieldOrBaseOffsets.insert(FieldOrBaseOffsets.end(), std::make_pair(offs, base)); } Modified: cfe/trunk/test/CodeGenObjCXX/encode.mm URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/encode.mm?rev=190912r1=190911r2=190912view=diff == --- cfe/trunk/test/CodeGenObjCXX/encode.mm (original) +++ cfe/trunk/test/CodeGenObjCXX/encode.mm Tue Sep 17 20:59:16 2013 @@ -214,3 +214,13 @@ public: } @end // CHECK: internal global [41 x i8] c{dynamic_class=\22_vptr$dynamic_class\22^^?}\00 + +namespace PR17142 { + struct A { virtual ~A(); }; + struct B : virtual A { int y; }; + struct C { virtual ~C(); int z; }; + struct D : C, B { int a; }; + struct E : D {}; + // CHECK: @_ZN7PR171421xE = constant [14 x i8] c{E=^^?i^^?ii}\00 + extern const char x[] = @encode(E); +} ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Make Preprocessor::Lex non-recursive
On Mon, Sep 2, 2013 at 5:26 PM, Eli Friedman eli.fried...@gmail.com wrote: Per subject, patch attached which makes Preprocessor::Lex non-recursive. Before this patch, Lex() would recurse whenever the current lexer changed (e.g. upon entry into a macro). This patch turns the recursion into a loop: the various lex routines now don't return a token when the current lexer changes, and at the top level Preprocessor::Lex() now loops until it finds a token. Normally, the recursion doesn't end up being very deep, but the recursion depth can explode in edge cases like a bunch of consecutive macros which expand to nothing (like in the testcase test/Preprocessor/macro_expand_empty.c in this patch). To make this work, I made some substantial changes to the way the whitespace and empty macro flags are propagated. It had to be rewritten alongside this patch because the previous code to handle this wasn't tail-recursive. The code for this is pretty straightforward: it just adds a couple flags to the Lexer class, then updates them whenever we leave a macro expansion. I also fixed a minor bug while I was in the area: the previous code didn't work correctly for macros which had tokens in the definitions which expanded to nothing. I still need to do performance measurements, but I'm not anticipating any measurable changes. Ping. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Add __builtin_convertvector
Comment at: include/clang/AST/Expr.h:3474 @@ +3473,3 @@ +/// vector type of the same arity. +class ConvertVectorExpr : public Expr { // Should this be an ExplicitCastExpr? +private: No, this should not be a CastExpr. Comment at: lib/Sema/TreeTransform.h:9171 @@ +9170,3 @@ + llvm_unreachable(Cannot transform convertVector expressions yet); +} + It would be nice to fix this before committing. Comment at: lib/CodeGen/CGExprScalar.cpp:1011 @@ +1010,3 @@ + Res = Builder.CreateIntCast(Src, DstTy, InputSigned, conv); +else if (InputSigned) + Res = Builder.CreateSIToFP(Src, DstTy, conv); What if the destination type is a bool vector? Comment at: lib/Sema/SemaExpr.cpp:4489 @@ +4488,3 @@ + E-getSourceRange()); + + return Owned(new (Context) ConvertVectorExpr(E, DstTy, VK, OK, BuiltinLoc, You might want to be careful exactly what vector types we accept here... but I can't think of anything you actually need to check for, so maybe not. http://llvm-reviews.chandlerc.com/D1677 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190834 - Address review comment on r189557.
Author: efriedma Date: Mon Sep 16 19:51:31 2013 New Revision: 190834 URL: http://llvm.org/viewvc/llvm-project?rev=190834view=rev Log: Address review comment on r189557. We need to escape filenames the same way in InclusionRewriter whether UseLineDirective is true or false. Review comment from http://llvm.org/bugs/show_bug.cgi?id=17018#c2 Modified: cfe/trunk/lib/Rewrite/Frontend/InclusionRewriter.cpp Modified: cfe/trunk/lib/Rewrite/Frontend/InclusionRewriter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Rewrite/Frontend/InclusionRewriter.cpp?rev=190834r1=190833r2=190834view=diff == --- cfe/trunk/lib/Rewrite/Frontend/InclusionRewriter.cpp (original) +++ cfe/trunk/lib/Rewrite/Frontend/InclusionRewriter.cpp Mon Sep 16 19:51:31 2013 @@ -110,7 +110,9 @@ void InclusionRewriter::WriteLineInfo(co if (!ShowLineMarkers) return; if (UseLineDirective) { -OS #line ' ' Line ' ' '' Filename ''; +OS #line ' ' Line ' ' ''; +OS.write_escaped(Filename); +OS ''; } else { // Use GNU linemarkers as described here: // http://gcc.gnu.org/onlinedocs/cpp/Preprocessor-Output.html ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190833 - Fix potential race in module building code.
Author: efriedma Date: Mon Sep 16 19:51:29 2013 New Revision: 190833 URL: http://llvm.org/viewvc/llvm-project?rev=190833view=rev Log: Fix potential race in module building code. Let the module building code handle the case of overwriting an existing file itself, so the existing locking infrastructure works correctly. rdar://problem/14403381 Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=190833r1=190832r2=190833view=diff == --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original) +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Mon Sep 16 19:51:29 2013 @@ -1186,15 +1186,9 @@ CompilerInstance::loadModule(SourceLocat case ASTReader::Success: break; -case ASTReader::OutOfDate: { - // The module file is out-of-date. Remove it, then rebuild it. - bool Existed; - llvm::sys::fs::remove(ModuleFileName, Existed); -} -// Fall through to build the module again. - +case ASTReader::OutOfDate: case ASTReader::Missing: { - // The module file is (now) missing. Build it. + // The module file is missing or out-of-date. Build it. // If we don't have a module, we don't know how to build the module file. // Complain and return. ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190844 - Fix const-eval of vector init-lists of a vector.
Author: efriedma Date: Mon Sep 16 23:07:02 2013 New Revision: 190844 URL: http://llvm.org/viewvc/llvm-project?rev=190844view=rev Log: Fix const-eval of vector init-lists of a vector. Like any other type, an init list for a vector can have the same type as the vector itself; handle that case. rdar://problem/14990460 Added: cfe/trunk/test/CodeGenCXX/static-init-4.cpp Modified: cfe/trunk/lib/AST/ExprConstant.cpp Modified: cfe/trunk/lib/AST/ExprConstant.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=190844r1=190843r2=190844view=diff == --- cfe/trunk/lib/AST/ExprConstant.cpp (original) +++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Sep 16 23:07:02 2013 @@ -5262,7 +5262,7 @@ VectorExprEvaluator::VisitInitListExpr(c while (CountElts NumElements) { // Handle nested vector initialization. if (CountInits NumInits - E-getInit(CountInits)-getType()-isExtVectorType()) { + E-getInit(CountInits)-getType()-isVectorType()) { APValue v; if (!EvaluateVector(E-getInit(CountInits), v, Info)) return Error(E); Added: cfe/trunk/test/CodeGenCXX/static-init-4.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/static-init-4.cpp?rev=190844view=auto == --- cfe/trunk/test/CodeGenCXX/static-init-4.cpp (added) +++ cfe/trunk/test/CodeGenCXX/static-init-4.cpp Mon Sep 16 23:07:02 2013 @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 -std=c++11 -S -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s + +typedef __attribute__((vector_size(4*4))) float float32x4_t; +union QDSUnion { float32x4_t q; float s[4]; }; +constexpr float32x4_t a = {1,2,3,4}; +QDSUnion t = {{(a)}}; +// CHECK: @t = global %union.QDSUnion { 4 x float float 1.00e+00, float 2.00e+00, float 3.00e+00, float 4.00e+00 } ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190845 - Don't build extra init lists.
Author: efriedma Date: Mon Sep 16 23:07:04 2013 New Revision: 190845 URL: http://llvm.org/viewvc/llvm-project?rev=190845view=rev Log: Don't build extra init lists. AssignConvertType::IncompatibleVectors means the two types are in fact compatible. :) No testcase; I don't think the extra init list has any actual visible effect other than making the resulting AST dump look a bit strange. Modified: cfe/trunk/lib/Sema/SemaInit.cpp Modified: cfe/trunk/lib/Sema/SemaInit.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=190845r1=190844r2=190845view=diff == --- cfe/trunk/lib/Sema/SemaInit.cpp (original) +++ cfe/trunk/lib/Sema/SemaInit.cpp Mon Sep 16 23:07:04 2013 @@ -901,7 +901,7 @@ void InitListChecker::CheckSubElementTyp if ((ElemType-isRecordType() || ElemType-isVectorType()) SemaRef.CheckSingleAssignmentConstraints(ElemType, ExprRes, !VerifyOnly) - == Sema::Compatible) { + != Sema::Incompatible) { if (ExprRes.isInvalid()) hadError = true; else { ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Add __builtin_convertvector
On Mon, Sep 16, 2013 at 8:41 PM, hfin...@anl.gov hfin...@anl.gov wrote: Comment at: include/clang/AST/Expr.h:3474 @@ +3473,3 @@ +/// vector type of the same arity. +class ConvertVectorExpr : public Expr { // Should this be an ExplicitCastExpr? +private: Eli Friedman wrote: No, this should not be a CastExpr. Okay; I copied this comment from the AsTypeExpr node, so we should probably remove it from there too. Okay. Comment at: lib/CodeGen/CGExprScalar.cpp:1011 @@ +1010,3 @@ + Res = Builder.CreateIntCast(Src, DstTy, InputSigned, conv); +else if (InputSigned) + Res = Builder.CreateSIToFP(Src, DstTy, conv); Eli Friedman wrote: What if the destination type is a bool vector? I think bool vectors are explicitly forbidden (in r190721). You can still use the ext_vector_type attribute to get one. Comment at: lib/Sema/SemaExpr.cpp:4489 @@ +4488,3 @@ + E-getSourceRange()); + + return Owned(new (Context) ConvertVectorExpr(E, DstTy, VK, OK, BuiltinLoc, Eli Friedman wrote: You might want to be careful exactly what vector types we accept here... but I can't think of anything you actually need to check for, so maybe not. I *think* that everything relevant is checked in Sema::BuildExtVectorType and HandleVectorSizeAttr (in SemaType.cpp), and so checking that we have a vector type should be sufficient. I could add an assert here just to be on the safe side; something like assert(Ty-isBuiltinType() !Ty-isBooleanType() (Ty-isIntegerType() || Ty--isRealFloatingType()) ...); Okay. Comment at: lib/Sema/TreeTransform.h:9171 @@ +9170,3 @@ + llvm_unreachable(Cannot transform convertVector expressions yet); +} + Eli Friedman wrote: It would be nice to fix this before committing. Agreed; this is another thing coped from the AsTypeExpr code. How can I test this? This ought to trigger it: typedef int int_v4 __attribute__((vector_size(16))); typedef float float_v4 __attribute__((vector_size(16))); templatetypename T T f(int_v4 x) { return __builtin_convertvector(x, T); } float_v4 g(int_v4 x) { return ffloat_v4(x); } -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] diagnosing flexible array assignments
On Thu, Sep 12, 2013 at 12:58 PM, Aaron Ballman aa...@aaronballman.comwrote: So declarations are a bit more hairy. It seems that GCC allows static initalization of flexible array members as an extension: http://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html We don't support that extension. :) And if we did, we could just not warn on declarations using that extension. Also, the C standard itself shows a declaration of a value type using a flexible array member (granted, it is to show what could be undefined behavior): 6.7.2.1p19 (ISO/IEC 9899:2011): struct s { int n; double d[]; }; struct s t1 = { 0 }; // valid struct s t2 = { 1, { 4.2 }}; // invalid t1.n = 4; // valid t1.d[0] = 4.2; // might be undefined behavior I don't think this indicates anything. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow casts of mixed-size same-arity vectors
On Thu, Sep 12, 2013 at 2:14 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Sep 12, 2013, at 2:08 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Wed, Sep 11, 2013 at 3:34 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Sep 11, 2013, at 3:29 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - - Original Message - Hal — Am I understanding this correctly: vector4float x; vector4int16 y = (vector4int16)x; // this is a conversion vector4int32 z = (vector4int32)x; // this is a bitcast This seems confusing in the extreme. On the other hand, I don't really like the implied int-to-float bitcast semantics. Maybe it would be better to just add a warning for this case. What do you think? bitcast semantics for vector casts are an essential tool for SIMD programming. Essentially all warnings generated would be false positives. Do you specifically mean in OpenCL, or generally? (I've worked on SIMD special function implementations, so I understand the utility). OpenCL doesn't allow writing casts like this at all, if that's what you're asking; OpenCL requires using intrinsics. So is there already an intrinsic that does what I want here? In OpenCL there are ~5000 of them (IIRC), yes. OIC, all of the convert_* functions. I've been thinking about constructing a patch to implement __builtin_vectorconvert(type, value), do you think that is a reasonable approach? You mean, as a generic replacement for intrinsics like _mm_cvtepi32_ps? That seems reasonable. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow casts of mixed-size same-arity vectors
On Wed, Sep 11, 2013 at 3:34 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Sep 11, 2013, at 3:29 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - - Original Message - Hal — Am I understanding this correctly: vector4float x; vector4int16 y = (vector4int16)x; // this is a conversion vector4int32 z = (vector4int32)x; // this is a bitcast This seems confusing in the extreme. On the other hand, I don't really like the implied int-to-float bitcast semantics. Maybe it would be better to just add a warning for this case. What do you think? bitcast semantics for vector casts are an essential tool for SIMD programming. Essentially all warnings generated would be false positives. Do you specifically mean in OpenCL, or generally? (I've worked on SIMD special function implementations, so I understand the utility). OpenCL doesn't allow writing casts like this at all, if that's what you're asking; OpenCL requires using intrinsics. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Restore the sqrt - llvm.sqrt mapping in fast-math mode
On Thu, Sep 12, 2013 at 2:03 PM, Hal Finkel hfin...@anl.gov wrote: Hello, Please review the attached patch which restores the libm sqrt* - @llvm.sqrt* mapping, but only in fast-math mode (specifically, when the UnsafeFPMath or NoNaNsFPMath CodeGen options are enabled). The @llvm.sqrt* intrinsics have slightly different semantics from the libm call, specifically, they are undefined when given a non-zero negative number (the libm calls will always return NaN for any negative number). This mapping was removed in r100613, and replaced with a TODO, but at that time the fast-math flags were not yet implemented. Now that we have these, restoring this mapping is important because it will enable autovectorization of sqrt calls in loops (at least in fast-math mode). This is dangerous, if LangRef is actually correct. People don't associate -ffast-math with my program will crash at random. :) Of course, LangRef is probably overstating the issue. That said, there's actually a general issue here: if we map the LLVM intrinsics to libc functions, and the libc functions set errno, we could break code that depends on errno for non-math calls (e.g. fopen().) -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Restore the sqrt - llvm.sqrt mapping in fast-math mode
On Thu, Sep 12, 2013 at 2:44 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Thu, Sep 12, 2013 at 2:03 PM, Hal Finkel hfin...@anl.gov wrote: Hello, Please review the attached patch which restores the libm sqrt* - @llvm.sqrt* mapping, but only in fast-math mode (specifically, when the UnsafeFPMath or NoNaNsFPMath CodeGen options are enabled). The @llvm.sqrt* intrinsics have slightly different semantics from the libm call, specifically, they are undefined when given a non-zero negative number (the libm calls will always return NaN for any negative number). This mapping was removed in r100613, and replaced with a TODO, but at that time the fast-math flags were not yet implemented. Now that we have these, restoring this mapping is important because it will enable autovectorization of sqrt calls in loops (at least in fast-math mode). This is dangerous, if LangRef is actually correct. People don't associate -ffast-math with my program will crash at random. :) Of course, LangRef is probably overstating the issue. I agree, and the LangRef does indeed say undefined behavior, but I assume that should really mean, returns an undefined value. Do you agree? Well, if we map llvm.sqrt to sqrt and sqrt sets errno, we really do mean undefined behavior... or at least something more that returns an undefined value. That said, there's actually a general issue here: if we map the LLVM intrinsics to libc functions, and the libc functions set errno, we could break code that depends on errno for non-math calls (e.g. fopen().) Perhaps, but I'm not changing that here. For one thing, if the mapping does, in effect, sqrt - llvm.sqrt - sqrt, and only when -fmath-errno=0. Are you worried about cases where the libm functions actually do set errno (even though we have -fmath-errno=0)? I'm not sure our implementation of -fno-math-errno is safe: according to the gcc manual, it isn't equivalent to marking the math functions with attribute((const)). (For example, the gcc manual's definition allows transforming a call to sqrt() into the SSE sqrt instruction, but it doesn't allow hoisting a call to sqrt out of arbitrary loops on a machine where the sqrt() call could set errno.) -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190635 - Fix regression from r190427.
Author: efriedma Date: Thu Sep 12 17:36:24 2013 New Revision: 190635 URL: http://llvm.org/viewvc/llvm-project?rev=190635view=rev Log: Fix regression from r190427. rdar://problem/14970968 Modified: cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/test/SemaObjC/blocks.m Modified: cfe/trunk/lib/Sema/SemaExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=190635r1=190634r2=190635view=diff == --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) +++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Sep 12 17:36:24 2013 @@ -10012,7 +10012,7 @@ ExprResult Sema::ActOnChooseExpr(SourceL void Sema::ActOnBlockStart(SourceLocation CaretLoc, Scope *CurScope) { BlockDecl *Block = BlockDecl::Create(Context, CurContext, CaretLoc); - { + if (LangOpts.CPlusPlus) { Decl *ManglingContextDecl; if (MangleNumberingContext *MCtx = getCurrentMangleNumberContext(Block-getDeclContext(), Modified: cfe/trunk/test/SemaObjC/blocks.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/blocks.m?rev=190635r1=190634r2=190635view=diff == --- cfe/trunk/test/SemaObjC/blocks.m (original) +++ cfe/trunk/test/SemaObjC/blocks.m Thu Sep 12 17:36:24 2013 @@ -216,3 +216,8 @@ void testAnonymousEnumTypes(int arg) { SB = ^{ if (arg) return TDFTE_Value; else return getTDFTE(); }; SB = ^{ if (arg) return getTDFTE(); else return TDFTE_Value; }; } + +static inline void inlinefunc() { + ^{}(); +} +void inlinefunccaller() { inlinefunc(); } ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Restore the sqrt - llvm.sqrt mapping in fast-math mode
On Thu, Sep 12, 2013 at 3:55 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Thu, Sep 12, 2013 at 2:44 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Thu, Sep 12, 2013 at 2:03 PM, Hal Finkel hfin...@anl.gov wrote: Hello, Please review the attached patch which restores the libm sqrt* - @llvm.sqrt* mapping, but only in fast-math mode (specifically, when the UnsafeFPMath or NoNaNsFPMath CodeGen options are enabled). The @llvm.sqrt* intrinsics have slightly different semantics from the libm call, specifically, they are undefined when given a non-zero negative number (the libm calls will always return NaN for any negative number). This mapping was removed in r100613, and replaced with a TODO, but at that time the fast-math flags were not yet implemented. Now that we have these, restoring this mapping is important because it will enable autovectorization of sqrt calls in loops (at least in fast-math mode). This is dangerous, if LangRef is actually correct. People don't associate -ffast-math with my program will crash at random. :) Of course, LangRef is probably overstating the issue. I agree, and the LangRef does indeed say undefined behavior, but I assume that should really mean, returns an undefined value. Do you agree? Well, if we map llvm.sqrt to sqrt and sqrt sets errno, we really do mean undefined behavior... or at least something more that returns an undefined value. Agreed, but this possibility-of-setting-errno problem exists for all LLVM libm-style intrinsics, and so also exists for pow() [for which we currently do this exact kind of replacement whenever -fmath-errno=0]. So, FWIW, this is not without precedent. That said, there's actually a general issue here: if we map the LLVM intrinsics to libc functions, and the libc functions set errno, we could break code that depends on errno for non-math calls (e.g. fopen().) Perhaps, but I'm not changing that here. For one thing, if the mapping does, in effect, sqrt - llvm.sqrt - sqrt, and only when -fmath-errno=0. Are you worried about cases where the libm functions actually do set errno (even though we have -fmath-errno=0)? I'm not sure our implementation of -fno-math-errno is safe: according to the gcc manual, it isn't equivalent to marking the math functions with attribute((const)). (For example, the gcc manual's definition allows transforming a call to sqrt() into the SSE sqrt instruction, but it doesn't allow hoisting a call to sqrt out of arbitrary loops on a machine where the sqrt() call could set errno.) I'm sure it is not safe, for the very reason that you highlight. Nevertheless, this is a long-standing problem, affecting the implementation of -fmath-errno=0 on all systems for which libm math functions actually do set errno, and will require a general solution (fairly orthogonal to this patch). I recommend that we: 1. Commit this change (so that we can autovectorize calls to sqrt()). 2. Have a discussion about how to actually solve this problem: I think that it involves making a specific function attribute for setting errno, and teaching the alias analysis infrastructure to do something sensible with it. Okay. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] [2/6] Convert non-printing characters to their octal sequence before emitting #line directive or __FILE__ macro
Lexer::StringifyWithAddedEscape already exists; it's called raw_ostream::write_escaped. And please rebase this against trunk. -Eli On Wed, Sep 11, 2013 at 12:07 PM, Yunzhong Gao yunzhong_...@playstation.sony.com wrote: This is part of a bigger effort to support foreign characters in file names. This sentence is merely to give a background of this patch, which is only the second of six patches... I am particularly interested in supporting Japanese shift-jis encoding (windows code page 932) on Windows. On these systems, #include directives will use UTF-8 encoding but file names on command prompt will use shift-jis encoding. Both will be translated to UTF-16/unicode before making system calls to the underlying file system. http://llvm-reviews.chandlerc.com/D1291 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] [2/6] Convert non-printing characters to their octal sequence before emitting #line directive or __FILE__ macro
On Wed, Sep 11, 2013 at 1:07 PM, Yunzhong Gao yunzhong_...@playstation.sony.com wrote: Hi Eli, Thanks for the hint. Looks like you already fixed #line directive in r189557. So what is left to fix is just the __FILE__ macro. I have updated my patch here. - Gao. http://llvm-reviews.chandlerc.com/D1291 CHANGE SINCE LAST DIFF http://llvm-reviews.chandlerc.com/D1291?vs=3212id=4207#toc Files: lib/Lex/PPMacroExpansion.cpp test/Preprocessor/line-directive-output.c Index: lib/Lex/PPMacroExpansion.cpp === --- lib/Lex/PPMacroExpansion.cpp +++ lib/Lex/PPMacroExpansion.cpp @@ -1317,8 +1317,9 @@ SmallString128 FN; if (PLoc.isValid()) { FN += PLoc.getFilename(); - Lexer::Stringify(FN); - OS '' FN.str() ''; + OS ''; + OS.write_escaped(FN); + OS ''; } Tok.setKind(tok::string_literal); } else if (II == Ident__DATE__) { Index: test/Preprocessor/line-directive-output.c === --- test/Preprocessor/line-directive-output.c +++ test/Preprocessor/line-directive-output.c @@ -76,3 +76,8 @@ // CHECK: # 50 a\n.c # 50 a\012.c + +// CHECK: # 100 \202\261\202\361\202\311\202\277\202\315.c +// CHECK: filename = \202\261\202\361\202\311\202\277\202\315.c; +# 100 \202\261\202\361\202\311\202\277\202\315.c +const char *filename = __FILE__; \ No newline at end of file Missing newline? Otherwise, LGTM. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190437 - Fix regression from r190382.
Author: efriedma Date: Tue Sep 10 16:10:25 2013 New Revision: 190437 URL: http://llvm.org/viewvc/llvm-project?rev=190437view=rev Log: Fix regression from r190382. Make sure we perform the correct referenced-but-not-used check for static member constants. Fixes bug reported on cfe-commits by Alexey Samsonov. Modified: cfe/trunk/lib/Sema/Sema.cpp cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp Modified: cfe/trunk/lib/Sema/Sema.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=190437r1=190436r2=190437view=diff == --- cfe/trunk/lib/Sema/Sema.cpp (original) +++ cfe/trunk/lib/Sema/Sema.cpp Tue Sep 10 16:10:25 2013 @@ -358,6 +358,15 @@ static bool ShouldRemoveFromUnused(Sema } if (const VarDecl *VD = dyn_castVarDecl(D)) { +// If a variable usable in constant expressions is referenced, +// don't warn if it isn't used: if the value of a variable is required +// for the computation of a constant expression, it doesn't make sense to +// warn even if the variable isn't odr-used. (isReferenced doesn't +// precisely reflect that, but it's a decent approximation.) +if (VD-isReferenced() +VD-isUsableInConstantExpressions(SemaRef-Context)) + return true; + // UnusedFileScopedDecls stores the first declaration. // The declaration may have become definition so check again. const VarDecl *DeclToCheck = VD-getDefinition(); Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=190437r1=190436r2=190437view=diff == --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Sep 10 16:10:25 2013 @@ -1220,14 +1220,6 @@ bool Sema::ShouldWarnIfUnusedFileScopedD if (!isMainFileLoc(*this, VD-getLocation())) return false; -// If a variable usable in constant expressions is referenced, -// don't warn if it isn't used: if the value of a variable is required -// for the computation of a constant expression, it doesn't make sense to -// warn even if the variable isn't odr-used. (isReferenced doesn't -// precisely reflect that, but it's a decent approximation.) -if (VD-isReferenced() VD-isUsableInConstantExpressions(Context)) - return false; - if (Context.DeclMustBeEmitted(VD)) return false; Modified: cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp?rev=190437r1=190436r2=190437view=diff == --- cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp (original) +++ cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp Tue Sep 10 16:10:25 2013 @@ -178,4 +178,13 @@ namespace pr14776 { auto b = X(); // expected-warning {{unused variable 'b'}} } +namespace UndefinedInternalStaticMember { + namespace { +struct X { + static const unsigned x = 3; + int y[x]; +}; + } +} + #endif ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] diagnosing flexible array assignments
On Tue, Sep 10, 2013 at 2:03 PM, Aaron Ballman aa...@aaronballman.comwrote: Currently, if you have a value assignment involving structures containing a flexible array, no diagnostic is emitted. However, since the array members won't be copied as part of the assignment, a diagnostic will help programmers avoid bugs. This patch emits said diagnostic. +def warn_flexible_array_assignment : ExtWarn + assignment of flexible arrays does not copy the array members, + InGroupFlexibleArrayExtensions; I assume this is supposed to be a Warning, not an ExtWarn? Also, I'm not sure putting this into the FlexibleArrayExtensions diagnostic group is the best idea. What sort of mistakes do you expect this to catch in practice? Have you considered warning about the variable declaration rather than the assignment? In your testcase, it's impossible to call foo() without triggering the warning, so it seems better to warn about foo() itself rather than the call. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: r190382 - Make -Wunused warning rules more consistent.
On Tue, Sep 10, 2013 at 5:28 AM, Alexey Samsonov samso...@google.comwrote: Hi Eli, Recently sanitizer-x86_64-linux buildbot which bootstraps Clang started failing with -Wunused warnings which looks like false positives to me: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/848/steps/build%2064-bit%20llvm%20using%20clang/logs/stdio /build/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp:108:23: error: variable 'kNumberOfAccessSizes' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration] static const size_t kNumberOfAccessSizes = 5; This static member is actually used to define array sizes, and in the function code as well. Can this be related to your change? r190382. Thanks for the report. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: r190382 - Make -Wunused warning rules more consistent.
Hopefully r190382 fixes most of the issues. -Eli On Tue, Sep 10, 2013 at 9:39 AM, NAKAMURA Takumi geek4ci...@gmail.comwrote: Eli, could you work for making llvm/clang tree as clean to new-Wunused? I was not certain to prune lines according to new warnings. http://bb.pgr.jp/builders/clang-3stage-x86_64-linux/builds/2184/steps/build/logs/warnings%20%2874%29 2013/9/10 Eli Friedman eli.fried...@gmail.com: Author: efriedma Date: Mon Sep 9 22:05:56 2013 New Revision: 190382 URL: http://llvm.org/viewvc/llvm-project?rev=190382view=rev Log: Make -Wunused warning rules more consistent. This patch does a few different things. This patch improves unused var diags for const vars: we no longer unconditionally suppress diagnostics for const vars, instead only suppressing the diagnostic when the declaration appears to be useful. This patch also makes us more consistently use whether a variable/function is declared in the main file to suppress diagnostics where appropriate. Fixes rdar://problem/14907887. Modified: cfe/trunk/lib/Sema/Sema.cpp cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/test/SemaCXX/Inputs/warn-unused-variables.h cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp Modified: cfe/trunk/lib/Sema/Sema.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=190382r1=190381r2=190382view=diff == --- cfe/trunk/lib/Sema/Sema.cpp (original) +++ cfe/trunk/lib/Sema/Sema.cpp Mon Sep 9 22:05:56 2013 @@ -749,11 +749,7 @@ void Sema::ActOnEndOfTranslationUnit() { if (DiagD-isReferenced()) { Diag(DiagD-getLocation(), diag::warn_unneeded_internal_decl) /*variable*/1 DiagD-getDeclName(); -} else if (SourceMgr.isInMainFile(DiagD-getLocation())) { - // If the declaration is in a header which is included into multiple - // TUs, it will declare one variable per TU, and one of the other - // variables may be used. So, only warn if the declaration is in the - // main file. +} else { Diag(DiagD-getLocation(), diag::warn_unused_variable) DiagD-getDeclName(); } Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=190382r1=190381r2=190382view=diff == --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Sep 9 22:05:56 2013 @@ -1177,6 +1177,14 @@ bool Sema::mightHaveNonExternalLinkage(c return !D-isExternallyVisible(); } +// FIXME: This needs to be refactored; some other isInMainFile users want +// these semantics. +static bool isMainFileLoc(const Sema S, SourceLocation Loc) { + if (S.TUKind != TU_Complete) +return false; + return S.SourceMgr.isInMainFile(Loc); +} + bool Sema::ShouldWarnIfUnusedFileScopedDecl(const DeclaratorDecl *D) const { assert(D); @@ -1196,12 +1204,9 @@ bool Sema::ShouldWarnIfUnusedFileScopedD if (MD-isVirtual() || IsDisallowedCopyOrAssign(MD)) return false; } else { - // 'static inline' functions are used in headers; don't warn. - // Make sure we get the storage class from the canonical declaration, - // since otherwise we will get spurious warnings on specialized - // static template functions. - if (FD-getCanonicalDecl()-getStorageClass() == SC_Static - FD-isInlineSpecified()) + // 'static inline' functions are defined in headers; don't warn. + if (FD-isInlineSpecified() + !isMainFileLoc(*this, FD-getLocation())) return false; } @@ -1209,21 +1214,26 @@ bool Sema::ShouldWarnIfUnusedFileScopedD Context.DeclMustBeEmitted(FD)) return false; } else if (const VarDecl *VD = dyn_castVarDecl(D)) { -// Don't warn on variables of const-qualified or reference type, since their -// values can be used even if though they're not odr-used, and because const -// qualified variables can appear in headers in contexts where they're not -// intended to be used. -// FIXME: Use more principled rules for these exemptions. -if (!VD-isFileVarDecl() || -VD-getType().isConstQualified() || -VD-getType()-isReferenceType() || -Context.DeclMustBeEmitted(VD)) +// Constants and utility variables are defined in headers with internal +// linkage; don't warn. (Unlike functions, there isn't a convenient marker +// like inline.) +if (!isMainFileLoc(*this, VD-getLocation())) + return false; + +// If a variable usable in constant expressions is referenced, +// don't warn if it isn't used: if the value of a variable
Re: r190437 - Fix regression from r190382.
On Tue, Sep 10, 2013 at 3:43 PM, Nick Lewycky nlewy...@google.com wrote: On 10 September 2013 14:10, Eli Friedman eli.fried...@gmail.com wrote: Author: efriedma Date: Tue Sep 10 16:10:25 2013 New Revision: 190437 URL: http://llvm.org/viewvc/llvm-project?rev=190437view=rev Log: Fix regression from r190382. Make sure we perform the correct referenced-but-not-used check for static member constants. Fixes bug reported on cfe-commits by Alexey Samsonov. This breaks -Werror clang bootstrap. Eli, could you help triage? Looking. It looks like just a bunch of unused junk that nobody ever noticed. The main problem looks like this: lib/Transforms/Scalar/EarlyCSE.cpp:77:21: error: unused variable 'value' [-Werror,-Wunused-variable] static const bool value = true; ^lib/Transforms/Scalar/EarlyCSE.cpp:225:23: error: unused variable 'value' [-Werror,-Wunused-variable] static const bool value = true; ^ I'm not sure whether this counts as a true positive or not? Is the problem that the template is never instantiated? Yes: the variable would be referenced if the relevant DenseMap specialization were completely instantiated... I'm still thinking about it, but the best approach might be to just use LLVM_ATTRIBUTE_UNUSED markings. This sort of thing occurs here, lib/Transforms/InstCombine/InstCombinePHI.cpp:608, tools/clang/lib/Sema/SemaType.cpp:256 and lib/Rewrite/Frontend/RewriteModernObjC.cpp:61. The others are various: lib/CodeGen/RegAllocGreedy.cpp:123:28: error: unused variable 'StageName' [-Werror,-Wunused-variable] static const char *const StageName[]; ^ StageName is only used in debug builds. I suppose we can fix it with (void)StageName? Yes, or put it in an #ifndef NDEBUG. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190443 - Add unused markings to suppress warnings.
Author: efriedma Date: Tue Sep 10 17:57:15 2013 New Revision: 190443 URL: http://llvm.org/viewvc/llvm-project?rev=190443view=rev Log: Add unused markings to suppress warnings. trunk clang is a bit more aggressive about emitting unused-declaration warnings, so adjust some AST code to match. Specifically, use LLVM_ATTRIBUTE_UNUSED for declarations which are never supposed to be referenced, and turn references to declarations which are supposed to be referenced into odr-uses. Modified: cfe/trunk/lib/AST/Comment.cpp cfe/trunk/lib/AST/Stmt.cpp Modified: cfe/trunk/lib/AST/Comment.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Comment.cpp?rev=190443r1=190442r2=190443view=diff == --- cfe/trunk/lib/AST/Comment.cpp (original) +++ cfe/trunk/lib/AST/Comment.cpp Tue Sep 10 17:57:15 2013 @@ -42,14 +42,16 @@ good implements_child_begin_end(Comment: return good(); } +LLVM_ATTRIBUTE_UNUSED static inline bad implements_child_begin_end( Comment::child_iterator (Comment::*)() const) { return bad(); } #define ASSERT_IMPLEMENTS_child_begin(function) \ - (void) sizeof(good(implements_child_begin_end(function))) + (void) good(implements_child_begin_end(function)) +LLVM_ATTRIBUTE_UNUSED static inline void CheckCommentASTNodes() { #define ABSTRACT_COMMENT(COMMENT) #define COMMENT(CLASS, PARENT) \ Modified: cfe/trunk/lib/AST/Stmt.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Stmt.cpp?rev=190443r1=190442r2=190443view=diff == --- cfe/trunk/lib/AST/Stmt.cpp (original) +++ cfe/trunk/lib/AST/Stmt.cpp Tue Sep 10 17:57:15 2013 @@ -135,6 +135,7 @@ namespace { template class T good implements_children(children_t T::*) { return good(); } + LLVM_ATTRIBUTE_UNUSED static inline bad implements_children(children_t Stmt::*) { return bad(); } @@ -143,6 +144,7 @@ namespace { template class T good implements_getLocStart(getLocStart_t T::*) { return good(); } + LLVM_ATTRIBUTE_UNUSED static inline bad implements_getLocStart(getLocStart_t Stmt::*) { return bad(); } @@ -151,20 +153,22 @@ namespace { template class T good implements_getLocEnd(getLocEnd_t T::*) { return good(); } + LLVM_ATTRIBUTE_UNUSED static inline bad implements_getLocEnd(getLocEnd_t Stmt::*) { return bad(); } #define ASSERT_IMPLEMENTS_children(type) \ - (void) sizeof(is_good(implements_children(type::children))) + (void) is_good(implements_children(type::children)) #define ASSERT_IMPLEMENTS_getLocStart(type) \ - (void) sizeof(is_good(implements_getLocStart(type::getLocStart))) + (void) is_good(implements_getLocStart(type::getLocStart)) #define ASSERT_IMPLEMENTS_getLocEnd(type) \ - (void) sizeof(is_good(implements_getLocEnd(type::getLocEnd))) + (void) is_good(implements_getLocEnd(type::getLocEnd)) } /// Check whether the various Stmt classes implement their member /// functions. +LLVM_ATTRIBUTE_UNUSED static inline void check_implementations() { #define ABSTRACT_STMT(type) #define STMT(type, base) \ ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190445 - Remove unused variable.
Author: efriedma Date: Tue Sep 10 18:00:03 2013 New Revision: 190445 URL: http://llvm.org/viewvc/llvm-project?rev=190445view=rev Log: Remove unused variable. Modified: cfe/trunk/lib/Rewrite/Frontend/RewriteModernObjC.cpp Modified: cfe/trunk/lib/Rewrite/Frontend/RewriteModernObjC.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Rewrite/Frontend/RewriteModernObjC.cpp?rev=190445r1=190444r2=190445view=diff == --- cfe/trunk/lib/Rewrite/Frontend/RewriteModernObjC.cpp (original) +++ cfe/trunk/lib/Rewrite/Frontend/RewriteModernObjC.cpp Tue Sep 10 18:00:03 2013 @@ -58,7 +58,6 @@ namespace { BLOCK_IS_GLOBAL = (1 28), BLOCK_HAS_DESCRIPTOR =(1 29) }; -static const int OBJC_ABI_VERSION = 7; Rewriter Rewrite; DiagnosticsEngine Diags; ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] diagnosing flexible array assignments
On Tue, Sep 10, 2013 at 2:33 PM, Aaron Ballman aa...@aaronballman.comwrote: On Tue, Sep 10, 2013 at 5:21 PM, Eli Friedman eli.fried...@gmail.com wrote: On Tue, Sep 10, 2013 at 2:03 PM, Aaron Ballman aa...@aaronballman.com wrote: Currently, if you have a value assignment involving structures containing a flexible array, no diagnostic is emitted. However, since the array members won't be copied as part of the assignment, a diagnostic will help programmers avoid bugs. This patch emits said diagnostic. +def warn_flexible_array_assignment : ExtWarn + assignment of flexible arrays does not copy the array members, + InGroupFlexibleArrayExtensions; I assume this is supposed to be a Warning, not an ExtWarn? Yes, thanks for pointing that out. Also, I'm not sure putting this into the FlexibleArrayExtensions diagnostic group is the best idea. I wasn't certain either, but it was the closest existing group. Would it make more sense to add a new group for it? I think so. What sort of mistakes do you expect this to catch in practice? Simple programmer mistakes, mostly. It also nicely covers a CERT secure C coding rule: https://www.securecoding.cert.org/confluence/display/seccode/MEM33-C.++Allocate+and+copy+structures+containing+flexible+array+members+dynamically Okay. Do you have any idea what the false positive rate is? Have you considered warning about the variable declaration rather than the assignment? In your testcase, it's impossible to call foo() without triggering the warning, so it seems better to warn about foo() itself rather than the call. Hmmm, would there ever be a case where it would make sense to declare a structure with a flexible array member as a value type?The only situation I could think of would be overlaying the value type with stack-allocated memory in some sort of bizarre union type punning scenario. So I'm thinking that may be a better approach than checking on assignment, unless there are intermediary ways you could get this assignment to happen in C. Well, you can write something like *a = *b;, which doesn't involve declaring a variable... -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] diagnosing flexible array assignments
On Tue, Sep 10, 2013 at 2:50 PM, Aaron Ballman aa...@aaronballman.comwrote: On Tue, Sep 10, 2013 at 5:40 PM, Eli Friedman eli.fried...@gmail.com wrote: On Tue, Sep 10, 2013 at 2:33 PM, Aaron Ballman aa...@aaronballman.com wrote: On Tue, Sep 10, 2013 at 5:21 PM, Eli Friedman eli.fried...@gmail.com wrote: On Tue, Sep 10, 2013 at 2:03 PM, Aaron Ballman aa...@aaronballman.com wrote: Currently, if you have a value assignment involving structures containing a flexible array, no diagnostic is emitted. However, since the array members won't be copied as part of the assignment, a diagnostic will help programmers avoid bugs. This patch emits said diagnostic. +def warn_flexible_array_assignment : ExtWarn + assignment of flexible arrays does not copy the array members, + InGroupFlexibleArrayExtensions; I assume this is supposed to be a Warning, not an ExtWarn? Yes, thanks for pointing that out. Also, I'm not sure putting this into the FlexibleArrayExtensions diagnostic group is the best idea. I wasn't certain either, but it was the closest existing group. Would it make more sense to add a new group for it? I think so. Any complaints with naming the group flexible-arrays? That's sort of vague; maybe flexible-array-slicing? What sort of mistakes do you expect this to catch in practice? Simple programmer mistakes, mostly. It also nicely covers a CERT secure C coding rule: https://www.securecoding.cert.org/confluence/display/seccode/MEM33-C.++Allocate+and+copy+structures+containing+flexible+array+members+dynamically Okay. Do you have any idea what the false positive rate is? I would expect it to be quite low, but have nothing concrete to back that up. If the warning turns out to be chatty for some reason, it could be tweaked easily enough (esp with a separate diagnostic group). Okay. Have you considered warning about the variable declaration rather than the assignment? In your testcase, it's impossible to call foo() without triggering the warning, so it seems better to warn about foo() itself rather than the call. Hmmm, would there ever be a case where it would make sense to declare a structure with a flexible array member as a value type?The only situation I could think of would be overlaying the value type with stack-allocated memory in some sort of bizarre union type punning scenario. So I'm thinking that may be a better approach than checking on assignment, unless there are intermediary ways you could get this assignment to happen in C. Well, you can write something like *a = *b;, which doesn't involve declaring a variable... True. Assignment would be the catch-all way to do it, since that's what causes the slicing behavior. Perhaps both declaration and assignment diagnostics would make sense? Declarations like that are definitively bizarre, but assignment still misbehaves. The CERT page classifies the mistakes into three categories; covering all of them seems reasonable. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Support __builtin_ms_va_list.
On Tue, Sep 10, 2013 at 2:25 PM, Charles Davis cdavi...@gmail.com wrote: @rsmith wrote: This seems very weird to me. Can you explain a bit about the background for this? Certainly. Is this to support writing variadic __attribute__((ms_abi)) functions when not targeting Win64? Yes. Some software in the wild (Wine, for instance) has variadic `ms_abi` functions. (I wouldn't be surprised, frankly, if this were originally implemented in GCC at the behest of Wine. I know that the `ms_hook_prologue` attribute was.) How is this is supposed to work when we *are* targeting Win64 directly? Exactly the same as a normal `va_list`. I even added a test for this. (Do we get a corresponding __builtin_sysv_va_list to go in the opposite direction, for instance? Are the two types the same in that case?) We don't. Obviously, this is something GCC overlooked--probably because it was implemented specifically so Wine could use it (see above). (To be honest, I contemplated this myself before submitting. I wanted to add an orthogonal extension for `va_list`s in a `sysv_abi` function. But I decided that was better suited for another patch--this one's big enough as it is.) Comment at: include/clang/Serialization/ASTBitCodes.h:1343 @@ -1340,1 +1342,3 @@ + EXPR_LAMBDA,// LambdaExpr + EXPR_MS_VA_ARG // VAArgExpr (with isMicrosoftABI() true). }; Richard Smith wrote: Why are you using a separate StmtCode here, rather than just serializing the flag normally? I think it was because I didn't want to break backwards compatibility with the existing encoding of `clang::VAArgExpr` in a PCH/module. (That's going to become important later when modules are ready for primetime.) But, if you want me to serialize the bit normally, I can go do that. We don't care about PCH backward compatibility at the moment, and that's not likely to change in the near future. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190462 - Remove unused class.
Author: efriedma Date: Tue Sep 10 19:37:06 2013 New Revision: 190462 URL: http://llvm.org/viewvc/llvm-project?rev=190462view=rev Log: Remove unused class. Modified: cfe/trunk/lib/Sema/SemaType.cpp Modified: cfe/trunk/lib/Sema/SemaType.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaType.cpp?rev=190462r1=190461r2=190462view=diff == --- cfe/trunk/lib/Sema/SemaType.cpp (original) +++ cfe/trunk/lib/Sema/SemaType.cpp Tue Sep 10 19:37:06 2013 @@ -235,26 +235,6 @@ namespace { savedAttrs.back()-setNext(0); } }; - - /// Basically std::pair except that we really want to avoid an - /// implicit operator= for safety concerns. It's also a minor - /// link-time optimization for this to be a private type. - struct AttrAndList { -/// The attribute. -AttributeList first; - -/// The head of the list the attribute is currently in. -AttributeList *second; - -AttrAndList(AttributeList attr, AttributeList *head) - : first(attr), second(head) {} - }; -} - -namespace llvm { - template struct isPodLikeAttrAndList { -static const bool value = true; - }; } static void spliceAttrIntoList(AttributeList attr, AttributeList *head) { ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190463 - Get rid of unused isPodLike definition.
Author: efriedma Date: Tue Sep 10 19:37:10 2013 New Revision: 190463 URL: http://llvm.org/viewvc/llvm-project?rev=190463view=rev Log: Get rid of unused isPodLike definition. Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=190463r1=190462r2=190463view=diff == --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Tue Sep 10 19:37:10 2013 @@ -417,8 +417,6 @@ template struct DenseMapInfoObjCSumm } }; -template -struct isPodLikeObjCSummaryKey { static const bool value = true; }; } // end llvm namespace namespace { ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: r190437 - Fix regression from r190382.
On Tue, Sep 10, 2013 at 4:07 PM, Eli Friedman eli.fried...@gmail.comwrote: On Tue, Sep 10, 2013 at 3:43 PM, Nick Lewycky nlewy...@google.com wrote: On 10 September 2013 14:10, Eli Friedman eli.fried...@gmail.com wrote: Author: efriedma Date: Tue Sep 10 16:10:25 2013 New Revision: 190437 URL: http://llvm.org/viewvc/llvm-project?rev=190437view=rev Log: Fix regression from r190382. Make sure we perform the correct referenced-but-not-used check for static member constants. Fixes bug reported on cfe-commits by Alexey Samsonov. This breaks -Werror clang bootstrap. Eli, could you help triage? Looking. It looks like just a bunch of unused junk that nobody ever noticed. I think I got everything as of r190463; please confirm. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190482 - Fix is_trivially_constructible preconditions.
Author: efriedma Date: Tue Sep 10 21:53:02 2013 New Revision: 190482 URL: http://llvm.org/viewvc/llvm-project?rev=190482view=rev Log: Fix is_trivially_constructible preconditions. Fixes a crash in cases where the first argument was an incomplete type or an uninstantiated template type. rdar://problem/14938471 Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp cfe/trunk/test/SemaCXX/type-traits.cpp Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=190482r1=190481r2=190482view=diff == --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original) +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Tue Sep 10 21:53:02 2013 @@ -3522,24 +3522,24 @@ static bool evaluateTypeTrait(Sema S, T 1 1 1 (int)Args.size(); return false; } - -bool SawVoid = false; + +// Precondition: T and all types in the parameter pack Args shall be +// complete types, (possibly cv-qualified) void, or arrays of +// unknown bound. for (unsigned I = 0, N = Args.size(); I != N; ++I) { - if (Args[I]-getType()-isVoidType()) { -SawVoid = true; + QualType ArgTy = Args[I]-getType(); + if (ArgTy-isVoidType() || ArgTy-isIncompleteArrayType()) continue; - } - - if (!Args[I]-getType()-isIncompleteType() -S.RequireCompleteType(KWLoc, Args[I]-getType(), + + if (S.RequireCompleteType(KWLoc, ArgTy, diag::err_incomplete_type_used_in_type_trait_expr)) return false; } - -// If any argument was 'void', of course it won't type-check. -if (SawVoid) + +// Make sure the first argument is a complete type. +if (Args[0]-getType()-isIncompleteType()) return false; - + SmallVectorOpaqueValueExpr, 2 OpaqueArgExprs; SmallVectorExpr *, 2 ArgExprs; ArgExprs.reserve(Args.size() - 1); Modified: cfe/trunk/test/SemaCXX/type-traits.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/type-traits.cpp?rev=190482r1=190481r2=190482view=diff == --- cfe/trunk/test/SemaCXX/type-traits.cpp (original) +++ cfe/trunk/test/SemaCXX/type-traits.cpp Tue Sep 10 21:53:02 2013 @@ -1571,7 +1571,7 @@ templatetypename T struct DerivedB : B templatetypename T struct CrazyDerived : T { }; -class class_forward; // expected-note {{forward declaration of 'class_forward'}} +class class_forward; // expected-note 2 {{forward declaration of 'class_forward'}} template typename Base, typename Derived void isBaseOfT() { @@ -1770,6 +1770,8 @@ void is_trivial() { int arr[F(__is_trivial(cvoid))]; } } +templatetypename T struct TriviallyConstructibleTemplate {}; + void trivial_checks() { { int arr[T(__is_trivially_copyable(int))]; } @@ -1848,6 +1850,11 @@ void trivial_checks() { int arr[F((__is_trivially_constructible(ExtDefaulted, ExtDefaulted )))]; } + { int arr[T((__is_trivially_constructible(TriviallyConstructibleTemplateint)))]; } + { int arr[F((__is_trivially_constructible(class_forward)))]; } // expected-error {{incomplete type 'class_forward' used in type trait expression}} + { int arr[F((__is_trivially_constructible(class_forward[])))]; } + { int arr[F((__is_trivially_constructible(void)))]; } + { int arr[T((__is_trivially_assignable(int, int)))]; } { int arr[T((__is_trivially_assignable(int, int)))]; } { int arr[T((__is_trivially_assignable(int, int)))]; } ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190483 - Fix test.
Author: efriedma Date: Tue Sep 10 22:49:30 2013 New Revision: 190483 URL: http://llvm.org/viewvc/llvm-project?rev=190483view=rev Log: Fix test. Modified: cfe/trunk/test/CodeGenCXX/attr-cleanup.cpp Modified: cfe/trunk/test/CodeGenCXX/attr-cleanup.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/attr-cleanup.cpp?rev=190483r1=190482r2=190483view=diff == --- cfe/trunk/test/CodeGenCXX/attr-cleanup.cpp (original) +++ cfe/trunk/test/CodeGenCXX/attr-cleanup.cpp Tue Sep 10 22:49:30 2013 @@ -5,7 +5,7 @@ namespace N { } int main(void) { - // CHECK: call void @_ZN1N4freeEPv(i8* %0) + // CHECK: call void @_ZN1N4freeEPv void *fp __attribute__((cleanup(N::free))); return 0; } ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190484 - volatile types are not trivially copyable.
Author: efriedma Date: Tue Sep 10 22:49:34 2013 New Revision: 190484 URL: http://llvm.org/viewvc/llvm-project?rev=190484view=rev Log: volatile types are not trivially copyable. PR17123. Modified: cfe/trunk/lib/AST/Type.cpp cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/test/SemaCXX/type-traits.cpp Modified: cfe/trunk/lib/AST/Type.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=190484r1=190483r2=190484view=diff == --- cfe/trunk/lib/AST/Type.cpp (original) +++ cfe/trunk/lib/AST/Type.cpp Tue Sep 10 22:49:34 2013 @@ -1097,15 +1097,18 @@ bool QualType::isTriviallyCopyableType(A } } - // C++0x [basic.types]p9 + // C++11 [basic.types]p9 // Scalar types, trivially copyable class types, arrays of such types, and - // cv-qualified versions of these types are collectively called trivial - // types. + // non-volatile const-qualified versions of these types are collectively + // called trivially copyable types. QualType CanonicalType = getCanonicalType(); if (CanonicalType-isDependentType()) return false; + if (CanonicalType.isVolatileQualified()) +return false; + // Return false for incomplete types after skipping any incomplete array types // which are expressly allowed by the standard and thus our API. if (CanonicalType-isIncompleteType()) Modified: cfe/trunk/lib/Sema/SemaChecking.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=190484r1=190483r2=190484view=diff == --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Sep 10 22:49:34 2013 @@ -1038,7 +1038,8 @@ ExprResult Sema::SemaAtomicOpsOverloaded return ExprError(); } - if (!IsC11 !AtomTy.isTriviallyCopyableType(Context)) { + if (!IsC11 !AtomTy.isTriviallyCopyableType(Context) + !AtomTy-isScalarType()) { // For GNU atomics, require a trivially-copyable type. This is not part of // the GNU atomics specification, but we enforce it for sanity. Diag(DRE-getLocStart(), diag::err_atomic_op_needs_trivial_copy) Modified: cfe/trunk/test/SemaCXX/type-traits.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/type-traits.cpp?rev=190484r1=190483r2=190484view=diff == --- cfe/trunk/test/SemaCXX/type-traits.cpp (original) +++ cfe/trunk/test/SemaCXX/type-traits.cpp Tue Sep 10 22:49:34 2013 @@ -1072,6 +1072,9 @@ void is_trivially_copyable2() int t31[F(__is_trivially_copyable(SuperNonTrivialStruct))]; int t32[F(__is_trivially_copyable(NonTCStruct))]; int t33[F(__is_trivially_copyable(ExtDefaulted))]; + + int t34[T(__is_trivially_copyable(const int))]; + int t35[F(__is_trivially_copyable(volatile int))]; } struct CStruct { ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: r190353 - Fix a crash introduced in r189828.
On Mon, Sep 9, 2013 at 2:07 PM, Matt Beaumont-Gay matthe...@google.comwrote: Author: matthewbg Date: Mon Sep 9 16:07:58 2013 New Revision: 190353 URL: http://llvm.org/viewvc/llvm-project?rev=190353view=rev Log: Fix a crash introduced in r189828. The predicates in CXXRecordDecl which test various properties of special members can't be called on incomplete decls. Modified: cfe/trunk/lib/Analysis/CFG.cpp cfe/trunk/test/Analysis/dtor.cpp Modified: cfe/trunk/lib/Analysis/CFG.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=190353r1=190352r2=190353view=diff == --- cfe/trunk/lib/Analysis/CFG.cpp (original) +++ cfe/trunk/lib/Analysis/CFG.cpp Mon Sep 9 16:07:58 2013 @@ -3133,7 +3133,7 @@ CFGBlock *CFGBuilder::VisitCXXDeleteExpr DTy = DTy.getNonReferenceType(); CXXRecordDecl *RD = Context-getBaseElementType(DTy)-getAsCXXRecordDecl(); if (RD) { -if (!RD-hasTrivialDestructor()) +if (RD-isCompleteDefinition() !RD-hasTrivialDestructor()) appendDeleteDtor(Block, RD, DE); } Modified: cfe/trunk/test/Analysis/dtor.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dtor.cpp?rev=190353r1=190352r2=190353view=diff == --- cfe/trunk/test/Analysis/dtor.cpp (original) +++ cfe/trunk/test/Analysis/dtor.cpp Mon Sep 9 16:07:58 2013 @@ -431,3 +431,8 @@ namespace PseudoDtor { clang_analyzer_eval(true); // expected-warning{{TRUE}} } } + +namespace Incomplete { + class Foo; // expected-note{{forward declaration}} + void f(Foo *foo) { delete foo; } // expected-warning{{deleting pointer to incomplete type}} +} Is this the same as PR17162? -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: Fix for ICE in clang due to infinite loop.
On Mon, Sep 9, 2013 at 5:08 AM, MAYUR PANDEY mayu...@samsung.com wrote: Hi, Attached is the fix for Compiler crash caused by infinite loop when we try to compile the following code. struct A; struct B { B (A const ); B (B ); }; struct A { A (B); }; B f (B const b) { return b; } The patch is missing a testcase. Patches should generally include a corresponding change to clang/test/. (In this case, somewhere in clang/test/SemaCXX/ is probably appropriate.) This is not the right approach to fixing this issue. One, we're supposed to detect issues like this inside the InitializationSequence constructor, when we actually check the initialization. Not handling this correctly will break overload resolution. Second, using a global map like this is rather suspect: I'm not sure it will cause issues, but introducing global state for something like this makes it more difficult to understand the code. My guess is that the right fix is somewhere in SemaOverload.cpp: we're probably mishandling this case in the user-defined conversion code somewhere. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: PR16752[PATCH]: 'mode' attribute for unusual targets doesn't work properly.
LGTM. -Eli On Mon, Sep 9, 2013 at 5:04 AM, Stepan Dyatkovskiy stpwo...@narod.ruwrote: Since I had split the patch and committed only ASTContext and TargetInfo getIntTypeByWidth and friends, there is the second patch, that fixes PR16752 itself. Please, find it in attachment for review. -Stepan. Eli Friedman wrote: On Tue, Sep 3, 2013 at 2:16 AM, Stepan Dyatkovskiy stpwo...@narod.ru mailto:stpwo...@narod.ru wrote: Hi Eli, Sorry for latency. As you remember this patch should correct 'mode' attr implementation. You proposed to use generic way of type detection for each target: just scan for target types and select one with suitable width. Unfortunately I can't commit patch with generic implementation. I got test failure for clang. And I expect more failures for another targets. The reason is next. The target could still keep mode unsupported even if it has type of suitable width. I mean the next. For example this string should cause error for i686 machines (128 bit float): typedef float f128ibm __attribute__ ((mode (TF))); But in case of generic approach for all targets it would be passed. In this case virtual functions allows to implement exceptions. The generic implementation is still essentially correct; the the issue is that getLongDoubleWidth() isn't actually the right value to check. It returns sizeof(long double) * 8, not the actual width of the underlying float format. You should be able to work around this by checking getLongDoubleFormat() instead of getLongDoubleWidth(). In this case 'getIntTypeByWidth' may be a bit confusing name. Instead it is supposed to return type for some particular mode, not by width. Something like getInt/RealTypeForMode(width, sign). Though, currently I kept the original name. If you want to change it, that's fine. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: patch: avoid ubsan errors in implicit copy constructors
On Mon, Sep 9, 2013 at 3:03 PM, Nick Lewycky nlewy...@google.com wrote: The attached patch disables the bool and enum sanitizers when emitting the implicitly-defined copy constructor. To start with an example: struct X { X(); X(const X); }; struct Y { X x; bool b; }; if you don't initialize Y::b then try to copy an object of Y type, ubsan will complain. Technically, with the standard as written, ubsan is correct. However, this is a useful thing to do -- you may have a discriminator which decides which elements are not interesting and therefore never initialize or read them. Secondly, it's a departure from the rules in C, making well-defined C code have undefined behaviour in C++ (structs are never trap values, see C99 6.2.6.1p6). Thirdly, it's checked incompletely right now -- if you make subtle changes (f.e. add an int i; member to Y) ubsan will stop complaining. The semantic I'm implementing is as if the implicit copy constructor is copying the value representation (the bytes) not the object representation (the meaning of those bytes). Does copy assignment have the same issue? Being more consistent seems like a good idea. And I agree that performing this checking in defaulted copy-constructors probably isn't productive. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190382 - Make -Wunused warning rules more consistent.
Author: efriedma Date: Mon Sep 9 22:05:56 2013 New Revision: 190382 URL: http://llvm.org/viewvc/llvm-project?rev=190382view=rev Log: Make -Wunused warning rules more consistent. This patch does a few different things. This patch improves unused var diags for const vars: we no longer unconditionally suppress diagnostics for const vars, instead only suppressing the diagnostic when the declaration appears to be useful. This patch also makes us more consistently use whether a variable/function is declared in the main file to suppress diagnostics where appropriate. Fixes rdar://problem/14907887. Modified: cfe/trunk/lib/Sema/Sema.cpp cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/test/SemaCXX/Inputs/warn-unused-variables.h cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp Modified: cfe/trunk/lib/Sema/Sema.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=190382r1=190381r2=190382view=diff == --- cfe/trunk/lib/Sema/Sema.cpp (original) +++ cfe/trunk/lib/Sema/Sema.cpp Mon Sep 9 22:05:56 2013 @@ -749,11 +749,7 @@ void Sema::ActOnEndOfTranslationUnit() { if (DiagD-isReferenced()) { Diag(DiagD-getLocation(), diag::warn_unneeded_internal_decl) /*variable*/1 DiagD-getDeclName(); -} else if (SourceMgr.isInMainFile(DiagD-getLocation())) { - // If the declaration is in a header which is included into multiple - // TUs, it will declare one variable per TU, and one of the other - // variables may be used. So, only warn if the declaration is in the - // main file. +} else { Diag(DiagD-getLocation(), diag::warn_unused_variable) DiagD-getDeclName(); } Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=190382r1=190381r2=190382view=diff == --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Sep 9 22:05:56 2013 @@ -1177,6 +1177,14 @@ bool Sema::mightHaveNonExternalLinkage(c return !D-isExternallyVisible(); } +// FIXME: This needs to be refactored; some other isInMainFile users want +// these semantics. +static bool isMainFileLoc(const Sema S, SourceLocation Loc) { + if (S.TUKind != TU_Complete) +return false; + return S.SourceMgr.isInMainFile(Loc); +} + bool Sema::ShouldWarnIfUnusedFileScopedDecl(const DeclaratorDecl *D) const { assert(D); @@ -1196,12 +1204,9 @@ bool Sema::ShouldWarnIfUnusedFileScopedD if (MD-isVirtual() || IsDisallowedCopyOrAssign(MD)) return false; } else { - // 'static inline' functions are used in headers; don't warn. - // Make sure we get the storage class from the canonical declaration, - // since otherwise we will get spurious warnings on specialized - // static template functions. - if (FD-getCanonicalDecl()-getStorageClass() == SC_Static - FD-isInlineSpecified()) + // 'static inline' functions are defined in headers; don't warn. + if (FD-isInlineSpecified() + !isMainFileLoc(*this, FD-getLocation())) return false; } @@ -1209,21 +1214,26 @@ bool Sema::ShouldWarnIfUnusedFileScopedD Context.DeclMustBeEmitted(FD)) return false; } else if (const VarDecl *VD = dyn_castVarDecl(D)) { -// Don't warn on variables of const-qualified or reference type, since their -// values can be used even if though they're not odr-used, and because const -// qualified variables can appear in headers in contexts where they're not -// intended to be used. -// FIXME: Use more principled rules for these exemptions. -if (!VD-isFileVarDecl() || -VD-getType().isConstQualified() || -VD-getType()-isReferenceType() || -Context.DeclMustBeEmitted(VD)) +// Constants and utility variables are defined in headers with internal +// linkage; don't warn. (Unlike functions, there isn't a convenient marker +// like inline.) +if (!isMainFileLoc(*this, VD-getLocation())) + return false; + +// If a variable usable in constant expressions is referenced, +// don't warn if it isn't used: if the value of a variable is required +// for the computation of a constant expression, it doesn't make sense to +// warn even if the variable isn't odr-used. (isReferenced doesn't +// precisely reflect that, but it's a decent approximation.) +if (VD-isReferenced() VD-isUsableInConstantExpressions(Context)) + return false; + +if (Context.DeclMustBeEmitted(VD)) return false; if (VD-isStaticDataMember() VD-getTemplateSpecializationKind() == TSK_ImplicitInstantiation) return false; - } else { return false; } Modified: cfe/trunk/test/SemaCXX/Inputs/warn-unused-variables.h URL:
Re: [PATCH] Fix missing source location in CXXTemporaryObjectExpr.
On Fri, Sep 6, 2013 at 2:50 AM, Enea Zaffanella zaffane...@cs.unipr.itwrote: On 09/05/2013 10:12 PM, Eli Friedman wrote: On Thu, Sep 5, 2013 at 12:34 AM, Enea Zaffanella zaffane...@cs.unipr.it mailto:zaffane...@cs.unipr.it** wrote: [...] To summarize, I see two options: (a) rename getParenRange() as getParenOrBraceRange(); (b) have two different methods, getParenRange() and getBraceRange(), to be used based on the value of a boolean flag. Question: is it the case that bool CXXConstructExpr::__**isListInitialization() is such an appropriate flag to be used here? Yes. I would probably opt for (b). Do you see a third option which is better? I don't really see any other options; I guess I would lean towards option A, and let the users branch if they care about the difference, but either way works. Please find attached a revised patch implementing option A. OK to commit? LGTM. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Trapping math and libfuncs improvements
On Fri, Sep 6, 2013 at 3:32 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Fri, Sep 6, 2013 at 8:27 AM, Hal Finkel hfin...@anl.gov wrote: Ben, Chad, et al., The attached patch cleans up the LIBBUILTIN libm function definitions, and improves the way in which -ftrapping-math is handled in a complementary way. Specifically, this patch: 1. The libm LIBBUILTIN definitions are synchronized with the __builtin_* definitions for libm functions. We currently have __builtin_* definitions for many more functions than those for which we have LIBBUILTIN definitions. This unnecessarily pessimizes code generation around many of the remaining libm functions. 2. Many of these functions can be marked as readnone so long as we can neglect floating point exceptions. I've added a new specifier 'T', which like 'e' for errno, marks the builtin as const so long as trapping math is disabled. I've added a TrappingMath lang option to track this. 3. Disables -ftrapping-math by default. This is currently enabled by default (following gcc), but this is silly. No system of which I'm aware actually traps on floating-point exceptions by default (some system-specific mechanism is necessary in order to enable SIGFPE generation). In addition, in Clang, we currently don't support fenv access. As a result, we can turn this off by default (and enable better code generation around the functions for which we have LIBBUILTIN definitions). 4. Updates the CodeGen/libcall-declarations.c test case. As a side effect, the existing libm LIBBUILTIN definitions that were missing the 'n' (nothrow) specifier now have it. For clarity, please split the patch into four separate patches: reformatting, adding missing 'n' specifiers, the trapping-math-related changes, and adding new LIBBUILTIN definitions (preferably in that order). As it is, the Builtins.def changes are completely unreviewable. Unfortunately, because almost all of the LIBBUILTIN's in the patch are new, I don't think that splitting the patch will help that :( -- FWIW, I think that reviewing the Builtins.def changes means, in practice (either trusting me, or) going through the documentation on each of those functions to make sure that the flags are correct. I don't think that splitting the patch makes that easier either. Nevertheless, I'll split the changes and re-post as separate patches. Maybe I'm wrong ;) I didn't try to count how many were new... it looked like you were changing a bunch of existing ones, but if the balance is as you say, it might not be as useful as I'd hope. Your changes could be substantially simplified if we don't allow enabling -ftrapping-math; as you've noted, we don't honor it anyway. What do you think? I considered that, but I think that we might as well not make the situation worse for users who do enable trapping math for debugging purposes (as I do myself on occasion). Debugging how, exactly? With these changes, we'll mark a lot more libm functions that could trap when exceptions are enabled as readnone (including some which never set errno), and if we ignore -ftrapping-math completely, we'll provide no way of indicating that those functions have side effects when we're interested in floating point exceptions. Providing -ftrapping-math as an option is deceptive at best because clang will break code which expects us to honor -ftrapping-math, even if it doesn't even use any library calls. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Trapping math and libfuncs improvements
On Fri, Sep 6, 2013 at 3:53 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Fri, Sep 6, 2013 at 3:32 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Fri, Sep 6, 2013 at 8:27 AM, Hal Finkel hfin...@anl.gov wrote: Ben, Chad, et al., The attached patch cleans up the LIBBUILTIN libm function definitions, and improves the way in which -ftrapping-math is handled in a complementary way. Specifically, this patch: 1. The libm LIBBUILTIN definitions are synchronized with the __builtin_* definitions for libm functions. We currently have __builtin_* definitions for many more functions than those for which we have LIBBUILTIN definitions. This unnecessarily pessimizes code generation around many of the remaining libm functions. 2. Many of these functions can be marked as readnone so long as we can neglect floating point exceptions. I've added a new specifier 'T', which like 'e' for errno, marks the builtin as const so long as trapping math is disabled. I've added a TrappingMath lang option to track this. 3. Disables -ftrapping-math by default. This is currently enabled by default (following gcc), but this is silly. No system of which I'm aware actually traps on floating-point exceptions by default (some system-specific mechanism is necessary in order to enable SIGFPE generation). In addition, in Clang, we currently don't support fenv access. As a result, we can turn this off by default (and enable better code generation around the functions for which we have LIBBUILTIN definitions). 4. Updates the CodeGen/libcall-declarations.c test case. As a side effect, the existing libm LIBBUILTIN definitions that were missing the 'n' (nothrow) specifier now have it. For clarity, please split the patch into four separate patches: reformatting, adding missing 'n' specifiers, the trapping-math-related changes, and adding new LIBBUILTIN definitions (preferably in that order). As it is, the Builtins.def changes are completely unreviewable. Unfortunately, because almost all of the LIBBUILTIN's in the patch are new, I don't think that splitting the patch will help that :( -- FWIW, I think that reviewing the Builtins.def changes means, in practice (either trusting me, or) going through the documentation on each of those functions to make sure that the flags are correct. I don't think that splitting the patch makes that easier either. Nevertheless, I'll split the changes and re-post as separate patches. Maybe I'm wrong ;) I didn't try to count how many were new... it looked like you were changing a bunch of existing ones, but if the balance is as you say, it might not be as useful as I'd hope. There may have been a few changes to the existing ones, I'll go over them again, and try to make that more clear in the splitting. Your changes could be substantially simplified if we don't allow enabling -ftrapping-math; as you've noted, we don't honor it anyway. What do you think? I considered that, but I think that we might as well not make the situation worse for users who do enable trapping math for debugging purposes (as I do myself on occasion). Debugging how, exactly? If I have an application that is producing NaNs, Inf, etc. where that is not expected, I can add something like this: #include fenv.h #if defined(__i386__) defined(__SSE__) #include xmmintrin.h #endif ... #if defined(FE_NOMASK_ENV) !defined(__INTEL_COMPILER) fesetenv(FE_NOMASK_ENV); fedisableexcept(/* FE_OVERFLOW | */ FE_UNDERFLOW | FE_INEXACT); #elif defined(__i386__) defined(__SSE__) _MM_SET_EXCEPTION_MASK(_MM_GET_EXCEPTION_MASK() ~(_MM_MASK_OVERFLOW|_MM_MASK_INVALID|_MM_MASK_DIV_ZERO)); #endif to the beginning of the code. Then I should get a much better idea (based on when the SIGFPE is delivered) of where the problem is. Ah, interesting. With these changes, we'll mark a lot more libm functions that could trap when exceptions are enabled as readnone (including some which never set errno), and if we ignore -ftrapping-math completely, we'll provide no way of indicating that those functions have side effects when we're interested in floating point exceptions. Providing -ftrapping-math as an option is deceptive at best because clang will break code which expects us to honor -ftrapping-math, even if it doesn't even use any library calls. Fair enough. I'll leave that out for now, and we can always revisit it later if someone cares. As a separate patch, we could issue a warning (or an error?) if -ftrapping-math is provided? Yes, that's a good idea. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190206 - Preserve exception specs in function decl merging.
Author: efriedma Date: Fri Sep 6 16:09:09 2013 New Revision: 190206 URL: http://llvm.org/viewvc/llvm-project?rev=190206view=rev Log: Preserve exception specs in function decl merging. Exception specs are not part of the canonical type, but we shouldn't drop them just because we merged a noreturn attribute. Fixes PR17110. Modified: cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/test/CXX/except/except.spec/p4.cpp Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=190206r1=190205r2=190206view=diff == --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Sep 6 16:09:09 2013 @@ -2382,9 +2382,11 @@ bool Sema::MergeFunctionDecl(FunctionDec } if (RequiresAdjustment) { -NewType = Context.adjustFunctionType(NewType, NewTypeInfo); -New-setType(QualType(NewType, 0)); +const FunctionType *AdjustedType = New-getType()-getAsFunctionType(); +AdjustedType = Context.adjustFunctionType(AdjustedType, NewTypeInfo); +New-setType(QualType(AdjustedType, 0)); NewQType = Context.getCanonicalType(New-getType()); +NewType = castFunctionType(NewQType); } // If this redeclaration makes the function inline, we may need to add it to Modified: cfe/trunk/test/CXX/except/except.spec/p4.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/except/except.spec/p4.cpp?rev=190206r1=190205r2=190206view=diff == --- cfe/trunk/test/CXX/except/except.spec/p4.cpp (original) +++ cfe/trunk/test/CXX/except/except.spec/p4.cpp Fri Sep 6 16:09:09 2013 @@ -34,3 +34,8 @@ templatetypename T struct U { templatetypename T UT::~U() noexcept(true) {} // expected-error {{exception specification in declaration does not match previous declaration}} templatetypename T void UT::operator delete(void*) noexcept(false) {} // expected-error {{exception specification in declaration does not match previous declaration}} + + +// Make sure this restriction interacts properly with __attribute__((noreturn)) +void __attribute__ ((__noreturn__)) PR17110(int status) throw(); +void PR17110(int status) throw(); ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Trapping math and libfuncs improvements
On Fri, Sep 6, 2013 at 8:27 AM, Hal Finkel hfin...@anl.gov wrote: Ben, Chad, et al., The attached patch cleans up the LIBBUILTIN libm function definitions, and improves the way in which -ftrapping-math is handled in a complementary way. Specifically, this patch: 1. The libm LIBBUILTIN definitions are synchronized with the __builtin_* definitions for libm functions. We currently have __builtin_* definitions for many more functions than those for which we have LIBBUILTIN definitions. This unnecessarily pessimizes code generation around many of the remaining libm functions. 2. Many of these functions can be marked as readnone so long as we can neglect floating point exceptions. I've added a new specifier 'T', which like 'e' for errno, marks the builtin as const so long as trapping math is disabled. I've added a TrappingMath lang option to track this. 3. Disables -ftrapping-math by default. This is currently enabled by default (following gcc), but this is silly. No system of which I'm aware actually traps on floating-point exceptions by default (some system-specific mechanism is necessary in order to enable SIGFPE generation). In addition, in Clang, we currently don't support fenv access. As a result, we can turn this off by default (and enable better code generation around the functions for which we have LIBBUILTIN definitions). 4. Updates the CodeGen/libcall-declarations.c test case. As a side effect, the existing libm LIBBUILTIN definitions that were missing the 'n' (nothrow) specifier now have it. For clarity, please split the patch into four separate patches: reformatting, adding missing 'n' specifiers, the trapping-math-related changes, and adding new LIBBUILTIN definitions (preferably in that order). As it is, the Builtins.def changes are completely unreviewable. Your changes could be substantially simplified if we don't allow enabling -ftrapping-math; as you've noted, we don't honor it anyway. What do you think? -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Trapping math and libfuncs improvements [Part 1 - missing n]
Looks fine. -Eli On Fri, Sep 6, 2013 at 4:10 PM, Hal Finkel hfin...@anl.gov wrote: Eli, Here's the first patch -- adding of the missing 'n' (nothrow) flag to the existing libm LIBBUILTIN definitions (plus a test case -- unfortunately making the source also valid C++ seems to make the test case uglier). Thanks again, Hal - Original Message - On Fri, Sep 6, 2013 at 3:53 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Fri, Sep 6, 2013 at 3:32 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Fri, Sep 6, 2013 at 8:27 AM, Hal Finkel hfin...@anl.gov wrote: Ben, Chad, et al., The attached patch cleans up the LIBBUILTIN libm function definitions, and improves the way in which -ftrapping-math is handled in a complementary way. Specifically, this patch: 1. The libm LIBBUILTIN definitions are synchronized with the __builtin_* definitions for libm functions. We currently have __builtin_* definitions for many more functions than those for which we have LIBBUILTIN definitions. This unnecessarily pessimizes code generation around many of the remaining libm functions. 2. Many of these functions can be marked as readnone so long as we can neglect floating point exceptions. I've added a new specifier 'T', which like 'e' for errno, marks the builtin as const so long as trapping math is disabled. I've added a TrappingMath lang option to track this. 3. Disables -ftrapping-math by default. This is currently enabled by default (following gcc), but this is silly. No system of which I'm aware actually traps on floating-point exceptions by default (some system-specific mechanism is necessary in order to enable SIGFPE generation). In addition, in Clang, we currently don't support fenv access. As a result, we can turn this off by default (and enable better code generation around the functions for which we have LIBBUILTIN definitions). 4. Updates the CodeGen/libcall-declarations.c test case. As a side effect, the existing libm LIBBUILTIN definitions that were missing the 'n' (nothrow) specifier now have it. For clarity, please split the patch into four separate patches: reformatting, adding missing 'n' specifiers, the trapping-math-related changes, and adding new LIBBUILTIN definitions (preferably in that order). As it is, the Builtins.def changes are completely unreviewable. Unfortunately, because almost all of the LIBBUILTIN's in the patch are new, I don't think that splitting the patch will help that :( -- FWIW, I think that reviewing the Builtins.def changes means, in practice (either trusting me, or) going through the documentation on each of those functions to make sure that the flags are correct. I don't think that splitting the patch makes that easier either. Nevertheless, I'll split the changes and re-post as separate patches. Maybe I'm wrong ;) I didn't try to count how many were new... it looked like you were changing a bunch of existing ones, but if the balance is as you say, it might not be as useful as I'd hope. There may have been a few changes to the existing ones, I'll go over them again, and try to make that more clear in the splitting. Your changes could be substantially simplified if we don't allow enabling -ftrapping-math; as you've noted, we don't honor it anyway. What do you think? I considered that, but I think that we might as well not make the situation worse for users who do enable trapping math for debugging purposes (as I do myself on occasion). Debugging how, exactly? If I have an application that is producing NaNs, Inf, etc. where that is not expected, I can add something like this: #include fenv.h #if defined(__i386__) defined(__SSE__) #include xmmintrin.h #endif ... #if defined(FE_NOMASK_ENV) !defined(__INTEL_COMPILER) fesetenv(FE_NOMASK_ENV); fedisableexcept(/* FE_OVERFLOW | */ FE_UNDERFLOW | FE_INEXACT); #elif defined(__i386__) defined(__SSE__) _MM_SET_EXCEPTION_MASK(_MM_GET_EXCEPTION_MASK() ~(_MM_MASK_OVERFLOW|_MM_MASK_INVALID|_MM_MASK_DIV_ZERO)); #endif to the beginning of the code. Then I should get a much better idea (based on when the SIGFPE is delivered) of where the problem is. Ah, interesting. With these changes, we'll mark a lot more libm functions that could trap when exceptions are enabled as readnone (including some which never set errno), and if we ignore -ftrapping-math completely, we'll provide no way of indicating that those functions have side effects when we're interested in floating point exceptions. Providing -ftrapping-math
Re: r190044 - Add new methods for TargetInfo:
On Fri, Sep 6, 2013 at 2:53 AM, Stepan Dyatkovskiy stpwo...@narod.ruwrote: Eli OK. You mean this one: case 128: -if (getLongDoubleFormat() == llvm::APFloat::**PPCDoubleDouble) +if (getLongDoubleFormat() == llvm::APFloat::**PPCDoubleDouble || +getLongDoubleFormat() == llvm::APFloat::IEEEquad) return LongDouble; break; } If yes - can I fix it? Yes. -Eli Eli Friedman wrote: On Thu, Sep 5, 2013 at 4:23 AM, Stepan Dyatkovskiy stpwo...@narod.ru mailto:stpwo...@narod.ru wrote: Author: dyatkovskiy Date: Thu Sep 5 06:23:21 2013 New Revision: 190044 URL: http://llvm.org/viewvc/llvm-**project?rev=190044view=revhttp://llvm.org/viewvc/llvm-project?rev=190044view=rev Log: Add new methods for TargetInfo: getRealTypeByWidth and getIntTypeByWidth for ASTContext names are almost same(invokes new methods from TargetInfo): getIntTypeForBitwidth and getRealTypeForBitwidth. As first commit for PR16752 fix: 'mode' attribute for unusual targets doesn't work properly Description: Troubles could be happened due to some assumptions in handleModeAttr function (see SemaDeclAttr.cpp). For example, it assumes that 32 bit integer is 'int', while it could be 16 bit only. Instead of asking target: 'which type do you want to use for int32_t ?' it just hardcodes general opinion. That doesn't looks pretty correct. Please consider the next solution: 1. In Basic/TargetInfo add getIntTypeByWidth and getRealTypeByWidth virtual methods. By default current behaviour could be implemented here. 2. Fix handleModeAttr according to new methods in TargetInfo. This approach is implemented in the patch attached to this post. Fixes: 1st Commit (Current): Add new methods for TargetInfo: getRealTypeByWidth and getIntTypeByWidth for ASTContext names are almost same(invokes new methods from TargetInfo): getIntTypeForBitwidth and getRealTypeForBitwidth 2nd Commit (Next): Fix SemaDeclAttr, handleModeAttr function. Modified: cfe/trunk/include/clang/AST/**ASTContext.h cfe/trunk/include/clang/Basic/**TargetInfo.h cfe/trunk/lib/AST/ASTContext.**cpp cfe/trunk/lib/Basic/**TargetInfo.cpp cfe/trunk/lib/Frontend/**InitPreprocessor.cpp Modified: cfe/trunk/include/clang/AST/**ASTContext.h URL: http://llvm.org/viewvc/llvm-**project/cfe/trunk/include/** clang/AST/ASTContext.h?rev=**190044r1=190043r2=190044**view=diffhttp://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=190044r1=190043r2=190044view=diff ==**==** == --- cfe/trunk/include/clang/AST/**ASTContext.h (original) +++ cfe/trunk/include/clang/AST/**ASTContext.h Thu Sep 5 06:23:21 2013 @@ -480,6 +480,17 @@ public: const TargetInfo getTargetInfo() const { return *Target; } + /// getIntTypeForBitwidth - + /// sets integer QualTy according to specified details: + /// bitwidth, signed/unsigned. + /// Returns empty type if there is no appropriate target types. + QualType getIntTypeForBitwidth(unsigned DestWidth, + unsigned Signed) const; + /// getRealTypeForBitwidth - + /// sets floating point QualTy according to specified bitwidth. + /// Returns empty type if there is no appropriate target types. + QualType getRealTypeForBitwidth(**unsigned DestWidth) const; + bool AtomicUsesUnsupportedLibcall(**const AtomicExpr *E) const; const LangOptions getLangOpts() const { return LangOpts; } Modified: cfe/trunk/include/clang/Basic/**TargetInfo.h URL: http://llvm.org/viewvc/llvm-**project/cfe/trunk/include/** clang/Basic/TargetInfo.h?rev=**190044r1=190043r2=190044**view=diffhttp://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=190044r1=190043r2=190044view=diff ==**==** == --- cfe/trunk/include/clang/Basic/**TargetInfo.h (original) +++ cfe/trunk/include/clang/Basic/**TargetInfo.h Thu Sep 5 06:23:21 2013 @@ -112,6 +112,8 @@ public: ///=== Target Data Type Query Methods --**-===// enum IntType { NoInt = 0, +SignedChar, +UnsignedChar, SignedShort, UnsignedShort, SignedInt, @@ -123,6 +125,7 @@ public: }; enum RealType { +NoFloat = 255, Float = 0, Double, LongDouble @@ -220,6 +223,12 @@ public: /// For example, SignedInt - getIntWidth(). unsigned getTypeWidth(IntType T) const; + /// \brief Return integer type with specified width
Re: [PATCH] Trapping math and libfuncs improvements [Part 2 - fixing atan]
On Fri, Sep 6, 2013 at 5:20 PM, Hal Finkel hfin...@anl.gov wrote: Eli, Here's the second part for review: Currently the LIBBUILTIN definitions for atan and atan2 say that these functions might set errno, but they never do (this is the only other relevant functional change to the existing definitions in the original patch). http://pubs.opengroup.org/onlinepubs/9699919799/functions/atan.html says atan can in fact set errno. -Eli Thanks again, Hal - Original Message - - Original Message - - Original Message - Looks fine. r190217, thanks! [I also committed, in r190218, a trivial reordering of the existing definitions.] And in r190222, I reordered the existing definitions to match the order of the __builtin_* definitions (which is a small change, but should make comparing to the new list easier). -Hal -Hal -Eli On Fri, Sep 6, 2013 at 4:10 PM, Hal Finkel hfin...@anl.gov wrote: Eli, Here's the first patch -- adding of the missing 'n' (nothrow) flag to the existing libm LIBBUILTIN definitions (plus a test case -- unfortunately making the source also valid C++ seems to make the test case uglier). Thanks again, Hal - Original Message - On Fri, Sep 6, 2013 at 3:53 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Fri, Sep 6, 2013 at 3:32 PM, Hal Finkel hfin...@anl.gov wrote: - Original Message - On Fri, Sep 6, 2013 at 8:27 AM, Hal Finkel hfin...@anl.gov wrote: Ben, Chad, et al., The attached patch cleans up the LIBBUILTIN libm function definitions, and improves the way in which -ftrapping-math is handled in a complementary way. Specifically, this patch: 1. The libm LIBBUILTIN definitions are synchronized with the __builtin_* definitions for libm functions. We currently have __builtin_* definitions for many more functions than those for which we have LIBBUILTIN definitions. This unnecessarily pessimizes code generation around many of the remaining libm functions. 2. Many of these functions can be marked as readnone so long as we can neglect floating point exceptions. I've added a new specifier 'T', which like 'e' for errno, marks the builtin as const so long as trapping math is disabled. I've added a TrappingMath lang option to track this. 3. Disables -ftrapping-math by default. This is currently enabled by default (following gcc), but this is silly. No system of which I'm aware actually traps on floating-point exceptions by default (some system-specific mechanism is necessary in order to enable SIGFPE generation). In addition, in Clang, we currently don't support fenv access. As a result, we can turn this off by default (and enable better code generation around the functions for which we have LIBBUILTIN definitions). 4. Updates the CodeGen/libcall-declarations.c test case. As a side effect, the existing libm LIBBUILTIN definitions that were missing the 'n' (nothrow) specifier now have it. For clarity, please split the patch into four separate patches: reformatting, adding missing 'n' specifiers, the trapping-math-related changes, and adding new LIBBUILTIN definitions (preferably in that order). As it is, the Builtins.def changes are completely unreviewable. Unfortunately, because almost all of the LIBBUILTIN's in the patch are new, I don't think that splitting the patch will help that :( -- FWIW, I think that reviewing the Builtins.def changes means, in practice (either trusting me, or) going through the documentation on each of those functions to make sure that the flags are correct. I don't think that splitting the patch makes that easier either. Nevertheless, I'll split the changes and re-post as separate patches. Maybe I'm wrong ;) I didn't try to count how many were new... it looked like you were changing a bunch of existing ones, but if the balance is as you say, it might not be as useful as I'd hope. There may have been a few changes to the existing ones, I'll go over them again, and try to make that more clear in
r190094 - Fix regression from r190016.
Author: efriedma Date: Thu Sep 5 15:13:32 2013 New Revision: 190094 URL: http://llvm.org/viewvc/llvm-project?rev=190094view=rev Log: Fix regression from r190016. I don't have a reduced testcase yet. Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=190094r1=190093r2=190094view=diff == --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Thu Sep 5 15:13:32 2013 @@ -2840,7 +2840,9 @@ void ASTDeclReader::UpdateDecl(Decl *D, } case UPD_DECL_MARKED_USED: { - D-markUsed(Reader.getContext()); + // FIXME: This doesn't send the right notifications if there are + // ASTMutationListeners other than an ASTWriter. + D-setIsUsed(true); break; } } ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] [ms-cxxabi] Implement guard variables for static initialization
On Wed, Sep 4, 2013 at 10:10 AM, Reid Kleckner r...@google.com wrote: On Fri, Aug 30, 2013 at 10:45 AM, Reid Kleckner r...@google.com wrote: Comment at: lib/Sema/SemaLambda.cpp:101 @@ -98,1 +100,3 @@ +(isaFunctionDecl(CurContext) + Context.getTargetInfo().getCXXABI().isMicrosoft())) { ManglingContextDecl = 0; Eli Friedman wrote: Why do you need to number anything in a function which isn't inline? CodeGen can mangle static variables etc. however it wants because they aren't externally visible. I'm using these to number bits in a bitfield, not to mangle. I have to use this mechanism to number the bits of inline functions, so it makes sense to use it to number bits of non inline functions too. Otherwise I have to add back the CXXABIFunctionState code I had in the first version of this patch. I'm having a very hard time extracting this function from Sema and moving it over to AST/ItaniumCXXABI.cpp as John suggested. It accesses a bunch of Sema fields. Do you have any thoughts on how this should look in the long run? Any thoughts on this? This is needed to avoid double initialization of llvm::outs/errs(), and factoring this one check out will require a massive migration of Eli's Itanium logic from Sema to AST. The Itanium ABI code actually uses two numbering schemes: one for externally-visible stuff, and one for non-exernally visible stuff. Not sure if you want to go that route. See the beginning of the definition of class ItaniumMangleContext for how it numbers non-externally-visible stuff. The idea is that we can avoid tracking the numbering in the AST for declarations where the numbers don't matter. (Also, it involved changing less code when I implemented it.) If you want to, I'm fine with leaving that check as-is. (I haven't actually reviewed the rest of the patch.) -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190109 - Improve error for override + non-virtual func.
Author: efriedma Date: Thu Sep 5 18:51:03 2013 New Revision: 190109 URL: http://llvm.org/viewvc/llvm-project?rev=190109view=rev Log: Improve error for override + non-virtual func. Consider something like the following: struct X { virtual void foo(float x); }; struct Y : X { void foo(double x) override; }; The error is almost certainly that Y::foo() has the wrong signature, rather than incorrect usage of the override keyword. This patch adds an appropriate diagnostic for that case. Fixes rdar://problem/14785106. Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/Sema/SemaDeclCXX.cpp cfe/trunk/test/CXX/class.derived/class.virtual/p3-0x.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=190109r1=190108r2=190109view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Sep 5 18:51:03 2013 @@ -1538,6 +1538,9 @@ def err_auto_fn_virtual : Error // C++11 override control def override_keyword_only_allowed_on_virtual_member_functions : Error only virtual member functions can be marked '%0'; +def override_keyword_hides_virtual_member_function : Error + non-virtual member function marked '%0' hides virtual member + %select{function|functions}1; def err_function_marked_override_not_overriding : Error %0 marked 'override' but does not override any member functions; def err_class_marked_final_used_as_base : Error Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=190109r1=190108r2=190109view=diff == --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Thu Sep 5 18:51:03 2013 @@ -1490,7 +1490,11 @@ public: bool CheckConstexprFunctionDecl(const FunctionDecl *FD); bool CheckConstexprFunctionBody(const FunctionDecl *FD, Stmt *Body); - void DiagnoseHiddenVirtualMethods(CXXRecordDecl *DC, CXXMethodDecl *MD); + void DiagnoseHiddenVirtualMethods(CXXMethodDecl *MD); + void FindHiddenVirtualMethods(CXXMethodDecl *MD, + SmallVectorImplCXXMethodDecl* OverloadedMethods); + void NoteHiddenVirtualMethods(CXXMethodDecl *MD, + SmallVectorImplCXXMethodDecl* OverloadedMethods); // Returns true if the function declaration is a redeclaration bool CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD, LookupResult Previous, @@ -4807,7 +4811,7 @@ public: bool CheckPureMethod(CXXMethodDecl *Method, SourceRange InitRange); /// CheckOverrideControl - Check C++11 override control semantics. - void CheckOverrideControl(Decl *D); + void CheckOverrideControl(NamedDecl *D); /// CheckForFunctionMarkedFinal - Checks whether a virtual member function /// overrides a virtual member function marked 'final', according to Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=190109r1=190108r2=190109view=diff == --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Sep 5 18:51:03 2013 @@ -1700,37 +1700,61 @@ bool Sema::ActOnAccessSpecifier(AccessSp } /// CheckOverrideControl - Check C++11 override control semantics. -void Sema::CheckOverrideControl(Decl *D) { +void Sema::CheckOverrideControl(NamedDecl *D) { if (D-isInvalidDecl()) return; - const CXXMethodDecl *MD = dyn_castCXXMethodDecl(D); + // We only care about override and final declarations. + if (!D-hasAttrOverrideAttr() !D-hasAttrFinalAttr()) +return; - // Do we know which functions this declaration might be overriding? - bool OverridesAreKnown = !MD || - (!MD-getParent()-hasAnyDependentBases() - !MD-getType()-isDependentType()); + CXXMethodDecl *MD = dyn_castCXXMethodDecl(D); - if (!MD || !MD-isVirtual()) { -if (OverridesAreKnown) { + // We can't check dependent instance methods. + if (MD MD-isInstance() + (MD-getParent()-hasAnyDependentBases() || + MD-getType()-isDependentType())) +return; + + if (MD !MD-isVirtual()) { +// If we have a non-virtual method, check if if hides a virtual method. +// (In that case, it's most likely the method has the wrong type.) +SmallVectorCXXMethodDecl *, 8 OverloadedMethods; +FindHiddenVirtualMethods(MD, OverloadedMethods); + +if (!OverloadedMethods.empty()) { if (OverrideAttr *OA = D-getAttrOverrideAttr()) { Diag(OA-getLocation(), -
r190108 - Add a bit more info to modules fatal error.
Author: efriedma Date: Thu Sep 5 18:50:58 2013 New Revision: 190108 URL: http://llvm.org/viewvc/llvm-project?rev=190108view=rev Log: Add a bit more info to modules fatal error. Just a minor tweak to make it easier to track down the cause of fatal errors with modules. Modified: cfe/trunk/lib/Serialization/ModuleManager.cpp Modified: cfe/trunk/lib/Serialization/ModuleManager.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ModuleManager.cpp?rev=190108r1=190107r2=190108view=diff == --- cfe/trunk/lib/Serialization/ModuleManager.cpp (original) +++ cfe/trunk/lib/Serialization/ModuleManager.cpp Thu Sep 5 18:50:58 2013 @@ -62,11 +62,13 @@ ModuleManager::addModule(StringRef FileN // Look for the file entry. This only fails if the expected size or // modification time differ. const FileEntry *Entry; - if (lookupModuleFile(FileName, ExpectedSize, ExpectedModTime, Entry)) + if (lookupModuleFile(FileName, ExpectedSize, ExpectedModTime, Entry)) { +ErrorStr = module file out of date; return OutOfDate; + } if (!Entry FileName != -) { -ErrorStr = file not found; +ErrorStr = module file not found; return Missing; } ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: r190044 - Add new methods for TargetInfo:
On Thu, Sep 5, 2013 at 4:23 AM, Stepan Dyatkovskiy stpwo...@narod.ruwrote: Author: dyatkovskiy Date: Thu Sep 5 06:23:21 2013 New Revision: 190044 URL: http://llvm.org/viewvc/llvm-project?rev=190044view=rev Log: Add new methods for TargetInfo: getRealTypeByWidth and getIntTypeByWidth for ASTContext names are almost same(invokes new methods from TargetInfo): getIntTypeForBitwidth and getRealTypeForBitwidth. As first commit for PR16752 fix: 'mode' attribute for unusual targets doesn't work properly Description: Troubles could be happened due to some assumptions in handleModeAttr function (see SemaDeclAttr.cpp). For example, it assumes that 32 bit integer is 'int', while it could be 16 bit only. Instead of asking target: 'which type do you want to use for int32_t ?' it just hardcodes general opinion. That doesn't looks pretty correct. Please consider the next solution: 1. In Basic/TargetInfo add getIntTypeByWidth and getRealTypeByWidth virtual methods. By default current behaviour could be implemented here. 2. Fix handleModeAttr according to new methods in TargetInfo. This approach is implemented in the patch attached to this post. Fixes: 1st Commit (Current): Add new methods for TargetInfo: getRealTypeByWidth and getIntTypeByWidth for ASTContext names are almost same(invokes new methods from TargetInfo): getIntTypeForBitwidth and getRealTypeForBitwidth 2nd Commit (Next): Fix SemaDeclAttr, handleModeAttr function. Modified: cfe/trunk/include/clang/AST/ASTContext.h cfe/trunk/include/clang/Basic/TargetInfo.h cfe/trunk/lib/AST/ASTContext.cpp cfe/trunk/lib/Basic/TargetInfo.cpp cfe/trunk/lib/Frontend/InitPreprocessor.cpp Modified: cfe/trunk/include/clang/AST/ASTContext.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=190044r1=190043r2=190044view=diff == --- cfe/trunk/include/clang/AST/ASTContext.h (original) +++ cfe/trunk/include/clang/AST/ASTContext.h Thu Sep 5 06:23:21 2013 @@ -480,6 +480,17 @@ public: const TargetInfo getTargetInfo() const { return *Target; } + /// getIntTypeForBitwidth - + /// sets integer QualTy according to specified details: + /// bitwidth, signed/unsigned. + /// Returns empty type if there is no appropriate target types. + QualType getIntTypeForBitwidth(unsigned DestWidth, + unsigned Signed) const; + /// getRealTypeForBitwidth - + /// sets floating point QualTy according to specified bitwidth. + /// Returns empty type if there is no appropriate target types. + QualType getRealTypeForBitwidth(unsigned DestWidth) const; + bool AtomicUsesUnsupportedLibcall(const AtomicExpr *E) const; const LangOptions getLangOpts() const { return LangOpts; } Modified: cfe/trunk/include/clang/Basic/TargetInfo.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=190044r1=190043r2=190044view=diff == --- cfe/trunk/include/clang/Basic/TargetInfo.h (original) +++ cfe/trunk/include/clang/Basic/TargetInfo.h Thu Sep 5 06:23:21 2013 @@ -112,6 +112,8 @@ public: ///=== Target Data Type Query Methods ---===// enum IntType { NoInt = 0, +SignedChar, +UnsignedChar, SignedShort, UnsignedShort, SignedInt, @@ -123,6 +125,7 @@ public: }; enum RealType { +NoFloat = 255, Float = 0, Double, LongDouble @@ -220,6 +223,12 @@ public: /// For example, SignedInt - getIntWidth(). unsigned getTypeWidth(IntType T) const; + /// \brief Return integer type with specified width. + IntType getIntTypeByWidth(unsigned BitWidth, bool IsSigned) const; + + /// \brief Return floating point type with specified width. + RealType getRealTypeByWidth(unsigned BitWidth) const; + /// \brief Return the alignment (in bits) of the specified integer type enum. /// /// For example, SignedInt - getIntAlign(). Modified: cfe/trunk/lib/AST/ASTContext.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=190044r1=190043r2=190044view=diff == --- cfe/trunk/lib/AST/ASTContext.cpp (original) +++ cfe/trunk/lib/AST/ASTContext.cpp Thu Sep 5 06:23:21 2013 @@ -6314,6 +6314,8 @@ ASTContext::getSubstTemplateTemplateParm CanQualType ASTContext::getFromTargetType(unsigned Type) const { switch (Type) { case TargetInfo::NoInt: return CanQualType(); + case TargetInfo::SignedChar: return SignedCharTy; + case TargetInfo::UnsignedChar: return UnsignedCharTy; case TargetInfo::SignedShort: return ShortTy; case TargetInfo::UnsignedShort: return UnsignedShortTy; case TargetInfo::SignedInt: return IntTy; @@ -7990,6
Re: [PATCH] Warning for Out of bound access of Array of Structures/Unions not reported
memExpr-getBase() will never return null. Nit: there are a bunch of extra newlines in your testcase. -Eli On Thu, Sep 5, 2013 at 7:27 AM, Karthik Bhat blitz.opensou...@gmail.comwrote: Hi Jordan,Ted,Richard Any inputs on this patch? http://llvm-reviews.chandlerc.com/D1580 Thanks Karthik Bhat On Tue, Sep 3, 2013 at 7:40 PM, Karthik Bhat blitz.opensou...@gmail.comwrote: Hi jordan_rose, Hi All, Out of bound access of array of structures/unions are not reported. Added a patch to handle the same. Please let me know if it is good to commit. Thanks Karthik Bhat http://llvm-reviews.chandlerc.com/D1580 Files: lib/Sema/SemaChecking.cpp test/SemaCXX/array-bounds.cpp Index: lib/Sema/SemaChecking.cpp === --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -6373,6 +6373,12 @@ CheckArrayAccess(rhs); return; } + case Stmt::MemberExprClass: { +const MemberExpr *memExpr = castMemberExpr(expr); +if (const Expr *base = memExpr-getBase()) + CheckArrayAccess(base); +return; + } default: return; } Index: test/SemaCXX/array-bounds.cpp === --- test/SemaCXX/array-bounds.cpp +++ test/SemaCXX/array-bounds.cpp @@ -253,3 +253,20 @@ int a[128]; // expected-note {{array 'a' declared here}} a[(unsigned char)'\xA1'] = 1; // expected-warning {{array index 161 is past the end of the array}} } + +typedef struct { +int a; +} structTest; + +void +test_arraystructOverflow() { + structTest data[2]; // expected-note 2 {{array 'data' declared here}} + if (!data[1].a !data[2].a) { // expected-warning {{array index 2 is past the end of the array (which contains 2 elements)}} +data[2].a = 1; // expected-warning {{array index 2 is past the end of the array (which contains 2 elements)}} + } + return; +} + + + + ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Fix missing source location in CXXTemporaryObjectExpr.
On Thu, Sep 5, 2013 at 12:34 AM, Enea Zaffanella zaffane...@cs.unipr.itwrote: On 09/04/2013 11:05 PM, Eli Friedman wrote: On Wed, Sep 4, 2013 at 4:24 AM, Enea Zaffanella zaffane...@cs.unipr.it mailto:zaffane...@cs.unipr.it** wrote: When dumping the following C++11 chunk of code $ cat chunk.cc struct A { A(int, int); }; A a_paren( A(0,0) ); A a_range( A{0,0} ); the CXXTemporaryObjectExpr generated by A{0,0} is missing a proper end location (the source location is correctly set for A(0,0)). $ clang -cc1 -std=c++11 -ast-dump chunk.cc [...] `-VarDecl 0x5a963e0 line:3:1, col:19 a_range 'struct A' `-CXXConstructExpr 0x5a96570 col:3, col:19 'struct A' 'void (struct A ) noexcept' elidable `-MaterializeTemporaryExpr 0x5a96550 col:12, invalid sloc 'struct A' xvalue `-CXXTemporaryObjectExpr 0x5a964d8 col:12, invalid sloc 'struct A' 'void (int, int)' |-IntegerLiteral 0x5a96448 col:14 'int' 0 `-IntegerLiteral 0x5a96468 col:16 'int' 0 Please find attached a patch (with testcase) fixing this issue. OK to commit? +if (Kind.getKind() == InitializationKind::IK_**DirectList) + ParenRange = SourceRange(LBraceLoc, RBraceLoc); +else ParenRange = Kind.getParenRange(); This isn't right: there aren't any parentheses. getParenRange() on a CXXTemporaryObjectExpr shouldn't return source locations pointing at tokens which aren't parentheses. You should be able to fix this purely in CXXTemporaryObjectExpr::**getLocStart()/getLocEnd(). -Eli I see your point but ... My guess is that getParenRange() method was designed considering only C++03; now that we also have C++11 list initialization, I was freely reinterpreting it as meaning getParenOrBraceRange(). Note that, in the dump above, the arguments of the CXXTemporaryObjectExpr node are the elements of the InitListExpr. That is, the InitListExpr node is unwrapped before creating the temporary object node. So, if the getParenRange() is interpreted to strictly mean only parentheses (not braces), we end up having no place to store the source location for the braces themselves (and hence no place to store the end location of the node). That is, we can not query the arguments to obtain the end location (which is the trick used in CXXConstructExpr::getLocEnd). To summarize, I see two options: (a) rename getParenRange() as getParenOrBraceRange(); (b) have two different methods, getParenRange() and getBraceRange(), to be used based on the value of a boolean flag. Question: is it the case that bool CXXConstructExpr::**isListInitialization() is such an appropriate flag to be used here? Yes. I would probably opt for (b). Do you see a third option which is better? I don't really see any other options; I guess I would lean towards option A, and let the users branch if they care about the difference, but either way works. Enea. P.S.: while playing on variations of the testcase in the patch, I noticed the following: # cat bug.cc struct A { A(const int()[1]); }; A a_paren( A( {0} ) ); A a_range( A{ {0} } ); When dumping `a_paren', we see that A( {0} ) produces a CXXFunctionaCastExpr node (which is fine). In contrast, when dumping `a_range' we see that A{ {0} } produces a CXXTemporaryObjectExpr node. This does not match the doxygen documentation for this AST node, where it is said: Represents a C++ functional cast expression that builds a temporary object. This expression type represents a C++ functional cast (C++[expr.type.conv]) with N != 1 arguments that invokes a constructor to build a temporary object. With N == 1 arguments the functional cast expression will be represented by CXXFunctionalCastExpr. Is the doxygen documentation obsolete? Or is it the case that a CXXFunctionalCastExpr node should have been produced for `a_range' too? The doxygen just hasn't been updated for C++11. CXXFunctionalCastExpr represents an explicit type conversion which is equivalent to a cast per [expr.type.conv]p1, and nothing else. -Eli ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190114 - Reduce stack usage of TreeTransform.
Author: efriedma Date: Thu Sep 5 20:13:30 2013 New Revision: 190114 URL: http://llvm.org/viewvc/llvm-project?rev=190114view=rev Log: Reduce stack usage of TreeTransform. Without this patch, TreeTransform::TransformExpr uses a ridiculous amount of stack space (around 5000 bytes). Preventing inlining brings the stack usage down to something sane. On a testcase I have, on my computer, this allows changing -ftemplate-depth from 210 to around 750 before we crash. I'm not sure I should commit the testcase, though: I don't want to cause test failures on platforms with less stack space available. rdar://problem/14098189. Modified: cfe/trunk/lib/Sema/TreeTransform.h Modified: cfe/trunk/lib/Sema/TreeTransform.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=190114r1=190113r2=190114view=diff == --- cfe/trunk/lib/Sema/TreeTransform.h (original) +++ cfe/trunk/lib/Sema/TreeTransform.h Thu Sep 5 20:13:30 2013 @@ -598,14 +598,19 @@ public: ExprResult TransformDependentScopeDeclRefExpr(DependentScopeDeclRefExpr *E, bool IsAddressOfOperand); +// FIXME: We use LLVM_ATTRIBUTE_NOINLINE because inlining causes a ridiculous +// amount of stack usage with clang. #define STMT(Node, Parent)\ + LLVM_ATTRIBUTE_NOINLINE \ StmtResult Transform##Node(Node *S); #define EXPR(Node, Parent)\ + LLVM_ATTRIBUTE_NOINLINE \ ExprResult Transform##Node(Node *E); #define ABSTRACT_STMT(Stmt) #include clang/AST/StmtNodes.inc #define OPENMP_CLAUSE(Name, Class)\ + LLVM_ATTRIBUTE_NOINLINE \ OMPClause *Transform ## Class(Class *S); #include clang/Basic/OpenMPKinds.def ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r190118 - Add self-comparison warnings for fields.
Author: efriedma Date: Thu Sep 5 22:13:09 2013 New Revision: 190118 URL: http://llvm.org/viewvc/llvm-project?rev=190118view=rev Log: Add self-comparison warnings for fields. This expands very slightly what -Wtautological-compare considers to be tautological to include implicit accesses to C++ fields and ObjC ivars. I don't want to turn this into a full expression-identity check, but these additions seem pretty well-contained, and maintain the theme of checking for x == x. rdar://problem/14431127 Added: cfe/trunk/test/SemaCXX/self-comparison.cpp cfe/trunk/test/SemaObjC/self-comparison.m Modified: cfe/trunk/lib/Sema/SemaExpr.cpp Modified: cfe/trunk/lib/Sema/SemaExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=190118r1=190117r2=190118view=diff == --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) +++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Sep 5 22:13:09 2013 @@ -7472,6 +7472,22 @@ static void diagnoseLogicalNotOnLHSofCom FixItHint::CreateInsertion(SecondClose, )); } +// Get the decl for a simple expression: a reference to a variable, +// an implicit C++ field reference, or an implicit ObjC ivar reference. +static ValueDecl *getCompareDecl(Expr *E) { + if (DeclRefExpr* DR = dyn_castDeclRefExpr(E)) +return DR-getDecl(); + if (ObjCIvarRefExpr* Ivar = dyn_castObjCIvarRefExpr(E)) { +if (Ivar-isFreeIvar()) + return Ivar-getDecl(); + } + if (MemberExpr* Mem = dyn_castMemberExpr(E)) { +if (Mem-isImplicitAccess()) + return Mem-getMemberDecl(); + } + return 0; +} + // C99 6.5.8, C++ [expr.rel] QualType Sema::CheckCompareOperands(ExprResult LHS, ExprResult RHS, SourceLocation Loc, unsigned OpaqueOpc, @@ -7508,37 +7524,34 @@ QualType Sema::CheckCompareOperands(Expr // obvious cases in the definition of the template anyways. The idea is to // warn when the typed comparison operator will always evaluate to the same // result. -if (DeclRefExpr* DRL = dyn_castDeclRefExpr(LHSStripped)) { - if (DeclRefExpr* DRR = dyn_castDeclRefExpr(RHSStripped)) { -if (DRL-getDecl() == DRR-getDecl() -!IsWithinTemplateSpecialization(DRL-getDecl())) { - DiagRuntimeBehavior(Loc, 0, PDiag(diag::warn_comparison_always) - 0 // self- - (Opc == BO_EQ - || Opc == BO_LE - || Opc == BO_GE)); -} else if (LHSType-isArrayType() RHSType-isArrayType() - !DRL-getDecl()-getType()-isReferenceType() - !DRR-getDecl()-getType()-isReferenceType()) { -// what is it always going to eval to? -char always_evals_to; -switch(Opc) { -case BO_EQ: // e.g. array1 == array2 - always_evals_to = 0; // false - break; -case BO_NE: // e.g. array1 != array2 - always_evals_to = 1; // true - break; -default: - // best we can say is 'a constant' - always_evals_to = 2; // e.g. array1 = array2 - break; -} -DiagRuntimeBehavior(Loc, 0, PDiag(diag::warn_comparison_always) - 1 // array - always_evals_to); +ValueDecl *DL = getCompareDecl(LHSStripped); +ValueDecl *DR = getCompareDecl(RHSStripped); +if (DL DR DL == DR !IsWithinTemplateSpecialization(DL)) { + DiagRuntimeBehavior(Loc, 0, PDiag(diag::warn_comparison_always) + 0 // self- + (Opc == BO_EQ + || Opc == BO_LE + || Opc == BO_GE)); +} else if (DL DR LHSType-isArrayType() RHSType-isArrayType() + !DL-getType()-isReferenceType() + !DR-getType()-isReferenceType()) { +// what is it always going to eval to? +char always_evals_to; +switch(Opc) { +case BO_EQ: // e.g. array1 == array2 + always_evals_to = 0; // false + break; +case BO_NE: // e.g. array1 != array2 + always_evals_to = 1; // true + break; +default: + // best we can say is 'a constant' + always_evals_to = 2; // e.g. array1 = array2 + break; } - } +DiagRuntimeBehavior(Loc, 0, PDiag(diag::warn_comparison_always) + 1 // array + always_evals_to); } if (isaCastExpr(LHSStripped)) Added: cfe/trunk/test/SemaCXX/self-comparison.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/self-comparison.cpp?rev=190118view=auto == --- cfe/trunk/test/SemaCXX/self-comparison.cpp