r258673 - [Sema] Improve constness
Author: danielmarjamaki Date: Mon Jan 25 03:29:38 2016 New Revision: 258673 URL: http://llvm.org/viewvc/llvm-project?rev=258673&view=rev Log: [Sema] Improve constness Modified: cfe/trunk/lib/Sema/SemaChecking.cpp Modified: cfe/trunk/lib/Sema/SemaChecking.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=258673&r1=258672&r2=258673&view=diff == --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) +++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Jan 25 03:29:38 2016 @@ -6215,7 +6215,7 @@ static IntRange GetValueRange(ASTContext return IntRange(MaxWidth, Ty->isUnsignedIntegerOrEnumerationType()); } -static QualType GetExprType(Expr *E) { +static QualType GetExprType(const Expr *E) { QualType Ty = E->getType(); if (const AtomicType *AtomicRHS = Ty->getAs()) Ty = AtomicRHS->getValueType(); @@ -6226,7 +6226,7 @@ static QualType GetExprType(Expr *E) { /// range of values it might take. /// /// \param MaxWidth - the width to which the value will be truncated -static IntRange GetExprRange(ASTContext &C, Expr *E, unsigned MaxWidth) { +static IntRange GetExprRange(ASTContext &C, const Expr *E, unsigned MaxWidth) { E = E->IgnoreParens(); // Try a full evaluation first. @@ -6237,7 +6237,7 @@ static IntRange GetExprRange(ASTContext // I think we only want to look through implicit casts here; if the // user has an explicit widening cast, we should treat the value as // being of the new, wider type. - if (ImplicitCastExpr *CE = dyn_cast(E)) { + if (const auto *CE = dyn_cast(E)) { if (CE->getCastKind() == CK_NoOp || CE->getCastKind() == CK_LValueToRValue) return GetExprRange(C, CE->getSubExpr(), MaxWidth); @@ -6264,7 +6264,7 @@ static IntRange GetExprRange(ASTContext SubRange.NonNegative || OutputTypeRange.NonNegative); } - if (ConditionalOperator *CO = dyn_cast(E)) { + if (const auto *CO = dyn_cast(E)) { // If we can fold the condition, just take that operand. bool CondResult; if (CO->getCond()->EvaluateAsBooleanCondition(CondResult, C)) @@ -6278,7 +6278,7 @@ static IntRange GetExprRange(ASTContext return IntRange::join(L, R); } - if (BinaryOperator *BO = dyn_cast(E)) { + if (const auto *BO = dyn_cast(E)) { switch (BO->getOpcode()) { // Boolean-valued operations are single-bit and positive. @@ -6418,7 +6418,7 @@ static IntRange GetExprRange(ASTContext return IntRange::join(L, R); } - if (UnaryOperator *UO = dyn_cast(E)) { + if (const auto *UO = dyn_cast(E)) { switch (UO->getOpcode()) { // Boolean-valued operations are white-listed. case UO_LNot: @@ -6434,17 +6434,17 @@ static IntRange GetExprRange(ASTContext } } - if (OpaqueValueExpr *OVE = dyn_cast(E)) + if (const auto *OVE = dyn_cast(E)) return GetExprRange(C, OVE->getSourceExpr(), MaxWidth); - if (FieldDecl *BitField = E->getSourceBitField()) + if (const auto *BitField = E->getSourceBitField()) return IntRange(BitField->getBitWidthValue(C), BitField->getType()->isUnsignedIntegerOrEnumerationType()); return IntRange::forValueOfType(C, GetExprType(E)); } -static IntRange GetExprRange(ASTContext &C, Expr *E) { +static IntRange GetExprRange(ASTContext &C, const Expr *E) { return GetExprRange(C, E, C.getIntWidth(GetExprType(E))); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15040: [ARM] Add command-line options for ARMv8.2-A
olista01 added a comment. Ping? http://reviews.llvm.org/D15040 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16438: Fix printing of types in initializers with suppressed tags.
This revision was automatically updated to reflect the committed changes. Closed by commit rL258679: Fix printing of types in initializers with suppressed tags. (authored by d0k). Changed prior to commit: http://reviews.llvm.org/D16438?vs=45658&id=45849#toc Repository: rL LLVM http://reviews.llvm.org/D16438 Files: cfe/trunk/lib/AST/DeclPrinter.cpp cfe/trunk/test/Sema/ast-print.c Index: cfe/trunk/test/Sema/ast-print.c === --- cfe/trunk/test/Sema/ast-print.c +++ cfe/trunk/test/Sema/ast-print.c @@ -53,3 +53,13 @@ // CHECK: struct pair_t p = {a: 3, .b = 4}; struct pair_t p = {a: 3, .b = 4}; + +void initializers() { + // CHECK: int *x = ((void *)0), *y = ((void *)0); + int *x = ((void *)0), *y = ((void *)0); + struct Z{}; + struct { +struct Z z; + // CHECK: } z = {(struct Z){}}; + } z = {(struct Z){}}; +} Index: cfe/trunk/lib/AST/DeclPrinter.cpp === --- cfe/trunk/lib/AST/DeclPrinter.cpp +++ cfe/trunk/lib/AST/DeclPrinter.cpp @@ -751,7 +751,10 @@ else if (D->getInitStyle() == VarDecl::CInit) { Out << " = "; } - Init->printPretty(Out, nullptr, Policy, Indentation); + PrintingPolicy SubPolicy(Policy); + SubPolicy.SuppressSpecifiers = false; + SubPolicy.SuppressTag = false; + Init->printPretty(Out, nullptr, SubPolicy, Indentation); if ((D->getInitStyle() == VarDecl::CallInit) && !isa(Init)) Out << ")"; } Index: cfe/trunk/test/Sema/ast-print.c === --- cfe/trunk/test/Sema/ast-print.c +++ cfe/trunk/test/Sema/ast-print.c @@ -53,3 +53,13 @@ // CHECK: struct pair_t p = {a: 3, .b = 4}; struct pair_t p = {a: 3, .b = 4}; + +void initializers() { + // CHECK: int *x = ((void *)0), *y = ((void *)0); + int *x = ((void *)0), *y = ((void *)0); + struct Z{}; + struct { +struct Z z; + // CHECK: } z = {(struct Z){}}; + } z = {(struct Z){}}; +} Index: cfe/trunk/lib/AST/DeclPrinter.cpp === --- cfe/trunk/lib/AST/DeclPrinter.cpp +++ cfe/trunk/lib/AST/DeclPrinter.cpp @@ -751,7 +751,10 @@ else if (D->getInitStyle() == VarDecl::CInit) { Out << " = "; } - Init->printPretty(Out, nullptr, Policy, Indentation); + PrintingPolicy SubPolicy(Policy); + SubPolicy.SuppressSpecifiers = false; + SubPolicy.SuppressTag = false; + Init->printPretty(Out, nullptr, SubPolicy, Indentation); if ((D->getInitStyle() == VarDecl::CallInit) && !isa(Init)) Out << ")"; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r258679 - Fix printing of types in initializers with suppressed tags.
Author: d0k Date: Mon Jan 25 04:34:06 2016 New Revision: 258679 URL: http://llvm.org/viewvc/llvm-project?rev=258679&view=rev Log: Fix printing of types in initializers with suppressed tags. Tag and specifier printing can be suppressed in Decl::printGroup, but these suppressions leak into the initializers. Thus int *x = ((void *)0), *y = ((void *)0); gets printed as int *x = ((void *)0), *y = ((*)0); And struct { struct Z z; } z = {(struct Z){}}; gets printed as struct { struct Z z; } z = {(){}}; The stops the suppressions from leaking into the initializers. Patch by Nick Sumner! Differential Revision: http://reviews.llvm.org/D16438 Modified: cfe/trunk/lib/AST/DeclPrinter.cpp cfe/trunk/test/Sema/ast-print.c Modified: cfe/trunk/lib/AST/DeclPrinter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclPrinter.cpp?rev=258679&r1=258678&r2=258679&view=diff == --- cfe/trunk/lib/AST/DeclPrinter.cpp (original) +++ cfe/trunk/lib/AST/DeclPrinter.cpp Mon Jan 25 04:34:06 2016 @@ -751,7 +751,10 @@ void DeclPrinter::VisitVarDecl(VarDecl * else if (D->getInitStyle() == VarDecl::CInit) { Out << " = "; } - Init->printPretty(Out, nullptr, Policy, Indentation); + PrintingPolicy SubPolicy(Policy); + SubPolicy.SuppressSpecifiers = false; + SubPolicy.SuppressTag = false; + Init->printPretty(Out, nullptr, SubPolicy, Indentation); if ((D->getInitStyle() == VarDecl::CallInit) && !isa(Init)) Out << ")"; } Modified: cfe/trunk/test/Sema/ast-print.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/ast-print.c?rev=258679&r1=258678&r2=258679&view=diff == --- cfe/trunk/test/Sema/ast-print.c (original) +++ cfe/trunk/test/Sema/ast-print.c Mon Jan 25 04:34:06 2016 @@ -53,3 +53,13 @@ struct pair_t { // CHECK: struct pair_t p = {a: 3, .b = 4}; struct pair_t p = {a: 3, .b = 4}; + +void initializers() { + // CHECK: int *x = ((void *)0), *y = ((void *)0); + int *x = ((void *)0), *y = ((void *)0); + struct Z{}; + struct { +struct Z z; + // CHECK: } z = {(struct Z){}}; + } z = {(struct Z){}}; +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16310: new clang-tidy checker misc-long-cast
danielmarjamaki marked 3 inline comments as done. Comment at: docs/clang-tidy/checks/misc-long-cast.rst:11 @@ +10,3 @@ + +Example code:: + alexfh wrote: > LegalizeAdulthood wrote: > > Please add an example for another type other than `long`, such as casting a > > `float` expression to a `double`. > Is the use of two colons intended? The use of two colons is intended. When I look at the output here: http://rst.ninjs.org/ it will say "System Message: WARNING/2 (, line 15)" if I use a single colon. If you have a different tool to look at rst that I should use let me know. Repository: rL LLVM http://reviews.llvm.org/D16310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.
hokein updated this revision to Diff 45852. hokein added a comment. Change delimeter from "," to ";". http://reviews.llvm.org/D16113 Files: clang-tidy/ClangTidy.cpp clang-tidy/ClangTidy.h clang-tidy/google/GlobalNamesInHeadersCheck.cpp clang-tidy/google/GlobalNamesInHeadersCheck.h clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp clang-tidy/google/UnnamedNamespaceInHeaderCheck.h clang-tidy/misc/DefinitionsInHeadersCheck.cpp clang-tidy/misc/DefinitionsInHeadersCheck.h clang-tidy/utils/CMakeLists.txt clang-tidy/utils/HeaderFileExtensionsUtils.cpp clang-tidy/utils/HeaderFileExtensionsUtils.h Index: clang-tidy/utils/HeaderFileExtensionsUtils.h === --- /dev/null +++ clang-tidy/utils/HeaderFileExtensionsUtils.h @@ -0,0 +1,50 @@ +//===--- HeaderFileExtensionsUtils.h - clang-tidy*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_FILE_EXTENSIONS_UTILS_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_FILE_EXTENSIONS_UTILS_H + +#include + +#include "clang/Basic/SourceLocation.h" +#include "clang/Basic/SourceManager.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/ADT/SmallSet.h" +#include "llvm/Support/Path.h" + +namespace clang { +namespace tidy { +namespace header_file_extensions_utils { + +typedef llvm::SmallSet HeaderFileExtensionsSet; + +/// \brief Checks whether expansion location of Loc is in header file. +bool isExpansionLocInHeaderFile( +SourceLocation Loc, const SourceManager &SM, +const HeaderFileExtensionsSet &HeaderFileExtensions); + +/// \brief Checks whether presumed location of Loc is in header file. +bool isPresumedLocInHeaderFile( +SourceLocation Loc, SourceManager &SM, +const HeaderFileExtensionsSet &HeaderFileExtensions); + +/// \brief Checks whether spelling location of Loc is in header file. +bool isSpellingLocInHeaderFile( +SourceLocation Loc, SourceManager &SM, +const HeaderFileExtensionsSet &HeaderFileExtensions); + +/// \brief Parses header file extensions from a semicolon-separated list. +bool parseHeaderFileExtensions(llvm::StringRef AllHeaderFileExtensions, + HeaderFileExtensionsSet &HeaderFileExtensions); + +} // namespace header_file_extensions_utils +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_FILE_EXTENSIONS_UTILS_H Index: clang-tidy/utils/HeaderFileExtensionsUtils.cpp === --- /dev/null +++ clang-tidy/utils/HeaderFileExtensionsUtils.cpp @@ -0,0 +1,64 @@ +//===--- HeaderFileExtensionsUtils.cpp - clang-tidy--*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "HeaderFileExtensionsUtils.h" +#include "clang/Basic/CharInfo.h" + +namespace clang { +namespace tidy { +namespace header_file_extensions_utils { + +bool isExpansionLocInHeaderFile( +SourceLocation Loc, const SourceManager &SM, +const HeaderFileExtensionsSet &HeaderFileExtensions) { + SourceLocation ExpansionLoc = SM.getExpansionLoc(Loc); + StringRef FileExtension = + llvm::sys::path::extension(SM.getFilename(ExpansionLoc)); + return HeaderFileExtensions.count(FileExtension.substr(1)) > 0; +} + +bool isPresumedLocInHeaderFile( +SourceLocation Loc, SourceManager &SM, +const HeaderFileExtensionsSet &HeaderFileExtensions) { + PresumedLoc PresumedLocation = SM.getPresumedLoc(Loc); + StringRef FileExtension = + llvm::sys::path::extension(PresumedLocation.getFilename()); + return HeaderFileExtensions.count(FileExtension.substr(1)) > 0; +} + +bool isSpellingLocInHeaderFile( +SourceLocation Loc, SourceManager &SM, +const HeaderFileExtensionsSet &HeaderFileExtensions) { + SourceLocation SpellingLoc = SM.getSpellingLoc(Loc); + StringRef FileExtension = + llvm::sys::path::extension(SM.getFilename(SpellingLoc)); + + return HeaderFileExtensions.count(FileExtension.substr(1)) > 0; +} + +bool parseHeaderFileExtensions(llvm::StringRef AllHeaderFileExtensions, + HeaderFileExtensionsSet &HeaderFileExtensions) { + SmallVector Suffixes; + AllHeaderFileExtensions.split(Suffixes, ';'); + HeaderFileExtensions.clear(); + for (llvm::StringRef Suffix : Suffixes) { +llvm::StringRef Extension = Suffix.trim(); +for (llvm::StringRef::const_iterator it = Extension.begin(); + it != Extension.end(); ++it) { + if (!isAlphanumeric(*it)) +
[PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check
LegalizeAdulthood created this revision. LegalizeAdulthood added a reviewer: alexfh. LegalizeAdulthood added a subscriber: cfe-commits. This check examines string literals and looks for those that could be more directly expressed as a raw string literal. Example: ``` char const *const BackSlash{"goink\\frob"}; ``` becomes: ``` char const *const BackSlash{R"(goink\frob)"}; ``` Addresses PR2 http://reviews.llvm.org/D16529 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidyModule.cpp clang-tidy/modernize/RawStringLiteralCheck.cpp clang-tidy/modernize/RawStringLiteralCheck.h docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/modernize-raw-string-literal.rst test/clang-tidy/modernize-raw-string-literal.cpp Index: test/clang-tidy/modernize-raw-string-literal.cpp === --- /dev/null +++ test/clang-tidy/modernize-raw-string-literal.cpp @@ -0,0 +1,55 @@ +// RUN: %check_clang_tidy %s modernize-raw-string-literal %t + +char const *const BackSlash{"goink\\frob"}; +// CHECK-MESSAGES: :[[@LINE-1]]:29: warning: escaped string literal can be written as a raw string literal [modernize-raw-string-literal] +// CHECK-FIXES: {{^}}char const *const BackSlash{R"(goink\frob)"};{{$}} + +char const *const Bell{"goink\a\\frob"}; +char const *const BackSpace{"goink\b\\frob"}; +char const *const FormFeed{"goink\f\\frob"}; +char const *const CarraigeReturn{"goink\r\\frob"}; +char const *const HorizontalTab{"goink\t\\frob"}; +char const *const VerticalTab{"goink\v\\frob"}; +char const *const OctalNonPrintable{"\003"}; +char const *const HexNonPrintable{"\x03"}; + +char const *const NewLine{"goink\nfrob"}; +// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: {{.*}} can be written as a raw string literal +// CHECK-FIXES: {{^}}char const *const NewLine{R"(goink{{$}} +// CHECK-FIXES-NEXT: {{^}}frob)"};{{$}} + +char const *const SingleQuote{"goink\'frob"}; +// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: {{.*}} can be written as a raw string literal +// CHECK-XFIXES: {{^}}char const *const SingleQuote{R"(goink'frob)"};{{$}} + +char const *const DoubleQuote{"goink\"frob"}; +// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: {{.*}} can be written as a raw string literal +// CHECK-FIXES: {{^}}char const *const DoubleQuote{R"(goink"frob)"};{{$}} + +char const *const QuestionMark{"goink\?frob"}; +// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: {{.*}} can be written as a raw string literal +// CHECK-FIXES: {{^}}char const *const QuestionMark{R"(goink?frob)"};{{$}} + +char const *const RegEx{"goink\\(one|two\\)\\?.*\\nfrob"}; +// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: {{.*}} can be written as a raw string literal +// CHECK-FIXES: {{^}}char const *const RegEx{R"(goink\(one|two\)\\\?.*\nfrob)"};{{$}} + +char const *const Path{"C:\\Program Files\\Vendor\\Application\\Application.exe"}; +// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: {{.*}} can be written as a raw string literal +// CHECK-FIXES: {{^}}char const *const Path{R"(C:\Program Files\Vendor\Application\Application.exe)"};{{$}} + +char const *const ContainsSentinel{"who\\ops)\""}; +// CHECK-MESSAGES: :[[@LINE-1]]:36: warning: {{.*}} can be written as a raw string literal +// CHECK-FIXES: {{^}}char const *const ContainsSentinel{R"lit(who\ops)")lit"};{{$}} + +char const *const ContainsDelim{"whoops)\")lit\""}; +// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: {{.*}} can be written as a raw string literal +// CHECK-FIXES: {{^}}char const *const ContainsDelim{R"lit1(whoops)")lit")lit1"};{{$}} + +char const *const OctalPrintable{"\100\\"}; +// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: {{.*}} can be written as a raw string literal +// CHECK-FIXES: {{^}}char const *const OctalPrintable{R"(@\)"};{{$}} + +char const *const HexPrintable{"\x40\\"}; +// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: {{.*}} can be written as a raw string literal +// CHECK-FIXES: {{^}}char const *const HexPrintable{R"(@\)"};{{$}} Index: docs/clang-tidy/checks/modernize-raw-string-literal.rst === --- /dev/null +++ docs/clang-tidy/checks/modernize-raw-string-literal.rst @@ -0,0 +1,29 @@ +.. title:: clang-tidy - modernize-raw-string-literal + +modernize-raw-string-literal + + +This check replaces string literals containing escaped characters with +raw string literals. + +Example: + +.. code-blocK:: c++ + + const char *const quotes{"embedded \"quotes\""}; + +becomes + +.. code-block:: c++ + + const char *const quotes{R"(embedded "quotes")"}; + +The presence of any of the following escapes cause the string to be +converted to a raw string literal: ``\\``, ``\'``, ``\"``, ``\?``, +and octal or hexadecimal escapes for printable ASCII characters. + +If an escaped newline is present in a converted string, it is +replaced with a physical newline. If an escaped tab or vertical +tab is present in a string, it prevents the string from being +convert
Re: [PATCH] D15914: [OpenCL] Pipe builtin functions
pekka.jaaskelainen accepted this revision. pekka.jaaskelainen added a comment. LGTM too. Good job. You can try and ask the release manager if you can sneak it in. It's rather container patch so there might be a chance, but I don't know the policy at this stage of the release. http://reviews.llvm.org/D15914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16527: [OpenMP] Parsing + sema for defaultmap clause.
arpith-jacob updated this revision to Diff 45856. arpith-jacob added a comment. Addressed fixes by Alexey Bataev. Thanks. http://reviews.llvm.org/D16527 Files: include/clang/AST/OpenMPClause.h include/clang/AST/RecursiveASTVisitor.h include/clang/Basic/OpenMPKinds.def include/clang/Basic/OpenMPKinds.h include/clang/Sema/Sema.h lib/AST/StmtPrinter.cpp lib/AST/StmtProfile.cpp lib/Basic/OpenMPKinds.cpp lib/CodeGen/CGStmtOpenMP.cpp lib/Parse/ParseOpenMP.cpp lib/Sema/SemaOpenMP.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp test/OpenMP/target_ast_print.cpp test/OpenMP/target_defaultmap_messages.cpp tools/libclang/CIndex.cpp Index: tools/libclang/CIndex.cpp === --- tools/libclang/CIndex.cpp +++ tools/libclang/CIndex.cpp @@ -2229,6 +2229,8 @@ Visitor->AddStmt(C->getChunkSize()); Visitor->AddStmt(C->getHelperChunkSize()); } +void OMPClauseEnqueue::VisitOMPDefaultmapClause(const OMPDefaultmapClause *C) { +} } void EnqueueVisitor::EnqueueChildren(const OMPClause *S) { Index: test/OpenMP/target_defaultmap_messages.cpp === --- /dev/null +++ test/OpenMP/target_defaultmap_messages.cpp @@ -0,0 +1,56 @@ +// RUN: %clang_cc1 -verify -fopenmp %s + +void foo() { +} + +template +T tmain(T argc, S **argv) { + #pragma omp target defaultmap // expected-error {{expected '(' after 'defaultmap'}} + foo(); + #pragma omp target defaultmap ( // expected-error {{expected 'tofrom' in OpenMP clause 'defaultmap'}} expected-error {{expected ')'}} expected-note {{to match this '('}} + foo(); + #pragma omp target defaultmap () // expected-error {{expected 'tofrom' in OpenMP clause 'defaultmap'}} + foo(); + #pragma omp target defaultmap (tofrom // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-warning {{missing ':' after defaultmap modifier - ignoring}} expected-error {{expected 'scalar' in OpenMP clause 'defaultmap'}} + foo(); + #pragma omp target defaultmap (tofrom: // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-error {{expected 'scalar' in OpenMP clause 'defaultmap'}} + foo(); + #pragma omp target defaultmap (tofrom) // expected-warning {{missing ':' after defaultmap modifier - ignoring}} expected-error {{expected 'scalar' in OpenMP clause 'defaultmap'}} + foo(); + #pragma omp target defaultmap (tofrom scalar) // expected-warning {{missing ':' after defaultmap modifier - ignoring}} + foo(); + #pragma omp target defaultmap (tofrom, // expected-error {{expected ')'}} expected-error {{expected 'scalar' in OpenMP clause 'defaultmap'}} expected-warning {{missing ':' after defaultmap modifier - ignoring}} expected-note {{to match this '('}} + foo(); + #pragma omp target defaultmap (scalar: // expected-error {{expected ')'}} expected-error {{expected 'tofrom' in OpenMP clause 'defaultmap'}} expected-note {{to match this '('}} + foo(); + #pragma omp target defaultmap (tofrom, scalar // expected-error {{expected ')'}} expected-warning {{missing ':' after defaultmap modifier - ignoring}} expected-error {{expected 'scalar' in OpenMP clause 'defaultmap'}} expected-note {{to match this '('}} + foo(); + + return argc; +} + +int main(int argc, char **argv) { + #pragma omp target defaultmap // expected-error {{expected '(' after 'defaultmap'}} + foo(); + #pragma omp target defaultmap ( // expected-error {{expected 'tofrom' in OpenMP clause 'defaultmap'}} expected-error {{expected ')'}} expected-note {{to match this '('}} + foo(); + #pragma omp target defaultmap () // expected-error {{expected 'tofrom' in OpenMP clause 'defaultmap'}} + foo(); + #pragma omp target defaultmap (tofrom // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-warning {{missing ':' after defaultmap modifier - ignoring}} expected-error {{expected 'scalar' in OpenMP clause 'defaultmap'}} + foo(); + #pragma omp target defaultmap (tofrom: // expected-error {{expected ')'}} expected-note {{to match this '('}} expected-error {{expected 'scalar' in OpenMP clause 'defaultmap'}} + foo(); + #pragma omp target defaultmap (tofrom) // expected-warning {{missing ':' after defaultmap modifier - ignoring}} expected-error {{expected 'scalar' in OpenMP clause 'defaultmap'}} + foo(); + #pragma omp target defaultmap (tofrom scalar) // expected-warning {{missing ':' after defaultmap modifier - ignoring}} + foo(); + #pragma omp target defaultmap (tofrom, // expected-error {{expected ')'}} expected-error {{expected 'scalar' in OpenMP clause 'defaultmap'}} expected-warning {{missing ':' after defaultmap modifier - ignoring}} expected-note {{to match this '('}} + foo(); + #pragma omp target defaultmap (scalar: // expected-error {{expected ')'}} expected-error {{expected 'tofrom' in OpenMP clause 'defaultmap'}} expected-note {{to match this '('}} + foo(
Re: [PATCH] D16526: Add hasRetValue narrowing matcher for returnStmt
aaron.ballman added a comment. In http://reviews.llvm.org/D16526#334795, @LegalizeAdulthood wrote: > In http://reviews.llvm.org/D16526#334742, @aaron.ballman wrote: > > > I get the same behavior from the existing has() matcher -- how is this > > meant to differ? > > > Hmm! Good question. I suppose it doesn't do anything different. > > I was thinking how to write such matching expressions and I looked through > the matcher reference and the header file for things related to ReturnStmt > and found only `returns`. It never occurred to me to use `has`. Not seeing > returnStmt mentioned anywhere, it seemed that I couldn't write such matching > expressions and needed a new matcher. Yes, I have definitely run into this line of thinking in the past as well. Specifically, I ran into it with cxxCatchStmt (to determine what kind of type the catch statement will handle). > So the question remains: is it bad to have a matcher that explicitly models > the narrowing for a return statement's return expression when you can achieve > the same thing with `has`? I'm of two minds on this: (1) the status quo is a bit obtuse and so a dedicated matcher might not be a bad thing, (2) AST matchers are a bit expensive due to the template metaprogramming and duplicate functionality is easy to get out of sync. I am leaning towards not duplicating AST matcher functionality because of (2) and think (1) can be corrected with some better documentation for has(), returnStmt(), and perhaps cxxCatchHandler(). The fact that there are > 1 instances where has() works but we might want a dedicated matcher also suggests that we don't want to duplicate. @klimek, what is your stance on the idea? http://reviews.llvm.org/D16526 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16480: [libcxx] NFC: suppress warning on systems where sizeof(int) == sizeof(long)
bcraig abandoned this revision. bcraig added a comment. Looks like I found an out-of-tree warning in our compiler. Sorry about the noise. Abandoning. http://reviews.llvm.org/D16480 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15225: [Driver] Sanitizer support based on runtime library presence
ygribov added a subscriber: ygribov. Comment at: test/Driver/fsanitize.c:221 @@ +220,3 @@ +// RUN: %clang -target x86_64-apple-darwin10 -resource-dir=%S/Inputs/resource_dir -fsanitize=memory -fsanitize=thread,memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MSAN-TSAN-MSAN-DARWIN1 +// CHECK-MSAN-TSAN-MSAN-DARWIN1: unsupported option '-fsanitize=thread,memory' for target 'x86_64-apple-darwin10' +// CHECK-MSAN-TSAN-MSAN-DARWIN1-NOT: unsupported option samsonov wrote: > beanz wrote: > > kubabrecka wrote: > > > samsonov wrote: > > > > Again, I feel like we're lying to users here: `-fsanitize=thread` *is* > > > > supported for this target, it just requires building a runtime. > > > I'd like to see this from the point-of-view of a binary distribution. If > > > the binary distribution (e.g. the one from llvm.org or Apple's Clang in > > > Xcode) doesn't contain a runtime library, then the sanitizer is *not* > > > supported in that distribution. Also, see > > > http://reviews.llvm.org/D14846, we'd like to have CMake options to select > > > which runtimes will be built. If you deliberately choose not to build > > > ThreadSanitizer, then that sanitizer is *not* supported in your version > > > of Clang. If you're experimenting and porting a runtime to a new > > > platform, then this sanitizer *is* supported in your version of Clang. > > Maybe the point is we should have a different error message for cases where > > the runtime is just missing. Something like "runtime components for > > '-fsanitize=thread' not available" > I see, so essentially you want to use a different approach for determining > sanitizer availability (on OS X for now): if the library is present, then we > support sanitizer, otherwise we don't: i.e. the binary distribution is the > source of truth, not the list of sanitizers hardcoded into Clang driver > source code. I'm fine with that, and see why it would make sense. > > It's just that error message looks misleading: the problem is not TSan is > unsupported for target, it's just unavailable in this distribution for one > reason or another. > the binary distribution is the source of truth, not the list of sanitizers > hardcoded into Clang driver source code. This will not work for cross-compilers. It _may_ be ok for OSX but not for other platforms. http://reviews.llvm.org/D15225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16310: new clang-tidy checker misc-long-cast
danielmarjamaki added inline comments. Comment at: clang-tidy/misc/LongCastCheck.cpp:97 @@ +96,3 @@ + + if (!CastType->isIntegerType() || !SubType->isIntegerType()) +return; LegalizeAdulthood wrote: > danielmarjamaki wrote: > > LegalizeAdulthood wrote: > > > Why don't you check for casting a `float` expression to a `double` or > > > `long double`? > > > > > > Isn't this the exact same issue? > > > > > > If so, add a test case for casting a `float` expression to `double` and a > > > test case for casting a `double` expression to a `long double`. > > in theory yes.. but somehow that feels strange to me. yes there will > > possibly be loss of precision in some decimals, that is normal when using > > floating point numbers. if such loss of precision would be unwanted then > > float should be avoided to start with. > > > > so I do agree in theory but I don't think I would feel good about adding > > such warnings. > > > For floating-point quantities, when I think of the term "precision" I am > thinking of the number of bits allocated to the mantissa. This may or may > not be correct terminology according to floating-point experts, but from what > I can tell it seems to agree with how the term is used on wikipedias [[ > https://en.wikipedia.org/wiki/Floating_point | article on floating-point ]]. > > So while digits are technically lost when we add a small floating-point > quantity to a large floating-point quantity (the large quantity gobbles up > all the bits of the mantissa and the small quantity has its least-significant > mantissa bits discarded), the precision of the result isn't changed -- the > number of bits in the mantissa is the same for the result as it was for the > inputs. > > To then take a quantity of N bits of mantissa and cast that to a quantity of > M bits of mantissa where M > N seems just as pointless as the approach of > casting an `int` to a `long`. The extra tokens for casting do absolutely > nothing and are redundant as far as the computation as written is concerned. It does feel strange to me. But I think you are technically right. I can implement it and see what warnings that generates. http://reviews.llvm.org/D16310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16310: new clang-tidy checker misc-long-cast
danielmarjamaki marked 5 inline comments as done. danielmarjamaki added a comment. http://reviews.llvm.org/D16310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16308: clang-tidy Enhance readability-simplify-boolean-expr check to handle implicit conversions of integral types to bool and member pointers
aaron.ballman added inline comments. Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.cpp:192 @@ +191,3 @@ + const Expr *E, bool Negated) { + return compareExpressionToConstant(Result, E, Negated, "nullptr"); +} Can we add a test to ensure that we aren't suggesting nullptr for a C program? Perhaps we could simply substitute nullptr for >= C++11 and NULL for everything else? Comment at: clang-tidy/readability/SimplifyBooleanExprCheck.h:77 @@ -74,3 +76,3 @@ /// implicit conversion of `i & 1` to `bool` and becomes -/// `bool b = static_cast(i & 1);`. +/// `bool b = i & 1 != 0;`. /// Perhaps a different idea regarding parens isn't to add/remove parens in various checks for readability, but instead have two readability checks that do different things (both disabled by default?): (1) remove spurious parens where the presence of the parens has no effect on the order of evaluation of the subexpressions, and (2) add parens where there is an operator precedence difference between the operators of two subexpressions. Both of these checks are at odds with one another (which is why I don't think it makes sense to enable them by default), but both certainly seem like they would be useful. Thoughts on the general idea (not trying to sign either of us up for work to implement it)? Comment at: docs/clang-tidy/checks/readability-simplify-boolean-expr.rst:59 @@ -56,3 +58,3 @@ 4. The conditional return ``if (p) return true; return false;`` has an implicit conversion of a pointer to ``bool`` and becomes I guess I view it differently; this isn't being hyper-anal, it's precisely stating what we support so that the user doesn't have to guess. However, I'm also fine leaving it out because it isn't likely to cause confusion for too long -- a simple test gives the answer if anyone is wondering. http://reviews.llvm.org/D16308 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16310: new clang-tidy checker misc-long-cast
danielmarjamaki removed rL LLVM as the repository for this revision. danielmarjamaki updated this revision to Diff 45854. danielmarjamaki added a comment. Fixed review comments http://reviews.llvm.org/D16310 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/MisplacedWideningCastCheck.cpp clang-tidy/misc/MisplacedWideningCastCheck.h docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/misc-misplaced-widening-cast.rst test/clang-tidy/misc-misplaced-widening-cast.cpp Index: test/clang-tidy/misc-misplaced-widening-cast.cpp === --- test/clang-tidy/misc-misplaced-widening-cast.cpp +++ test/clang-tidy/misc-misplaced-widening-cast.cpp @@ -0,0 +1,64 @@ +// RUN: %check_clang_tidy %s misc-misplaced-widening-cast %t + +void assign(int a, int b) { + long l; + + l = a * b; + l = (long)(a * b); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' is ineffective, or there is loss of precision before the conversion [misc-misplaced-widening-cast] + l = (long)a * b; + + l = (long)(a << 8); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' + l = (long)b << 8; + + l = static_cast(a * b); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' is ineffective, or there is loss of precision before the conversion [misc-misplaced-widening-cast] +} + +void init(unsigned int n) { + long l = (long)(n << 8); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'unsigned int' +} + +long ret(int a) { + return (long)(a * 1000); + // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: either cast from 'int' to 'long' +} + +void dontwarn1(unsigned char a, int i, unsigned char *p) { + long l; + // the result is a 9 bit value, there is no truncation in the implicit cast + l = (long)(a + 15); + // the result is a 12 bit value, there is no truncation in the implicit cast + l = (long)(a << 4); + // the result is a 3 bit value, there is no truncation in the implicit cast + l = (long)((i%5)+1); + // the result is a 16 bit value, there is no truncation in the implicit cast + l = (long)(((*p)<<8) + *(p+1)); +} + +template struct DontWarn2 { + void assign(T a, T b) { +long l; +l = (long)(a * b); + } +}; +DontWarn2 DW2; + +// Cast is not suspicious when casting macro +#define A (X<<2) +long macro1(int X) { + return (long)A; +} + +// Don't warn about cast in macro +#define B(X,Y) (long)(X*Y) +long macro2(int x, int y) { + return B(x,y); +} + +void floatingpoint(float a, float b) { + double d = (double)(a * b); // currently we don't warn for this +} + Index: docs/clang-tidy/checks/misc-misplaced-widening-cast.rst === --- docs/clang-tidy/checks/misc-misplaced-widening-cast.rst +++ docs/clang-tidy/checks/misc-misplaced-widening-cast.rst @@ -0,0 +1,39 @@ +.. title:: clang-tidy - misc-misplaced-widening-cast + +misc-misplaced-widening-cast +== + +This check will warn when there is a explicit redundant cast of a calculation +result to a bigger type. If the intention of the cast is to avoid loss of +precision then the cast is misplaced, and there can be loss of precision. +Otherwise the cast is ineffective. + +Example code:: + +long f(int x) { +return (long)(x*1000); +} + +The result x*1000 is first calculated using int precision. If the result +exceeds int precision there is loss of precision. Then the result is casted to +long. + +If there is no loss of precision then the cast can be removed or you can +explicitly cast to int instead. + +If you want to avoid loss of precision then put the cast in a proper location, +for instance:: + +long f(int x) { +return (long)x * 1000; +} + +Floating point +-- + +Currently warnings are only written for integer conversion. No warning is +written for this code:: + +double f(float x) { +return (double)(x * 10.0f); +} Index: docs/clang-tidy/checks/list.rst === --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -51,6 +51,7 @@ misc-inefficient-algorithm misc-macro-parentheses misc-macro-repeated-side-effects + misc-misplaced-widening-cast misc-move-constructor-init misc-new-delete-overloads misc-noexcept-move-constructor Index: clang-tidy/misc/MisplacedWideningCastCheck.h === --- clang-tidy/misc/MisplacedWideningCastCheck.h +++ clang-tidy/misc/MisplacedWideningCastCheck.h @@ -0,0 +1,33 @@ +//===--- MisplacedWideningCastCheck.h - clang-tidy---*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===---
Re: [PATCH] D11035: trivial patch, improve constness
danielmarjamaki closed this revision. danielmarjamaki added a comment. Thanks! Committed with r258673 http://reviews.llvm.org/D11035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16310: new clang-tidy checker misc-long-cast
danielmarjamaki marked 13 inline comments as done. danielmarjamaki added a comment. http://reviews.llvm.org/D16310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15977: [Clang] Supporting all entities declared in lexical scope in LLVM debug info
aaboud marked an inline comment as done. aaboud added a comment. Thanks David for the feedback. I have some answers below, please let me know if they are not sufficient or you are still have concerns. Comment at: lib/CodeGen/CGDebugInfo.cpp:1479 @@ +1478,3 @@ +void CGDebugInfo::recordDeclarationLexicalScope(const Decl &D) { + assert(LexicalBlockMap.find(&D) == LexicalBlockMap.end() && + "D is already mapped to lexical block scope"); dblaikie wrote: > Dose this assert not fire if there are two decls in the same lexical scope? Why two decls would have the same object instance "Decl"? I am not sure even that you can declare two decls with same name at same lexical-scope, but even if they are defined in the same lexical scope, they will have different line numbers, right? Comment at: lib/CodeGen/CGDebugInfo.cpp:1481 @@ +1480,3 @@ + "D is already mapped to lexical block scope"); + if (!LexicalBlockStack.empty()) +LexicalBlockMap[&D] = LexicalBlockStack.back(); dblaikie wrote: > Should we assert that LexicalBlockStack isn't empty? It might be empty, if the declaration is in the program scope, right? So, we do not want to assert, just to check. Comment at: lib/CodeGen/CGDebugInfo.cpp:1488 @@ +1487,3 @@ + auto I = LexicalBlockMap.find(&D); + if (I != LexicalBlockMap.end()) { +RetainedTypes.push_back(Ty.getAsOpaquePtr()); dblaikie wrote: > Perhaps we should assert here that if D's scope is a lexical block, that it > should have a corresponding entry in the LexicalBlockMap? We will get to this function also for D's that are not in lexical scope. We should not assert, that is also why we have the else handling (which is how we used to get context before). I can agree with you that the function name is misleading, and I am open to other suggestions. Comment at: lib/CodeGen/CGDebugInfo.cpp:1489 @@ +1488,3 @@ + if (I != LexicalBlockMap.end()) { +RetainedTypes.push_back(Ty.getAsOpaquePtr()); +return I->second; dblaikie wrote: > Why does the type need to be added to the retained types list? Excellent question :) This was your suggestion few months ago. The reason is that backend will need to know what types are declared in lexical blocks. Imported entities are collected in the import entity list, while types are collected in the retained types. Check line 518 at DwarfDebug.cpp (http://reviews.llvm.org/D15976#51cfb106) Comment at: lib/CodeGen/CGDebugInfo.cpp:2066 @@ -2045,3 +2065,3 @@ if (isImportedFromModule || !ED->getDefinition()) { -llvm::DIScope *EDContext = getDeclContextDescriptor(ED); +llvm::DIScope *EDContext = getDeclarationLexicalScope(*ED, QualType(Ty, 0)); llvm::DIFile *DefUnit = getOrCreateFile(ED->getLocation()); dblaikie wrote: > Would it be reasonable/possible to do these changes one at a time & > demonstrate the different situations in which types were not correctly nested > in lexical scopes? Or are they all necessary for some correctness/invariant > that must hold? Clang changes cannot be committed before the LLVM changes, otherwise we would crash in backend. Regarding splitting Clang changes into several comments it is possible with the following splits: 1. Static variable (lines 2508-2517) 2. Types (all the others) Now types can be split into several comments as well: Records and Typedef But the infrastructure of recording lexical-scope declarations and getting the lexical-scope context should be committed with the first patch (nevertheless what one it would be, Records or Typedef). Regarding the test, I used to have 3 (even 4) separate tests, but I was asked to merged them. Now, if we do commit patches in steps, I suggest that I still end up with one final test, but I will need to modify it with each patch (I will be building it step by step). Now, what is your call? do you want me to commit the this patch in 3 steps? Or just go with it as one piece? http://reviews.llvm.org/D15977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16259: Add clang-tidy readability-redundant-control-flow check
aaron.ballman added inline comments. Comment at: clang-tidy/readability/RedundantControlFlowCheck.cpp:24 @@ +23,3 @@ +const char *const RedundantReturnDiag = +"redundant return statement at the end of void function"; +const char *const RedundantContinueDiag = void function -> function with a void return type. Comment at: clang-tidy/readability/RedundantControlFlowCheck.cpp:45 @@ +44,3 @@ + Finder->addMatcher(whileStmt(CompoundContinue), this); + Finder->addMatcher(doStmt(CompoundContinue), this); +} Instead of adding four different matchers, it would be better to add one matcher with anyOf(). e.g., ``` Finder->addMatcher(stmt(anyOf(forStmt(), whileStmt(), doStmt(), cxxForRangeStmt()), has(compoundStmt(hasAnySubstatement(continueStmt(), this); ``` Comment at: clang-tidy/readability/RedundantControlFlowCheck.cpp:59 @@ +58,3 @@ + CompoundStmt::const_reverse_body_iterator last = Block->body_rbegin(); + if (const auto *Return = dyn_cast(*last)) { +issueDiagnostic(Result, Block, Return->getSourceRange(), Elide braces. Comment at: clang-tidy/readability/RedundantControlFlowCheck.cpp:68 @@ +67,3 @@ + CompoundStmt::const_reverse_body_iterator last = Block->body_rbegin(); + if (const auto *Continue = dyn_cast(*last)) { +issueDiagnostic(Result, Block, Continue->getSourceRange(), Elide braces http://reviews.llvm.org/D16259 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16533: Bug 20796 - GCC's -Wstrict-prototypes warning not implemented in Clang
ptitei created this revision. ptitei added a reviewer: cfe-commits. Implementation for C only warning -Wstrict-prototypes. Function declarations which have no parameters specified are diagnosed and also K&R function definitions with more than 0 parameters which are not preceded by previous prototype declaration. http://reviews.llvm.org/D16533 Files: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDecl.cpp lib/Sema/SemaType.cpp test/Sema/warn-strict-prototypes.c Index: test/Sema/warn-strict-prototypes.c === --- test/Sema/warn-strict-prototypes.c +++ test/Sema/warn-strict-prototypes.c @@ -0,0 +1,78 @@ +// RUN: %clang_cc1 -fsyntax-only -Wstrict-prototypes -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wstrict-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +// function declaration with unspecified params +void foo1(); // expected-warning{{this function declaration is not a prototype}} +// CHECK: fix-it:"{{.*}}":{5:11-5:11}:"void" +// function declaration with 0 params +void foo2(void); + +// function definition with 0 params(for both cases), valid according to 6.7.5.3/14 +void foo1() {} +void foo2(void) {} + +// function type typedef unspecified params +typedef void foo3(); // expected-warning{{this function declaration is not a prototype}} +// CHECK: fix-it:"{{.*}}":{15:19-15:19}:"void" +void bar1(void) { + foo3 *fp = 0; + (*fp)(); +} + +// global fp unspecified params +void (*foo4)(); // expected-warning{{this function declaration is not a prototype}} +// CHECK: fix-it:"{{.*}}":{23:14-23:14}:"void" + +// struct member fp unspecified params +struct { void (*foo5)(); } s; // expected-warning{{this function declaration is not a prototype}} + // CHECK: fix-it:"{{.*}}":{27:23-27:23}:"void" + +// param fp unspecified params +void bar2(void (*foo6)()) { // expected-warning{{this function declaration is not a prototype}} +// CHECK: fix-it:"{{.*}}":{31:24-31:24}:"void" + // local fp unspecified params + void (*foo7)() = 0; // expected-warning{{this function declaration is not a prototype}} + // CHECK: fix-it:"{{.*}}":{34:16-34:16}:"void" + // array fp unspecified params + void (*foo8[2])() = {0}; // expected-warning{{this function declaration is not a prototype}} + // CHECK: fix-it:"{{.*}}":{37:19-37:19}:"void" + // use them + foo4(); + s.foo5(); + foo6(); + foo7(); + foo8[0](); +} + +// function type cast using using an anonymous function declaration +void bar3(int a) { + // casting function w/out prototype to unspecified params function type + (void)(void(*)()) foo1; // expected-warning{{this function declaration is not a prototype}} + // CHECK: fix-it:"{{.*}}":{50:18-50:18}:"void" + // .. specified params + (void)(void(*)(void)) foo1; + + // casting function w/ prototype to unspecified params function type + (void)(void(*)()) foo2; // expected-warning{{this function declaration is not a prototype}} + // CHECK: fix-it:"{{.*}}":{56:18-56:18}:"void" + // .. specified params + (void)(void(*)(void)) foo2; +} + +// K&R function definition not preceded by full prototype +int foo9(a, b) // expected-warning{{old-style function definition is not preceded by a prototype}} + int a, b; +{ + return a + b; +} + +// Function declaration with no types +void foo10(); // expected-warning{{this function declaration is not a prototype}} + // CHECK: fix-it:"{{.*}}":{70:12-70:12}:"void" +// K&R function definition with incomplete param list declared +void foo10(p, p2) void *p; {} // expected-warning{{old-style function definition is not preceded by a prototype}} + +// Prototype declaration +void foo11(int p, int p2); +// K&R function definition with previous prototype declared is not diagnosed. +void foo11(p, p2) int p; int p2; {} Index: lib/Sema/SemaType.cpp === --- lib/Sema/SemaType.cpp +++ lib/Sema/SemaType.cpp @@ -3919,6 +3919,20 @@ if (FTI.isAmbiguous) warnAboutAmbiguousFunction(S, D, DeclType, T); + // GNU warning -Wstrict-prototypes + // Warn if function declaration is without prototype. + // This warning is issued for all kinds of unprototyped function + // declarations (i.e. function type typedef, function pointer etc.) + // C99 6.7.5.3p14: + // The empty list in a function declarator that is not part of a + // definition of that function specifies that no information + // about the number or types of the parameters is supplied. + if (D.getFunctionDefinitionKind() == FDK_Declaration && + FTI.NumParams == 0 && !LangOpts.CPlusPlus) { + S.Diag(DeclType.Loc, diag::warn_strict_prototypes) << 0 + << FixItHint::CreateInsertion(FTI.getRParenLoc(), "void
Re: [PATCH] D16286: [clang-tidy] Readability check for redundant parenthesis in return expression.
aaron.ballman added a comment. In http://reviews.llvm.org/D16286#333911, @LegalizeAdulthood wrote: > As a general comment, I am continually fascinated by the **huge** variety of > opinion as to matters of readability within the C++ community. I don't know > why we have so many differing opinions compared to the communities built > around other programming languages, but I enjoy hearing all of them. I enjoy > having my opinions on readability challenged because it forces me to think > **why** I find one form preferable over another and that is a good thing. Likewise! I think these discussions have been great (and I apologize if they are a source of frustration while we wrestle with the details). > > > In http://reviews.llvm.org/D16286#330390, @aaron.ballman wrote: > > > In http://reviews.llvm.org/D16286#330360, @LegalizeAdulthood wrote: > > > > > One could argue that this check should be part of the google module. The > > > google style guide specifically forbids writing return with extra > > > parenthesis. > > > > > > If it is meant to work for a particular style guide, then it should > > absolutely go under that style guide. But for something more generally > > under the readability-* umbrella, I think it doesn't hurt to discuss where > > to draw the line, or what options may be valuable. > > > I think that is the correct conclusion here; according to the bug report this > check is motivated by the Google style guide which explicitly prohibits the > parens for return expressions. Then perhaps this should go under google-* instead of readability-*? However, I posed an idea in a different review about a more general solution to the problem that may make sense under readability-* with specific options for an alias under google-*. Perhaps it would make sense to make this a general "strip *all* parens that don't have an effect on the order of evaluation in all expressions" check, which leaves room for an additional (different) check in the future that says "put parens in anywhere based on this heuristic we can debate in the future". I would say that these two checks should be off-by-default since they will definitely conflict, but I think the alias under google-* should be on by default and only focus on return statements (or whatever else the google style guide requires). (Not saying that you have to implement this -- I'm just exploring the design space. The initial implementation of this idea could simply be to take your existing check and put it under google-* and we can refactor some day in the future for a more generalized readability-* check.) http://reviews.llvm.org/D16286 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16113: [clang-tdiy] Add header file extension configuration support.
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you for working on this! http://reviews.llvm.org/D16113 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16529: [clang-tidy] Add modernize-raw-string-literal check
aaron.ballman added a subscriber: aaron.ballman. Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:35 @@ +34,3 @@ + const bool HasBackSlash = Text.find(R"(\\)") != StringRef::npos; + const bool HasNewLine = Text.find(R"(\n)") != StringRef::npos; + const bool HasQuote = Text.find(R"(\')") != StringRef::npos; Newlines are hard. Should this also handle \r (for CRLF code like \r\n)? Comment at: clang-tidy/modernize/RawStringLiteralCheck.cpp:66 @@ +65,3 @@ +void RawStringLiteralCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(stringLiteral().bind("lit"), this); +} Please limit registration of the checker to just C++11 or later. Comment at: test/clang-tidy/modernize-raw-string-literal.cpp:18 @@ +17,3 @@ +// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: {{.*}} can be written as a raw string literal +// CHECK-FIXES: {{^}}char const *const NewLine{R"(goink{{$}} +// CHECK-FIXES-NEXT: {{^}}frob)"};{{$}} Is there a way that this fix-it can mangle the source file in scary ways, though? For instance, say I have a source file saved with CRLF and a string literal of "foo\nbar" -- this will either make the string literal contain a CRLF instead of just LF, or it will make the string literal contain an LF right up until the user's editor saves the file and converts the line ending (I think). Comment at: test/clang-tidy/modernize-raw-string-literal.cpp:55 @@ +54,2 @@ +// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: {{.*}} can be written as a raw string literal +// CHECK-FIXES: {{^}}char const *const HexPrintable{R"(@\)"};{{$}} Can you also add a test that ensures we don't mangle already-raw string literals (by re-rawifying them)? http://reviews.llvm.org/D16529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16152: [clang-tidy] Add check performance-faster-string-find
aaron.ballman added inline comments. Comment at: clang-tidy/performance/FasterStringFindCheck.cpp:30 @@ +29,3 @@ +Class = Class.trim(); +if (!Class.empty()) + Result.push_back(Class); alexfh wrote: > aaron.ballman wrote: > > > Also changed the separator to be ';' instead of ','. > > > The latter could be part of a type name. Eg. Foo::Bar > > > > That's a great catch! > > > > @alexfh -- do you think we should try to get our checkers to be consistent > > with their choice of list separators? Specifically, I am thinking about > > D16113. It seems like it would improve the user experience to not have to > > wonder "do I use comma, or semi colon, for this list?" > We can try to be consistent with the choice of delimiters, but I'm not sure > we can use the same delimiter always without introducing more syntax > complexities like escaping or the use of quotes (like in CSV). In any case, > we should clearly document the format for each option and think how we can > issue diagnostics when we suspect incorrect format used in the option value. I think we should strive for consistency with ";" for right now, and agree that documentation/assistance is a reasonable fallback. http://reviews.llvm.org/D16152 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16360: unordered_map: Avoid unnecessary mallocs when no insert occurs
mclow.lists added a comment. I don't have any comments on this code at this time, but I want to caution people that this part of the standard library is **extremely** carefully specified, and meeting all the requirements is a fiddly bit of work. For an example of this, look at LWG issue #2464, which has been added to the draft C++17 standard. http://reviews.llvm.org/D16360 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16376: clang-tidy check: User-defined copy without assignment
aaron.ballman added inline comments. Comment at: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp:22 @@ +21,3 @@ +void UserDefinedCopyWithoutAssignmentCheck::registerMatchers( +MatchFinder *Finder) { + Finder->addMatcher( Before registering the matchers, can you early return if not in CPlusPlus mode? (No need to register these matchers for C code.) Comment at: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.cpp:63 @@ +62,3 @@ +void UserDefinedCopyWithoutAssignmentCheck::check( +const MatchFinder::MatchResult &Result) { + if (const auto *MatchedDecl = This seems to have a lot of duplicate code that could be consolidated. Comment at: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h:21 @@ +20,3 @@ +/// +/// MSVC 2015 will generate an assignment operator even if the user defines a +/// copy-constructor. This check finds classes with user-defined Should update these comments to reflect the check's behavior better (and remove mention of MSVC since this isn't specific to that compiler). Comment at: clang-tidy/misc/UserDefinedCopyWithoutAssignmentCheck.h:28 @@ +27,3 @@ +/// http://clang.llvm.org/extra/clang-tidy/checks/misc-user-defined-copy-without-assignment.html +class UserDefinedCopyWithoutAssignmentCheck : public ClangTidyCheck { public: + UserDefinedCopyWithoutAssignmentCheck(StringRef Name, ClangTidyContext This no longer is about just copy, so you may want to rename the class (and files and check) to something different. Comment at: docs/clang-tidy/checks/misc-user-defined-copy-without-assignment.rst:8 @@ +7,3 @@ +constructor. This behaviour is deprecated by the standard (C++ 14 draft +standard 12.8.18) + Best to use the tags ([class.copy] paragraph 18) when referring to the standard, since the numbers can change. http://reviews.llvm.org/D16376 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14653: [libcxx] Introduce the mechanism for fixing -fno-exceptions test failures.
rmaprath updated this revision to Diff 45868. rmaprath added a comment. Re-based on trunk. Trivial. http://reviews.llvm.org/D14653 Files: include/__config include/array test/std/containers/sequences/array/at.pass.cpp test/support/noexcept.h test/support/test_allocator.h Index: test/support/test_allocator.h === --- test/support/test_allocator.h +++ test/support/test_allocator.h @@ -65,13 +65,8 @@ pointer allocate(size_type n, const void* = 0) { assert(data_ >= 0); -if (time_to_throw >= throw_after) { -#ifndef _LIBCPP_NO_EXCEPTIONS -throw std::bad_alloc(); -#else -std::terminate(); -#endif -} +if (time_to_throw >= throw_after) +_LIBCPP_THROW(std::bad_alloc(), "std::bad_alloc"); ++time_to_throw; ++alloc_count; return (pointer)::operator new(n * sizeof(T)); @@ -125,13 +120,8 @@ pointer allocate(size_type n, const void* = 0) { assert(data_ >= 0); -if (time_to_throw >= throw_after) { -#ifndef _LIBCPP_NO_EXCEPTIONS -throw std::bad_alloc(); -#else -std::terminate(); -#endif -} +if (time_to_throw >= throw_after) +_LIBCPP_THROW(std::bad_alloc(), "std::bad_alloc"); ++time_to_throw; ++alloc_count; return (pointer)::operator new (n * sizeof(T)); Index: test/support/noexcept.h === --- /dev/null +++ test/support/noexcept.h @@ -0,0 +1,61 @@ +// -*- C++ -*- +//===- noexcept.h -===// +// +// The LLVM Compiler Infrastructure +// +// This file is dual licensed under the MIT and the University of Illinois Open +// Source Licenses. See LICENSE.TXT for details. +// +//===--===// +#ifndef NOEXCEPT_H +#define NOEXCEPT_H + +// These helper macros enable writing tests for the standard library exceptions +// (i.e. tests that check the library throws where it should) while at the same +// time catering for the no-exceptions library variant. In the no-exceptions +// variant, the macros translate into a setjmp/longjmp duo that mimic the +// control flow of exceptions. The goal here is to allow the tests to verify +// that the library called __libcxx_noexceptions_abort() at the point where it +// originally threw. Note that the longjmp (and hence the setjmp) is necessary +// here as we cannot allow the execution to proceed past the point where the +// library detects an error. Finally, this simple translation does not work when +// tests use multiple catch statements, in those cases we have to use the +// _LIBCPP_NO_EXCEPTIONS macro and exclude the additional catch statements. +#ifndef _LIBCPP_NO_EXCEPTIONS +#define TEST_TRY try +#define TEST_CATCH(E) catch (E) +#define TEST_THROW(E) throw E +#else +#define TEST_TRY if (!setjmp(try_buf)) +#define TEST_CATCH(E) else +#define TEST_THROW(E) __libcxx_noexceptions_abort("exception") + +#include +#include +#include "test_macros.h" + +#ifndef _LIBCPP_HAS_NO_THREADS +# if TEST_STD_VER >= 11 +# define TLS_SPEC thread_local +# elif defined(_LIBCPP_MSVC) +# define TLS_SPEC __declspec(thread) +# else +# define TLS_SPEC __thread +# endif +#else +# define TLS_SPEC +#endif + +// Some tests launch multiple threads, in which case we need to make sure that +// try_buf is maintained per-thread, otherwise setjmp()/longjmp() will attempt +// to jump between threads! +TLS_SPEC jmp_buf try_buf; +#undef TLS_SPEC + +void __libcxx_noexceptions_abort(const char *msg) { + fprintf(stderr, "%s\n", msg); + longjmp(try_buf, 1); +} +#endif // _LIBCPP_NO_EXCEPTIONS + +#endif // NOEXCEPT_H Index: test/std/containers/sequences/array/at.pass.cpp === --- test/std/containers/sequences/array/at.pass.cpp +++ test/std/containers/sequences/array/at.pass.cpp @@ -7,7 +7,6 @@ // //===--===// -// XFAIL: libcpp-no-exceptions // // reference operator[] (size_type) @@ -19,6 +18,7 @@ #include #include "test_macros.h" +#include "noexcept.h" // std::array is explicitly allowed to be initialized with A a = { init-list };. // Disable the missing braces warning for this reason. @@ -40,8 +40,8 @@ r2 = 7.5; assert(c.back() == 7.5); -try { (void) c.at(3); } -catch (const std::out_of_range &) {} +TEST_TRY { (void) c.at(3); } +TEST_CATCH (const std::out_of_range &) {} } { typedef double T; @@ -53,8 +53,8 @@ C::const_reference r2 = c.at(2); assert(r2 == 3.5); -try { (void) c.at(3); } -catch (const std::out_of_r
Re: [PATCH] D15225: [Driver] Sanitizer support based on runtime library presence
kubabrecka added inline comments. Comment at: test/Driver/fsanitize.c:221 @@ +220,3 @@ +// RUN: %clang -target x86_64-apple-darwin10 -resource-dir=%S/Inputs/resource_dir -fsanitize=memory -fsanitize=thread,memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MSAN-TSAN-MSAN-DARWIN1 +// CHECK-MSAN-TSAN-MSAN-DARWIN1: unsupported option '-fsanitize=thread,memory' for target 'x86_64-apple-darwin10' +// CHECK-MSAN-TSAN-MSAN-DARWIN1-NOT: unsupported option ygribov wrote: > samsonov wrote: > > beanz wrote: > > > kubabrecka wrote: > > > > samsonov wrote: > > > > > Again, I feel like we're lying to users here: `-fsanitize=thread` > > > > > *is* supported for this target, it just requires building a runtime. > > > > I'd like to see this from the point-of-view of a binary distribution. > > > > If the binary distribution (e.g. the one from llvm.org or Apple's Clang > > > > in Xcode) doesn't contain a runtime library, then the sanitizer is > > > > *not* supported in that distribution. Also, see > > > > http://reviews.llvm.org/D14846, we'd like to have CMake options to > > > > select which runtimes will be built. If you deliberately choose not to > > > > build ThreadSanitizer, then that sanitizer is *not* supported in your > > > > version of Clang. If you're experimenting and porting a runtime to a > > > > new platform, then this sanitizer *is* supported in your version of > > > > Clang. > > > Maybe the point is we should have a different error message for cases > > > where the runtime is just missing. Something like "runtime components for > > > '-fsanitize=thread' not available" > > I see, so essentially you want to use a different approach for determining > > sanitizer availability (on OS X for now): if the library is present, then > > we support sanitizer, otherwise we don't: i.e. the binary distribution is > > the source of truth, not the list of sanitizers hardcoded into Clang driver > > source code. I'm fine with that, and see why it would make sense. > > > > It's just that error message looks misleading: the problem is not TSan is > > unsupported for target, it's just unavailable in this distribution for one > > reason or another. > > the binary distribution is the source of truth, not the list of sanitizers > > hardcoded into Clang driver source code. > > This will not work for cross-compilers. It _may_ be ok for OSX but not for > other platforms. Why not? On Linux, there are statically-linked sanitizers, if you want to cross-compile, you need to have the runtime libraries for the target. And dynamic libraries are a similar story – they're version-tied to the compiler and your compiler should really have the libraries of the sanitizers that it supports. http://reviews.llvm.org/D15225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15862: A possible direction for fixing https://llvm.org/bugs/show_bug.cgi?id=25973.
mclow.lists closed this revision. mclow.lists added a comment. This was landed as r257682 http://reviews.llvm.org/D15862 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16535: [clang-tidy] Check to find unintended semicolons that changes the semantics.
xazax.hun created this revision. xazax.hun added a reviewer: alexfh. xazax.hun added subscribers: dkrupp, cfe-commits. This patch adds a checker to find semicolons that are probably unintended and modify the semantics of the program. See the examples and the documentation for more details. http://reviews.llvm.org/D16535 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/SuspiciousSemicolonCheck.cpp clang-tidy/misc/SuspiciousSemicolonCheck.h docs/clang-tidy/checks/list.rst docs/clang-tidy/checks/misc-suspicious-semicolon.rst test/clang-tidy/misc-suspicious-semicolon.cpp Index: test/clang-tidy/misc-suspicious-semicolon.cpp === --- /dev/null +++ test/clang-tidy/misc-suspicious-semicolon.cpp @@ -0,0 +1,95 @@ +// RUN: %check_clang_tidy %s misc-suspicious-semicolon %t + +int x = 5; + +void nop(); + +void correct1() +{ + if(x < 5) nop(); +} + +void correct2() +{ + if(x == 5) + nop(); +} + +void correct3() +{ + if(x > 5) + { + nop(); + } +} + +void fail1() +{ + if(x > 5); nop(); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: potentially unintended semicolon [misc-suspicious-semicolon] + // CHECK-FIXES: if(x > 5) nop(); +} + +void fail2() +{ + if(x == 5); + nop(); + // CHECK-MESSAGES: :[[@LINE-2]]:12: warning: potentially unintended semicolon [misc-suspicious-semicolon] + // CHECK-FIXES: if(x == 5){{$}} +} + +void fail3() +{ + if(x < 5); + { + nop(); + } + // CHECK-MESSAGES: :[[@LINE-4]]:11: warning: potentially unintended semicolon + // CHECK-FIXES: if(x < 5){{$}} +} + +void correct4() +{ + while(x % 5 == 1); + nop(); +} + +void correct5() +{ + for(int i = 0; i < x; ++i) + ; +} + +void fail4() +{ + for(int i = 0; i < x; ++i); + nop(); + // CHECK-MESSAGES: :[[@LINE-2]]:28: warning: potentially unintended semicolon + // CHECK-FIXES: for(int i = 0; i < x; ++i){{$}} +} + +void fail5() +{ + if(x % 5 == 1); + nop(); + // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: potentially unintended semicolon + // CHECK-FIXES: if(x % 5 == 1){{$}} +} + +void correct6() +{ + do; while(false); +} + +int correct7() +{ + int t_num = 0; + char c = 'b'; + char * s = "a"; + if (s == "(" || s != "'" || c == '"') { +t_num += 3; +return (c == ')' && c == '\''); + } + + return 0; +} Index: docs/clang-tidy/checks/misc-suspicious-semicolon.rst === --- /dev/null +++ docs/clang-tidy/checks/misc-suspicious-semicolon.rst @@ -0,0 +1,72 @@ +.. title:: clang-tidy - misc-suspicious-semicolon + +misc-suspicious-semicolon += + +Finds most instances of stray semicolons that unexpectedly alter the meaning of +the code. More specifically, it looks for `if`, `while`, `for` and `for-range` +statements whose body is a single semicolon, and then analyzes the +context of the code (e.g. indentation) in an attempt to determine whether that +is intentional. + + .. code-block:: c++ + +if(x < y); +{ + x++; +} + +Here the body of the `if` statement consists of only the semicolon at the end of +the first line, and `x` will be increased regardless of the condition. + + + .. code-block:: c++ + +while((line = readLine(file)) != NULL); + processLine(line); + +As a result of this code, `processLine()` will only be called once, when the +`while` loop with the empty body exits with `line == NULL`. The identation of +the code indicates the intention of the programmer. + + + .. code-block:: c++ + +if(x >= y); +x -= y; + +While the indentation does not imply any nesting, there is simply no valid +reason to have an `if` statement with an empty body (but it can make sense for +a loop). + +To solve the issue remove the stray semicolon or in case the empty body is +intentional, reflect this using code indentation or put the semicolon in a new +line. For example: + + .. code-block:: c++ + +while(readWhitespace()); + Token t = readNextToken(); + +Here the second line is indented in a way that suggests that it is meant to be +the body of the `while` loop - whose body is in fact empty, becasue of the +semicolon at the end of the first line. + +Either remove the indentation from the second line: + + .. code-block:: c++ + +while(readWhitespace()); +Token t = readNextToken(); + +... or move the semicolon from the end of the first line to a new line: + + .. code-block:: c++ + +while(readWhitespace()) + ; + + Token t = readNextToken(); + +In this case the checker will assume that you know what you are doing, and will +not raise a warning. Index: docs/clang-tidy/checks/list.rst === --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -58,6 +58,7 @@ misc-sizeof-container misc-static-assert misc-string-integer-assignment + misc-suspicious-semicolon misc-swapped-arguments misc-throw-
Re: [PATCH] D8149: Extend hasType narrowing matcher for TypedefDecls, add functionProtoType matcher for FunctionProtoType nodes, extend parameterCountIs to FunctionProtoType nodes
aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM with one addition, I still would like to see this test added and passing: EXPECT_TRUE(matchesC("void f();", functionDecl(parameterCountIs(0; Thank you for working on these! http://reviews.llvm.org/D8149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16467: [libcxx] re.results.form: Format out-of-range subexpression references as null
mclow.lists added a comment. Not crashing is good; but silently returning a "wrong" answer rubs me the wrong way. I'll do some research and get back to you on this, but maybe throwing an exception is a better response. The code looks fine, though; I'm just not sure it's the right thing to do. http://reviews.llvm.org/D16467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16533: Bug 20796 - GCC's -Wstrict-prototypes warning not implemented in Clang
Thanks for working on this! On Mon, Jan 25, 2016 at 03:18:31PM +, Paul Titei via cfe-commits wrote: > +// function definition with 0 params(for both cases), valid according to > 6.7.5.3/14 > +void foo1() {} I still want to get a warning for this. At best it is inconsistent. > +// Function declaration with no types > +void foo10(); // expected-warning{{this function declaration is not a > prototype}} > + // CHECK: fix-it:"{{.*}}":{70:12-70:12}:"void" > +// K&R function definition with incomplete param list declared > +void foo10(p, p2) void *p; {} // expected-warning{{old-style function > definition is not preceded by a prototype}} > + > +// Prototype declaration > +void foo11(int p, int p2); > +// K&R function definition with previous prototype declared is not diagnosed. > +void foo11(p, p2) int p; int p2; {} Same here. I'm perfectly happy if that is a separate option though. Joerg ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16536: Fix crashing on user-defined conversion.
congliu created this revision. congliu added a reviewer: alexfh. congliu added a subscriber: cfe-commits. Fix the assertion failure for the user-defined conversion method. e.g.: operator bool() http://reviews.llvm.org/D16536 Files: clang-tidy/misc/VirtualNearMissCheck.cpp test/clang-tidy/misc-virtual-near-miss.cpp Index: test/clang-tidy/misc-virtual-near-miss.cpp === --- test/clang-tidy/misc-virtual-near-miss.cpp +++ test/clang-tidy/misc-virtual-near-miss.cpp @@ -69,6 +69,7 @@ int decaz(const char str[]); // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::decaz' has {{.*}} 'Mother::decay' + operator bool(); private: void funk(); // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::funk' has {{.*}} 'Father::func' Index: clang-tidy/misc/VirtualNearMissCheck.cpp === --- clang-tidy/misc/VirtualNearMissCheck.cpp +++ clang-tidy/misc/VirtualNearMissCheck.cpp @@ -169,6 +169,14 @@ return MD->getQualifiedNameAsString() + " " + MD->getType().getAsString(); } +/// Finds out if the given method is a user-defined conversion. +/// A user-defined conversion is of the following form: +/// operator TypeName() +static bool isUserDefinedConversion(const CXXMethodDecl *MD) { + return StringRef(MD->getNameAsString()).find(StringRef("operator ")) != + StringRef::npos; +} + bool VirtualNearMissCheck::isPossibleToBeOverridden( const CXXMethodDecl *BaseMD) { std::string Id = generateMethodId(BaseMD); @@ -178,7 +186,8 @@ bool IsPossible = !BaseMD->isImplicit() && !isa(BaseMD) && !isa(BaseMD) && BaseMD->isVirtual() && -!BaseMD->isOverloadedOperator(); +!BaseMD->isOverloadedOperator() && +!isUserDefinedConversion(BaseMD); PossibleMap[Id] = IsPossible; return IsPossible; } @@ -226,6 +235,9 @@ if (DerivedMD->isOverloadedOperator()) return; + if (isUserDefinedConversion(DerivedMD)) +return; + const ASTContext *Context = Result.Context; const auto *DerivedRD = DerivedMD->getParent(); Index: test/clang-tidy/misc-virtual-near-miss.cpp === --- test/clang-tidy/misc-virtual-near-miss.cpp +++ test/clang-tidy/misc-virtual-near-miss.cpp @@ -69,6 +69,7 @@ int decaz(const char str[]); // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::decaz' has {{.*}} 'Mother::decay' + operator bool(); private: void funk(); // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::funk' has {{.*}} 'Father::func' Index: clang-tidy/misc/VirtualNearMissCheck.cpp === --- clang-tidy/misc/VirtualNearMissCheck.cpp +++ clang-tidy/misc/VirtualNearMissCheck.cpp @@ -169,6 +169,14 @@ return MD->getQualifiedNameAsString() + " " + MD->getType().getAsString(); } +/// Finds out if the given method is a user-defined conversion. +/// A user-defined conversion is of the following form: +/// operator TypeName() +static bool isUserDefinedConversion(const CXXMethodDecl *MD) { + return StringRef(MD->getNameAsString()).find(StringRef("operator ")) != + StringRef::npos; +} + bool VirtualNearMissCheck::isPossibleToBeOverridden( const CXXMethodDecl *BaseMD) { std::string Id = generateMethodId(BaseMD); @@ -178,7 +186,8 @@ bool IsPossible = !BaseMD->isImplicit() && !isa(BaseMD) && !isa(BaseMD) && BaseMD->isVirtual() && -!BaseMD->isOverloadedOperator(); +!BaseMD->isOverloadedOperator() && +!isUserDefinedConversion(BaseMD); PossibleMap[Id] = IsPossible; return IsPossible; } @@ -226,6 +235,9 @@ if (DerivedMD->isOverloadedOperator()) return; + if (isUserDefinedConversion(DerivedMD)) +return; + const ASTContext *Context = Result.Context; const auto *DerivedRD = DerivedMD->getParent(); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16536: Fix crashing on user-defined conversion.
aaron.ballman added a subscriber: aaron.ballman. Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:175 @@ +174,3 @@ +/// operator TypeName() +static bool isUserDefinedConversion(const CXXMethodDecl *MD) { + return StringRef(MD->getNameAsString()).find(StringRef("operator ")) != Why not `isa(MD)` instead? http://reviews.llvm.org/D16536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16467: [libcxx] re.results.form: Format out-of-range subexpression references as null
mclow.lists added inline comments. Comment at: include/regex:5390 @@ -5389,3 +5389,3 @@ size_t __i = *__fmt_first - '0'; -__out = _VSTD::copy(__matches_[__i].first, - __matches_[__i].second, __out); +if (__i < __matches_.size()) { +__out = _VSTD::copy(__matches_[__i].first, A better idea. Replace this code: __out = _VSTD::copy(__matches_[__i].first, __matches_[__i].second, __out); with: __out = _VSTD::copy((*this)[__i].first, (*this)[__i].second, __out); `match_results::operator[]` already has the logic to do something with out of range indexes. Comment at: include/regex:5444 @@ -5441,3 +5443,3 @@ } -__out = _VSTD::copy(__matches_[__i].first, - __matches_[__i].second, __out); +if (__i < __matches_.size()) { +__out = _VSTD::copy(__matches_[__i].first, Same as above. Comment at: test/std/re/re.results/re.results.form/form1.pass.cpp:61 @@ +60,3 @@ +assert(r == out + 54); +assert(std::string(out) == "prefix: ab, match: cdefghi, suffix: jk, m[1]: , m[2]: "); +} Keep the tests. http://reviews.llvm.org/D16467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15836: [libcxx] Fix undefined behavior in forward_list
mclow.lists added a comment. This looks good to me. Comment at: include/forward_list:369 @@ +368,3 @@ +} +__node_pointer __get_node_unchecked() const { +return static_cast<__node_pointer>( I would think about calling this `__get_node_dereferencable` or something similar. When I looked at this first, I wondered "is there another call that does checking?" http://reviews.llvm.org/D15836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
RE: [PATCH] D16533: Bug 20796 - GCC's -Wstrict-prototypes warning not implemented in Clang
Thanks for the fast review. > On Mon, Jan 25, 2016 at 03:18:31PM +, Paul Titei via cfe-commits wrote: > > +// function definition with 0 params(for both cases), valid according > > +to 6.7.5.3/14 void foo1() {} > I still want to get a warning for this. At best it is inconsistent. I agree this is inconsistent with GCC behavior of this warning which warns even for void main(){}. But from what I understand the standard makes a distinction between unspecified parameters in declaration VS definitions (C99 6.7.5.3/14): "An empty list in a function declarator that is part of a definition of that function specifies that the function has no parameters." As opposed with: "The empty list in a function declarator that is not part of a definition of that function specifies that no information about the number or types of the parameters is supplied." > > +// Function declaration with no types void foo10(); // > > +expected-warning{{this function declaration is not a prototype}} > > + // CHECK: fix-it:"{{.*}}":{70:12-70:12}:"void" > > +// K&R function definition with incomplete param list declared void > > +foo10(p, p2) void *p; {} // expected-warning{{old-style function > > +definition is not preceded by a prototype}} > > + > > +// Prototype declaration > > +void foo11(int p, int p2); > > +// K&R function definition with previous prototype declared is not > > diagnosed. > > +void foo11(p, p2) int p; int p2; {} > Same here. I'm perfectly happy if that is a separate option though. It would make sense to not diagnose this last case since there is there is a previous prototype. Also GCC does not diagnose this: "An old-style function definition is permitted without a warning if preceded by a declaration that specifies the argument types." Paul Titei National Instruments Romania S.R.L. -- B-dul 21 Decembrie 1989, nr. 77, A2 Cluj-Napoca 400604, Romania C.I.F.: RO17961616 | O.R.C.: J12/3337/2005 Telefon: +40 264 406428 | Fax: +40 264 406429 E-mail: office.c...@ni.com Web: romania.ni.com Vanzari si suport tehnic: Telefon gratuit : 0800 070071 E-mail vanzari: ni.roma...@ni.com E-mail suport tehnic: techsupp...@ni.com ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16536: Fix crashing on user-defined conversion.
congliu updated this revision to Diff 45873. congliu added a comment. - Address the comment and clean up the code. http://reviews.llvm.org/D16536 Files: clang-tidy/misc/VirtualNearMissCheck.cpp test/clang-tidy/misc-virtual-near-miss.cpp Index: test/clang-tidy/misc-virtual-near-miss.cpp === --- test/clang-tidy/misc-virtual-near-miss.cpp +++ test/clang-tidy/misc-virtual-near-miss.cpp @@ -69,6 +69,7 @@ int decaz(const char str[]); // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::decaz' has {{.*}} 'Mother::decay' + operator bool(); private: void funk(); // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::funk' has {{.*}} 'Father::func' Index: clang-tidy/misc/VirtualNearMissCheck.cpp === --- clang-tidy/misc/VirtualNearMissCheck.cpp +++ clang-tidy/misc/VirtualNearMissCheck.cpp @@ -178,7 +178,8 @@ bool IsPossible = !BaseMD->isImplicit() && !isa(BaseMD) && !isa(BaseMD) && BaseMD->isVirtual() && -!BaseMD->isOverloadedOperator(); +!BaseMD->isOverloadedOperator() && +!isa(BaseMD); PossibleMap[Id] = IsPossible; return IsPossible; } @@ -210,8 +211,9 @@ return; Finder->addMatcher( - cxxMethodDecl(unless(anyOf(isOverride(), isImplicit(), - cxxConstructorDecl(), cxxDestructorDecl( + cxxMethodDecl( + unless(anyOf(isOverride(), isImplicit(), cxxConstructorDecl(), + cxxDestructorDecl(), cxxConversionDecl( .bind("method"), this); } Index: test/clang-tidy/misc-virtual-near-miss.cpp === --- test/clang-tidy/misc-virtual-near-miss.cpp +++ test/clang-tidy/misc-virtual-near-miss.cpp @@ -69,6 +69,7 @@ int decaz(const char str[]); // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::decaz' has {{.*}} 'Mother::decay' + operator bool(); private: void funk(); // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::funk' has {{.*}} 'Father::func' Index: clang-tidy/misc/VirtualNearMissCheck.cpp === --- clang-tidy/misc/VirtualNearMissCheck.cpp +++ clang-tidy/misc/VirtualNearMissCheck.cpp @@ -178,7 +178,8 @@ bool IsPossible = !BaseMD->isImplicit() && !isa(BaseMD) && !isa(BaseMD) && BaseMD->isVirtual() && -!BaseMD->isOverloadedOperator(); +!BaseMD->isOverloadedOperator() && +!isa(BaseMD); PossibleMap[Id] = IsPossible; return IsPossible; } @@ -210,8 +211,9 @@ return; Finder->addMatcher( - cxxMethodDecl(unless(anyOf(isOverride(), isImplicit(), - cxxConstructorDecl(), cxxDestructorDecl( + cxxMethodDecl( + unless(anyOf(isOverride(), isImplicit(), cxxConstructorDecl(), + cxxDestructorDecl(), cxxConversionDecl( .bind("method"), this); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16536: Fix crashing on user-defined conversion.
congliu added inline comments. Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:175 @@ +174,3 @@ +/// operator TypeName() +static bool isUserDefinedConversion(const CXXMethodDecl *MD) { + return StringRef(MD->getNameAsString()).find(StringRef("operator ")) != aaron.ballman wrote: > Why not `isa(MD)` instead? Thanks! http://reviews.llvm.org/D16536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15636: Reduce false positives in printf/scanf format checker
AndyG added a reviewer: rtrieu. AndyG added a comment. Richard, you have been recommended to me as a suitable reviewer by David. If that's not ok with you, please could you recommend another! Thanks, Andy. http://reviews.llvm.org/D15636 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16536: Fix crashing on user-defined conversion.
aaron.ballman accepted this revision. aaron.ballman added a reviewer: aaron.ballman. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM, thank you! http://reviews.llvm.org/D16536 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16533: Bug 20796 - GCC's -Wstrict-prototypes warning not implemented in Clang
thakis added a subscriber: thakis. thakis added a comment. (This is http://llvm.org/PR20796, which has some discussion and test cases. If you haven't, maybe you want to look at the test cases there. It's possible they're a subset of your test though.) http://reviews.llvm.org/D16533 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16344: [libcxx] Fix PR26103 - Error calling is_convertible with incomplete type
mdaniels added a comment. I dont have commit access, so if the change still looks good, and someone is feeling generous.. :) http://reviews.llvm.org/D16344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libcxx] r258697 - Implement LWG#2385; remove the allocator-aware std::function::assign call. It was useless, and didn't actually *do anything* with the allocator. Now it's gone. On the off chance tha
Author: marshall Date: Mon Jan 25 11:29:55 2016 New Revision: 258697 URL: http://llvm.org/viewvc/llvm-project?rev=258697&view=rev Log: Implement LWG#2385; remove the allocator-aware std::function::assign call. It was useless, and didn't actually *do anything* with the allocator. Now it's gone. On the off chance that someone is mistakenly calling it, it's only gone in C++1z Modified: libcxx/trunk/include/functional libcxx/trunk/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.mod/assign_F_alloc.pass.cpp libcxx/trunk/www/cxx1z_status.html Modified: libcxx/trunk/include/functional URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/functional?rev=258697&r1=258696&r2=258697&view=diff == --- libcxx/trunk/include/functional (original) +++ libcxx/trunk/include/functional Mon Jan 25 11:29:55 2016 @@ -407,7 +407,7 @@ public: // function modifiers: void swap(function&) noexcept; template - void assign(F&&, const Alloc&); + void assign(F&&, const Alloc&); // Removed in C++17 // function capacity: explicit operator bool() const noexcept; @@ -1626,10 +1626,13 @@ public: // function modifiers: void swap(function&) _NOEXCEPT; + +#if _LIBCPP_STD_VER <= 14 template _LIBCPP_INLINE_VISIBILITY void assign(_Fp&& __f, const _Alloc& __a) {function(allocator_arg, __a, _VSTD::forward<_Fp>(__f)).swap(*this);} +#endif // function capacity: _LIBCPP_INLINE_VISIBILITY Modified: libcxx/trunk/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.mod/assign_F_alloc.pass.cpp URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.mod/assign_F_alloc.pass.cpp?rev=258697&r1=258696&r2=258697&view=diff == --- libcxx/trunk/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.mod/assign_F_alloc.pass.cpp (original) +++ libcxx/trunk/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.mod/assign_F_alloc.pass.cpp Mon Jan 25 11:29:55 2016 @@ -12,10 +12,12 @@ // class function // template void assign(F&&, const A&); +// This call was removed post-C++14 #include #include +#include "test_macros.h" #include "test_allocator.h" class A @@ -49,6 +51,7 @@ int A::count = 0; int main() { +#if TEST_STD_VER <= 14 { std::function f; f.assign(A(), test_allocator()); @@ -57,4 +60,5 @@ int main() assert(f.target() == 0); } assert(A::count == 0); +#endif } Modified: libcxx/trunk/www/cxx1z_status.html URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/cxx1z_status.html?rev=258697&r1=258696&r2=258697&view=diff == --- libcxx/trunk/www/cxx1z_status.html (original) +++ libcxx/trunk/www/cxx1z_status.html Mon Jan 25 11:29:55 2016 @@ -168,7 +168,7 @@ http://cplusplus.github.io/LWG/lwg-defects.html#2367";>2367pair and tuple are not correctly implemented for is_constructible with no argsKonaComplete http://cplusplus.github.io/LWG/lwg-defects.html#2380";>2380Mayprovide long ::abs(long) and long long ::abs(long long)?KonaComplete http://cplusplus.github.io/LWG/lwg-defects.html#2384";>2384Allocator's deallocate function needs better specificationKonaComplete - http://cplusplus.github.io/LWG/lwg-defects.html#2385";>2385function::assign allocator argument doesn't make senseKona + http://cplusplus.github.io/LWG/lwg-defects.html#2385";>2385function::assign allocator argument doesn't make senseKonaComplete http://cplusplus.github.io/LWG/lwg-defects.html#2435";>2435reference_wrapper::operator()'s Remark should be deletedKonaComplete http://cplusplus.github.io/LWG/lwg-defects.html#2447";>2447Allocators and volatile-qualified value typesKonaComplete http://cplusplus.github.io/LWG/lwg-defects.html#2462";>2462std::ios_base::failure is overspecifiedKonaComplete @@ -179,7 +179,7 @@ http://cplusplus.github.io/LWG/lwg-defects.html#2477";>2477Inconsistency of wordings in std::vector::erase() and std::deque::erase()KonaComplete http://cplusplus.github.io/LWG/lwg-defects.html#2483";>2483throw_with_nested() should use is_finalKonaComplete http://cplusplus.github.io/LWG/lwg-defects.html#2484";>2484rethrow_if_nested() is doubly unimplementableKonaComplete - http://cplusplus.github.io/LWG/lwg-defects.html#2485";>2485get() should be overloaded for const tuple&&KonaCompleted + http://cplusplus.github.io/LWG/lwg-defects.html#2485";>2485get() should be overloaded for const tuple&&KonaComplete http://cplusplus.github.io/LWG/lwg-defects.html#2486";>2486mem_fn() should be required
Re: [PATCH] D15225: [Driver] Sanitizer support based on runtime library presence
ygribov added inline comments. Comment at: test/Driver/fsanitize.c:221 @@ +220,3 @@ +// RUN: %clang -target x86_64-apple-darwin10 -resource-dir=%S/Inputs/resource_dir -fsanitize=memory -fsanitize=thread,memory %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-MSAN-TSAN-MSAN-DARWIN1 +// CHECK-MSAN-TSAN-MSAN-DARWIN1: unsupported option '-fsanitize=thread,memory' for target 'x86_64-apple-darwin10' +// CHECK-MSAN-TSAN-MSAN-DARWIN1-NOT: unsupported option kubabrecka wrote: > ygribov wrote: > > samsonov wrote: > > > beanz wrote: > > > > kubabrecka wrote: > > > > > samsonov wrote: > > > > > > Again, I feel like we're lying to users here: `-fsanitize=thread` > > > > > > *is* supported for this target, it just requires building a runtime. > > > > > I'd like to see this from the point-of-view of a binary distribution. > > > > > If the binary distribution (e.g. the one from llvm.org or Apple's > > > > > Clang in Xcode) doesn't contain a runtime library, then the sanitizer > > > > > is *not* supported in that distribution. Also, see > > > > > http://reviews.llvm.org/D14846, we'd like to have CMake options to > > > > > select which runtimes will be built. If you deliberately choose not > > > > > to build ThreadSanitizer, then that sanitizer is *not* supported in > > > > > your version of Clang. If you're experimenting and porting a runtime > > > > > to a new platform, then this sanitizer *is* supported in your version > > > > > of Clang. > > > > Maybe the point is we should have a different error message for cases > > > > where the runtime is just missing. Something like "runtime components > > > > for '-fsanitize=thread' not available" > > > I see, so essentially you want to use a different approach for > > > determining sanitizer availability (on OS X for now): if the library is > > > present, then we support sanitizer, otherwise we don't: i.e. the binary > > > distribution is the source of truth, not the list of sanitizers hardcoded > > > into Clang driver source code. I'm fine with that, and see why it would > > > make sense. > > > > > > It's just that error message looks misleading: the problem is not TSan is > > > unsupported for target, it's just unavailable in this distribution for > > > one reason or another. > > > the binary distribution is the source of truth, not the list of > > > sanitizers hardcoded into Clang driver source code. > > > > This will not work for cross-compilers. It _may_ be ok for OSX but not for > > other platforms. > Why not? On Linux, there are statically-linked sanitizers, if you want to > cross-compile, you need to have the runtime libraries for the target. And > dynamic libraries are a similar story – they're version-tied to the compiler > and your compiler should really have the libraries of the sanitizers that it > supports. Ah, for some strange reason I thought that you were searching runtime lib in sysroot rather than compiler root. http://reviews.llvm.org/D15225 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15721: [Sema] Fix ICE on casting a vector of bools to a vector of T
Anastasia added a comment. I am generally not clear about the scope of this patch. Is it intended for OpenCL or C++? As for OpenCL, boolean vector types are not permitted (Table 6.2). Also conversions between vector types are disallowed in general (Section 6.2.2) but unfortunately Clang doesn't diagnose it properly at the moment. :( Comment at: test/CodeGenCXX/bool-vector-conversion.cpp:8 @@ +7,3 @@ + +// Nothing but OpenCL allows vectors of booleans. +// CHECK-LABEL: @_Z4testv Can you explain why? Table 6.2 doesn't list it among allowed valid types and Table 6.4 only says it's a reserved data type, but doesn't allow to use in application code anyways (Section 6.1.4). http://reviews.llvm.org/D15721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15173: [Preprocessor] Fix assertion in AnnotatePreviousCachedTokens
Ping! On Wed, Jan 20, 2016 at 11:22 AM, Bruno Cardoso Lopes wrote: > bruno updated this revision to Diff 45421. > bruno added a comment. > > Update patch after Richard comments > > > http://reviews.llvm.org/D15173 > > Files: > include/clang/Lex/Preprocessor.h > lib/Lex/PPCaching.cpp > lib/Parse/ParseTemplate.cpp > test/Parser/objcxx11-protocol-in-template.mm > > Index: test/Parser/objcxx11-protocol-in-template.mm > === > --- test/Parser/objcxx11-protocol-in-template.mm > +++ test/Parser/objcxx11-protocol-in-template.mm > @@ -8,3 +8,11 @@ > > vector> v; > vector>> v2; > + > +@protocol PA; > +@protocol PB; > + > +@class NSArray; > +typedef int some_t; > + > +id FA(NSArray> *h, some_t group); > Index: lib/Parse/ParseTemplate.cpp > === > --- lib/Parse/ParseTemplate.cpp > +++ lib/Parse/ParseTemplate.cpp > @@ -827,6 +827,7 @@ >} > >// Strip the initial '>' from the token. > + Token PrevTok = Tok; >if (RemainingToken == tok::equal && Next.is(tok::equal) && >areTokensAdjacent(Tok, Next)) { > // Join two adjacent '=' tokens into one, for cases like: > @@ -843,6 +844,17 @@ > PP.getSourceManager(), > getLangOpts())); > > + // The advance from '>>' to '>' in a ObjectiveC template argument list > needs > + // to be properly reflected in the token cache to allow correct interaction > + // between annotation and backtracking. > + if (ObjCGenericList && PrevTok.getKind() == tok::greatergreater && > + RemainingToken == tok::greater && PP.IsPreviousCachedToken(PrevTok)) { > +PrevTok.setKind(RemainingToken); > +PrevTok.setLength(1); > +SmallVector NewToks = {PrevTok, Tok}; > +PP.ReplacePreviousCachedToken(NewToks); > + } > + >if (!ConsumeLastToken) { > // Since we're not supposed to consume the '>' token, we need to push > // this token and revert the current token back to the '>'. > Index: lib/Lex/PPCaching.cpp > === > --- lib/Lex/PPCaching.cpp > +++ lib/Lex/PPCaching.cpp > @@ -116,3 +116,27 @@ > } >} > } > + > +bool Preprocessor::IsPreviousCachedToken(const Token &Tok) const { > + assert(CachedLexPos != 0 && "Expected to have some cached tokens"); > + const Token LastCachedTok = CachedTokens[CachedLexPos - 1]; > + > + if (LastCachedTok.getKind() != Tok.getKind()) > +return false; > + > + int RelOffset = 0; > + if ((!getSourceManager().isInSameSLocAddrSpace( > + Tok.getLocation(), getLastCachedTokenLocation(), &RelOffset)) || > + RelOffset) > +return false; > + > + return true; > +} > + > +void Preprocessor::ReplacePreviousCachedToken(SmallVectorImpl > &NewToks) { > + assert(CachedLexPos != 0 && "Expected to have some cached tokens"); > + CachedTokens.insert(CachedTokens.begin() + CachedLexPos - 1, > NewToks.begin(), > + NewToks.end()); > + CachedTokens.erase(CachedTokens.begin() + CachedLexPos - 1 + > NewToks.size()); > + CachedLexPos += NewToks.size() - 1; > +} > Index: include/clang/Lex/Preprocessor.h > === > --- include/clang/Lex/Preprocessor.h > +++ include/clang/Lex/Preprocessor.h > @@ -1185,6 +1185,17 @@ > return CachedTokens[CachedLexPos-1].getLastLoc(); >} > > + /// \brief Whether \p Tok is the most recent token (`CachedLexPos - 1`) in > + /// CachedTokens. > + bool IsPreviousCachedToken(const Token &Tok) const; > + > + /// \brief Replace token in `CachedLexPos - 1` in CachedTokens by the > tokens > + /// in \p NewToks. > + /// > + /// Useful when a token needs to be split in smaller ones and CachedTokens > + /// most recent token must to be updated to reflect that. > + void ReplacePreviousCachedToken(SmallVectorImpl &NewToks); > + >/// \brief Replace the last token with an annotation token. >/// >/// Like AnnotateCachedTokens(), this routine replaces an > > -- Bruno Cardoso Lopes http://www.brunocardoso.cc ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16469: Pass --wrap=pthread_create to linker for -fsplit-stack
rafael accepted this revision. rafael added a reviewer: rafael. rafael added a comment. This revision is now accepted and ready to land. LGTM http://reviews.llvm.org/D16469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16094: Debugger tuning via gold plugin
rafael added a subscriber: rafael. rafael accepted this revision. rafael added a reviewer: rafael. rafael added a comment. This revision is now accepted and ready to land. LGTM http://reviews.llvm.org/D16094 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15914: [OpenCL] Pipe builtin functions
Anastasia added a comment. Could you please copy us in CC? @Pekka, do you think it would make sense to include a description of the new OpenCL features in the release note too? Or shall we wait for the completion of OpenCL 2.0 support (hopefully in next release)? Could do both as well I guess. :) http://reviews.llvm.org/D15914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16469: Pass --wrap=pthread_create to linker for -fsplit-stack
hjl.tools added a comment. Can you check it in for me? Thanks. http://reviews.llvm.org/D16469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15705: Adding a scripted test for PR25717
rafael accepted this revision. rafael added a reviewer: rafael. rafael added a comment. This revision is now accepted and ready to land. LGTM assuming you can get the test to fail by reverting the fix http://reviews.llvm.org/D15705 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16467: [libcxx] re.results.form: Format out-of-range subexpression references as null
> On 2016-Jan-25, at 08:49, Marshall Clow wrote: > > mclow.lists added inline comments. > > > Comment at: include/regex:5390 > @@ -5389,3 +5389,3 @@ > size_t __i = *__fmt_first - '0'; > -__out = _VSTD::copy(__matches_[__i].first, > - __matches_[__i].second, __out); > +if (__i < __matches_.size()) { > +__out = _VSTD::copy(__matches_[__i].first, > > A better idea. > Replace this code: > __out = _VSTD::copy(__matches_[__i].first, __matches_[__i].second, __out); > with: > __out = _VSTD::copy((*this)[__i].first, (*this)[__i].second, __out); > > `match_results::operator[]` already has the logic to do something with out of > range indexes. > Yes, that's way better. I'll update the patch. > > > Comment at: include/regex:5444 > @@ -5441,3 +5443,3 @@ > } > -__out = _VSTD::copy(__matches_[__i].first, > - __matches_[__i].second, __out); > +if (__i < __matches_.size()) { > +__out = _VSTD::copy(__matches_[__i].first, > > Same as above. > > > Comment at: test/std/re/re.results/re.results.form/form1.pass.cpp:61 > @@ +60,3 @@ > +assert(r == out + 54); > +assert(std::string(out) == "prefix: ab, match: cdefghi, suffix: jk, > m[1]: , m[2]: "); > +} > > Keep the tests. > > > http://reviews.llvm.org/D16467 > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16467: [libcxx] re.results.form: Format out-of-range subexpression references as null
Given your follow-up review, and the behaviour of match_results::operator[], do you still have reservations? > On 2016-Jan-25, at 08:19, Marshall Clow wrote: > > mclow.lists added a comment. > > Not crashing is good; but silently returning a "wrong" answer rubs me the > wrong way. I'll do some research and get back to you on this, but maybe > throwing an exception is a better response. > > The code looks fine, though; I'm just not sure it's the right thing to do. > > > http://reviews.llvm.org/D16467 > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16467: [libcxx] re.results.form: Format out-of-range subexpression references as null
cfe-commits added a comment. Given your follow-up review, and the behaviour of match_results::operator[], do you still have reservations? http://reviews.llvm.org/D16467 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16541: [libc++] Renable test/std/re/re.alg/re.alg.match/awk.pass.cpp
dexonsmith created this revision. dexonsmith added a reviewer: mclow.lists. dexonsmith added a subscriber: cfe-commits. This entire test was effectively disabled in r187636 with no relevant explanation. The test passes for me just by removing the comment markers. http://reviews.llvm.org/D16541 Files: test/std/re/re.alg/re.alg.match/awk.pass.cpp Index: test/std/re/re.alg/re.alg.match/awk.pass.cpp === --- test/std/re/re.alg/re.alg.match/awk.pass.cpp +++ test/std/re/re.alg/re.alg.match/awk.pass.cpp @@ -25,7 +25,7 @@ int main() { -/*{ +{ std::cmatch m; const char s[] = "a"; assert(std::regex_match(s, m, std::regex("a", std::regex_constants::awk))); @@ -615,12 +615,12 @@ assert(m.size() == 0); } std::locale::global(std::locale(LOCALE_cs_CZ_ISO8859_2)); -*/{ +{ std::cmatch m; const char s[] = "m"; - /* assert(std::regex_match(s, m,*/ std::regex("[a[=M=]z]"/*, - std::regex_constants::awk*/);//)); -/*assert(m.size() == 1); +assert(std::regex_match(s, m, std::regex("[a[=M=]z]", + std::regex_constants::awk))); +assert(m.size() == 1); assert(!m.prefix().matched); assert(m.prefix().first == s); assert(m.prefix().second == m[0].first); @@ -630,8 +630,8 @@ assert(m.length(0) == std::char_traits::length(s)); assert(m.position(0) == 0); assert(m.str(0) == s); -*/} -/*{ +} +{ std::cmatch m; const char s[] = "Ch"; assert(std::regex_match(s, m, std::regex("[a[.ch.]z]", @@ -1387,4 +1387,4 @@ assert(m.position(0) == 0); assert(m.str(0) == s); } -*/} +} Index: test/std/re/re.alg/re.alg.match/awk.pass.cpp === --- test/std/re/re.alg/re.alg.match/awk.pass.cpp +++ test/std/re/re.alg/re.alg.match/awk.pass.cpp @@ -25,7 +25,7 @@ int main() { -/*{ +{ std::cmatch m; const char s[] = "a"; assert(std::regex_match(s, m, std::regex("a", std::regex_constants::awk))); @@ -615,12 +615,12 @@ assert(m.size() == 0); } std::locale::global(std::locale(LOCALE_cs_CZ_ISO8859_2)); -*/{ +{ std::cmatch m; const char s[] = "m"; - /* assert(std::regex_match(s, m,*/ std::regex("[a[=M=]z]"/*, - std::regex_constants::awk*/);//)); -/*assert(m.size() == 1); +assert(std::regex_match(s, m, std::regex("[a[=M=]z]", + std::regex_constants::awk))); +assert(m.size() == 1); assert(!m.prefix().matched); assert(m.prefix().first == s); assert(m.prefix().second == m[0].first); @@ -630,8 +630,8 @@ assert(m.length(0) == std::char_traits::length(s)); assert(m.position(0) == 0); assert(m.str(0) == s); -*/} -/*{ +} +{ std::cmatch m; const char s[] = "Ch"; assert(std::regex_match(s, m, std::regex("[a[.ch.]z]", @@ -1387,4 +1387,4 @@ assert(m.position(0) == 0); assert(m.str(0) == s); } -*/} +} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16467: [libcxx] re.results.form: Format out-of-range subexpression references as null
dexonsmith updated this revision to Diff 45879. dexonsmith added a comment. Addressed Marshall's review comments: deferring to match_results::operator[]() rather than duplicating the logic. http://reviews.llvm.org/D16467 Files: include/regex test/std/re/re.results/re.results.form/form1.pass.cpp Index: test/std/re/re.results/re.results.form/form1.pass.cpp === --- test/std/re/re.results/re.results.form/form1.pass.cpp +++ test/std/re/re.results/re.results.form/form1.pass.cpp @@ -38,6 +38,31 @@ { std::match_results m; const char s[] = "abcdefghijk"; +assert(std::regex_search(s, m, std::regex("cd((e)fg)hi", + std::regex_constants::nosubs))); + +char out[100] = {0}; +const char fmt[] = "prefix: $`, match: $&, suffix: $', m[1]: $1, m[2]: $2"; +char* r = m.format(output_iterator(out), +fmt, fmt + std::char_traits::length(fmt)).base(); +assert(r == out + 54); +assert(std::string(out) == "prefix: ab, match: cdefghi, suffix: jk, m[1]: , m[2]: "); +} +{ +std::match_results m; +const char s[] = "abcdefghijk"; +assert(std::regex_search(s, m, std::regex("cdefghi"))); + +char out[100] = {0}; +const char fmt[] = "prefix: $`, match: $&, suffix: $', m[1]: $1, m[2]: $2"; +char* r = m.format(output_iterator(out), +fmt, fmt + std::char_traits::length(fmt)).base(); +assert(r == out + 54); +assert(std::string(out) == "prefix: ab, match: cdefghi, suffix: jk, m[1]: , m[2]: "); +} +{ +std::match_results m; +const char s[] = "abcdefghijk"; assert(std::regex_search(s, m, std::regex("cd((e)fg)hi"))); char out[100] = {0}; @@ -61,6 +86,33 @@ assert(r == out + 34); assert(std::string(out) == "match: cdefghi, m[1]: efg, m[2]: e"); } +{ +std::match_results m; +const char s[] = "abcdefghijk"; +assert(std::regex_search(s, m, std::regex("cd((e)fg)hi", + std::regex_constants::nosubs))); + +char out[100] = {0}; +const char fmt[] = "match: &, m[1]: \\1, m[2]: \\2"; +char* r = m.format(output_iterator(out), +fmt, fmt + std::char_traits::length(fmt), +std::regex_constants::format_sed).base(); +assert(r == out + 30); +assert(std::string(out) == "match: cdefghi, m[1]: , m[2]: "); +} +{ +std::match_results m; +const char s[] = "abcdefghijk"; +assert(std::regex_search(s, m, std::regex("cdefghi"))); + +char out[100] = {0}; +const char fmt[] = "match: &, m[1]: \\1, m[2]: \\2"; +char* r = m.format(output_iterator(out), +fmt, fmt + std::char_traits::length(fmt), +std::regex_constants::format_sed).base(); +assert(r == out + 30); +assert(std::string(out) == "match: cdefghi, m[1]: , m[2]: "); +} { std::match_results m; Index: include/regex === --- include/regex +++ include/regex @@ -5387,8 +5387,8 @@ if ('0' <= *__fmt_first && *__fmt_first <= '9') { size_t __i = *__fmt_first - '0'; -__out = _VSTD::copy(__matches_[__i].first, - __matches_[__i].second, __out); +__out = _VSTD::copy((*this)[__i].first, +(*this)[__i].second, __out); } else { @@ -5439,8 +5439,8 @@ ++__fmt_first; __i = 10 * __i + *__fmt_first - '0'; } -__out = _VSTD::copy(__matches_[__i].first, - __matches_[__i].second, __out); +__out = _VSTD::copy((*this)[__i].first, +(*this)[__i].second, __out); } else { Index: test/std/re/re.results/re.results.form/form1.pass.cpp === --- test/std/re/re.results/re.results.form/form1.pass.cpp +++ test/std/re/re.results/re.results.form/form1.pass.cpp @@ -38,6 +38,31 @@ { std::match_results m; const char s[] = "abcdefghijk"; +assert(std::regex_search(s, m, std::regex("cd((e)fg)hi", + std::regex_constants::nosubs))); + +char out[100] = {0}; +const char fmt[] = "prefix: $`, match: $&, suffix: $', m[1]: $1, m[2]: $2"; +char* r = m.format(output_iterator(out), +fmt, fmt +
r258698 - Pass --wrap=pthread_create to linker for -fsplit-stack.
Author: rafael Date: Mon Jan 25 12:29:16 2016 New Revision: 258698 URL: http://llvm.org/viewvc/llvm-project?rev=258698&view=rev Log: Pass --wrap=pthread_create to linker for -fsplit-stack. From https://gcc.gnu.org/ml/gcc-patches/2010-09/msg01807.html -fsplit-stack should pass --wrap=pthread_create to linker for -fsplit-stack It is needed to initialize the stack guard. This fixes PR 20148. Patch by H.J Lu! Added: cfe/trunk/test/Driver/split-stack-ld.c Modified: cfe/trunk/lib/Driver/Tools.cpp Modified: cfe/trunk/lib/Driver/Tools.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=258698&r1=258697&r2=258698&view=diff == --- cfe/trunk/lib/Driver/Tools.cpp (original) +++ cfe/trunk/lib/Driver/Tools.cpp Mon Jan 25 12:29:16 2016 @@ -9001,6 +9001,9 @@ void gnutools::Linker::ConstructJob(Comp if (WantPthread && !isAndroid) CmdArgs.push_back("-lpthread"); + if (Args.hasArg(options::OPT_fsplit_stack)) +CmdArgs.push_back("--wrap=pthread_create"); + CmdArgs.push_back("-lc"); if (Args.hasArg(options::OPT_static)) Added: cfe/trunk/test/Driver/split-stack-ld.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/split-stack-ld.c?rev=258698&view=auto == --- cfe/trunk/test/Driver/split-stack-ld.c (added) +++ cfe/trunk/test/Driver/split-stack-ld.c Mon Jan 25 12:29:16 2016 @@ -0,0 +1,17 @@ +// Test split stack ld flags. +// +// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ +// RUN: -target i386-unknown-linux -fsplit-stack \ +// RUN: -resource-dir=%S/Inputs/resource_dir \ +// RUN: --sysroot=%S/Inputs/basic_linux_tree \ +// RUN: | FileCheck --check-prefix=CHECK-LINUX-I386 %s +// +// CHECK-LINUX-I386: "--wrap=pthread_create" +// +// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ +// RUN: -target x86_64-unknown-linux -fsplit-stack \ +// RUN: -resource-dir=%S/Inputs/resource_dir \ +// RUN: --sysroot=%S/Inputs/basic_linux_tree \ +// RUN: | FileCheck --check-prefix=CHECK-LINUX-X86-64 %s +// +// CHECK-LINUX-X86-64: "--wrap=pthread_create" ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16469: Pass --wrap=pthread_create to linker for -fsplit-stack
rafael closed this revision. rafael added a comment. 258698 http://reviews.llvm.org/D16469 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15914: [OpenCL] Pipe builtin functions
pekka.jaaskelainen added a comment. > @Pekka, do you think it would make sense to include a description of the new > OpenCL features in the release note too? Or shall we wait for the completion > of OpenCL 2.0 support (hopefully in next release)? Could do both as well I > guess. :) Yes I think it would make sense. It's good to advertise that the OpenCL support is progressing. http://reviews.llvm.org/D15914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16511: [MSVC Compat] Only warn for unknown clang-cl arguments
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm other than the warning group. Comment at: test/Misc/warning-flags.c:21 @@ -20,3 +20,3 @@ -CHECK: Warnings without flags (84): +CHECK: Warnings without flags (85): CHECK-NEXT: ext_excess_initializers This list shouldn't grow, we should have a warning group for this. -W[no-]unknown-argument sounds good to me. http://reviews.llvm.org/D16511 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
On Fri, Jan 22, 2016 at 1:43 PM, Sean Silva wrote: > silvas added a comment. > > In http://reviews.llvm.org/D15829#333902, @davidxl wrote: > >> For the longer term, one possible solution is to make FE based >> instrumentation only used for coverage testing which can be turned on >> with -fcoverage-mapping option (currently, -fcoverage-mapping can not >> be used alone and must be used together with >> -fprofile-instr-generate). To summarize: >> >> A. Current behavior: >> >> --- >> >> 1. -fprofile-instr-generate turns on FE based instrumentation >> 2. -fprofile-instr-generate -fcoverage-mapping turns on FE based >> instrumentation and coverage mapping data generation. >> 3. -fprofile-instr-use=<..> assumes profile data from FE instrumentation. >> >> B. Proposed new behavior: >> >> >> >> 1. -fprofile-instr-generate turns on IR late instrumentation >> 2. -fcoverage-mapping turns on FE instrumentation and coverage-mapping >> 3. -fprofile-instr-generate -fcoverage-mapping result in compiler warning >> 4. -fprofile-instr-use=<> will automatically determine how to use the >> profile data. > > > Very good observation that we can determine FE or IR automatically based on > the input profdata. That simplifies things. > >> B.2) above can be done today for improved usability. > > > I don't see how this improves usability. In fact, it is confusing because it > hijacks an existing option. > > Also B.3 causes existing user builds to emit a warning, which is annoying. Since it is pointless to specify -fcoverage-mapping option alone, why not do not automatically? Yes it means some behavior change (in a good way) and some annoying warnings initially, but those are trivially fixable by the user. > > I would propose the following modification of B: > > C.: > > 1. -fprofile-instr-generate defaults to IR instrumentation (i.e. behaves > exactly as before, except that it uses IR instrumentation) > 2. -fprofile-instr-generate -fcoverage-mapping turns on frontend > instrumentation and coverage. (i.e. behaves exactly as before) I am fine with this -- as long as the user does not need to bear the burden of understanding where and how the instrumentation is done. > 3. -fprofile-instr-use=<> automatically determines which method to use > > All existing user workflows continue to work, except for workflows that > attempt to `llvm-profdata merge` some old frontend profile data (e.g. they > have checked-in to version control and represents some special workload) with > the profile data from new binaries. > > Concretely, imagine the following workflow: > > clang -fprofile-instr-generate foo.c -o foo > ./foo > llvm-profdata merge default.profraw -o new.profdata > llvm-profdata merge new.profdata > /versioncontrol/some-important-but-expensive-to-reproduce-workload.profdata > -o foo.profdata > clang -fprofile-instr-use=foo.profdata foo.c -o foo_pgo > > I think this is a reasonable breakage. We would need to add a note in the > release notes. Unfortunately this is not expected breakage if we claim to > have forward compatibility for profdata (which IIRC Apple requires; > @bogner?). But I think this case will be rare and exceptional enough that we > can tolerate it: > The old profile data has to out live the program source versions which usually change fast. In reality, I don't expect profile data to live too long. > - a simple immediate workaround is to specify `-fcoverage-mapping` (which > also adds some extra stuff, but works around the issue) This is a reasonable workaround > - Presumably > /versioncontrol/some-important-but-expensive-to-reproduce-workload.profdata > is regenerated with some frequency which is more frequent than upgrading the > compiler, and so it is likely reasonable to regenerate it alongside a > compiler upgrade, using the workaround until then. > > > >> B.1) needs a > >> transition period before the IR based instrumentation becomes > >> stablized (and can be flipped to the default). During the transition > >> period, the behavior of 1) does not change, but a cc1 option can be > >> used to turn on IR instrumentation (as proposed by Sean). > > > Just to clarify, users are not allowed to use cc1 options. The cc1 option is > purely for us as compiler developers to do integration and testing, put off > some discussions for later, etc. > yes. thanks, David > > http://reviews.llvm.org/D15829 > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15705: Adding a scripted test for PR25717
ygao added a comment. Thanks, Rafael! I did verify that the test would fail without my fix. But what do I need to do to make buildbots turn on the new lit parameter? http://reviews.llvm.org/D15705 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16544: [libcxx] Framework to allow testing of static libc++abi
bcraig created this revision. bcraig added reviewers: mclow.lists, EricWF, jroelofs. bcraig added a subscriber: cfe-commits. NOTE: related libc++abi changes are in a different patch that will be posted shortly. Tested with static libcxx and libcxxabi against custom embedded target. Tested with static libcxx and libcxxabi against Linux x64. Tested with shared libcxx and libcxxabi against Linux x64. Really hoping that I didn't accidentally break Mac. These changes make linking against static libraries more explicit. Instead of using -lc++ and -lc++abi in the tests, an absolute path to the library is provided instead. The choices of shared vs. static, and the choices of library paths for both libcxx and libcxxabi needed to be exchanged for this to work. In other words, libcxx tests need to know the library path of libcxxabi, and whether libcxxabi is a static or shared library. Some Mac specific logic for testing against libc++abi had to be moved from libcxxabi's config.py, as it was overriding choices made in libcxx's config.py. That logic is now in libcxx's target_info.py. Testing a static libcxx on Linux will now automatically link in librt most of the time. Previously, lots of pthread tests would fail because of an unresolved clock_gettime. http://reviews.llvm.org/D16544 Files: test/CMakeLists.txt test/libcxx/test/config.py test/libcxx/test/target_info.py test/lit.site.cfg.in Index: test/lit.site.cfg.in === --- test/lit.site.cfg.in +++ test/lit.site.cfg.in @@ -20,6 +20,7 @@ config.target_info = "@LIBCXX_TARGET_INFO@" config.executor = "@LIBCXX_EXECUTOR@" config.llvm_unwinder= "@LIBCXXABI_USE_LLVM_UNWINDER@" +config.libcxxabi_shared = "@LIBCXXABI_ENABLE_SHARED@" # Let the main config do the real work. lit_config.load_config(config, "@LIBCXX_SOURCE_DIR@/test/lit.cfg") Index: test/libcxx/test/target_info.py === --- test/libcxx/test/target_info.py +++ test/libcxx/test/target_info.py @@ -104,6 +104,14 @@ env['DYLD_LIBRARY_PATH'] = ':'.join(library_paths) def allow_cxxabi_link(self): +# libc++ *should* export all of the symbols found in libc++abi on OS X. +# For this reason LibcxxConfiguration will not link libc++abi in OS X. +# However __cxa_throw_bad_new_array_length doesn't get exported into +# libc++ yet so we still need to explicitly link libc++abi when testing +# libc++abi +# See PR22654. +if(self.full_config.get_lit_conf('name', '') == 'libc++abi'): +return True # Don't link libc++abi explicitly on OS X because the symbols # should be available in libc++ directly. return False @@ -162,11 +170,14 @@ enable_threads = ('libcpp-has-no-threads' not in self.full_config.config.available_features) llvm_unwinder = self.full_config.get_lit_bool('llvm_unwinder', False) +shared_libcxx = self.full_config.get_lit_bool('enable_shared', False) flags += ['-lm'] if not llvm_unwinder: flags += ['-lgcc_s', '-lgcc'] if enable_threads: flags += ['-lpthread'] +if not shared_libcxx: + flags += ['-lrt'] flags += ['-lc'] if llvm_unwinder: flags += ['-lunwind', '-ldl'] Index: test/libcxx/test/config.py === --- test/libcxx/test/config.py +++ test/libcxx/test/config.py @@ -460,7 +460,16 @@ if libcxx_library: self.cxx.link_flags += [libcxx_library] else: -self.cxx.link_flags += ['-lc++'] +libcxx_shared = self.get_lit_bool('enable_shared') +if libcxx_shared: +self.cxx.link_flags += ['-lc++'] +else: +cxx_library_root = self.get_lit_conf('cxx_library_root') +if cxx_library_root: +abs_path = cxx_library_root + "/libc++.a" +self.cxx.link_flags += [abs_path] +else: +self.cxx.link_flags += ['-lc++'] def configure_link_flags_abi_library(self): cxx_abi = self.get_lit_conf('cxx_abi', 'libcxxabi') @@ -470,7 +479,16 @@ self.cxx.link_flags += ['-lsupc++'] elif cxx_abi == 'libcxxabi': if self.target_info.allow_cxxabi_link(): -self.cxx.link_flags += ['-lc++abi'] +libcxxabi_shared = self.get_lit_bool('libcxxabi_shared') +if libcxxabi_shared: +self.cxx.link_flags += ['-lc++abi'] +else: +cxxabi_library_root = self.get_lit_conf('abi_library_path') +if cxxabi_library_root: +abs_path = cxxabi_library_root + "/libc++abi.a" +
[PATCH] D16545: [libcxxabi] Enable testing for static libc++abi
bcraig created this revision. bcraig added reviewers: mclow.lists, jroelofs, EricWF. bcraig added a subscriber: cfe-commits. This change leverages framework changes made in libcxx. See those changes for more details. (http://reviews.llvm.org/D16544) Some Mac specific logic for testing against libc++abi had to be moved from libcxxabi's config.py, as it was overriding choices made in libcxx's config.py. That logic is now in libcxx's target_info.py. http://reviews.llvm.org/D16545 Files: CMakeLists.txt test/CMakeLists.txt test/libcxxabi/test/config.py test/lit.site.cfg.in Index: test/lit.site.cfg.in === --- test/lit.site.cfg.in +++ test/lit.site.cfg.in @@ -13,6 +13,8 @@ config.enable_32bit = "@LLVM_BUILD_32_BITS@" config.target_info = "@LIBCXXABI_TARGET_INFO@" config.executor = "@LIBCXXABI_EXECUTOR@" +config.libcxxabi_shared = "@LIBCXXABI_ENABLE_SHARED@" +config.enable_shared= "@LIBCXX_ENABLE_SHARED@" # Let the main config do the real work. lit_config.load_config(config, "@LIBCXXABI_SOURCE_DIR@/test/lit.cfg") Index: test/libcxxabi/test/config.py === --- test/libcxxabi/test/config.py +++ test/libcxxabi/test/config.py @@ -63,13 +63,3 @@ def configure_compile_flags_rtti(self): pass - -# TODO(ericwf): Remove this. This is a hack for OS X. -# libc++ *should* export all of the symbols found in libc++abi on OS X. -# For this reason LibcxxConfiguration will not link libc++abi in OS X. -# However __cxa_throw_bad_new_array_length doesn't get exported into libc++ -# yet so we still need to explicitly link libc++abi. -# See PR22654. -def configure_link_flags_abi_library(self): -self.cxx.link_flags += ['-lc++abi'] - Index: test/CMakeLists.txt === --- test/CMakeLists.txt +++ test/CMakeLists.txt @@ -7,6 +7,7 @@ endmacro() pythonize_bool(LLVM_BUILD_32_BITS) +pythonize_bool(LIBCXX_ENABLE_SHARED) pythonize_bool(LIBCXXABI_ENABLE_SHARED) pythonize_bool(LIBCXXABI_ENABLE_THREADS) pythonize_bool(LIBCXXABI_USE_LLVM_UNWINDER) @@ -21,7 +22,12 @@ ${CMAKE_CURRENT_BINARY_DIR}/lit.site.cfg @ONLY) -set(LIBCXXABI_TEST_DEPS cxxabi_shared) +if (LIBCXXABI_ENABLE_SHARED) + set(LIBCXXABI_TEST_DEPS cxxabi_shared) +else() + set(LIBCXXABI_TEST_DEPS cxxabi_static) +endif() + if (NOT LIBCXXABI_BUILT_STANDALONE) list(APPEND LIBCXXABI_TEST_DEPS cxx) endif() Index: CMakeLists.txt === --- CMakeLists.txt +++ CMakeLists.txt @@ -116,9 +116,8 @@ option(LIBCXXABI_ENABLE_WERROR "Fail and stop if a warning is triggered." OFF) option(LIBCXXABI_USE_LLVM_UNWINDER "Build and use the LLVM unwinder." OFF) option(LIBCXXABI_ENABLE_THREADS "Build with threads enabled" ON) -set(LIBCXXABI_GCC_TOOLCHAIN "" CACHE STRING "GCC toolchain for cross compiling.") -set(LIBCXXABI_SYSROOT "" CACHE STRING "Sysroot for cross compiling.") -set(LIBCXXABI_LIBCXX_LIBRARY_PATH "" CACHE STRING "The path to libc++ library.") +set(LIBCXXABI_GCC_TOOLCHAIN "" CACHE PATH "GCC toolchain for cross compiling.") +set(LIBCXXABI_SYSROOT "" CACHE PATH "Sysroot for cross compiling.") # Default to building a shared library so that the default options still test # the libc++abi that is being built. There are two problems with testing a @@ -182,6 +181,13 @@ set(LIBCXXABI_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}) set(LIBCXXABI_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBCXXABI_LIBDIR_SUFFIX}) +# By default, for non-standalone builds, libcxx and libcxxabi share a library +# directory. +if (NOT LIBCXXABI_LIBCXX_LIBRARY_PATH) + set(LIBCXXABI_LIBCXX_LIBRARY_PATH "${LIBCXXABI_LIBRARY_DIR}" CACHE PATH + "The path to libc++ library.") +endif () + #=== # Setup Compiler Flags #=== @@ -338,14 +344,16 @@ # soname, etc... add_subdirectory(src) -if(NOT LIBCXXABI_ENABLE_SHARED) - # TODO: Fix the libc++ cmake files so that libc++abi can be statically linked. - # As it is now, libc++ will prefer linking against a dynamic libc++abi in the - # system library paths over a static libc++abi in the out directory. This - # would test the system library rather than the one we just built, which isn't - # very helpful. - message(WARNING "The libc++abi tests are currently only valid when " - "LIBCXXABI_ENABLE_SHARED is on, no check target will be " +if (LIBCXXABI_BUILT_STANDALONE AND NOT LIBCXXABI_ENABLE_SHARED) + # We can't reasonably test the system C++ library with a static libc++abi. + # We either need to be able to replace libc++abi at run time (with a shared + # libc++abi), or we need to be able to replace the C++ runtime (with a
Re: [PATCH] D16499: [CUDA] Disable ctor/dtor aliases in device code.
tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. LGTM. http://reviews.llvm.org/D16499 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16501: [CUDA] Don't generate aliases for static extern "C" functions.
tra added a comment. Failing silently is not a good idea. At the very least there should produce an error. The right thing to do here, IMO, would be to generate a stub with alias name that just jumps to or calls aliasee. http://reviews.llvm.org/D16501 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16511: [MSVC Compat] Only warn for unknown clang-cl arguments
hans added a subscriber: hans. hans added a comment. Maybe this behavior should be called out in the Command-Line Options section under clang-cl in docs/UsersManual.rst? http://reviews.llvm.org/D16511 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r257947 - Avoid self-assignment of SmallString, trigger UB behavior down the road.
Joerg, do you want to go ahead and merge this to 3.8 with utils/release/merge.sh? (Or let me know if you'd prefer me to do it.) On Sun, Jan 24, 2016 at 11:29 PM, Richard Smith wrote: > Approved for branch. > > On 23 Jan 2016 11:56 p.m., "Joerg Sonnenberger" > wrote: >> >> On Sat, Jan 16, 2016 at 03:51:11PM +0100, Joerg Sonnenberger via >> cfe-commits wrote: >> > Hello Richard, >> > can this be merged into 3.8? It creates some trivial noise under >> > valgrind. >> >> Ping? >> >> Joerg ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16094: Debugger tuning via gold plugin
This revision was automatically updated to reflect the committed changes. Closed by commit rL258712: LTO via the gold plugin needs to be told about debugger tuning. (authored by probinson). Changed prior to commit: http://reviews.llvm.org/D16094?vs=44575&id=45893#toc Repository: rL LLVM http://reviews.llvm.org/D16094 Files: cfe/trunk/lib/Driver/Tools.cpp cfe/trunk/test/Driver/lto.c Index: cfe/trunk/test/Driver/lto.c === --- cfe/trunk/test/Driver/lto.c +++ cfe/trunk/test/Driver/lto.c @@ -49,3 +49,12 @@ // RUN: FileCheck -check-prefix=CHECK-LINK-NOLTO-ACTION < %t %s // // CHECK-LINK-NOLTO-ACTION-NOT: "-plugin" "{{.*}}/LLVMgold.so" + +// -flto passes along an explicit debugger tuning argument. +// RUN: %clang -target x86_64-unknown-linux -### %s -flto -glldb 2> %t +// RUN: FileCheck -check-prefix=CHECK-TUNING-LLDB < %t %s +// RUN: %clang -target x86_64-unknown-linux -### %s -flto -g 2> %t +// RUN: FileCheck -check-prefix=CHECK-NO-TUNING < %t %s +// +// CHECK-TUNING-LLDB: "-plugin-opt=-debugger-tune=lldb" +// CHECK-NO-TUNING-NOT: "-plugin-opt=-debugger-tune Index: cfe/trunk/lib/Driver/Tools.cpp === --- cfe/trunk/lib/Driver/Tools.cpp +++ cfe/trunk/lib/Driver/Tools.cpp @@ -1817,6 +1817,17 @@ if (IsThinLTO) CmdArgs.push_back("-plugin-opt=thinlto"); + + // If an explicit debugger tuning argument appeared, pass it along. + if (Arg *A = Args.getLastArg(options::OPT_gTune_Group, + options::OPT_ggdbN_Group)) { +if (A->getOption().matches(options::OPT_glldb)) + CmdArgs.push_back("-plugin-opt=-debugger-tune=lldb"); +else if (A->getOption().matches(options::OPT_gsce)) + CmdArgs.push_back("-plugin-opt=-debugger-tune=sce"); +else + CmdArgs.push_back("-plugin-opt=-debugger-tune=gdb"); + } } /// This is a helper function for validating the optional refinement step Index: cfe/trunk/test/Driver/lto.c === --- cfe/trunk/test/Driver/lto.c +++ cfe/trunk/test/Driver/lto.c @@ -49,3 +49,12 @@ // RUN: FileCheck -check-prefix=CHECK-LINK-NOLTO-ACTION < %t %s // // CHECK-LINK-NOLTO-ACTION-NOT: "-plugin" "{{.*}}/LLVMgold.so" + +// -flto passes along an explicit debugger tuning argument. +// RUN: %clang -target x86_64-unknown-linux -### %s -flto -glldb 2> %t +// RUN: FileCheck -check-prefix=CHECK-TUNING-LLDB < %t %s +// RUN: %clang -target x86_64-unknown-linux -### %s -flto -g 2> %t +// RUN: FileCheck -check-prefix=CHECK-NO-TUNING < %t %s +// +// CHECK-TUNING-LLDB: "-plugin-opt=-debugger-tune=lldb" +// CHECK-NO-TUNING-NOT: "-plugin-opt=-debugger-tune Index: cfe/trunk/lib/Driver/Tools.cpp === --- cfe/trunk/lib/Driver/Tools.cpp +++ cfe/trunk/lib/Driver/Tools.cpp @@ -1817,6 +1817,17 @@ if (IsThinLTO) CmdArgs.push_back("-plugin-opt=thinlto"); + + // If an explicit debugger tuning argument appeared, pass it along. + if (Arg *A = Args.getLastArg(options::OPT_gTune_Group, + options::OPT_ggdbN_Group)) { +if (A->getOption().matches(options::OPT_glldb)) + CmdArgs.push_back("-plugin-opt=-debugger-tune=lldb"); +else if (A->getOption().matches(options::OPT_gsce)) + CmdArgs.push_back("-plugin-opt=-debugger-tune=sce"); +else + CmdArgs.push_back("-plugin-opt=-debugger-tune=gdb"); + } } /// This is a helper function for validating the optional refinement step ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r258712 - LTO via the gold plugin needs to be told about debugger tuning.
Author: probinson Date: Mon Jan 25 13:46:40 2016 New Revision: 258712 URL: http://llvm.org/viewvc/llvm-project?rev=258712&view=rev Log: LTO via the gold plugin needs to be told about debugger tuning. Differential Revision: http://reviews.llvm.org/D16094 Modified: cfe/trunk/lib/Driver/Tools.cpp cfe/trunk/test/Driver/lto.c Modified: cfe/trunk/lib/Driver/Tools.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=258712&r1=258711&r2=258712&view=diff == --- cfe/trunk/lib/Driver/Tools.cpp (original) +++ cfe/trunk/lib/Driver/Tools.cpp Mon Jan 25 13:46:40 2016 @@ -1817,6 +1817,17 @@ static void AddGoldPlugin(const ToolChai if (IsThinLTO) CmdArgs.push_back("-plugin-opt=thinlto"); + + // If an explicit debugger tuning argument appeared, pass it along. + if (Arg *A = Args.getLastArg(options::OPT_gTune_Group, + options::OPT_ggdbN_Group)) { +if (A->getOption().matches(options::OPT_glldb)) + CmdArgs.push_back("-plugin-opt=-debugger-tune=lldb"); +else if (A->getOption().matches(options::OPT_gsce)) + CmdArgs.push_back("-plugin-opt=-debugger-tune=sce"); +else + CmdArgs.push_back("-plugin-opt=-debugger-tune=gdb"); + } } /// This is a helper function for validating the optional refinement step Modified: cfe/trunk/test/Driver/lto.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/lto.c?rev=258712&r1=258711&r2=258712&view=diff == --- cfe/trunk/test/Driver/lto.c (original) +++ cfe/trunk/test/Driver/lto.c Mon Jan 25 13:46:40 2016 @@ -49,3 +49,12 @@ // RUN: FileCheck -check-prefix=CHECK-LINK-NOLTO-ACTION < %t %s // // CHECK-LINK-NOLTO-ACTION-NOT: "-plugin" "{{.*}}/LLVMgold.so" + +// -flto passes along an explicit debugger tuning argument. +// RUN: %clang -target x86_64-unknown-linux -### %s -flto -glldb 2> %t +// RUN: FileCheck -check-prefix=CHECK-TUNING-LLDB < %t %s +// RUN: %clang -target x86_64-unknown-linux -### %s -flto -g 2> %t +// RUN: FileCheck -check-prefix=CHECK-NO-TUNING < %t %s +// +// CHECK-TUNING-LLDB: "-plugin-opt=-debugger-tune=lldb" +// CHECK-NO-TUNING-NOT: "-plugin-opt=-debugger-tune ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16501: [CUDA] Don't generate aliases for static extern "C" functions.
jlebar added a comment. In http://reviews.llvm.org/D16501#335225, @tra wrote: > Failing silently is not a good idea. At the very least there should produce > an error. > The right thing to do here, IMO, would be to generate a stub with alias name > that just jumps to or calls aliasee. Discussed IRL. We're not breaking anyone who's not already broken -- if you currently have "__device__ __attribute__((used)) static int foo()", that's just not going to compile for you without this patch. With this patch, at least this will work for some people; we can always add this stub function later if we want (I have concerns about potentially surprising performance implications, but we can postpone worrying about that). http://reviews.llvm.org/D16501 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16501: [CUDA] Don't generate aliases for static extern "C" functions.
tra accepted this revision. tra added a comment. This revision is now accepted and ready to land. OK. If someone attempts to rely on this feature in CUDA it will be obviously broken due to the missing C-style mangled name. We can deal with it then. http://reviews.llvm.org/D16501 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r258714 - Add support to the misc-assert-side-effect check for MSVC-style assert macros, which use !! instead of an if statement or a conditional operator.
Author: aaronballman Date: Mon Jan 25 14:00:53 2016 New Revision: 258714 URL: http://llvm.org/viewvc/llvm-project?rev=258714&view=rev Log: Add support to the misc-assert-side-effect check for MSVC-style assert macros, which use !! instead of an if statement or a conditional operator. Modified: clang-tools-extra/trunk/clang-tidy/misc/AssertSideEffectCheck.cpp clang-tools-extra/trunk/test/clang-tidy/misc-assert-side-effect.cpp Modified: clang-tools-extra/trunk/clang-tidy/misc/AssertSideEffectCheck.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/AssertSideEffectCheck.cpp?rev=258714&r1=258713&r2=258714&view=diff == --- clang-tools-extra/trunk/clang-tidy/misc/AssertSideEffectCheck.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/misc/AssertSideEffectCheck.cpp Mon Jan 25 14:00:53 2016 @@ -83,11 +83,18 @@ void AssertSideEffectCheck::storeOptions } void AssertSideEffectCheck::registerMatchers(MatchFinder *Finder) { - auto ConditionWithSideEffect = - hasCondition(hasDescendant(expr(hasSideEffect(CheckFunctionCalls; + auto DescendantWithSideEffect = + hasDescendant(expr(hasSideEffect(CheckFunctionCalls))); + auto ConditionWithSideEffect = hasCondition(DescendantWithSideEffect); Finder->addMatcher( - stmt(anyOf(conditionalOperator(ConditionWithSideEffect), - ifStmt(ConditionWithSideEffect))).bind("condStmt"), + stmt( + anyOf(conditionalOperator(ConditionWithSideEffect), +ifStmt(ConditionWithSideEffect), +unaryOperator(hasOperatorName("!"), + hasUnaryOperand(unaryOperator( + hasOperatorName("!"), + hasUnaryOperand(DescendantWithSideEffect)) + .bind("condStmt"), this); } Modified: clang-tools-extra/trunk/test/clang-tidy/misc-assert-side-effect.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-assert-side-effect.cpp?rev=258714&r1=258713&r2=258714&view=diff == --- clang-tools-extra/trunk/test/clang-tidy/misc-assert-side-effect.cpp (original) +++ clang-tools-extra/trunk/test/clang-tidy/misc-assert-side-effect.cpp Mon Jan 25 14:00:53 2016 @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s misc-assert-side-effect %t -- -config="{CheckOptions: [{key: misc-assert-side-effect.CheckFunctionCalls, value: 1}, {key: misc-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert'}]}" -- -fexceptions +// RUN: %check_clang_tidy %s misc-assert-side-effect %t -- -config="{CheckOptions: [{key: misc-assert-side-effect.CheckFunctionCalls, value: 1}, {key: misc-assert-side-effect.AssertMacros, value: 'assert,assert2,my_assert,convoluted_assert,msvc_assert'}]}" -- -fexceptions //===--- assert definition block --===// int abort() { return 0; } @@ -35,6 +35,12 @@ void print(...); #define wrap2(x) wrap1(x) #define convoluted_assert(x) wrap2(x) +#define msvc_assert(expression) (void)( \ +(!!(expression)) || \ +(abort(), 0) \ +) + + //===--===// class MyClass { @@ -101,5 +107,8 @@ int main() { assert2(1 == 2 - 1); + msvc_assert(mc2 = mc); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: found msvc_assert() with side effect + return 0; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
davidxl added inline comments. Comment at: lib/CodeGen/BackendUtil.cpp:440 @@ +439,3 @@ + if (CodeGenOpts.ProfileIRInstr) { +// Should not have ProfileInstrGenerate set -- it is for clang +// instrumentation only. xur wrote: > silvas wrote: > > Then change the existing references in clang. Please try to cleanly > > integrate the new functionality, not "sneak it in" with the smallest number > > of lines changed (and creating technical debt for future maintainers). > The integration is actually clean to me: For CodeGenOpt: ProfileInstrGenerate > is for Clang instrumentation and ProfileIRInstr is for IR instrumentation. > They need to mutually exclusive. > > Maybe I need to modify the name and description for ProfileInstrGenerate to > reflect the change. yes, you can change ProfileInstrGenerate to something like ClangProfInstrGen as a NFC first. Comment at: lib/Frontend/CompilerInvocation.cpp:487 @@ +486,3 @@ + // Opts.ProfileInstrGenerate will be used for Clang instrumentation only. + if (!Opts.ProfileIRInstr) +Opts.ProfileInstrGenerate = Args.hasArg(OPT_fprofile_instr_generate) || xur wrote: > silvas wrote: > > I don't like this. It breaks the 1:1 mapping of flags to codegen options. > > Like Chad said, this sort of complexity should be kept in the driver. > > > > Some refactoring may be needed to cleanly integrate the new IR > > instrumentation. > hmm. I don't think there is 1:1 mapping b/w flags and codegen options, > because -fprofile-instr-generate is shared by IR and FE instrumentation. If you rename the FE PGO CodeGen opt name as proposed, it will be clearer and less confusing. http://reviews.llvm.org/D15829 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15636: Reduce false positives in printf/scanf format checker
rtrieu added a comment. Hi Andy, Could you give some motivation for this patch? From your example: printf(minimal ? "%i\n" : "%i: %s\n", code, msg); I would expect the first format string to produce a warning that msg is not used. http://reviews.llvm.org/D15636 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16552: Let clang not error out on `const std::vector empty_vec; ` with libstdc++5.3.
thakis created this revision. thakis added a reviewer: rsmith. thakis added a subscriber: cfe-commits. libstdc++ is using `= default` instead of `{}` for more and more of its container classes, because they think DR 253 should be accepted (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60284). Since clang doesn't implement DR 253, clang complains on definitions of empty const objects of these containers. This is unfortunate, since `const std::vector empty_vec;` builds fine with gcc, clang with older libstdc++ versions, clang with libc++, and Visual Studio. This patch adds a clang-side workaround for libstdc++ types: For types in system headers that are in namespace std and that have no uninitialized fields and an explicitly defaulted constructed, clang now allows defining const objects of that type. Related to PR23381. Similar in spirit to r212238. http://reviews.llvm.org/D16552 Files: lib/Sema/SemaInit.cpp test/SemaCXX/cxx0x-const-defaulted-ctor-system-header.cpp Index: test/SemaCXX/cxx0x-const-defaulted-ctor-system-header.cpp === --- test/SemaCXX/cxx0x-const-defaulted-ctor-system-header.cpp +++ test/SemaCXX/cxx0x-const-defaulted-ctor-system-header.cpp @@ -0,0 +1,18 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s +// expected-no-diagnostics + +// libstdc++5.3 uses `= default` for its container constructors. +#ifdef BE_THE_HEADER +#pragma clang system_header +namespace std { +template +struct vector { + vector() = default; +}; +} +#else + +#define BE_THE_HEADER +#include __FILE__ +const std::vector empty_vector; // no diagnostic +#endif Index: lib/Sema/SemaInit.cpp === --- lib/Sema/SemaInit.cpp +++ lib/Sema/SemaInit.cpp @@ -360,6 +360,15 @@ // semantic analysis and code generation. InitListExpr *getFullyStructuredList() const { return FullyStructuredList; } }; + +bool IsContainedInNamespaceStd(Sema& S, CXXRecordDecl *R) { + for (NamespaceDecl *ND = dyn_cast(R->getDeclContext()); ND; + ND = dyn_cast(ND->getParent())) { +if (S.getStdNamespace()->InEnclosingNamespaceSetOf(ND)) + return true; + } + return false; +} } // end anonymous namespace ExprResult InitListChecker::PerformEmptyInit(Sema &SemaRef, @@ -419,16 +428,8 @@ if (CtorDecl->getMinRequiredArguments() == 0 && CtorDecl->isExplicit() && R->getDeclName() && SemaRef.SourceMgr.isInSystemHeader(CtorDecl->getLocation())) { - - - bool IsInStd = false; - for (NamespaceDecl *ND = dyn_cast(R->getDeclContext()); - ND && !IsInStd; ND = dyn_cast(ND->getParent())) { -if (SemaRef.getStdNamespace()->InEnclosingNamespaceSetOf(ND)) - IsInStd = true; - } - - if (IsInStd && llvm::StringSwitch(R->getName()) + if (IsContainedInNamespaceStd(SemaRef, R) && + llvm::StringSwitch(R->getName()) .Cases("basic_string", "deque", "forward_list", true) .Cases("list", "map", "multimap", "multiset", true) .Cases("priority_queue", "queue", "set", "stack", true) @@ -3420,6 +3421,18 @@ return CandidateSet.BestViableFunction(S, DeclLoc, Best); } +static bool HasUninitializedFields(CXXRecordDecl* R) { + for (const auto *F : R->fields()) { +if (F->hasInClassInitializer() || F->isUnnamedBitfield()) + continue; +if (CXXRecordDecl *FieldType = F->getType()->getAsCXXRecordDecl()) + if (FieldType->hasDefaultConstructor()) +continue; +return true; + } + return false; +} + /// \brief Attempt initialization by constructor (C++ [dcl.init]), which /// enumerates the constructors of the initialized entity and performs overload /// resolution to select the best. @@ -3517,18 +3530,28 @@ // If a program calls for the default initialization of an object // of a const-qualified type T, T shall be a class type with a // user-provided default constructor. + CXXConstructorDecl *CtorDecl = cast(Best->Function); if (Kind.getKind() == InitializationKind::IK_Default && - Entity.getType().isConstQualified() && - !cast(Best->Function)->isUserProvided()) { -if (!maybeRecoverWithZeroInitialization(S, Sequence, Entity)) - Sequence.SetFailed(InitializationSequence::FK_DefaultInitOfConst); -return; + Entity.getType().isConstQualified() && !CtorDecl->isUserProvided()) { +// Some libstdc++ types use `MyType() = default;` which technically isn't +// a user-provided default constructor. For types in system headers that +// are in namespace std and that only have fields that have default +// constructors, look the other way. See also core issue 253. +CXXRecordDecl *R = CtorDecl->getParent(); +bool AllowConstDefaultInit = +S.SourceMgr.isInSystemHeader(CtorDecl->getLocation()) && +CtorDecl->isDefaulted() && IsContainedInNamespaceStd(S, R) && +!HasUninitializedFields(R); +if (!
Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
kromanova removed rL LLVM as the repository for this revision. kromanova updated this revision to Diff 45902. kromanova added a comment. SCE's techinical writer, Craig Flores, did the code review and suggested a few changes to the documentation. http://reviews.llvm.org/D15999 Files: lib/Headers/__wmmintrin_pclmul.h Index: lib/Headers/__wmmintrin_pclmul.h === --- lib/Headers/__wmmintrin_pclmul.h +++ lib/Headers/__wmmintrin_pclmul.h @@ -23,6 +23,36 @@ #ifndef _WMMINTRIN_PCLMUL_H #define _WMMINTRIN_PCLMUL_H +/// \brief Multiplies two 64-bit integer values, which are selected from source +///operands using the immediate-value operand. The multiplication is a +///carry-less multiplication, and the 128-bit integer product is stored in +///the destination. +/// +/// \headerfile +/// +/// \code +/// __m128i _mm_clmulepi64_si128(__m128i __X, __m128i __Y, const int __I); +/// \endcode +/// +/// This intrinsic corresponds to \c VPCLMULQDQ instruction. +/// +/// \param __X +///A 128-bit vector of [2 x i64] containing one of the source +///operands. +/// \param __Y +///A 128-bit vector of [2 x i64] containing one of the source +///operands. +/// \param __I +///An immediate value specifying which 64-bit values to select +///from the operands. +///Bit 0 is used to select a value from operand __X, +///and bit 4 is used to select a value from operand __Y: +///Bit[0]=0 indicates that bits[63:0] of operand __X are used. +///Bit[0]=1 indicates that bits[127:64] of operand __X are used. +///Bit[4]=0 indicates that bits[63:0] of operand __Y are used. +///Bit[4]=1 indicates that bits[127:64] of operand __Y are used. +/// \returns The 128-bit integer vector containing the result of the carry-less +///multiplication of the selected 64-bit values. #define _mm_clmulepi64_si128(__X, __Y, __I) \ ((__m128i)__builtin_ia32_pclmulqdq128((__v2di)(__m128i)(__X), \ (__v2di)(__m128i)(__Y), (char)(__I))) Index: lib/Headers/__wmmintrin_pclmul.h === --- lib/Headers/__wmmintrin_pclmul.h +++ lib/Headers/__wmmintrin_pclmul.h @@ -23,6 +23,36 @@ #ifndef _WMMINTRIN_PCLMUL_H #define _WMMINTRIN_PCLMUL_H +/// \brief Multiplies two 64-bit integer values, which are selected from source +///operands using the immediate-value operand. The multiplication is a +///carry-less multiplication, and the 128-bit integer product is stored in +///the destination. +/// +/// \headerfile +/// +/// \code +/// __m128i _mm_clmulepi64_si128(__m128i __X, __m128i __Y, const int __I); +/// \endcode +/// +/// This intrinsic corresponds to \c VPCLMULQDQ instruction. +/// +/// \param __X +///A 128-bit vector of [2 x i64] containing one of the source +///operands. +/// \param __Y +///A 128-bit vector of [2 x i64] containing one of the source +///operands. +/// \param __I +///An immediate value specifying which 64-bit values to select +///from the operands. +///Bit 0 is used to select a value from operand __X, +///and bit 4 is used to select a value from operand __Y: +///Bit[0]=0 indicates that bits[63:0] of operand __X are used. +///Bit[0]=1 indicates that bits[127:64] of operand __X are used. +///Bit[4]=0 indicates that bits[63:0] of operand __Y are used. +///Bit[4]=1 indicates that bits[127:64] of operand __Y are used. +/// \returns The 128-bit integer vector containing the result of the carry-less +///multiplication of the selected 64-bit values. #define _mm_clmulepi64_si128(__X, __Y, __I) \ ((__m128i)__builtin_ia32_pclmulqdq128((__v2di)(__m128i)(__X), \ (__v2di)(__m128i)(__Y), (char)(__I))) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16527: [OpenMP] Parsing + sema for defaultmap clause.
arpith-jacob marked 5 inline comments as done. arpith-jacob added a comment. Patch fixed. http://reviews.llvm.org/D16527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r257947 - Avoid self-assignment of SmallString, trigger UB behavior down the road.
Merged in r258715. On Mon, Jan 25, 2016 at 11:39 AM, Hans Wennborg wrote: > Joerg, do you want to go ahead and merge this to 3.8 with > utils/release/merge.sh? (Or let me know if you'd prefer me to do it.) > > On Sun, Jan 24, 2016 at 11:29 PM, Richard Smith wrote: >> Approved for branch. >> >> On 23 Jan 2016 11:56 p.m., "Joerg Sonnenberger" >> wrote: >>> >>> On Sat, Jan 16, 2016 at 03:51:11PM +0100, Joerg Sonnenberger via >>> cfe-commits wrote: >>> > Hello Richard, >>> > can this be merged into 3.8? It creates some trivial noise under >>> > valgrind. >>> >>> Ping? >>> >>> Joerg ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16535: [clang-tidy] Check to find unintended semicolons that changes the semantics.
kimgr added a subscriber: kimgr. kimgr added a comment. Cool check, I like how it pays attention to indentation! I found some minor doc nits, see inline. Comment at: docs/clang-tidy/checks/misc-suspicious-semicolon.rst:8-9 @@ +7,4 @@ +the code. More specifically, it looks for `if`, `while`, `for` and `for-range` +statements whose body is a single semicolon, and then analyzes the +context of the code (e.g. indentation) in an attempt to determine whether that +is intentional. "context" looks like it would fit on the wrapped line. Comment at: docs/clang-tidy/checks/misc-suspicious-semicolon.rst:20 @@ +19,3 @@ +Here the body of the `if` statement consists of only the semicolon at the end of +the first line, and `x` will be increased regardless of the condition. + incremented? Comment at: docs/clang-tidy/checks/misc-suspicious-semicolon.rst:29 @@ +28,3 @@ +As a result of this code, `processLine()` will only be called once, when the +`while` loop with the empty body exits with `line == NULL`. The identation of +the code indicates the intention of the programmer. Typo: identation Comment at: docs/clang-tidy/checks/misc-suspicious-semicolon.rst:71 @@ +70,3 @@ + +In this case the checker will assume that you know what you are doing, and will +not raise a warning. There's been some preference for "check" over "checker" lately. Comment at: test/clang-tidy/misc-suspicious-semicolon.cpp:88 @@ +87,3 @@ + char c = 'b'; + char * s = "a"; + if (s == "(" || s != "'" || c == '"') { Weird pointer alignment is a little distracting here, better stick with LLVM convention and attach it to `s`? http://reviews.llvm.org/D16535 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
silvas added inline comments. Comment at: lib/Frontend/CompilerInvocation.cpp:487 @@ +486,3 @@ + // Opts.ProfileInstrGenerate will be used for Clang instrumentation only. + if (!Opts.ProfileIRInstr) +Opts.ProfileInstrGenerate = Args.hasArg(OPT_fprofile_instr_generate) || davidxl wrote: > xur wrote: > > silvas wrote: > > > I don't like this. It breaks the 1:1 mapping of flags to codegen options. > > > Like Chad said, this sort of complexity should be kept in the driver. > > > > > > Some refactoring may be needed to cleanly integrate the new IR > > > instrumentation. > > hmm. I don't think there is 1:1 mapping b/w flags and codegen options, > > because -fprofile-instr-generate is shared by IR and FE instrumentation. > If you rename the FE PGO CodeGen opt name as proposed, it will be clearer and > less confusing. > hmm. I don't think there is 1:1 mapping b/w flags and codegen options, > because -fprofile-instr-generate is shared by IR and FE instrumentation. Maybe at the driver level (that discussion has yet to conclude), but at cc1 level options should be factored to be clean and orthogonal. Like Chad said, the complexity of verifying options that are mutually exclusive or that must be present with other options should be done in the driver. cc1 is a private interface http://reviews.llvm.org/D15829 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16066: [clang-format] Add BeforeWhileInDoWhile BraceWrapping option
ebaker355 added a comment. Ping. Anyone have an opinion on this? http://reviews.llvm.org/D16066 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
OK. Now I understand what you meant. I was trying to avoid driver changes. It seems you are saying I can change the driver to support cc1 options more cleanly. This can be done. Thanks, Rong On Jan 25, 2016 12:43, "Sean Silva" wrote: > silvas added inline comments. > > > Comment at: lib/Frontend/CompilerInvocation.cpp:487 > @@ +486,3 @@ > + // Opts.ProfileInstrGenerate will be used for Clang instrumentation > only. > + if (!Opts.ProfileIRInstr) > +Opts.ProfileInstrGenerate = Args.hasArg(OPT_fprofile_instr_generate) > || > > davidxl wrote: > > xur wrote: > > > silvas wrote: > > > > I don't like this. It breaks the 1:1 mapping of flags to codegen > options. Like Chad said, this sort of complexity should be kept in the > driver. > > > > > > > > Some refactoring may be needed to cleanly integrate the new IR > instrumentation. > > > hmm. I don't think there is 1:1 mapping b/w flags and codegen options, > because -fprofile-instr-generate is shared by IR and FE instrumentation. > > If you rename the FE PGO CodeGen opt name as proposed, it will be > clearer and less confusing. > > hmm. I don't think there is 1:1 mapping b/w flags and codegen options, > because -fprofile-instr-generate is shared by IR and FE instrumentation. > > Maybe at the driver level (that discussion has yet to conclude), but at > cc1 level options should be factored to be clean and orthogonal. Like Chad > said, the complexity of verifying options that are mutually exclusive or > that must be present with other options should be done in the driver. cc1 > is a private interface > > > > http://reviews.llvm.org/D15829 > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
xur added a subscriber: xur. xur added a comment. OK. Now I understand what you meant. I was trying to avoid driver changes. It seems you are saying I can change the driver to support cc1 options more cleanly. This can be done. Thanks, Rong http://reviews.llvm.org/D15829 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
On Mon, Jan 25, 2016 at 12:52 PM, Rong Xu wrote: > OK. Now I understand what you meant. I was trying to avoid driver changes. > It seems you are saying I can change the driver to support cc1 options more > cleanly. This can be done. > Great! Sorry for the confusion. -- Sean Silva > Thanks, > > Rong > On Jan 25, 2016 12:43, "Sean Silva" wrote: > >> silvas added inline comments. >> >> >> Comment at: lib/Frontend/CompilerInvocation.cpp:487 >> @@ +486,3 @@ >> + // Opts.ProfileInstrGenerate will be used for Clang instrumentation >> only. >> + if (!Opts.ProfileIRInstr) >> +Opts.ProfileInstrGenerate = Args.hasArg(OPT_fprofile_instr_generate) >> || >> >> davidxl wrote: >> > xur wrote: >> > > silvas wrote: >> > > > I don't like this. It breaks the 1:1 mapping of flags to codegen >> options. Like Chad said, this sort of complexity should be kept in the >> driver. >> > > > >> > > > Some refactoring may be needed to cleanly integrate the new IR >> instrumentation. >> > > hmm. I don't think there is 1:1 mapping b/w flags and codegen >> options, because -fprofile-instr-generate is shared by IR and FE >> instrumentation. >> > If you rename the FE PGO CodeGen opt name as proposed, it will be >> clearer and less confusing. >> > hmm. I don't think there is 1:1 mapping b/w flags and codegen options, >> because -fprofile-instr-generate is shared by IR and FE instrumentation. >> >> Maybe at the driver level (that discussion has yet to conclude), but at >> cc1 level options should be factored to be clean and orthogonal. Like Chad >> said, the complexity of verifying options that are mutually exclusive or >> that must be present with other options should be done in the driver. cc1 >> is a private interface >> >> >> >> http://reviews.llvm.org/D15829 >> >> >> >> ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15829: [PGO] Clang Option that enables IR level PGO instrumentation
On Mon, Jan 25, 2016 at 11:00 AM, Xinliang David Li wrote: > On Fri, Jan 22, 2016 at 1:43 PM, Sean Silva wrote: > > silvas added a comment. > > > > In http://reviews.llvm.org/D15829#333902, @davidxl wrote: > > > >> For the longer term, one possible solution is to make FE based > >> instrumentation only used for coverage testing which can be turned on > >> with -fcoverage-mapping option (currently, -fcoverage-mapping can not > >> be used alone and must be used together with > >> -fprofile-instr-generate). To summarize: > >> > >> A. Current behavior: > >> > >> --- > >> > >> 1. -fprofile-instr-generate turns on FE based instrumentation > >> 2. -fprofile-instr-generate -fcoverage-mapping turns on FE based > instrumentation and coverage mapping data generation. > >> 3. -fprofile-instr-use=<..> assumes profile data from FE > instrumentation. > >> > >> B. Proposed new behavior: > >> > >> > >> > >> 1. -fprofile-instr-generate turns on IR late instrumentation > >> 2. -fcoverage-mapping turns on FE instrumentation and coverage-mapping > >> 3. -fprofile-instr-generate -fcoverage-mapping result in compiler > warning > >> 4. -fprofile-instr-use=<> will automatically determine how to use the > profile data. > > > > > > Very good observation that we can determine FE or IR automatically based > on the input profdata. That simplifies things. > > > >> B.2) above can be done today for improved usability. > > > > > > I don't see how this improves usability. In fact, it is confusing > because it hijacks an existing option. > > > > Also B.3 causes existing user builds to emit a warning, which is > annoying. > > Since it is pointless to specify -fcoverage-mapping option alone, why > not do not automatically? Yes it means some behavior change (in a good > way) and some annoying warnings initially, but those are trivially > fixable by the user. > "trivially fixable" needs to be weighed against the number of times it needs to be fixed across the user base. Anyway, emitting a warning for this is clearly a separate discussion (separate even from the discussion of whether to accept just `-fcoverage-mapping` without `-fprofile-instr-generate`), so let's not get hung up on it. > > > > > > I would propose the following modification of B: > > > > C.: > > > > 1. -fprofile-instr-generate defaults to IR instrumentation (i.e. behaves > exactly as before, except that it uses IR instrumentation) > > 2. -fprofile-instr-generate -fcoverage-mapping turns on frontend > instrumentation and coverage. (i.e. behaves exactly as before) > > I am fine with this -- as long as the user does not need to bear the > burden of understanding where and how the instrumentation is done. > Agreed. > > > 3. -fprofile-instr-use=<> automatically determines which method to use > > > > All existing user workflows continue to work, except for workflows that > attempt to `llvm-profdata merge` some old frontend profile data (e.g. they > have checked-in to version control and represents some special workload) > with the profile data from new binaries. > > > > Concretely, imagine the following workflow: > > > > clang -fprofile-instr-generate foo.c -o foo > > ./foo > > llvm-profdata merge default.profraw -o new.profdata > > llvm-profdata merge new.profdata > /versioncontrol/some-important-but-expensive-to-reproduce-workload.profdata > -o foo.profdata > > clang -fprofile-instr-use=foo.profdata foo.c -o foo_pgo > > > > I think this is a reasonable breakage. We would need to add a note in > the release notes. Unfortunately this is not expected breakage if we claim > to have forward compatibility for profdata (which IIRC Apple requires; > @bogner?). But I think this case will be rare and exceptional enough that > we can tolerate it: > > > > The old profile data has to out live the program source versions which > usually change fast. In reality, I don't expect profile data to live > too long. > I don't either, but it's not unthinkable. E.g. a user may have the sources of a dependency checked-in, and they only update the profdata for those sources when they update the dependency. But overall, I think that release notes mentioning the workaround for this situation is sufficient. -- Sean Silva > > > - a simple immediate workaround is to specify `-fcoverage-mapping` > (which also adds some extra stuff, but works around the issue) > > This is a reasonable workaround > > > - Presumably > /versioncontrol/some-important-but-expensive-to-reproduce-workload.profdata > is regenerated with some frequency which is more frequent than upgrading > the compiler, and so it is likely reasonable to regenerate it alongside a > compiler upgrade, using the workaround until then. > > > > > > > >> B.1) needs a > > > >> transition period before the IR based instrumentation becomes > > > >> stablized (and can be flipped to the default). During the transition > > > >> period, the behavior of 1) does not change, but a cc1
Re: [PATCH] D16511: [MSVC Compat] Only warn for unknown clang-cl arguments
Thanks for the reviews! Will address both when landing. On Mon, Jan 25, 2016 at 2:34 PM, Hans Wennborg wrote: > hans added a subscriber: hans. > hans added a comment. > > Maybe this behavior should be called out in the Command-Line Options > section under clang-cl in docs/UsersManual.rst? > > > http://reviews.llvm.org/D16511 > > > > -- Ehsan ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15705: Adding a scripted test for PR25717
On 25 January 2016 at 11:03, Yunzhong Gao wrote: > ygao added a comment. > > Thanks, Rafael! > I did verify that the test would fail without my fix. But what do I need to > do to make buildbots turn on the new lit parameter? That I do not know. Mike? Cheers, Rafael ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r258720 - [MSVC Compat] Only warn for unknown clang-cl arguments
Author: ehsan Date: Mon Jan 25 15:14:52 2016 New Revision: 258720 URL: http://llvm.org/viewvc/llvm-project?rev=258720&view=rev Log: [MSVC Compat] Only warn for unknown clang-cl arguments Summary: MSVC's driver accepts all unknown arguments but warns about them. clang by default rejects all unknown arguments. This causes issues specifically with build systems such as autoconf which liberally pass things such as $LDFLAGS to the compiler and expect everything to work. This patch teaches clang-cl to ignore unknown driver arguments. Reviewers: rnk Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D16511 Modified: cfe/trunk/docs/UsersManual.rst cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/lib/Driver/Driver.cpp cfe/trunk/lib/Driver/Tools.cpp cfe/trunk/test/Driver/cl-fallback.c cfe/trunk/test/Driver/unknown-arg.c cfe/trunk/test/Misc/serialized-diags-driver.c Modified: cfe/trunk/docs/UsersManual.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=258720&r1=258719&r2=258720&view=diff == --- cfe/trunk/docs/UsersManual.rst (original) +++ cfe/trunk/docs/UsersManual.rst Mon Jan 25 15:14:52 2016 @@ -2024,8 +2024,9 @@ with a warning. For example: To suppress warnings about unused arguments, use the ``-Qunused-arguments`` option. -Options that are not known to clang-cl will cause errors. If they are spelled with a -leading ``/``, they will be mistaken for a filename: +Options that are not known to clang-cl will be ignored by default. Use the +``-Werror=unknown-argument`` option in order to treat them as errors. If these +options are spelled with a leading ``/``, they will be mistaken for a filename: :: Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=258720&r1=258719&r2=258720&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Mon Jan 25 15:14:52 2016 @@ -93,6 +93,9 @@ def err_target_unsupported_arch def err_drv_I_dash_not_supported : Error< "'%0' not supported, please use -iquote instead">; def err_drv_unknown_argument : Error<"unknown argument: '%0'">; +def warn_drv_unknown_argument_clang_cl : Warning< + "unknown argument ignored in clang-cl: '%0'">, + InGroup; def err_drv_invalid_value : Error<"invalid value '%1' in '%0'">; def err_drv_invalid_int_value : Error<"invalid integral value '%1' in '%0'">; def err_drv_invalid_remap_file : Error< Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=258720&r1=258719&r2=258720&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Jan 25 15:14:52 2016 @@ -848,3 +848,5 @@ def FutureCompat : DiagGroup<"future-com def InvalidOrNonExistentDirectory : DiagGroup<"invalid-or-nonexistent-directory">; def OptionIgnored : DiagGroup<"option-ignored">; + +def UnknownArgument : DiagGroup<"unknown-argument">; Modified: cfe/trunk/lib/Driver/Driver.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=258720&r1=258719&r2=258720&view=diff == --- cfe/trunk/lib/Driver/Driver.cpp (original) +++ cfe/trunk/lib/Driver/Driver.cpp Mon Jan 25 15:14:52 2016 @@ -146,7 +146,9 @@ InputArgList Driver::ParseArgStrings(Arr } for (const Arg *A : Args.filtered(options::OPT_UNKNOWN)) -Diags.Report(diag::err_drv_unknown_argument) << A->getAsString(Args); +Diags.Report(IsCLMode() ? diag::warn_drv_unknown_argument_clang_cl : + diag::err_drv_unknown_argument) + << A->getAsString(Args); return Args; } @@ -1710,8 +1712,11 @@ void Driver::BuildJobs(Compilation &C) c continue; } - Diag(clang::diag::warn_drv_unused_argument) - << A->getAsString(C.getArgs()); + // In clang-cl, don't mention unknown arguments here since they have + // already been warned about. + if (!IsCLMode() || !A->getOption().matches(options::OPT_UNKNOWN)) +Diag(clang::diag::warn_drv_unused_argument) +<< A->getAsString(C.getArgs()); } } } Modified: cfe/trunk/lib/Driver/Tools.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=258720&r1=258719&r2=258720&view=diff == --- cfe/trunk/lib/Driver/Tools.cpp (original) +++ cf
Re: [PATCH] D16511: [MSVC Compat] Only warn for unknown clang-cl arguments
This revision was automatically updated to reflect the committed changes. Closed by commit rL258720: [MSVC Compat] Only warn for unknown clang-cl arguments (authored by ehsan). Changed prior to commit: http://reviews.llvm.org/D16511?vs=45796&id=45906#toc Repository: rL LLVM http://reviews.llvm.org/D16511 Files: cfe/trunk/docs/UsersManual.rst cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/lib/Driver/Driver.cpp cfe/trunk/lib/Driver/Tools.cpp cfe/trunk/test/Driver/cl-fallback.c cfe/trunk/test/Driver/unknown-arg.c cfe/trunk/test/Misc/serialized-diags-driver.c Index: cfe/trunk/lib/Driver/Tools.cpp === --- cfe/trunk/lib/Driver/Tools.cpp +++ cfe/trunk/lib/Driver/Tools.cpp @@ -9708,6 +9708,10 @@ options::OPT__SLASH_MT, options::OPT__SLASH_MTd)) A->render(Args, CmdArgs); + // Pass through all unknown arguments so that the fallback command can see + // them too. + Args.AddAllArgs(CmdArgs, options::OPT_UNKNOWN); + // Input filename. assert(Inputs.size() == 1); const InputInfo &II = Inputs[0]; Index: cfe/trunk/lib/Driver/Driver.cpp === --- cfe/trunk/lib/Driver/Driver.cpp +++ cfe/trunk/lib/Driver/Driver.cpp @@ -146,7 +146,9 @@ } for (const Arg *A : Args.filtered(options::OPT_UNKNOWN)) -Diags.Report(diag::err_drv_unknown_argument) << A->getAsString(Args); +Diags.Report(IsCLMode() ? diag::warn_drv_unknown_argument_clang_cl : + diag::err_drv_unknown_argument) + << A->getAsString(Args); return Args; } @@ -1710,8 +1712,11 @@ continue; } - Diag(clang::diag::warn_drv_unused_argument) - << A->getAsString(C.getArgs()); + // In clang-cl, don't mention unknown arguments here since they have + // already been warned about. + if (!IsCLMode() || !A->getOption().matches(options::OPT_UNKNOWN)) +Diag(clang::diag::warn_drv_unused_argument) +<< A->getAsString(C.getArgs()); } } } Index: cfe/trunk/docs/UsersManual.rst === --- cfe/trunk/docs/UsersManual.rst +++ cfe/trunk/docs/UsersManual.rst @@ -2024,8 +2024,9 @@ To suppress warnings about unused arguments, use the ``-Qunused-arguments`` option. -Options that are not known to clang-cl will cause errors. If they are spelled with a -leading ``/``, they will be mistaken for a filename: +Options that are not known to clang-cl will be ignored by default. Use the +``-Werror=unknown-argument`` option in order to treat them as errors. If these +options are spelled with a leading ``/``, they will be mistaken for a filename: :: Index: cfe/trunk/include/clang/Basic/DiagnosticGroups.td === --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td @@ -848,3 +848,5 @@ def InvalidOrNonExistentDirectory : DiagGroup<"invalid-or-nonexistent-directory">; def OptionIgnored : DiagGroup<"option-ignored">; + +def UnknownArgument : DiagGroup<"unknown-argument">; Index: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td === --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td @@ -93,6 +93,9 @@ def err_drv_I_dash_not_supported : Error< "'%0' not supported, please use -iquote instead">; def err_drv_unknown_argument : Error<"unknown argument: '%0'">; +def warn_drv_unknown_argument_clang_cl : Warning< + "unknown argument ignored in clang-cl: '%0'">, + InGroup; def err_drv_invalid_value : Error<"invalid value '%1' in '%0'">; def err_drv_invalid_int_value : Error<"invalid integral value '%1' in '%0'">; def err_drv_invalid_remap_file : Error< Index: cfe/trunk/test/Driver/cl-fallback.c === --- cfe/trunk/test/Driver/cl-fallback.c +++ cfe/trunk/test/Driver/cl-fallback.c @@ -3,6 +3,7 @@ // RUN: %clang_cl --target=i686-pc-win32 /fallback /Dfoo=bar /Ubaz /Ifoo /O0 /Ox /GR /GR- /Gy /Gy- \ // RUN: /Gw /Gw- /LD /LDd /EHs /EHs- /Zl /MD /MDd /MTd /MT /FImyheader.h /Zi \ +// RUN: -garbage -moregarbage \ // RUN: -### -- %s 2>&1 \ // RUN: | FileCheck %s // CHECK: "-fdiagnostics-format" "msvc-fallback" @@ -31,6 +32,8 @@ // CHECK: "/EHs-" // CHECK: "/Zl" // CHECK: "/MT" +// CHECK: "-garbage" +// CHECK: "-moregarbage" // CHECK: "/Tc" "{{.*cl-fallback.c}}" // CHECK: "/Fo{{.*cl-fallback.*.obj}}" Index: cfe/trunk/test/Driver/unknown-arg.c === --- cfe/trunk/test/Driver/unknown-arg.c +++ cfe/trunk/test/Driver/unknown-arg.c @@ -1,13 +1,28 @@ // RUN: not %clang %s -ca
Re: [PATCH] D16044: getVariableName() for MemRegion
Alexander_Droste added inline comments. Comment at: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:662 @@ +661,3 @@ +else if (auto SV = +ER->getIndex().getAs()) { +llvm::SmallString<8> buf; xazax.hun wrote: > These are the only cases that can occur? So it is not possible to have a loc > index? Maybe we could get the value for loc and nonloc ConcreteInts and use > getVariableName for everzthing else? Would that wok? `getIndex()` returns a `nonloc` and `loc` is in no subclass relation to `nonloc`. Therefore, `getIndex()` cannot be a `loc` type. There are actually 5 `nonloc` subclasses but I assumed that it only makes sense to check for `SymbolVal` and `ConcreteInt`. http://clang.llvm.org/doxygen/classclang_1_1ento_1_1NonLoc.html So would you suggest to only check for `ConcreteInt` and call `getVariableName()` recursively if it isn't one? Comment at: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:669 @@ +668,3 @@ + +ArrayIndices += "]" + idx + "["; +R = ER->getSuperRegion(); xazax.hun wrote: > In this case idx also needs to be reserved. Since it is very rare that there > are a huge level of []s, it is possible that the Twine solution is more > desirable. Ok, I'll look into the Twine API. Comment at: tools/clang/lib/StaticAnalyzer/Core/MemRegion.cpp:682 @@ +681,3 @@ + // Combine variable name with indices. + if (VariableName.size() && VariableName.back() == '\'') { +VariableName.insert(VariableName.size() - 1, ArrayIndices); xazax.hun wrote: > What is happening here exactly? In some cases there are ' symbols around the > name and in other cases there aren't? Is there a way to solve this > inconsistency (so we do not need to check for the last character)? Also it > would be great that this functions always had consistent result, so either > never put ' around the name or always. I think I wasn't completely sure if single quotes are always included when `printPretty()` is called. I looked into `MemRegion.cpp` and they always seem to be included, why the second case can be removed. http://reviews.llvm.org/D16044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15599: [CodeGen] Fix a crash that occurs when attribute "naked" is attached to a c++ member function
ahatanak added inline comments. Comment at: lib/CodeGen/CodeGenFunction.cpp:1954 @@ +1953,3 @@ + // Erase all allocas and their users. + for (auto I = EntryBB->begin(); &*I != &*AllocaInsertPt; ++I) +if (auto Alloca = dyn_cast(&*I)) hans wrote: > would declaring I as llvm::Instruction* allow you to get rid off the "&*" > stuff here and below? If so, same for "auto BB" further down. We can git rid of the "&*" in the code here if we declare I as Instruction*, but we will need to add "&*" to Entry->begin() to convert it to an Instruction*. In addition, we will have to use getNextNode instead of ++II or I++, so it seems to me that using an iterator is better in this case. http://reviews.llvm.org/D15599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15599: [CodeGen] Fix a crash that occurs when attribute "naked" is attached to a c++ member function
ahatanak updated this revision to Diff 45910. ahatanak added a comment. Address review comments from Hans and Manman. http://reviews.llvm.org/D15599 Files: lib/CodeGen/CGCall.cpp lib/CodeGen/CGStmt.cpp lib/CodeGen/CodeGenFunction.cpp lib/CodeGen/CodeGenFunction.h test/CodeGen/attr-naked.c test/CodeGenCXX/attr-naked.cpp test/CodeGenCXX/ms-inline-asm-return.cpp Index: test/CodeGenCXX/ms-inline-asm-return.cpp === --- test/CodeGenCXX/ms-inline-asm-return.cpp +++ test/CodeGenCXX/ms-inline-asm-return.cpp @@ -1,5 +1,5 @@ // REQUIRES: x86-registered-target -// RUN: %clang_cc1 %s -triple i686-pc-windows-msvc -emit-llvm -o - -fasm-blocks | FileCheck %s +// RUN: %clang_cc1 %s -triple i686-pc-windows-msvc -emit-llvm -o - -fasm-blocks -fms-compatibility | FileCheck %s // Check that we take EAX or EAX:EDX and return it from these functions for MSVC // compatibility. @@ -98,3 +98,17 @@ // CHECK-LABEL: define i32 @main() // CHECK: %[[r:[^ ]*]] = call i32 asm sideeffect inteldialect "xor eax, eax", "={eax},{{.*}}" // CHECK: ret i32 %[[r]] + +// Don't set the return value if the function is marked as naked. +__declspec(naked) int nakedFunc(int a, int b) +{ + __asm { +ret + } +} + +// CHECK: define i32 @{{.*}}nakedFunc +// CHECK-NEXT: entry: +// CHECK-NEXT: call void asm sideeffect inteldialect "ret +// CHECK-NEXT: unreachable +// CHECK-NEXT: } Index: test/CodeGenCXX/attr-naked.cpp === --- /dev/null +++ test/CodeGenCXX/attr-naked.cpp @@ -0,0 +1,55 @@ +// RUN: %clang_cc1 -triple=x86_64-apple-darwin -std=c++11 -emit-llvm -o - %s | FileCheck %s + +class Class1 { +public: + void __attribute__((naked)) m1() { __asm__ volatile("retq"); } + int __attribute__((naked)) m2() { __asm__ volatile("retq"); } +}; + +int foo1(int a, Class1 *c1) { + auto Fn = []() __attribute__((naked))->int { +__asm__ volatile("retq"); + }; + + if (a < 0) +return Fn(); + + if (a > 0) +return c1->m2(); + + c1->m1(); + return 1; +} + +// Check for the absence of llvm.trap. +int __attribute__((naked)) foo2(int a) { + __asm__ volatile("retq"); +} + +// CHECK: define internal i32 @"_ZZ4foo1iP6Class1ENK3$_0clEv"(%class.anon* %this) [[ATTR1:#[0-9]]] +// CHECK-NEXT: entry: +// CHECK-NEXT: call void asm sideeffect "retq +// CHECK-NEXT: unreachable +// CHECK-NEXT: } + + +// CHECK: define linkonce_odr i32 @_ZN6Class12m2Ev(%class.Class1* %this) [[ATTR2:#[0-9]]] +// CHECK-NEXT: entry: +// CHECK-NEXT: call void asm sideeffect "retq +// CHECK-NEXT: unreachable +// CHECK-NEXT: } + +// CHECK: define linkonce_odr void @_ZN6Class12m1Ev(%class.Class1* %this) [[ATTR2]] +// CHECK-NEXT: entry: +// CHECK-NEXT: call void asm sideeffect "retq +// CHECK-NEXT: unreachable +// CHECK-NEXT: } + +// CHECK: define i32 @_Z4foo2i(i32 %a) [[ATTR2]] +// CHECK-NEXT: entry: +// CHECK-NEXT: call void asm sideeffect "retq +// CHECK-NEXT: unreachable +// CHECK-NEXT: } + +// CHECK: attributes [[ATTR1]] = { {{.*}}naked{{.*}} } +// CHECK: attributes [[ATTR2]] = { {{.*}}naked{{.*}} } Index: test/CodeGen/attr-naked.c === --- test/CodeGen/attr-naked.c +++ test/CodeGen/attr-naked.c @@ -17,7 +17,7 @@ // Make sure not to generate prolog or epilog for naked functions. __attribute((naked)) void t3(int x) { -// CHECK: define void @t3(i32) +// CHECK: define void @t3(i32 %x) // CHECK-NOT: alloca // CHECK-NOT: store // CHECK: unreachable Index: lib/CodeGen/CodeGenFunction.h === --- lib/CodeGen/CodeGenFunction.h +++ lib/CodeGen/CodeGenFunction.h @@ -3096,6 +3096,13 @@ llvm::Value *emitBuiltinObjectSize(const Expr *E, unsigned Type, llvm::IntegerType *ResType); + /// Return true if CurCodeDecl has attribute Naked. + bool isNakedFunction() const; + + /// Clean up naked functions removing allocas and their users and all blocks + /// except the entry. + void cleanupNakedFunction(); + public: #ifndef NDEBUG // Determine whether the given argument is an Objective-C method Index: lib/CodeGen/CodeGenFunction.cpp === --- lib/CodeGen/CodeGenFunction.cpp +++ lib/CodeGen/CodeGenFunction.cpp @@ -355,6 +355,9 @@ CGBuilderTy(*this, AllocaInsertPt).CreateCall(FrameEscapeFn, EscapeArgs); } + if (isNakedFunction()) +cleanupNakedFunction(); + // Remove the AllocaInsertPt instruction, which is just a convenience for us. llvm::Instruction *Ptr = AllocaInsertPt; AllocaInsertPt = nullptr; @@ -1009,7 +1012,8 @@ // If the '}' that terminates a function is reached, and the value of the // function call is used by the caller, the behavior is undefined. if (getLangOpts().CPlusPlus && !FD->hasImplicitReturnZero() && !SawAsmBlock && - !FD->getReturnType()->isVoidType() && B
Re: [PATCH] D16259: Add clang-tidy readability-redundant-control-flow check
Eugene.Zelenko added a comment. Just encountered next situation in my work code base: void Function() { ... if (Condition) { } else return; } http://reviews.llvm.org/D16259 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r258727 - Move ObjCPropertyDecl to before ObjCContainerDecl.
Author: mren Date: Mon Jan 25 15:52:26 2016 New Revision: 258727 URL: http://llvm.org/viewvc/llvm-project?rev=258727&view=rev Log: Move ObjCPropertyDecl to before ObjCContainerDecl. After we add ObjCPropertyDecl::isClassProperty, we can use it in ObjCContainerDecl to define filter to iterate over instance properties and class properties. This is the first patch in a series of patches to support class properties in addition to instance properties in objective-c. rdar://23891898 Modified: cfe/trunk/include/clang/AST/DeclObjC.h Modified: cfe/trunk/include/clang/AST/DeclObjC.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=258727&r1=258726&r2=258727&view=diff == --- cfe/trunk/include/clang/AST/DeclObjC.h (original) +++ cfe/trunk/include/clang/AST/DeclObjC.h Mon Jan 25 15:52:26 2016 @@ -689,6 +689,197 @@ public: friend TrailingObjects; }; +/// \brief Represents one property declaration in an Objective-C interface. +/// +/// For example: +/// \code{.mm} +/// \@property (assign, readwrite) int MyProperty; +/// \endcode +class ObjCPropertyDecl : public NamedDecl { + void anchor() override; +public: + enum PropertyAttributeKind { +OBJC_PR_noattr= 0x00, +OBJC_PR_readonly = 0x01, +OBJC_PR_getter= 0x02, +OBJC_PR_assign= 0x04, +OBJC_PR_readwrite = 0x08, +OBJC_PR_retain= 0x10, +OBJC_PR_copy = 0x20, +OBJC_PR_nonatomic = 0x40, +OBJC_PR_setter= 0x80, +OBJC_PR_atomic= 0x100, +OBJC_PR_weak = 0x200, +OBJC_PR_strong= 0x400, +OBJC_PR_unsafe_unretained = 0x800, +/// Indicates that the nullability of the type was spelled with a +/// property attribute rather than a type qualifier. +OBJC_PR_nullability = 0x1000, +OBJC_PR_null_resettable = 0x2000, +// Adding a property should change NumPropertyAttrsBits + }; + + enum { +/// \brief Number of bits fitting all the property attributes. +NumPropertyAttrsBits = 14 + }; + + enum SetterKind { Assign, Retain, Copy, Weak }; + enum PropertyControl { None, Required, Optional }; +private: + SourceLocation AtLoc; // location of \@property + SourceLocation LParenLoc; // location of '(' starting attribute list or null. + QualType DeclType; + TypeSourceInfo *DeclTypeSourceInfo; + unsigned PropertyAttributes : NumPropertyAttrsBits; + unsigned PropertyAttributesAsWritten : NumPropertyAttrsBits; + // \@required/\@optional + unsigned PropertyImplementation : 2; + + Selector GetterName;// getter name of NULL if no getter + Selector SetterName;// setter name of NULL if no setter + + ObjCMethodDecl *GetterMethodDecl; // Declaration of getter instance method + ObjCMethodDecl *SetterMethodDecl; // Declaration of setter instance method + ObjCIvarDecl *PropertyIvarDecl; // Synthesize ivar for this property + + ObjCPropertyDecl(DeclContext *DC, SourceLocation L, IdentifierInfo *Id, + SourceLocation AtLocation, SourceLocation LParenLocation, + QualType T, TypeSourceInfo *TSI, + PropertyControl propControl) +: NamedDecl(ObjCProperty, DC, L, Id), AtLoc(AtLocation), + LParenLoc(LParenLocation), DeclType(T), DeclTypeSourceInfo(TSI), + PropertyAttributes(OBJC_PR_noattr), + PropertyAttributesAsWritten(OBJC_PR_noattr), + PropertyImplementation(propControl), + GetterName(Selector()), + SetterName(Selector()), + GetterMethodDecl(nullptr), SetterMethodDecl(nullptr), + PropertyIvarDecl(nullptr) {} + +public: + static ObjCPropertyDecl *Create(ASTContext &C, DeclContext *DC, + SourceLocation L, + IdentifierInfo *Id, SourceLocation AtLocation, + SourceLocation LParenLocation, + QualType T, + TypeSourceInfo *TSI, + PropertyControl propControl = None); + + static ObjCPropertyDecl *CreateDeserialized(ASTContext &C, unsigned ID); + + SourceLocation getAtLoc() const { return AtLoc; } + void setAtLoc(SourceLocation L) { AtLoc = L; } + + SourceLocation getLParenLoc() const { return LParenLoc; } + void setLParenLoc(SourceLocation L) { LParenLoc = L; } + + TypeSourceInfo *getTypeSourceInfo() const { return DeclTypeSourceInfo; } + + QualType getType() const { return DeclType; } + + void setType(QualType T, TypeSourceInfo *TSI) { +DeclType = T; +DeclTypeSourceInfo = TSI; + } + + /// Retrieve the type when this property is used with a specific base object + /// type. + QualType getUsageType(QualType objectType) const; + + PropertyAttributeKind getPropertyAttributes() const { +return PropertyAttributeKind(PropertyAttributes); + } + void setPropertyAttributes(PropertyAttributeKind PRVal) { +PropertyAttributes |= PRVal; + } + void overwr
Re: [PATCH] D15699: [cfi] Cross-DSO CFI diagnostic mode (clang part)
pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Comment at: test/CodeGen/cfi-check-fail.c:17 @@ +16,3 @@ + +// CHECK: [[TRAP]]: +// CHECK-NEXT: call void @llvm.trap() Does this match in a -asserts build? Repository: rL LLVM http://reviews.llvm.org/D15699 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15599: [CodeGen] Fix a crash that occurs when attribute "naked" is attached to a c++ member function
manmanren added inline comments. Comment at: lib/CodeGen/CodeGenFunction.cpp:1998 @@ +1997,3 @@ + if (auto *T = EntryBB->getTerminator()) +T->eraseFromParent(); + Yes, you are right. http://reviews.llvm.org/D15599 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits