[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros
klimek added a comment. In https://reviews.llvm.org/D28462#1147019, @enyquist wrote: > @klimek fair point. To be honest, I've pretty much lost interest / momentum > on this feature, I very much doubt I will ever go back and re-implement from > scratch as you suggest. > Not meaning to sound rude, I just don't want to waste anyone's time who is > waiting for this (seems there are a few). I do agree this is frustrating - I'd personally really like this feature, but having an implementation that fundamentally matches the architecture of the code is important for long-term maintenance and sustainability of the project. This is unfortunately a hard / lots-of-work feature to get right. I do appreciate the time you took to look into this. Repository: rL LLVM https://reviews.llvm.org/D28462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48159: [clangd] Implement hover for "auto" and "decltype"
klimek added a reviewer: rsmith. klimek added inline comments. Comment at: clangd/XRefs.cpp:559 + //- auto& i = 1; + bool VisitDeclaratorDecl(DeclaratorDecl *D) { +if (!D->getTypeSourceInfo() || malaperle wrote: > klimek wrote: > > sammccall wrote: > > > malaperle wrote: > > > > sammccall wrote: > > > > > out of curiosity, why not implement `VisitTypeLoc` and handle all the > > > > > cases where it turns out to be `auto` etc? > > > > > Even for `auto&` I'd expect the inner `auto` to have a `TypeLoc` you > > > > > could visit, saving the trouble of unwrapping. > > > > > > > > > > (I'm probably wrong about all this, I don't know the AST well. But > > > > > I'd like to learn!) > > > > From what I saw, there are actually two different AutoType* for each > > > > textual "auto". The AutoType* containing the deduced type does not get > > > > visited via a typeloc. It's not entirely clear to me why since I don't > > > > know the AST well either. I was thinking maybe the first is created > > > > when the type is not deduced yet and later on, then the rest of the > > > > function or expression is parsed, a second one with the actual type > > > > deduced is created. If I look at the code paths where they are created, > > > > it seems like this is roughly what's happening. The first one is > > > > created when the declarator is parsed (no deduced type yet) and the > > > > second is created when the expression of the initializer (or return > > > > statement) is evaluated and the type is then deduced. The visitor only > > > > visits the first one's typeloc. I don't think I'm knowledgeable enough > > > > to say whether or not that's a bug but it seems on purpose that it is > > > > modelled this way. Although it would be much nicer to only have to > > > > visit typelocs... > > > > The AutoType* containing the deduced type does not get visited via a > > > > typeloc > > > Ah, OK. > > > Could you add a high level comment (maybe on the class) saying this is > > > the reason for the implementation? Otherwise as a reader I'll think "this > > > seems unneccesarily complicated" but not understand why. > > > > > > @klimek Can you shed any light on this? > > Can't you go from AutoTypeLoc -> AutoType -> getDeducedType()? > The visitor doesn't visit the AutoTypeLoc that has the deduced type. In fact, > there are two AutoType* instances. I'm not sure that's is a bug that there > are two AutoType*, or if not visiting both AutoTypeLoc is a bug...or neither. +Richard Smith: This is weird. If I just create a minimal example: int f() { auto i = f(); return i; } I only get the undeduced auto type - Richard, in which cases are auto-typed being deduced? The AST dump doens't give an indication that there was an auto involved at all. Is this the famous syntactic vs. smenatic form problem? Do we have a backlink between the AutoTypeLoc and the deduced type somewhere? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48159 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well
JonasToth updated this revision to Diff 153457. JonasToth added a comment. This revision is now accepted and ready to land. - fix decltype deduction with new matcher I had to introduce a new matcher in clang for `DecltypeType` to reach its underlying type. It would be better to have `hasType` resolve all these issues but i dont know how much work that would be. We can discuss that in the patch for the matcher i guess. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48717 Files: clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic-pr36489.cpp Index: test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic-pr36489.cpp === --- /dev/null +++ test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic-pr36489.cpp @@ -0,0 +1,53 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-pointer-arithmetic %t -- -- -std=c++14 + +// Fix PR36489 and detect auto-deduced value correctly. +char *getPtr(); +auto getPtrAuto() { return getPtr(); } +decltype(getPtr()) getPtrDeclType(); +decltype(auto) getPtrDeclTypeAuto() { return getPtr(); } +auto getPtrWithTrailingReturnType() -> char *; + +void auto_deduction_binary() { + auto p1 = getPtr() + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: do not use pointer arithmetic + auto p2 = getPtrAuto() + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: do not use pointer arithmetic + auto p3 = getPtrWithTrailingReturnType() + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:44: warning: do not use pointer arithmetic + auto p4 = getPtr(); + auto *p5 = getPtr(); + p4 = p4 + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use pointer arithmetic + p5 = p5 + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use pointer arithmetic + auto p6 = getPtrDeclType() + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: do not use pointer arithmetic + auto p7 = getPtrDeclTypeAuto() + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: do not use pointer arithmetic + auto *p8 = getPtrDeclType() + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: do not use pointer arithmetic + auto *p9 = getPtrDeclTypeAuto() + 1; + // CHECK-MESSAGES: :[[@LINE-1]]:35: warning: do not use pointer arithmetic +} + +void auto_deduction_subscript() { + char p1 = getPtr()[2]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use pointer arithmetic + auto p2 = getPtr()[3]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use pointer arithmetic + + char p3 = getPtrAuto()[4]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use pointer arithmetic + auto p4 = getPtrAuto()[5]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use pointer arithmetic + + char p5 = getPtrWithTrailingReturnType()[6]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use pointer arithmetic + auto p6 = getPtrWithTrailingReturnType()[7]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use pointer arithmetic + + auto p7 = getPtrDeclType()[8]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use pointer arithmetic + auto p8 = getPtrDeclTypeAuto()[9]; + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use pointer arithmetic +} Index: clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp === --- clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp +++ clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp @@ -21,12 +21,16 @@ if (!getLangOpts().CPlusPlus) return; + const auto AllPointerTypes = anyOf( + hasType(pointerType()), hasType(autoType(hasDeducedType(pointerType(, + hasType(decltypeType(hasUnderlyingType(pointerType(); + // Flag all operators +, -, +=, -=, ++, -- that result in a pointer Finder->addMatcher( binaryOperator( anyOf(hasOperatorName("+"), hasOperatorName("-"), hasOperatorName("+="), hasOperatorName("-=")), - hasType(pointerType()), + AllPointerTypes, unless(hasLHS(ignoringImpCasts(declRefExpr(to(isImplicit())) .bind("expr"), this); @@ -41,7 +45,7 @@ Finder->addMatcher( arraySubscriptExpr( hasBase(ignoringImpCasts( - anyOf(hasType(pointerType()), + anyOf(AllPointerTypes, hasType(decayedType(hasDecayedType(pointerType( .bind("expr"), this); Index: test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic-pr36489.cpp === --- /dev/null +++ test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic-pr36489.cpp @@ -0,0 +1,53 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-pointer-arithmetic %t -- -- -std=c++14 + +// Fix PR36489 and detect auto-deduced value correctly. +char *getPtr(); +au
[PATCH] D48759: [ASTMatchers] add matcher for decltypeType and its underlyingType
JonasToth created this revision. JonasToth added reviewers: aaron.ballman, alexfh, NoQ, dcoughlin. Herald added a subscriber: cfe-commits. This patch introduces a new matcher for `DecltypeType` and its underlying type in order to fix a bug in clang-tidy, see https://reviews.llvm.org/D48717 for more. Repository: rC Clang https://reviews.llvm.org/D48759 Files: include/clang/ASTMatchers/ASTMatchers.h lib/ASTMatchers/ASTMatchersInternal.cpp lib/ASTMatchers/Dynamic/Registry.cpp Index: lib/ASTMatchers/Dynamic/Registry.cpp === --- lib/ASTMatchers/Dynamic/Registry.cpp +++ lib/ASTMatchers/Dynamic/Registry.cpp @@ -134,6 +134,7 @@ REGISTER_MATCHER(atomicExpr); REGISTER_MATCHER(atomicType); REGISTER_MATCHER(autoType); + REGISTER_MATCHER(decltypeType); REGISTER_MATCHER(binaryOperator); REGISTER_MATCHER(binaryConditionalOperator); REGISTER_MATCHER(blockDecl); Index: lib/ASTMatchers/ASTMatchersInternal.cpp === --- lib/ASTMatchers/ASTMatchersInternal.cpp +++ lib/ASTMatchers/ASTMatchersInternal.cpp @@ -798,6 +798,7 @@ const AstTypeMatcher variableArrayType; const AstTypeMatcher atomicType; const AstTypeMatcher autoType; +const AstTypeMatcher decltypeType; const AstTypeMatcher functionType; const AstTypeMatcher functionProtoType; const AstTypeMatcher parenType; Index: include/clang/ASTMatchers/ASTMatchers.h === --- include/clang/ASTMatchers/ASTMatchers.h +++ include/clang/ASTMatchers/ASTMatchers.h @@ -5063,6 +5063,18 @@ /// matches "auto n" and "auto i" extern const AstTypeMatcher autoType; +/// Matches types nodes representing C++11 decltype() types. +/// +/// Given: +/// \code +/// short i = 1; +/// int j = 42; +/// decltype(i + j) result = i + j; +/// \endcode +/// decltypeType() +/// matches "decltype(i + j)" +extern const AstTypeMatcher decltypeType; + /// Matches \c AutoType nodes where the deduced type is a specific type. /// /// Note: There is no \c TypeLoc for the deduced type and thus no @@ -5080,6 +5092,10 @@ AST_TYPE_TRAVERSE_MATCHER(hasDeducedType, getDeducedType, AST_POLYMORPHIC_SUPPORTED_TYPES(AutoType)); +/// Matches \c DecltypeType nodes to find out the underlying type. +AST_TYPE_TRAVERSE_MATCHER(hasUnderlyingType, getUnderlyingType, + AST_POLYMORPHIC_SUPPORTED_TYPES(DecltypeType)); + /// Matches \c FunctionType nodes. /// /// Given Index: lib/ASTMatchers/Dynamic/Registry.cpp === --- lib/ASTMatchers/Dynamic/Registry.cpp +++ lib/ASTMatchers/Dynamic/Registry.cpp @@ -134,6 +134,7 @@ REGISTER_MATCHER(atomicExpr); REGISTER_MATCHER(atomicType); REGISTER_MATCHER(autoType); + REGISTER_MATCHER(decltypeType); REGISTER_MATCHER(binaryOperator); REGISTER_MATCHER(binaryConditionalOperator); REGISTER_MATCHER(blockDecl); Index: lib/ASTMatchers/ASTMatchersInternal.cpp === --- lib/ASTMatchers/ASTMatchersInternal.cpp +++ lib/ASTMatchers/ASTMatchersInternal.cpp @@ -798,6 +798,7 @@ const AstTypeMatcher variableArrayType; const AstTypeMatcher atomicType; const AstTypeMatcher autoType; +const AstTypeMatcher decltypeType; const AstTypeMatcher functionType; const AstTypeMatcher functionProtoType; const AstTypeMatcher parenType; Index: include/clang/ASTMatchers/ASTMatchers.h === --- include/clang/ASTMatchers/ASTMatchers.h +++ include/clang/ASTMatchers/ASTMatchers.h @@ -5063,6 +5063,18 @@ /// matches "auto n" and "auto i" extern const AstTypeMatcher autoType; +/// Matches types nodes representing C++11 decltype() types. +/// +/// Given: +/// \code +/// short i = 1; +/// int j = 42; +/// decltype(i + j) result = i + j; +/// \endcode +/// decltypeType() +/// matches "decltype(i + j)" +extern const AstTypeMatcher decltypeType; + /// Matches \c AutoType nodes where the deduced type is a specific type. /// /// Note: There is no \c TypeLoc for the deduced type and thus no @@ -5080,6 +5092,10 @@ AST_TYPE_TRAVERSE_MATCHER(hasDeducedType, getDeducedType, AST_POLYMORPHIC_SUPPORTED_TYPES(AutoType)); +/// Matches \c DecltypeType nodes to find out the underlying type. +AST_TYPE_TRAVERSE_MATCHER(hasUnderlyingType, getUnderlyingType, + AST_POLYMORPHIC_SUPPORTED_TYPES(DecltypeType)); + /// Matches \c FunctionType nodes. /// /// Given ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48760: [clang-format] Support additional common functions for text proto formatting
krasimir created this revision. Herald added a subscriber: cfe-commits. This adds a few more common function names expecting a text proto argument. Repository: rC Clang https://reviews.llvm.org/D48760 Files: lib/Format/Format.cpp Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -775,8 +775,11 @@ }, /*EnclosingFunctionNames=*/ { - "PARSE_TEXT_PROTO", "EqualsProto", + "EquivToProto", + "PARSE_TEST_PROTO", + "PARSE_TEXT_PROTO", + "ParseTextOrDie", }, /*CanonicalDelimiter=*/"", /*BasedOnStyle=*/"google", Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -775,8 +775,11 @@ }, /*EnclosingFunctionNames=*/ { - "PARSE_TEXT_PROTO", "EqualsProto", + "EquivToProto", + "PARSE_TEST_PROTO", + "PARSE_TEXT_PROTO", + "ParseTextOrDie", }, /*CanonicalDelimiter=*/"", /*BasedOnStyle=*/"google", ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r335959 - [ASTImporter] Eliminated some unittest warnings.
Author: martong Date: Fri Jun 29 03:25:19 2018 New Revision: 335959 URL: http://llvm.org/viewvc/llvm-project?rev=335959&view=rev Log: [ASTImporter] Eliminated some unittest warnings. Summary: When running the ASTTests test, warnings produced by the compiler can be distracting when looking for test errors. A part of the warnings is removed by setting extra compiler options. Reviewers: a.sidorin Reviewed By: a.sidorin Subscribers: a_sidorin, martong, cfe-commits Differential Revision: https://reviews.llvm.org/D47459 Patch by Balazs Keri! Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=335959&r1=335958&r2=335959&view=diff == --- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original) +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Fri Jun 29 03:25:19 2018 @@ -472,138 +472,110 @@ TEST_P(CanonicalRedeclChain, ShouldBeSam TEST_P(ImportExpr, ImportStringLiteral) { MatchVerifier Verifier; - testImport("void declToImport() { \"foo\"; }", - Lang_CXX, "", Lang_CXX, Verifier, - functionDecl( - hasBody( - compoundStmt( - has( - stringLiteral( - hasType( - asString("const char [4]"; - testImport("void declToImport() { L\"foo\"; }", - Lang_CXX, "", Lang_CXX, Verifier, - functionDecl( - hasBody( - compoundStmt( - has( - stringLiteral( - hasType( -asString("const wchar_t [4]"; - testImport("void declToImport() { \"foo\" \"bar\"; }", - Lang_CXX, "", Lang_CXX, Verifier, - functionDecl( - hasBody( - compoundStmt( - has( - stringLiteral( - hasType( - asString("const char [7]"; + testImport( + "void declToImport() { (void)\"foo\"; }", + Lang_CXX, "", Lang_CXX, Verifier, + functionDecl(hasDescendant( + stringLiteral(hasType(asString("const char [4]")); + testImport( + "void declToImport() { (void)L\"foo\"; }", + Lang_CXX, "", Lang_CXX, Verifier, + functionDecl(hasDescendant( + stringLiteral(hasType(asString("const wchar_t [4]")); + testImport( + "void declToImport() { (void) \"foo\" \"bar\"; }", + Lang_CXX, "", Lang_CXX, Verifier, + functionDecl(hasDescendant( + stringLiteral(hasType(asString("const char [7]")); } TEST_P(ImportExpr, ImportGNUNullExpr) { MatchVerifier Verifier; - testImport("void declToImport() { __null; }", - Lang_CXX, "", Lang_CXX, Verifier, - functionDecl( - hasBody( - compoundStmt( - has( - gnuNullExpr( - hasType(isInteger(; + testImport( + "void declToImport() { (void)__null; }", + Lang_CXX, "", Lang_CXX, Verifier, + functionDecl(hasDescendant(gnuNullExpr(hasType(isInteger()); } TEST_P(ImportExpr, ImportCXXNullPtrLiteralExpr) { MatchVerifier Verifier; - testImport("void declToImport() { nullptr; }", - Lang_CXX11, "", Lang_CXX11, Verifier, - functionDecl( - hasBody( - compoundStmt( - has( - cxxNullPtrLiteralExpr()); + testImport( + "void declToImport() { (void)nullptr; }", + Lang_CXX11, "", Lang_CXX11, Verifier, + functionDecl(hasDescendant(cxxNullPtrLiteralExpr(; } TEST_P(ImportExpr, ImportFloatinglLiteralExpr) { MatchVerifier Verifier; - testImport("void declToImport() { 1.0; }", - Lang_C, "", Lang_C, Verifier, - functionDecl( - hasBody( - compoundStmt( - has( - floatLiteral( - equals(1.0), - hasType(asString("double"; - testImport("void declToImport() { 1.0e-5f; }", - Lang_C, "", Lang_C, Verifier, - functionDecl( -hasBody( - compoundStmt( -has( - floatLiteral( -equals(1.0e-5f), -hasType(asString("float"; + testImport( + "void declToImport() { (void)1.0; }", + Lang_C, "", Lang_C, Verifier, + functionDecl(hasDescendant( + floatLiteral(equals(1.0), hasType(asString("double")); + testImport( + "void declToImport() { (void)1.0e-5f; }", + Lang_C, "", Lang_C, Verifier, + functionDecl(hasDescendant( + floatLiteral(equals(1.0e-5f),
[PATCH] D47459: [ASTImporter] Eliminated some unittest warnings.
This revision was automatically updated to reflect the committed changes. Closed by commit rL335959: [ASTImporter] Eliminated some unittest warnings. (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D47459 Files: cfe/trunk/unittests/AST/ASTImporterTest.cpp Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp === --- cfe/trunk/unittests/AST/ASTImporterTest.cpp +++ cfe/trunk/unittests/AST/ASTImporterTest.cpp @@ -472,138 +472,110 @@ TEST_P(ImportExpr, ImportStringLiteral) { MatchVerifier Verifier; - testImport("void declToImport() { \"foo\"; }", - Lang_CXX, "", Lang_CXX, Verifier, - functionDecl( - hasBody( - compoundStmt( - has( - stringLiteral( - hasType( - asString("const char [4]"; - testImport("void declToImport() { L\"foo\"; }", - Lang_CXX, "", Lang_CXX, Verifier, - functionDecl( - hasBody( - compoundStmt( - has( - stringLiteral( - hasType( -asString("const wchar_t [4]"; - testImport("void declToImport() { \"foo\" \"bar\"; }", - Lang_CXX, "", Lang_CXX, Verifier, - functionDecl( - hasBody( - compoundStmt( - has( - stringLiteral( - hasType( - asString("const char [7]"; + testImport( + "void declToImport() { (void)\"foo\"; }", + Lang_CXX, "", Lang_CXX, Verifier, + functionDecl(hasDescendant( + stringLiteral(hasType(asString("const char [4]")); + testImport( + "void declToImport() { (void)L\"foo\"; }", + Lang_CXX, "", Lang_CXX, Verifier, + functionDecl(hasDescendant( + stringLiteral(hasType(asString("const wchar_t [4]")); + testImport( + "void declToImport() { (void) \"foo\" \"bar\"; }", + Lang_CXX, "", Lang_CXX, Verifier, + functionDecl(hasDescendant( + stringLiteral(hasType(asString("const char [7]")); } TEST_P(ImportExpr, ImportGNUNullExpr) { MatchVerifier Verifier; - testImport("void declToImport() { __null; }", - Lang_CXX, "", Lang_CXX, Verifier, - functionDecl( - hasBody( - compoundStmt( - has( - gnuNullExpr( - hasType(isInteger(; + testImport( + "void declToImport() { (void)__null; }", + Lang_CXX, "", Lang_CXX, Verifier, + functionDecl(hasDescendant(gnuNullExpr(hasType(isInteger()); } TEST_P(ImportExpr, ImportCXXNullPtrLiteralExpr) { MatchVerifier Verifier; - testImport("void declToImport() { nullptr; }", - Lang_CXX11, "", Lang_CXX11, Verifier, - functionDecl( - hasBody( - compoundStmt( - has( - cxxNullPtrLiteralExpr()); + testImport( + "void declToImport() { (void)nullptr; }", + Lang_CXX11, "", Lang_CXX11, Verifier, + functionDecl(hasDescendant(cxxNullPtrLiteralExpr(; } TEST_P(ImportExpr, ImportFloatinglLiteralExpr) { MatchVerifier Verifier; - testImport("void declToImport() { 1.0; }", - Lang_C, "", Lang_C, Verifier, - functionDecl( - hasBody( - compoundStmt( - has( - floatLiteral( - equals(1.0), - hasType(asString("double"; - testImport("void declToImport() { 1.0e-5f; }", - Lang_C, "", Lang_C, Verifier, - functionDecl( -hasBody( - compoundStmt( -has( - floatLiteral( -equals(1.0e-5f), -hasType(asString("float"; + testImport( + "void declToImport() { (void)1.0; }", + Lang_C, "", Lang_C, Verifier, + functionDecl(hasDescendant( + floatLiteral(equals(1.0), hasType(asString("double")); + testImport( + "void declToImport() { (void)1.0e-5f; }", + Lang_C, "", Lang_C, Verifier, + functionDecl(hasDescendant( + floatLiteral(equals(1.0e-5f), hasType(asString("float")); } TEST_P(ImportExpr, ImportCompoundLiteralExpr) { MatchVerifier Verifier; - testImport("void declToImport() {" - " struct s { int x; long y; unsigned z; }; " - " (struct s){ 42, 0L, 1U }; }", - Lang_CXX, "", Lang_CXX, Verifier, - functionDecl( - hasBody( - compoundStmt( - has( - compoundLiteralExpr( - ha
[PATCH] D48762: [clangd] codeComplete returns more structured completion items, LSP. NFC.
sammccall created this revision. sammccall added a reviewer: ioeric. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. LSP has some presentational fields with limited semantics (e.g. 'detail') and doesn't provide a good place to return information like namespace. Some places where more detailed information is useful: - tools like quality analysis - alternate frontends that aren't strictly LSP - code completion unit tests In this patch, ClangdServer::codeComplete still return LSP CompletionList, but I plan to switch that soon (should be a no-op for ClangdLSPServer). Deferring this makes it clear that we don't change behavior (tests stay the same) and also keeps the API break to a small patch which can be reverted. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48762 Files: clangd/ClangdServer.cpp clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/Protocol.h Index: clangd/Protocol.h === --- clangd/Protocol.h +++ clangd/Protocol.h @@ -668,20 +668,6 @@ Snippet = 2, }; -/// Provides details for how a completion item was scored. -/// This can be used for client-side filtering of completion items as the -/// user keeps typing. -/// This is a clangd extension. -struct CompletionItemScores { - /// The score that items are ranked by. - /// This is filterScore * symbolScore. - float finalScore = 0.f; - /// How the partial identifier matched filterText. [0-1] - float filterScore = 0.f; - /// How the symbol fits, ignoring the partial identifier. - float symbolScore = 0.f; -}; - struct CompletionItem { /// The label of this completion item. By default also the text that is /// inserted when selecting this completion. @@ -702,9 +688,6 @@ /// When `falsy` the label is used. std::string sortText; - /// Details about the quality of this completion item. (clangd extension) - llvm::Optional scoreInfo; - /// A string that should be used when filtering a set of completion items. /// When `falsy` the label is used. std::string filterText; Index: clangd/CodeComplete.h === --- clangd/CodeComplete.h +++ clangd/CodeComplete.h @@ -75,15 +75,78 @@ const SymbolIndex *Index = nullptr; }; +// Semi-structured representation of a code-complete suggestion for our C++ API. +// We don't use the LSP structures here (unlike most features) as we want +// to expose more data to allow for more precise testing and evaluation. +struct CodeCompletion { + // The unqualified name of the symbol or other completion item. + std::string Name; + // The scope qualifier for the symbol name. e.g. "ns1::ns2::" + // Empty for non-symbol completions. Not inserted, but may be displayed. + std::string Scope; + // Qualifiers that must be inserted before the name (e.g. base::). + std::string Qualifiers; + // Details to be displayed following the name. Not inserted. + std::string Signature; + // Text to be inserted following the name, in snippet format. + std::string SnippetSuffix; + // Type to be displayed for this completion. + std::string ReturnType; + std::string Documentation; + CompletionItemKind Kind = CompletionItemKind::Missing; + // This completion item may represent several symbols that can be inserted in + // the same way, such as function overloads. In this case BundleSize > 1, and + // the following fields are summaries: + // - Signature is e.g. "(...)" for functions. + // - SnippetSuffix is similarly e.g. "(${0})". + // - ReturnType may be empty + // - Documentation may contain a combined docstring from all comments. + // Other fields should apply equally to all bundled completions. + unsigned BundleSize; + // The header through which this symbol should be included. + // Empty for non-symbol completions. + std::string Header; + // Present if Header is set and should be inserted to use this item. + llvm::Optional HeaderInsertion; + + // Scores are used to rank completion items. + struct Scores { +// The score that items are ranked by. +float Total = 0.f; + +// The finalScore with the fuzzy name match score excluded. +// When filtering client-side, editors should calculate the new fuzzy score, +// whose scale is 0-1 (with 1 = prefix match, special case 2 = exact match), +// and recompute finalScore = fuzzyScore * symbolScore. +float ExcludingName = 0.f; + +// Component scores that contributed to the final score: + +// Quality describes how important we think this candidate is, +// independent of the query. +// e.g. symbols with lots of incoming references have higher quality. +float Quality = 0.f; +// Relevance describes how well this candidate matched the query. +// e.g. symbols from nearby files have higher relevance. +float Relevance = 0.f; + }; + Scores Score; + + // Serialize this to an LSP completion item. This is a lossy operation. +
[PATCH] D48762: [clangd] codeComplete returns more structured completion items, LSP. NFC.
sammccall added a comment. @jkorous FYI: this may be interesting from an XPC perspective - it'd be easy to serialize the LSP-style struct, or the more detailed one if that's useful. LSP will be more stable of course :-) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48764: [Analyzer] Hotfix for iterator checkers: Mark left-hand side of `SymIntExpr` objects as live in the program state maps.
baloghadamsoftware created this revision. baloghadamsoftware added reviewers: NoQ, mikhail.ramalho. Herald added subscribers: a.sidorin, dkrupp, rnkovacs, szepet, xazax.hun, whisperity. Herald added a reviewer: george.karpenkov. Marking a symbolic expression as live is not recursive. In our checkers we either use conjured symbols or conjured symbols plus/minus concrete integers to represent abstract positions of iterators, so we must mark the left-hand side of these expressions as live to prevent them from getting reaped. https://reviews.llvm.org/D48764 Files: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp === --- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp +++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp @@ -489,12 +489,16 @@ auto RegionMap = State->get(); for (const auto Reg : RegionMap) { const auto Pos = Reg.second; +if (const auto *SIE = dyn_cast(Pos.getOffset())) + SR.markLive(SIE->getLHS()); SR.markLive(Pos.getOffset()); } auto SymbolMap = State->get(); for (const auto Sym : SymbolMap) { const auto Pos = Sym.second; +if (const auto *SIE = dyn_cast(Pos.getOffset())) + SR.markLive(SIE->getLHS()); SR.markLive(Pos.getOffset()); } Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp === --- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp +++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp @@ -489,12 +489,16 @@ auto RegionMap = State->get(); for (const auto Reg : RegionMap) { const auto Pos = Reg.second; +if (const auto *SIE = dyn_cast(Pos.getOffset())) + SR.markLive(SIE->getLHS()); SR.markLive(Pos.getOffset()); } auto SymbolMap = State->get(); for (const auto Sym : SymbolMap) { const auto Pos = Sym.second; +if (const auto *SIE = dyn_cast(Pos.getOffset())) + SR.markLive(SIE->getLHS()); SR.markLive(Pos.getOffset()); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48712: [X86] Lowering integer truncation intrinsics to native IR
mike.dvoretsky updated this revision to Diff 153471. mike.dvoretsky marked 2 inline comments as done. mike.dvoretsky added a comment. Updated per comments. Typedefs for intermediate short vectors moved into the bodies of the functions using them. https://reviews.llvm.org/D48712 Files: clang/lib/Headers/avx512vlbwintrin.h clang/lib/Headers/avx512vlintrin.h clang/test/CodeGen/avx512vl-builtins.c clang/test/CodeGen/avx512vlbw-builtins.c Index: clang/test/CodeGen/avx512vlbw-builtins.c === --- clang/test/CodeGen/avx512vlbw-builtins.c +++ clang/test/CodeGen/avx512vlbw-builtins.c @@ -1792,7 +1792,8 @@ __m128i test_mm_cvtepi16_epi8(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi16_epi8 - // CHECK: @llvm.x86.avx512.mask.pmov.wb.128 + // CHECK: trunc <8 x i16> %{{.*}} to <8 x i8> + // CHECK: shufflevector <8 x i8> %{{.*}}, <8 x i8> %{{.*}}, <16 x i32> return _mm_cvtepi16_epi8(__A); } Index: clang/test/CodeGen/avx512vl-builtins.c === --- clang/test/CodeGen/avx512vl-builtins.c +++ clang/test/CodeGen/avx512vl-builtins.c @@ -6974,7 +6974,8 @@ __m128i test_mm_cvtepi32_epi8(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi32_epi8 - // CHECK: @llvm.x86.avx512.mask.pmov.db.128 + // CHECK: trunc <4 x i32> %{{.*}} to <4 x i8> + // CHECK: shufflevector <4 x i8> %{{.*}}, <4 x i8> %{{.*}}, <16 x i32> return _mm_cvtepi32_epi8(__A); } @@ -6998,7 +6999,8 @@ __m128i test_mm256_cvtepi32_epi8(__m256i __A) { // CHECK-LABEL: @test_mm256_cvtepi32_epi8 - // CHECK: @llvm.x86.avx512.mask.pmov.db.256 + // CHECK: trunc <8 x i32> %{{.*}} to <8 x i8> + // CHECK: shufflevector <8 x i8> %{{.*}}, <8 x i8> %{{.*}}, <16 x i32> return _mm256_cvtepi32_epi8(__A); } @@ -7022,7 +7024,8 @@ __m128i test_mm_cvtepi32_epi16(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi32_epi16 - // CHECK: @llvm.x86.avx512.mask.pmov.dw.128 + // CHECK: trunc <4 x i32> %{{.*}} to <4 x i16> + // CHECK: shufflevector <4 x i16> %{{.*}}, <4 x i16> %{{.*}}, <8 x i32> return _mm_cvtepi32_epi16(__A); } @@ -7070,7 +7073,8 @@ __m128i test_mm_cvtepi64_epi8(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi64_epi8 - // CHECK: @llvm.x86.avx512.mask.pmov.qb.128 + // CHECK: trunc <2 x i64> %{{.*}} to <2 x i8> + // CHECK: shufflevector <2 x i8> %{{.*}}, <2 x i8> %{{.*}}, <16 x i32> return _mm_cvtepi64_epi8(__A); } @@ -7094,7 +7098,8 @@ __m128i test_mm256_cvtepi64_epi8(__m256i __A) { // CHECK-LABEL: @test_mm256_cvtepi64_epi8 - // CHECK: @llvm.x86.avx512.mask.pmov.qb.256 + // CHECK: trunc <4 x i64> %{{.*}} to <4 x i8> + // CHECK: shufflevector <4 x i8> %{{.*}}, <4 x i8> %{{.*}}, <16 x i32> return _mm256_cvtepi64_epi8(__A); } @@ -7118,7 +7123,8 @@ __m128i test_mm_cvtepi64_epi32(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi64_epi32 - // CHECK: @llvm.x86.avx512.mask.pmov.qd.128 + // CHECK: trunc <2 x i64> %{{.*}} to <2 x i32> + // CHECK: shufflevector <2 x i32> %{{.*}}, <2 x i32> %{{.*}}, <4 x i32> return _mm_cvtepi64_epi32(__A); } @@ -7168,7 +7174,8 @@ __m128i test_mm_cvtepi64_epi16(__m128i __A) { // CHECK-LABEL: @test_mm_cvtepi64_epi16 - // CHECK: @llvm.x86.avx512.mask.pmov.qw.128 + // CHECK: trunc <2 x i64> %{{.*}} to <2 x i16> + // CHECK: shufflevector <2 x i16> %{{.*}}, <2 x i16> %{{.*}}, <8 x i32> return _mm_cvtepi64_epi16(__A); } @@ -7192,7 +7199,8 @@ __m128i test_mm256_cvtepi64_epi16(__m256i __A) { // CHECK-LABEL: @test_mm256_cvtepi64_epi16 - // CHECK: @llvm.x86.avx512.mask.pmov.qw.256 + // CHECK: trunc <4 x i64> %{{.*}} to <4 x i16> + // CHECK: shufflevector <4 x i16> %{{.*}}, <4 x i16> %{{.*}}, <8 x i32> return _mm256_cvtepi64_epi16(__A); } Index: clang/lib/Headers/avx512vlintrin.h === --- clang/lib/Headers/avx512vlintrin.h +++ clang/lib/Headers/avx512vlintrin.h @@ -7412,9 +7412,10 @@ static __inline__ __m128i __DEFAULT_FN_ATTRS _mm_cvtepi32_epi8 (__m128i __A) { - return (__m128i) __builtin_ia32_pmovdb128_mask ((__v4si) __A, - (__v16qi)_mm_undefined_si128(), - (__mmask8) -1); + typedef char __v4qi __attribute__((__vector_size__(4))); + return (__m128i)__builtin_shufflevector( + __builtin_convertvector((__v4si)__A, __v4qi), (__v4qi){0, 0, 0, 0}, 0, 1, + 2, 3, 4, 5, 6, 7, 4, 5, 6, 7, 4, 5, 6, 7); } static __inline__ __m128i __DEFAULT_FN_ATTRS @@ -7442,9 +7443,10 @@ static __inline__ __m128i __DEFAULT_FN_ATTRS _mm256_cvtepi32_epi8 (__m256i __A) { - return (__m128i) __builtin_ia32_pmovdb256_mask ((__v8si) __A, - (__v16qi)_mm_undefined_si128(), - (__mmask8) -1); + return (__m128i)__builtin_shufflevector( + __builtin_convertvector((__v8si)__A, __v8qi), + (__v8qi){0, 0, 0, 0, 0, 0, 0, 0}, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, + 12, 13, 14, 15); } static _
[PATCH] D48285: [analyzer][UninitializedObjectChecker] Added "NotesAsWarnings" flag
This revision was automatically updated to reflect the committed changes. Closed by commit rC335964: [analyzer][UninitializedObjectChecker] Added a NotesAsWarnings flag (authored by Szelethus, committed by ). Repository: rC Clang https://reviews.llvm.org/D48285 Files: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp test/Analysis/cxx-uninitialized-object-notes-as-warnings.cpp Index: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp === --- lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp +++ lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp @@ -10,10 +10,19 @@ // This file defines a checker that reports uninitialized fields in objects // created after a constructor call. // -// This checker has an option "Pedantic" (boolean). If its not set or is set to -// false, the checker won't emit warnings for objects that don't have at least -// one initialized field. This may be set with -// `-analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true`. +// This checker has two options: +// - "Pedantic" (boolean). If its not set or is set to false, the checker +// won't emit warnings for objects that don't have at least one initialized +// field. This may be set with +// +// `-analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true`. +// +// - "NotesAsWarnings" (boolean). If set to true, the checker will emit a +// warning for each uninitalized field, as opposed to emitting one warning +// per constructor call, and listing the uninitialized fields that belongs +// to it in notes. Defaults to false. +// +// `-analyzer-config alpha.cplusplus.UninitializedObject:NotesAsWarnings=true`. // //===--===// @@ -32,7 +41,9 @@ std::unique_ptr BT_uninitField; public: - bool IsPedantic; // Will be initialized when registering the checker. + // These fields will be initialized when registering the checker. + bool IsPedantic; + bool ShouldConvertNotesToWarnings; UninitializedObjectChecker() : BT_uninitField(new BuiltinBug(this, "Uninitialized fields")) {} @@ -216,6 +227,9 @@ return T->isBuiltinType() || T->isEnumeralType(); } +/// Constructs a note message for a given FieldChainInfo object. +void printNoteMessage(llvm::raw_ostream &Out, const FieldChainInfo &Chain); + } // end of anonymous namespace //===--===// @@ -264,6 +278,23 @@ LocUsedForUniqueing = PathDiagnosticLocation::createBegin( CallSite, Context.getSourceManager(), Node->getLocationContext()); + // For Plist consumers that don't support notes just yet, we'll convert notes + // to warnings. + if (ShouldConvertNotesToWarnings) { +for (const auto &Chain : UninitFields) { + SmallString<100> WarningBuf; + llvm::raw_svector_ostream WarningOS(WarningBuf); + + printNoteMessage(WarningOS, Chain); + + auto Report = llvm::make_unique( + *BT_uninitField, WarningOS.str(), Node, LocUsedForUniqueing, + Node->getLocationContext()->getDecl()); + Context.emitReport(std::move(Report)); +} +return; + } + SmallString<100> WarningBuf; llvm::raw_svector_ostream WarningOS(WarningBuf); WarningOS << UninitFields.size() << " uninitialized field" @@ -274,29 +305,16 @@ *BT_uninitField, WarningOS.str(), Node, LocUsedForUniqueing, Node->getLocationContext()->getDecl()); - // TODO: As of now, one warning is emitted per constructor call, and the - // uninitialized fields are listed in notes. Until there's a better support - // for notes avaible, a note-less version of this checker should be - // implemented. - for (const auto &FieldChain : UninitFields) { + for (const auto &Chain : UninitFields) { SmallString<200> NoteBuf; llvm::raw_svector_ostream NoteOS(NoteBuf); -if (FieldChain.isPointer()) { - if (FieldChain.isDereferenced()) -NoteOS << "uninitialized pointee 'this->"; - else -NoteOS << "uninitialized pointer 'this->"; -} else - NoteOS << "uninitialized field 'this->"; -FieldChain.print(NoteOS); -NoteOS << "'"; +printNoteMessage(NoteOS, Chain); Report->addNote(NoteOS.str(), -PathDiagnosticLocation::create(FieldChain.getEndOfChain(), +PathDiagnosticLocation::create(Chain.getEndOfChain(), Context.getSourceManager())); } - Context.emitReport(std::move(Report)); } @@ -662,10 +680,24 @@ return false; } +void printNoteMessage(llvm::raw_ostream &Out, const FieldChainInfo &Chain) { + if (Chain.isPointer()) { +if (Chain.isDereferenced()) + Out << "uninitialized pointee 'this->"; +else + Out << "uninitialized pointer 'this->"; + } else +Out << "uninitialized field 'this->"; + Cha
r335964 - [analyzer][UninitializedObjectChecker] Added a NotesAsWarnings flag
Author: szelethus Date: Fri Jun 29 04:25:24 2018 New Revision: 335964 URL: http://llvm.org/viewvc/llvm-project?rev=335964&view=rev Log: [analyzer][UninitializedObjectChecker] Added a NotesAsWarnings flag In order to better support consumers of the plist output that don't parse note entries just yet, a 'NotesAsWarnings' flag was added. If it's set to true, all notes will be converted to warnings. Differential Revision: https://reviews.llvm.org/D48285 Added: cfe/trunk/test/Analysis/cxx-uninitialized-object-notes-as-warnings.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp?rev=335964&r1=335963&r2=335964&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp Fri Jun 29 04:25:24 2018 @@ -10,10 +10,19 @@ // This file defines a checker that reports uninitialized fields in objects // created after a constructor call. // -// This checker has an option "Pedantic" (boolean). If its not set or is set to -// false, the checker won't emit warnings for objects that don't have at least -// one initialized field. This may be set with -// `-analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true`. +// This checker has two options: +// - "Pedantic" (boolean). If its not set or is set to false, the checker +// won't emit warnings for objects that don't have at least one initialized +// field. This may be set with +// +// `-analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true`. +// +// - "NotesAsWarnings" (boolean). If set to true, the checker will emit a +// warning for each uninitalized field, as opposed to emitting one warning +// per constructor call, and listing the uninitialized fields that belongs +// to it in notes. Defaults to false. +// +// `-analyzer-config alpha.cplusplus.UninitializedObject:NotesAsWarnings=true`. // //===--===// @@ -32,7 +41,9 @@ class UninitializedObjectChecker : publi std::unique_ptr BT_uninitField; public: - bool IsPedantic; // Will be initialized when registering the checker. + // These fields will be initialized when registering the checker. + bool IsPedantic; + bool ShouldConvertNotesToWarnings; UninitializedObjectChecker() : BT_uninitField(new BuiltinBug(this, "Uninitialized fields")) {} @@ -216,6 +227,9 @@ bool isPrimitiveType(const QualType &T) return T->isBuiltinType() || T->isEnumeralType(); } +/// Constructs a note message for a given FieldChainInfo object. +void printNoteMessage(llvm::raw_ostream &Out, const FieldChainInfo &Chain); + } // end of anonymous namespace //===--===// @@ -264,6 +278,23 @@ void UninitializedObjectChecker::checkEn LocUsedForUniqueing = PathDiagnosticLocation::createBegin( CallSite, Context.getSourceManager(), Node->getLocationContext()); + // For Plist consumers that don't support notes just yet, we'll convert notes + // to warnings. + if (ShouldConvertNotesToWarnings) { +for (const auto &Chain : UninitFields) { + SmallString<100> WarningBuf; + llvm::raw_svector_ostream WarningOS(WarningBuf); + + printNoteMessage(WarningOS, Chain); + + auto Report = llvm::make_unique( + *BT_uninitField, WarningOS.str(), Node, LocUsedForUniqueing, + Node->getLocationContext()->getDecl()); + Context.emitReport(std::move(Report)); +} +return; + } + SmallString<100> WarningBuf; llvm::raw_svector_ostream WarningOS(WarningBuf); WarningOS << UninitFields.size() << " uninitialized field" @@ -274,29 +305,16 @@ void UninitializedObjectChecker::checkEn *BT_uninitField, WarningOS.str(), Node, LocUsedForUniqueing, Node->getLocationContext()->getDecl()); - // TODO: As of now, one warning is emitted per constructor call, and the - // uninitialized fields are listed in notes. Until there's a better support - // for notes avaible, a note-less version of this checker should be - // implemented. - for (const auto &FieldChain : UninitFields) { + for (const auto &Chain : UninitFields) { SmallString<200> NoteBuf; llvm::raw_svector_ostream NoteOS(NoteBuf); -if (FieldChain.isPointer()) { - if (FieldChain.isDereferenced()) -NoteOS << "uninitialized pointee 'this->"; - else -NoteOS << "uninitialized pointer 'this->"; -} else - NoteOS << "uninitialized field 'this->"; -FieldChain.print(NoteOS); -NoteOS << "'"; +printNoteMessage(NoteOS, Chain); Report->addNote(NoteOS.str(), -
[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call
Szelethus added a comment. In https://reviews.llvm.org/D45532#1145512, @sylvestre.ledru wrote: > If you are interested, I executed this checker on Firefox code. Results can > be found here > http://sylvestre.ledru.info/reports/fx-scan-build/ Absolutely, thank you! :) > Andi reported this bug https://bugs.llvm.org/show_bug.cgi?id=37965 Well, that is intentional: not the pointer, but the pointee is uninitialized, as you can see from the note message. Now with that being said, I have had an other colleague of mine complain about a report, as he didn't see that the note message said "pointee" not "pointer", so maybe there's a point in trying to come up with a better message. In https://reviews.llvm.org/D45532#1145592, @george.karpenkov wrote: > @sylvestre.ledru Have you found any actual bugs using this checker? > @Szelethus Interesting, it seems that the pointer itself is initialized, but > not what it's pointing to. Exactly. > I think we should just check the fields directly, and do not attempt to > traverse the pointer hierarchy. Hmm, that's one way of thinking, but I think it's more beneficial to check pointers too. I'll take a look at some results and try to get back to you with some stats to support my view on this issue :) Repository: rC Clang https://reviews.llvm.org/D45532 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46652: [clang-cl, PCH] Implement support for MS-style PCH through headers
hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. This looks good to me. Comment at: include/clang/Driver/CC1Options.td:604 + HelpText<"When creating a pch stop at this file. When using a pch start " + "after this file.">; def fno_pch_timestamp : Flag<["-"], "fno-pch-timestamp">, mikerice wrote: > hans wrote: > > The "through header" terminology was new to me, and I didn't see it when > > browsing the MSDN articles about precompiled headers. The HelpText here > > probably isn't the right place, but it would be good if the term could be > > documented somewhere to make it clearer exactly what the behaviour is. It's > > not really obvious to me what "stop at this file" and "start after this > > file" means. I can guess, but it would be nice if it were more explicit :-) > You definitely have to look hard at the MSDN docs to find mention of through > headers. If you look at the documentation for /Yc and /Yu you can see some > vague references. I think it may have been more prominent many years ago. > > The MSDN page says "For /Yc, filename specifies the point at which > precompilation stops; the compiler precompiles all code though(sic) > filename..." https://msdn.microsoft.com/en-us/library/z0atkd6c.aspx > > I'll look for a place to document this better in a comment at least. > > Thanks for the pointer. Yeah, a clear explanation in a comment somewhere would be really helpful. Another idea might be to have a little "precompiled headers" section in the clang-cl section of docs/UsersManual.rst. Maybe we could do a better job than MS at explaining it precisely? :-) https://reviews.llvm.org/D46652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48661: [Fixed Point Arithmetic] Fixed Point Constant
ebevhan added a comment. I also think it would be good with some unit tests for this class once the functionality and interface is nailed down. Comment at: include/clang/Basic/FixedPoint.h:31 + SatNoPadding, +}; + rjmccall wrote: > I figured you'd want this to be a struct which include the scale, width, > signed-ness, and saturating-ness; and then `APFixedPoint` can just store one > of these next to a bare `APInt`. That's what I was expecting too, similar to the APFloat version. What width would it contain? The width with or without padding? Probably the one with padding as the APInt itself has the width without padding. Comment at: include/clang/Basic/FixedPoint.h:53 + // Change the width and scale of this fixed point number. + // Return true if overflow occurrd and false otherwise. + bool rescaleAndResize(unsigned DstWidth, unsigned DstScale); occurred Comment at: lib/AST/ASTContext.cpp:10320 + + if (DstWidth > SrcWidth) Val_ = Val_.extend(DstWidth); + leonardchan wrote: > ebevhan wrote: > > If you do rescaling before and after resizing, you can use `extOrTrunc` > > instead. > I think having this line just be `Val_ = Val_.extOrTrunc(DstWidth);` would > allow for truncation to occur first before potentially right shifting. True, but I meant something more along the lines of ``` if(DstScale < Scale) Val = Val >> (Scale - DstScale); Val = Val.extOrTrunc(DstWidth); if(DstScale > Scale) Val = Val << (DstScale - Scale); ``` But I think you've changed it a bit so I need to have a look at the revised version. Comment at: lib/Basic/FixedPoint.cpp:20 + +llvm::APSInt ShrToZero(const llvm::APSInt &Val, unsigned Amt) { + if (Val < 0) I'm unsure if this operation is useful for anything but converting fixed-point to integer. Make this a member of APFixedPoint, drop the arguments and give it a name that makes it clearer that it is a round-to-zero integer conversion? `toIntegerRoundToZero`? Comment at: lib/Basic/FixedPoint.cpp:53 +// We can overflow here +unsigned ShiftAmt = DstScale - Scale; +if (Val < 0 && Val.countLeadingOnes() >= ShiftAmt) I think saturation can be modeled a bit better. It should be possible to do the overflow check/saturation once instead of twice, and also have it fit in better with the other conversions. Saturation on a value is essentially checking whether all bits above and including a certain bit are all the same, and 'clamping' to either the minimum (the mask) or maximum (inverse of the mask) if not. ``` // Saturate Value to SatWidth. This will clamp Value at the signed min/max of a value that is SatWidth long. Saturate(SatWidth): Mask = highBits(Width, SatWidth + 1) Masked = Value & Mask Result = Value if (Masked == Mask || Masked == 0) { if (Masked.isNegative()) Result = Mask else Result = ~Mask } ``` This cannot be done on the value in the source scale, since upscaling after clamping would produce an incorrect value - we would shift in 0's from the right even though we should have saturated all bits completely. Also, we cannot upscale without first extending or we might shift out bits on the left that could affect saturation. One thing to note is that (I'm ***pretty sure*** this is the case) fractional bits cannot affect saturation. This means that we can find a common scale, then perform the saturation operation, and then resize, and the value should just fall into place. Basically: # Upscale if necessary, but extend first to ensure that we don't drop any bits on the left. Do this by resizing to `SrcWidth - SrcScale + std::max(SrcScale, DstScale)` and then upscaling normally by `DstScale - SrcScale`. # Downscale if necessary by `SrcScale - DstScale`. (I think; in our downstream, we saturate first and then downscale, but we also assume that saturation can only occur on _Fract, and doing saturation first makes the saturation width calculation a bit messier because it will be a `max` expression. I'm unsure if the order can be changed.) # At this point, the scale of the value should be `DstScale`. Saturate with `Saturate(DstWidth)`. # Now the value should be in range for the new width, and will be at the right scale as well. Resize with `extOrTrunc(DstWidth)`. # (Also; if the result was negative and the dest type is unsigned, just make the result zero here instead.) Note that the order of operations is different than what I've mentioned before with non-saturated conversion (downscale if needed, resize, upscale if needed), but it works because we only do the upscale as a resize+upscale. Somewhere in here you also need to change the signedness of the value, but I'm not entirely certain where. Likely after 4. Everything I've mentioned here is semi-conjectural, since our implementation has different type widths than
[PATCH] D48759: [ASTMatchers] add matcher for decltypeType and its underlyingType
alexfh added a comment. Please add a test. Repository: rC Clang https://reviews.llvm.org/D48759 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r335968 - [ASTImporter] Added import of CXXStdInitializerListExpr
Author: martong Date: Fri Jun 29 05:17:34 2018 New Revision: 335968 URL: http://llvm.org/viewvc/llvm-project?rev=335968&view=rev Log: [ASTImporter] Added import of CXXStdInitializerListExpr Reviewers: a.sidorin Reviewed By: a.sidorin Subscribers: martong, cfe-commits Differential Revision: https://reviews.llvm.org/D48631 Patch by Balazs Keri! Added: cfe/trunk/test/ASTMerge/std-initializer-list/ cfe/trunk/test/ASTMerge/std-initializer-list/Inputs/ cfe/trunk/test/ASTMerge/std-initializer-list/Inputs/il.cpp cfe/trunk/test/ASTMerge/std-initializer-list/test.cpp Modified: cfe/trunk/lib/AST/ASTImporter.cpp Modified: cfe/trunk/lib/AST/ASTImporter.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=335968&r1=335967&r2=335968&view=diff == --- cfe/trunk/lib/AST/ASTImporter.cpp (original) +++ cfe/trunk/lib/AST/ASTImporter.cpp Fri Jun 29 05:17:34 2018 @@ -384,6 +384,7 @@ namespace clang { Expr *VisitCallExpr(CallExpr *E); Expr *VisitLambdaExpr(LambdaExpr *LE); Expr *VisitInitListExpr(InitListExpr *E); +Expr *VisitCXXStdInitializerListExpr(CXXStdInitializerListExpr *E); Expr *VisitArrayInitLoopExpr(ArrayInitLoopExpr *E); Expr *VisitArrayInitIndexExpr(ArrayInitIndexExpr *E); Expr *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E); @@ -6622,6 +6623,19 @@ Expr *ASTNodeImporter::VisitInitListExpr return To; } +Expr *ASTNodeImporter::VisitCXXStdInitializerListExpr( +CXXStdInitializerListExpr *E) { + QualType T = Importer.Import(E->getType()); + if (T.isNull()) +return nullptr; + + Expr *SE = Importer.Import(E->getSubExpr()); + if (!SE) +return nullptr; + + return new (Importer.getToContext()) CXXStdInitializerListExpr(T, SE); +} + Expr *ASTNodeImporter::VisitArrayInitLoopExpr(ArrayInitLoopExpr *E) { QualType ToType = Importer.Import(E->getType()); if (ToType.isNull()) Added: cfe/trunk/test/ASTMerge/std-initializer-list/Inputs/il.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ASTMerge/std-initializer-list/Inputs/il.cpp?rev=335968&view=auto == --- cfe/trunk/test/ASTMerge/std-initializer-list/Inputs/il.cpp (added) +++ cfe/trunk/test/ASTMerge/std-initializer-list/Inputs/il.cpp Fri Jun 29 05:17:34 2018 @@ -0,0 +1,9 @@ +namespace std { +template +struct initializer_list { + const T *begin, *end; + initializer_list(); +}; +} // namespace std + +std::initializer_list IL = {1, 2, 3, 4}; Added: cfe/trunk/test/ASTMerge/std-initializer-list/test.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ASTMerge/std-initializer-list/test.cpp?rev=335968&view=auto == --- cfe/trunk/test/ASTMerge/std-initializer-list/test.cpp (added) +++ cfe/trunk/test/ASTMerge/std-initializer-list/test.cpp Fri Jun 29 05:17:34 2018 @@ -0,0 +1,3 @@ +// RUN: %clang_cc1 -emit-pch -o %t.1.ast %S/Inputs/il.cpp +// RUN: %clang_cc1 -ast-merge %t.1.ast -fsyntax-only %s 2>&1 | FileCheck --allow-empty %s +// CHECK-NOT: unsupported AST node ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48631: [ASTImporter] Added import of CXXStdInitializerListExpr
This revision was automatically updated to reflect the committed changes. Closed by commit rL335968: [ASTImporter] Added import of CXXStdInitializerListExpr (authored by martong, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D48631 Files: cfe/trunk/lib/AST/ASTImporter.cpp cfe/trunk/test/ASTMerge/std-initializer-list/Inputs/il.cpp cfe/trunk/test/ASTMerge/std-initializer-list/test.cpp Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -384,6 +384,7 @@ Expr *VisitCallExpr(CallExpr *E); Expr *VisitLambdaExpr(LambdaExpr *LE); Expr *VisitInitListExpr(InitListExpr *E); +Expr *VisitCXXStdInitializerListExpr(CXXStdInitializerListExpr *E); Expr *VisitArrayInitLoopExpr(ArrayInitLoopExpr *E); Expr *VisitArrayInitIndexExpr(ArrayInitIndexExpr *E); Expr *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E); @@ -6622,6 +6623,19 @@ return To; } +Expr *ASTNodeImporter::VisitCXXStdInitializerListExpr( +CXXStdInitializerListExpr *E) { + QualType T = Importer.Import(E->getType()); + if (T.isNull()) +return nullptr; + + Expr *SE = Importer.Import(E->getSubExpr()); + if (!SE) +return nullptr; + + return new (Importer.getToContext()) CXXStdInitializerListExpr(T, SE); +} + Expr *ASTNodeImporter::VisitArrayInitLoopExpr(ArrayInitLoopExpr *E) { QualType ToType = Importer.Import(E->getType()); if (ToType.isNull()) Index: cfe/trunk/test/ASTMerge/std-initializer-list/Inputs/il.cpp === --- cfe/trunk/test/ASTMerge/std-initializer-list/Inputs/il.cpp +++ cfe/trunk/test/ASTMerge/std-initializer-list/Inputs/il.cpp @@ -0,0 +1,9 @@ +namespace std { +template +struct initializer_list { + const T *begin, *end; + initializer_list(); +}; +} // namespace std + +std::initializer_list IL = {1, 2, 3, 4}; Index: cfe/trunk/test/ASTMerge/std-initializer-list/test.cpp === --- cfe/trunk/test/ASTMerge/std-initializer-list/test.cpp +++ cfe/trunk/test/ASTMerge/std-initializer-list/test.cpp @@ -0,0 +1,3 @@ +// RUN: %clang_cc1 -emit-pch -o %t.1.ast %S/Inputs/il.cpp +// RUN: %clang_cc1 -ast-merge %t.1.ast -fsyntax-only %s 2>&1 | FileCheck --allow-empty %s +// CHECK-NOT: unsupported AST node Index: cfe/trunk/lib/AST/ASTImporter.cpp === --- cfe/trunk/lib/AST/ASTImporter.cpp +++ cfe/trunk/lib/AST/ASTImporter.cpp @@ -384,6 +384,7 @@ Expr *VisitCallExpr(CallExpr *E); Expr *VisitLambdaExpr(LambdaExpr *LE); Expr *VisitInitListExpr(InitListExpr *E); +Expr *VisitCXXStdInitializerListExpr(CXXStdInitializerListExpr *E); Expr *VisitArrayInitLoopExpr(ArrayInitLoopExpr *E); Expr *VisitArrayInitIndexExpr(ArrayInitIndexExpr *E); Expr *VisitCXXDefaultInitExpr(CXXDefaultInitExpr *E); @@ -6622,6 +6623,19 @@ return To; } +Expr *ASTNodeImporter::VisitCXXStdInitializerListExpr( +CXXStdInitializerListExpr *E) { + QualType T = Importer.Import(E->getType()); + if (T.isNull()) +return nullptr; + + Expr *SE = Importer.Import(E->getSubExpr()); + if (!SE) +return nullptr; + + return new (Importer.getToContext()) CXXStdInitializerListExpr(T, SE); +} + Expr *ASTNodeImporter::VisitArrayInitLoopExpr(ArrayInitLoopExpr *E) { QualType ToType = Importer.Import(E->getType()); if (ToType.isNull()) Index: cfe/trunk/test/ASTMerge/std-initializer-list/Inputs/il.cpp === --- cfe/trunk/test/ASTMerge/std-initializer-list/Inputs/il.cpp +++ cfe/trunk/test/ASTMerge/std-initializer-list/Inputs/il.cpp @@ -0,0 +1,9 @@ +namespace std { +template +struct initializer_list { + const T *begin, *end; + initializer_list(); +}; +} // namespace std + +std::initializer_list IL = {1, 2, 3, 4}; Index: cfe/trunk/test/ASTMerge/std-initializer-list/test.cpp === --- cfe/trunk/test/ASTMerge/std-initializer-list/test.cpp +++ cfe/trunk/test/ASTMerge/std-initializer-list/test.cpp @@ -0,0 +1,3 @@ +// RUN: %clang_cc1 -emit-pch -o %t.1.ast %S/Inputs/il.cpp +// RUN: %clang_cc1 -ast-merge %t.1.ast -fsyntax-only %s 2>&1 | FileCheck --allow-empty %s +// CHECK-NOT: unsupported AST node ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32845: [Analyzer] Iterator Checker - Part 4: Mismatched iterator checker for function parameters
baloghadamsoftware updated this revision to Diff 153481. baloghadamsoftware added a comment. Previous rebase was wrong, this is the correct one. https://reviews.llvm.org/D32845 Files: include/clang/StaticAnalyzer/Checkers/Checkers.td lib/StaticAnalyzer/Checkers/IteratorChecker.cpp test/Analysis/Inputs/system-header-simulator-cxx.h test/Analysis/mismatched-iterator.cpp Index: test/Analysis/mismatched-iterator.cpp === --- /dev/null +++ test/Analysis/mismatched-iterator.cpp @@ -0,0 +1,25 @@ +// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.MismatchedIterator -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=false %s -verify +// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.MismatchedIterator -analyzer-eagerly-assume -analyzer-config aggressive-relational-comparison-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify + +#include "Inputs/system-header-simulator-cxx.h" + +void good_find(std::vector &v, int n) { + std::find(v.cbegin(), v.cend(), n); // no-warning +} + +void good_find_first_of(std::vector &v1, std::vector &v2) { + std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v2.cend()); // no-warning +} + +void good_copy(std::vector &v1, std::vector &v2, int n) { + std::copy(v1.cbegin(), v1.cend(), v2.begin()); // no-warning +} + +void bad_find(std::vector &v1, std::vector &v2, int n) { + std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterator access mismatched}} +} + +void bad_find_first_of(std::vector &v1, std::vector &v2) { + std::find_first_of(v1.cbegin(), v2.cend(), v2.cbegin(), v2.cend()); // expected-warning{{Iterator access mismatched}} + std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v1.cend()); // expected-warning{{Iterator access mismatched}} +} Index: test/Analysis/Inputs/system-header-simulator-cxx.h === --- test/Analysis/Inputs/system-header-simulator-cxx.h +++ test/Analysis/Inputs/system-header-simulator-cxx.h @@ -642,6 +642,12 @@ template InputIterator find(InputIterator first, InputIterator last, const T &val); + template + ForwardIterator1 find_first_of(ForwardIterator1 first1, + ForwardIterator1 last1, + ForwardIterator2 first2, + ForwardIterator2 last2); + template OutputIterator copy(InputIterator first, InputIterator last, OutputIterator result); Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp === --- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp +++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp @@ -199,6 +199,7 @@ eval::Assume> { std::unique_ptr OutOfRangeBugType; + std::unique_ptr MismatchedBugType; std::unique_ptr InvalidatedBugType; void handleComparison(CheckerContext &C, const SVal &RetVal, const SVal &LVal, @@ -222,16 +223,23 @@ void verifyRandomIncrOrDecr(CheckerContext &C, OverloadedOperatorKind Op, const SVal &RetVal, const SVal &LHS, const SVal &RHS) const; + void verifyMatch(CheckerContext &C, const SVal &Iter1, + const SVal &Iter2) const; + void reportOutOfRangeBug(const StringRef &Message, const SVal &Val, CheckerContext &C, ExplodedNode *ErrNode) const; + void reportMismatchedBug(const StringRef &Message, const SVal &Val1, + const SVal &Val2, CheckerContext &C, + ExplodedNode *ErrNode) const; void reportInvalidatedBug(const StringRef &Message, const SVal &Val, CheckerContext &C, ExplodedNode *ErrNode) const; public: IteratorChecker(); enum CheckKind { CK_IteratorRangeChecker, +CK_MismatchedIteratorChecker, CK_InvalidatedIteratorChecker, CK_NumCheckKinds }; @@ -321,6 +329,9 @@ OutOfRangeBugType.reset( new BugType(this, "Iterator out of range", "Misuse of STL APIs")); OutOfRangeBugType->setSuppressOnSink(true); + MismatchedBugType.reset( + new BugType(this, "Iterator(s) mismatched", "Misuse of STL APIs")); + MismatchedBugType->setSuppressOnSink(true); InvalidatedBugType.reset( new BugType(this, "Iterator invalidated", "Misuse of STL APIs")); InvalidatedBugType->setSuppressOnSink(true); @@ -369,6 +380,65 @@ verifyDereference(C, Call.getArgSVal(0)); } } + } else if (!isa(&Call)) { +// The main purpose of iterators is to abstract away from different +// containers and provide a (maybe limited) uniform access to them. +// This implies that any correctly written template functio
[PATCH] D32859: [Analyzer] Iterator Checker - Part 5: Move Assignment of Containers
baloghadamsoftware updated this revision to Diff 153482. baloghadamsoftware added a comment. Herald added a subscriber: mikhail.ramalho. Rebased to https://reviews.llvm.org/rL335835. https://reviews.llvm.org/D32859 Files: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp test/Analysis/mismatched-iterator.cpp Index: test/Analysis/mismatched-iterator.cpp === --- test/Analysis/mismatched-iterator.cpp +++ test/Analysis/mismatched-iterator.cpp @@ -15,11 +15,48 @@ std::copy(v1.cbegin(), v1.cend(), v2.begin()); // no-warning } +void good_move_find1(std::vector &v1, std::vector &v2, int n) { + auto i0 = v2.cbegin(); + v1 = std::move(v2); + std::find(i0, v1.cend(), n); // no-warning +} + +void good_move_find2(std::vector &v1, std::vector &v2, int n) { + auto i0 = --v2.cend(); + v1 = std::move(v2); + std::find(i0, v1.cend(), n); // no-warning +} + +void good_move_find3(std::vector &v1, std::vector &v2, int n) { + auto i0 = v2.cend(); + v1 = std::move(v2); + v2.push_back(n); + std::find(v2.cbegin(), i0, n); // no-warning +} + void bad_find(std::vector &v1, std::vector &v2, int n) { std::find(v1.cbegin(), v2.cend(), n); // expected-warning{{Iterator access mismatched}} } void bad_find_first_of(std::vector &v1, std::vector &v2) { std::find_first_of(v1.cbegin(), v2.cend(), v2.cbegin(), v2.cend()); // expected-warning{{Iterator access mismatched}} std::find_first_of(v1.cbegin(), v1.cend(), v2.cbegin(), v1.cend()); // expected-warning{{Iterator access mismatched}} } + +void bad_move_find1(std::vector &v1, std::vector &v2, int n) { + auto i0 = v2.cbegin(); + v1 = std::move(v2); + std::find(i0, v2.cend(), n); // expected-warning{{Iterator access mismatched}} +} + +void bad_move_find2(std::vector &v1, std::vector &v2, int n) { + auto i0 = --v2.cend(); + v1 = std::move(v2); + std::find(i0, v2.cend(), n); // expected-warning{{Iterator access mismatched}} +} + +void bad_move_find3(std::vector &v1, std::vector &v2, int n) { + auto i0 = v2.cend(); + v1 = std::move(v2); + std::find(v1.cbegin(), i0, n); // expected-warning{{Iterator access mismatched}} +} Index: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp === --- lib/StaticAnalyzer/Checkers/IteratorChecker.cpp +++ lib/StaticAnalyzer/Checkers/IteratorChecker.cpp @@ -114,6 +114,10 @@ return IteratorPosition(Cont, Valid, NewOf); } + IteratorPosition reAssign(const MemRegion *NewCont) const { +return IteratorPosition(NewCont, Valid, Offset); + } + bool operator==(const IteratorPosition &X) const { return Cont == X.Cont && Valid == X.Valid && Offset == X.Offset; } @@ -219,7 +223,9 @@ const SVal &Cont) const; void assignToContainer(CheckerContext &C, const Expr *CE, const SVal &RetVal, const MemRegion *Cont) const; - void handleAssign(CheckerContext &C, const SVal &Cont) const; + void handleAssign(CheckerContext &C, const SVal &Cont, +const Expr *CE = nullptr, +const SVal &OldCont = UndefinedVal()) const; void verifyRandomIncrOrDecr(CheckerContext &C, OverloadedOperatorKind Op, const SVal &RetVal, const SVal &LHS, const SVal &RHS) const; @@ -317,6 +323,17 @@ bool Equal); ProgramStateRef invalidateAllIteratorPositions(ProgramStateRef State, const MemRegion *Cont); +ProgramStateRef reassignAllIteratorPositions(ProgramStateRef State, + const MemRegion *Cont, + const MemRegion *NewCont); +ProgramStateRef reassignAllIteratorPositionsUnless(ProgramStateRef State, + const MemRegion *Cont, + const MemRegion *NewCont, + SymbolRef Offset, + BinaryOperator::Opcode Opc); +ProgramStateRef replaceSymbolInIteratorPositionsIf( +ProgramStateRef State, SValBuilder &SVB, SymbolRef OldSym, +SymbolRef NewSym, SymbolRef CondSym, BinaryOperator::Opcode Opc); const ContainerData *getContainerData(ProgramStateRef State, const MemRegion *Cont); ProgramStateRef setContainerData(ProgramStateRef State, const MemRegion *Cont, @@ -453,7 +470,12 @@ const auto Op = Func->getOverloadedOperator(); if (isAssignmentOperator(Op)) { const auto *InstCall = dyn_cast(&Call); - handleAssign(C, InstCall->getCXXThisVal()); + if (Func->getParamDecl(0)->getType()->isRValueReferenceType()) { +handleAssign(C, InstCall->getCXXThisVal(), Call.getOriginExpr(), + Call.getArgSVal(0)); + } else { +handl
[PATCH] D48412: [RISCV] Add support for interrupt attribute
aaron.ballman added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:5305 + + if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) { +S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 0; I would have assumed this would be: `!hasFunctionProto(D) || getFunctionOrMethodNumParams(D) != 0`, but it depends on whether you want to support K&R C functions. Comment at: lib/Sema/SemaDeclAttr.cpp:5301 + + if (!isFunctionOrMethod(D)) { +S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type) apazos wrote: > aaron.ballman wrote: > > apazos wrote: > > > aaron.ballman wrote: > > > > I don't think you need to perform this check -- I believe it's handled > > > > automatically (because you don't have custom parsing enabled). > > > I think need it. Will double check in the test. > > See `handleCommonAttributeFeatures()` -- it calls `diagnoseAppertainsTo()` > > which handles this for you. As it is, your check here does not match the > > subject list on the attribute. The declarative bit says it only appertains > > to a function and this check is for a function or Obj-C method. > > > > Which brings up my next question: should this appertain to an ObjC method? > It looks like handleCommonAttributeFeatures should take care of the check, > but I do not see it happening, it returns true in AL.diagnoseAppertainsTo(S, > D) even when we have > struct a test __attribute__((interrupt)); > > I will remove the Subjects in Attr.td and keep the checks as they are in > handleRISCVInterruptAttr. > > Several other targets do the same thing, they are reusing the helper > functions that apply to both Function or Method. We would have to create > helper functions just for function types. Ah, the reason is because the parsed attributes that share a `ParseKind` can have different subject lists, so there's no way to do the semantic checking at that point -- we don't know which semantic attribute to check the subjects against until later. Please put the `Subjects` list back in to Attr.td; it's still useful declarative information and I may solve this problem someday in the future. I am not tied to whether the attribute appertains to a function and an obj-c method as that depends on the attribute in question, but the code as it stands is wrong. It checks whether the declaration is a function or a method and then tells the user the attribute can only appertain to a function and not a method. Which is correct? Comment at: test/Sema/riscv-interrupt-attr.c:18 + +__attribute__((interrupt("user"), interrupt("supervisor"))) void foo6() { } // expected-warning {{repeated RISC-V 'interrupt' attribute}} \ +// expected-note {{repeated RISC-V 'interrupt' attribute is here}} apazos wrote: > aaron.ballman wrote: > > apazos wrote: > > > aaron.ballman wrote: > > > > You should also add tests for: > > > > ``` > > > > __attribute__((interrupt("user"))) void f(void); > > > > __attribute__((interrupt("machine"))) void f(void); > > > > > > > > void f(void) { } > > > > > > > > [[gnu::interrupt("user") gnu::interrupt("machine")]] void g() {} > > > > > > > > [[gnu::interrupt("user")]] [[gnu::interrupt("machine")]] void h() {} > > > > ``` > > > For this test case tt seems LLVM honors the last setting, "machine". > > > But gcc is honoring the first. > > > I think the last setting should prevail. Will check with GCC folks. > > Do all of these cases get diagnosed as being a repeated interrupt > > attribute? Should add them as test cases. > The warning for repeated attribute is when it occurs more than once in the > same declaration. If you have repeated declarations, the last one prevails. Please document this in AttrDocs.td. Comment at: test/Sema/riscv-interrupt-attr.c:23 + // expected-note {{repeated RISC-V 'interrupt' attribute is here}} +__attribute__((interrupt("user"))) void foo8() {} +__attribute__((interrupt("supervisor"))) void foo9() {} aaron.ballman wrote: > Do you intend for functions without a prototype to be accepted? foo8() can be > passed an arbitrary number of arguments, which is a bit different than what I > thought you wanted the semantic check to be. This question remains outstanding. https://reviews.llvm.org/D48412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48712: [X86] Lowering integer truncation intrinsics to native IR
RKSimon added inline comments. Comment at: clang/lib/Headers/avx512vlbwintrin.h:1501 + (__v8qi){0, 0, 0, 0, 0, 0, 0, 0}, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, + 12, 13, 14, 15); } Are we happy with using illegal types like this? What about flipping the shuffle and convert? ``` return (__m128i)__builtin_convertvector( __builtin_shufflevector((__v8hi)__A, (__v8hi){0, 0, 0, 0, 0, 0, 0, 0}, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15), __v16qi); ``` https://reviews.llvm.org/D48712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48734: [Sema] Consider all format_arg attributes.
aaron.ballman added inline comments. Comment at: lib/Sema/SemaChecking.cpp:5523 + StringLiteralCheckType CommonResult; + for (const FormatArgAttr *FA : ND->specific_attrs()) { const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex()); You can use `const auto *FA` here instead; the type is spelled out in the initializer. Comment at: lib/Sema/SemaChecking.cpp:5536 + + if (const FunctionDecl *FD = dyn_cast(ND)) { unsigned BuiltinID = FD->getBuiltinID(); `const auto *` Comment at: test/Sema/attr-format_arg.c:17 + + printf(h("", ""), 123); // expected-warning 2{{format string is empty}} } Can you add tests for the other permutations as well? It would also be useful to put the string literals on their own line so that the expected-warning designation matches the specific argument that's being diagnosed. e.g., ``` h( "%d", "" ) h( "", "%d" ) h( "%d", "%d" ) ``` Repository: rC Clang https://reviews.llvm.org/D48734 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48762: [clangd] codeComplete returns more structured completion items, LSP. NFC.
ioeric added a comment. Looks great! Mostly nits. Comment at: clangd/CodeComplete.cpp:259 + : ASTCtx(ASTCtx), ExtractDocumentation(Opts.IncludeComments) { +if (C.SemaResult) { + Completion.Name = llvm::StringRef(SemaCCS->getTypedText()); nit: shall we keep `assert(bool(SemaResult) == bool(SemaCCS));`? Comment at: clangd/CodeComplete.cpp:279 +if (auto Inserted = C.headerToInsertIfNotPresent()) { + llvm::errs() << "Inserted = " << *Inserted << "\n"; + Completion.Header = *Inserted; remove? Comment at: clangd/CodeComplete.cpp:280 + llvm::errs() << "Inserted = " << *Inserted << "\n"; + Completion.Header = *Inserted; + // Turn absolute path into a literal string that can be #included. `Inserted` can be an absolute path which might have long uninteresting prefix. It might make sense to use the result from `calculateIncludePath`? Comment at: clangd/CodeComplete.cpp:309 + + void add(const CompletionCandidate &C, CodeCompletionString *SemaCCS) { +Bundled.emplace_back(); nit: It's unclear why `add` is a public interface given that the constructor also adds candidate. Might worth a comment. Also, it seems that the interface design has been driven by by the bundling logic, so it might also make sense to explain a bit on the class level. Comment at: clangd/CodeComplete.cpp:322 +} +if (ExtractDocumentation && Completion.Documentation.empty()) { + if (C.IndexResult && C.IndexResult->Detail) nit: the documentation for the bundling says `Documentation may contain a combined docstring from all comments`. The implementation seems to take the documentation from the last comment (which seems fine to me), but they should be consistent. Comment at: clangd/CodeComplete.cpp:346 + + template + const std::string *onlyValue() const { nit: this might worth a comment. it's unclear what this does by just looking at the name. Comment at: clangd/CodeComplete.cpp:1145 +Scores.ExcludingName = Relevance.NameMatch + ? Scores.Total / Relevance.NameMatch + : Scores.Quality; nit: this seems to assume that `NameMatch` is a factor of the final score, which seems to be changeable given the abstraction of `evaluateSymbolAndRelevance`. Not sure if there is an alternative to the current approach, but this probably deserves a comment. Comment at: clangd/CodeComplete.cpp:1161 - CompletionItem toCompletionItem(const CompletionCandidate::Bundle &Bundle, - const CompletionItemScores &Scores) { -CodeCompletionString *SemaCCS = nullptr; -std::string FrontDocComment; -if (auto *SR = Bundle.front().SemaResult) { - SemaCCS = Recorder->codeCompletionString(*SR); - if (Opts.IncludeComments) { -assert(Recorder->CCSema); -FrontDocComment = getDocComment(Recorder->CCSema->getASTContext(), *SR, -/*CommentsFromHeader=*/false); - } + CodeCompletion toCompletionItem(const CompletionCandidate::Bundle &Bundle) { +llvm::Optional Builder; nit: toCompletionItem might be ambiguous now that this returns `CodeCompletion`. Comment at: clangd/CodeComplete.h:88 + // Qualifiers that must be inserted before the name (e.g. base::). + std::string Qualifiers; + // Details to be displayed following the name. Not inserted. Give that this field has caused confusion before, would it make sense to make the name a bit more concrete like `RequiredQualifier` or `InsertedQualifier`? Comment at: clangd/CodeComplete.h:106 + unsigned BundleSize; + // The header through which this symbol should be included. + // Empty for non-symbol completions. Would this be an absolute path or an include path? Comment at: clangd/CodeComplete.h:140 + std::vector Completions; + bool More = false; +}; I found `Result.More` a bit not straightforward. Maybe `HasMore`? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48759: [ASTMatchers] add matcher for decltypeType and its underlyingType
aaron.ballman added a comment. Once this goes in, you can also update TrailingReturnTypeCheck.cpp to remove its local instance of this type matcher. Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:137 REGISTER_MATCHER(autoType); + REGISTER_MATCHER(decltypeType); REGISTER_MATCHER(binaryOperator); Please keep this list sorted in alphabetical order. Repository: rC Clang https://reviews.llvm.org/D48759 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48717: [clang-tidy] fix PR36489 - respect deduced pointer types from auto as well
aaron.ballman added a comment. In https://reviews.llvm.org/D48717#1147644, @JonasToth wrote: > - fix decltype deduction with new matcher > > I had to introduce a new matcher in clang for `DecltypeType` to reach its > underlying type. It would be better to have `hasType` resolve all these > issues but i dont know how much work that would be. I don't think we want to modify `hasType()` -- that would strip off too much type information if it automatically reached through to the underlying type. However, we may want to consider adding something like `hasUnderlyingType()` that checks the matcher against each level of type sugar. I'm not certain this would be required for your patch, however. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48717 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48634: [clangd] Improve output of --help and --version. NFC.
This revision was automatically updated to reflect the committed changes. Closed by commit rL335972: [clangd] Improve output of --help and --version. NFC. (authored by sammccall, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D48634 Files: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp === --- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp +++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp @@ -18,6 +18,7 @@ #include "llvm/Support/Program.h" #include "llvm/Support/Signals.h" #include "llvm/Support/raw_ostream.h" +#include "clang/Basic/Version.h" #include #include #include @@ -153,7 +154,16 @@ int main(int argc, char *argv[]) { llvm::sys::PrintStackTraceOnErrorSignal(argv[0]); - llvm::cl::ParseCommandLineOptions(argc, argv, "clangd"); + llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) { +OS << clang::getClangToolFullVersion("clangd") << "\n"; + }); + llvm::cl::ParseCommandLineOptions( + argc, argv, + "clangd is a language server that provides IDE-like features to editors. " + "\n\nIt should be used via an editor plugin rather than invoked directly." + "For more information, see:" + "\n\thttps://clang.llvm.org/extra/clangd.html"; + "\n\thttps://microsoft.github.io/language-server-protocol/";); if (Test) { RunSynchronously = true; InputStyle = JSONStreamStyle::Delimited; Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp === --- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp +++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp @@ -18,6 +18,7 @@ #include "llvm/Support/Program.h" #include "llvm/Support/Signals.h" #include "llvm/Support/raw_ostream.h" +#include "clang/Basic/Version.h" #include #include #include @@ -153,7 +154,16 @@ int main(int argc, char *argv[]) { llvm::sys::PrintStackTraceOnErrorSignal(argv[0]); - llvm::cl::ParseCommandLineOptions(argc, argv, "clangd"); + llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) { +OS << clang::getClangToolFullVersion("clangd") << "\n"; + }); + llvm::cl::ParseCommandLineOptions( + argc, argv, + "clangd is a language server that provides IDE-like features to editors. " + "\n\nIt should be used via an editor plugin rather than invoked directly." + "For more information, see:" + "\n\thttps://clang.llvm.org/extra/clangd.html"; + "\n\thttps://microsoft.github.io/language-server-protocol/";); if (Test) { RunSynchronously = true; InputStyle = JSONStreamStyle::Delimited; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r335972 - [clangd] Improve output of --help and --version. NFC.
Author: sammccall Date: Fri Jun 29 06:24:20 2018 New Revision: 335972 URL: http://llvm.org/viewvc/llvm-project?rev=335972&view=rev Log: [clangd] Improve output of --help and --version. NFC. Reviewers: ilya-biryukov Subscribers: ioeric, MaskRay, jkorous, cfe-commits Differential Revision: https://reviews.llvm.org/D48634 Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=335972&r1=335971&r2=335972&view=diff == --- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original) +++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Fri Jun 29 06:24:20 2018 @@ -18,6 +18,7 @@ #include "llvm/Support/Program.h" #include "llvm/Support/Signals.h" #include "llvm/Support/raw_ostream.h" +#include "clang/Basic/Version.h" #include #include #include @@ -153,7 +154,16 @@ static llvm::cl::opt YamlSymbolFil int main(int argc, char *argv[]) { llvm::sys::PrintStackTraceOnErrorSignal(argv[0]); - llvm::cl::ParseCommandLineOptions(argc, argv, "clangd"); + llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) { +OS << clang::getClangToolFullVersion("clangd") << "\n"; + }); + llvm::cl::ParseCommandLineOptions( + argc, argv, + "clangd is a language server that provides IDE-like features to editors. " + "\n\nIt should be used via an editor plugin rather than invoked directly." + "For more information, see:" + "\n\thttps://clang.llvm.org/extra/clangd.html"; + "\n\thttps://microsoft.github.io/language-server-protocol/";); if (Test) { RunSynchronously = true; InputStyle = JSONStreamStyle::Delimited; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48661: [Fixed Point Arithmetic] Fixed Point Constant
ebevhan added inline comments. Comment at: lib/Basic/FixedPoint.cpp:53 +// We can overflow here +unsigned ShiftAmt = DstScale - Scale; +if (Val < 0 && Val.countLeadingOnes() >= ShiftAmt) ebevhan wrote: > I think saturation can be modeled a bit better. It should be possible to do > the overflow check/saturation once instead of twice, and also have it fit in > better with the other conversions. > > Saturation on a value is essentially checking whether all bits above and > including a certain bit are all the same, and 'clamping' to either the > minimum (the mask) or maximum (inverse of the mask) if not. > ``` > // Saturate Value to SatWidth. This will clamp Value at the signed min/max of > a value that is SatWidth long. > Saturate(SatWidth): > Mask = highBits(Width, SatWidth + 1) > Masked = Value & Mask > Result = Value > if (Masked == Mask || Masked == 0) { > if (Masked.isNegative()) > Result = Mask > else > Result = ~Mask > } > ``` > This cannot be done on the value in the source scale, since upscaling after > clamping would produce an incorrect value - we would shift in 0's from the > right even though we should have saturated all bits completely. Also, we > cannot upscale without first extending or we might shift out bits on the left > that could affect saturation. > > One thing to note is that (I'm ***pretty sure*** this is the case) fractional > bits cannot affect saturation. This means that we can find a common scale, > then perform the saturation operation, and then resize, and the value should > just fall into place. Basically: > # Upscale if necessary, but extend first to ensure that we don't drop any > bits on the left. Do this by resizing to `SrcWidth - SrcScale + > std::max(SrcScale, DstScale)` and then upscaling normally by `DstScale - > SrcScale`. > # Downscale if necessary by `SrcScale - DstScale`. (I think; in our > downstream, we saturate first and then downscale, but we also assume that > saturation can only occur on _Fract, and doing saturation first makes the > saturation width calculation a bit messier because it will be a `max` > expression. I'm unsure if the order can be changed.) > # At this point, the scale of the value should be `DstScale`. Saturate with > `Saturate(DstWidth)`. > # Now the value should be in range for the new width, and will be at the > right scale as well. Resize with `extOrTrunc(DstWidth)`. > # (Also; if the result was negative and the dest type is unsigned, just > make the result zero here instead.) > > Note that the order of operations is different than what I've mentioned > before with non-saturated conversion (downscale if needed, resize, upscale if > needed), but it works because we only do the upscale as a resize+upscale. > Somewhere in here you also need to change the signedness of the value, but > I'm not entirely certain where. Likely after 4. > > Everything I've mentioned here is semi-conjectural, since our implementation > has different type widths than the defaults in this one, we can only saturate > on _Fract, and our unsigned types have padding. It's possible that there are > some assumptions that cause this method to fail for unsigned without padding, > or perhaps for some other type conversion, but I haven't come up with a > counterexample yet. Now that I think about it a bit more, it's clear that the Saturate routine does not work for unsigned fixed-point without padding. That would need to be taken into consideration, but the rest should work. Repository: rC Clang https://reviews.llvm.org/D48661 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48098: clang-format-diff: Switch to python3 by default, support python 2.7
krasimir accepted this revision. krasimir added a comment. This revision is now accepted and ready to land. Checked with both python 2 and python 3 as suggested. Works like a charm! Thank you! https://reviews.llvm.org/D48098 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r335978 - [clang-format] Support additional common functions for text proto formatting
Author: krasimir Date: Fri Jun 29 07:25:25 2018 New Revision: 335978 URL: http://llvm.org/viewvc/llvm-project?rev=335978&view=rev Log: [clang-format] Support additional common functions for text proto formatting Summary: This adds a few more common function names expecting a text proto argument. Reviewers: djasper Reviewed By: djasper Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D48760 Modified: cfe/trunk/lib/Format/Format.cpp Modified: cfe/trunk/lib/Format/Format.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=335978&r1=335977&r2=335978&view=diff == --- cfe/trunk/lib/Format/Format.cpp (original) +++ cfe/trunk/lib/Format/Format.cpp Fri Jun 29 07:25:25 2018 @@ -775,8 +775,11 @@ FormatStyle getGoogleStyle(FormatStyle:: }, /*EnclosingFunctionNames=*/ { - "PARSE_TEXT_PROTO", "EqualsProto", + "EquivToProto", + "PARSE_TEST_PROTO", + "PARSE_TEXT_PROTO", + "ParseTextOrDie", }, /*CanonicalDelimiter=*/"", /*BasedOnStyle=*/"google", ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48760: [clang-format] Support additional common functions for text proto formatting
This revision was automatically updated to reflect the committed changes. Closed by commit rC335978: [clang-format] Support additional common functions for text proto formatting (authored by krasimir, committed by ). Changed prior to commit: https://reviews.llvm.org/D48760?vs=153459&id=153491#toc Repository: rC Clang https://reviews.llvm.org/D48760 Files: lib/Format/Format.cpp Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -775,8 +775,11 @@ }, /*EnclosingFunctionNames=*/ { - "PARSE_TEXT_PROTO", "EqualsProto", + "EquivToProto", + "PARSE_TEST_PROTO", + "PARSE_TEXT_PROTO", + "ParseTextOrDie", }, /*CanonicalDelimiter=*/"", /*BasedOnStyle=*/"google", Index: lib/Format/Format.cpp === --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -775,8 +775,11 @@ }, /*EnclosingFunctionNames=*/ { - "PARSE_TEXT_PROTO", "EqualsProto", + "EquivToProto", + "PARSE_TEST_PROTO", + "PARSE_TEXT_PROTO", + "ParseTextOrDie", }, /*CanonicalDelimiter=*/"", /*BasedOnStyle=*/"google", ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48762: [clangd] codeComplete returns more structured completion items, LSP. NFC.
sammccall updated this revision to Diff 153493. sammccall marked 11 inline comments as done. sammccall added a comment. Address review comments. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48762 Files: clangd/ClangdServer.cpp clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/Protocol.h Index: clangd/Protocol.h === --- clangd/Protocol.h +++ clangd/Protocol.h @@ -668,20 +668,6 @@ Snippet = 2, }; -/// Provides details for how a completion item was scored. -/// This can be used for client-side filtering of completion items as the -/// user keeps typing. -/// This is a clangd extension. -struct CompletionItemScores { - /// The score that items are ranked by. - /// This is filterScore * symbolScore. - float finalScore = 0.f; - /// How the partial identifier matched filterText. [0-1] - float filterScore = 0.f; - /// How the symbol fits, ignoring the partial identifier. - float symbolScore = 0.f; -}; - struct CompletionItem { /// The label of this completion item. By default also the text that is /// inserted when selecting this completion. @@ -702,9 +688,6 @@ /// When `falsy` the label is used. std::string sortText; - /// Details about the quality of this completion item. (clangd extension) - llvm::Optional scoreInfo; - /// A string that should be used when filtering a set of completion items. /// When `falsy` the label is used. std::string filterText; Index: clangd/CodeComplete.h === --- clangd/CodeComplete.h +++ clangd/CodeComplete.h @@ -75,15 +75,79 @@ const SymbolIndex *Index = nullptr; }; +// Semi-structured representation of a code-complete suggestion for our C++ API. +// We don't use the LSP structures here (unlike most features) as we want +// to expose more data to allow for more precise testing and evaluation. +struct CodeCompletion { + // The unqualified name of the symbol or other completion item. + std::string Name; + // The scope qualifier for the symbol name. e.g. "ns1::ns2::" + // Empty for non-symbol completions. Not inserted, but may be displayed. + std::string Scope; + // Text that must be inserted before the name, and displayed (e.g. base::). + std::string RequiredQualifier; + // Details to be displayed following the name. Not inserted. + std::string Signature; + // Text to be inserted following the name, in snippet format. + std::string SnippetSuffix; + // Type to be displayed for this completion. + std::string ReturnType; + std::string Documentation; + CompletionItemKind Kind = CompletionItemKind::Missing; + // This completion item may represent several symbols that can be inserted in + // the same way, such as function overloads. In this case BundleSize > 1, and + // the following fields are summaries: + // - Signature is e.g. "(...)" for functions. + // - SnippetSuffix is similarly e.g. "(${0})". + // - ReturnType may be empty + // - Documentation may be from one symbol, or a combination of several + // Other fields should apply equally to all bundled completions. + unsigned BundleSize; + // The header through which this symbol could be included. + // Quoted string as expected by an #include directive, e.g. "". + // Empty for non-symbol completions, or when not known. + std::string Header; + // Present if Header is set and should be inserted to use this item. + llvm::Optional HeaderInsertion; + + // Scores are used to rank completion items. + struct Scores { +// The score that items are ranked by. +float Total = 0.f; + +// The finalScore with the fuzzy name match score excluded. +// When filtering client-side, editors should calculate the new fuzzy score, +// whose scale is 0-1 (with 1 = prefix match, special case 2 = exact match), +// and recompute finalScore = fuzzyScore * symbolScore. +float ExcludingName = 0.f; + +// Component scores that contributed to the final score: + +// Quality describes how important we think this candidate is, +// independent of the query. +// e.g. symbols with lots of incoming references have higher quality. +float Quality = 0.f; +// Relevance describes how well this candidate matched the query. +// e.g. symbols from nearby files have higher relevance. +float Relevance = 0.f; + }; + Scores Score; + + // Serialize this to an LSP completion item. This is a lossy operation. + CompletionItem render(const CodeCompleteOptions &) const; +}; +struct CodeCompleteResult { + std::vector Completions; + bool HasMore = false; +}; + /// Get code completions at a specified \p Pos in \p FileName. -CompletionList codeComplete(PathRef FileName, -const tooling::CompileCommand &Command, -PrecompiledPreamble const *Preamble, -const std::vector &PreambleInclusions, -
[PATCH] D48762: [clangd] codeComplete returns more structured completion items, LSP. NFC.
sammccall added inline comments. Comment at: clangd/CodeComplete.cpp:259 + : ASTCtx(ASTCtx), ExtractDocumentation(Opts.IncludeComments) { +if (C.SemaResult) { + Completion.Name = llvm::StringRef(SemaCCS->getTypedText()); ioeric wrote: > nit: shall we keep `assert(bool(SemaResult) == bool(SemaCCS));`? Done (in `add()`, and hoisted the call to `add()` to the top here) Comment at: clangd/CodeComplete.cpp:322 +} +if (ExtractDocumentation && Completion.Documentation.empty()) { + if (C.IndexResult && C.IndexResult->Detail) ioeric wrote: > nit: the documentation for the bundling says `Documentation may contain a > combined docstring from all comments`. The implementation seems to take the > documentation from the last comment (which seems fine to me), but they should > be consistent. This was meant to be ambiguous ("may") to allow a better implementation later. But rewrote it to mention the current behavior as a possibility :-) Comment at: clangd/CodeComplete.cpp:1145 +Scores.ExcludingName = Relevance.NameMatch + ? Scores.Total / Relevance.NameMatch + : Scores.Quality; ioeric wrote: > nit: this seems to assume that `NameMatch` is a factor of the final score, > which seems to be changeable given the abstraction of > `evaluateSymbolAndRelevance`. Not sure if there is an alternative to the > current approach, but this probably deserves a comment. Yeah, client-side filtering is the great abstraction-breaker :-( Added the comment, not sure there's much more to be done. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48762: [clangd] codeComplete returns more structured completion items, LSP. NFC.
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. lgtm Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48762 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] r335980 - [clangd] codeComplete returns more structured completion items, LSP. NFC.
Author: sammccall Date: Fri Jun 29 07:47:57 2018 New Revision: 335980 URL: http://llvm.org/viewvc/llvm-project?rev=335980&view=rev Log: [clangd] codeComplete returns more structured completion items, LSP. NFC. Summary: LSP has some presentational fields with limited semantics (e.g. 'detail') and doesn't provide a good place to return information like namespace. Some places where more detailed information is useful: - tools like quality analysis - alternate frontends that aren't strictly LSP - code completion unit tests In this patch, ClangdServer::codeComplete still return LSP CompletionList, but I plan to switch that soon (should be a no-op for ClangdLSPServer). Deferring this makes it clear that we don't change behavior (tests stay the same) and also keeps the API break to a small patch which can be reverted. Reviewers: ioeric Subscribers: ilya-biryukov, MaskRay, cfe-commits, jkorous Differential Revision: https://reviews.llvm.org/D48762 Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp clang-tools-extra/trunk/clangd/CodeComplete.cpp clang-tools-extra/trunk/clangd/CodeComplete.h clang-tools-extra/trunk/clangd/Protocol.h Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=335980&r1=335979&r2=335980&view=diff == --- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Fri Jun 29 07:47:57 2018 @@ -165,11 +165,15 @@ void ClangdServer::codeComplete(PathRef // FIXME(ibiryukov): even if Preamble is non-null, we may want to check // both the old and the new version in case only one of them matches. -CompletionList Result = clangd::codeComplete( +CodeCompleteResult Result = clangd::codeComplete( File, IP->Command, PreambleData ? &PreambleData->Preamble : nullptr, PreambleData ? PreambleData->Inclusions : std::vector(), IP->Contents, Pos, FS, PCHs, CodeCompleteOpts); -CB(std::move(Result)); +CompletionList LSPResult; +LSPResult.isIncomplete = Result.HasMore; +for (const auto &Completion : Result.Completions) + LSPResult.items.push_back(Completion.render(CodeCompleteOpts)); +CB(std::move(LSPResult)); }; WorkScheduler.runWithPreamble("CodeComplete", File, Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=335980&r1=335979&r2=335980&view=diff == --- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original) +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Fri Jun 29 07:47:57 2018 @@ -43,7 +43,7 @@ #include // We log detailed candidate here if you run with -debug-only=codecomplete. -#define DEBUG_TYPE "codecomplete" +#define DEBUG_TYPE "CodeComplete" namespace clang { namespace clangd { @@ -237,138 +237,154 @@ struct CompletionCandidate { return IndexResult->Detail->IncludeHeader; } - // Builds an LSP completion item. - CompletionItem build(StringRef FileName, const CompletionItemScores &Scores, - const CodeCompleteOptions &Opts, - CodeCompletionString *SemaCCS, - const IncludeInserter &Includes, - llvm::StringRef SemaDocComment) const { -assert(bool(SemaResult) == bool(SemaCCS)); -assert(SemaResult || IndexResult); - -CompletionItem I; -bool InsertingInclude = false; // Whether a new #include will be added. -if (SemaResult) { - llvm::StringRef Name(SemaCCS->getTypedText()); - std::string Signature, SnippetSuffix, Qualifiers; - getSignature(*SemaCCS, &Signature, &SnippetSuffix, &Qualifiers); - I.label = (Qualifiers + Name + Signature).str(); - I.filterText = Name; - I.insertText = (Qualifiers + Name).str(); - if (Opts.EnableSnippets) -I.insertText += SnippetSuffix; - I.documentation = formatDocumentation(*SemaCCS, SemaDocComment); - I.detail = getReturnType(*SemaCCS); - if (SemaResult->Kind == CodeCompletionResult::RK_Declaration) -if (const auto *D = SemaResult->getDeclaration()) - if (const auto *ND = llvm::dyn_cast(D)) -I.SymbolScope = splitQualifiedName(printQualifiedName(*ND)).first; - I.kind = toCompletionItemKind(SemaResult->Kind, SemaResult->Declaration); -} -if (IndexResult) { - if (I.SymbolScope.empty()) -I.SymbolScope = IndexResult->Scope; - if (I.kind == CompletionItemKind::Missing) -I.kind = toCompletionItemKind(IndexResult->SymInfo.Kind); - // FIXME: reintroduce a way to show the index source for debugging. - if (I.label.empty()) -I.label = (IndexResult->Name + IndexResult->Signature).str(); - if (I.filterText
[PATCH] D48762: [clangd] codeComplete returns more structured completion items, LSP. NFC.
This revision was automatically updated to reflect the committed changes. Closed by commit rCTE335980: [clangd] codeComplete returns more structured completion items, LSP. NFC. (authored by sammccall, committed by ). Changed prior to commit: https://reviews.llvm.org/D48762?vs=153493&id=153495#toc Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48762 Files: clangd/ClangdServer.cpp clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/Protocol.h Index: clangd/ClangdServer.cpp === --- clangd/ClangdServer.cpp +++ clangd/ClangdServer.cpp @@ -165,11 +165,15 @@ // FIXME(ibiryukov): even if Preamble is non-null, we may want to check // both the old and the new version in case only one of them matches. -CompletionList Result = clangd::codeComplete( +CodeCompleteResult Result = clangd::codeComplete( File, IP->Command, PreambleData ? &PreambleData->Preamble : nullptr, PreambleData ? PreambleData->Inclusions : std::vector(), IP->Contents, Pos, FS, PCHs, CodeCompleteOpts); -CB(std::move(Result)); +CompletionList LSPResult; +LSPResult.isIncomplete = Result.HasMore; +for (const auto &Completion : Result.Completions) + LSPResult.items.push_back(Completion.render(CodeCompleteOpts)); +CB(std::move(LSPResult)); }; WorkScheduler.runWithPreamble("CodeComplete", File, Index: clangd/Protocol.h === --- clangd/Protocol.h +++ clangd/Protocol.h @@ -668,20 +668,6 @@ Snippet = 2, }; -/// Provides details for how a completion item was scored. -/// This can be used for client-side filtering of completion items as the -/// user keeps typing. -/// This is a clangd extension. -struct CompletionItemScores { - /// The score that items are ranked by. - /// This is filterScore * symbolScore. - float finalScore = 0.f; - /// How the partial identifier matched filterText. [0-1] - float filterScore = 0.f; - /// How the symbol fits, ignoring the partial identifier. - float symbolScore = 0.f; -}; - struct CompletionItem { /// The label of this completion item. By default also the text that is /// inserted when selecting this completion. @@ -702,9 +688,6 @@ /// When `falsy` the label is used. std::string sortText; - /// Details about the quality of this completion item. (clangd extension) - llvm::Optional scoreInfo; - /// A string that should be used when filtering a set of completion items. /// When `falsy` the label is used. std::string filterText; Index: clangd/CodeComplete.cpp === --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -43,7 +43,7 @@ #include // We log detailed candidate here if you run with -debug-only=codecomplete. -#define DEBUG_TYPE "codecomplete" +#define DEBUG_TYPE "CodeComplete" namespace clang { namespace clangd { @@ -237,138 +237,154 @@ return IndexResult->Detail->IncludeHeader; } - // Builds an LSP completion item. - CompletionItem build(StringRef FileName, const CompletionItemScores &Scores, - const CodeCompleteOptions &Opts, - CodeCompletionString *SemaCCS, - const IncludeInserter &Includes, - llvm::StringRef SemaDocComment) const { -assert(bool(SemaResult) == bool(SemaCCS)); -assert(SemaResult || IndexResult); - -CompletionItem I; -bool InsertingInclude = false; // Whether a new #include will be added. -if (SemaResult) { - llvm::StringRef Name(SemaCCS->getTypedText()); - std::string Signature, SnippetSuffix, Qualifiers; - getSignature(*SemaCCS, &Signature, &SnippetSuffix, &Qualifiers); - I.label = (Qualifiers + Name + Signature).str(); - I.filterText = Name; - I.insertText = (Qualifiers + Name).str(); - if (Opts.EnableSnippets) -I.insertText += SnippetSuffix; - I.documentation = formatDocumentation(*SemaCCS, SemaDocComment); - I.detail = getReturnType(*SemaCCS); - if (SemaResult->Kind == CodeCompletionResult::RK_Declaration) -if (const auto *D = SemaResult->getDeclaration()) - if (const auto *ND = llvm::dyn_cast(D)) -I.SymbolScope = splitQualifiedName(printQualifiedName(*ND)).first; - I.kind = toCompletionItemKind(SemaResult->Kind, SemaResult->Declaration); -} -if (IndexResult) { - if (I.SymbolScope.empty()) -I.SymbolScope = IndexResult->Scope; - if (I.kind == CompletionItemKind::Missing) -I.kind = toCompletionItemKind(IndexResult->SymInfo.Kind); - // FIXME: reintroduce a way to show the index source for debugging. - if (I.label.empty()) -I.label = (IndexResult->Name + IndexResult->Signature).str(); - if (I.filterText.empty()) -I.filterText = IndexResult->Name; - - // FIXME(ioeric): support ins
[PATCH] D48773: [ASTImporter] Fix import of objects with anonymous types
martong created this revision. martong added reviewers: a.sidorin, balazske, r.stahl. Herald added subscribers: cfe-commits, dkrupp, rnkovacs. Currently, anonymous types are merged into the same redecl chain even if they are structurally inequivalent. This results that global objects are not imported, if there are at least two global objects with different anonymous types. This patch provides a fix. Repository: rC Clang https://reviews.llvm.org/D48773 Files: lib/AST/ASTImporter.cpp unittests/AST/ASTImporterTest.cpp Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1622,6 +1622,35 @@ .match(ToTU, classTemplateSpecializationDecl())); } +TEST_P(ASTImporterTestBase, ObjectsWithUnnamedStructType) { + Decl *FromTU = getTuDecl( + R"( + struct { int a; int b; } object0 = { 2, 3 }; + struct { int x; int y; int z; } object1; + )", + Lang_CXX, "input0.cc"); + + auto getRecordDecl = [](VarDecl *VD) { +auto *ET = cast(VD->getType().getTypePtr()); +return cast(ET->getNamedType().getTypePtr())->getDecl(); + }; + + auto *Obj0 = + FirstDeclMatcher().match(FromTU, varDecl(hasName("object0"))); + auto *From0 = getRecordDecl(Obj0); + auto *Obj1 = + FirstDeclMatcher().match(FromTU, varDecl(hasName("object1"))); + auto *From1 = getRecordDecl(Obj1); + + auto *To0 = Import(From0, Lang_CXX); + auto *To1 = Import(From1, Lang_CXX); + + EXPECT_TRUE(To0); + EXPECT_TRUE(To1); + EXPECT_NE(To0, To1); + EXPECT_NE(To0->getCanonicalDecl(), To1->getCanonicalDecl()); +} + struct ImportFunctions : ASTImporterTestBase {}; TEST_P(ImportFunctions, Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -2082,6 +2082,9 @@ if (*Index1 != *Index2) continue; } + } else { +if (!IsStructuralMatch(D, FoundRecord, false)) + continue; } } Index: unittests/AST/ASTImporterTest.cpp === --- unittests/AST/ASTImporterTest.cpp +++ unittests/AST/ASTImporterTest.cpp @@ -1622,6 +1622,35 @@ .match(ToTU, classTemplateSpecializationDecl())); } +TEST_P(ASTImporterTestBase, ObjectsWithUnnamedStructType) { + Decl *FromTU = getTuDecl( + R"( + struct { int a; int b; } object0 = { 2, 3 }; + struct { int x; int y; int z; } object1; + )", + Lang_CXX, "input0.cc"); + + auto getRecordDecl = [](VarDecl *VD) { +auto *ET = cast(VD->getType().getTypePtr()); +return cast(ET->getNamedType().getTypePtr())->getDecl(); + }; + + auto *Obj0 = + FirstDeclMatcher().match(FromTU, varDecl(hasName("object0"))); + auto *From0 = getRecordDecl(Obj0); + auto *Obj1 = + FirstDeclMatcher().match(FromTU, varDecl(hasName("object1"))); + auto *From1 = getRecordDecl(Obj1); + + auto *To0 = Import(From0, Lang_CXX); + auto *To1 = Import(From1, Lang_CXX); + + EXPECT_TRUE(To0); + EXPECT_TRUE(To1); + EXPECT_NE(To0, To1); + EXPECT_NE(To0->getCanonicalDecl(), To1->getCanonicalDecl()); +} + struct ImportFunctions : ASTImporterTestBase {}; TEST_P(ImportFunctions, Index: lib/AST/ASTImporter.cpp === --- lib/AST/ASTImporter.cpp +++ lib/AST/ASTImporter.cpp @@ -2082,6 +2082,9 @@ if (*Index1 != *Index2) continue; } + } else { +if (!IsStructuralMatch(D, FoundRecord, false)) + continue; } } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r335983 - [clang-format/ObjC] Fix NS_SWIFT_NAME(foo(bar:baz:)) after ObjC method decl
Author: benhamilton Date: Fri Jun 29 08:26:37 2018 New Revision: 335983 URL: http://llvm.org/viewvc/llvm-project?rev=335983&view=rev Log: [clang-format/ObjC] Fix NS_SWIFT_NAME(foo(bar:baz:)) after ObjC method decl Summary: In D44638, I partially fixed `NS_SWIFT_NAME(foo(bar:baz:))`-style annotations on C functions, but didn't add a test for Objective-C method declarations. For ObjC method declarations which are annotated with `NS_SWIFT_NAME(...)`, we currently fail to annotate the final component of the selector name as `TT_SelectorName`. Because the token type is left unknown, clang-format will happily cause a compilation error when it changes the following: ``` @interface Foo - (void)doStuffWithFoo:(id)name bar:(id)bar baz:(id)baz NS_SWIFT_NAME(doStuff(withFoo:bar:baz:)); @end ``` to: ``` @interface Foo - (void)doStuffWithFoo:(id)name bar:(id)bar baz:(id)baz NS_SWIFT_NAME(doStuff(withFoo:bar:baz :)); @end ``` (note the linebreak before the final `:`). The logic which decides whether or not to annotate the token before a `:` with `TT_SelectorName` is pretty fragile, and has to handle some pretty odd cases like pair-parameters: ``` [I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd]; ``` So, to minimize the effect of this change, I decided to only annotate unknown identifiers before a `:` as `TT_SelectorName` for Objective-C declaration lines. Test Plan: New tests included. Confirmed tests failed before change and passed after change. Ran tests with: % make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests Reviewers: djasper, krasimir, jolesiak Reviewed By: krasimir Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D48679 Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp cfe/trunk/unittests/Format/FormatTestObjC.cpp Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=335983&r1=335982&r2=335983&view=diff == --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original) +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Jun 29 08:26:37 2018 @@ -698,13 +698,19 @@ private: Line.startsWith(TT_ObjCMethodSpecifier)) { Tok->Type = TT_ObjCMethodExpr; const FormatToken *BeforePrevious = Tok->Previous->Previous; +// Ensure we tag all identifiers in method declarations as +// TT_SelectorName. +bool UnknownIdentifierInMethodDeclaration = +Line.startsWith(TT_ObjCMethodSpecifier) && +Tok->Previous->is(tok::identifier) && Tok->Previous->is(TT_Unknown); if (!BeforePrevious || // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen. !(BeforePrevious->is(TT_CastRParen) || (BeforePrevious->is(TT_ObjCMethodExpr) && BeforePrevious->is(tok::colon))) || BeforePrevious->is(tok::r_square) || -Contexts.back().LongestObjCSelectorName == 0) { +Contexts.back().LongestObjCSelectorName == 0 || +UnknownIdentifierInMethodDeclaration) { Tok->Previous->Type = TT_SelectorName; if (!Contexts.back().FirstObjCSelectorName) Contexts.back().FirstObjCSelectorName = Tok->Previous; Modified: cfe/trunk/unittests/Format/FormatTestObjC.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestObjC.cpp?rev=335983&r1=335982&r2=335983&view=diff == --- cfe/trunk/unittests/Format/FormatTestObjC.cpp (original) +++ cfe/trunk/unittests/Format/FormatTestObjC.cpp Fri Jun 29 08:26:37 2018 @@ -923,6 +923,14 @@ TEST_F(FormatTestObjC, ObjCSnippets) { verifyFormat("@property(assign, nonatomic) CGFloat hoverAlpha;"); verifyFormat("@property(assign, getter=isEditable) BOOL editable;"); + Style.ColumnLimit = 50; + verifyFormat("@interface Foo\n" + "- (void)doStuffWithFoo:(id)name\n" + " bar:(id)bar\n" + " baz:(id)baz\n" + "NS_SWIFT_NAME(doStuff(withFoo:bar:baz:));\n" + "@end"); + Style = getMozillaStyle(); verifyFormat("@property (assign, getter=isEditable) BOOL editable;"); verifyFormat("@property BOOL editable;"); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48679: [clang-format/ObjC] Fix NS_SWIFT_NAME(foo(bar:baz:)) after ObjC method decl
This revision was automatically updated to reflect the committed changes. Closed by commit rL335983: [clang-format/ObjC] Fix NS_SWIFT_NAME(foo(bar:baz:)) after ObjC method decl (authored by benhamilton, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D48679 Files: cfe/trunk/lib/Format/TokenAnnotator.cpp cfe/trunk/unittests/Format/FormatTestObjC.cpp Index: cfe/trunk/lib/Format/TokenAnnotator.cpp === --- cfe/trunk/lib/Format/TokenAnnotator.cpp +++ cfe/trunk/lib/Format/TokenAnnotator.cpp @@ -698,13 +698,19 @@ Line.startsWith(TT_ObjCMethodSpecifier)) { Tok->Type = TT_ObjCMethodExpr; const FormatToken *BeforePrevious = Tok->Previous->Previous; +// Ensure we tag all identifiers in method declarations as +// TT_SelectorName. +bool UnknownIdentifierInMethodDeclaration = +Line.startsWith(TT_ObjCMethodSpecifier) && +Tok->Previous->is(tok::identifier) && Tok->Previous->is(TT_Unknown); if (!BeforePrevious || // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen. !(BeforePrevious->is(TT_CastRParen) || (BeforePrevious->is(TT_ObjCMethodExpr) && BeforePrevious->is(tok::colon))) || BeforePrevious->is(tok::r_square) || -Contexts.back().LongestObjCSelectorName == 0) { +Contexts.back().LongestObjCSelectorName == 0 || +UnknownIdentifierInMethodDeclaration) { Tok->Previous->Type = TT_SelectorName; if (!Contexts.back().FirstObjCSelectorName) Contexts.back().FirstObjCSelectorName = Tok->Previous; Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp === --- cfe/trunk/unittests/Format/FormatTestObjC.cpp +++ cfe/trunk/unittests/Format/FormatTestObjC.cpp @@ -923,6 +923,14 @@ verifyFormat("@property(assign, nonatomic) CGFloat hoverAlpha;"); verifyFormat("@property(assign, getter=isEditable) BOOL editable;"); + Style.ColumnLimit = 50; + verifyFormat("@interface Foo\n" + "- (void)doStuffWithFoo:(id)name\n" + " bar:(id)bar\n" + " baz:(id)baz\n" + "NS_SWIFT_NAME(doStuff(withFoo:bar:baz:));\n" + "@end"); + Style = getMozillaStyle(); verifyFormat("@property (assign, getter=isEditable) BOOL editable;"); verifyFormat("@property BOOL editable;"); Index: cfe/trunk/lib/Format/TokenAnnotator.cpp === --- cfe/trunk/lib/Format/TokenAnnotator.cpp +++ cfe/trunk/lib/Format/TokenAnnotator.cpp @@ -698,13 +698,19 @@ Line.startsWith(TT_ObjCMethodSpecifier)) { Tok->Type = TT_ObjCMethodExpr; const FormatToken *BeforePrevious = Tok->Previous->Previous; +// Ensure we tag all identifiers in method declarations as +// TT_SelectorName. +bool UnknownIdentifierInMethodDeclaration = +Line.startsWith(TT_ObjCMethodSpecifier) && +Tok->Previous->is(tok::identifier) && Tok->Previous->is(TT_Unknown); if (!BeforePrevious || // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen. !(BeforePrevious->is(TT_CastRParen) || (BeforePrevious->is(TT_ObjCMethodExpr) && BeforePrevious->is(tok::colon))) || BeforePrevious->is(tok::r_square) || -Contexts.back().LongestObjCSelectorName == 0) { +Contexts.back().LongestObjCSelectorName == 0 || +UnknownIdentifierInMethodDeclaration) { Tok->Previous->Type = TT_SelectorName; if (!Contexts.back().FirstObjCSelectorName) Contexts.back().FirstObjCSelectorName = Tok->Previous; Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp === --- cfe/trunk/unittests/Format/FormatTestObjC.cpp +++ cfe/trunk/unittests/Format/FormatTestObjC.cpp @@ -923,6 +923,14 @@ verifyFormat("@property(assign, nonatomic) CGFloat hoverAlpha;"); verifyFormat("@property(assign, getter=isEditable) BOOL editable;"); + Style.ColumnLimit = 50; + verifyFormat("@interface Foo\n" + "- (void)doStuffWithFoo:(id)name\n" + " bar:(id)bar\n" + " baz:(id)baz\n" + "NS_SWIFT_NAME(doStuff(withFoo:bar:baz:));\n" + "@end"); + Style = getMozillaStyle(); verifyFormat("@property (assign, getter=isEditable) BOOL editable;"); verifyFormat("@property BOOL editable;"); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48721: Patch to fix pragma metadata for do-while loops
deepak2427 added a comment. Do I need to add specific reviewers? Repository: rC Clang https://reviews.llvm.org/D48721 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47632: [ASTImporter] Refactor Decl creation
martong added a comment. I realized that, this patch brakes 3 lldb tests ATM: - `TestTopLevelExprs.py`. If https://reviews.llvm.org/D48722 was merged then this test would not be broken. - `TestPersistentTypes.py` If https://reviews.llvm.org/D48773 was merged then this test would not be broken. - `TestRecursiveTypes.py`. I am still working on this. The newly introduced assert fires: `Assertion `(Pos == ImportedDecls.end() || Pos->second == To) && "Try to import an already imported Decl"' failed.`. Repository: rC Clang https://reviews.llvm.org/D47632 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48441: [clangd] Incorporate transitive #includes into code complete proximity scoring.
sammccall updated this revision to Diff 153504. sammccall marked 2 inline comments as done. sammccall added a comment. Address review comments. Most notably: limit the up-traversals from some sources. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D48441 Files: clangd/CMakeLists.txt clangd/ClangdServer.cpp clangd/ClangdUnit.cpp clangd/ClangdUnit.h clangd/CodeComplete.cpp clangd/CodeComplete.h clangd/FileDistance.cpp clangd/FileDistance.h clangd/Headers.cpp clangd/Headers.h clangd/Quality.cpp clangd/Quality.h clangd/XRefs.cpp unittests/clangd/CMakeLists.txt unittests/clangd/FileDistanceTests.cpp unittests/clangd/HeadersTests.cpp unittests/clangd/QualityTests.cpp Index: unittests/clangd/QualityTests.cpp === --- unittests/clangd/QualityTests.cpp +++ unittests/clangd/QualityTests.cpp @@ -17,6 +17,7 @@ // //===--===// +#include "FileDistance.h" #include "Quality.h" #include "TestFS.h" #include "TestTU.h" @@ -157,9 +158,22 @@ PoorNameMatch.NameMatch = 0.2f; EXPECT_LT(PoorNameMatch.evaluate(), Default.evaluate()); - SymbolRelevanceSignals WithProximity; - WithProximity.SemaProximityScore = 0.2f; - EXPECT_GT(WithProximity.evaluate(), Default.evaluate()); + SymbolRelevanceSignals WithSemaProximity; + WithSemaProximity.SemaProximityScore = 0.2f; + EXPECT_GT(WithSemaProximity.evaluate(), Default.evaluate()); + + SymbolRelevanceSignals IndexProximate; + IndexProximate.SymbolURI = "unittest:/foo/bar.h"; + llvm::StringMap ProxSources; + ProxSources.try_emplace(testPath("foo/baz.h")); + URIDistance Distance(ProxSources); + IndexProximate.FileProximityMatch = &Distance; + EXPECT_GT(IndexProximate.evaluate(), Default.evaluate()); + SymbolRelevanceSignals IndexDistant = IndexProximate; + IndexDistant.SymbolURI = "unittest:/elsewhere/path.h"; + EXPECT_GT(IndexProximate.evaluate(), IndexDistant.evaluate()) + << IndexProximate << IndexDistant; + EXPECT_GT(IndexDistant.evaluate(), Default.evaluate()); SymbolRelevanceSignals Scoped; Scoped.Scope = SymbolRelevanceSignals::FileScope; @@ -180,59 +194,6 @@ EXPECT_LT(sortText(0, "a"), sortText(0, "z")); } -// {a,b,c} becomes /clangd-test/a/b/c -std::string joinPaths(llvm::ArrayRef Parts) { - return testPath( - llvm::join(Parts.begin(), Parts.end(), llvm::sys::path::get_separator())); -} - -static constexpr float ProximityBase = 0.7; - -// Calculates a proximity score for an index symbol with declaration file -// SymPath with the given URI scheme. -float URIProximity(const FileProximityMatcher &Matcher, StringRef SymPath, - StringRef Scheme = "file") { - auto U = URI::create(SymPath, Scheme); - EXPECT_TRUE(static_cast(U)) << llvm::toString(U.takeError()); - return Matcher.uriProximity(U->toString()); -} - -TEST(QualityTests, URIProximityScores) { - FileProximityMatcher Matcher( - /*ProximityPaths=*/{joinPaths({"a", "b", "c", "d", "x"})}); - - EXPECT_FLOAT_EQ(URIProximity(Matcher, joinPaths({"a", "b", "c", "d", "x"})), - 1); - EXPECT_FLOAT_EQ(URIProximity(Matcher, joinPaths({"a", "b", "c", "d", "y"})), - ProximityBase); - EXPECT_FLOAT_EQ(URIProximity(Matcher, joinPaths({"a", "y", "z"})), - std::pow(ProximityBase, 5)); - EXPECT_FLOAT_EQ( - URIProximity(Matcher, joinPaths({"a", "b", "c", "d", "e", "y"})), - std::pow(ProximityBase, 2)); - EXPECT_FLOAT_EQ( - URIProximity(Matcher, joinPaths({"a", "b", "m", "n", "o", "y"})), - std::pow(ProximityBase, 5)); - EXPECT_FLOAT_EQ( - URIProximity(Matcher, joinPaths({"a", "t", "m", "n", "o", "y"})), - std::pow(ProximityBase, 6)); - // Note the common directory is /clang-test/ - EXPECT_FLOAT_EQ(URIProximity(Matcher, joinPaths({"m", "n", "o", "p", "y"})), - std::pow(ProximityBase, 7)); -} - -TEST(QualityTests, URIProximityScoresWithTestURI) { - FileProximityMatcher Matcher( - /*ProximityPaths=*/{joinPaths({"b", "c", "x"})}); - EXPECT_FLOAT_EQ(URIProximity(Matcher, joinPaths({"b", "c", "x"}), "unittest"), - 1); - EXPECT_FLOAT_EQ(URIProximity(Matcher, joinPaths({"b", "y"}), "unittest"), - std::pow(ProximityBase, 2)); - // unittest:///b/c/x vs unittest:///m/n/y. No common directory. - EXPECT_FLOAT_EQ(URIProximity(Matcher, joinPaths({"m", "n", "y"}), "unittest"), - std::pow(ProximityBase, 4)); -} - } // namespace } // namespace clangd } // namespace clang Index: unittests/clangd/HeadersTests.cpp === --- unittests/clangd/HeadersTests.cpp +++ unittests/clangd/HeadersTests.cpp @@ -64,18 +64,17 @@ } protected: - std::vector collectIncludes() { + IncludeStructure collectIncludes() { auto Clang = setupClang(); PreprocessOnlyAction Action; EX
[PATCH] D48712: [X86] Lowering integer truncation intrinsics to native IR
mike.dvoretsky added inline comments. Comment at: clang/lib/Headers/avx512vlbwintrin.h:1501 + (__v8qi){0, 0, 0, 0, 0, 0, 0, 0}, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, + 12, 13, 14, 15); } RKSimon wrote: > Are we happy with using illegal types like this? What about flipping the > shuffle and convert? > > ``` > return (__m128i)__builtin_convertvector( > __builtin_shufflevector((__v8hi)__A, > (__v8hi){0, 0, 0, 0, 0, 0, 0, 0}, > 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, > 11, 12, 13, 14, 15), __v16qi); > ``` This would bring its own issues, since in the cvtepi64_epi8 cases the inner shuffle would produce vectors of 16 64-bit values. There would be no extra typedef, but in the back-end these would be split in type legalization, making it harder to fold them into VPMOV instructions. https://reviews.llvm.org/D48712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct
paulsemel updated this revision to Diff 153511. paulsemel added a comment. Fixed version problem. Now building. Repository: rC Clang https://reviews.llvm.org/D47953 Files: lib/CodeGen/CGBuiltin.cpp Index: lib/CodeGen/CGBuiltin.cpp === --- lib/CodeGen/CGBuiltin.cpp +++ lib/CodeGen/CGBuiltin.cpp @@ -1170,6 +1170,7 @@ RecordDecl *RD = RT->getDecl()->getDefinition(); ASTContext &Ctx = RD->getASTContext(); const ASTRecordLayout &RL = Ctx.getASTRecordLayout(RD); + const CGRecordLayout &CGRL = CGF.getTypes().getCGRecordLayout(RD); std::string Pad = std::string(Lvl * 4, ' '); Value *GString = @@ -1208,7 +1209,7 @@ FieldPtr, CGF.ConvertType(Context.getPointerType(FD->getType(; else FieldPtr = CGF.Builder.CreateStructGEP(CGF.ConvertType(RType), FieldPtr, - FD->getFieldIndex()); + CGRL.getLLVMFieldNo(FD)); GString = CGF.Builder.CreateGlobalStringPtr( llvm::Twine(Pad) @@ -1236,10 +1237,37 @@ ? Types[Context.VoidPtrTy] : Types[CanonicalType]; +if (FD->isBitField()) + FieldPtr = CGF.Builder.CreatePointerCast( + FieldPtr, CGF.ConvertType(Context.getPointerType(CanonicalType))); + Address FieldAddress = Address(FieldPtr, Align); FieldPtr = CGF.Builder.CreateLoad(FieldAddress); -// FIXME Need to handle bitfield here +if (FD->isBitField()) { + const CGBitFieldInfo &Info = CGRL.getBitFieldInfo(FD); + if (Info.IsSigned) { +unsigned HighBits = Info.StorageSize - Info.Offset - Info.Size; +if (HighBits) + FieldPtr = CGF.Builder.CreateShl(FieldPtr, HighBits); +if (Info.Offset + HighBits) + FieldPtr = CGF.Builder.CreateAShr(FieldPtr, Info.Offset + HighBits); + } else { +if (Info.Offset) + FieldPtr = CGF.Builder.CreateLShr(FieldPtr, Info.Offset); +if (static_cast(Info.Offset) + Info.Size < Info.StorageSize) + FieldPtr = CGF.Builder.CreateAnd( + FieldPtr, + ConstantInt::get(CGF.ConvertType(CanonicalType), + llvm::APInt::getLowBitsSet( + CGF.CGM.getDataLayout().getTypeSizeInBits( + CGF.ConvertType(CanonicalType)), + Info.Size))); + } + FieldPtr = CGF.Builder.CreateIntCast( + FieldPtr, CGF.ConvertType(CanonicalType), Info.IsSigned); +} + GString = CGF.Builder.CreateGlobalStringPtr( Format.concat(llvm::Twine('\n')).str()); TmpRes = CGF.Builder.CreateCall(Func, {GString, FieldPtr}); Index: lib/CodeGen/CGBuiltin.cpp === --- lib/CodeGen/CGBuiltin.cpp +++ lib/CodeGen/CGBuiltin.cpp @@ -1170,6 +1170,7 @@ RecordDecl *RD = RT->getDecl()->getDefinition(); ASTContext &Ctx = RD->getASTContext(); const ASTRecordLayout &RL = Ctx.getASTRecordLayout(RD); + const CGRecordLayout &CGRL = CGF.getTypes().getCGRecordLayout(RD); std::string Pad = std::string(Lvl * 4, ' '); Value *GString = @@ -1208,7 +1209,7 @@ FieldPtr, CGF.ConvertType(Context.getPointerType(FD->getType(; else FieldPtr = CGF.Builder.CreateStructGEP(CGF.ConvertType(RType), FieldPtr, - FD->getFieldIndex()); + CGRL.getLLVMFieldNo(FD)); GString = CGF.Builder.CreateGlobalStringPtr( llvm::Twine(Pad) @@ -1236,10 +1237,37 @@ ? Types[Context.VoidPtrTy] : Types[CanonicalType]; +if (FD->isBitField()) + FieldPtr = CGF.Builder.CreatePointerCast( + FieldPtr, CGF.ConvertType(Context.getPointerType(CanonicalType))); + Address FieldAddress = Address(FieldPtr, Align); FieldPtr = CGF.Builder.CreateLoad(FieldAddress); -// FIXME Need to handle bitfield here +if (FD->isBitField()) { + const CGBitFieldInfo &Info = CGRL.getBitFieldInfo(FD); + if (Info.IsSigned) { +unsigned HighBits = Info.StorageSize - Info.Offset - Info.Size; +if (HighBits) + FieldPtr = CGF.Builder.CreateShl(FieldPtr, HighBits); +if (Info.Offset + HighBits) + FieldPtr = CGF.Builder.CreateAShr(FieldPtr, Info.Offset + HighBits); + } else { +if (Info.Offset) + FieldPtr = CGF.Builder.CreateLShr(FieldPtr, Info.Offset); +if (static_cast(Info.Offset) + Info.Size < Info.StorageSize) + FieldPtr = CGF.Builder.CreateAnd( + FieldPtr, + ConstantInt::get(CGF.ConvertType(CanonicalType), + llvm::APInt::getLowBitsSet( + CGF.CGM.getDataLayout().getTypeSizeInBits( +
[PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct
paulsemel added a comment. In https://reviews.llvm.org/D47953#1143044, @dblaikie wrote: > This doesn't seem to build for me - so hard to debug/probe it: > > llvm/src/tools/clang/lib/CodeGen/CGBuiltin.cpp:1264:65: error: no viable > conversion from 'clang::QualType' to 'llvm::Type *' > > CGF.CGM.getDataLayout().getTypeSizeInBits(CanonicalType), > ^ > > llvm/src/include/llvm/IR/DataLayout.h:560:53: note: passing argument to > parameter 'Ty' here > inline uint64_t DataLayout::getTypeSizeInBits(Type *Ty) const { > > ^ > > 1 error generated. Very sorry about it, I should have paid more attention.. Repository: rC Clang https://reviews.llvm.org/D47953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47297: [Modules][ObjC] Add protocol redefinition to the current scope/context
bruno added a comment. Herald added a subscriber: dexonsmith. Ping! Repository: rC Clang https://reviews.llvm.org/D47297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r335993 - [Fixed Point Arithmetic] Rename `-fsame-fbits` flag
Author: leonardchan Date: Fri Jun 29 10:08:19 2018 New Revision: 335993 URL: http://llvm.org/viewvc/llvm-project?rev=335993&view=rev Log: [Fixed Point Arithmetic] Rename `-fsame-fbits` flag - Rename the `-fsame-fbits` flag to `-fpadding-on-unsigned-fixed-point` - Move the flag from a driver option to a cc1 option - Rename the `SameFBits` member in TargetInfo to `PaddingOnUnsignedFixedPoint` - Updated descriptions Differential Revision: https://reviews.llvm.org/D48727 Modified: cfe/trunk/include/clang/Basic/LangOptions.def cfe/trunk/include/clang/Basic/TargetInfo.h cfe/trunk/include/clang/Driver/CC1Options.td cfe/trunk/include/clang/Driver/Options.td cfe/trunk/lib/Basic/TargetInfo.cpp cfe/trunk/lib/Driver/ToolChains/Clang.cpp cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/test/Frontend/fixed_point_same_fbits.c Modified: cfe/trunk/include/clang/Basic/LangOptions.def URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=335993&r1=335992&r2=335993&view=diff == --- cfe/trunk/include/clang/Basic/LangOptions.def (original) +++ cfe/trunk/include/clang/Basic/LangOptions.def Fri Jun 29 10:08:19 2018 @@ -306,8 +306,8 @@ ENUM_LANGOPT(ClangABICompat, ClangABI, 4 COMPATIBLE_VALUE_LANGOPT(FunctionAlignment, 5, 0, "Default alignment for functions") LANGOPT(FixedPoint, 1, 0, "fixed point types") -LANGOPT(SameFBits, 1, 0, -"unsigned and signed fixed point type having the same number of fractional bits") +LANGOPT(PaddingOnUnsignedFixedPoint, 1, 0, +"unsigned fixed point types having one extra padding bit") #undef LANGOPT #undef COMPATIBLE_LANGOPT Modified: cfe/trunk/include/clang/Basic/TargetInfo.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=335993&r1=335992&r2=335993&view=diff == --- cfe/trunk/include/clang/Basic/TargetInfo.h (original) +++ cfe/trunk/include/clang/Basic/TargetInfo.h Fri Jun 29 10:08:19 2018 @@ -84,10 +84,11 @@ protected: unsigned char LongFractWidth, LongFractAlign; // If true, unsigned fixed point types have the same number of fractional bits - // as their signed counterparts. Otherwise, unsigned fixed point types have + // as their signed counterparts, forcing the unsigned types to have one extra + // bit of padding. Otherwise, unsigned fixed point types have // one more fractional bit than its corresponding signed type. This is false // by default. - bool SameFBits; + bool PaddingOnUnsignedFixedPoint; // Fixed point integral and fractional bit sizes // Saturated types share the same integral/fractional bits as their @@ -95,7 +96,7 @@ protected: // For simplicity, the fractional bits in a _Fract type will be one less the // width of that _Fract type. This leaves all signed _Fract types having no // padding and unsigned _Fract types will only have 1 bit of padding after the - // sign if SameFBits is set. + // sign if PaddingOnUnsignedFixedPoint is set. unsigned char ShortAccumScale; unsigned char AccumScale; unsigned char LongAccumScale; @@ -436,30 +437,33 @@ public: /// getUnsignedShortAccumScale/IBits - Return the number of /// fractional/integral bits in a 'unsigned short _Accum' type. unsigned getUnsignedShortAccumScale() const { -return SameFBits ? ShortAccumScale : ShortAccumScale + 1; +return PaddingOnUnsignedFixedPoint ? ShortAccumScale : ShortAccumScale + 1; } unsigned getUnsignedShortAccumIBits() const { -return SameFBits ? getShortAccumIBits() - : ShortAccumWidth - getUnsignedShortAccumScale(); +return PaddingOnUnsignedFixedPoint + ? getShortAccumIBits() + : ShortAccumWidth - getUnsignedShortAccumScale(); } /// getUnsignedAccumScale/IBits - Return the number of fractional/integral /// bits in a 'unsigned _Accum' type. unsigned getUnsignedAccumScale() const { -return SameFBits ? AccumScale : AccumScale + 1; +return PaddingOnUnsignedFixedPoint ? AccumScale : AccumScale + 1; } unsigned getUnsignedAccumIBits() const { -return SameFBits ? getAccumIBits() : AccumWidth - getUnsignedAccumScale(); +return PaddingOnUnsignedFixedPoint ? getAccumIBits() + : AccumWidth - getUnsignedAccumScale(); } /// getUnsignedLongAccumScale/IBits - Return the number of fractional/integral /// bits in a 'unsigned long _Accum' type. unsigned getUnsignedLongAccumScale() const { -return SameFBits ? LongAccumScale : LongAccumScale + 1; +return PaddingOnUnsignedFixedPoint ? LongAccumScale : LongAccumScale + 1; } unsigned getUnsignedLongAccumIBits() const { -return SameFBits ? getLongAccumIBits() - : LongAccumWidth - getUnsignedLongAccumScal
[PATCH] D48727: [Fixed Point Arithmetic] Rename `-fsame-fbits` flag
This revision was automatically updated to reflect the committed changes. Closed by commit rC335993: [Fixed Point Arithmetic] Rename `-fsame-fbits` flag (authored by leonardchan, committed by ). Repository: rC Clang https://reviews.llvm.org/D48727 Files: include/clang/Basic/LangOptions.def include/clang/Basic/TargetInfo.h include/clang/Driver/CC1Options.td include/clang/Driver/Options.td lib/Basic/TargetInfo.cpp lib/Driver/ToolChains/Clang.cpp lib/Frontend/CompilerInvocation.cpp lib/Sema/SemaExpr.cpp test/Frontend/fixed_point_same_fbits.c Index: lib/Basic/TargetInfo.cpp === --- lib/Basic/TargetInfo.cpp +++ lib/Basic/TargetInfo.cpp @@ -53,7 +53,7 @@ // We give the _Accum 1 fewer fractional bits than their corresponding _Fract // types by default to have the same number of fractional bits between _Accum // and _Fract types. - SameFBits = false; + PaddingOnUnsignedFixedPoint = false; ShortAccumScale = 7; AccumScale = 15; LongAccumScale = 31; @@ -377,7 +377,7 @@ // Each unsigned fixed point type has the same number of fractional bits as // its corresponding signed type. - SameFBits |= Opts.SameFBits; + PaddingOnUnsignedFixedPoint |= Opts.PaddingOnUnsignedFixedPoint; CheckFixedPointBits(); } Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -2338,8 +2338,9 @@ Opts.FixedPoint = Args.hasFlag(OPT_ffixed_point, OPT_fno_fixed_point, /*Default=*/false) && !Opts.CPlusPlus; - Opts.SameFBits = - Args.hasFlag(OPT_fsame_fbits, OPT_fno_same_fbits, + Opts.PaddingOnUnsignedFixedPoint = + Args.hasFlag(OPT_fpadding_on_unsigned_fixed_point, + OPT_fno_padding_on_unsigned_fixed_point, /*Default=*/false) && Opts.FixedPoint; Index: lib/Driver/ToolChains/Clang.cpp === --- lib/Driver/ToolChains/Clang.cpp +++ lib/Driver/ToolChains/Clang.cpp @@ -3765,11 +3765,6 @@ /*Default=*/false)) Args.AddLastArg(CmdArgs, options::OPT_ffixed_point); - if (Args.hasFlag(options::OPT_fsame_fbits, - options::OPT_fno_same_fbits, - /*Default=*/false)) -Args.AddLastArg(CmdArgs, options::OPT_fsame_fbits); - // Handle -{std, ansi, trigraphs} -- take the last of -{std, ansi} // (-ansi is equivalent to -std=c89 or -std=c++98). // Index: lib/Sema/SemaExpr.cpp === --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -3358,7 +3358,7 @@ bool Overflowed = Literal.GetFixedPointValue(Val, scale); // Do not use bit_width since some types may have padding like _Fract or -// unsigned _Accums if SameFBits is set. +// unsigned _Accums if PaddingOnUnsignedFixedPoint is set. auto MaxVal = llvm::APInt::getMaxValue(ibits + scale).zextOrSelf(bit_width); if (Literal.isFract && Val == MaxVal + 1) // Clause 6.4.4 - The value of a constant shall be in the range of Index: include/clang/Driver/CC1Options.td === --- include/clang/Driver/CC1Options.td +++ include/clang/Driver/CC1Options.td @@ -36,6 +36,10 @@ def mfpmath : Separate<["-"], "mfpmath">, HelpText<"Which unit to use for fp math">; +def fpadding_on_unsigned_fixed_point : Flag<["-"], "fpadding-on-unsigned-fixed-point">, + HelpText<"Force each unsigned fixed point type to have an extra bit of padding to align their scales with those of signed fixed point types">; +def fno_padding_on_unsigned_fixed_point : Flag<["-"], "fno-padding-on-unsigned-fixed-point">; + //===--===// // Analyzer Options //===--===// Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -893,10 +893,6 @@ Flags<[CC1Option]>, HelpText<"Enable fixed point types">; def fno_fixed_point : Flag<["-"], "fno-fixed-point">, Group, HelpText<"Disable fixed point types">; -def fsame_fbits : Flag<["-"], "fsame-fbits">, Group, - Flags<[CC1Option]>, - HelpText<"Force each unsigned fixed point type to have the same number of fractional bits as its corresponding signed type">; -def fno_same_fbits : Flag<["-"], "fno-same-fbits">, Group; // Begin sanitizer flags. These should all be core options exposed in all driver // modes. Index: include/clang/Basic/TargetInfo.h === --- include/clang/Basic/TargetInfo.h +++ include/clang/Basic/Target
[PATCH] D48712: [X86] Lowering integer truncation intrinsics to native IR
RKSimon added a comment. Please can you create a llvm side parallel patch that updates the relevant fast-isel tests Comment at: clang/lib/Headers/avx512vlbwintrin.h:1501 + (__v8qi){0, 0, 0, 0, 0, 0, 0, 0}, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, + 12, 13, 14, 15); } mike.dvoretsky wrote: > RKSimon wrote: > > Are we happy with using illegal types like this? What about flipping the > > shuffle and convert? > > > > ``` > > return (__m128i)__builtin_convertvector( > > __builtin_shufflevector((__v8hi)__A, > > (__v8hi){0, 0, 0, 0, 0, 0, 0, > > 0}, > > 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, > > 10, 11, 12, 13, 14, 15), __v16qi); > > ``` > This would bring its own issues, since in the cvtepi64_epi8 cases the inner > shuffle would produce vectors of 16 64-bit values. There would be no extra > typedef, but in the back-end these would be split in type legalization, > making it harder to fold them into VPMOV instructions. Yeah, neither solution is particularly clean. Please keep it as is. https://reviews.llvm.org/D48712 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47297: [Modules][ObjC] Add protocol redefinition to the current scope/context
arphaman added inline comments. Comment at: lib/Sema/SemaDeclObjC.cpp:1208 +// serialize something meaningful. +if (getLangOpts().Modules) + PushOnScopeChains(PDecl, TUScope); Is this a problem only for modules or does this show up in PCHs too? What would be the cost of using PushOnScopeChains unconditionally? Repository: rC Clang https://reviews.llvm.org/D47297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48764: [Analyzer] Hotfix for iterator checkers: Mark left-hand side of `SymIntExpr` objects as live in the program state maps.
NoQ added a comment. That's right. You only need to mark "atomic" symbols (`SymbolData`) as live, and expressions that contain them would automatically become live. So i think you should just iterate through a `symbol_iterator` and mark all `SymbolData` symbols you encounter as live. Is this a hotfix for a test failure? Otherwise it'd be great to have tests. https://reviews.llvm.org/D48764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45454: Add llvm_gcov_flush to be called outside a shared library
chh updated this revision to Diff 153532. chh retitled this revision from "Make __gcov_flush visible outside a shared library" to "Add llvm_gcov_flush to be called outside a shared library". chh edited the summary of this revision. chh added a comment. Now keep __gcov_flush hidden as libgcov; add llvm_gcov_flush to call from outside of .so files. https://reviews.llvm.org/D45454 Files: lib/profile/GCDAProfiling.c test/profile/Inputs/instrprof-dlopen-dlclose-main.c Index: test/profile/Inputs/instrprof-dlopen-dlclose-main.c === --- test/profile/Inputs/instrprof-dlopen-dlclose-main.c +++ test/profile/Inputs/instrprof-dlopen-dlclose-main.c @@ -16,6 +16,31 @@ return EXIT_FAILURE; } + void (*gcov_flush)() = (void (*)())dlsym(f1_handle, "__gcov_flush"); + if (gcov_flush != NULL) { +fprintf(stderr, "__gcov_flush should not be visible in func.shared'\n"); +return EXIT_FAILURE; + } + + void (*f1_flush)() = (void (*)())dlsym(f1_handle, "llvm_gcov_flush"); + if (f1_flush == NULL) { +fprintf(stderr, "unable to find llvm_gcov_flush in func.shared'\n"); +return EXIT_FAILURE; + } + f1_flush(); + + void (*f2_flush)() = (void (*)())dlsym(f2_handle, "llvm_gcov_flush"); + if (f2_flush == NULL) { +fprintf(stderr, "unable to find llvm_gcov_flush in func2.shared'\n"); +return EXIT_FAILURE; + } + f2_flush(); + + if (f1_flush == f2_flush) { +fprintf(stderr, "Same llvm_gcov_flush found in func.shared and func2.shared\n"); +return EXIT_FAILURE; + } + if (dlclose(f2_handle) != 0) { fprintf(stderr, "unable to close 'func2.shared': %s\n", dlerror()); return EXIT_FAILURE; Index: lib/profile/GCDAProfiling.c === --- lib/profile/GCDAProfiling.c +++ lib/profile/GCDAProfiling.c @@ -527,6 +527,10 @@ } } +// __gcov_flush is hidden. When called in a .so file, +// it dumps profile data of the calling .so file. +// If a main program needs to dump profile data of each linked +// .so files, it should use dlsym to find and call llvm_gcov_flush. COMPILER_RT_VISIBILITY void __gcov_flush() { struct flush_fn_node *curr = flush_fn_head; @@ -537,6 +541,14 @@ } } +// llvm_gcov_flush is not hidden for a program to use dlsym to +// find and call. In that case, it dumps profile data of a .so file. +// If it is called directly inside a .so file, the unified copy of +// llvm_gcov_flush might dump data of other .so file or the main module. +void llvm_gcov_flush() { + __gcov_flush(); +} + COMPILER_RT_VISIBILITY void llvm_delete_flush_function_list(void) { while (flush_fn_head) { Index: test/profile/Inputs/instrprof-dlopen-dlclose-main.c === --- test/profile/Inputs/instrprof-dlopen-dlclose-main.c +++ test/profile/Inputs/instrprof-dlopen-dlclose-main.c @@ -16,6 +16,31 @@ return EXIT_FAILURE; } + void (*gcov_flush)() = (void (*)())dlsym(f1_handle, "__gcov_flush"); + if (gcov_flush != NULL) { +fprintf(stderr, "__gcov_flush should not be visible in func.shared'\n"); +return EXIT_FAILURE; + } + + void (*f1_flush)() = (void (*)())dlsym(f1_handle, "llvm_gcov_flush"); + if (f1_flush == NULL) { +fprintf(stderr, "unable to find llvm_gcov_flush in func.shared'\n"); +return EXIT_FAILURE; + } + f1_flush(); + + void (*f2_flush)() = (void (*)())dlsym(f2_handle, "llvm_gcov_flush"); + if (f2_flush == NULL) { +fprintf(stderr, "unable to find llvm_gcov_flush in func2.shared'\n"); +return EXIT_FAILURE; + } + f2_flush(); + + if (f1_flush == f2_flush) { +fprintf(stderr, "Same llvm_gcov_flush found in func.shared and func2.shared\n"); +return EXIT_FAILURE; + } + if (dlclose(f2_handle) != 0) { fprintf(stderr, "unable to close 'func2.shared': %s\n", dlerror()); return EXIT_FAILURE; Index: lib/profile/GCDAProfiling.c === --- lib/profile/GCDAProfiling.c +++ lib/profile/GCDAProfiling.c @@ -527,6 +527,10 @@ } } +// __gcov_flush is hidden. When called in a .so file, +// it dumps profile data of the calling .so file. +// If a main program needs to dump profile data of each linked +// .so files, it should use dlsym to find and call llvm_gcov_flush. COMPILER_RT_VISIBILITY void __gcov_flush() { struct flush_fn_node *curr = flush_fn_head; @@ -537,6 +541,14 @@ } } +// llvm_gcov_flush is not hidden for a program to use dlsym to +// find and call. In that case, it dumps profile data of a .so file. +// If it is called directly inside a .so file, the unified copy of +// llvm_gcov_flush might dump data of other .so file or the main module. +void llvm_gcov_flush() { + __gcov_flush(); +} + COMPILER_RT_VISIBILITY void llvm_delete_flush_function_list(void) { while (flush_fn_head) { ___ cfe-commits mailing list cfe-commits
[PATCH] D48781: [ms] Fix mangling of char16_t and char32_t to be compatible with MSVC.
thakis created this revision. thakis added a reviewer: majnemer. MSVC limits char16_t and char32_t string literal names to 32 bytes of character data, not to 32 characters. wchar_t string literal names on the other hand can get up to 64 bytes of character data. https://reviews.llvm.org/D48781 Files: clang/lib/AST/MicrosoftMangle.cpp clang/test/CodeGenCXX/mangle-ms-string-literals.cpp Index: clang/test/CodeGenCXX/mangle-ms-string-literals.cpp === --- clang/test/CodeGenCXX/mangle-ms-string-literals.cpp +++ clang/test/CodeGenCXX/mangle-ms-string-literals.cpp @@ -719,9 +719,35 @@ // CHECK: @"??_C@_1EK@KFPEBLPK@?$AA0?$AA1?$AA2?$AA3?$AA4?$AA5?$AA6?$AA7?$AA8?$AA9?$AA0?$AA1?$AA2?$AA3?$AA4?$AA5?$AA6?$AA7?$AA8?$AA9?$AA0?$AA1?$AA2?$AA3?$AA4?$AA5?$AA6?$AA7?$AA8?$AA9?$AAA?$AAB@" const wchar_t *UnicodeLiteral = L"\ud7ff"; // CHECK: @"??_C@_13IIHIAFKH@?W?$PP?$AA?$AA@" + const char *U8Literal = u8"hi"; // CHECK: @"??_C@_02PCEFGMJL@hi?$AA@" +const char *LongU8Literal = u8"012345678901234567890123456789ABCDEF"; +// CHECK: @"??_C@_0CF@LABBIIMO@012345678901234567890123456789AB@" + const char16_t *U16Literal = u"hi"; // CHECK: @"??_C@_05OMLEGLOC@h?$AAi?$AA?$AA?$AA@" +// Note this starts with o instead of 0. Else LongWideString would have +// the same initializer and CodeGenModule::ConstantStringMap would map them +// to the same global with a shared mangling. +// FIXME: ConstantStringMap probably shouldn't map things with the same data +// but different manglings to the same variable. +const char16_t *LongU16Literal = u"o12345678901234567890123456789ABCDEF"; +// CHECK: @"??_C@_0EK@FEAOBHPP@o?$AA1?$AA2?$AA3?$AA4?$AA5?$AA6?$AA7?$AA8?$AA9?$AA0?$AA1?$AA2?$AA3?$AA4?$AA5?$AA@" + const char32_t *U32Literal = U"hi"; // CHECK: @"??_C@_0M@GFNAJIPG@h?$AA?$AA?$AAi?$AA?$AA?$AA?$AA?$AA?$AA?$AA@" +const char32_t *LongU32Literal = U"012345678901234567890123456789ABCDEF"; +// CHECK: @"??_C@_0JE@IMHFEDAA@0?$AA?$AA?$AA1?$AA?$AA?$AA2?$AA?$AA?$AA3?$AA?$AA?$AA4?$AA?$AA?$AA5?$AA?$AA?$AA6?$AA?$AA?$AA7?$AA?$AA?$AA@" + +// These all have just the right length that the trailing 0 just fits. +const char *MaxASCIIString = "012345678901234567890123456789A"; +// CHECK: @"??_C@_0CA@NMANGEKF@012345678901234567890123456789A?$AA@" +const wchar_t *MaxWideString = L"012345678901234567890123456789A"; +// CHECK: @"??_C@_1EA@LJAFPILO@?$AA0?$AA1?$AA2?$AA3?$AA4?$AA5?$AA6?$AA7?$AA8?$AA9?$AA0?$AA1?$AA2?$AA3?$AA4?$AA5?$AA6?$AA7?$AA8?$AA9?$AA0?$AA1?$AA2?$AA3?$AA4?$AA5?$AA6?$AA7?$AA8?$AA9?$AAA?$AA?$AA@" +const char *MaxU8String = u8"012345678901234567890123456789A"; +// CHECK: @"??_C@_0CA@NMANGEKF@012345678901234567890123456789A?$AA@" +const char16_t *MaxU16String = u"012345678901234"; +// CHECK: @"??_C@_0CA@NFEFHIFO@0?$AA1?$AA2?$AA3?$AA4?$AA5?$AA6?$AA7?$AA8?$AA9?$AA0?$AA1?$AA2?$AA3?$AA4?$AA?$AA?$AA@" +const char32_t *MaxU32String = U"0123456"; +// CHECK: @"??_C@_0CA@KFPHPCC@0?$AA?$AA?$AA1?$AA?$AA?$AA2?$AA?$AA?$AA3?$AA?$AA?$AA4?$AA?$AA?$AA5?$AA?$AA?$AA6?$AA?$AA?$AA?$AA?$AA?$AA?$AA@" Index: clang/lib/AST/MicrosoftMangle.cpp === --- clang/lib/AST/MicrosoftMangle.cpp +++ clang/lib/AST/MicrosoftMangle.cpp @@ -3164,9 +3164,9 @@ void MicrosoftMangleContextImpl::mangleStringLiteral(const StringLiteral *SL, raw_ostream &Out) { - // ::= 0 # char - // ::= 1 # wchar_t - // ::= ??? # char16_t/char32_t will need a mangling too... + // ::= 0 # char, char16_t, char32_t + // # (little endian char data in mangling) + // ::= 1 # wchar_t (big endian char data in mangling) // // ::= # the length of the literal // @@ -3228,8 +3228,8 @@ // scheme. Mangler.mangleNumber(JC.getCRC()); - // : The mangled name also contains the first 32 _characters_ - // (including null-terminator bytes) of the StringLiteral. + // : The mangled name also contains the first 32 bytes + // (including null-terminator bytes) of the encoded StringLiteral. // Each character is encoded by splitting them into bytes and then encoding // the constituent bytes. auto MangleByte = [&Mangler](char Byte) { @@ -3258,17 +3258,17 @@ } }; - // Enforce our 32 character max. - unsigned NumCharsToMangle = std::min(32U, SL->getLength()); - for (unsigned I = 0, E = NumCharsToMangle * SL->getCharByteWidth(); I != E; - ++I) + // Enforce our 32 bytes max, except wchar_t which gets 32 chars instead. + unsigned MaxBytesToWrite = SL->isWide() ? 64U : 32U; + unsigned NumBytesToWrite = std::min(MaxBytesToWrite, SL->getByteLength()); + for (unsigned I = 0; I != NumBytesToWrite; ++I) if (SL->isWide()) MangleByte(GetBigEndianByte(I)); else MangleByte(GetLittleEndianByte(I)); // Encode the NUL terminator if there is room. - if (NumCharsToMangle < 32) + if (NumBytesToWrite < MaxBytesToWrit
[PATCH] D48781: [ms] Fix mangling of char16_t and char32_t to be compatible with MSVC.
thakis added a comment. For your convenience: https://godbolt.org/g/KXxbKb https://reviews.llvm.org/D48781 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48412: [RISCV] Add support for interrupt attribute
apazos added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:5305 + + if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) { +S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 0; aaron.ballman wrote: > I would have assumed this would be: `!hasFunctionProto(D) || > getFunctionOrMethodNumParams(D) != 0`, but it depends on whether you want to > support K&R C functions. hasFunctionProto also returns true for a function definition like this one __attribute__((interrupt)) void foo1(int) {}. Comment at: lib/Sema/SemaDeclAttr.cpp:5301 + + if (!isFunctionOrMethod(D)) { +S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type) aaron.ballman wrote: > apazos wrote: > > aaron.ballman wrote: > > > apazos wrote: > > > > aaron.ballman wrote: > > > > > I don't think you need to perform this check -- I believe it's > > > > > handled automatically (because you don't have custom parsing enabled). > > > > I think need it. Will double check in the test. > > > See `handleCommonAttributeFeatures()` -- it calls > > > `diagnoseAppertainsTo()` which handles this for you. As it is, your check > > > here does not match the subject list on the attribute. The declarative > > > bit says it only appertains to a function and this check is for a > > > function or Obj-C method. > > > > > > Which brings up my next question: should this appertain to an ObjC method? > > It looks like handleCommonAttributeFeatures should take care of the check, > > but I do not see it happening, it returns true in > > AL.diagnoseAppertainsTo(S, D) even when we have > > struct a test __attribute__((interrupt)); > > > > I will remove the Subjects in Attr.td and keep the checks as they are in > > handleRISCVInterruptAttr. > > > > Several other targets do the same thing, they are reusing the helper > > functions that apply to both Function or Method. We would have to create > > helper functions just for function types. > Ah, the reason is because the parsed attributes that share a `ParseKind` can > have different subject lists, so there's no way to do the semantic checking > at that point -- we don't know which semantic attribute to check the subjects > against until later. > > Please put the `Subjects` list back in to Attr.td; it's still useful > declarative information and I may solve this problem someday in the future. > > I am not tied to whether the attribute appertains to a function and an obj-c > method as that depends on the attribute in question, but the code as it > stands is wrong. It checks whether the declaration is a function or a method > and then tells the user the attribute can only appertain to a function and > not a method. Which is correct? Sure I can add Subjects back in. I will remove the helper function and use the simple check D->getFunctionType() == nullptr. Comment at: test/Sema/riscv-interrupt-attr.c:18 + +__attribute__((interrupt("user"), interrupt("supervisor"))) void foo6() { } // expected-warning {{repeated RISC-V 'interrupt' attribute}} \ +// expected-note {{repeated RISC-V 'interrupt' attribute is here}} aaron.ballman wrote: > apazos wrote: > > aaron.ballman wrote: > > > apazos wrote: > > > > aaron.ballman wrote: > > > > > You should also add tests for: > > > > > ``` > > > > > __attribute__((interrupt("user"))) void f(void); > > > > > __attribute__((interrupt("machine"))) void f(void); > > > > > > > > > > void f(void) { } > > > > > > > > > > [[gnu::interrupt("user") gnu::interrupt("machine")]] void g() {} > > > > > > > > > > [[gnu::interrupt("user")]] [[gnu::interrupt("machine")]] void h() {} > > > > > ``` > > > > For this test case tt seems LLVM honors the last setting, "machine". > > > > But gcc is honoring the first. > > > > I think the last setting should prevail. Will check with GCC folks. > > > Do all of these cases get diagnosed as being a repeated interrupt > > > attribute? Should add them as test cases. > > The warning for repeated attribute is when it occurs more than once in the > > same declaration. If you have repeated declarations, the last one prevails. > Please document this in AttrDocs.td. Sure, I can add that info to the description. Comment at: test/Sema/riscv-interrupt-attr.c:23 + // expected-note {{repeated RISC-V 'interrupt' attribute is here}} +__attribute__((interrupt("user"))) void foo8() {} +__attribute__((interrupt("supervisor"))) void foo9() {} aaron.ballman wrote: > aaron.ballman wrote: > > Do you intend for functions without a prototype to be accepted? foo8() can > > be passed an arbitrary number of arguments, which is a bit different than > > what I thought you wanted the semantic check to be. > This question remains outstanding. The checks are v
[PATCH] D45454: Add llvm_gcov_flush to be called outside a shared library
srhines added a comment. Thanks for picking this up again and updating the change to add llvm_gcov_flush(). Comment at: test/profile/Inputs/instrprof-dlopen-dlclose-main.c:20 + void (*gcov_flush)() = (void (*)())dlsym(f1_handle, "__gcov_flush"); + if (gcov_flush != NULL) { +fprintf(stderr, "__gcov_flush should not be visible in func.shared'\n"); Should also clear dlerror() before this call and check that dlerror() returned a non-NULL pointer indicating that this search failed. https://reviews.llvm.org/D45454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48412: [RISCV] Add support for interrupt attribute
apazos updated this revision to Diff 153544. https://reviews.llvm.org/D48412 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td include/clang/Basic/DiagnosticSemaKinds.td lib/CodeGen/TargetInfo.cpp lib/Sema/SemaDeclAttr.cpp test/Sema/riscv-interrupt-attr.c test/Sema/riscv-interrupt-attr.cpp Index: test/Sema/riscv-interrupt-attr.cpp === --- /dev/null +++ test/Sema/riscv-interrupt-attr.cpp @@ -0,0 +1,7 @@ +// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only +// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only + +[[gnu::interrupt]] [[gnu::interrupt]] void foo1() {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \ + // expected-note {{repeated RISC-V 'interrupt' attribute is here}} +[[gnu::interrupt]] void foo2() {} + Index: test/Sema/riscv-interrupt-attr.c === --- /dev/null +++ test/Sema/riscv-interrupt-attr.c @@ -0,0 +1,34 @@ +// RUN: %clang_cc1 %s -triple riscv32-unknown-elf -verify -fsyntax-only +// RUN: %clang_cc1 %s -triple riscv64-unknown-elf -verify -fsyntax-only + +struct a { int b; }; + +struct a test __attribute__((interrupt)); // expected-warning {{'interrupt' attribute only applies to functions}} + +__attribute__((interrupt("USER"))) void foo1() {} // expected-warning {{'interrupt' attribute argument not supported: USER}} + +__attribute__((interrupt("user", 1))) void foo2() {} // expected-error {{'interrupt' attribute takes no more than 1 argument}} + +__attribute__((interrupt)) int foo3() {return 0;} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a 'void' return type}} + +__attribute__((interrupt())) int foo4(void) {} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have a 'void' return type}} + +__attribute__((interrupt())) void foo5(int a) {} // expected-warning {{RISC-V 'interrupt' attribute only applies to functions that have no parameters}} + +__attribute__((interrupt("user"), interrupt("supervisor"))) void foo6() {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \ +// expected-note {{repeated RISC-V 'interrupt' attribute is here}} + +__attribute__((interrupt, interrupt)) void foo7() {} // expected-warning {{repeated RISC-V 'interrupt' attribute}} \ + // expected-note {{repeated RISC-V 'interrupt' attribute is here}} +__attribute__((interrupt(""))) void foo8() {} // expected-warning {{'interrupt' attribute argument not supported}} + +__attribute__((interrupt("user"))) void foo9(); +__attribute__((interrupt("supervisor"))) void foo9(); +__attribute__((interrupt("machine"))) void foo9(); + +__attribute__((interrupt("user"))) void foo10() {} +__attribute__((interrupt("supervisor"))) void foo11() {} +__attribute__((interrupt("machine"))) void foo12() {} +__attribute__((interrupt())) void foo13() {} +__attribute__((interrupt)) void foo14() {} + Index: lib/Sema/SemaDeclAttr.cpp === --- lib/Sema/SemaDeclAttr.cpp +++ lib/Sema/SemaDeclAttr.cpp @@ -5266,6 +5266,63 @@ handleSimpleAttribute(S, D, AL); } +static void handleRISCVInterruptAttr(Sema &S, Decl *D, + const AttributeList &AL) { + // Warn about repeated attributes. + if (const auto *A = D->getAttr()) { +S.Diag(AL.getRange().getBegin(), + diag::warn_riscv_repeated_interrupt_attribute); +S.Diag(A->getLocation(), diag::note_riscv_repeated_interrupt_attribute); +return; + } + + // Check the attribute argument. Argument is optional. + if (!checkAttributeAtMostNumArgs(S, AL, 1)) +return; + + StringRef Str; + SourceLocation ArgLoc; + + // 'machine'is the default interrupt mode. + if (AL.getNumArgs() == 0) +Str = "machine"; + else if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, &ArgLoc)) +return; + + // Semantic checks for a function with the 'interrupt' attribute: + // - Must be a function. + // - Must have no parameters. + // - Must have the 'void' return type. + // - The attribute itself must either have no argument or one of the + // valid interrupt types, see [RISCVInterruptDocs]. + + if (D->getFunctionType() == nullptr) { +S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type) + << "'interrupt'" << ExpectedFunction; +return; + } + + if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) { +S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 0; +return; + } + + if (!getFunctionOrMethodResultType(D)->isVoidType()) { +S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 1; +return; + } + + RISCVInterruptAttr::InterruptType Kind; + if (!RISCVIn
[PATCH] D45454: Add llvm_gcov_flush to be called outside a shared library
marco-c added a comment. Why keeping a __gcov_flush which is incompatible with GCC and previous versions of LLVM and adding a __llvm_gcov_flush which is compatible? I feel the other way around (or even just having an unhidden "__gcov_flush" and not introducing "__llvm_gcov_flush") would be more sensible. https://reviews.llvm.org/D45454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336001 - [MachineOutliner] Make -mno-outline use -enable-machine-outliner=never
Author: paquette Date: Fri Jun 29 11:06:10 2018 New Revision: 336001 URL: http://llvm.org/viewvc/llvm-project?rev=336001&view=rev Log: [MachineOutliner] Make -mno-outline use -enable-machine-outliner=never This updates -mno-outline so that it passes -enable-machine-outliner=never instead of nothing. This puts it in sync with the behaviour in llc and other tools. Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp cfe/trunk/test/Driver/aarch64-outliner.c Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=336001&r1=336000&r2=336001&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Fri Jun 29 11:06:10 2018 @@ -4797,23 +4797,30 @@ void Clang::ConstructJob(Compilation &C, options::OPT_fno_complete_member_pointers, false)) CmdArgs.push_back("-fcomplete-member-pointers"); - if (Args.hasFlag(options::OPT_moutline, options::OPT_mno_outline, false)) { -// We only support -moutline in AArch64 right now. If we're not compiling -// for AArch64, emit a warning and ignore the flag. Otherwise, add the -// proper mllvm flags. -if (Triple.getArch() != llvm::Triple::aarch64) { - D.Diag(diag::warn_drv_moutline_unsupported_opt) << Triple.getArchName(); -} else { - CmdArgs.push_back("-mllvm"); - CmdArgs.push_back("-enable-machine-outliner"); - - // The outliner shouldn't compete with linkers that dedupe linkonceodr - // functions in LTO. Enable that behaviour by default when compiling with - // LTO. - if (getToolChain().getDriver().isUsingLTO()) { + if (Arg *A = Args.getLastArg(options::OPT_moutline, + options::OPT_mno_outline)) { +if (A->getOption().matches(options::OPT_moutline)) { + // We only support -moutline in AArch64 right now. If we're not compiling + // for AArch64, emit a warning and ignore the flag. Otherwise, add the + // proper mllvm flags. + if (Triple.getArch() != llvm::Triple::aarch64) { +D.Diag(diag::warn_drv_moutline_unsupported_opt) << Triple.getArchName(); + } else { CmdArgs.push_back("-mllvm"); -CmdArgs.push_back("-enable-linkonceodr-outlining"); +CmdArgs.push_back("-enable-machine-outliner"); + +// The outliner shouldn't compete with linkers that dedupe linkonceodr +// functions in LTO. Enable that behaviour by default when compiling with +// LTO. +if (getToolChain().getDriver().isUsingLTO()) { + CmdArgs.push_back("-mllvm"); + CmdArgs.push_back("-enable-linkonceodr-outlining"); +} } +} else { + // Disable all outlining behaviour. + CmdArgs.push_back("-mllvm"); + CmdArgs.push_back("-enable-machine-outliner=never"); } } Modified: cfe/trunk/test/Driver/aarch64-outliner.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-outliner.c?rev=336001&r1=336000&r2=336001&view=diff == --- cfe/trunk/test/Driver/aarch64-outliner.c (original) +++ cfe/trunk/test/Driver/aarch64-outliner.c Fri Jun 29 11:06:10 2018 @@ -2,7 +2,7 @@ // RUN: %clang -target aarch64 -moutline -S %s -### 2>&1 | FileCheck %s -check-prefix=ON // ON: "-mllvm" "-enable-machine-outliner" // RUN: %clang -target aarch64 -moutline -mno-outline -S %s -### 2>&1 | FileCheck %s -check-prefix=OFF -// OFF-NOT: "-mllvm" "-enable-machine-outliner" +// OFF: "-mllvm" "-enable-machine-outliner=never" // RUN: %clang -target aarch64 -moutline -flto=thin -S %s -### 2>&1 | FileCheck %s -check-prefix=FLTO // FLTO: "-mllvm" "-enable-linkonceodr-outlining" // RUN: %clang -target aarch64 -moutline -flto=full -S %s -### 2>&1 | FileCheck %s -check-prefix=TLTO ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336002 - [analyzer] Replace the vector of ConstraintSets by a single ConstraintSet and a function to merge ConstraintSets
Author: mramalho Date: Fri Jun 29 11:11:43 2018 New Revision: 336002 URL: http://llvm.org/viewvc/llvm-project?rev=336002&view=rev Log: [analyzer] Replace the vector of ConstraintSets by a single ConstraintSet and a function to merge ConstraintSets Now, instead of adding the constraints when they are removed, this patch adds them when they first appear and, since we walk the bug report backward, it should be the last set of ranges generated by the CSA for a given symbol. These are the number before and after the patch: ``` Project| current | patch | tmux | 283.222 | 123.052 | redis | 614.858 | 400.347 | openssl| 308.292 | 307.149 | twin | 274.478 | 245.411 | git| 547.687 | 477.335 | postgresql | 2927.495 | 2002.526 | sqlite3| 3264.305 | 1028.416 | ``` Major speedups in tmux and sqlite (less than half of the time), redis and postgresql were about 25% faster while the rest are basically the same. Reviewers: NoQ, george.karpenkov Reviewed By: george.karpenkov Subscribers: rnkovacs, xazax.hun, szepet, a.sidorin Differential Revision: https://reviews.llvm.org/D48565 Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h?rev=336002&r1=336001&r2=336002&view=diff == --- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h Fri Jun 29 11:11:43 2018 @@ -336,11 +336,10 @@ public: class FalsePositiveRefutationBRVisitor final : public BugReporterVisitor { private: /// Holds the constraints in a given path - // TODO: should we use a set? - llvm::SmallVector Constraints; + ConstraintRangeTy Constraints; public: - FalsePositiveRefutationBRVisitor() = default; + FalsePositiveRefutationBRVisitor(); void Profile(llvm::FoldingSetNodeID &ID) const override; @@ -348,6 +347,9 @@ public: const ExplodedNode *PrevN, BugReporterContext &BRC, BugReport &BR) override; + + void finalizeVisitor(BugReporterContext &BRC, const ExplodedNode *EndPathNode, + BugReport &BR) override; }; namespace bugreporter { Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=336002&r1=336001&r2=336002&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Fri Jun 29 11:11:43 2018 @@ -2349,9 +2349,8 @@ TaintBugVisitor::VisitNode(const Explode return std::make_shared(L, "Taint originated here"); } -static bool -areConstraintsUnfeasible(BugReporterContext &BRC, - const llvm::SmallVector &Cs) { +static bool areConstraintsUnfeasible(BugReporterContext &BRC, + const ConstraintRangeTy &Cs) { // Create a refutation manager std::unique_ptr RefutationMgr = CreateZ3ConstraintManager( BRC.getStateManager(), BRC.getStateManager().getOwningEngine()); @@ -2360,28 +2359,45 @@ areConstraintsUnfeasible(BugReporterCont static_cast(RefutationMgr.get()); // Add constraints to the solver - for (const auto &C : Cs) -SMTRefutationMgr->addRangeConstraints(C); + SMTRefutationMgr->addRangeConstraints(Cs); // And check for satisfiability return SMTRefutationMgr->isModelFeasible().isConstrainedFalse(); } +static void addNewConstraints(ConstraintRangeTy &Cs, + const ConstraintRangeTy &NewCs, + ConstraintRangeTy::Factory &CF) { + // Add constraints if we don't have them yet + for (auto const &C : NewCs) { +const SymbolRef &Sym = C.first; +if (!Cs.contains(Sym)) { + Cs = CF.add(Cs, Sym, C.second); +} + } +} + +FalsePositiveRefutationBRVisitor::FalsePositiveRefutationBRVisitor() +: Constraints(ConstraintRangeTy::Factory().getEmptyMap()) {} + +void FalsePositiveRefutationBRVisitor::finalizeVisitor( +BugReporterContext &BRC, const ExplodedNode *EndPathNode, BugReport &BR) { + // Collect new constraints + VisitNode(EndPathNode, nullptr, BRC, BR); + + // Create a new refutation manager and check feasibility + if (areConstraintsUnfeasible(BRC, Constraints)) +BR.markInvalid("Infeasible constraints", EndPathNode->getLocationContext()); +} + std::shared_ptr
[PATCH] D48565: [analyzer] Replace the vector of ConstraintSets by a single ConstraintSet and a function to merge ConstraintSets
This revision was automatically updated to reflect the committed changes. Closed by commit rC336002: [analyzer] Replace the vector of ConstraintSets by a single ConstraintSet and a… (authored by mramalho, committed by ). Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D48565 Files: include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Index: include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h === --- include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h +++ include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h @@ -336,18 +336,20 @@ class FalsePositiveRefutationBRVisitor final : public BugReporterVisitor { private: /// Holds the constraints in a given path - // TODO: should we use a set? - llvm::SmallVector Constraints; + ConstraintRangeTy Constraints; public: - FalsePositiveRefutationBRVisitor() = default; + FalsePositiveRefutationBRVisitor(); void Profile(llvm::FoldingSetNodeID &ID) const override; std::shared_ptr VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC, BugReport &BR) override; + + void finalizeVisitor(BugReporterContext &BRC, const ExplodedNode *EndPathNode, + BugReport &BR) override; }; namespace bugreporter { Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp === --- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -2349,39 +2349,55 @@ return std::make_shared(L, "Taint originated here"); } -static bool -areConstraintsUnfeasible(BugReporterContext &BRC, - const llvm::SmallVector &Cs) { +static bool areConstraintsUnfeasible(BugReporterContext &BRC, + const ConstraintRangeTy &Cs) { // Create a refutation manager std::unique_ptr RefutationMgr = CreateZ3ConstraintManager( BRC.getStateManager(), BRC.getStateManager().getOwningEngine()); SMTConstraintManager *SMTRefutationMgr = static_cast(RefutationMgr.get()); // Add constraints to the solver - for (const auto &C : Cs) -SMTRefutationMgr->addRangeConstraints(C); + SMTRefutationMgr->addRangeConstraints(Cs); // And check for satisfiability return SMTRefutationMgr->isModelFeasible().isConstrainedFalse(); } +static void addNewConstraints(ConstraintRangeTy &Cs, + const ConstraintRangeTy &NewCs, + ConstraintRangeTy::Factory &CF) { + // Add constraints if we don't have them yet + for (auto const &C : NewCs) { +const SymbolRef &Sym = C.first; +if (!Cs.contains(Sym)) { + Cs = CF.add(Cs, Sym, C.second); +} + } +} + +FalsePositiveRefutationBRVisitor::FalsePositiveRefutationBRVisitor() +: Constraints(ConstraintRangeTy::Factory().getEmptyMap()) {} + +void FalsePositiveRefutationBRVisitor::finalizeVisitor( +BugReporterContext &BRC, const ExplodedNode *EndPathNode, BugReport &BR) { + // Collect new constraints + VisitNode(EndPathNode, nullptr, BRC, BR); + + // Create a new refutation manager and check feasibility + if (areConstraintsUnfeasible(BRC, Constraints)) +BR.markInvalid("Infeasible constraints", EndPathNode->getLocationContext()); +} + std::shared_ptr FalsePositiveRefutationBRVisitor::VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC, BugReport &BR) { - // Collect the constraint for the current state - const ConstraintRangeTy &CR = N->getState()->get(); - Constraints.push_back(CR); - - // If there are no predecessor, we reached the root node. In this point, - // a new refutation manager will be created and the path will be checked - // for reachability - if (PrevN->pred_size() == 0 && areConstraintsUnfeasible(BRC, Constraints)) { -BR.markInvalid("Infeasible constraints", N->getLocationContext()); - } + // Collect new constraints + addNewConstraints(Constraints, N->getState()->get(), +N->getState()->get_context()); return nullptr; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r335925 - PR37979: integral promotions in C++ treat enum bit-fields like enums,
Hello Richard, This commit broke tests at one of our builders: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/10599 Failing Tests (1): Clang :: CXX/conv/conv.prom/p5.cpp Please have a look? Thanks Galina On Thu, Jun 28, 2018 at 2:17 PM, Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rsmith > Date: Thu Jun 28 14:17:55 2018 > New Revision: 335925 > > URL: http://llvm.org/viewvc/llvm-project?rev=335925&view=rev > Log: > PR37979: integral promotions in C++ treat enum bit-fields like enums, > not like bit-fields. > > We used to get this right "by accident", because conversions for the > selected built-in overloaded operator would convert the enum bit-field > to its corresponding underlying type early. But after DR1687 that no > longer happens. > > Technically this change should also apply to C, where bit-fields only > have special promotion rules if the bit-field's declared type is > _Bool, int, signed int, or unsigned int, but for GCC compatibility we > only look at the bit-width and not the underlying type when performing > bit-field integral promotions in C. > > Added: > cfe/trunk/test/CXX/conv/conv.prom/p5.cpp > Modified: > cfe/trunk/lib/AST/ASTContext.cpp > cfe/trunk/lib/Sema/SemaOverload.cpp > > Modified: cfe/trunk/lib/AST/ASTContext.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ > ASTContext.cpp?rev=335925&r1=335924&r2=335925&view=diff > > == > --- cfe/trunk/lib/AST/ASTContext.cpp (original) > +++ cfe/trunk/lib/AST/ASTContext.cpp Thu Jun 28 14:17:55 2018 > @@ -5456,6 +5456,12 @@ QualType ASTContext::isPromotableBitFiel >if (E->isTypeDependent() || E->isValueDependent()) > return {}; > > + // C++ [conv.prom]p5: > + //If the bit-field has an enumerated type, it is treated as any > other > + //value of that type for promotion purposes. > + if (getLangOpts().CPlusPlus && E->getType()->isEnumeralType()) > +return {}; > + >// FIXME: We should not do this unless E->refersToBitField() is true. > This >// matters in C where getSourceBitField() will find bit-fields for > various >// cases where the source expression is not a bit-field designator. > @@ -5482,13 +5488,15 @@ QualType ASTContext::isPromotableBitFiel >// >// FIXME: C does not permit promotion of a 'long : 3' bitfield to int. >//We perform that promotion here to match GCC and C++. > + // FIXME: C does not permit promotion of an enum bit-field whose rank is > + //greater than that of 'int'. We perform that promotion to > match GCC. >if (BitWidth < IntSize) > return IntTy; > >if (BitWidth == IntSize) > return FT->isSignedIntegerType() ? IntTy : UnsignedIntTy; > > - // Types bigger than int are not subject to promotions, and therefore > act > + // Bit-fields wider than int are not subject to promotions, and > therefore act >// like the base type. GCC has some weird bugs in this area that we >// deliberately do not follow (GCC follows a pre-standard resolution to >// C's DR315 which treats bit-width as being part of the type, and this > leaks > > Modified: cfe/trunk/lib/Sema/SemaOverload.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/ > SemaOverload.cpp?rev=335925&r1=335924&r2=335925&view=diff > > == > --- cfe/trunk/lib/Sema/SemaOverload.cpp (original) > +++ cfe/trunk/lib/Sema/SemaOverload.cpp Thu Jun 28 14:17:55 2018 > @@ -2007,6 +2007,14 @@ bool Sema::IsIntegralPromotion(Expr *Fro > isCompleteType(From->getLocStart(), FromType)) >return Context.hasSameUnqualifiedType( >ToType, FromEnumType->getDecl()->getPromotionType()); > + > +// C++ [conv.prom]p5: > +// If the bit-field has an enumerated type, it is treated as any > other > +// value of that type for promotion purposes. > +// > +// ... so do not fall through into the bit-field checks below in C++. > +if (getLangOpts().CPlusPlus) > + return false; >} > >// C++0x [conv.prom]p2: > @@ -2054,6 +2062,11 @@ bool Sema::IsIntegralPromotion(Expr *Fro >// other value of that type for promotion purposes (C++ 4.5p3). >// FIXME: We should delay checking of bit-fields until we actually > perform the >// conversion. > + // > + // FIXME: In C, only bit-fields of types _Bool, int, or unsigned int > may be > + // promoted, per C11 6.3.1.1/2. We promote all bit-fields (including > enum > + // bit-fields and those whose underlying type is larger than int) for > GCC > + // compatibility. >if (From) { > if (FieldDecl *MemberDecl = From->getSourceBitField()) { >llvm::APSInt BitWidth; > > Added: cfe/trunk/test/CXX/conv/conv.prom/p5.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/ > conv/conv.prom/p5.cpp?rev=335925&view=auto > ===
[PATCH] D45454: Add llvm_gcov_flush to be called outside a shared library
davidxl added a comment. With the current gcov_flush implementation in LLVM, making gcov_flush's visibility to be default will simply lead to wrong behavior. GCC libgcov's implementation is more elaborate -- it allows gcov_flush to dump gcda data for all dynamic objects while making sure gcov_exit only dumps the gcda files from the same dynamic module (otherwise, there will be wrong profile merging). This is done via two levels of linked list. The patch https://reviews.llvm.org/D48538 is in the right direction, but not complete yet. https://reviews.llvm.org/D45454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct
Yeah, doesn't look like this code handles a value crossing the boundary of the size of the bitfield type (it's reading only the low bit). I suspect looking at the code that generates bitfield accesses would be useful - and/or maybe actually calling into that very code, rather than reimplementing it here? CodeGenFunction::EmitLoadOfBitfieldLValue seems to be the place to go (I found this by writing a short example that loads one of these strided bitfield values & then breaking in llvm::BinaryOperator::BinaryOperator until I found the 'and' operation, since that seemed like the more interesting one). On Fri, Jun 29, 2018 at 9:47 AM Paul Semel via Phabricator < revi...@reviews.llvm.org> wrote: > paulsemel added a comment. > > In https://reviews.llvm.org/D47953#1143044, @dblaikie wrote: > > > This doesn't seem to build for me - so hard to debug/probe it: > > > > llvm/src/tools/clang/lib/CodeGen/CGBuiltin.cpp:1264:65: error: no viable > conversion from 'clang::QualType' to 'llvm::Type *' > > > > CGF.CGM.getDataLayout().getTypeSizeInBits(CanonicalType), > > ^ > > > > llvm/src/include/llvm/IR/DataLayout.h:560:53: note: passing argument to > parameter 'Ty' here > > inline uint64_t DataLayout::getTypeSizeInBits(Type *Ty) const { > > > > ^ > > > > 1 error generated. > > > Very sorry about it, I should have paid more attention.. > > > Repository: > rC Clang > > https://reviews.llvm.org/D47953 > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47953: [builtin] Add bitfield support for __builtin_dump_struct
dblaikie added a subscriber: paulsemel. dblaikie added a comment. Yeah, doesn't look like this code handles a value crossing the boundary of the size of the bitfield type (it's reading only the low bit). I suspect looking at the code that generates bitfield accesses would be useful - and/or maybe actually calling into that very code, rather than reimplementing it here? CodeGenFunction::EmitLoadOfBitfieldLValue seems to be the place to go (I found this by writing a short example that loads one of these strided bitfield values & then breaking in llvm::BinaryOperator::BinaryOperator until I found the 'and' operation, since that seemed like the more interesting one). Repository: rC Clang https://reviews.llvm.org/D47953 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47297: [Modules][ObjC] Add protocol redefinition to the current scope/context
bruno added inline comments. Comment at: lib/Sema/SemaDeclObjC.cpp:1208 +// serialize something meaningful. +if (getLangOpts().Modules) + PushOnScopeChains(PDecl, TUScope); arphaman wrote: > Is this a problem only for modules or does this show up in PCHs too? What > would be the cost of using PushOnScopeChains unconditionally? > Is this a problem only for modules or does this show up in PCHs too? It only shows up in PCHs if Modules are being used, otherwise works as expected, because we don't have to serialize a different module which contains the dup, we just ignore it. > What would be the cost of using PushOnScopeChains unconditionally? I'm afraid that without the modules visibility rules, adding it unconditionally will make it available for name lookup, which we don't want here. Repository: rC Clang https://reviews.llvm.org/D47297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45454: Add llvm_gcov_flush to be called outside a shared library
chh added a comment. Marco, latest patch does not change __gcov_flush, which is also hidden in libgcov. Android coverage test programs have depended on an earlier compiler-rt that did not hide __gcov_flush. If that's the only use case broken by recent change of compiler-rt, which hide __gcov_flush, I think it is okay to keep compiler-rt the same as it is now. Adding a visible llvm_gcov_flush will allow Android coverage test program to use it and dump system .so profiling data, from the main test program. Keeping an hidden __gcov_flush has the purpose that David mentioned earlier, to let each .so file dump its own profiling data. https://reviews.llvm.org/D45454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47297: [Modules][ObjC] Add protocol redefinition to the current scope/context
arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. Thanks for explaining! LGTM Repository: rC Clang https://reviews.llvm.org/D47297 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48412: [RISCV] Add support for interrupt attribute
aaron.ballman added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:5305 + + if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) { +S.Diag(D->getLocation(), diag::warn_riscv_interrupt_attribute) << 0; apazos wrote: > aaron.ballman wrote: > > I would have assumed this would be: `!hasFunctionProto(D) || > > getFunctionOrMethodNumParams(D) != 0`, but it depends on whether you want > > to support K&R C functions. > hasFunctionProto also returns true for a function definition like this one > __attribute__((interrupt)) void foo1(int) {}. That's correct, which is why I recommended the predicate the way I did. If the function has no prototype, we want to diagnose it. If the function has a prototype, we want to see how many parameters are listed and if there are more than zero parameters, we want to diagnose it. Comment at: test/Sema/riscv-interrupt-attr.c:23 + // expected-note {{repeated RISC-V 'interrupt' attribute is here}} +__attribute__((interrupt("user"))) void foo8() {} +__attribute__((interrupt("supervisor"))) void foo9() {} apazos wrote: > aaron.ballman wrote: > > aaron.ballman wrote: > > > Do you intend for functions without a prototype to be accepted? foo8() > > > can be passed an arbitrary number of arguments, which is a bit different > > > than what I thought you wanted the semantic check to be. > > This question remains outstanding. > The checks are validating both function definitions and function prototypes > like these: > _attribute__((interrupt)) void foo1() {} > __attribute__((interrupt)) void foo(void); > Not sure what the confusion is. Ah, now I see where the confusion is. In C, an empty parameter list declares a function without a prototype; functions without prototypes can accept any number of arguments. To declare a function that accepts no arguments, you must have a prototype for the function and the parameter list is void. In C++, all functions are prototyped and an empty parameter list is equivalent to a parameter list of void. The word "prototype" doesn't mean "forward declaration". e.g., ``` // C code void foo1(); // Declaration; no prototype; accepts any number of arguments. void foo2() {} // Definition; no prototype; accepts any number of arguments. void foo3(void); // Declaration; prototype; accepts no arguments. void foo4(void) {} // Definition; prototype; accepts no arguments. foo2(1, 2, 3); // ok foo4(1, 2, 3); // error ``` Because a function without a prototype can accept any number of arguments, I think you want to diagnose such a function signature. https://reviews.llvm.org/D48412 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48785: Add a blank line to docs/README.txt test commit access
rupprecht created this revision. Herald added subscribers: cfe-commits, christof. Repository: rUNW libunwind https://reviews.llvm.org/D48785 Files: docs/README.txt Index: docs/README.txt === --- docs/README.txt +++ docs/README.txt @@ -11,3 +11,4 @@ After configuring libunwind with these options the make rule `docs-libunwind-html` should be available. + Index: docs/README.txt === --- docs/README.txt +++ docs/README.txt @@ -11,3 +11,4 @@ After configuring libunwind with these options the make rule `docs-libunwind-html` should be available. + ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48515: [mips][ias] Enable IAS by default for OpenBSD / FreeBSD mips64/mips64el.
This revision was automatically updated to reflect the committed changes. Closed by commit rC336004: [mips][ias] Enable IAS by default for OpenBSD / FreeBSD mips64/mips64el. (authored by brad, committed by ). Changed prior to commit: https://reviews.llvm.org/D48515?vs=152580&id=153550#toc Repository: rC Clang https://reviews.llvm.org/D48515 Files: lib/Driver/ToolChains/Gnu.cpp test/Driver/freebsd.c test/Driver/openbsd.c Index: lib/Driver/ToolChains/Gnu.cpp === --- lib/Driver/ToolChains/Gnu.cpp +++ lib/Driver/ToolChains/Gnu.cpp @@ -2412,11 +2412,13 @@ return true; case llvm::Triple::mips64: case llvm::Triple::mips64el: -// Enabled for Debian and Android mips64/mipsel, as they can precisely -// identify the ABI in use (Debian) or only use N64 for MIPS64 (Android). -// Other targets are unable to distinguish N32 from N64. +// Enabled for Debian, Android, FreeBSD and OpenBSD mips64/mipsel, as they +// can precisely identify the ABI in use (Debian) or only use N64 for MIPS64 +// (Android). Other targets are unable to distinguish N32 from N64. if (getTriple().getEnvironment() == llvm::Triple::GNUABI64 || -getTriple().isAndroid()) +getTriple().isAndroid() || +getTriple().isOSFreeBSD() || +getTriple().isOSOpenBSD()) return true; return false; default: Index: test/Driver/openbsd.c === --- test/Driver/openbsd.c +++ test/Driver/openbsd.c @@ -74,6 +74,13 @@ // CHECK-MIPS64EL: as{{.*}}" "-mabi" "64" "-EL" // CHECK-MIPS64EL-PIC: as{{.*}}" "-mabi" "64" "-EL" "-KPIC" +// Check that the integrated assembler is enabled for MIPS64 +// RUN: %clang -target mips64-unknown-openbsd -### -c %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-MIPS64-AS %s +// RUN: %clang -target mips64el-unknown-openbsd -### -c %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-MIPS64-AS %s +// CHECK-MIPS64-AS-NOT: "-no-integrated-as" + // Check linking against correct startup code when (not) using PIE // RUN: %clang -no-canonical-prefixes -target i686-pc-openbsd %s -### 2>&1 \ // RUN: | FileCheck -check-prefix=CHECK-PIE %s Index: test/Driver/freebsd.c === --- test/Driver/freebsd.c +++ test/Driver/freebsd.c @@ -183,3 +183,10 @@ // RUN: %clang -target mips64el-unknown-freebsd -### -c %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHECK-MIPS64-CPU %s // CHECK-MIPS64-CPU: "-target-cpu" "mips3" + +// Check that the integrated assembler is enabled for MIPS64 +// RUN: %clang -target mips64-unknown-freebsd -### -c %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-MIPS64-AS %s +// RUN: %clang -target mips64el-unknown-freebsd -### -c %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-MIPS64-AS %s +// CHECK-MIPS64-AS-NOT: "-no-integrated-as" Index: lib/Driver/ToolChains/Gnu.cpp === --- lib/Driver/ToolChains/Gnu.cpp +++ lib/Driver/ToolChains/Gnu.cpp @@ -2412,11 +2412,13 @@ return true; case llvm::Triple::mips64: case llvm::Triple::mips64el: -// Enabled for Debian and Android mips64/mipsel, as they can precisely -// identify the ABI in use (Debian) or only use N64 for MIPS64 (Android). -// Other targets are unable to distinguish N32 from N64. +// Enabled for Debian, Android, FreeBSD and OpenBSD mips64/mipsel, as they +// can precisely identify the ABI in use (Debian) or only use N64 for MIPS64 +// (Android). Other targets are unable to distinguish N32 from N64. if (getTriple().getEnvironment() == llvm::Triple::GNUABI64 || -getTriple().isAndroid()) +getTriple().isAndroid() || +getTriple().isOSFreeBSD() || +getTriple().isOSOpenBSD()) return true; return false; default: Index: test/Driver/openbsd.c === --- test/Driver/openbsd.c +++ test/Driver/openbsd.c @@ -74,6 +74,13 @@ // CHECK-MIPS64EL: as{{.*}}" "-mabi" "64" "-EL" // CHECK-MIPS64EL-PIC: as{{.*}}" "-mabi" "64" "-EL" "-KPIC" +// Check that the integrated assembler is enabled for MIPS64 +// RUN: %clang -target mips64-unknown-openbsd -### -c %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-MIPS64-AS %s +// RUN: %clang -target mips64el-unknown-openbsd -### -c %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-MIPS64-AS %s +// CHECK-MIPS64-AS-NOT: "-no-integrated-as" + // Check linking against correct startup code when (not) using PIE // RUN: %clang -no-canonical-prefixes -target i686-pc-openbsd %s -### 2>&1 \ // RUN: | FileCheck -check-prefix=CHECK-PIE %s Index: test/Driver/freebsd.c === --- test/Driver/freebsd.c +++ test/Driver/freebsd.c @@ -183,3 +183,10 @@ // RUN: %clang -target mips64el-unknown-freebsd -###
r336004 - [mips][ias] Enable IAS by default for OpenBSD / FreeBSD mips64/mips64el.
Author: brad Date: Fri Jun 29 12:03:03 2018 New Revision: 336004 URL: http://llvm.org/viewvc/llvm-project?rev=336004&view=rev Log: [mips][ias] Enable IAS by default for OpenBSD / FreeBSD mips64/mips64el. Reviewers: atanasyan Differential Revision: https://reviews.llvm.org/D48515 Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp cfe/trunk/test/Driver/freebsd.c cfe/trunk/test/Driver/openbsd.c Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=336004&r1=336003&r2=336004&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Fri Jun 29 12:03:03 2018 @@ -2412,11 +2412,13 @@ bool Generic_GCC::IsIntegratedAssemblerD return true; case llvm::Triple::mips64: case llvm::Triple::mips64el: -// Enabled for Debian and Android mips64/mipsel, as they can precisely -// identify the ABI in use (Debian) or only use N64 for MIPS64 (Android). -// Other targets are unable to distinguish N32 from N64. +// Enabled for Debian, Android, FreeBSD and OpenBSD mips64/mipsel, as they +// can precisely identify the ABI in use (Debian) or only use N64 for MIPS64 +// (Android). Other targets are unable to distinguish N32 from N64. if (getTriple().getEnvironment() == llvm::Triple::GNUABI64 || -getTriple().isAndroid()) +getTriple().isAndroid() || +getTriple().isOSFreeBSD() || +getTriple().isOSOpenBSD()) return true; return false; default: Modified: cfe/trunk/test/Driver/freebsd.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/freebsd.c?rev=336004&r1=336003&r2=336004&view=diff == --- cfe/trunk/test/Driver/freebsd.c (original) +++ cfe/trunk/test/Driver/freebsd.c Fri Jun 29 12:03:03 2018 @@ -183,3 +183,10 @@ // RUN: %clang -target mips64el-unknown-freebsd -### -c %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHECK-MIPS64-CPU %s // CHECK-MIPS64-CPU: "-target-cpu" "mips3" + +// Check that the integrated assembler is enabled for MIPS64 +// RUN: %clang -target mips64-unknown-freebsd -### -c %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-MIPS64-AS %s +// RUN: %clang -target mips64el-unknown-freebsd -### -c %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-MIPS64-AS %s +// CHECK-MIPS64-AS-NOT: "-no-integrated-as" Modified: cfe/trunk/test/Driver/openbsd.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/openbsd.c?rev=336004&r1=336003&r2=336004&view=diff == --- cfe/trunk/test/Driver/openbsd.c (original) +++ cfe/trunk/test/Driver/openbsd.c Fri Jun 29 12:03:03 2018 @@ -74,6 +74,13 @@ // CHECK-MIPS64EL: as{{.*}}" "-mabi" "64" "-EL" // CHECK-MIPS64EL-PIC: as{{.*}}" "-mabi" "64" "-EL" "-KPIC" +// Check that the integrated assembler is enabled for MIPS64 +// RUN: %clang -target mips64-unknown-openbsd -### -c %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-MIPS64-AS %s +// RUN: %clang -target mips64el-unknown-openbsd -### -c %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHECK-MIPS64-AS %s +// CHECK-MIPS64-AS-NOT: "-no-integrated-as" + // Check linking against correct startup code when (not) using PIE // RUN: %clang -no-canonical-prefixes -target i686-pc-openbsd %s -### 2>&1 \ // RUN: | FileCheck -check-prefix=CHECK-PIE %s ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r336006 - Add a blank line to docs/README.txt test commit access
Author: rupprecht Date: Fri Jun 29 12:05:21 2018 New Revision: 336006 URL: http://llvm.org/viewvc/llvm-project?rev=336006&view=rev Log: Add a blank line to docs/README.txt test commit access Subscribers: christof, cfe-commits Differential Revision: https://reviews.llvm.org/D48785 Modified: libunwind/trunk/docs/README.txt Modified: libunwind/trunk/docs/README.txt URL: http://llvm.org/viewvc/llvm-project/libunwind/trunk/docs/README.txt?rev=336006&r1=336005&r2=336006&view=diff == --- libunwind/trunk/docs/README.txt (original) +++ libunwind/trunk/docs/README.txt Fri Jun 29 12:05:21 2018 @@ -11,3 +11,4 @@ To build the documents into html configu After configuring libunwind with these options the make rule `docs-libunwind-html` should be available. + ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r336005 - Revert "Support for multiarch runtimes layout"
Author: rupprecht Date: Fri Jun 29 12:05:20 2018 New Revision: 336005 URL: http://llvm.org/viewvc/llvm-project?rev=336005&view=rev Log: Revert "Support for multiarch runtimes layout" This reverts commit 0c7cea3c0c6338b99e30c13201365a3dd4edc6f4. Modified: libunwind/trunk/CMakeLists.txt Modified: libunwind/trunk/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/libunwind/trunk/CMakeLists.txt?rev=336005&r1=336004&r2=336005&view=diff == --- libunwind/trunk/CMakeLists.txt (original) +++ libunwind/trunk/CMakeLists.txt Fri Jun 29 12:05:20 2018 @@ -170,14 +170,7 @@ set(CMAKE_MODULE_PATH set(LIBUNWIND_COMPILER${CMAKE_CXX_COMPILER}) set(LIBUNWIND_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}) set(LIBUNWIND_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}) - -string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION - ${PACKAGE_VERSION}) - -if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) - set(DEFAULT_INSTALL_PREFIX lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/) - set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LLVM_RUNTIMES_LIBDIR_SUFFIX}) -elseif(LLVM_LIBRARY_OUTPUT_INTDIR) +if (LLVM_LIBRARY_OUTPUT_INTDIR) set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}) else() set(LIBUNWIND_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBUNWIND_LIBDIR_SUFFIX}) @@ -187,9 +180,13 @@ set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIB set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LIBUNWIND_LIBRARY_DIR}) set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LIBUNWIND_LIBRARY_DIR}) -set(LIBUNWIND_INSTALL_PREFIX ${DEFAULT_INSTALL_PREFIX} CACHE STRING +set(LIBUNWIND_INSTALL_PREFIX "" CACHE STRING "Define libunwind destination prefix.") +if (NOT LIBUNWIND_INSTALL_PREFIX MATCHES "^$|.*/") + message(FATAL_ERROR "LIBUNWIND_INSTALL_PREFIX has to end with \"/\".") +endif() + set(LIBUNWIND_C_FLAGS "") set(LIBUNWIND_CXX_FLAGS "") set(LIBUNWIND_COMPILE_FLAGS "") ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48785: Add a blank line to docs/README.txt test commit access
This revision was automatically updated to reflect the committed changes. Closed by commit rL336006: Add a blank line to docs/README.txt test commit access (authored by rupprecht, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D48785 Files: libunwind/trunk/docs/README.txt Index: libunwind/trunk/docs/README.txt === --- libunwind/trunk/docs/README.txt +++ libunwind/trunk/docs/README.txt @@ -11,3 +11,4 @@ After configuring libunwind with these options the make rule `docs-libunwind-html` should be available. + Index: libunwind/trunk/docs/README.txt === --- libunwind/trunk/docs/README.txt +++ libunwind/trunk/docs/README.txt @@ -11,3 +11,4 @@ After configuring libunwind with these options the make rule `docs-libunwind-html` should be available. + ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48786: [Preprocessor] Stop entering included files after hitting a fatal error.
vsapsai created this revision. vsapsai added reviewers: bruno, rsmith. Herald added a subscriber: dexonsmith. Fixes a problem when we have multiple inclusion cycles and try to enumerate all possible ways to reach the max inclusion depth. rdar://problem/38871876 https://reviews.llvm.org/D48786 Files: clang/lib/Lex/PPDirectives.cpp clang/test/Preprocessor/Inputs/cycle/a.h clang/test/Preprocessor/Inputs/cycle/b.h clang/test/Preprocessor/Inputs/cycle/c.h clang/test/Preprocessor/include-cycle.c Index: clang/test/Preprocessor/include-cycle.c === --- /dev/null +++ clang/test/Preprocessor/include-cycle.c @@ -0,0 +1,5 @@ +// RUN: not %clang_cc1 -E -I%S/Inputs -ferror-limit 20 %s + +// Test that preprocessing terminates even if we have inclusion cycles. + +#include "cycle/a.h" Index: clang/test/Preprocessor/Inputs/cycle/c.h === --- /dev/null +++ clang/test/Preprocessor/Inputs/cycle/c.h @@ -0,0 +1 @@ +#include "a.h" Index: clang/test/Preprocessor/Inputs/cycle/b.h === --- /dev/null +++ clang/test/Preprocessor/Inputs/cycle/b.h @@ -0,0 +1 @@ +#include "a.h" Index: clang/test/Preprocessor/Inputs/cycle/a.h === --- /dev/null +++ clang/test/Preprocessor/Inputs/cycle/a.h @@ -0,0 +1,8 @@ +// Presence of 2 inclusion cycles +//b.h -> a.h -> b.h -> ... +//c.h -> a.h -> c.h -> ... +// makes it unfeasible to reach max inclusion depth in all possible ways. Need +// to stop earlier. + +#include "b.h" +#include "c.h" Index: clang/lib/Lex/PPDirectives.cpp === --- clang/lib/Lex/PPDirectives.cpp +++ clang/lib/Lex/PPDirectives.cpp @@ -1871,6 +1871,11 @@ if (PPOpts->SingleFileParseMode) ShouldEnter = false; + // Stop further preprocessing if a fatal error has occurred. Any diagnostics + // we might have raised will not be visible. + if (ShouldEnter && Diags->hasFatalErrorOccurred()) +ShouldEnter = false; + // Determine whether we should try to import the module for this #include, if // there is one. Don't do so if precompiled module support is disabled or we // are processing this module textually (because we're building the module). Index: clang/test/Preprocessor/include-cycle.c === --- /dev/null +++ clang/test/Preprocessor/include-cycle.c @@ -0,0 +1,5 @@ +// RUN: not %clang_cc1 -E -I%S/Inputs -ferror-limit 20 %s + +// Test that preprocessing terminates even if we have inclusion cycles. + +#include "cycle/a.h" Index: clang/test/Preprocessor/Inputs/cycle/c.h === --- /dev/null +++ clang/test/Preprocessor/Inputs/cycle/c.h @@ -0,0 +1 @@ +#include "a.h" Index: clang/test/Preprocessor/Inputs/cycle/b.h === --- /dev/null +++ clang/test/Preprocessor/Inputs/cycle/b.h @@ -0,0 +1 @@ +#include "a.h" Index: clang/test/Preprocessor/Inputs/cycle/a.h === --- /dev/null +++ clang/test/Preprocessor/Inputs/cycle/a.h @@ -0,0 +1,8 @@ +// Presence of 2 inclusion cycles +//b.h -> a.h -> b.h -> ... +//c.h -> a.h -> c.h -> ... +// makes it unfeasible to reach max inclusion depth in all possible ways. Need +// to stop earlier. + +#include "b.h" +#include "c.h" Index: clang/lib/Lex/PPDirectives.cpp === --- clang/lib/Lex/PPDirectives.cpp +++ clang/lib/Lex/PPDirectives.cpp @@ -1871,6 +1871,11 @@ if (PPOpts->SingleFileParseMode) ShouldEnter = false; + // Stop further preprocessing if a fatal error has occurred. Any diagnostics + // we might have raised will not be visible. + if (ShouldEnter && Diags->hasFatalErrorOccurred()) +ShouldEnter = false; + // Determine whether we should try to import the module for this #include, if // there is one. Don't do so if precompiled module support is disabled or we // are processing this module textually (because we're building the module). ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336008 - Request init/fini array on FreeBSD 12 and later
Author: dim Date: Fri Jun 29 12:18:17 2018 New Revision: 336008 URL: http://llvm.org/viewvc/llvm-project?rev=336008&view=rev Log: Request init/fini array on FreeBSD 12 and later Summary: It seems a bad idea to change the default in the middle of a release branch due to possible changes in global ctor / dtor ordering between .ctors and .init_array. With FreeBSD 11.0's release imminent lets change the default now for FreeBSD 12 (the current development stream) and later. FreeBSD rtld has supported .init_array / .fini_array for many years. As of Jan 1 2017 all supported FreeBSD releases and branches will have support. Reviewers: dim, brooks, arichardson Reviewed By: dim, brooks, arichardson Subscribers: bsdjhb, krytarowski, emaste, cfe-commits Differential Revision: https://reviews.llvm.org/D24867 Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp cfe/trunk/test/Driver/constructors.c Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=336008&r1=336007&r2=336008&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Fri Jun 29 12:18:17 2018 @@ -2543,6 +2543,8 @@ void Generic_ELF::addClangTargetOptions( bool UseInitArrayDefault = getTriple().getArch() == llvm::Triple::aarch64 || getTriple().getArch() == llvm::Triple::aarch64_be || + (getTriple().getOS() == llvm::Triple::FreeBSD && + getTriple().getOSMajorVersion() >= 12) || (getTriple().getOS() == llvm::Triple::Linux && ((!GCCInstallation.isValid() || !V.isOlderThan(4, 7, 0)) || getTriple().isAndroid())) || Modified: cfe/trunk/test/Driver/constructors.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/constructors.c?rev=336008&r1=336007&r2=336008&view=diff == --- cfe/trunk/test/Driver/constructors.c (original) +++ cfe/trunk/test/Driver/constructors.c Fri Jun 29 12:18:17 2018 @@ -80,6 +80,14 @@ // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \ // RUN: -target arm64-none-none-eabi \ // RUN: | FileCheck --check-prefix=CHECK-INIT-ARRAY %s + +// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \ +// RUN: -target i386-unknown-freebsd11 \ +// RUN: | FileCheck --check-prefix=CHECK-NO-INIT-ARRAY %s + +// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \ +// RUN: -target i386-unknown-freebsd12 \ +// RUN: | FileCheck --check-prefix=CHECK-INIT-ARRAY %s // // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1\ // RUN: -target sparc-sun-solaris2.11 \ ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24867: Request init/fini array on FreeBSD 12 and later
This revision was automatically updated to reflect the committed changes. Closed by commit rL336008: Request init/fini array on FreeBSD 12 and later (authored by dim, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D24867?vs=72282&id=153554#toc Repository: rL LLVM https://reviews.llvm.org/D24867 Files: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp cfe/trunk/test/Driver/constructors.c Index: cfe/trunk/test/Driver/constructors.c === --- cfe/trunk/test/Driver/constructors.c +++ cfe/trunk/test/Driver/constructors.c @@ -80,6 +80,14 @@ // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \ // RUN: -target arm64-none-none-eabi \ // RUN: | FileCheck --check-prefix=CHECK-INIT-ARRAY %s + +// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \ +// RUN: -target i386-unknown-freebsd11 \ +// RUN: | FileCheck --check-prefix=CHECK-NO-INIT-ARRAY %s + +// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \ +// RUN: -target i386-unknown-freebsd12 \ +// RUN: | FileCheck --check-prefix=CHECK-INIT-ARRAY %s // // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1\ // RUN: -target sparc-sun-solaris2.11 \ Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp === --- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp +++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp @@ -2543,6 +2543,8 @@ bool UseInitArrayDefault = getTriple().getArch() == llvm::Triple::aarch64 || getTriple().getArch() == llvm::Triple::aarch64_be || + (getTriple().getOS() == llvm::Triple::FreeBSD && + getTriple().getOSMajorVersion() >= 12) || (getTriple().getOS() == llvm::Triple::Linux && ((!GCCInstallation.isValid() || !V.isOlderThan(4, 7, 0)) || getTriple().isAndroid())) || Index: cfe/trunk/test/Driver/constructors.c === --- cfe/trunk/test/Driver/constructors.c +++ cfe/trunk/test/Driver/constructors.c @@ -80,6 +80,14 @@ // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \ // RUN: -target arm64-none-none-eabi \ // RUN: | FileCheck --check-prefix=CHECK-INIT-ARRAY %s + +// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \ +// RUN: -target i386-unknown-freebsd11 \ +// RUN: | FileCheck --check-prefix=CHECK-NO-INIT-ARRAY %s + +// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \ +// RUN: -target i386-unknown-freebsd12 \ +// RUN: | FileCheck --check-prefix=CHECK-INIT-ARRAY %s // // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1\ // RUN: -target sparc-sun-solaris2.11 \ Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp === --- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp +++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp @@ -2543,6 +2543,8 @@ bool UseInitArrayDefault = getTriple().getArch() == llvm::Triple::aarch64 || getTriple().getArch() == llvm::Triple::aarch64_be || + (getTriple().getOS() == llvm::Triple::FreeBSD && + getTriple().getOSMajorVersion() >= 12) || (getTriple().getOS() == llvm::Triple::Linux && ((!GCCInstallation.isValid() || !V.isOlderThan(4, 7, 0)) || getTriple().isAndroid())) || ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48719: [clang-format] Fix split priorities for ObjC methods
benhamilton accepted this revision. benhamilton added a comment. This revision is now accepted and ready to land. Nice improvement! Repository: rC Clang https://reviews.llvm.org/D48719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D45454: Add llvm_gcov_flush to be called outside a shared library
OK! Sounds good to me to keep it hidden until https://reviews.llvm.org/D48538 is done (I'm going to try to finish it soon). Il 29/06/2018 19:34, David Li via Phabricator ha scritto: > davidxl added a comment. > > With the current gcov_flush implementation in LLVM, making gcov_flush's > visibility to be default will simply lead to wrong behavior. GCC libgcov's > implementation is more elaborate -- it allows gcov_flush to dump gcda data > for all dynamic objects while making sure gcov_exit only dumps the gcda files > from the same dynamic module (otherwise, there will be wrong profile > merging). This is done via two levels of linked list. > > The patch https://reviews.llvm.org/D48538 is in the right direction, but not > complete yet. > > > https://reviews.llvm.org/D45454 > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45454: Add llvm_gcov_flush to be called outside a shared library
marco-c added a comment. OK! Sounds good to me to keep it hidden until https://reviews.llvm.org/D48538 is done (I'm going to try to finish it soon). Il 29/06/2018 19:34, David Li via Phabricator ha scritto: > davidxl added a comment. > > With the current gcov_flush implementation in LLVM, making gcov_flush's > visibility to be default will simply lead to wrong behavior. GCC libgcov's > implementation is more elaborate -- it allows gcov_flush to dump gcda data > for all dynamic objects while making sure gcov_exit only dumps the gcda files > from the same dynamic module (otherwise, there will be wrong profile > merging). This is done via two levels of linked list. > > The patch https://reviews.llvm.org/D48538 is in the right direction, but not > complete yet. > > https://reviews.llvm.org/D45454 https://reviews.llvm.org/D45454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48787: Un-revert "Support for multiarch runtimes layout"This reverts https://reviews.llvm.org/rL336005, which was an accidental commit.
rupprecht created this revision. Herald added subscribers: cfe-commits, christof, mgorny. Repository: rUNW libunwind https://reviews.llvm.org/D48787 Files: CMakeLists.txt Index: CMakeLists.txt === --- CMakeLists.txt +++ CMakeLists.txt @@ -170,7 +170,14 @@ set(LIBUNWIND_COMPILER${CMAKE_CXX_COMPILER}) set(LIBUNWIND_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}) set(LIBUNWIND_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}) -if (LLVM_LIBRARY_OUTPUT_INTDIR) + +string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION + ${PACKAGE_VERSION}) + +if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) + set(DEFAULT_INSTALL_PREFIX lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/) + set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LLVM_RUNTIMES_LIBDIR_SUFFIX}) +elseif(LLVM_LIBRARY_OUTPUT_INTDIR) set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}) else() set(LIBUNWIND_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBUNWIND_LIBDIR_SUFFIX}) @@ -180,13 +187,9 @@ set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LIBUNWIND_LIBRARY_DIR}) set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LIBUNWIND_LIBRARY_DIR}) -set(LIBUNWIND_INSTALL_PREFIX "" CACHE STRING +set(LIBUNWIND_INSTALL_PREFIX ${DEFAULT_INSTALL_PREFIX} CACHE STRING "Define libunwind destination prefix.") -if (NOT LIBUNWIND_INSTALL_PREFIX MATCHES "^$|.*/") - message(FATAL_ERROR "LIBUNWIND_INSTALL_PREFIX has to end with \"/\".") -endif() - set(LIBUNWIND_C_FLAGS "") set(LIBUNWIND_CXX_FLAGS "") set(LIBUNWIND_COMPILE_FLAGS "") Index: CMakeLists.txt === --- CMakeLists.txt +++ CMakeLists.txt @@ -170,7 +170,14 @@ set(LIBUNWIND_COMPILER${CMAKE_CXX_COMPILER}) set(LIBUNWIND_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}) set(LIBUNWIND_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}) -if (LLVM_LIBRARY_OUTPUT_INTDIR) + +string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION + ${PACKAGE_VERSION}) + +if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) + set(DEFAULT_INSTALL_PREFIX lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/) + set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LLVM_RUNTIMES_LIBDIR_SUFFIX}) +elseif(LLVM_LIBRARY_OUTPUT_INTDIR) set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}) else() set(LIBUNWIND_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBUNWIND_LIBDIR_SUFFIX}) @@ -180,13 +187,9 @@ set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LIBUNWIND_LIBRARY_DIR}) set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LIBUNWIND_LIBRARY_DIR}) -set(LIBUNWIND_INSTALL_PREFIX "" CACHE STRING +set(LIBUNWIND_INSTALL_PREFIX ${DEFAULT_INSTALL_PREFIX} CACHE STRING "Define libunwind destination prefix.") -if (NOT LIBUNWIND_INSTALL_PREFIX MATCHES "^$|.*/") - message(FATAL_ERROR "LIBUNWIND_INSTALL_PREFIX has to end with \"/\".") -endif() - set(LIBUNWIND_C_FLAGS "") set(LIBUNWIND_CXX_FLAGS "") set(LIBUNWIND_COMPILE_FLAGS "") ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48322: [Sema] Discarded statment should be an evaluatable context
erik.pilkington added a comment. Herald added a subscriber: dexonsmith. Ping! Repository: rC Clang https://reviews.llvm.org/D48322 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48720: [clang-format] Put ObjC method arguments into one line when they fit
benhamilton accepted this revision. benhamilton added inline comments. This revision is now accepted and ready to land. Comment at: lib/Format/ContinuationIndenter.cpp:1411 + // line). + if (Current.MatchingParen && Current.MatchingParen->Previous) { +const FormatToken &CurrentScopeOpener = *Current.MatchingParen->Previous; Should we check if `State.Stack.back().BreakBeforeParameter` is `true` before doing any of this? Repository: rC Clang https://reviews.llvm.org/D48720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48322: [Sema] Discarded statment should be an evaluatable context
rsmith added a comment. Hmm, so this will mean that we can have internal linkage declarations marked `Used` for which there is no definition, and we need to not warn on that. I think it might be better to avoid marking things used in this case (even though they're still technically odr-used). Repository: rC Clang https://reviews.llvm.org/D48322 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D47846: [clangd] Implementation of textDocument/documentSymbol
malaperle added inline comments. Comment at: clangd/FindSymbols.cpp:181 +/// Finds document symbols in the main file of the AST. +class DocumentSymbolsConsumer : public index::IndexDataConsumer { + ASTContext &AST; sammccall wrote: > I guess the alternative here is to use SymbolCollector and then convert > Symbol->SymbolInformation (which we should already have for workspace/symbol). > > I also guess there's some divergence in behavior, which is why you went this > route :-) > Mostly in filtering? (I'd think that for e.g. printing, we'd want to be > consistent) > > Do you think it's worth adding enough customization to SymbolCollector to > make it usable here? Even if it was making `shouldFilterDecl` a > std::function, there'd be some value in unifying the rest. WDYT? I put a bit of the justification in the summary, perhaps you missed it or maybe did you think it was not enough of a justification? > An AST-based approach is used to retrieve the document symbols rather than an > in-memory index query. The index is not an ideal fit to achieve this because > of > the file-centric query being done here whereas the index is suited for > project-wide queries. Document symbols also includes more symbols and need to > keep the order as seen in the file. It's not a clear cut decision but I felt that there was more diverging parts than common and that it would be detrimental to SymbolCollector in terms of added complexity. Differences: - We need a different way to filter (as you suggested) - Don't need anything about completion - Don't need to distinguish declaration vs definition or canonical declaration - Don't need a map, we just need them in same order as they appear and SymbolCollector should be free to store them in whichever order for purposes of proving a project-wide collection of symbols - Don't want to track references - DocumentSymbols needs symbols in main files Common things: - Both need to generate Position/Uri from a SourceLocation. But they can both call sourceLocToPosition. - Both need to generate a symbol name from Decl&. But they can both call AST.h's printQualifiedName (I fixed this in a new version). - Both use/extend a IndexDataConsumer. Not worth sharing the same code path just to not have to create a class definition IMO. Let me know if that makes sense to you, otherwise I can try to make another version that uses SymbolCollector. Comment at: unittests/clangd/FindSymbolsTests.cpp:447 + EXPECT_THAT(getSymbols(FilePath), + ElementsAre(AllOf(QName("Tmpl"), WithKind(SymbolKind::Struct)), + AllOf(QName("Tmpl::x"), WithKind(SymbolKind::Field)), sammccall wrote: > in a perfect world, I think the template definitions might be printed > `Tmpl`, `Tmpl`. Not sure about `Tmpl::x` vs `Tmpl::x` though. WDYT? > > (Not necessarily worth doing this patch, keep it simple) I'm thinking `Tmpl`, `Tmpl`, so that we can more easily distinguish between a primary and specialization, especially in partial specializations and when the generic type might not be just a single letter. And `Tmpl::x`. Maybe this is too much :) Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D47846 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r336012 - Revert "Revert "Support for multiarch runtimes layout""
Author: echristo Date: Fri Jun 29 13:27:40 2018 New Revision: 336012 URL: http://llvm.org/viewvc/llvm-project?rev=336012&view=rev Log: Revert "Revert "Support for multiarch runtimes layout"" This reverts commit r336005 that was accidentally committed. Modified: libunwind/trunk/CMakeLists.txt Modified: libunwind/trunk/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/libunwind/trunk/CMakeLists.txt?rev=336012&r1=336011&r2=336012&view=diff == --- libunwind/trunk/CMakeLists.txt (original) +++ libunwind/trunk/CMakeLists.txt Fri Jun 29 13:27:40 2018 @@ -170,7 +170,14 @@ set(CMAKE_MODULE_PATH set(LIBUNWIND_COMPILER${CMAKE_CXX_COMPILER}) set(LIBUNWIND_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}) set(LIBUNWIND_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}) -if (LLVM_LIBRARY_OUTPUT_INTDIR) + +string(REGEX MATCH "[0-9]+\\.[0-9]+(\\.[0-9]+)?" CLANG_VERSION + ${PACKAGE_VERSION}) + +if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE) + set(DEFAULT_INSTALL_PREFIX lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/) + set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/${LLVM_DEFAULT_TARGET_TRIPLE}/lib${LLVM_RUNTIMES_LIBDIR_SUFFIX}) +elseif(LLVM_LIBRARY_OUTPUT_INTDIR) set(LIBUNWIND_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}) else() set(LIBUNWIND_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBUNWIND_LIBDIR_SUFFIX}) @@ -180,13 +187,9 @@ set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LIB set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LIBUNWIND_LIBRARY_DIR}) set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LIBUNWIND_LIBRARY_DIR}) -set(LIBUNWIND_INSTALL_PREFIX "" CACHE STRING +set(LIBUNWIND_INSTALL_PREFIX ${DEFAULT_INSTALL_PREFIX} CACHE STRING "Define libunwind destination prefix.") -if (NOT LIBUNWIND_INSTALL_PREFIX MATCHES "^$|.*/") - message(FATAL_ERROR "LIBUNWIND_INSTALL_PREFIX has to end with \"/\".") -endif() - set(LIBUNWIND_C_FLAGS "") set(LIBUNWIND_CXX_FLAGS "") set(LIBUNWIND_COMPILE_FLAGS "") ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48616: Implement LWG 2946, 3075 and 3076
ldionne accepted this revision. ldionne added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D48616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r336013 - Specify an explicit underlying type for this enum to fix Windows
Author: rsmith Date: Fri Jun 29 13:41:23 2018 New Revision: 336013 URL: http://llvm.org/viewvc/llvm-project?rev=336013&view=rev Log: Specify an explicit underlying type for this enum to fix Windows buildbots. On Windows targets, enums always get an underlying type of 'int', even if they have wider enumerators. (This is non-conforming, but it's effectively part of the target ABI.) Modified: cfe/trunk/test/CXX/conv/conv.prom/p5.cpp Modified: cfe/trunk/test/CXX/conv/conv.prom/p5.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/conv/conv.prom/p5.cpp?rev=336013&r1=336012&r2=336013&view=diff == --- cfe/trunk/test/CXX/conv/conv.prom/p5.cpp (original) +++ cfe/trunk/test/CXX/conv/conv.prom/p5.cpp Fri Jun 29 13:41:23 2018 @@ -8,7 +8,7 @@ static_assert(sizeof(+X().e) == sizeof(i static_assert(sizeof(X().e + 1) == sizeof(int), ""); static_assert(sizeof(true ? X().e : 0) == sizeof(int), ""); -enum E { a = __LONG_LONG_MAX__ }; +enum E : long long { a = __LONG_LONG_MAX__ }; static_assert(sizeof(E{}) == sizeof(long long), ""); // If the bit-field has an enumerated type, it is treated as any other value of ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r336014 - Introduce a separate preprocessor macro, _LIBUNWIND_USE_DLADDR, for directly controlling a dependency on dladdr(). This will allow us to use libunwind without adding a libdl depe
Author: rupprecht Date: Fri Jun 29 13:41:50 2018 New Revision: 336014 URL: http://llvm.org/viewvc/llvm-project?rev=336014&view=rev Log: Introduce a separate preprocessor macro, _LIBUNWIND_USE_DLADDR, for directly controlling a dependency on dladdr(). This will allow us to use libunwind without adding a libdl dependency. Reviewers: saugustine Subscribers: christof, chrib, cfe-commits, echristo Differential Revision: https://reviews.llvm.org/D48733 Modified: libunwind/trunk/src/AddressSpace.hpp Modified: libunwind/trunk/src/AddressSpace.hpp URL: http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/AddressSpace.hpp?rev=336014&r1=336013&r2=336014&view=diff == --- libunwind/trunk/src/AddressSpace.hpp (original) +++ libunwind/trunk/src/AddressSpace.hpp Fri Jun 29 13:41:50 2018 @@ -18,7 +18,15 @@ #include #include -#if !defined(_LIBUNWIND_IS_BAREMETAL) && !defined(_WIN32) +#ifndef _LIBUNWIND_USE_DLADDR + #if !defined(_LIBUNWIND_IS_BAREMETAL) && !defined(_WIN32) +#define _LIBUNWIND_USE_DLADDR 1 + #else +#define _LIBUNWIND_USE_DLADDR 0 + #endif +#endif + +#if _LIBUNWIND_USE_DLADDR #include #endif @@ -576,7 +584,7 @@ inline bool LocalAddressSpace::findOther inline bool LocalAddressSpace::findFunctionName(pint_t addr, char *buf, size_t bufLen, unw_word_t *offset) { -#if !defined(_LIBUNWIND_IS_BAREMETAL) && !defined(_WIN32) +#if _LIBUNWIND_USE_DLADDR Dl_info dyldInfo; if (dladdr((void *)addr, &dyldInfo)) { if (dyldInfo.dli_sname != NULL) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48733: Introduce a separate preprocessor macro, _LIBUNWIND_USE_DLADDR, for directly controlling a dependency on dladdr(). This will allow us to use libunwind without adding a libdl dependency
This revision was automatically updated to reflect the committed changes. Closed by commit rL336014: Introduce a separate preprocessor macro, _LIBUNWIND_USE_DLADDR, for directly… (authored by rupprecht, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D48733 Files: libunwind/trunk/src/AddressSpace.hpp Index: libunwind/trunk/src/AddressSpace.hpp === --- libunwind/trunk/src/AddressSpace.hpp +++ libunwind/trunk/src/AddressSpace.hpp @@ -18,7 +18,15 @@ #include #include -#if !defined(_LIBUNWIND_IS_BAREMETAL) && !defined(_WIN32) +#ifndef _LIBUNWIND_USE_DLADDR + #if !defined(_LIBUNWIND_IS_BAREMETAL) && !defined(_WIN32) +#define _LIBUNWIND_USE_DLADDR 1 + #else +#define _LIBUNWIND_USE_DLADDR 0 + #endif +#endif + +#if _LIBUNWIND_USE_DLADDR #include #endif @@ -576,7 +584,7 @@ inline bool LocalAddressSpace::findFunctionName(pint_t addr, char *buf, size_t bufLen, unw_word_t *offset) { -#if !defined(_LIBUNWIND_IS_BAREMETAL) && !defined(_WIN32) +#if _LIBUNWIND_USE_DLADDR Dl_info dyldInfo; if (dladdr((void *)addr, &dyldInfo)) { if (dyldInfo.dli_sname != NULL) { Index: libunwind/trunk/src/AddressSpace.hpp === --- libunwind/trunk/src/AddressSpace.hpp +++ libunwind/trunk/src/AddressSpace.hpp @@ -18,7 +18,15 @@ #include #include -#if !defined(_LIBUNWIND_IS_BAREMETAL) && !defined(_WIN32) +#ifndef _LIBUNWIND_USE_DLADDR + #if !defined(_LIBUNWIND_IS_BAREMETAL) && !defined(_WIN32) +#define _LIBUNWIND_USE_DLADDR 1 + #else +#define _LIBUNWIND_USE_DLADDR 0 + #endif +#endif + +#if _LIBUNWIND_USE_DLADDR #include #endif @@ -576,7 +584,7 @@ inline bool LocalAddressSpace::findFunctionName(pint_t addr, char *buf, size_t bufLen, unw_word_t *offset) { -#if !defined(_LIBUNWIND_IS_BAREMETAL) && !defined(_WIN32) +#if _LIBUNWIND_USE_DLADDR Dl_info dyldInfo; if (dladdr((void *)addr, &dyldInfo)) { if (dyldInfo.dli_sname != NULL) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r335925 - PR37979: integral promotions in C++ treat enum bit-fields like enums,
Should be fixed in r336013. Thanks for pointing this out. On Fri, 29 Jun 2018 at 11:24, Galina Kistanova via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Hello Richard, > > This commit broke tests at one of our builders: > > http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/10599 > > Failing Tests (1): > Clang :: CXX/conv/conv.prom/p5.cpp > > Please have a look? > > Thanks > > Galina > > On Thu, Jun 28, 2018 at 2:17 PM, Richard Smith via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: rsmith >> Date: Thu Jun 28 14:17:55 2018 >> New Revision: 335925 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=335925&view=rev >> Log: >> PR37979: integral promotions in C++ treat enum bit-fields like enums, >> not like bit-fields. >> >> We used to get this right "by accident", because conversions for the >> selected built-in overloaded operator would convert the enum bit-field >> to its corresponding underlying type early. But after DR1687 that no >> longer happens. >> >> Technically this change should also apply to C, where bit-fields only >> have special promotion rules if the bit-field's declared type is >> _Bool, int, signed int, or unsigned int, but for GCC compatibility we >> only look at the bit-width and not the underlying type when performing >> bit-field integral promotions in C. >> >> Added: >> cfe/trunk/test/CXX/conv/conv.prom/p5.cpp >> Modified: >> cfe/trunk/lib/AST/ASTContext.cpp >> cfe/trunk/lib/Sema/SemaOverload.cpp >> >> Modified: cfe/trunk/lib/AST/ASTContext.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=335925&r1=335924&r2=335925&view=diff >> >> == >> --- cfe/trunk/lib/AST/ASTContext.cpp (original) >> +++ cfe/trunk/lib/AST/ASTContext.cpp Thu Jun 28 14:17:55 2018 >> @@ -5456,6 +5456,12 @@ QualType ASTContext::isPromotableBitFiel >>if (E->isTypeDependent() || E->isValueDependent()) >> return {}; >> >> + // C++ [conv.prom]p5: >> + //If the bit-field has an enumerated type, it is treated as any >> other >> + //value of that type for promotion purposes. >> + if (getLangOpts().CPlusPlus && E->getType()->isEnumeralType()) >> +return {}; >> + >>// FIXME: We should not do this unless E->refersToBitField() is true. >> This >>// matters in C where getSourceBitField() will find bit-fields for >> various >>// cases where the source expression is not a bit-field designator. >> @@ -5482,13 +5488,15 @@ QualType ASTContext::isPromotableBitFiel >>// >>// FIXME: C does not permit promotion of a 'long : 3' bitfield to int. >>//We perform that promotion here to match GCC and C++. >> + // FIXME: C does not permit promotion of an enum bit-field whose rank >> is >> + //greater than that of 'int'. We perform that promotion to >> match GCC. >>if (BitWidth < IntSize) >> return IntTy; >> >>if (BitWidth == IntSize) >> return FT->isSignedIntegerType() ? IntTy : UnsignedIntTy; >> >> - // Types bigger than int are not subject to promotions, and therefore >> act >> + // Bit-fields wider than int are not subject to promotions, and >> therefore act >>// like the base type. GCC has some weird bugs in this area that we >>// deliberately do not follow (GCC follows a pre-standard resolution to >>// C's DR315 which treats bit-width as being part of the type, and >> this leaks >> >> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=335925&r1=335924&r2=335925&view=diff >> >> == >> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Thu Jun 28 14:17:55 2018 >> @@ -2007,6 +2007,14 @@ bool Sema::IsIntegralPromotion(Expr *Fro >> isCompleteType(From->getLocStart(), FromType)) >>return Context.hasSameUnqualifiedType( >>ToType, FromEnumType->getDecl()->getPromotionType()); >> + >> +// C++ [conv.prom]p5: >> +// If the bit-field has an enumerated type, it is treated as any >> other >> +// value of that type for promotion purposes. >> +// >> +// ... so do not fall through into the bit-field checks below in C++. >> +if (getLangOpts().CPlusPlus) >> + return false; >>} >> >>// C++0x [conv.prom]p2: >> @@ -2054,6 +2062,11 @@ bool Sema::IsIntegralPromotion(Expr *Fro >>// other value of that type for promotion purposes (C++ 4.5p3). >>// FIXME: We should delay checking of bit-fields until we actually >> perform the >>// conversion. >> + // >> + // FIXME: In C, only bit-fields of types _Bool, int, or unsigned int >> may be >> + // promoted, per C11 6.3.1.1/2. We promote all bit-fields (including >> enum >> + // bit-fields and those whose underlying type is larger than int) for >> GCC >> + // compat
r336016 - [modules] Emit the type of the TypeSourceInfo for a DeclaratorDecl (but
Author: rsmith Date: Fri Jun 29 13:46:25 2018 New Revision: 336016 URL: http://llvm.org/viewvc/llvm-project?rev=336016&view=rev Log: [modules] Emit the type of the TypeSourceInfo for a DeclaratorDecl (but not the corresponding location information) earlier. We need the type as written in order to properly merge functions with deduced return types, so we need to load that early. But we don't want to load the location information early, because that contains problematic things such as the function parameters. Modified: cfe/trunk/include/clang/Serialization/ASTReader.h cfe/trunk/include/clang/Serialization/ASTWriter.h cfe/trunk/lib/Serialization/ASTReader.cpp cfe/trunk/lib/Serialization/ASTReaderDecl.cpp cfe/trunk/lib/Serialization/ASTWriter.cpp cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Modified: cfe/trunk/include/clang/Serialization/ASTReader.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=336016&r1=336015&r2=336016&view=diff == --- cfe/trunk/include/clang/Serialization/ASTReader.h (original) +++ cfe/trunk/include/clang/Serialization/ASTReader.h Fri Jun 29 13:46:25 2018 @@ -1771,6 +1771,10 @@ public: TypeSourceInfo *GetTypeSourceInfo(ModuleFile &F, const RecordData &Record, unsigned &Idx); + /// Raad the type locations for the given TInfo. + void ReadTypeLoc(ModuleFile &F, const RecordData &Record, unsigned &Idx, + TypeLoc TL); + /// Resolve a type ID into a type, potentially building a new /// type. QualType GetType(serialization::TypeID ID); @@ -2460,6 +2464,11 @@ public: return Reader->GetTypeSourceInfo(*F, Record, Idx); } + /// Reads the location information for a type. + void readTypeLoc(TypeLoc TL) { +return Reader->ReadTypeLoc(*F, Record, Idx, TL); + } + /// Map a local type ID within a given AST file to a global type ID. serialization::TypeID getGlobalTypeID(unsigned LocalID) const { return Reader->getGlobalTypeID(*F, LocalID); Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=336016&r1=336015&r2=336016&view=diff == --- cfe/trunk/include/clang/Serialization/ASTWriter.h (original) +++ cfe/trunk/include/clang/Serialization/ASTWriter.h Fri Jun 29 13:46:25 2018 @@ -886,7 +886,7 @@ public: /// Emits a reference to a declarator info. void AddTypeSourceInfo(TypeSourceInfo *TInfo); - /// Emits a type with source-location information. + /// Emits source location information for a type. Does not emit the type. void AddTypeLoc(TypeLoc TL); /// Emits a template argument location info. Modified: cfe/trunk/lib/Serialization/ASTReader.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=336016&r1=336015&r2=336016&view=diff == --- cfe/trunk/lib/Serialization/ASTReader.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri Jun 29 13:46:25 2018 @@ -6733,6 +6733,13 @@ void TypeLocReader::VisitPipeTypeLoc(Pip TL.setKWLoc(ReadSourceLocation()); } +void ASTReader::ReadTypeLoc(ModuleFile &F, const ASTReader::RecordData &Record, +unsigned &Idx, TypeLoc TL) { + TypeLocReader TLR(F, *this, Record, Idx); + for (; !TL.isNull(); TL = TL.getNextTypeLoc()) +TLR.Visit(TL); +} + TypeSourceInfo * ASTReader::GetTypeSourceInfo(ModuleFile &F, const ASTReader::RecordData &Record, unsigned &Idx) { @@ -6741,9 +6748,7 @@ ASTReader::GetTypeSourceInfo(ModuleFile return nullptr; TypeSourceInfo *TInfo = getContext().CreateTypeSourceInfo(InfoTy); - TypeLocReader TLR(F, *this, Record, Idx); - for (TypeLoc TL = TInfo->getTypeLoc(); !TL.isNull(); TL = TL.getNextTypeLoc()) -TLR.Visit(TL); + ReadTypeLoc(F, Record, Idx, TInfo->getTypeLoc()); return TInfo; } Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=336016&r1=336015&r2=336016&view=diff == --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Fri Jun 29 13:46:25 2018 @@ -522,13 +522,8 @@ void ASTDeclReader::Visit(Decl *D) { IsDeclMarkedUsed = false; if (auto *DD = dyn_cast(D)) { -if (DD->DeclInfo) { - auto *Info = DD->DeclInfo.get(); - Info->TInfo = GetTypeSourceInfo(); -} -else { - DD->DeclInfo = GetTypeSourceInfo(); -} +if (auto *TInfo = DD->getTypeSourceInfo()) + Record.readTypeLoc(TInfo->getTypeLoc()); } if (auto *TD = dyn_cast(D)) { @@ -815
[PATCH] D45454: Add llvm_gcov_flush to be called outside a shared library
chh updated this revision to Diff 153569. chh marked an inline comment as done. chh added a comment. Added calls to dlerror() before other dl* functions. https://reviews.llvm.org/D45454 Files: lib/profile/GCDAProfiling.c test/profile/Inputs/instrprof-dlopen-dlclose-main.c Index: test/profile/Inputs/instrprof-dlopen-dlclose-main.c === --- test/profile/Inputs/instrprof-dlopen-dlclose-main.c +++ test/profile/Inputs/instrprof-dlopen-dlclose-main.c @@ -10,12 +10,42 @@ return EXIT_FAILURE; } + dlerror(); void *f2_handle = dlopen("func2.shared", RTLD_LAZY | RTLD_GLOBAL); if (f2_handle == NULL) { fprintf(stderr, "unable to open 'func2.shared': %s\n", dlerror()); return EXIT_FAILURE; } + dlerror(); + void (*gcov_flush)() = (void (*)())dlsym(f1_handle, "__gcov_flush"); + if (gcov_flush != NULL) { +fprintf(stderr, "__gcov_flush should not be visible in func.shared'\n"); +return EXIT_FAILURE; + } + + dlerror(); + void (*f1_flush)() = (void (*)())dlsym(f1_handle, "llvm_gcov_flush"); + if (f1_flush == NULL) { +fprintf(stderr, "unable to find llvm_gcov_flush in func.shared': %s\n", dlerror()); +return EXIT_FAILURE; + } + f1_flush(); + + dlerror(); + void (*f2_flush)() = (void (*)())dlsym(f2_handle, "llvm_gcov_flush"); + if (f2_flush == NULL) { +fprintf(stderr, "unable to find llvm_gcov_flush in func2.shared': %s\n", dlerror()); +return EXIT_FAILURE; + } + f2_flush(); + + if (f1_flush == f2_flush) { +fprintf(stderr, "Same llvm_gcov_flush found in func.shared and func2.shared\n"); +return EXIT_FAILURE; + } + + dlerror(); if (dlclose(f2_handle) != 0) { fprintf(stderr, "unable to close 'func2.shared': %s\n", dlerror()); return EXIT_FAILURE; Index: lib/profile/GCDAProfiling.c === --- lib/profile/GCDAProfiling.c +++ lib/profile/GCDAProfiling.c @@ -527,6 +527,10 @@ } } +// __gcov_flush is hidden. When called in a .so file, +// it dumps profile data of the calling .so file. +// If a main program needs to dump profile data of each linked +// .so files, it should use dlsym to find and call llvm_gcov_flush. COMPILER_RT_VISIBILITY void __gcov_flush() { struct flush_fn_node *curr = flush_fn_head; @@ -537,6 +541,14 @@ } } +// llvm_gcov_flush is not hidden for a program to use dlsym to +// find and call. In that case, it dumps profile data of a .so file. +// If it is called directly inside a .so file, the unified copy of +// llvm_gcov_flush might dump data of other .so file or the main module. +void llvm_gcov_flush() { + __gcov_flush(); +} + COMPILER_RT_VISIBILITY void llvm_delete_flush_function_list(void) { while (flush_fn_head) { Index: test/profile/Inputs/instrprof-dlopen-dlclose-main.c === --- test/profile/Inputs/instrprof-dlopen-dlclose-main.c +++ test/profile/Inputs/instrprof-dlopen-dlclose-main.c @@ -10,12 +10,42 @@ return EXIT_FAILURE; } + dlerror(); void *f2_handle = dlopen("func2.shared", RTLD_LAZY | RTLD_GLOBAL); if (f2_handle == NULL) { fprintf(stderr, "unable to open 'func2.shared': %s\n", dlerror()); return EXIT_FAILURE; } + dlerror(); + void (*gcov_flush)() = (void (*)())dlsym(f1_handle, "__gcov_flush"); + if (gcov_flush != NULL) { +fprintf(stderr, "__gcov_flush should not be visible in func.shared'\n"); +return EXIT_FAILURE; + } + + dlerror(); + void (*f1_flush)() = (void (*)())dlsym(f1_handle, "llvm_gcov_flush"); + if (f1_flush == NULL) { +fprintf(stderr, "unable to find llvm_gcov_flush in func.shared': %s\n", dlerror()); +return EXIT_FAILURE; + } + f1_flush(); + + dlerror(); + void (*f2_flush)() = (void (*)())dlsym(f2_handle, "llvm_gcov_flush"); + if (f2_flush == NULL) { +fprintf(stderr, "unable to find llvm_gcov_flush in func2.shared': %s\n", dlerror()); +return EXIT_FAILURE; + } + f2_flush(); + + if (f1_flush == f2_flush) { +fprintf(stderr, "Same llvm_gcov_flush found in func.shared and func2.shared\n"); +return EXIT_FAILURE; + } + + dlerror(); if (dlclose(f2_handle) != 0) { fprintf(stderr, "unable to close 'func2.shared': %s\n", dlerror()); return EXIT_FAILURE; Index: lib/profile/GCDAProfiling.c === --- lib/profile/GCDAProfiling.c +++ lib/profile/GCDAProfiling.c @@ -527,6 +527,10 @@ } } +// __gcov_flush is hidden. When called in a .so file, +// it dumps profile data of the calling .so file. +// If a main program needs to dump profile data of each linked +// .so files, it should use dlsym to find and call llvm_gcov_flush. COMPILER_RT_VISIBILITY void __gcov_flush() { struct flush_fn_node *curr = flush_fn_head; @@ -537,6 +541,14 @@ } } +// llvm_gcov_flush is not hidden for a program to use dlsym to +// find and call. In that cas
[PATCH] D45454: Add llvm_gcov_flush to be called outside a shared library
davidxl added inline comments. Comment at: lib/profile/GCDAProfiling.c:546 +// find and call. In that case, it dumps profile data of a .so file. +// If it is called directly inside a .so file, the unified copy of +// llvm_gcov_flush might dump data of other .so file or the main module. Just document this interface has non-hidden visibility and will resolve to a unified copy. The right underlying behavior is for it to dump profiles from all dynamic modules, but it is not there until Marco's patch is in. In other words, do not need to document the current behavior for now. https://reviews.llvm.org/D45454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45454: Add llvm_gcov_flush to be called outside a shared library
chh updated this revision to Diff 153573. chh added a comment. check dlerror() where it shouldn't be NULL https://reviews.llvm.org/D45454 Files: lib/profile/GCDAProfiling.c test/profile/Inputs/instrprof-dlopen-dlclose-main.c Index: test/profile/Inputs/instrprof-dlopen-dlclose-main.c === --- test/profile/Inputs/instrprof-dlopen-dlclose-main.c +++ test/profile/Inputs/instrprof-dlopen-dlclose-main.c @@ -10,12 +10,42 @@ return EXIT_FAILURE; } + dlerror(); void *f2_handle = dlopen("func2.shared", RTLD_LAZY | RTLD_GLOBAL); if (f2_handle == NULL) { fprintf(stderr, "unable to open 'func2.shared': %s\n", dlerror()); return EXIT_FAILURE; } + dlerror(); + void (*gcov_flush)() = (void (*)())dlsym(f1_handle, "__gcov_flush"); + if (gcov_flush != NULL || dlerror() == NULL) { +fprintf(stderr, "__gcov_flush should not be visible in func.shared'\n"); +return EXIT_FAILURE; + } + + dlerror(); + void (*f1_flush)() = (void (*)())dlsym(f1_handle, "llvm_gcov_flush"); + if (f1_flush == NULL) { +fprintf(stderr, "unable to find llvm_gcov_flush in func.shared': %s\n", dlerror()); +return EXIT_FAILURE; + } + f1_flush(); + + dlerror(); + void (*f2_flush)() = (void (*)())dlsym(f2_handle, "llvm_gcov_flush"); + if (f2_flush == NULL) { +fprintf(stderr, "unable to find llvm_gcov_flush in func2.shared': %s\n", dlerror()); +return EXIT_FAILURE; + } + f2_flush(); + + if (f1_flush == f2_flush) { +fprintf(stderr, "Same llvm_gcov_flush found in func.shared and func2.shared\n"); +return EXIT_FAILURE; + } + + dlerror(); if (dlclose(f2_handle) != 0) { fprintf(stderr, "unable to close 'func2.shared': %s\n", dlerror()); return EXIT_FAILURE; Index: lib/profile/GCDAProfiling.c === --- lib/profile/GCDAProfiling.c +++ lib/profile/GCDAProfiling.c @@ -527,6 +527,10 @@ } } +// __gcov_flush is hidden. When called in a .so file, +// it dumps profile data of the calling .so file. +// If a main program needs to dump profile data of each linked +// .so files, it should use dlsym to find and call llvm_gcov_flush. COMPILER_RT_VISIBILITY void __gcov_flush() { struct flush_fn_node *curr = flush_fn_head; @@ -537,6 +541,14 @@ } } +// llvm_gcov_flush is not hidden for a program to use dlsym to +// find and call. In that case, it dumps profile data of a .so file. +// If it is called directly inside a .so file, the unified copy of +// llvm_gcov_flush might dump data of other .so file or the main module. +void llvm_gcov_flush() { + __gcov_flush(); +} + COMPILER_RT_VISIBILITY void llvm_delete_flush_function_list(void) { while (flush_fn_head) { Index: test/profile/Inputs/instrprof-dlopen-dlclose-main.c === --- test/profile/Inputs/instrprof-dlopen-dlclose-main.c +++ test/profile/Inputs/instrprof-dlopen-dlclose-main.c @@ -10,12 +10,42 @@ return EXIT_FAILURE; } + dlerror(); void *f2_handle = dlopen("func2.shared", RTLD_LAZY | RTLD_GLOBAL); if (f2_handle == NULL) { fprintf(stderr, "unable to open 'func2.shared': %s\n", dlerror()); return EXIT_FAILURE; } + dlerror(); + void (*gcov_flush)() = (void (*)())dlsym(f1_handle, "__gcov_flush"); + if (gcov_flush != NULL || dlerror() == NULL) { +fprintf(stderr, "__gcov_flush should not be visible in func.shared'\n"); +return EXIT_FAILURE; + } + + dlerror(); + void (*f1_flush)() = (void (*)())dlsym(f1_handle, "llvm_gcov_flush"); + if (f1_flush == NULL) { +fprintf(stderr, "unable to find llvm_gcov_flush in func.shared': %s\n", dlerror()); +return EXIT_FAILURE; + } + f1_flush(); + + dlerror(); + void (*f2_flush)() = (void (*)())dlsym(f2_handle, "llvm_gcov_flush"); + if (f2_flush == NULL) { +fprintf(stderr, "unable to find llvm_gcov_flush in func2.shared': %s\n", dlerror()); +return EXIT_FAILURE; + } + f2_flush(); + + if (f1_flush == f2_flush) { +fprintf(stderr, "Same llvm_gcov_flush found in func.shared and func2.shared\n"); +return EXIT_FAILURE; + } + + dlerror(); if (dlclose(f2_handle) != 0) { fprintf(stderr, "unable to close 'func2.shared': %s\n", dlerror()); return EXIT_FAILURE; Index: lib/profile/GCDAProfiling.c === --- lib/profile/GCDAProfiling.c +++ lib/profile/GCDAProfiling.c @@ -527,6 +527,10 @@ } } +// __gcov_flush is hidden. When called in a .so file, +// it dumps profile data of the calling .so file. +// If a main program needs to dump profile data of each linked +// .so files, it should use dlsym to find and call llvm_gcov_flush. COMPILER_RT_VISIBILITY void __gcov_flush() { struct flush_fn_node *curr = flush_fn_head; @@ -537,6 +541,14 @@ } } +// llvm_gcov_flush is not hidden for a program to use dlsym to +// find and call. In that case, it
[PATCH] D45454: Add llvm_gcov_flush to be called outside a shared library
chh added inline comments. Comment at: test/profile/Inputs/instrprof-dlopen-dlclose-main.c:20 + void (*gcov_flush)() = (void (*)())dlsym(f1_handle, "__gcov_flush"); + if (gcov_flush != NULL) { +fprintf(stderr, "__gcov_flush should not be visible in func.shared'\n"); srhines wrote: > Should also clear dlerror() before this call and check that dlerror() > returned a non-NULL pointer indicating that this search failed. Look up of __gcov_flush is expected to fail because it's hidden. Other places checks for returned value and report dlerror() messages. https://reviews.llvm.org/D45454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45454: Add llvm_gcov_flush to be called outside a shared library
srhines added a comment. LGTM. Thanks for making the checking more clear. This should help bridge the gap for now, so that we can resume getting per-library profile data in Android. https://reviews.llvm.org/D45454 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits