[PATCH] D37192: [clang-format] Add support for generic Obj-C categories
danielmartin updated this revision to Diff 112855. danielmartin added a comment. Make comment fit in one line https://reviews.llvm.org/D37192 Files: lib/Format/UnwrappedLineParser.cpp unittests/Format/FormatTestObjC.cpp Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -821,6 +821,13 @@ " NSBundle.mainBundle.infoDictionary[@\"a\"]\n" "]];"); } + +TEST_F(FormatTestObjC, FormatGenericObjCCategory) { + verifyFormat( + "@interface NSHashTable (MYFoundation)\n" + "- (void)xyz_addObjectsFromArray:(nonnull NSArray *)array;\n" + "@end"); +} } // end namespace } // end namespace format } // end namespace clang Index: lib/Format/UnwrappedLineParser.cpp === --- lib/Format/UnwrappedLineParser.cpp +++ lib/Format/UnwrappedLineParser.cpp @@ -2096,6 +2096,10 @@ if (FormatTok->Tok.is(tok::less)) parseObjCProtocolList(); + // After a protocol list, we can have a category (Obj-C generic category). + if (FormatTok->Tok.is(tok::l_paren)) +parseParens(); + if (FormatTok->Tok.is(tok::l_brace)) { if (Style.BraceWrapping.AfterObjCDeclaration) addUnwrappedLine(); Index: unittests/Format/FormatTestObjC.cpp === --- unittests/Format/FormatTestObjC.cpp +++ unittests/Format/FormatTestObjC.cpp @@ -821,6 +821,13 @@ " NSBundle.mainBundle.infoDictionary[@\"a\"]\n" "]];"); } + +TEST_F(FormatTestObjC, FormatGenericObjCCategory) { + verifyFormat( + "@interface NSHashTable (MYFoundation)\n" + "- (void)xyz_addObjectsFromArray:(nonnull NSArray *)array;\n" + "@end"); +} } // end namespace } // end namespace format } // end namespace clang Index: lib/Format/UnwrappedLineParser.cpp === --- lib/Format/UnwrappedLineParser.cpp +++ lib/Format/UnwrappedLineParser.cpp @@ -2096,6 +2096,10 @@ if (FormatTok->Tok.is(tok::less)) parseObjCProtocolList(); + // After a protocol list, we can have a category (Obj-C generic category). + if (FormatTok->Tok.is(tok::l_paren)) +parseParens(); + if (FormatTok->Tok.is(tok::l_brace)) { if (Style.BraceWrapping.AfterObjCDeclaration) addUnwrappedLine(); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check
lebedev.ri added a comment. Ping Repository: rL LLVM https://reviews.llvm.org/D36836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37142: clang-format: [JS] simplify template string wrapping.
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Yay for *removing* complexity for a change :). Let me know how it goes in practice. https://reviews.llvm.org/D37142 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37103: [StaticAnalyzer] LoopUnrolling fixes
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D37103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36962: [StaticAnalyzer] LoopUnrolling: Excluding loops which splits the state (make more branches)
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. LGTM! I guess later it'd be great to comment on every `numTimesReached()` why is it supposed to be reached exactly that many times, as it's often unclear, especially with tricky control flow. Because if somebody accidentally breaks the test, they'd have no idea what it tests or why it should work this way. https://reviews.llvm.org/D36962 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37101: [clangd] [WIP] Add support for snippet completions
ilya-biryukov added a comment. In https://reviews.llvm.org/D37101#853509, @rwols wrote: > After digging into VSCode, I now think that presenting snippets is the wrong > way forward, and I believe the right way is to implement the > `signatureHelpProvider` protocol for method/function parameters. See: > https://github.com/Microsoft/vscode-docs/blob/master/docs/extensionAPI/language-support.md#help-with-function-and-method-signatures. > > I'll update this diff accordingly. I still think having support for snippets is useful in completion. Let's not have snippets for functions and only present them for `RK_Pattern`s. Signature help is useful, but it's a separate feature and should also go in as a separate commit. For example, consider the differences between completion and signature help. vector vec; vec.push_back(/*cursor*/ In that case, - signature help must show all overloads of push_back, show documentation for the first parameter, if any; - code completion must show global completions. https://reviews.llvm.org/D37101 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r311877 - [analyzer][GSoC] Re-implemente current virtual calls checker in a path-sensitive way
Author: xazax Date: Mon Aug 28 01:44:43 2017 New Revision: 311877 URL: http://llvm.org/viewvc/llvm-project?rev=311877&view=rev Log: [analyzer][GSoC] Re-implemente current virtual calls checker in a path-sensitive way Patch by: Xin Wang Differential Revision: https://reviews.llvm.org/D34275 Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp cfe/trunk/test/Analysis/virtualcall.cpp cfe/trunk/test/Analysis/virtualcall.h Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp?rev=311877&r1=311876&r2=311877&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp Mon Aug 28 01:44:43 2017 @@ -14,279 +14,272 @@ #include "ClangSACheckers.h" #include "clang/AST/DeclCXX.h" -#include "clang/AST/StmtVisitor.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" -#include "llvm/ADT/SmallString.h" -#include "llvm/Support/SaveAndRestore.h" -#include "llvm/Support/raw_ostream.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h" using namespace clang; using namespace ento; namespace { - -class WalkAST : public StmtVisitor { - const CheckerBase *Checker; - BugReporter &BR; - AnalysisDeclContext *AC; - - /// The root constructor or destructor whose callees are being analyzed. - const CXXMethodDecl *RootMethod = nullptr; - - /// Whether the checker should walk into bodies of called functions. - /// Controlled by the "Interprocedural" analyzer-config option. - bool IsInterprocedural = false; - - /// Whether the checker should only warn for calls to pure virtual functions - /// (which is undefined behavior) or for all virtual functions (which may - /// may result in unexpected behavior). - bool ReportPureOnly = false; - - typedef const CallExpr * WorkListUnit; - typedef SmallVector DFSWorkList; - - /// A vector representing the worklist which has a chain of CallExprs. - DFSWorkList WList; - - // PreVisited : A CallExpr to this FunctionDecl is in the worklist, but the - // body has not been visited yet. - // PostVisited : A CallExpr to this FunctionDecl is in the worklist, and the - // body has been visited. - enum Kind { NotVisited, - PreVisited, /**< A CallExpr to this FunctionDecl is in the -worklist, but the body has not yet been -visited. */ - PostVisited /**< A CallExpr to this FunctionDecl is in the -worklist, and the body has been visited. */ - }; - - /// A DenseMap that records visited states of FunctionDecls. - llvm::DenseMap VisitedFunctions; - - /// The CallExpr whose body is currently being visited. This is used for - /// generating bug reports. This is null while visiting the body of a - /// constructor or destructor. - const CallExpr *visitingCallExpr; - -public: - WalkAST(const CheckerBase *checker, BugReporter &br, AnalysisDeclContext *ac, - const CXXMethodDecl *rootMethod, bool isInterprocedural, - bool reportPureOnly) - : Checker(checker), BR(br), AC(ac), RootMethod(rootMethod), -IsInterprocedural(isInterprocedural), ReportPureOnly(reportPureOnly), -visitingCallExpr(nullptr) { -// Walking should always start from either a constructor or a destructor. -assert(isa(rootMethod) || - isa(rootMethod)); +enum class ObjectState : bool { CtorCalled, DtorCalled }; +} // end namespace + // FIXME: Ascending over StackFrameContext maybe another method. + +namespace llvm { +template <> struct FoldingSetTrait { + static inline void Profile(ObjectState X, FoldingSetNodeID &ID) { +ID.AddInteger(static_cast(X)); } +}; +} // end namespace llvm - bool hasWork() const { return !WList.empty(); } - - /// This method adds a CallExpr to the worklist and marks the callee as - /// being PreVisited. - void Enqueue(WorkListUnit WLUnit) { -const FunctionDecl *FD = WLUnit->getDirectCallee(); -if (!FD || !FD->getBody()) - return; -Kind &K = VisitedFunctions[FD]; -if (K != NotVisited) - return; -K = PreVisited; -WList.push_back(WLUnit); - } +namespace { +class VirtualCallChecker +: public Checker { + mutable std::unique_ptr BT; - /// This method returns an item from the worklist without removing it. - WorkListUnit Dequeue() { -assert(!
[PATCH] D34275: [analyzer] Re-implemente current virtual calls checker in a path-sensitive way
This revision was automatically updated to reflect the committed changes. Closed by commit rL311877: [analyzer][GSoC] Re-implemente current virtual calls checker in a path… (authored by xazax). Changed prior to commit: https://reviews.llvm.org/D34275?vs=112196&id=112857#toc Repository: rL LLVM https://reviews.llvm.org/D34275 Files: cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp cfe/trunk/test/Analysis/virtualcall.cpp cfe/trunk/test/Analysis/virtualcall.h Index: cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp === --- cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp @@ -14,279 +14,272 @@ #include "ClangSACheckers.h" #include "clang/AST/DeclCXX.h" -#include "clang/AST/StmtVisitor.h" #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" #include "clang/StaticAnalyzer/Core/Checker.h" -#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h" -#include "llvm/ADT/SmallString.h" -#include "llvm/Support/SaveAndRestore.h" -#include "llvm/Support/raw_ostream.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h" using namespace clang; using namespace ento; namespace { - -class WalkAST : public StmtVisitor { - const CheckerBase *Checker; - BugReporter &BR; - AnalysisDeclContext *AC; - - /// The root constructor or destructor whose callees are being analyzed. - const CXXMethodDecl *RootMethod = nullptr; - - /// Whether the checker should walk into bodies of called functions. - /// Controlled by the "Interprocedural" analyzer-config option. - bool IsInterprocedural = false; - - /// Whether the checker should only warn for calls to pure virtual functions - /// (which is undefined behavior) or for all virtual functions (which may - /// may result in unexpected behavior). - bool ReportPureOnly = false; - - typedef const CallExpr * WorkListUnit; - typedef SmallVector DFSWorkList; - - /// A vector representing the worklist which has a chain of CallExprs. - DFSWorkList WList; - - // PreVisited : A CallExpr to this FunctionDecl is in the worklist, but the - // body has not been visited yet. - // PostVisited : A CallExpr to this FunctionDecl is in the worklist, and the - // body has been visited. - enum Kind { NotVisited, - PreVisited, /**< A CallExpr to this FunctionDecl is in the -worklist, but the body has not yet been -visited. */ - PostVisited /**< A CallExpr to this FunctionDecl is in the -worklist, and the body has been visited. */ - }; - - /// A DenseMap that records visited states of FunctionDecls. - llvm::DenseMap VisitedFunctions; - - /// The CallExpr whose body is currently being visited. This is used for - /// generating bug reports. This is null while visiting the body of a - /// constructor or destructor. - const CallExpr *visitingCallExpr; - -public: - WalkAST(const CheckerBase *checker, BugReporter &br, AnalysisDeclContext *ac, - const CXXMethodDecl *rootMethod, bool isInterprocedural, - bool reportPureOnly) - : Checker(checker), BR(br), AC(ac), RootMethod(rootMethod), -IsInterprocedural(isInterprocedural), ReportPureOnly(reportPureOnly), -visitingCallExpr(nullptr) { -// Walking should always start from either a constructor or a destructor. -assert(isa(rootMethod) || - isa(rootMethod)); +enum class ObjectState : bool { CtorCalled, DtorCalled }; +} // end namespace + // FIXME: Ascending over StackFrameContext maybe another method. + +namespace llvm { +template <> struct FoldingSetTrait { + static inline void Profile(ObjectState X, FoldingSetNodeID &ID) { +ID.AddInteger(static_cast(X)); } +}; +} // end namespace llvm - bool hasWork() const { return !WList.empty(); } - - /// This method adds a CallExpr to the worklist and marks the callee as - /// being PreVisited. - void Enqueue(WorkListUnit WLUnit) { -const FunctionDecl *FD = WLUnit->getDirectCallee(); -if (!FD || !FD->getBody()) - return; -Kind &K = VisitedFunctions[FD]; -if (K != NotVisited) - return; -K = PreVisited; -WList.push_back(WLUnit); - } +namespace { +class VirtualCallChecker +: public Checker { + mutable std::unique_ptr BT; - /// This method returns an item from the worklist without removing it. - WorkListUnit Dequeue() { -assert(!WList.empty()); -return WList.back(); - } +public: + // The flag to determine if pure virtual functions should be issued only. + DefaultBool IsPureOnly; -
[PATCH] D37181: {StaticAnalyzer} LoopUnrolling: Keep track the maximum number of steps for each loop
NoQ added a comment. LGTM, thanks! Minor nit included. I hope our matchers actually cover all the cases (not a big deal if they don't though). Comment at: lib/StaticAnalyzer/Core/LoopUnrolling.cpp:26 +#define MAXIMUM_STEP_UNROLLED 128 + I'd prefer a (`static`) `const int`, dealing preprocessor doesn't seem to be necessary here. https://reviews.llvm.org/D37181 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37101: [clangd] [WIP] Add support for snippet completions
ilya-biryukov added inline comments. Comment at: clangd/ClangdUnit.cpp:321 + +// Fill in the label, detail, documentation and insertText fields of the +// CompletionItem. rwols wrote: > ilya-biryukov wrote: > > Maybe split writes into `Item.label` and other `Item` fields? > > That would simplify the function, i.e. something like this: > > > > ``` > > // Set Item.label approriately. > > switch (Result.Kind) { > > case RK_Declaration: Item.label = Result.Declaration->getNameAsString(); > > //... other enum value > > } > > > > > > // Set other Item fields. > > // Note that is also works as expected for RK_Pattern. > > auto Completion = Result.CreateCodeCompletionString(/**/); > > ProcessCodeCompleteString(/*...*/, Item); > > // > > ``` > From some experimentation and skimming over SemaCodeComplete.cpp I notice the > result of `CreateCodeCompletionString(/**/)` is never null anyway, so one > can just skip those switch cases. I'm not sure why a pointer is returned. Probably just a way to indicate that memory for returned item is managed by `Allocator`. Comment at: clangd/ClangdUnit.cpp:316 +// CK_Placeholder or CK_CurrentParameter, this will be modified to +// InsertTextFormat::PlainText. +Item.insertTextFormat = InsertTextFormat::PlainText; Probably meant to be `InsertTextFormat::Snippet`, not `InsertTextFormat::PlainText`. Comment at: clangd/ClangdUnit.cpp:379 + +// TODO: Maybe add an option to allow presenting the optional chunks? +break; Style guide: use FIXME instead of TODO Comment at: clangd/ClangdUnit.cpp:393 +// Terrible hack. +// TODO: See SemaCodeComplete.cpp:2598 +if (Chunk.Text[0] == ' ') Style guide: use FIXME instead of TODO Comment at: clangd/ClangdUnit.cpp:393 +// Terrible hack. +// TODO: See SemaCodeComplete.cpp:2598 +if (Chunk.Text[0] == ' ') ilya-biryukov wrote: > Style guide: use FIXME instead of TODO Maybe reference function name instead of the line number? Line numbers change too often. Comment at: clangd/ClangdUnit.cpp:395 +if (Chunk.Text[0] == ' ') + Item.documentation += "[" + std::string(Chunk.Text + 1) + "] "; +else After looking at code completion code, it seems that informative chunks are parts of function signature (that have custom highlightings in XCode, probably). Maybe we could put them into label? It feels that `const` belongs to the parameter list and we put parameter list into `label` now. That would also make the `Chunk.Text + 1` part unnecessary. Comment at: clangd/ClangdUnit.cpp:452 + + void AddSnippetParameter(const char *Text, unsigned &ArgCount, + CompletionItem &Item) const { Maybe use `StringRef` instead of `const char*`? Comment at: clangd/ClangdUnit.cpp:461 + + std::string SnippetEscape(const char *Text) const { +std::string Result; Maybe use `StringRef` instead of `const char*`? https://reviews.llvm.org/D37101 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37142: clang-format: [JS] simplify template string wrapping.
mprobst updated this revision to Diff 112858. mprobst added a comment. - clang-format: [JS] wrap calls w/ literals in template strings. https://reviews.llvm.org/D37142 Files: lib/Format/ContinuationIndenter.cpp unittests/Format/FormatTestJS.cpp Index: unittests/Format/FormatTestJS.cpp === --- unittests/Format/FormatTestJS.cpp +++ unittests/Format/FormatTestJS.cpp @@ -1793,43 +1793,38 @@ verifyFormat("var x = someFunction(`${})`) //\n" ".oon();"); verifyFormat("var x = someFunction(`${}${\n" - " a( //\n" - " a)\n" - " })`);"); + "a( //\n" + "a)})`);"); } TEST_F(FormatTestJS, TemplateStringMultiLineExpression) { verifyFormat("var f = `aa: ${\n" - " a + //\n" - " \n" - " }`;", + "a + //\n" + "}`;", "var f = `aa: ${a + //\n" " }`;"); verifyFormat("var f = `\n" " aa: ${\n" - "a + //\n" - "\n" - " }`;", + "a + //\n" + "}`;", "var f = `\n" " aa: ${ a + //\n" " }`;"); verifyFormat("var f = `\n" " aa: ${\n" - "someFunction(\n" - "a + //\n" - ")\n" - " }`;", + "someFunction(\n" + "a + //\n" + ")}`;", "var f = `\n" " aa: ${someFunction (\n" "a + //\n" ")}`;"); - - // It might be preferable to wrap before "someFunction". verifyFormat("var f = `\n" - " aa: ${someFunction({\n" - " : a,\n" - " : b,\n" - "})}`;", + " aa: ${\n" + "someFunction({\n" + " : a,\n" + " : b,\n" + "})}`;", "var f = `\n" " aa: ${someFunction ({\n" " : a,\n" Index: lib/Format/ContinuationIndenter.cpp === --- lib/Format/ContinuationIndenter.cpp +++ lib/Format/ContinuationIndenter.cpp @@ -661,9 +661,7 @@ // before the corresponding } or ]. if (PreviousNonComment && (PreviousNonComment->isOneOf(tok::l_brace, TT_ArrayInitializerLSquare) || - opensProtoMessageField(*PreviousNonComment, Style) || - (PreviousNonComment->is(TT_TemplateString) && -PreviousNonComment->opensScope( + opensProtoMessageField(*PreviousNonComment, Style))) State.Stack.back().BreakBeforeClosingBrace = true; if (State.Stack.back().AvoidBinPacking) { @@ -925,11 +923,6 @@ moveStatePastFakeLParens(State, Newline); moveStatePastScopeCloser(State); - if (Current.is(TT_TemplateString) && Current.opensScope()) -State.Stack.back().LastSpace = -(Current.IsMultiline ? Current.LastLineColumnWidth - : State.Column + Current.ColumnWidth) - -strlen("${"); bool CanBreakProtrudingToken = !State.Stack.back().NoLineBreak && !State.Stack.back().NoLineBreakInOperand; moveStatePastScopeOpener(State, Newline); @@ -1101,18 +1094,6 @@ LastSpace = std::max(LastSpace, State.Stack.back().Indent); } -// JavaScript template strings are special as we always want to indent -// nested expressions relative to the ${}. Otherwise, this can create quite -// a mess. -if (Current.is(TT_TemplateString)) { - unsigned Column = Current.IsMultiline -? Current.LastLineColumnWidth -: State.Column + Current.ColumnWidth; - NewIndent = Column; - LastSpace = Column; - NestedBlockIndent = Column; -} - bool EndsInComma = Current.MatchingParen && Current.MatchingParen->getPreviousNonComment() && @
[PATCH] D37203: [Bash-autocompletion] Follow up patch for D36782
yamaguchi created this revision. https://reviews.llvm.org/D36782 broke lld buildbots, because I've changed OptionInfo's type from ArrayRef to std::vector, so it became a copy, not a reference. In this patch, I changed Table to static, so that it won't be freed after the return. However, I'm not sure if this is Ok for lld, because it requires this function to be called atomic. https://reviews.llvm.org/D37203 Files: lld/COFF/DriverUtils.cpp Index: lld/COFF/DriverUtils.cpp === --- lld/COFF/DriverUtils.cpp +++ lld/COFF/DriverUtils.cpp @@ -727,7 +727,7 @@ std::vector Argv = replaceResponseFiles(ArgsArr); // Make InputArgList from string vectors. - COFFOptTable Table; + static COFFOptTable Table; unsigned MissingIndex; unsigned MissingCount; opt::InputArgList Args = Table.ParseArgs(Argv, MissingIndex, MissingCount); Index: lld/COFF/DriverUtils.cpp === --- lld/COFF/DriverUtils.cpp +++ lld/COFF/DriverUtils.cpp @@ -727,7 +727,7 @@ std::vector Argv = replaceResponseFiles(ArgsArr); // Make InputArgList from string vectors. - COFFOptTable Table; + static COFFOptTable Table; unsigned MissingIndex; unsigned MissingCount; opt::InputArgList Args = Table.ParseArgs(Argv, MissingIndex, MissingCount); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type
arphaman added a comment. Not at all, go ahead Repository: rL LLVM https://reviews.llvm.org/D37104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36075: [refactor] Initial support for refactoring action rules
ioeric accepted this revision. ioeric added a comment. This revision is now accepted and ready to land. Thanks for the changes! Lgtm with a few nits. Comment at: include/clang/Tooling/Refactoring/RefactoringActionRules.h:33 +/// +/// - requiredSelection: The refactoring function won't be invoked unless the +/// given selection requirement is satisfied. arphaman wrote: > ioeric wrote: > > We might want to document supported requirements somewhere else so that we > > don't need to update this file every time a new requirement is added. > Do you think it should be in Clang's documentation? I can start on a new > document there but I'd prefer to do it in a separate patch. WDYT? Sure, this is fine for now. It would be nice to have proper documentation in the future when pieces get into places. Comment at: include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h:33 +public: + virtual Expected> + perform(RefactoringRuleContext &Context) = 0; Why isn't this a interface in `SpecificRefactoringRuleAdapter` with return type `Expected>`? Comment at: unittests/Tooling/RefactoringActionRulesTest.cpp:67 + { +RefactoringRuleContext Operation(Context.Sources); +SourceLocation Cursor = It would be nice to also rename the variable from `Operation` to `Context`. Repository: rL LLVM https://reviews.llvm.org/D36075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37142: clang-format: [JS] simplify template string wrapping.
djasper added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1139 + + // On lines containing template strings, propagate NoLineBreak even for dict + // and array literals. This is to force wrapping an initial function call if This is not the right way to implement this: - This is a static computation that we could do ahead of time. Doing it inside the combinatorial exploration of solutions is a waste. - You are doing this always, even in code that doesn't have template strings or isn't even JavaScript. - This can lead to unexpected behavior if the template string is in a completely unrelated part of the statement. E.g. someFunction(`test`, { ... }); will be formatted differently from someFunction('test', { ... }); https://reviews.llvm.org/D37142 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37142: clang-format: [JS] simplify template string wrapping.
mprobst added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1139 + + // On lines containing template strings, propagate NoLineBreak even for dict + // and array literals. This is to force wrapping an initial function call if djasper wrote: > This is not the right way to implement this: > > - This is a static computation that we could do ahead of time. Doing it > inside the combinatorial exploration of solutions is a waste. > - You are doing this always, even in code that doesn't have template strings > or isn't even JavaScript. > - This can lead to unexpected behavior if the template string is in a > completely unrelated part of the statement. E.g. > > > someFunction(`test`, { ... }); > > will be formatted differently from > > someFunction('test', { ... }); Ack. Do you have suggestions? I could introduce a `bool ParenState::NoLineBreakMustPropagate;` that controls this behaviour. https://reviews.llvm.org/D37142 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37181: {StaticAnalyzer} LoopUnrolling: Keep track the maximum number of steps for each loop
szepet updated this revision to Diff 112866. szepet marked an inline comment as done. szepet added a comment. Updated to use static int. https://reviews.llvm.org/D37181 Files: include/clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h lib/StaticAnalyzer/Core/ExprEngine.cpp lib/StaticAnalyzer/Core/LoopUnrolling.cpp test/Analysis/loop-unrolling.cpp Index: test/Analysis/loop-unrolling.cpp === --- test/Analysis/loop-unrolling.cpp +++ test/Analysis/loop-unrolling.cpp @@ -5,7 +5,7 @@ int getNum(); void foo(int &); -// Testing for loops. + int simple_unroll1() { int a[9]; int k = 42; @@ -259,7 +259,6 @@ return 0; } - int recursion_unroll1(bool b) { int k = 2; for (int i = 0; i < 5; i++) { @@ -289,7 +288,7 @@ int k = 2; for (int i = 0; i < 5; i++) { clang_analyzer_numTimesReached(); // expected-warning {{10}} -if(i == 4 && b) { +if (i == 4 && b) { recursion_unroll3(false); break; } @@ -319,3 +318,57 @@ ; return 0; } + +int num_steps_on_limit() { + for (int i = 0; i < 128; i++) { +clang_analyzer_numTimesReached(); // expected-warning {{128}} + } + clang_analyzer_numTimesReached(); // expected-warning {{1}} + return 0; +} + +int num_steps_over_limit1() { + for (int i = 0; i < 129; i++) { +clang_analyzer_numTimesReached(); // expected-warning {{4}} + } + return 0; +} + +int num_steps_on_limit2() { + for (int i = 0; i < 2; i++) { +for (int j = 0; j < 64; j++) { + clang_analyzer_numTimesReached(); // expected-warning {{128}} +} + } + return 0; +} + +int num_steps_over_limit2() { + for (int i = 0; i < 2; i++) { +clang_analyzer_numTimesReached(); // expected-warning {{1}} +for (int j = 0; j <= 64; j++) { + clang_analyzer_numTimesReached(); // expected-warning {{4}} +} + } + return 0; +} + +int num_steps_on_limit3() { + for (int i = 0; i < getNum(); i++) { +clang_analyzer_numTimesReached(); // expected-warning {{4}} +for (int j = 0; j < 32; j++) { + clang_analyzer_numTimesReached(); // expected-warning {{128}} +} + } + return 0; +} + +int num_steps_over_limit3() { + for (int i = 0; i < getNum(); i++) { +clang_analyzer_numTimesReached(); // expected-warning {{1}} +for (int j = 0; j < 33; j++) { + clang_analyzer_numTimesReached(); // expected-warning {{4}} +} + } + return 0; +} Index: lib/StaticAnalyzer/Core/LoopUnrolling.cpp === --- lib/StaticAnalyzer/Core/LoopUnrolling.cpp +++ lib/StaticAnalyzer/Core/LoopUnrolling.cpp @@ -23,22 +23,28 @@ using namespace ento; using namespace clang::ast_matchers; +static const int MAXIMUM_STEP_UNROLLED = 128; + struct LoopState { private: enum Kind { Normal, Unrolled } K; const Stmt *LoopStmt; const LocationContext *LCtx; - LoopState(Kind InK, const Stmt *S, const LocationContext *L) - : K(InK), LoopStmt(S), LCtx(L) {} + unsigned maxStep; + LoopState(Kind InK, const Stmt *S, const LocationContext *L, unsigned N) + : K(InK), LoopStmt(S), LCtx(L), maxStep(N) {} public: - static LoopState getNormal(const Stmt *S, const LocationContext *L) { -return LoopState(Normal, S, L); + static LoopState getNormal(const Stmt *S, const LocationContext *L, + unsigned N) { +return LoopState(Normal, S, L, N); } - static LoopState getUnrolled(const Stmt *S, const LocationContext *L) { -return LoopState(Unrolled, S, L); + static LoopState getUnrolled(const Stmt *S, const LocationContext *L, + unsigned N) { +return LoopState(Unrolled, S, L, N); } bool isUnrolled() const { return K == Unrolled; } + unsigned getMaxStep() const { return maxStep; } const Stmt *getLoopStmt() const { return LoopStmt; } const LocationContext *getLocationContext() const { return LCtx; } bool operator==(const LoopState &X) const { @@ -48,6 +54,7 @@ ID.AddInteger(K); ID.AddPointer(LoopStmt); ID.AddPointer(LCtx); +ID.AddInteger(maxStep); } }; @@ -74,12 +81,14 @@ } static internal::Matcher simpleCondition(StringRef BindName) { - return binaryOperator( - anyOf(hasOperatorName("<"), hasOperatorName(">"), hasOperatorName("<="), -hasOperatorName(">="), hasOperatorName("!=")), - hasEitherOperand(ignoringParenImpCasts( - declRefExpr(to(varDecl(hasType(isInteger())).bind(BindName), - hasEitherOperand(ignoringParenImpCasts(integerLiteral(; + return binaryOperator(anyOf(hasOperatorName("<"), hasOperatorName(">"), + hasOperatorName("<="), hasOperatorName(">="), + hasOperatorName("!=")), +hasEitherOperand(ignoringParenImpCasts(declRefExpr( +to(varDecl(hasType(isInteger())).bind(BindName), +hasEitherOperand(ignori
[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas
hamzasood updated this revision to Diff 112867. hamzasood added a comment. Hi Richard, thanks for your feedback. This update makes the changes that you requested. It turns out that the casting is no longer an issue since `ParseTemplateParameters` has been refactored to use `NamedDecl`. https://reviews.llvm.org/D36527 Files: include/clang/AST/ExprCXX.h include/clang/Basic/DiagnosticParseKinds.td include/clang/Sema/ScopeInfo.h include/clang/Sema/Sema.h lib/AST/ExprCXX.cpp lib/Parse/ParseExprCXX.cpp lib/Sema/Sema.cpp lib/Sema/SemaLambda.cpp lib/Sema/SemaType.cpp test/CXX/temp/temp.decls/temp.variadic/p4.cpp test/Parser/cxx2a-template-lambdas.cpp test/SemaCXX/cxx2a-template-lambdas.cpp www/cxx_status.html Index: www/cxx_status.html === --- www/cxx_status.html +++ www/cxx_status.html @@ -822,7 +822,7 @@ template-parameter-list for generic lambdas http://wg21.link/p0428r2";>P0428R2 - No + SVN Initializer list constructors in class template argument deduction Index: test/SemaCXX/cxx2a-template-lambdas.cpp === --- test/SemaCXX/cxx2a-template-lambdas.cpp +++ test/SemaCXX/cxx2a-template-lambdas.cpp @@ -0,0 +1,34 @@ +// RUN: %clang_cc1 -std=c++2a -verify %s + +template +constexpr bool is_same = false; + +template +constexpr bool is_same = true; + +template +struct DummyTemplate { }; + +void func() { + auto L0 = [](T arg) { +static_assert(is_same); + }; + L0(0); + + auto L1 = [] { +static_assert(I == 5); + }; + L1.operator()<5>(); + + auto L2 = [] class T, class U>(T &&arg) { +static_assert(is_same, DummyTemplate>); + }; + L2(DummyTemplate()); +} + +template // expected-note {{declared here}} +struct ShadowMe { + void member_func() { +auto L = [] { }; // expected-error {{'T' shadows template parameter}} + } +}; Index: test/Parser/cxx2a-template-lambdas.cpp === --- test/Parser/cxx2a-template-lambdas.cpp +++ test/Parser/cxx2a-template-lambdas.cpp @@ -0,0 +1,8 @@ +// RUN: %clang_cc1 -std=c++2a %s -verify + +auto L0 = []<> { }; //expected-error {{cannot be empty}} + +auto L1 = [] { }; +auto L2 = [](T1 arg1, T2 arg2) -> T1 { }; +auto L3 = [](auto arg) { T t; }; +auto L4 = []() { }; Index: test/CXX/temp/temp.decls/temp.variadic/p4.cpp === --- test/CXX/temp/temp.decls/temp.variadic/p4.cpp +++ test/CXX/temp/temp.decls/temp.variadic/p4.cpp @@ -213,8 +213,10 @@ }; #endif +#if __cplusplus > 201703L //- in a template parameter pack that is a pack expansion - // FIXME: We do not support any way to reach this case yet. + swallow([] typename ...W>(W ...wv) { }); +#endif //- in an initializer-list int arr[] = {T().x...}; @@ -279,11 +281,6 @@ struct T { int x; using U = int; }; void g() { f(1, 2, 3); } - template void pack_in_lambda(U ...u) { // expected-note {{here}} -// FIXME: Move this test into 'f' above once we support this syntax. -[] typename ...U>(U ...uv) {}; // expected-error {{expected body of lambda}} expected-error {{does not refer to a value}} - } - template void pack_expand_attr() { // FIXME: Move this test into 'f' above once we support this. [[gnu::aligned(alignof(T))...]] int x; // expected-error {{cannot be used as an attribute pack}} expected-error {{unexpanded}} Index: lib/Sema/SemaType.cpp === --- lib/Sema/SemaType.cpp +++ lib/Sema/SemaType.cpp @@ -2790,7 +2790,7 @@ sema::LambdaScopeInfo *LSI = SemaRef.getCurLambda(); assert(LSI && "No LambdaScopeInfo on the stack!"); const unsigned TemplateParameterDepth = LSI->AutoTemplateParameterDepth; -const unsigned AutoParameterPosition = LSI->AutoTemplateParams.size(); +const unsigned AutoParameterPosition = LSI->TemplateParams.size(); const bool IsParameterPack = D.hasEllipsis(); // Create the TemplateTypeParmDecl here to retrieve the corresponding @@ -2802,7 +2802,8 @@ /*KeyLoc*/SourceLocation(), /*NameLoc*/D.getLocStart(), TemplateParameterDepth, AutoParameterPosition, /*Identifier*/nullptr, false, IsParameterPack); -LSI->AutoTemplateParams.push_back(CorrespondingTemplateParam); +CorrespondingTemplateParam->setImplicit(); +LSI->TemplateParams.push_back(CorrespondingTemplateParam); // Replace the 'auto' in the function parameter with this invented // template type parameter. // FIXME: Retain some type sugar to indicate that this was written Index: lib/Sema/SemaLambda.cpp === --- lib/Sema/SemaLambda.cpp +++ lib/Sema/SemaLambda.cpp @@ -21,6 +21,
[PATCH] D37142: clang-format: [JS] simplify template string wrapping.
djasper added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1139 + + // On lines containing template strings, propagate NoLineBreak even for dict + // and array literals. This is to force wrapping an initial function call if mprobst wrote: > djasper wrote: > > This is not the right way to implement this: > > > > - This is a static computation that we could do ahead of time. Doing it > > inside the combinatorial exploration of solutions is a waste. > > - You are doing this always, even in code that doesn't have template > > strings or isn't even JavaScript. > > - This can lead to unexpected behavior if the template string is in a > > completely unrelated part of the statement. E.g. > > > > > > someFunction(`test`, { ... }); > > > > will be formatted differently from > > > > someFunction('test', { ... }); > Ack. Do you have suggestions? > > I could introduce a `bool ParenState::NoLineBreakMustPropagate;` that > controls this behaviour. That, too, would create a significant memory and runtime overhead to everyone just to fix this rather minor formatting detail. If we were to set MatchingParen correctly to match up "${" to "}", we could measure the respective length and at a very early state (mustBreak()) decide that we need a line break between "${" and the corresponding "}". Now setting that correctly might be hard. Maybe it is ok, to linearly scan in that case as we would only do that if we actually find a template string ending in "${" and the distance to the "}" is usually short. https://reviews.llvm.org/D37142 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37206: [ItaniumCXXABI] Always use linkonce_odr linkage for RTTI data on MinGW
mstorsjo created this revision. This fixes cases where dynamic classes produced RTTI data with external linkage, producing linker errors about duplicate symbols. This touches code close to what was changed in SVN r244266, but this change doesn't break the tests added in that revision. I have to admit that I have little clue about what I'm doing here and I'm not sure if the exact fix is correct (it feels very heavy handed). Hopefully the testcase shows what this fixes at least; this fixes linker errors with pretty trivial setups, where one object file contains the definition of a class with a virtual method (making the class dynamic, which previously meant that the typeinfo was emitted with external linkage) and another object file references typeid(ThatClass) (which produced a linkonce_odr version of that class' typeinfo). https://reviews.llvm.org/D37206 Files: lib/CodeGen/ItaniumCXXABI.cpp test/CodeGenCXX/rtti-mingw64.cpp Index: test/CodeGenCXX/rtti-mingw64.cpp === --- test/CodeGenCXX/rtti-mingw64.cpp +++ test/CodeGenCXX/rtti-mingw64.cpp @@ -2,7 +2,12 @@ struct A { int a; }; struct B : virtual A { int b; }; B b; +class C { + virtual ~C(); +}; +C::~C() {} +// CHECK: @_ZTI1C = linkonce_odr // CHECK: @_ZTI1B = linkonce_odr constant { i8*, i8*, i32, i32, i8*, i64 } // CHECK-SAME: i8* bitcast (i8** getelementptr inbounds (i8*, i8** @_ZTVN10__cxxabiv121__vmi_class_type_infoE, i64 2) to i8*), // CHECK-SAME: i8* getelementptr inbounds ([3 x i8], [3 x i8]* @_ZTS1B, i32 0, i32 0), Index: lib/CodeGen/ItaniumCXXABI.cpp === --- lib/CodeGen/ItaniumCXXABI.cpp +++ lib/CodeGen/ItaniumCXXABI.cpp @@ -2987,15 +2987,13 @@ if (RD->hasAttr() && ShouldUseExternalRTTIDescriptor(CGM, Ty)) return llvm::GlobalValue::ExternalLinkage; - if (RD->isDynamicClass()) { -llvm::GlobalValue::LinkageTypes LT = CGM.getVTableLinkage(RD); -// MinGW won't export the RTTI information when there is a key function. -// Make sure we emit our own copy instead of attempting to dllimport it. -if (RD->hasAttr() && -llvm::GlobalValue::isAvailableExternallyLinkage(LT)) - LT = llvm::GlobalValue::LinkOnceODRLinkage; -return LT; - } + bool IsMinGW = + CGM.getContext().getTargetInfo().getTriple().getEnvironment() == + llvm::Triple::GNU && + CGM.getContext().getTargetInfo().getTriple().isOSWindows(); + // MinGW always uses LinkOnceODRLinkage for type info. + if (RD->isDynamicClass() && !IsMinGW) +return CGM.getVTableLinkage(RD); } return llvm::GlobalValue::LinkOnceODRLinkage; Index: test/CodeGenCXX/rtti-mingw64.cpp === --- test/CodeGenCXX/rtti-mingw64.cpp +++ test/CodeGenCXX/rtti-mingw64.cpp @@ -2,7 +2,12 @@ struct A { int a; }; struct B : virtual A { int b; }; B b; +class C { + virtual ~C(); +}; +C::~C() {} +// CHECK: @_ZTI1C = linkonce_odr // CHECK: @_ZTI1B = linkonce_odr constant { i8*, i8*, i32, i32, i8*, i64 } // CHECK-SAME: i8* bitcast (i8** getelementptr inbounds (i8*, i8** @_ZTVN10__cxxabiv121__vmi_class_type_infoE, i64 2) to i8*), // CHECK-SAME: i8* getelementptr inbounds ([3 x i8], [3 x i8]* @_ZTS1B, i32 0, i32 0), Index: lib/CodeGen/ItaniumCXXABI.cpp === --- lib/CodeGen/ItaniumCXXABI.cpp +++ lib/CodeGen/ItaniumCXXABI.cpp @@ -2987,15 +2987,13 @@ if (RD->hasAttr() && ShouldUseExternalRTTIDescriptor(CGM, Ty)) return llvm::GlobalValue::ExternalLinkage; - if (RD->isDynamicClass()) { -llvm::GlobalValue::LinkageTypes LT = CGM.getVTableLinkage(RD); -// MinGW won't export the RTTI information when there is a key function. -// Make sure we emit our own copy instead of attempting to dllimport it. -if (RD->hasAttr() && -llvm::GlobalValue::isAvailableExternallyLinkage(LT)) - LT = llvm::GlobalValue::LinkOnceODRLinkage; -return LT; - } + bool IsMinGW = + CGM.getContext().getTargetInfo().getTriple().getEnvironment() == + llvm::Triple::GNU && + CGM.getContext().getTargetInfo().getTriple().isOSWindows(); + // MinGW always uses LinkOnceODRLinkage for type info. + if (RD->isDynamicClass() && !IsMinGW) +return CGM.getVTableLinkage(RD); } return llvm::GlobalValue::LinkOnceODRLinkage; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r311880 - [StaticAnalyzer] LoopUnrolling fixes
Author: szepet Date: Mon Aug 28 03:21:24 2017 New Revision: 311880 URL: http://llvm.org/viewvc/llvm-project?rev=311880&view=rev Log: [StaticAnalyzer] LoopUnrolling fixes 1. The LoopUnrolling feature needs the LoopExit included in the CFG so added this dependency via the config options 2. The LoopExit element can be encountered even if we haven't encountered the block of the corresponding LoopStmt. So the asserts were not right. 3. If we are caching out the Node then we get a nullptr from generateNode which case was not handled. Differential Revision: https://reviews.llvm.org/D37103 Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp cfe/trunk/test/Analysis/loop-unrolling.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp?rev=311880&r1=311879&r2=311880&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp Mon Aug 28 03:21:24 2017 @@ -27,7 +27,9 @@ AnalysisManager::AnalysisManager(ASTCont /*AddInitializers=*/true, Options.includeTemporaryDtorsInCFG(), Options.includeLifetimeInCFG(), - Options.includeLoopExitInCFG(), + // Adding LoopExit elements to the CFG is a requirement for loop + // unrolling. + Options.includeLoopExitInCFG() || Options.shouldUnrollLoops(), Options.shouldSynthesizeBodies(), Options.shouldConditionalizeStaticInitializers(), /*addCXXNewAllocator=*/true, Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=311880&r1=311879&r2=311880&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Mon Aug 28 03:21:24 2017 @@ -1527,8 +1527,11 @@ void ExprEngine::processCFGBlockEntrance if (Term) { ProgramStateRef NewState = updateLoopStack(Term, AMgr.getASTContext(), Pred); - if (NewState != Pred->getState()){ -Pred = nodeBuilder.generateNode(NewState, Pred); + if (NewState != Pred->getState()) { +ExplodedNode *UpdatedNode = nodeBuilder.generateNode(NewState, Pred); +if (!UpdatedNode) + return; +Pred = UpdatedNode; } } // Is we are inside an unrolled loop then no need the check the counters. Modified: cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp?rev=311880&r1=311879&r2=311880&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp Mon Aug 28 03:21:24 2017 @@ -13,15 +13,11 @@ /// //===--===// -#include "clang/Analysis/CFGStmtMap.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/AST/ParentMap.h" -#include "clang/AST/StmtVisitor.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h" -#include "llvm/ADT/Statistic.h" using namespace clang; using namespace ento; @@ -72,11 +68,8 @@ static bool isLoopStmt(const Stmt *S) { ProgramStateRef processLoopEnd(const Stmt *LoopStmt, ProgramStateRef State) { auto LS = State->get(); - assert(!LS.isEmpty() && "Loop not added to the stack."); - assert(LoopStmt == LS.getHead().getLoopStmt() && - "Loop is not on top of the stack."); - - State = State->set(LS.getTail()); + if (!LS.isEmpty() && LS.getHead().getLoopStmt() == LoopStmt) +State = State->set(LS.getTail()); return State; } Modified: cfe/trunk/test/Analysis/loop-unrolling.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/loop-unrolling.cpp?rev=311880&r1=311879&r2=311880&view=diff == --- cfe/trunk/test/Analysis/loop-unrolling.cpp (original) +++ cfe/trunk/test/Analysis/loop-unrolling.cpp Mon Aug 28 03:21:24 2017 @@ -272,3 +272,10 @@ int recursion_unroll4(bool b) { int a = 22 / k; return 0; } + +int loop_exit_while_empty_loop_stack() { + if (getNum()) +for (int i = 1; i < 8; i++) + ; + return 0; +}
[PATCH] D37103: [StaticAnalyzer] LoopUnrolling fixes
This revision was automatically updated to reflect the committed changes. Closed by commit rL311880: [StaticAnalyzer] LoopUnrolling fixes (authored by szepet). Changed prior to commit: https://reviews.llvm.org/D37103?vs=112824&id=112872#toc Repository: rL LLVM https://reviews.llvm.org/D37103 Files: cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp cfe/trunk/test/Analysis/loop-unrolling.cpp Index: cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp @@ -27,7 +27,9 @@ /*AddInitializers=*/true, Options.includeTemporaryDtorsInCFG(), Options.includeLifetimeInCFG(), - Options.includeLoopExitInCFG(), + // Adding LoopExit elements to the CFG is a requirement for loop + // unrolling. + Options.includeLoopExitInCFG() || Options.shouldUnrollLoops(), Options.shouldSynthesizeBodies(), Options.shouldConditionalizeStaticInitializers(), /*addCXXNewAllocator=*/true, Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1527,8 +1527,11 @@ if (Term) { ProgramStateRef NewState = updateLoopStack(Term, AMgr.getASTContext(), Pred); - if (NewState != Pred->getState()){ -Pred = nodeBuilder.generateNode(NewState, Pred); + if (NewState != Pred->getState()) { +ExplodedNode *UpdatedNode = nodeBuilder.generateNode(NewState, Pred); +if (!UpdatedNode) + return; +Pred = UpdatedNode; } } // Is we are inside an unrolled loop then no need the check the counters. Index: cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp @@ -13,15 +13,11 @@ /// //===--===// -#include "clang/Analysis/CFGStmtMap.h" #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/AST/ParentMap.h" -#include "clang/AST/StmtVisitor.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" #include "clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h" -#include "llvm/ADT/Statistic.h" using namespace clang; using namespace ento; @@ -72,11 +68,8 @@ ProgramStateRef processLoopEnd(const Stmt *LoopStmt, ProgramStateRef State) { auto LS = State->get(); - assert(!LS.isEmpty() && "Loop not added to the stack."); - assert(LoopStmt == LS.getHead().getLoopStmt() && - "Loop is not on top of the stack."); - - State = State->set(LS.getTail()); + if (!LS.isEmpty() && LS.getHead().getLoopStmt() == LoopStmt) +State = State->set(LS.getTail()); return State; } Index: cfe/trunk/test/Analysis/loop-unrolling.cpp === --- cfe/trunk/test/Analysis/loop-unrolling.cpp +++ cfe/trunk/test/Analysis/loop-unrolling.cpp @@ -272,3 +272,10 @@ int a = 22 / k; return 0; } + +int loop_exit_while_empty_loop_stack() { + if (getNum()) +for (int i = 1; i < 8; i++) + ; + return 0; +} Index: cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/AnalysisManager.cpp @@ -27,7 +27,9 @@ /*AddInitializers=*/true, Options.includeTemporaryDtorsInCFG(), Options.includeLifetimeInCFG(), - Options.includeLoopExitInCFG(), + // Adding LoopExit elements to the CFG is a requirement for loop + // unrolling. + Options.includeLoopExitInCFG() || Options.shouldUnrollLoops(), Options.shouldSynthesizeBodies(), Options.shouldConditionalizeStaticInitializers(), /*addCXXNewAllocator=*/true, Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1527,8 +1527,11 @@ if (Term) { ProgramStateRef NewState = updateLoopStack(Term, AMgr.getASTContext(), Pred); - if (N
[PATCH] D37207: [libc++] Add MINGW_LIBRARIES to the linker flags
mstorsjo created this revision. Herald added a subscriber: mgorny. This is essential when building with -nodefaultlibs. This is similar to what already is done in libcxxabi in SVN r302760. https://reviews.llvm.org/D37207 Files: lib/CMakeLists.txt Index: lib/CMakeLists.txt === --- lib/CMakeLists.txt +++ lib/CMakeLists.txt @@ -91,6 +91,7 @@ add_library_flags_if(LIBCXX_HAS_GCC_S_LIB gcc_s) endif() add_library_flags_if(LIBCXX_HAVE_CXX_ATOMICS_WITH_LIB atomic) +add_library_flags_if(MINGW "${MINGW_LIBRARIES}") # Add the unwinder library. if (LIBCXXABI_USE_LLVM_UNWINDER) Index: lib/CMakeLists.txt === --- lib/CMakeLists.txt +++ lib/CMakeLists.txt @@ -91,6 +91,7 @@ add_library_flags_if(LIBCXX_HAS_GCC_S_LIB gcc_s) endif() add_library_flags_if(LIBCXX_HAVE_CXX_ATOMICS_WITH_LIB atomic) +add_library_flags_if(MINGW "${MINGW_LIBRARIES}") # Add the unwinder library. if (LIBCXXABI_USE_LLVM_UNWINDER) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type
arphaman added a comment. FYI, I'd like to see this merged into LLVM5 if it's still possible, but it's not super critical Repository: rL LLVM https://reviews.llvm.org/D37104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r311881 - [StaticAnalyzer] LoopUnrolling: Excluding loops which splits the state
Author: szepet Date: Mon Aug 28 03:34:50 2017 New Revision: 311881 URL: http://llvm.org/viewvc/llvm-project?rev=311881&view=rev Log: [StaticAnalyzer] LoopUnrolling: Excluding loops which splits the state Added check if the execution of the last step of the given unrolled loop has generated more branches. If yes, than treat it as a normal (non-unrolled) loop in the remaining part of the analysis. Differential Revision: https://reviews.llvm.org/D36962 Modified: cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp cfe/trunk/test/Analysis/loop-unrolling.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp?rev=311881&r1=311880&r2=311881&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp Mon Aug 28 03:34:50 2017 @@ -204,6 +204,26 @@ bool shouldCompletelyUnroll(const Stmt * return !isPossiblyEscaped(CounterVar->getCanonicalDecl(), Pred); } +bool madeNewBranch(ExplodedNode* N, const Stmt* LoopStmt) { + const Stmt* S = nullptr; + while (!N->pred_empty()) + { +if (N->succ_size() > 1) + return true; + +ProgramPoint P = N->getLocation(); +if (Optional BE = P.getAs()) + S = BE->getBlock()->getTerminator(); + +if (S == LoopStmt) + return false; + +N = N->getFirstPred(); + } + + llvm_unreachable("Reached root without encountering the previous step"); +} + // updateLoopStack is called on every basic block, therefore it needs to be fast ProgramStateRef updateLoopStack(const Stmt *LoopStmt, ASTContext &ASTCtx, ExplodedNode* Pred) { @@ -215,8 +235,13 @@ ProgramStateRef updateLoopStack(const St auto LS = State->get(); if (!LS.isEmpty() && LoopStmt == LS.getHead().getLoopStmt() && - LCtx == LS.getHead().getLocationContext()) + LCtx == LS.getHead().getLocationContext()) { +if (LS.getHead().isUnrolled() && madeNewBranch(Pred, LoopStmt)) { + State = State->set(LS.getTail()); + State = State->add(LoopState::getNormal(LoopStmt, LCtx)); +} return State; + } if (!shouldCompletelyUnroll(LoopStmt, ASTCtx, Pred)) { State = State->add(LoopState::getNormal(LoopStmt, LCtx)); Modified: cfe/trunk/test/Analysis/loop-unrolling.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/loop-unrolling.cpp?rev=311881&r1=311880&r2=311881&view=diff == --- cfe/trunk/test/Analysis/loop-unrolling.cpp (original) +++ cfe/trunk/test/Analysis/loop-unrolling.cpp Mon Aug 28 03:34:50 2017 @@ -1,6 +1,7 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify -std=c++11 %s void clang_analyzer_numTimesReached(); +void clang_analyzer_warnIfReached(); int getNum(); void foo(int &); @@ -62,8 +63,7 @@ int simple_no_unroll2() { int simple_no_unroll3() { int a[9]; int k = 42; - int i; - for (i = 0; i < 9; i++) { + for (int i = 0; i < 9; i++) { clang_analyzer_numTimesReached(); // expected-warning {{4}} a[i] = 42; (void)&i; @@ -98,6 +98,44 @@ int simple_no_unroll5() { return 0; } +int make_new_branches_loop_cached() { + for (int i = 0; i < 8; i++) { +clang_analyzer_numTimesReached(); // expected-warning {{4}} +if(getNum()){ +(void) i; // Since this Stmt does not change the State the analyzer + // won't make a new execution path but reuse the earlier nodes. + } + } + clang_analyzer_warnIfReached(); // no-warning + return 0; +} + +int make_new_branches_loop_uncached() { + int l = 2; + for (int i = 0; i < 8; i++) { +clang_analyzer_numTimesReached(); // expected-warning {{10}} +if(getNum()){ + ++l; +} + } + clang_analyzer_warnIfReached(); // no-warning + return 0; +} + +int make_new_branches_loop_uncached2() { + int l = 2; + for (int i = 0; i < 8; i++) { +clang_analyzer_numTimesReached(); // expected-warning {{10}} +if(getNum()){ + ++l; +} +(void)&i; // This ensures that the loop won't be unrolled. + } + clang_analyzer_warnIfReached(); // no-warning + return 0; +} + + int escape_before_loop_no_unroll1() { int a[9]; int k = 42; @@ -142,10 +180,11 @@ int nested_outer_unrolled() { int k = 42; int j = 0; for (int i = 0; i < 9; i++) { -clang_analyzer_numTimesReached(); // expected-warning {{16}} -for (j = 0; j < getNum(); ++j) { - clang_analyzer_numTimesReached(); // expected-warning {{15}} +clang_analyzer_numTimesReached(); // expected-warning {{1}} +for (j = 0; j < 9; ++j) { + clang_analyzer_numTimesReached(); // expected-warning {{4}} a[j] = 22; + (void) &j; // ensures that the inner loop won't be unrolled } a[i]
[PATCH] D36962: [StaticAnalyzer] LoopUnrolling: Excluding loops which splits the state (make more branches)
This revision was automatically updated to reflect the committed changes. Closed by commit rL311881: [StaticAnalyzer] LoopUnrolling: Excluding loops which splits the state (authored by szepet). Changed prior to commit: https://reviews.llvm.org/D36962?vs=112710&id=112874#toc Repository: rL LLVM https://reviews.llvm.org/D36962 Files: cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp cfe/trunk/test/Analysis/loop-unrolling.cpp Index: cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp @@ -204,6 +204,26 @@ return !isPossiblyEscaped(CounterVar->getCanonicalDecl(), Pred); } +bool madeNewBranch(ExplodedNode* N, const Stmt* LoopStmt) { + const Stmt* S = nullptr; + while (!N->pred_empty()) + { +if (N->succ_size() > 1) + return true; + +ProgramPoint P = N->getLocation(); +if (Optional BE = P.getAs()) + S = BE->getBlock()->getTerminator(); + +if (S == LoopStmt) + return false; + +N = N->getFirstPred(); + } + + llvm_unreachable("Reached root without encountering the previous step"); +} + // updateLoopStack is called on every basic block, therefore it needs to be fast ProgramStateRef updateLoopStack(const Stmt *LoopStmt, ASTContext &ASTCtx, ExplodedNode* Pred) { @@ -215,8 +235,13 @@ auto LS = State->get(); if (!LS.isEmpty() && LoopStmt == LS.getHead().getLoopStmt() && - LCtx == LS.getHead().getLocationContext()) + LCtx == LS.getHead().getLocationContext()) { +if (LS.getHead().isUnrolled() && madeNewBranch(Pred, LoopStmt)) { + State = State->set(LS.getTail()); + State = State->add(LoopState::getNormal(LoopStmt, LCtx)); +} return State; + } if (!shouldCompletelyUnroll(LoopStmt, ASTCtx, Pred)) { State = State->add(LoopState::getNormal(LoopStmt, LCtx)); Index: cfe/trunk/test/Analysis/loop-unrolling.cpp === --- cfe/trunk/test/Analysis/loop-unrolling.cpp +++ cfe/trunk/test/Analysis/loop-unrolling.cpp @@ -1,6 +1,7 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config unroll-loops=true,cfg-loopexit=true -verify -std=c++11 %s void clang_analyzer_numTimesReached(); +void clang_analyzer_warnIfReached(); int getNum(); void foo(int &); @@ -62,8 +63,7 @@ int simple_no_unroll3() { int a[9]; int k = 42; - int i; - for (i = 0; i < 9; i++) { + for (int i = 0; i < 9; i++) { clang_analyzer_numTimesReached(); // expected-warning {{4}} a[i] = 42; (void)&i; @@ -98,6 +98,44 @@ return 0; } +int make_new_branches_loop_cached() { + for (int i = 0; i < 8; i++) { +clang_analyzer_numTimesReached(); // expected-warning {{4}} +if(getNum()){ +(void) i; // Since this Stmt does not change the State the analyzer + // won't make a new execution path but reuse the earlier nodes. + } + } + clang_analyzer_warnIfReached(); // no-warning + return 0; +} + +int make_new_branches_loop_uncached() { + int l = 2; + for (int i = 0; i < 8; i++) { +clang_analyzer_numTimesReached(); // expected-warning {{10}} +if(getNum()){ + ++l; +} + } + clang_analyzer_warnIfReached(); // no-warning + return 0; +} + +int make_new_branches_loop_uncached2() { + int l = 2; + for (int i = 0; i < 8; i++) { +clang_analyzer_numTimesReached(); // expected-warning {{10}} +if(getNum()){ + ++l; +} +(void)&i; // This ensures that the loop won't be unrolled. + } + clang_analyzer_warnIfReached(); // no-warning + return 0; +} + + int escape_before_loop_no_unroll1() { int a[9]; int k = 42; @@ -142,10 +180,11 @@ int k = 42; int j = 0; for (int i = 0; i < 9; i++) { -clang_analyzer_numTimesReached(); // expected-warning {{16}} -for (j = 0; j < getNum(); ++j) { - clang_analyzer_numTimesReached(); // expected-warning {{15}} +clang_analyzer_numTimesReached(); // expected-warning {{1}} +for (j = 0; j < 9; ++j) { + clang_analyzer_numTimesReached(); // expected-warning {{4}} a[j] = 22; + (void) &j; // ensures that the inner loop won't be unrolled } a[i] = 42; } @@ -213,8 +252,8 @@ int nested_inlined_no_unroll1() { int k; for (int i = 0; i < 9; i++) { -clang_analyzer_numTimesReached(); // expected-warning {{26}} -k = simple_unknown_bound_loop(); // reevaluation without inlining +clang_analyzer_numTimesReached(); // expected-warning {{15}} +k = simple_unknown_bound_loop(); // reevaluation without inlining, splits the state as well } int a = 22 / k; // no-warning return 0; @@ -224,22 +263,23 @@ int recursion_unroll1(bool b) { int k = 2; for (int i = 0; i < 5; i++) { -clang_analyzer_numTimesReached(); // expected-warning {{14}} -if(i == 0 && b) +cl
[PATCH] D36075: [refactor] Initial support for refactoring action rules
arphaman added a comment. Thanks, I'll start working on the documentation patch and will split follow-up `clang-refactor` patch into one or two parts today. Comment at: include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h:33 +public: + virtual Expected> + perform(RefactoringRuleContext &Context) = 0; ioeric wrote: > Why isn't this a interface in `SpecificRefactoringRuleAdapter` with return > type `Expected>`? A method declaration in `SpecificRefactoringRuleAdapter` won't work since it won't be available in either the template specialisation or the deriving class as the classes won't be directly related. I could use a separate parent class independent of SpecificRefactoringRuleAdapter that declares a generic interface though. Repository: rL LLVM https://reviews.llvm.org/D36075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37023: [analyzer] Fix bugreporter::getDerefExpr() again.
xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. Generally, it looks good to me. Though it looks like some of the cases covered in the code do not have corresponding tests (e.g.: the parenexprs). I think this approach is good in a sense there should be such a cast at the interesting places. But I wonder if there could be some cases where we have such cast earlier than expected. https://reviews.llvm.org/D37023 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37025: [analyzer] Support more pointer arithmetic in bugreporter::getDerefExpr().
xazax.hun accepted this revision. xazax.hun added a comment. This revision is now accepted and ready to land. LGTM! https://reviews.llvm.org/D37025 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37005: [clang-diff] Initial implementation of patching
arphaman added inline comments. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:658 + SourceRange Range; + if (auto *Arg = N.ASTNode.get()) +Range = TemplateArgumentLocations.at(&N - &Nodes[0]); You can drop the `auto *Arg` since Arg is unused. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:1305 +struct Patcher { + SyntaxTree::Impl &ModelSrc, &ModelDst, &Target; Please move the patcher into a new file. https://reviews.llvm.org/D37005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r311883 - [StaticAnalyzer] LoopUnrolling: Keep track the maximum number of steps for each loop
Author: szepet Date: Mon Aug 28 03:50:28 2017 New Revision: 311883 URL: http://llvm.org/viewvc/llvm-project?rev=311883&view=rev Log: [StaticAnalyzer] LoopUnrolling: Keep track the maximum number of steps for each loop This way the unrolling can be restricted for loops which will take at most a given number of steps. It is defined as 128 in this patch and it seems to have a good number for that purpose. Differential Revision: https://reviews.llvm.org/D37181 Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp cfe/trunk/test/Analysis/loop-unrolling.cpp Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h?rev=311883&r1=311882&r2=311883&view=diff == --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h (original) +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h Mon Aug 28 03:50:28 2017 @@ -38,7 +38,7 @@ bool isUnrolledState(ProgramStateRef Sta /// Updates the stack of loops contained by the ProgramState. ProgramStateRef updateLoopStack(const Stmt *LoopStmt, ASTContext &ASTCtx, -ExplodedNode* Pred); +ExplodedNode* Pred, unsigned maxVisitOnPath); /// Updates the given ProgramState. In current implementation it removes the top /// element of the stack of loops. Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=311883&r1=311882&r2=311883&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Mon Aug 28 03:50:28 2017 @@ -1523,10 +1523,11 @@ void ExprEngine::processCFGBlockEntrance // If we reach a loop which has a known bound (and meets // other constraints) then consider completely unrolling it. if(AMgr.options.shouldUnrollLoops()) { +unsigned maxBlockVisitOnPath = AMgr.options.maxBlockVisitOnPath; const Stmt *Term = nodeBuilder.getContext().getBlock()->getTerminator(); if (Term) { ProgramStateRef NewState = updateLoopStack(Term, AMgr.getASTContext(), - Pred); + Pred, maxBlockVisitOnPath); if (NewState != Pred->getState()) { ExplodedNode *UpdatedNode = nodeBuilder.generateNode(NewState, Pred); if (!UpdatedNode) Modified: cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp?rev=311883&r1=311882&r2=311883&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp Mon Aug 28 03:50:28 2017 @@ -23,22 +23,28 @@ using namespace clang; using namespace ento; using namespace clang::ast_matchers; +static const int MAXIMUM_STEP_UNROLLED = 128; + struct LoopState { private: enum Kind { Normal, Unrolled } K; const Stmt *LoopStmt; const LocationContext *LCtx; - LoopState(Kind InK, const Stmt *S, const LocationContext *L) - : K(InK), LoopStmt(S), LCtx(L) {} + unsigned maxStep; + LoopState(Kind InK, const Stmt *S, const LocationContext *L, unsigned N) + : K(InK), LoopStmt(S), LCtx(L), maxStep(N) {} public: - static LoopState getNormal(const Stmt *S, const LocationContext *L) { -return LoopState(Normal, S, L); + static LoopState getNormal(const Stmt *S, const LocationContext *L, + unsigned N) { +return LoopState(Normal, S, L, N); } - static LoopState getUnrolled(const Stmt *S, const LocationContext *L) { -return LoopState(Unrolled, S, L); + static LoopState getUnrolled(const Stmt *S, const LocationContext *L, + unsigned N) { +return LoopState(Unrolled, S, L, N); } bool isUnrolled() const { return K == Unrolled; } + unsigned getMaxStep() const { return maxStep; } const Stmt *getLoopStmt() const { return LoopStmt; } const LocationContext *getLocationContext() const { return LCtx; } bool operator==(const LoopState &X) const { @@ -48,6 +54,7 @@ public: ID.AddInteger(K); ID.AddPointer(LoopStmt); ID.AddPointer(LCtx); +ID.AddInteger(maxStep); } }; @@ -74,12 +81,14 @@ ProgramStateRef processLoopEnd(const Stm } static internal::Matcher simpleCondition(StringRef BindName) { - return binaryOperator( - anyOf(hasOperatorName("<"), hasOperatorName
[PATCH] D37181: {StaticAnalyzer} LoopUnrolling: Keep track the maximum number of steps for each loop
This revision was automatically updated to reflect the committed changes. Closed by commit rL311883: [StaticAnalyzer] LoopUnrolling: Keep track the maximum number of steps for each… (authored by szepet). Changed prior to commit: https://reviews.llvm.org/D37181?vs=112866&id=112877#toc Repository: rL LLVM https://reviews.llvm.org/D37181 Files: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp cfe/trunk/test/Analysis/loop-unrolling.cpp Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -1523,10 +1523,11 @@ // If we reach a loop which has a known bound (and meets // other constraints) then consider completely unrolling it. if(AMgr.options.shouldUnrollLoops()) { +unsigned maxBlockVisitOnPath = AMgr.options.maxBlockVisitOnPath; const Stmt *Term = nodeBuilder.getContext().getBlock()->getTerminator(); if (Term) { ProgramStateRef NewState = updateLoopStack(Term, AMgr.getASTContext(), - Pred); + Pred, maxBlockVisitOnPath); if (NewState != Pred->getState()) { ExplodedNode *UpdatedNode = nodeBuilder.generateNode(NewState, Pred); if (!UpdatedNode) Index: cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp === --- cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/LoopUnrolling.cpp @@ -23,22 +23,28 @@ using namespace ento; using namespace clang::ast_matchers; +static const int MAXIMUM_STEP_UNROLLED = 128; + struct LoopState { private: enum Kind { Normal, Unrolled } K; const Stmt *LoopStmt; const LocationContext *LCtx; - LoopState(Kind InK, const Stmt *S, const LocationContext *L) - : K(InK), LoopStmt(S), LCtx(L) {} + unsigned maxStep; + LoopState(Kind InK, const Stmt *S, const LocationContext *L, unsigned N) + : K(InK), LoopStmt(S), LCtx(L), maxStep(N) {} public: - static LoopState getNormal(const Stmt *S, const LocationContext *L) { -return LoopState(Normal, S, L); + static LoopState getNormal(const Stmt *S, const LocationContext *L, + unsigned N) { +return LoopState(Normal, S, L, N); } - static LoopState getUnrolled(const Stmt *S, const LocationContext *L) { -return LoopState(Unrolled, S, L); + static LoopState getUnrolled(const Stmt *S, const LocationContext *L, + unsigned N) { +return LoopState(Unrolled, S, L, N); } bool isUnrolled() const { return K == Unrolled; } + unsigned getMaxStep() const { return maxStep; } const Stmt *getLoopStmt() const { return LoopStmt; } const LocationContext *getLocationContext() const { return LCtx; } bool operator==(const LoopState &X) const { @@ -48,6 +54,7 @@ ID.AddInteger(K); ID.AddPointer(LoopStmt); ID.AddPointer(LCtx); +ID.AddInteger(maxStep); } }; @@ -74,12 +81,14 @@ } static internal::Matcher simpleCondition(StringRef BindName) { - return binaryOperator( - anyOf(hasOperatorName("<"), hasOperatorName(">"), hasOperatorName("<="), -hasOperatorName(">="), hasOperatorName("!=")), - hasEitherOperand(ignoringParenImpCasts( - declRefExpr(to(varDecl(hasType(isInteger())).bind(BindName), - hasEitherOperand(ignoringParenImpCasts(integerLiteral(; + return binaryOperator(anyOf(hasOperatorName("<"), hasOperatorName(">"), + hasOperatorName("<="), hasOperatorName(">="), + hasOperatorName("!=")), +hasEitherOperand(ignoringParenImpCasts(declRefExpr( +to(varDecl(hasType(isInteger())).bind(BindName), +hasEitherOperand(ignoringParenImpCasts( +integerLiteral().bind("boundNum" + .bind("conditionOperator"); } static internal::Matcher @@ -134,13 +143,13 @@ return forStmt( hasCondition(simpleCondition("initVarName")), // Initialization should match the form: 'int i = 6' or 'i = 42'. - hasLoopInit( - anyOf(declStmt(hasSingleDecl( - varDecl(allOf(hasInitializer(integerLiteral()), - equalsBoundNode("initVarName"), - binaryOperator(hasLHS(declRefExpr(to(varDecl( - equalsBoundNode("initVarName"), - hasRHS(integerLiteral(), + hasLoopInit(anyOf( + declStmt(hasSingleDecl(varDecl( + allOf(h
[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor
arphaman added inline comments. Comment at: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h:69 + template + struct has_same_member_pointer_type + : std::true_type {}; Why exactly is this template struct needed? Comment at: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h:75 + Derived &getDerived() { return *static_cast(this); } + I don't think you need this since `getDerived` in RecursiveASTVisitor is already public. Comment at: include/clang/AST/RecursiveASTVisitor.h:3212 #undef TRAVERSE_STMT -#undef TRAVERSE_STMT_BASE Getting rid of `undef` is not ideal. You might want to extract these macros into one .def file that's included by both RecursiveASTVisitor.h and the LexicallyOrdered one. https://reviews.llvm.org/D37200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor
johannes added inline comments. Comment at: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h:75 + Derived &getDerived() { return *static_cast(this); } + arphaman wrote: > I don't think you need this since `getDerived` in RecursiveASTVisitor is > already public. `has_same_member_pointer` and `getDerived` are used by the TRAVERSE_STMT macro, which should be the right one to use here. Also the `RecursiveASTVisitor` typedef just above. Comment at: include/clang/AST/RecursiveASTVisitor.h:3212 #undef TRAVERSE_STMT -#undef TRAVERSE_STMT_BASE arphaman wrote: > Getting rid of `undef` is not ideal. You might want to extract these macros > into one .def file that's included by both RecursiveASTVisitor.h and the > LexicallyOrdered one. Ok, I will do that https://reviews.llvm.org/D37200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37023: [analyzer] Fix bugreporter::getDerefExpr() again.
NoQ added a comment. Thank you for the review! > Though it looks like some of the cases covered in the code do not have > corresponding tests (e.g.: the parenexprs). These are covered by tests in `inline-defensive-checks.c:150,156,169,179` (old code had `IgnoreParenCasts`). This function is actually used a lot (even more so since https://reviews.llvm.org/D32291), and seems fairly well-tested, so i felt relatively safe when refactoring it. > But I wonder if there could be some cases where we have such cast earlier > than expected. I'd love to know :o https://reviews.llvm.org/D37023 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r311884 - [refactor] initial support for refactoring action rules
Author: arphaman Date: Mon Aug 28 04:12:05 2017 New Revision: 311884 URL: http://llvm.org/viewvc/llvm-project?rev=311884&view=rev Log: [refactor] initial support for refactoring action rules This patch implements the initial support for refactoring action rules. The first rule that's supported is a "source change" rule that returns a set of atomic changes. This patch is based on the ideas presented in my RFC: http://lists.llvm.org/pipermail/cfe-dev/2017-July/054831.html The following pieces from the RFC are added by this patch: - `createRefactoringRule` (known as `apply` in the RFC) - `requiredSelection` refactoring action rule requirement. - `selection::SourceSelectionRange` selection constraint. Differential Revision: https://reviews.llvm.org/D36075 Added: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRules.h cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h cfe/trunk/include/clang/Tooling/Refactoring/RefactoringRuleContext.h cfe/trunk/include/clang/Tooling/Refactoring/SourceSelectionConstraints.h cfe/trunk/lib/Tooling/Refactoring/SourceSelectionConstraints.cpp cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp Modified: cfe/trunk/include/clang/Basic/LLVM.h cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt cfe/trunk/unittests/Tooling/CMakeLists.txt Modified: cfe/trunk/include/clang/Basic/LLVM.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LLVM.h?rev=311884&r1=311883&r2=311884&view=diff == --- cfe/trunk/include/clang/Basic/LLVM.h (original) +++ cfe/trunk/include/clang/Basic/LLVM.h Mon Aug 28 04:12:05 2017 @@ -35,6 +35,7 @@ namespace llvm { template class SmallVector; template class SmallVectorImpl; template class Optional; + template class Expected; template struct SaveAndRestore; @@ -71,6 +72,9 @@ namespace clang { using llvm::SmallVectorImpl; using llvm::SaveAndRestore; + // Error handling. + using llvm::Expected; + // Reference counting. using llvm::IntrusiveRefCntPtr; using llvm::IntrusiveRefCntPtrInfo; Modified: cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h?rev=311884&r1=311883&r2=311884&view=diff == --- cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h (original) +++ cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h Mon Aug 28 04:12:05 2017 @@ -46,6 +46,12 @@ public: AtomicChange(llvm::StringRef FilePath, llvm::StringRef Key) : Key(Key), FilePath(FilePath) {} + AtomicChange(AtomicChange &&) = default; + AtomicChange(const AtomicChange &) = default; + + AtomicChange &operator=(AtomicChange &&) = default; + AtomicChange &operator=(const AtomicChange &) = default; + /// \brief Returns the atomic change as a YAML string. std::string toYAMLString(); @@ -130,6 +136,8 @@ private: tooling::Replacements Replaces; }; +using AtomicChanges = std::vector; + // Defines specs for applying changes. struct ApplyChangesSpec { // If true, cleans up redundant/erroneous code around changed code with Added: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h?rev=311884&view=auto == --- cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h (added) +++ cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h Mon Aug 28 04:12:05 2017 @@ -0,0 +1,70 @@ +//===--- RefactoringActionRule.h - Clang refactoring library -===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_H +#define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_H + +#include "clang/Basic/LLVM.h" +#include "clang/Tooling/Refactoring/AtomicChange.h" +#include "llvm/Support/Error.h" +#include + +namespace clang { +namespace tooling { + +class RefactoringRuleContext; + +/// A common refactoring action rule interface. +class RefactoringActionRule { +public: + enum RuleKind { SourceChangeRefactoringRuleKind }; + + RuleKind getRuleKind() const { return Kind; } + + virtual
[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor
arphaman added a comment. Ah I see, then you should something completely different (forget the header). Please extract this code into a protected function in RecursiveASTVisitor: for (Stmt *SubStmt : Children) if (!TRAVERSE_STMT_BASE(Stmt, Stmt, SubStmt, Queue)) return false; And reuse it in this class. Then you won't have to use any macros or related template magic here. https://reviews.llvm.org/D37200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36075: [refactor] Initial support for refactoring action rules
This revision was automatically updated to reflect the committed changes. Closed by commit rL311884: [refactor] initial support for refactoring action rules (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D36075?vs=112678&id=112879#toc Repository: rL LLVM https://reviews.llvm.org/D36075 Files: cfe/trunk/include/clang/Basic/LLVM.h cfe/trunk/include/clang/Tooling/Refactoring/AtomicChange.h cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirementsInternal.h cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRules.h cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h cfe/trunk/include/clang/Tooling/Refactoring/RefactoringRuleContext.h cfe/trunk/include/clang/Tooling/Refactoring/SourceSelectionConstraints.h cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt cfe/trunk/lib/Tooling/Refactoring/SourceSelectionConstraints.cpp cfe/trunk/unittests/Tooling/CMakeLists.txt cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp Index: cfe/trunk/lib/Tooling/Refactoring/SourceSelectionConstraints.cpp === --- cfe/trunk/lib/Tooling/Refactoring/SourceSelectionConstraints.cpp +++ cfe/trunk/lib/Tooling/Refactoring/SourceSelectionConstraints.cpp @@ -0,0 +1,23 @@ +//===--- SourceSelectionConstraints.cpp - Evaluate selection constraints --===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "clang/Tooling/Refactoring/SourceSelectionConstraints.h" +#include "clang/Tooling/Refactoring/RefactoringRuleContext.h" + +using namespace clang; +using namespace tooling; +using namespace selection; + +Optional +SourceSelectionRange::evaluate(RefactoringRuleContext &Context) { + SourceRange R = Context.getSelectionRange(); + if (R.isInvalid()) +return None; + return SourceSelectionRange(Context.getSources(), R); +} Index: cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt === --- cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt +++ cfe/trunk/lib/Tooling/Refactoring/CMakeLists.txt @@ -8,6 +8,7 @@ Rename/USRFinder.cpp Rename/USRFindingAction.cpp Rename/USRLocFinder.cpp + SourceSelectionConstraints.cpp LINK_LIBS clangAST Index: cfe/trunk/unittests/Tooling/CMakeLists.txt === --- cfe/trunk/unittests/Tooling/CMakeLists.txt +++ cfe/trunk/unittests/Tooling/CMakeLists.txt @@ -25,6 +25,7 @@ RecursiveASTVisitorTestDeclVisitor.cpp RecursiveASTVisitorTestExprVisitor.cpp RecursiveASTVisitorTestTypeLocVisitor.cpp + RefactoringActionRulesTest.cpp RefactoringCallbacksTest.cpp RefactoringTest.cpp ReplacementsYamlTest.cpp Index: cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp === --- cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp +++ cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp @@ -0,0 +1,165 @@ +//===- unittest/Tooling/RefactoringTestActionRulesTest.cpp ===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "ReplacementTest.h" +#include "RewriterTestContext.h" +#include "clang/Tooling/Refactoring.h" +#include "clang/Tooling/Refactoring/RefactoringActionRules.h" +#include "clang/Tooling/Tooling.h" +#include "llvm/Support/Errc.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace tooling; +using namespace refactoring_action_rules; + +namespace { + +class RefactoringActionRulesTest : public ::testing::Test { +protected: + void SetUp() override { +Context.Sources.setMainFileID( +Context.createInMemoryFile("input.cpp", DefaultCode)); + } + + RewriterTestContext Context; + std::string DefaultCode = std::string(100, 'a'); +}; + +Expected> +createReplacements(const std::unique_ptr &Rule, + RefactoringRuleContext &Context) { + return cast(*Rule).createSourceReplacements( + Context); +} + +TEST_F(RefactoringActionRulesTest, MyFirstRefactoringRule) { + auto ReplaceAWithB = + [](std::pair Selection) + -> Expected { +const SourceManager &SM = Selection.first.getSources(); +SourceLocation Loc = Selection.first.getRange().getBegin().getLocWithOffset( +Selection.second); +AtomicChange Change(SM, Loc); +llvm::Error E = Change.
[PATCH] D37210: [refactor] add a refactoring action rule that returns symbol occurrences
arphaman created this revision. This patch adds a second kind of refactoring action rule that produces symbol occurrences. It will be used by the updated `clang-refactor` patch at https://reviews.llvm.org/D36574. Repository: rL LLVM https://reviews.llvm.org/D37210 Files: include/clang/Tooling/Refactoring/RefactoringActionRule.h include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h unittests/Tooling/RefactoringActionRulesTest.cpp Index: unittests/Tooling/RefactoringActionRulesTest.cpp === --- unittests/Tooling/RefactoringActionRulesTest.cpp +++ unittests/Tooling/RefactoringActionRulesTest.cpp @@ -11,6 +11,7 @@ #include "RewriterTestContext.h" #include "clang/Tooling/Refactoring.h" #include "clang/Tooling/Refactoring/RefactoringActionRules.h" +#include "clang/Tooling/Refactoring/Rename/SymbolName.h" #include "clang/Tooling/Tooling.h" #include "llvm/Support/Errc.h" #include "gtest/gtest.h" @@ -162,4 +163,41 @@ EXPECT_EQ(Message, "bad selection"); } +Expected> +findOccurrences(const std::unique_ptr &Rule, +RefactoringRuleContext &Context) { + return cast(*Rule) + .findSymbolOccurrences(Context); +} + +TEST_F(RefactoringActionRulesTest, ReturnSymbolOccurrences) { + auto Rule = createRefactoringRule( + [](selection::SourceSelectionRange Selection) + -> Expected { +SymbolOccurrences Occurrences; +Occurrences.push_back(SymbolOccurrence( +SymbolName("test"), SymbolOccurrence::MatchingSymbol, +Selection.getRange().getBegin())); +return Occurrences; + }, + requiredSelection( + selection::identity())); + + RefactoringRuleContext RefContext(Context.Sources); + SourceLocation Cursor = + Context.Sources.getLocForStartOfFile(Context.Sources.getMainFileID()); + RefContext.setSelectionRange({Cursor, Cursor}); + Expected> Result = + findOccurrences(Rule, RefContext); + + ASSERT_FALSE(!Result); + ASSERT_FALSE(!*Result); + SymbolOccurrences Occurrences = std::move(**Result); + EXPECT_EQ(Occurrences.size(), 1u); + EXPECT_EQ(Occurrences[0].getKind(), SymbolOccurrence::MatchingSymbol); + EXPECT_EQ(Occurrences[0].getNameRanges().size(), 1u); + EXPECT_EQ(Occurrences[0].getNameRanges()[0], +SourceRange(Cursor, Cursor.getLocWithOffset(strlen("test"; +} + } // end anonymous namespace Index: include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h === --- include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h +++ include/clang/Tooling/Refactoring/RefactoringActionRulesInternal.h @@ -26,18 +26,26 @@ /// 'perform' method from the specific refactoring method. template struct SpecificRefactoringRuleAdapter {}; -template <> -class SpecificRefactoringRuleAdapter -: public SourceChangeRefactoringRule { -public: - virtual Expected> - perform(RefactoringRuleContext &Context) = 0; - - Expected> - createSourceReplacements(RefactoringRuleContext &Context) final override { -return perform(Context); - } -}; +#define CLANG_REFACTOR_DEFINE_RULE_ADAPTER(ResultType, RuleType, OverrideName) \ + template <> \ + class SpecificRefactoringRuleAdapter : public RuleType { \ + public: \ +virtual Expected> \ +perform(RefactoringRuleContext &Context) = 0; \ + \ +Expected> \ +OverrideName(RefactoringRuleContext &Context) final override { \ + return perform(Context); \ +} \ + }; + +CLANG_REFACTOR_DEFINE_RULE_ADAPTER(AtomicChanges, SourceChangeRefactoringRule, + createSourceReplacements) +CLANG_REFACTOR_DEFINE_RULE_ADAPTER(SymbolOccurrences, + FindSymbolOccurrencesRefactoringRule, + findSymbolOccurrences) + +#undef CLANG_REFACTOR_DEFINE_RULE_ADAPTER /// A specialized refactoring action rule that calls the stored function once /// all the of the requirements are fullfilled. The values produced during the Index: include/clang/Tooling/Refactoring/RefactoringActionRule.h === --- include/clang/Tooling/Refactoring/RefactoringActionRule.h +++ include/clang/Tooling/Refactoring/RefactoringActionRule.h @@ -12,6 +12,7 @@ #include "clang/Basic/LLVM.h" #include "clang/Tooling/Refactoring/AtomicChange.h" +#include "clang/Tooling/Refactoring/Rename/SymbolOccurrences.h" #include "llvm
[PATCH] D37120: [analyzer] Fix modeling arithmetic
alexshap added a comment. updated the patch per suggestion by @NoQ Repository: rL LLVM https://reviews.llvm.org/D37120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37150: [clangd] Command line arg to specify compile_commands.json path
ilya-biryukov added inline comments. Comment at: clangd/GlobalCompilationDatabase.cpp:29 + +static cl::opt CompileCommands("compileCommands", + cl::desc("Start with absolute path to compile_commands.json")); Please place this flag in `ClangdMain.cpp`. Comment at: clangd/GlobalCompilationDatabase.cpp:29 + +static cl::opt CompileCommands("compileCommands", + cl::desc("Start with absolute path to compile_commands.json")); ilya-biryukov wrote: > Please place this flag in `ClangdMain.cpp`. Following naming convention for flags, this should be `compile-commands`. Given the semantics of the flag, maybe it's better to name it `compile-commands-dir`? Comment at: clangd/GlobalCompilationDatabase.cpp:30 +static cl::opt CompileCommands("compileCommands", + cl::desc("Start with absolute path to compile_commands.json")); +} A description is somewhat vague. What is the intention of this flag? - Always look for `compile_commands.json` only inside specified directory? - First look inside specified directory, then proceed as normal (i.e. looking at the parent paths of each source file)? It looks like the code also looks at the parent directories of `compile-commands`. Is that the original intention? Maybe consider making it look only inside the directory itself and not its parents? Comment at: clangd/GlobalCompilationDatabase.cpp:57 + + Incidental whitespace change? Comment at: clangd/GlobalCompilationDatabase.cpp:89 + // if --compileCommands arg was invoked, check value and override default path + std::size_t found = CompileCommands.find_first_of("/"); + std::string TempString = CompileCommands; Please check command arguments for validity in `main()`. What is the purpose of `find_first_of("/")`? Checking for absolute paths? If yes, you could use `llvm::sys::path::is_absolute`. Comment at: clangd/GlobalCompilationDatabase.cpp:97 + std::string Error; + bool badPath = false; + File = path::parent_path(File); Naming: local variables should be `UpperCamelCase` Comment at: clangd/GlobalCompilationDatabase.cpp:99 + File = path::parent_path(File); + auto CachedIt = CompilationDatabases.find(File); + if (CachedIt != CompilationDatabases.end()) Maybe extract a function from the for loop body instead of copying it here? Could be named `tryLoadDatabaseFromPath`. Comment at: clangd/GlobalCompilationDatabase.cpp:113 - for (auto Path = path::parent_path(File); !Path.empty(); + if (badPath) + { Maybe rewrite to avoid extra nesting? For example, ``` if (!badPath) { auto result = CDB.get(); CompilationDatabases.insert(std::make_pair(File, std::move(CDB))); return result; } // Proceed as before... ``` Comment at: clangd/GlobalCompilationDatabase.cpp:138 + { +auto result = CDB.get(); +CompilationDatabases.insert(std::make_pair(File, std::move(CDB))); Please consider removing this code duplication by extracting a function from the loop body (also see my previous comment). https://reviews.llvm.org/D37150 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30946: [ScopePrinting] Added support to print full scopes of types and declarations.
schroedersi added a comment. Ping :) https://reviews.llvm.org/D30946 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33852: Enable __declspec(selectany) on linux
aaron.ballman added a comment. In https://reviews.llvm.org/D33852#853410, @Prazek wrote: > Sorry for so late fixes, but it would be good to put it in 5.0 I do not think this should be in 5.0, as I believe we're only accepting regression fixes at this point. Comment at: include/clang/Basic/Attr.td:2477 let Spellings = [Declspec<"selectany">, GCC<"selectany">]; let Documentation = [Undocumented]; } Since we're drastically modifying what platforms this is supported on, can you please add documentation for this attribute? https://reviews.llvm.org/D33852 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor
johannes updated this revision to Diff 112885. johannes added a comment. use RecursiveASTVisitor::TraverseStmt https://reviews.llvm.org/D37200 Files: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp Index: unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp === --- unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp +++ unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp @@ -21,9 +21,10 @@ : public LexicallyOrderedRecursiveASTVisitor { public: LexicallyOrderedDeclVisitor(DummyMatchVisitor &Matcher, - const SourceManager &SM, bool EmitIndices) + const SourceManager &SM, bool EmitDeclIndices, + bool EmitStmtIndices) : LexicallyOrderedRecursiveASTVisitor(SM), Matcher(Matcher), -EmitIndices(EmitIndices) {} +EmitDeclIndices(EmitDeclIndices), EmitStmtIndices(EmitStmtIndices) {} bool TraverseDecl(Decl *D) { TraversalStack.push_back(D); @@ -33,28 +34,33 @@ } bool VisitNamedDecl(const NamedDecl *D); + bool VisitDeclRefExpr(const DeclRefExpr *D); private: DummyMatchVisitor &Matcher; - bool EmitIndices; + bool EmitDeclIndices, EmitStmtIndices; unsigned Index = 0; llvm::SmallVector TraversalStack; }; class DummyMatchVisitor : public ExpectedLocationVisitor { - bool EmitIndices; + bool EmitDeclIndices, EmitStmtIndices; public: - DummyMatchVisitor(bool EmitIndices = false) : EmitIndices(EmitIndices) {} + DummyMatchVisitor(bool EmitDeclIndices = false, bool EmitStmtIndices = false) + : EmitDeclIndices(EmitDeclIndices), EmitStmtIndices(EmitStmtIndices) {} bool VisitTranslationUnitDecl(TranslationUnitDecl *TU) { const ASTContext &Context = TU->getASTContext(); const SourceManager &SM = Context.getSourceManager(); -LexicallyOrderedDeclVisitor SubVisitor(*this, SM, EmitIndices); +LexicallyOrderedDeclVisitor SubVisitor(*this, SM, EmitDeclIndices, + EmitStmtIndices); SubVisitor.TraverseDecl(TU); return false; } - void match(StringRef Path, const Decl *D) { Match(Path, D->getLocStart()); } + template void match(StringRef Path, const T *D) { +Match(Path, D->getLocStart()); + } }; bool LexicallyOrderedDeclVisitor::VisitNamedDecl(const NamedDecl *D) { @@ -73,7 +79,16 @@ if (isa(D) or isa(D)) OS << "/"; } - if (EmitIndices) + if (EmitDeclIndices) +OS << "@" << Index++; + Matcher.match(OS.str(), D); + return true; +} + +bool LexicallyOrderedDeclVisitor::VisitDeclRefExpr(const DeclRefExpr *D) { + std::string Name = D->getFoundDecl()->getNameAsString(); + llvm::raw_string_ostream OS(Name); + if (EmitStmtIndices) OS << "@" << Index++; Matcher.match(OS.str(), D); return true; @@ -151,13 +166,50 @@ template T f(); template class Class {}; )"; - DummyMatchVisitor Visitor(/*EmitIndices=*/true); + DummyMatchVisitor Visitor(/*EmitDeclIndices=*/true); Visitor.ExpectMatch("/f/T@0", 2, 11); Visitor.ExpectMatch("/f/f/@1", 2, 20); Visitor.ExpectMatch("/Class/U@2", 3, 11); Visitor.ExpectMatch("/Class/@3", 3, 20); Visitor.ExpectMatch("/Class/Class/@4", 3, 34); EXPECT_TRUE(Visitor.runOver(Source)); } +TEST(LexicallyOrderedRecursiveASTVisitor, VisitCXXOperatorCallExpr) { + StringRef Source = R"( +struct S { + S &operator+(S&); + S *operator->(); + S &operator++(); + S operator++(int); + void operator()(int, int); + void operator[](int); + void f(); +}; +S a, b, c; + +void test() { + a = b + c; + a->f(); + a(1, 2); + b[0]; + ++a; + b++; +} +)"; + DummyMatchVisitor Visitor(/*EmitDeclIndices=*/false, +/*EmitStmtIndices=*/true); + Visitor.ExpectMatch("a@0", 14, 3); + Visitor.ExpectMatch("operator=@1", 14, 5); + Visitor.ExpectMatch("b@2", 14, 7); + Visitor.ExpectMatch("operator+@3", 14, 9); + Visitor.ExpectMatch("c@4", 14, 11); + Visitor.ExpectMatch("operator->@6", 15, 4); + Visitor.ExpectMatch("operator()@8", 16, 4); + Visitor.ExpectMatch("operator[]@10", 17, 4); + Visitor.ExpectMatch("operator++@11", 18, 3); + Visitor.ExpectMatch("operator++@14", 19, 4); + EXPECT_TRUE(Visitor.runOver(Source)); +} + } // end anonymous namespace Index: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h === --- include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h +++ include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h @@ -60,6 +60,7 @@ class LexicallyOrderedRecursiveASTVisitor : public RecursiveASTVisitor { using BaseType = RecursiveASTVisitor; + using typename BaseType::DataRecursionQueue; public: LexicallyOrderedRecursiveASTVisitor(const SourceManager &SM) : SM(SM) {} @@ -126,6 +127,36 @@ DEF_TRAVERSE_TEMPLATE_DECL(Function) #undef DEF_T
r311886 - Avoid missing std error code in RefactoringActionRulesTest.cpp
Author: arphaman Date: Mon Aug 28 05:03:08 2017 New Revision: 311886 URL: http://llvm.org/viewvc/llvm-project?rev=311886&view=rev Log: Avoid missing std error code in RefactoringActionRulesTest.cpp This should fix this bot: http://bb.pgr.jp/builders/i686-mingw32-RA-on-linux Modified: cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp Modified: cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp?rev=311886&r1=311885&r2=311886&view=diff == --- cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp (original) +++ cfe/trunk/unittests/Tooling/RefactoringActionRulesTest.cpp Mon Aug 28 05:03:08 2017 @@ -111,7 +111,7 @@ TEST_F(RefactoringActionRulesTest, Retur Expected (*Func)(selection::SourceSelectionRange) = [](selection::SourceSelectionRange) -> Expected { return llvm::make_error( -"Error", std::make_error_code(std::errc::bad_message)); +"Error", llvm::make_error_code(llvm::errc::invalid_argument)); }; auto Rule = createRefactoringRule( Func, requiredSelection( @@ -139,7 +139,7 @@ TEST_F(RefactoringActionRulesTest, Retur Expected> evaluateSelection(selection::SourceSelectionRange Selection) const { return llvm::make_error( - "bad selection", std::make_error_code(std::errc::bad_message)); + "bad selection", llvm::make_error_code(llvm::errc::invalid_argument)); } }; auto Rule = createRefactoringRule( ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor
arphaman added a comment. This works as well I think https://reviews.llvm.org/D37200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor
arphaman added inline comments. Comment at: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h:63 using BaseType = RecursiveASTVisitor; + using typename BaseType::DataRecursionQueue; Do you still need the using here? https://reviews.llvm.org/D37200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37023: [analyzer] Fix bugreporter::getDerefExpr() again.
xazax.hun added a comment. In https://reviews.llvm.org/D37023#853941, @NoQ wrote: > Thank you for the review! > > > Though it looks like some of the cases covered in the code do not have > > corresponding tests (e.g.: the parenexprs). > > These are covered by tests in `inline-defensive-checks.c:150,156,169,179` > (old code had `IgnoreParenCasts`). This function is actually used a lot (even > more so since https://reviews.llvm.org/D32291), and seems fairly well-tested, > so i felt relatively safe when refactoring it. Oh, indeed, thanks :) > > >> But I wonder if there could be some cases where we have such cast earlier >> than expected. > > I'd love to know :o I started to check some of the cases I was thinking about, but all of them looked fine :) https://reviews.llvm.org/D37023 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36487: Emit section information for extern variables.
eandrews added a comment. *ping* https://reviews.llvm.org/D36487 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor
johannes updated this revision to Diff 112889. johannes added a comment. do call derived TraverseStmt for children of CXXOperatorCallExpr https://reviews.llvm.org/D37200 Files: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h include/clang/AST/RecursiveASTVisitor.h unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp Index: unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp === --- unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp +++ unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp @@ -21,42 +21,55 @@ : public LexicallyOrderedRecursiveASTVisitor { public: LexicallyOrderedDeclVisitor(DummyMatchVisitor &Matcher, - const SourceManager &SM, bool EmitIndices) + const SourceManager &SM, bool EmitDeclIndices, + bool EmitStmtIndices) : LexicallyOrderedRecursiveASTVisitor(SM), Matcher(Matcher), -EmitIndices(EmitIndices) {} +EmitDeclIndices(EmitDeclIndices), EmitStmtIndices(EmitStmtIndices) {} bool TraverseDecl(Decl *D) { TraversalStack.push_back(D); LexicallyOrderedRecursiveASTVisitor::TraverseDecl(D); TraversalStack.pop_back(); return true; } + bool TraverseStmt(Stmt *S); + bool VisitNamedDecl(const NamedDecl *D); + bool VisitDeclRefExpr(const DeclRefExpr *D); private: DummyMatchVisitor &Matcher; - bool EmitIndices; + bool EmitDeclIndices, EmitStmtIndices; unsigned Index = 0; llvm::SmallVector TraversalStack; }; class DummyMatchVisitor : public ExpectedLocationVisitor { - bool EmitIndices; + bool EmitDeclIndices, EmitStmtIndices; public: - DummyMatchVisitor(bool EmitIndices = false) : EmitIndices(EmitIndices) {} + DummyMatchVisitor(bool EmitDeclIndices = false, bool EmitStmtIndices = false) + : EmitDeclIndices(EmitDeclIndices), EmitStmtIndices(EmitStmtIndices) {} bool VisitTranslationUnitDecl(TranslationUnitDecl *TU) { const ASTContext &Context = TU->getASTContext(); const SourceManager &SM = Context.getSourceManager(); -LexicallyOrderedDeclVisitor SubVisitor(*this, SM, EmitIndices); +LexicallyOrderedDeclVisitor SubVisitor(*this, SM, EmitDeclIndices, + EmitStmtIndices); SubVisitor.TraverseDecl(TU); return false; } - void match(StringRef Path, const Decl *D) { Match(Path, D->getLocStart()); } + template void match(StringRef Path, const T *D) { +Match(Path, D->getLocStart()); + } }; +bool LexicallyOrderedDeclVisitor::TraverseStmt(Stmt *S) { + Matcher.match("overridden TraverseStmt", S); + return LexicallyOrderedRecursiveASTVisitor::TraverseStmt(S); +} + bool LexicallyOrderedDeclVisitor::VisitNamedDecl(const NamedDecl *D) { std::string Path; llvm::raw_string_ostream OS(Path); @@ -73,7 +86,16 @@ if (isa(D) or isa(D)) OS << "/"; } - if (EmitIndices) + if (EmitDeclIndices) +OS << "@" << Index++; + Matcher.match(OS.str(), D); + return true; +} + +bool LexicallyOrderedDeclVisitor::VisitDeclRefExpr(const DeclRefExpr *D) { + std::string Name = D->getFoundDecl()->getNameAsString(); + llvm::raw_string_ostream OS(Name); + if (EmitStmtIndices) OS << "@" << Index++; Matcher.match(OS.str(), D); return true; @@ -151,13 +173,55 @@ template T f(); template class Class {}; )"; - DummyMatchVisitor Visitor(/*EmitIndices=*/true); + DummyMatchVisitor Visitor(/*EmitDeclIndices=*/true); Visitor.ExpectMatch("/f/T@0", 2, 11); Visitor.ExpectMatch("/f/f/@1", 2, 20); Visitor.ExpectMatch("/Class/U@2", 3, 11); Visitor.ExpectMatch("/Class/@3", 3, 20); Visitor.ExpectMatch("/Class/Class/@4", 3, 34); EXPECT_TRUE(Visitor.runOver(Source)); } +TEST(LexicallyOrderedRecursiveASTVisitor, VisitCXXOperatorCallExpr) { + StringRef Source = R"( +struct S { + S &operator+(S&); + S *operator->(); + S &operator++(); + S operator++(int); + void operator()(int, int); + void operator[](int); + void f(); +}; +S a, b, c; + +void test() { + a = b + c; + a->f(); + a(1, 2); + b[0]; + ++a; + b++; +} +)"; + DummyMatchVisitor Visitor(/*EmitDeclIndices=*/false, +/*EmitStmtIndices=*/true); + // There are two overloaded operators that start at this point + // This makes sure they are both traversed using the overridden + // TraverseStmt, as the traversal is implemented by us for + // CXXOperatorCallExpr. + Visitor.ExpectMatch("overridden TraverseStmt", 14, 3, 2); + Visitor.ExpectMatch("a@0", 14, 3); + Visitor.ExpectMatch("operator=@1", 14, 5); + Visitor.ExpectMatch("b@2", 14, 7); + Visitor.ExpectMatch("operator+@3", 14, 9); + Visitor.ExpectMatch("c@4", 14, 11); + Visitor.ExpectMatch("operator->@6", 15, 4); + Visitor.ExpectMatch("operator()@8", 16, 4); + Visitor.ExpectMatch("operator[]@10", 17, 4); + Visitor.ExpectMatch("operator++@11", 18, 3); + Visitor.E
[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor
johannes added a comment. The previous version didn't call TraverseDecl of the derived class, this is fixed now. The public getDerived.TraverseStmt() does not accept a DataRecursionQueue, so this also could not be used (I think) I used the wrapper TraverseStmtBase, which should behave exactly as the method that originally traverses CXXOperatorCallExpr Comment at: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h:63 using BaseType = RecursiveASTVisitor; + using typename BaseType::DataRecursionQueue; arphaman wrote: > Do you still need the using here? removed it now as it's only used once https://reviews.llvm.org/D37200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor
arphaman added inline comments. Comment at: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h:149 + auto Offset = [&](Stmt *S) { return SM.getFileOffset(S->getLocStart()); }; + Swap = Offset(Children[0]) > Offset(Children[1]); + break; For `++` and `--` you can see whether its prefix or postfix by looking at the number of arguments. If there's one argument, then `++` and `--` are prefix. Otherwise, they're postfix. https://reviews.llvm.org/D37200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor
johannes updated this revision to Diff 112894. johannes added a comment. detect prefix/postfix from number of arguments https://reviews.llvm.org/D37200 Files: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h include/clang/AST/RecursiveASTVisitor.h unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp Index: unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp === --- unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp +++ unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp @@ -21,42 +21,55 @@ : public LexicallyOrderedRecursiveASTVisitor { public: LexicallyOrderedDeclVisitor(DummyMatchVisitor &Matcher, - const SourceManager &SM, bool EmitIndices) + const SourceManager &SM, bool EmitDeclIndices, + bool EmitStmtIndices) : LexicallyOrderedRecursiveASTVisitor(SM), Matcher(Matcher), -EmitIndices(EmitIndices) {} +EmitDeclIndices(EmitDeclIndices), EmitStmtIndices(EmitStmtIndices) {} bool TraverseDecl(Decl *D) { TraversalStack.push_back(D); LexicallyOrderedRecursiveASTVisitor::TraverseDecl(D); TraversalStack.pop_back(); return true; } + bool TraverseStmt(Stmt *S); + bool VisitNamedDecl(const NamedDecl *D); + bool VisitDeclRefExpr(const DeclRefExpr *D); private: DummyMatchVisitor &Matcher; - bool EmitIndices; + bool EmitDeclIndices, EmitStmtIndices; unsigned Index = 0; llvm::SmallVector TraversalStack; }; class DummyMatchVisitor : public ExpectedLocationVisitor { - bool EmitIndices; + bool EmitDeclIndices, EmitStmtIndices; public: - DummyMatchVisitor(bool EmitIndices = false) : EmitIndices(EmitIndices) {} + DummyMatchVisitor(bool EmitDeclIndices = false, bool EmitStmtIndices = false) + : EmitDeclIndices(EmitDeclIndices), EmitStmtIndices(EmitStmtIndices) {} bool VisitTranslationUnitDecl(TranslationUnitDecl *TU) { const ASTContext &Context = TU->getASTContext(); const SourceManager &SM = Context.getSourceManager(); -LexicallyOrderedDeclVisitor SubVisitor(*this, SM, EmitIndices); +LexicallyOrderedDeclVisitor SubVisitor(*this, SM, EmitDeclIndices, + EmitStmtIndices); SubVisitor.TraverseDecl(TU); return false; } - void match(StringRef Path, const Decl *D) { Match(Path, D->getLocStart()); } + template void match(StringRef Path, const T *D) { +Match(Path, D->getLocStart()); + } }; +bool LexicallyOrderedDeclVisitor::TraverseStmt(Stmt *S) { + Matcher.match("overridden TraverseStmt", S); + return LexicallyOrderedRecursiveASTVisitor::TraverseStmt(S); +} + bool LexicallyOrderedDeclVisitor::VisitNamedDecl(const NamedDecl *D) { std::string Path; llvm::raw_string_ostream OS(Path); @@ -73,7 +86,16 @@ if (isa(D) or isa(D)) OS << "/"; } - if (EmitIndices) + if (EmitDeclIndices) +OS << "@" << Index++; + Matcher.match(OS.str(), D); + return true; +} + +bool LexicallyOrderedDeclVisitor::VisitDeclRefExpr(const DeclRefExpr *D) { + std::string Name = D->getFoundDecl()->getNameAsString(); + llvm::raw_string_ostream OS(Name); + if (EmitStmtIndices) OS << "@" << Index++; Matcher.match(OS.str(), D); return true; @@ -151,13 +173,55 @@ template T f(); template class Class {}; )"; - DummyMatchVisitor Visitor(/*EmitIndices=*/true); + DummyMatchVisitor Visitor(/*EmitDeclIndices=*/true); Visitor.ExpectMatch("/f/T@0", 2, 11); Visitor.ExpectMatch("/f/f/@1", 2, 20); Visitor.ExpectMatch("/Class/U@2", 3, 11); Visitor.ExpectMatch("/Class/@3", 3, 20); Visitor.ExpectMatch("/Class/Class/@4", 3, 34); EXPECT_TRUE(Visitor.runOver(Source)); } +TEST(LexicallyOrderedRecursiveASTVisitor, VisitCXXOperatorCallExpr) { + StringRef Source = R"( +struct S { + S &operator+(S&); + S *operator->(); + S &operator++(); + S operator++(int); + void operator()(int, int); + void operator[](int); + void f(); +}; +S a, b, c; + +void test() { + a = b + c; + a->f(); + a(1, 2); + b[0]; + ++a; + b++; +} +)"; + DummyMatchVisitor Visitor(/*EmitDeclIndices=*/false, +/*EmitStmtIndices=*/true); + // There are two overloaded operators that start at this point + // This makes sure they are both traversed using the overridden + // TraverseStmt, as the traversal is implemented by us for + // CXXOperatorCallExpr. + Visitor.ExpectMatch("overridden TraverseStmt", 14, 3, 2); + Visitor.ExpectMatch("a@0", 14, 3); + Visitor.ExpectMatch("operator=@1", 14, 5); + Visitor.ExpectMatch("b@2", 14, 7); + Visitor.ExpectMatch("operator+@3", 14, 9); + Visitor.ExpectMatch("c@4", 14, 11); + Visitor.ExpectMatch("operator->@6", 15, 4); + Visitor.ExpectMatch("operator()@8", 16, 4); + Visitor.ExpectMatch("operator[]@10", 17, 4); + Visitor.ExpectMatch("operator++@11", 18, 3); + Visitor.ExpectMatch("operat
[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor
arphaman added a comment. The fact `TraverseCXXOperatorCallExpr` can't call its super `TraverseCXXOperatorCallExpr` makes the current solution kind of broken. The super implementation of `TraverseCXXOperatorCallExpr` is responsible for dispatch to WalkUp##STMT functions which actually call `VisitCXXOperatorCallExpr` which our current visitor will never call because of the custom "override". I would like to suggest an alternative approach that I think solves the problem more "elegantly": Modify the `DEF_TRAVERSE_STMT` macro in RecursiveASTVisitor.h call to a wrapper function to get the children: class RecursiveASTVisitor { public: Stmt::child_range getStmtChildren(Stmt *S) { return S->children(); } ... #define DEF_TRAVERSE_STMT(STMT, CODE) \ template \ bool RecursiveASTVisitor::Traverse##STMT( \ STMT *S, DataRecursionQueue *Queue) { \ bool ShouldVisitChildren = true; \ bool ReturnValue = true; \ if (!getDerived().shouldTraversePostOrder()) \ TRY_TO(WalkUpFrom##STMT(S)); \ { CODE; } \ if (ShouldVisitChildren) { \ for (Stmt *SubStmt : getDerived().getStmtChildren(S)) { \ // The only change is on this line. TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(SubStmt); \ } \ } \ if (!Queue && ReturnValue && getDerived().shouldTraversePostOrder()) \ TRY_TO(WalkUpFrom##STMT(S)); \ return ReturnValue; \ } ... }; Then you could provide an "override" in the `LexicallyOrderedRecursiveASTVisitor` that adjusts children just for `CXXOperatorCallExpr`. Something like this should work: class LexicallyOrderedRecursiveASTVisitor { public: ... Stmt::child_range getStmtChildren(Stmt *S) { return S->children(); } SmallVector getStmtChildren(CXXOperatorCallExpr *CE) { SmallVector Children(CE->children()); bool Swap; // Switch the operator and the first operand for all infix and postfix // operations. switch (CE->getOperator()) { case OO_Arrow: case OO_Call: case OO_Subscript: Swap = true; break; case OO_PlusPlus: case OO_MinusMinus: // These are postfix unless there is exactly one argument. Swap = Children.size() != 2; break; default: Swap = CE->isInfixBinaryOp(); break; } if (Swap && Children.size() > 1) std::swap(Children[0], Children[1]); return Children; } ... }; WDYT? Sorry about not seeing this earlier. Thanks for your patience! https://reviews.llvm.org/D37200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37120: [analyzer] Fix modeling arithmetic
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Thanks, LGTM! Repository: rL LLVM https://reviews.llvm.org/D37120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36075: [refactor] Initial support for refactoring action rules
klimek added inline comments. Comment at: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h:23-37 +/// A common refactoring action rule interface. +class RefactoringActionRule { +public: + enum RuleKind { SourceChangeRefactoringRuleKind }; + + RuleKind getRuleKind() const { return Kind; } + Sorry for being late, was out on vacation. Generally, why do we need this tag-based abstraction here instead of using the more typical OO double-dispatch where necessary? (We do this in the AST a lot, but the AST is special, because there we want to implement a lot of different algorithms that rely on the node type, while I don't see how that applies here) Repository: rL LLVM https://reviews.llvm.org/D36075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37188: [clang-tools-extra] [cmake] Support running extra clang tool tests without static analyzer
alexfh added a comment. This could be done even more granularly, since clang-tidy only needs static analyzer to run clang-analyzer-* checks. But that would involve some careful #ifdef-ing of the parts of clang-tidy/ClangTidy.cpp, so I'll understand if you say you have little interest in this. Repository: rL LLVM https://reviews.llvm.org/D37188 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36075: [refactor] Initial support for refactoring action rules
arphaman added inline comments. Comment at: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h:23-37 +/// A common refactoring action rule interface. +class RefactoringActionRule { +public: + enum RuleKind { SourceChangeRefactoringRuleKind }; + + RuleKind getRuleKind() const { return Kind; } + klimek wrote: > Sorry for being late, was out on vacation. > Generally, why do we need this tag-based abstraction here instead of using > the more typical OO double-dispatch where necessary? > (We do this in the AST a lot, but the AST is special, because there we want > to implement a lot of different algorithms that rely on the node type, while > I don't see how that applies here) Generally the clients will have to somehow distinguish between the types of results that are produced by rules to figure out what to do (e.g. AtomicChanges -> apply, SymbolOccurrences -> ask user, Continuation -> look for more ASTs). So far I've thought that the LLVM based dynamic casts will work well for this, e.g. ``` if (auto *Action = dyn_cast()) { Expected> Changes = Action->createSourceReplacements(); applyChanges(Changes); } else if (...) { ... } else (...) { ... } ``` But you're probably right, there might be a better way to do this rather than the tag based approach. Something like a consumer class that clients can implement that provides consumer functions that take in the specific results. I reckon a single consumer will actually work better in the long-run when we might Continuations that both return changes in the first TU and information for searches in other TUs. I'll see if I can get a patch out that removes this tag and uses the consumer approach. Repository: rL LLVM https://reviews.llvm.org/D36075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37188: [clang-tools-extra] [cmake] Support running extra clang tool tests without static analyzer
alexfh added a comment. IIUC, these are the only clang-tidy tests that need static analyzer: clang/tools/extra/test/clang-tidy/nolint.cpp clang/tools/extra/test/clang-tidy/static-analyzer.cpp clang/tools/extra/test/clang-tidy/static-analyzer-config.cpp clang/tools/extra/test/clang-tidy/temporaries.cpp as well as the mpi module and its tests: clang/tools/extra/clang-tidy/mpi/* clang/tools/extra/test/clang-tidy/mpi-buffer-deref.cpp clang/tools/extra/test/clang-tidy/mpi-type-mismatch.cpp Repository: rL LLVM https://reviews.llvm.org/D37188 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36075: [refactor] Initial support for refactoring action rules
klimek added inline comments. Comment at: cfe/trunk/include/clang/Tooling/Refactoring/RefactoringActionRule.h:23-37 +/// A common refactoring action rule interface. +class RefactoringActionRule { +public: + enum RuleKind { SourceChangeRefactoringRuleKind }; + + RuleKind getRuleKind() const { return Kind; } + arphaman wrote: > klimek wrote: > > Sorry for being late, was out on vacation. > > Generally, why do we need this tag-based abstraction here instead of using > > the more typical OO double-dispatch where necessary? > > (We do this in the AST a lot, but the AST is special, because there we want > > to implement a lot of different algorithms that rely on the node type, > > while I don't see how that applies here) > Generally the clients will have to somehow distinguish between the types of > results that are produced by rules to figure out what to do (e.g. > AtomicChanges -> apply, SymbolOccurrences -> ask user, Continuation -> look > for more ASTs). So far I've thought that the LLVM based dynamic casts will > work well for this, e.g. > > ``` > if (auto *Action = dyn_cast()) { > Expected> Changes = > Action->createSourceReplacements(); > applyChanges(Changes); > } else if (...) { > ... > } else (...) { >... > } > ``` > > But you're probably right, there might be a better way to do this rather than > the tag based approach. Something like a consumer class that clients can > implement that provides consumer functions that take in the specific results. > I reckon a single consumer will actually work better in the long-run when we > might Continuations that both return changes in the first TU and information > for searches in other TUs. I'll see if I can get a patch out that removes > this tag and uses the consumer approach. Cool, looking forward to it :) Repository: rL LLVM https://reviews.llvm.org/D36075 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37142: clang-format: [JS] simplify template string wrapping.
mprobst added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1139 + + // On lines containing template strings, propagate NoLineBreak even for dict + // and array literals. This is to force wrapping an initial function call if djasper wrote: > mprobst wrote: > > djasper wrote: > > > This is not the right way to implement this: > > > > > > - This is a static computation that we could do ahead of time. Doing it > > > inside the combinatorial exploration of solutions is a waste. > > > - You are doing this always, even in code that doesn't have template > > > strings or isn't even JavaScript. > > > - This can lead to unexpected behavior if the template string is in a > > > completely unrelated part of the statement. E.g. > > > > > > > > > someFunction(`test`, { ... }); > > > > > > will be formatted differently from > > > > > > someFunction('test', { ... }); > > Ack. Do you have suggestions? > > > > I could introduce a `bool ParenState::NoLineBreakMustPropagate;` that > > controls this behaviour. > That, too, would create a significant memory and runtime overhead to everyone > just to fix this rather minor formatting detail. > If we were to set MatchingParen correctly to match up "${" to "}", we could > measure the respective length and at a very early state (mustBreak()) decide > that we need a line break between "${" and the corresponding "}". Now setting > that correctly might be hard. Maybe it is ok, to linearly scan in that case > as we would only do that if we actually find a template string ending in "${" > and the distance to the "}" is usually short. I cannot decide whether I must break in `mustBreak` - I need to just force wrapping after `${` if I need to wrap somewhere below, but if the code fits on the line, I don't want to wrap. So when I'm visiting `${` I cannot decide that, and when I'm at the object literal expression, the decision for `${` has already been made - right? Or am I missing something? https://reviews.llvm.org/D37142 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37188: [clang-tools-extra] [cmake] Support running extra clang tool tests without static analyzer
mgorny added a comment. I might be missing something but currently clang-tidy is not built at all when static analyzer is disabled. Repository: rL LLVM https://reviews.llvm.org/D37188 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37203: [Bash-autocompletion] Follow up patch for D36782
ruiu added a comment. Technically, this patch fixes your issue, but I don't think this is a good way of fixing it because `parse()` function now has an internal state which is not obvious to the user of the function. Can you review https://reviews.llvm.org/D37217? This is an alternative fix that doesn't use a static local variables. https://reviews.llvm.org/D37203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33852: Enable __declspec(selectany) on linux
Prazek added a comment. In https://reviews.llvm.org/D33852#853982, @aaron.ballman wrote: > In https://reviews.llvm.org/D33852#853410, @Prazek wrote: > > > Sorry for so late fixes, but it would be good to put it in 5.0 > > > I do not think this should be in 5.0, as I believe we're only accepting > regression fixes at this point. This is a regression. __declspec(selectany) works completely fine on linux with 4.0. Without it clang-5.0 will be useless for any major windows project compiled on linux. https://reviews.llvm.org/D33852 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35133: [cmake] Use new GetSVN.cmake SOURCE_DIRS parameter
beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. Since the dependent patch landed, this LGTM too. https://reviews.llvm.org/D35133 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36802: AMDGPU: Cleanup most of the macros
b-sumner added inline comments. Comment at: lib/Basic/Targets/AMDGPU.cpp:362 +Builder.defineMacro(Twine("__") + Twine(GPUName)); +Builder.defineMacro(Twine("__") + Twine(GPUName) + Twine("__")); + } At the meeting we discussed defining every alias of the given GPU, rather than only the mcpu name. Were we going to proceed with that? https://reviews.llvm.org/D36802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33852: Enable __declspec(selectany) on linux
aaron.ballman added a comment. In https://reviews.llvm.org/D33852#854176, @Prazek wrote: > In https://reviews.llvm.org/D33852#853982, @aaron.ballman wrote: > > > In https://reviews.llvm.org/D33852#853410, @Prazek wrote: > > > > > Sorry for so late fixes, but it would be good to put it in 5.0 > > > > > > I do not think this should be in 5.0, as I believe we're only accepting > > regression fixes at this point. > > > This is a regression. __declspec(selectany) works completely fine on linux > with 4.0. Without it clang-5.0 will be useless for any major windows project > compiled on linux. > > edit: > here is a regression: https://reviews.llvm.org/D32083 This is also adding new functionality that has had zero testing because it removes *all* target-specific checking for the attribute. Under the previous functionality (changed in https://reviews.llvm.org/D32083), this still required some mention of microsoft something (it went from requiring microsoft extensions to be enabled to instead require a Windows target) -- that's been entirely removed from this patch so now you can use this attribute for all target architectures, so it's not purely fixing a regression. Given how late we are in the release cycle, I am uncomfortable with this going in to 5.0, but I'd have no problems letting it bake for a bit and putting it into 5.1 (or 5.0.1, however we're naming bug releases these days). https://reviews.llvm.org/D33852 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types
ddcc added inline comments. Comment at: lib/StaticAnalyzer/Core/SValBuilder.cpp:364 if (symLHS && symRHS && - (symLHS->computeComplexity() + symRHS->computeComplexity()) < MaxComp) + (symLHS->computeComplexity() + symRHS->computeComplexity()) < MaxComp) return makeNonLoc(symLHS, Op, symRHS, ResultTy); zaks.anna wrote: > ddcc wrote: > > zaks.anna wrote: > > > As a follow up to the previous version of this patch, I do not think we > > > should set the default complexity limit to 1. > > > > > > What is the relation between this limit and the limit in > > > VisitNonLocSymbolVal? If they are related, would it be worthwhile turning > > > these into an analyzer option? > > To clarify, the current version of this patch does not modify the `MaxComp` > > parameter. > > > > My understanding is that `MaxComp` is the upper bound for building a new > > `NonLoc` symbol from two children, based on the sum of the number of child > > symbols (complexity) across both children. > > > > In contrast, the limit in `VisitNonLocSymbolVal` (@NoQ, correct me if I'm > > wrong), is the upper bound for recursively evaluating and inlining a > > `NonLoc` symbol, triggered from `simplifySVal` by `evalBinOpNN`. Note that > > these two latter functions indirectly call each other recursively (through > > `evalBinOp`), causing the previous runtime blowup. Furthermore, each call > > to `computeComplexity`will re-iterate through all child symbols of the > > current symbol, but only the first complexity check at the root symbol is > > actually necessary, because by definition the complexity of a child symbol > > at each recursive call is monotonically decreasing. > > > > I think it'd be useful to allow both to be configurable, but I don't see a > > direct relationship between the two. > > To clarify, the current version of this patch does not modify the MaxComp > > parameter. > > I know. Also, currently, it is only used in the unsupported taint tracking > mode and only for tainted symbols. I would like a different smaller default > for all expressions. Ok. I can make both configurable as part of this patch, with a new default of 10 for `VisitNonLocSymbolVal`. But I've never used the taint tracking mode, so I don't know what would be a reasonable default for `MaxComp`. https://reviews.llvm.org/D35450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36806: Switch to cantFail(), since it does the same assertion.
lhames added a comment. In https://reviews.llvm.org/D36806#848246, @hintonda wrote: > It's just too bad llvm::cantFail() doesn't take an optional string. > But the code is cleaner, so I won't comment further. That's not a bad idea actually. Let me add an optional error message to cantFail for you. https://reviews.llvm.org/D36806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37122: Change Diagnostic Category size error from runtime to compiletime
rnk added inline comments. Comment at: include/clang/Basic/AllDiagnostics.h:37 +#define STRINGIFY_NAME(NAME) #NAME +#define VALIDATE_DIAG_SIZE(NAME) \ + static_assert( \ I'd prefer it if we sank this into a .cpp file so that when it fails, it doesn't create a waterfall of colorful static_assert errors. :) https://reviews.llvm.org/D37122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type
mclow.lists added inline comments. Comment at: test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp:1 +//===--===// +// Is this file in the right place? If it's supposed to test functionality that is required by the standard, it should go in `test/std` somewhere. If it's supposed to test a libc++ extension, then it should go into `test/libcxx`. Repository: rL LLVM https://reviews.llvm.org/D37104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type
mclow.lists added inline comments. Comment at: test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp:1 +//===--===// +// mclow.lists wrote: > Is this file in the right place? > If it's supposed to test functionality that is required by the standard, it > should go in `test/std` somewhere. > If it's supposed to test a libc++ extension, then it should go into > `test/libcxx`. Never mind - I missed the "Non-standard extension" bit. Repository: rL LLVM https://reviews.llvm.org/D37104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26796: [Driver] Use arch type to find compiler-rt libraries (on Linux)
eugenis added a comment. In https://reviews.llvm.org/D26796#853446, @mgorny wrote: > The problems: > > 1. > http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/2140/steps/annotate/logs/stdio > -- purely this change, This one still builds i686 libraries. Maybe it needs a clean cmake? Try "clobber" "1" in build properties. [171/178] Linking CXX shared library /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm_build64/lib/clang/6.0.0/lib/linux/libclang_rt.asan-i686-android.so [172/178] Linking CXX static library /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm_build64/lib/clang/6.0.0/lib/linux/libclang_rt.asan-i686-android.a > 2. > http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-android/builds/2146/steps/run%20instrumented%20asan%20tests%20%5Bi686%2Ffugu-userdebug%2FN2G48C%5D/logs/stdio > -- after https://reviews.llvm.org/D26764. This probably needs an update of zorg/buildbot/builders/sanitizers/buildbot_android.sh in the zorg repo. Repository: rL LLVM https://reviews.llvm.org/D26796 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r311898 - Revert r311857 "Emit static constexpr member as available_externally definition"
Author: hans Date: Mon Aug 28 10:53:00 2017 New Revision: 311898 URL: http://llvm.org/viewvc/llvm-project?rev=311898&view=rev Log: Revert r311857 "Emit static constexpr member as available_externally definition" It caused PR759744. > Emit static constexpr member as available_externally definition > > By exposing the constant initializer, the optimizer can fold many > of these constructs. > > Differential Revision: https://reviews.llvm.org/D34992 Removed: cfe/trunk/test/CodeGenCXX/cxx11-extern-constexpr.cpp Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=311898&r1=311897&r2=311898&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Mon Aug 28 10:53:00 2017 @@ -2437,28 +2437,6 @@ CodeGenModule::GetOrCreateLLVMGlobal(Str D->getType().isConstant(Context) && isExternallyVisible(D->getLinkageAndVisibility().getLinkage())) GV->setSection(".cp.rodata"); - -// Check if we a have a const declaration with an initializer, we may be -// able to emit it as available_externally to expose it's value to the -// optimizer. -if (Context.getLangOpts().CPlusPlus && GV->hasExternalLinkage() && -D->getType().isConstQualified() && !GV->hasInitializer() && -!D->hasDefinition() && D->hasInit() && !D->hasAttr()) { - const auto *Record = - Context.getBaseElementType(D->getType())->getAsCXXRecordDecl(); - bool HasMutableFields = Record && Record->hasMutableFields(); - if (!HasMutableFields) { -const VarDecl *InitDecl; -const Expr *InitExpr = D->getAnyInitializer(InitDecl); -if (InitExpr) { - GV->setConstant(true); - GV->setLinkage(llvm::GlobalValue::AvailableExternallyLinkage); - ConstantEmitter emitter(*this); - GV->setInitializer(emitter.tryEmitForInitializer(*InitDecl)); - emitter.finalize(GV); -} - } -} } auto ExpectedAS = Removed: cfe/trunk/test/CodeGenCXX/cxx11-extern-constexpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx11-extern-constexpr.cpp?rev=311897&view=auto == --- cfe/trunk/test/CodeGenCXX/cxx11-extern-constexpr.cpp (original) +++ cfe/trunk/test/CodeGenCXX/cxx11-extern-constexpr.cpp (removed) @@ -1,55 +0,0 @@ -// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK --check-prefix=CXX11 -// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK --check-prefix=CXX17 - -struct A { - static const int Foo = 123; -}; -// CHECK: @_ZN1A3FooE = constant i32 123, align 4 -const int *p = &A::Foo; // emit available_externally -const int A::Foo; // convert to full definition - -struct Bar { - int b; -}; - -struct MutableBar { - mutable int b; -}; - -struct Foo { - // CXX11: @_ZN3Foo21ConstexprStaticMemberE = available_externally constant i32 42, - // CXX17: @_ZN3Foo21ConstexprStaticMemberE = linkonce_odr constant i32 42, - static constexpr int ConstexprStaticMember = 42; - // CHECK: @_ZN3Foo17ConstStaticMemberE = available_externally constant i32 43, - static const int ConstStaticMember = 43; - - // CXX11: @_ZN3Foo23ConstStaticStructMemberE = available_externally constant %struct.Bar { i32 44 }, - // CXX17: @_ZN3Foo23ConstStaticStructMemberE = linkonce_odr constant %struct.Bar { i32 44 }, - static constexpr Bar ConstStaticStructMember = {44}; - - // CXX11: @_ZN3Foo34ConstexprStaticMutableStructMemberE = external global %struct.MutableBar, - // CXX17: @_ZN3Foo34ConstexprStaticMutableStructMemberE = linkonce_odr global %struct.MutableBar { i32 45 }, - static constexpr MutableBar ConstexprStaticMutableStructMember = {45}; -}; -// CHECK: @_ZL15ConstStaticexpr = internal constant i32 46, -static constexpr int ConstStaticexpr = 46; -// CHECK: @_ZL9ConstExpr = internal constant i32 46, align 4 -static const int ConstExpr = 46; - -// CHECK: @_ZL21ConstexprStaticStruct = internal constant %struct.Bar { i32 47 }, -static constexpr Bar ConstexprStaticStruct = {47}; - -// CHECK: @_ZL28ConstexprStaticMutableStruct = internal global %struct.MutableBar { i32 48 }, -static constexpr MutableBar ConstexprStaticMutableStruct = {48}; - -void use(const int &); -void foo() { - use(Foo::ConstexprStaticMember); - use(Foo::ConstStaticMember); - use(Foo::ConstStaticStructMember.b); - use(Foo::ConstexprStaticMutableStructMember.b); - use(ConstStaticexpr); - use(ConstExpr); - use(ConstexprStaticStruct.b); - use(ConstexprStaticMutableStruct.b); -} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l
[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type
rsmith added inline comments. Comment at: test/libcxx/utilities/function.objects/func.require/incomplete_return_type.pass.cpp:17-19 +struct IncompleteReturnType { + std::function fn; +}; Would it make sense to also check that this struct is copy-constructible? As-is, whether this test tests anything is entirely dependent on what amount to implementation details of the constructor. Repository: rL LLVM https://reviews.llvm.org/D37104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r311899 - Fix inaccurate comment about -fdelayed-template-parsing and MSVC
Author: rnk Date: Mon Aug 28 10:59:24 2017 New Revision: 311899 URL: http://llvm.org/viewvc/llvm-project?rev=311899&view=rev Log: Fix inaccurate comment about -fdelayed-template-parsing and MSVC Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=311899&r1=311898&r2=311899&view=diff == --- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original) +++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Mon Aug 28 10:59:24 2017 @@ -3846,8 +3846,10 @@ void Clang::ConstructJob(Compilation &C, !IsWindowsMSVC || IsMSVC2015Compatible)) CmdArgs.push_back("-fno-threadsafe-statics"); - // -fno-delayed-template-parsing is default, except for Windows where MSVC STL - // needs it. + // -fno-delayed-template-parsing is default, except when targetting MSVC. + // Many old Windows SDK versions require this to parse. + // FIXME: MSVC introduced /Zc:twoPhase- to disable this behavior in their + // compiler. We should be able to disable this by default at some point. if (Args.hasFlag(options::OPT_fdelayed_template_parsing, options::OPT_fno_delayed_template_parsing, IsWindowsMSVC)) CmdArgs.push_back("-fdelayed-template-parsing"); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37122: Change Diagnostic Category size error from runtime to compiletime
erichkeane updated this revision to Diff 112916. erichkeane marked an inline comment as done. erichkeane added a comment. Moved static assert into a .cpp file, DiagnosticIDs.cpp seems like the one that makes the most sense. https://reviews.llvm.org/D37122 Files: include/clang/Basic/DiagnosticIDs.h lib/Basic/DiagnosticIDs.cpp Index: lib/Basic/DiagnosticIDs.cpp === --- lib/Basic/DiagnosticIDs.cpp +++ lib/Basic/DiagnosticIDs.cpp @@ -68,6 +68,29 @@ } }; +#define STRINGIFY_NAME(NAME) #NAME +#define VALIDATE_DIAG_SIZE(NAME) \ + static_assert( \ + static_cast(diag::NUM_BUILTIN_##NAME##_DIAGNOSTICS) < \ + static_cast(diag::DIAG_START_##NAME) + \ + static_cast(diag::DIAG_SIZE_##NAME), \ + STRINGIFY_NAME( \ + DIAG_SIZE_##NAME) " is insufficient to contain all " \ +"diagnostics, it may need to be made larger in " \ +"DiagnosticIDs.h."); +VALIDATE_DIAG_SIZE(COMMON) +VALIDATE_DIAG_SIZE(DRIVER) +VALIDATE_DIAG_SIZE(FRONTEND) +VALIDATE_DIAG_SIZE(SERIALIZATION) +VALIDATE_DIAG_SIZE(LEX) +VALIDATE_DIAG_SIZE(PARSE) +VALIDATE_DIAG_SIZE(AST) +VALIDATE_DIAG_SIZE(COMMENT) +VALIDATE_DIAG_SIZE(SEMA) +VALIDATE_DIAG_SIZE(ANALYSIS) +#undef VALIDATE_DIAG_SIZE +#undef STRINGIFY_NAME + } // namespace anonymous static const StaticDiagInfoRec StaticDiagInfo[] = { @@ -96,18 +119,6 @@ /// GetDiagInfo - Return the StaticDiagInfoRec entry for the specified DiagID, /// or null if the ID is invalid. static const StaticDiagInfoRec *GetDiagInfo(unsigned DiagID) { - // If assertions are enabled, verify that the StaticDiagInfo array is sorted. -#ifndef NDEBUG - static bool IsFirst = true; // So the check is only performed on first call. - if (IsFirst) { -assert(std::is_sorted(std::begin(StaticDiagInfo), - std::end(StaticDiagInfo)) && - "Diag ID conflict, the enums at the start of clang::diag (in " - "DiagnosticIDs.h) probably need to be increased"); -IsFirst = false; - } -#endif - // Out of bounds diag. Can't be in the table. using namespace diag; if (DiagID >= DIAG_UPPER_LIMIT || DiagID <= DIAG_START_COMMON) Index: include/clang/Basic/DiagnosticIDs.h === --- include/clang/Basic/DiagnosticIDs.h +++ include/clang/Basic/DiagnosticIDs.h @@ -26,19 +26,32 @@ // Import the diagnostic enums themselves. namespace diag { +// Size of each of the diagnostic categories. +enum { + DIAG_SIZE_COMMON= 300, + DIAG_SIZE_DRIVER= 200, + DIAG_SIZE_FRONTEND = 100, + DIAG_SIZE_SERIALIZATION = 120, + DIAG_SIZE_LEX = 400, + DIAG_SIZE_PARSE = 500, + DIAG_SIZE_AST = 110, + DIAG_SIZE_COMMENT = 100, + DIAG_SIZE_SEMA = 3500, + DIAG_SIZE_ANALYSIS = 100 +}; // Start position for diagnostics. enum { - DIAG_START_COMMON= 0, - DIAG_START_DRIVER= DIAG_START_COMMON + 300, - DIAG_START_FRONTEND = DIAG_START_DRIVER + 200, - DIAG_START_SERIALIZATION = DIAG_START_FRONTEND+ 100, - DIAG_START_LEX = DIAG_START_SERIALIZATION + 120, - DIAG_START_PARSE = DIAG_START_LEX + 400, - DIAG_START_AST = DIAG_START_PARSE + 500, - DIAG_START_COMMENT = DIAG_START_AST + 110, - DIAG_START_SEMA = DIAG_START_COMMENT + 100, - DIAG_START_ANALYSIS = DIAG_START_SEMA+ 3500, - DIAG_UPPER_LIMIT = DIAG_START_ANALYSIS+ 100 + DIAG_START_COMMON= 0, + DIAG_START_DRIVER= DIAG_START_COMMON+ DIAG_SIZE_COMMON, + DIAG_START_FRONTEND = DIAG_START_DRIVER+ DIAG_SIZE_DRIVER, + DIAG_START_SERIALIZATION = DIAG_START_FRONTEND + DIAG_SIZE_FRONTEND, + DIAG_START_LEX = DIAG_START_SERIALIZATION + DIAG_SIZE_SERIALIZATION, + DIAG_START_PARSE = DIAG_START_LEX + DIAG_SIZE_LEX, + DIAG_START_AST = DIAG_START_PARSE + DIAG_SIZE_PARSE, + DIAG_START_COMMENT = DIAG_START_AST + DIAG_SIZE_AST, + DIAG_START_SEMA = DIAG_START_COMMENT + DIAG_SIZE_COMMENT, + DIAG_START_ANALYSIS = DIAG_START_SEMA + DIAG_SIZE_SEMA, + DIAG_UPPER_LIMIT = DIAG_START_ANALYSIS + DIAG_SIZE_ANALYSIS }; class CustomDiagInfo; ___ cfe-commits mailing list cfe-
[PATCH] D36915: [Sema] Diagnose local variables and parameters captured by lambda and block expressions in a default argument
ahatanak added a comment. In https://reviews.llvm.org/D36915#849622, @faisalv wrote: > I don't think this approach is entirely correct for at least the following > reasons: > > 1. in the lambda case the machinery that diagnoses capture failures should be > the machinery erroring on the lambda (when the parameter is odr-used) Does this mean that it is OK to have a parameter or local variable appear in a potentially-evaluated expression as long as it is not odr-used? I think this is slightly different from the standard, which says a parameter or local variable cannot appear as a potentially-evaluated expression in a default argument. For example: void foo2(int); void func() { const int a = 1; void foo1(S s = [](){ foo2(a); }); // "a" is not in an unevaluated context and is not odr-used. } I think I need clarification as it will affect how or where we detect this error. https://reviews.llvm.org/D36915 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer
morehouse updated this revision to Diff 112923. morehouse added a comment. Herald added a subscriber: kubamracek. - Add weak definition of __sancov_lowest_stack to runtime. https://reviews.llvm.org/D37156 Files: clang/lib/Driver/SanitizerArgs.cpp compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cc compiler-rt/test/fuzzer/deep-recursion.test llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll === --- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll +++ llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll @@ -1,9 +1,9 @@ ; This check verifies that stack depth instrumentation works correctly. ; RUN: opt < %s -sancov -sanitizer-coverage-level=1 \ -; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s --enable-var-scope +; RUN: -sanitizer-coverage-stack-depth -S | FileCheck %s ; RUN: opt < %s -sancov -sanitizer-coverage-level=3 \ ; RUN: -sanitizer-coverage-stack-depth -sanitizer-coverage-trace-pc-guard \ -; RUN: -S | FileCheck %s --enable-var-scope +; RUN: -S | FileCheck %s target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-unknown-linux-gnu" @@ -14,13 +14,8 @@ define i32 @foo() { entry: ; CHECK-LABEL: define i32 @foo -; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0) -; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType:i[0-9]+]] -; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack -; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]] -; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label -; CHECK: :[[ifLabel]]: -; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack +; CHECK-NOT: call i8* @llvm.frameaddress(i32 0) +; CHECK-NOT: @__sancov_lowest_stack ; CHECK: ret i32 7 ret i32 7 @@ -30,12 +25,12 @@ entry: ; CHECK-LABEL: define i32 @bar ; CHECK: [[framePtr:%[^ \t]+]] = call i8* @llvm.frameaddress(i32 0) -; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[$intType]] -; CHECK: [[lowest:%[^ \t]+]] = load [[$intType]], [[$intType]]* @__sancov_lowest_stack -; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[$intType]] [[frameInt]], [[lowest]] +; CHECK: [[frameInt:%[^ \t]+]] = ptrtoint i8* [[framePtr]] to [[intType:i[0-9]+]] +; CHECK: [[lowest:%[^ \t]+]] = load [[intType]], [[intType]]* @__sancov_lowest_stack +; CHECK: [[cmp:%[^ \t]+]] = icmp ult [[intType]] [[frameInt]], [[lowest]] ; CHECK: br i1 [[cmp]], label %[[ifLabel:[^ \t]+]], label ; CHECK: :[[ifLabel]]: -; CHECK: store [[$intType]] [[frameInt]], [[$intType]]* @__sancov_lowest_stack +; CHECK: store [[intType]] [[frameInt]], [[intType]]* @__sancov_lowest_stack ; CHECK: %call = call i32 @foo() ; CHECK: ret i32 %call Index: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp === --- llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp +++ llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp @@ -25,6 +25,7 @@ #include "llvm/IR/GlobalVariable.h" #include "llvm/IR/IRBuilder.h" #include "llvm/IR/InlineAsm.h" +#include "llvm/IR/IntrinsicInst.h" #include "llvm/IR/Intrinsics.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/MDBuilder.h" @@ -200,13 +201,15 @@ ArrayRef GepTraceTargets); void InjectTraceForSwitch(Function &F, ArrayRef SwitchTraceTargets); - bool InjectCoverage(Function &F, ArrayRef AllBlocks); + bool InjectCoverage(Function &F, ArrayRef AllBlocks, + bool IsLeafFunc = true); GlobalVariable *CreateFunctionLocalArrayInSection(size_t NumElements, Function &F, Type *Ty, const char *Section); void CreateFunctionLocalArrays(Function &F, ArrayRef AllBlocks); void CreatePCArray(Function &F, ArrayRef AllBlocks); - void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx); + void InjectCoverageAtBlock(Function &F, BasicBlock &BB, size_t Idx, + bool IsLeafFunc = true); Function *CreateInitCallsForSections(Module &M, const char *InitFunctionName, Type *Ty, const char *Section); std::pair @@ -491,6 +494,7 @@ &getAnalysis(F).getDomTree(); const PostDominatorTree *PDT = &getAnalysis(F).getPostDomTree(); + bool IsLeafFunc = true; for (auto &BB : F) { if (shouldInstrumentBlock(F, &BB, DT, PDT, Options)) @@ -515,10 +519,14 @@ if (Options.TraceGep) if (GetElementPtrInst *GEP = dyn_cast(&Inst)) GepTraceTargets.push_back(GEP); - } + if (Options.StackDepth) +if (isa(Inst) || +(isa(Inst) && !is
[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer
kcc added a reviewer: george.karpenkov. kcc added a comment. +George, in case he knows about __attribute__((tls_model("initial-exec"))) on Mac Comment at: compiler-rt/lib/sanitizer_common/sanitizer_coverage_libcdep_new.cc:218 +SANITIZER_INTERFACE_ATTRIBUTE SANITIZER_WEAK_ATTRIBUTE +__attribute__((tls_model("initial-exec"))) +thread_local uintptr_t __sancov_lowest_stack; I wonder if this going to work on Mac. https://reviews.llvm.org/D37156 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35110: [Analyzer] Constraint Manager Negates Difference
zaks.anna added a comment. Is this blocked on the same reasons as what was raised in https://reviews.llvm.org/D35109? https://reviews.llvm.org/D35110 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer
george.karpenkov added a comment. I don't, but I can check whether tests pass. https://reviews.llvm.org/D37156 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35450: [analyzer] Support generating and reasoning over more symbolic constraint types
zaks.anna added a comment. > But I've never used the taint tracking mode, so I don't know what would be a > reasonable default for MaxComp. that one is very experimental anyway. I'd just keep the functional changes to tain out of this patch and use the current default that taint uses. https://reviews.llvm.org/D35450 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37005: [clang-diff] Initial implementation of patching
johannes updated this revision to Diff 112926. johannes edited the summary of this revision. johannes added a comment. split to ASTDiff/ASTPatch https://reviews.llvm.org/D37005 Files: include/clang/Tooling/ASTDiff/ASTDiff.h include/clang/Tooling/ASTDiff/ASTPatch.h lib/Tooling/ASTDiff/ASTDiff.cpp lib/Tooling/ASTDiff/ASTPatch.cpp lib/Tooling/ASTDiff/CMakeLists.txt test/Tooling/clang-diff-patch.test tools/clang-diff/CMakeLists.txt tools/clang-diff/ClangDiff.cpp unittests/Tooling/ASTDiffTest.cpp unittests/Tooling/CMakeLists.txt Index: unittests/Tooling/CMakeLists.txt === --- unittests/Tooling/CMakeLists.txt +++ unittests/Tooling/CMakeLists.txt @@ -11,6 +11,7 @@ endif() add_clang_unittest(ToolingTests + ASTDiffTest.cpp ASTSelectionTest.cpp CastExprTest.cpp CommentHandlerTest.cpp @@ -43,4 +44,5 @@ clangTooling clangToolingCore clangToolingRefactor + clangToolingASTDiff ) Index: unittests/Tooling/ASTDiffTest.cpp === --- /dev/null +++ unittests/Tooling/ASTDiffTest.cpp @@ -0,0 +1,86 @@ +//===- unittest/Tooling/ASTDiffTest.cpp ---===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "clang/Tooling/ASTDiff/ASTDiff.h" +#include "clang/Tooling/ASTDiff/ASTPatch.h" +#include "clang/Tooling/Tooling.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace tooling; + +static std::string patchResult(std::array Codes) { + diff::SyntaxTree Trees[3]; + std::unique_ptr ASTs[3]; + std::vector Args = {}; + for (int I = 0; I < 3; I++) { +ASTs[I] = buildASTFromCode(Codes[I]); +if (!ASTs[I]) { + llvm::errs() << "Failed to build AST from code:\n" << Codes[I] << "\n"; + return ""; +} +Trees[I] = diff::SyntaxTree(*ASTs[I]); + } + + diff::ComparisonOptions Options; + std::string TargetDstCode; + llvm::raw_string_ostream OS(TargetDstCode); + if (!diff::patch(/*ModelSrc=*/Trees[0], /*ModelDst=*/Trees[1], + /*TargetSrc=*/Trees[2], Options, OS)) +return ""; + return OS.str(); +} + +// abstract the EXPECT_EQ call so that the code snippets align properly +// use macros for this to make test failures have proper line numbers +#define PATCH(Preamble, ModelSrc, ModelDst, Target, Expected) \ + EXPECT_EQ(patchResult({{std::string(Preamble) + ModelSrc,\ + std::string(Preamble) + ModelDst,\ + std::string(Preamble) + Target}}), \ +std::string(Preamble) + Expected) + +TEST(ASTDiff, TestDeleteArguments) { + PATCH(R"(void printf(const char *, ...);)", +R"(void foo(int x) { printf("%d", x, x); })", +R"(void foo(int x) { printf("%d", x); })", +R"(void foo(int x) { printf("different string %d", x, x); })", +R"(void foo(int x) { printf("different string %d", x); })"); + + PATCH(R"(void foo(...);)", +R"(void test1() { foo ( 1 + 1); })", +R"(void test1() { foo ( ); })", +R"(void test2() { foo ( 1 + 1 ); })", +R"(void test2() { foo ( ); })"); + + PATCH(R"(void foo(...);)", +R"(void test1() { foo (1, 2 + 2); })", +R"(void test1() { foo (2 + 2); })", +R"(void test2() { foo (/*L*/ 0 /*R*/ , 2 + 2); })", +R"(void test2() { foo (/*L*/ 2 + 2); })"); + + PATCH(R"(void foo(...);)", +R"(void test1() { foo (1, 2); })", +R"(void test1() { foo (1); })", +R"(void test2() { foo (0, /*L*/ 0 /*R*/); })", +R"(void test2() { foo (0 /*R*/); })"); +} + +TEST(ASTDiff, TestDeleteDecls) { + PATCH(R"()", +R"()", +R"()", +R"()", +R"()"); + + PATCH(R"()", +R"(void foo(){})", +R"()", +R"(int x; void foo() {;;} int y;)", +R"(int x; int y;)"); +} Index: tools/clang-diff/ClangDiff.cpp === --- tools/clang-diff/ClangDiff.cpp +++ tools/clang-diff/ClangDiff.cpp @@ -13,6 +13,7 @@ //===--===// #include "clang/Tooling/ASTDiff/ASTDiff.h" +#include "clang/Tooling/ASTDiff/ASTPatch.h" #include "clang/Tooling/CommonOptionsParser.h" #include "clang/Tooling/Tooling.h" #include "llvm/Support/CommandLine.h" @@ -42,6 +43,12 @@ cl::desc("Output a side-by-side diff in HTML."), cl::init(false), cl::cat(ClangDiffCategory)); +static cl::opt +Patch("patch", + cl::desc("Try to apply the edit actions between the two input " + "files to the specifi
[PATCH] D37038: Replace temp MD nodes with unique/distinct before cloning
aprantl added inline comments. Comment at: lib/CodeGen/CGVTables.cpp:157 + if (DebugInfo) +DebugInfo->replaceTemporaryNodes(); + probinson wrote: > aprantl wrote: > > Have you looked at what it would take to only finalize the nodes reachable > > via the function? > > Otherwise — have you audited that this is always safe? > I do not know how to find nodes reachable from a particular function, either > before or after they are finalized. > > GenerateVarArgsThunk is called after we do EmitGlobalFunctionDefinition on > the function we are about to clone. The ReplaceMap holds replaceable forward > type declarations, I think? So I can imagine that codegen for a subsequent > function might need to create a complete type, even one that is reachable > from the function we are about to clone. > > Of course I find the whole metadata memory management scheme pretty opaque, > but this is my best guess about the situation. Alternatively, would it be possible to instruct CloneFunction to not clone any temporary MDNodes via one of the flags that are passed to ValueMapper? https://reviews.llvm.org/D37038 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37122: Change Diagnostic Category size error from runtime to compiletime
This revision was automatically updated to reflect the committed changes. Closed by commit rL311905: Change Diagnostic Category size error from runtime to compiletime (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D37122?vs=112916&id=112931#toc Repository: rL LLVM https://reviews.llvm.org/D37122 Files: cfe/trunk/include/clang/Basic/DiagnosticIDs.h cfe/trunk/lib/Basic/DiagnosticIDs.cpp Index: cfe/trunk/lib/Basic/DiagnosticIDs.cpp === --- cfe/trunk/lib/Basic/DiagnosticIDs.cpp +++ cfe/trunk/lib/Basic/DiagnosticIDs.cpp @@ -68,6 +68,29 @@ } }; +#define STRINGIFY_NAME(NAME) #NAME +#define VALIDATE_DIAG_SIZE(NAME) \ + static_assert( \ + static_cast(diag::NUM_BUILTIN_##NAME##_DIAGNOSTICS) < \ + static_cast(diag::DIAG_START_##NAME) + \ + static_cast(diag::DIAG_SIZE_##NAME), \ + STRINGIFY_NAME( \ + DIAG_SIZE_##NAME) " is insufficient to contain all " \ +"diagnostics, it may need to be made larger in " \ +"DiagnosticIDs.h."); +VALIDATE_DIAG_SIZE(COMMON) +VALIDATE_DIAG_SIZE(DRIVER) +VALIDATE_DIAG_SIZE(FRONTEND) +VALIDATE_DIAG_SIZE(SERIALIZATION) +VALIDATE_DIAG_SIZE(LEX) +VALIDATE_DIAG_SIZE(PARSE) +VALIDATE_DIAG_SIZE(AST) +VALIDATE_DIAG_SIZE(COMMENT) +VALIDATE_DIAG_SIZE(SEMA) +VALIDATE_DIAG_SIZE(ANALYSIS) +#undef VALIDATE_DIAG_SIZE +#undef STRINGIFY_NAME + } // namespace anonymous static const StaticDiagInfoRec StaticDiagInfo[] = { @@ -96,18 +119,6 @@ /// GetDiagInfo - Return the StaticDiagInfoRec entry for the specified DiagID, /// or null if the ID is invalid. static const StaticDiagInfoRec *GetDiagInfo(unsigned DiagID) { - // If assertions are enabled, verify that the StaticDiagInfo array is sorted. -#ifndef NDEBUG - static bool IsFirst = true; // So the check is only performed on first call. - if (IsFirst) { -assert(std::is_sorted(std::begin(StaticDiagInfo), - std::end(StaticDiagInfo)) && - "Diag ID conflict, the enums at the start of clang::diag (in " - "DiagnosticIDs.h) probably need to be increased"); -IsFirst = false; - } -#endif - // Out of bounds diag. Can't be in the table. using namespace diag; if (DiagID >= DIAG_UPPER_LIMIT || DiagID <= DIAG_START_COMMON) Index: cfe/trunk/include/clang/Basic/DiagnosticIDs.h === --- cfe/trunk/include/clang/Basic/DiagnosticIDs.h +++ cfe/trunk/include/clang/Basic/DiagnosticIDs.h @@ -26,19 +26,32 @@ // Import the diagnostic enums themselves. namespace diag { +// Size of each of the diagnostic categories. +enum { + DIAG_SIZE_COMMON= 300, + DIAG_SIZE_DRIVER= 200, + DIAG_SIZE_FRONTEND = 100, + DIAG_SIZE_SERIALIZATION = 120, + DIAG_SIZE_LEX = 400, + DIAG_SIZE_PARSE = 500, + DIAG_SIZE_AST = 110, + DIAG_SIZE_COMMENT = 100, + DIAG_SIZE_SEMA = 3500, + DIAG_SIZE_ANALYSIS = 100 +}; // Start position for diagnostics. enum { - DIAG_START_COMMON= 0, - DIAG_START_DRIVER= DIAG_START_COMMON + 300, - DIAG_START_FRONTEND = DIAG_START_DRIVER + 200, - DIAG_START_SERIALIZATION = DIAG_START_FRONTEND+ 100, - DIAG_START_LEX = DIAG_START_SERIALIZATION + 120, - DIAG_START_PARSE = DIAG_START_LEX + 400, - DIAG_START_AST = DIAG_START_PARSE + 500, - DIAG_START_COMMENT = DIAG_START_AST + 110, - DIAG_START_SEMA = DIAG_START_COMMENT + 100, - DIAG_START_ANALYSIS = DIAG_START_SEMA+ 3500, - DIAG_UPPER_LIMIT = DIAG_START_ANALYSIS+ 100 + DIAG_START_COMMON= 0, + DIAG_START_DRIVER= DIAG_START_COMMON+ DIAG_SIZE_COMMON, + DIAG_START_FRONTEND = DIAG_START_DRIVER+ DIAG_SIZE_DRIVER, + DIAG_START_SERIALIZATION = DIAG_START_FRONTEND + DIAG_SIZE_FRONTEND, + DIAG_START_LEX = DIAG_START_SERIALIZATION + DIAG_SIZE_SERIALIZATION, + DIAG_START_PARSE = DIAG_START_LEX + DIAG_SIZE_LEX, + DIAG_START_AST = DIAG_START_PARSE + DIAG_SIZE_PARSE, + DIAG_START_COMMENT = DIAG_START_AST + DIAG_SIZE_AST, + DIAG_START_SEMA = DIAG_START_COMMENT + DIAG_SIZE_COMMENT, + DIAG_START_ANALYSIS = DIAG_START_SEMA + DIAG_SIZE_SEMA, + DIAG_UPPER_LIMIT = DIAG_S
r311905 - Change Diagnostic Category size error from runtime to compiletime
Author: erichkeane Date: Mon Aug 28 11:53:17 2017 New Revision: 311905 URL: http://llvm.org/viewvc/llvm-project?rev=311905&view=rev Log: Change Diagnostic Category size error from runtime to compiletime Diagnostic Categories are fairly annoying, and are only enforced by a runtime-debug-only assert. This puts in a touch more work to get this all done at compile-time with static asserts Differential Revision: https://reviews.llvm.org/D37122 Modified: cfe/trunk/include/clang/Basic/DiagnosticIDs.h cfe/trunk/lib/Basic/DiagnosticIDs.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticIDs.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticIDs.h?rev=311905&r1=311904&r2=311905&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticIDs.h (original) +++ cfe/trunk/include/clang/Basic/DiagnosticIDs.h Mon Aug 28 11:53:17 2017 @@ -26,19 +26,32 @@ namespace clang { // Import the diagnostic enums themselves. namespace diag { +// Size of each of the diagnostic categories. +enum { + DIAG_SIZE_COMMON= 300, + DIAG_SIZE_DRIVER= 200, + DIAG_SIZE_FRONTEND = 100, + DIAG_SIZE_SERIALIZATION = 120, + DIAG_SIZE_LEX = 400, + DIAG_SIZE_PARSE = 500, + DIAG_SIZE_AST = 110, + DIAG_SIZE_COMMENT = 100, + DIAG_SIZE_SEMA = 3500, + DIAG_SIZE_ANALYSIS = 100 +}; // Start position for diagnostics. enum { - DIAG_START_COMMON= 0, - DIAG_START_DRIVER= DIAG_START_COMMON + 300, - DIAG_START_FRONTEND = DIAG_START_DRIVER + 200, - DIAG_START_SERIALIZATION = DIAG_START_FRONTEND+ 100, - DIAG_START_LEX = DIAG_START_SERIALIZATION + 120, - DIAG_START_PARSE = DIAG_START_LEX + 400, - DIAG_START_AST = DIAG_START_PARSE + 500, - DIAG_START_COMMENT = DIAG_START_AST + 110, - DIAG_START_SEMA = DIAG_START_COMMENT + 100, - DIAG_START_ANALYSIS = DIAG_START_SEMA+ 3500, - DIAG_UPPER_LIMIT = DIAG_START_ANALYSIS+ 100 + DIAG_START_COMMON= 0, + DIAG_START_DRIVER= DIAG_START_COMMON+ DIAG_SIZE_COMMON, + DIAG_START_FRONTEND = DIAG_START_DRIVER+ DIAG_SIZE_DRIVER, + DIAG_START_SERIALIZATION = DIAG_START_FRONTEND + DIAG_SIZE_FRONTEND, + DIAG_START_LEX = DIAG_START_SERIALIZATION + DIAG_SIZE_SERIALIZATION, + DIAG_START_PARSE = DIAG_START_LEX + DIAG_SIZE_LEX, + DIAG_START_AST = DIAG_START_PARSE + DIAG_SIZE_PARSE, + DIAG_START_COMMENT = DIAG_START_AST + DIAG_SIZE_AST, + DIAG_START_SEMA = DIAG_START_COMMENT + DIAG_SIZE_COMMENT, + DIAG_START_ANALYSIS = DIAG_START_SEMA + DIAG_SIZE_SEMA, + DIAG_UPPER_LIMIT = DIAG_START_ANALYSIS + DIAG_SIZE_ANALYSIS }; class CustomDiagInfo; Modified: cfe/trunk/lib/Basic/DiagnosticIDs.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/DiagnosticIDs.cpp?rev=311905&r1=311904&r2=311905&view=diff == --- cfe/trunk/lib/Basic/DiagnosticIDs.cpp (original) +++ cfe/trunk/lib/Basic/DiagnosticIDs.cpp Mon Aug 28 11:53:17 2017 @@ -68,6 +68,29 @@ struct StaticDiagInfoRec { } }; +#define STRINGIFY_NAME(NAME) #NAME +#define VALIDATE_DIAG_SIZE(NAME) \ + static_assert( \ + static_cast(diag::NUM_BUILTIN_##NAME##_DIAGNOSTICS) < \ + static_cast(diag::DIAG_START_##NAME) + \ + static_cast(diag::DIAG_SIZE_##NAME), \ + STRINGIFY_NAME( \ + DIAG_SIZE_##NAME) " is insufficient to contain all " \ +"diagnostics, it may need to be made larger in " \ +"DiagnosticIDs.h."); +VALIDATE_DIAG_SIZE(COMMON) +VALIDATE_DIAG_SIZE(DRIVER) +VALIDATE_DIAG_SIZE(FRONTEND) +VALIDATE_DIAG_SIZE(SERIALIZATION) +VALIDATE_DIAG_SIZE(LEX) +VALIDATE_DIAG_SIZE(PARSE) +VALIDATE_DIAG_SIZE(AST) +VALIDATE_DIAG_SIZE(COMMENT) +VALIDATE_DIAG_SIZE(SEMA) +VALIDATE_DIAG_SIZE(ANALYSIS) +#undef VALIDATE_DIAG_SIZE +#undef STRINGIFY_NAME + } // namespace anonymous static const StaticDiagInfoRec StaticDiagInfo[] = { @@ -96,18 +119,6 @@ static const unsigned StaticDiagInfoSize /// GetDiagInfo - Return the StaticDiagInfoRec entry for the specified DiagID, /// or null if the ID is invalid. static const StaticDiagInfoRec *GetDiagInf
[PATCH] D37122: Change Diagnostic Category size error from runtime to compiletime
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. lgtm https://reviews.llvm.org/D37122 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26796: [Driver] Use arch type to find compiler-rt libraries (on Linux)
mgorny added a comment. @eugenis, thanks for the suggestion. I've found the problematic push (that attempts to move the wrong file) and filed https://reviews.llvm.org/D37226 for review. Repository: rL LLVM https://reviews.llvm.org/D26796 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r311908 - [OPENMP] Remove unused header files, NFC.
Author: abataev Date: Mon Aug 28 12:26:54 2017 New Revision: 311908 URL: http://llvm.org/viewvc/llvm-project?rev=311908&view=rev Log: [OPENMP] Remove unused header files, NFC. Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=311908&r1=311907&r2=311908&view=diff == --- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original) +++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Mon Aug 28 12:26:54 2017 @@ -22,10 +22,7 @@ #include "clang/AST/StmtCXX.h" #include "clang/AST/StmtOpenMP.h" #include "clang/AST/StmtVisitor.h" -#include "clang/AST/TypeOrdering.h" #include "clang/Basic/OpenMPKinds.h" -#include "clang/Basic/TargetInfo.h" -#include "clang/Lex/Preprocessor.h" #include "clang/Sema/Initialization.h" #include "clang/Sema/Lookup.h" #include "clang/Sema/Scope.h" ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r311857 - Emit static constexpr member as available_externally definition
I reverted this in r311898 as it caused Chromium builds to fail with an assertion; see PR34348. On Sun, Aug 27, 2017 at 1:24 PM, Mehdi Amini via cfe-commits wrote: > Author: mehdi_amini > Date: Sun Aug 27 13:24:09 2017 > New Revision: 311857 > > URL: http://llvm.org/viewvc/llvm-project?rev=311857&view=rev > Log: > Emit static constexpr member as available_externally definition > > By exposing the constant initializer, the optimizer can fold many > of these constructs. > > Differential Revision: https://reviews.llvm.org/D34992 > > Added: > cfe/trunk/test/CodeGenCXX/cxx11-extern-constexpr.cpp > Modified: > cfe/trunk/lib/CodeGen/CodeGenModule.cpp > > Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=311857&r1=311856&r2=311857&view=diff > == > --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original) > +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Sun Aug 27 13:24:09 2017 > @@ -2437,6 +2437,28 @@ CodeGenModule::GetOrCreateLLVMGlobal(Str > D->getType().isConstant(Context) && > isExternallyVisible(D->getLinkageAndVisibility().getLinkage())) >GV->setSection(".cp.rodata"); > + > +// Check if we a have a const declaration with an initializer, we may be > +// able to emit it as available_externally to expose it's value to the > +// optimizer. > +if (Context.getLangOpts().CPlusPlus && GV->hasExternalLinkage() && > +D->getType().isConstQualified() && !GV->hasInitializer() && > +!D->hasDefinition() && D->hasInit() && !D->hasAttr()) > { > + const auto *Record = > + Context.getBaseElementType(D->getType())->getAsCXXRecordDecl(); > + bool HasMutableFields = Record && Record->hasMutableFields(); > + if (!HasMutableFields) { > +const VarDecl *InitDecl; > +const Expr *InitExpr = D->getAnyInitializer(InitDecl); > +if (InitExpr) { > + GV->setConstant(true); > + GV->setLinkage(llvm::GlobalValue::AvailableExternallyLinkage); > + ConstantEmitter emitter(*this); > + GV->setInitializer(emitter.tryEmitForInitializer(*InitDecl)); > + emitter.finalize(GV); > +} > + } > +} >} > >auto ExpectedAS = > > Added: cfe/trunk/test/CodeGenCXX/cxx11-extern-constexpr.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx11-extern-constexpr.cpp?rev=311857&view=auto > == > --- cfe/trunk/test/CodeGenCXX/cxx11-extern-constexpr.cpp (added) > +++ cfe/trunk/test/CodeGenCXX/cxx11-extern-constexpr.cpp Sun Aug 27 13:24:09 > 2017 > @@ -0,0 +1,55 @@ > +// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - -triple x86_64-linux-gnu | > FileCheck %s --check-prefix=CHECK --check-prefix=CXX11 > +// RUN: %clang_cc1 -std=c++1z %s -emit-llvm -o - -triple x86_64-linux-gnu | > FileCheck %s --check-prefix=CHECK --check-prefix=CXX17 > + > +struct A { > + static const int Foo = 123; > +}; > +// CHECK: @_ZN1A3FooE = constant i32 123, align 4 > +const int *p = &A::Foo; // emit available_externally > +const int A::Foo; // convert to full definition > + > +struct Bar { > + int b; > +}; > + > +struct MutableBar { > + mutable int b; > +}; > + > +struct Foo { > + // CXX11: @_ZN3Foo21ConstexprStaticMemberE = available_externally constant > i32 42, > + // CXX17: @_ZN3Foo21ConstexprStaticMemberE = linkonce_odr constant i32 42, > + static constexpr int ConstexprStaticMember = 42; > + // CHECK: @_ZN3Foo17ConstStaticMemberE = available_externally constant i32 > 43, > + static const int ConstStaticMember = 43; > + > + // CXX11: @_ZN3Foo23ConstStaticStructMemberE = available_externally > constant %struct.Bar { i32 44 }, > + // CXX17: @_ZN3Foo23ConstStaticStructMemberE = linkonce_odr constant > %struct.Bar { i32 44 }, > + static constexpr Bar ConstStaticStructMember = {44}; > + > + // CXX11: @_ZN3Foo34ConstexprStaticMutableStructMemberE = external global > %struct.MutableBar, > + // CXX17: @_ZN3Foo34ConstexprStaticMutableStructMemberE = linkonce_odr > global %struct.MutableBar { i32 45 }, > + static constexpr MutableBar ConstexprStaticMutableStructMember = {45}; > +}; > +// CHECK: @_ZL15ConstStaticexpr = internal constant i32 46, > +static constexpr int ConstStaticexpr = 46; > +// CHECK: @_ZL9ConstExpr = internal constant i32 46, align 4 > +static const int ConstExpr = 46; > + > +// CHECK: @_ZL21ConstexprStaticStruct = internal constant %struct.Bar { i32 > 47 }, > +static constexpr Bar ConstexprStaticStruct = {47}; > + > +// CHECK: @_ZL28ConstexprStaticMutableStruct = internal global > %struct.MutableBar { i32 48 }, > +static constexpr MutableBar ConstexprStaticMutableStruct = {48}; > + > +void use(const int &); > +void foo() { > + use(Foo::ConstexprStaticMember); > + use(Foo::ConstStaticMember); > + use(Foo::ConstStat
[PATCH] D36915: [Sema] Diagnose local variables and parameters captured by lambda and block expressions in a default argument
faisalv added a comment. In https://reviews.llvm.org/D36915#854317, @ahatanak wrote: > In https://reviews.llvm.org/D36915#849622, @faisalv wrote: > > > I don't think this approach is entirely correct for at least the following > > reasons: > > > > 1. in the lambda case the machinery that diagnoses capture failures should > > be the machinery erroring on the lambda (when the parameter is odr-used) > > > Does this mean that it is OK to have a parameter or local variable appear in > a potentially-evaluated expression as long as it is not odr-used? I think > this is slightly different from the standard, which says a parameter or local > variable cannot appear as a potentially-evaluated expression in a default > argument. > > For example: > > void foo2(int); > > void func() { > const int a = 1; > void foo1(S s = [](){ foo2(a); }); // "a" is not in an unevaluated > context and is not odr-used. > } > > > > I think I need clarification as it will affect how or where we detect this > error. 'a' is not captured (and does not need to be captured) above, so I think that code should be ok. But I also think the following code should be ok at local scope within func. void foo(int = a); I thought there was a DR against the standard-draft with the intent of making these valid (if they are not already so)? https://reviews.llvm.org/D36915 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37089: [Sema] Error out early for tags defined inside an enumeration.
vsapsai added a comment. It occurred to me that another approach for fixing the crash can be setting `IsTypeSpecifier` to `true` while we ParseCastExpression. According to cast-expression: unary-expression ( type-id ) cast-expression type-id: type-specifier-seq abstract-declarator[opt] it looks reasonable to require `IsTypeSpecifier` during parsing part of cast expression inside parentheses. What do you think? Is this approach worth pursuing? Does it sound better than the one offered initially? https://reviews.llvm.org/D37089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37156: [SanitizeCoverage] Enable stack-depth coverage for -fsanitize=fuzzer
george.karpenkov added a comment. @kcc I've disabled the relevant test on Mac in r311916, please revert my change once this CR goes through. https://reviews.llvm.org/D37156 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better
lebedev.ri added a comment. I did not test this yet, but looks better :) Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:26 } throw non_derived_exception(); // Bad + // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception' Could you please update the comments? I find them misleading. What does this `bad` mean? That this line is not handled correctly? Then please use `FIXME: wrong handling` That this line is invalid? The `// CHECK-MESSAGES:` already states that... Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:54-62 +// CHECK-MESSAGES: [[@LINE-1]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception' +// CHECK-MESSAGES: 71:1: note: type defined here +// CHECK-MESSAGES: [[@LINE-3]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception' +// CHECK-MESSAGES: 71:1: note: type defined here +// CHECK-MESSAGES: [[@LINE-5]]:31: warning: throwing an exception whose type 'exotic_exception' is not derived from 'std::exception' +// CHECK-MESSAGES: 74:1: note: type defined here +// CHECK-MESSAGES: [[@LINE-7]]:31: warning: throwing an exception whose type 'int' is not derived from 'std::exception' JonasToth wrote: > these diagnostic come from the many instantiations of this function. > do you think, they should exist or rather not? They definitively should exist. But they also should ideally point to the origin of the `T`. https://reviews.llvm.org/D37060 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor
johannes updated this revision to Diff 112951. johannes added a comment. use a specialized getStmtChildren to fix the order for CXXOperatorCallExpr https://reviews.llvm.org/D37200 Files: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h include/clang/AST/RecursiveASTVisitor.h unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp Index: unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp === --- unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp +++ unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp @@ -21,42 +21,55 @@ : public LexicallyOrderedRecursiveASTVisitor { public: LexicallyOrderedDeclVisitor(DummyMatchVisitor &Matcher, - const SourceManager &SM, bool EmitIndices) + const SourceManager &SM, bool EmitDeclIndices, + bool EmitStmtIndices) : LexicallyOrderedRecursiveASTVisitor(SM), Matcher(Matcher), -EmitIndices(EmitIndices) {} +EmitDeclIndices(EmitDeclIndices), EmitStmtIndices(EmitStmtIndices) {} bool TraverseDecl(Decl *D) { TraversalStack.push_back(D); LexicallyOrderedRecursiveASTVisitor::TraverseDecl(D); TraversalStack.pop_back(); return true; } + bool TraverseStmt(Stmt *S); + bool VisitNamedDecl(const NamedDecl *D); + bool VisitDeclRefExpr(const DeclRefExpr *D); private: DummyMatchVisitor &Matcher; - bool EmitIndices; + bool EmitDeclIndices, EmitStmtIndices; unsigned Index = 0; llvm::SmallVector TraversalStack; }; class DummyMatchVisitor : public ExpectedLocationVisitor { - bool EmitIndices; + bool EmitDeclIndices, EmitStmtIndices; public: - DummyMatchVisitor(bool EmitIndices = false) : EmitIndices(EmitIndices) {} + DummyMatchVisitor(bool EmitDeclIndices = false, bool EmitStmtIndices = false) + : EmitDeclIndices(EmitDeclIndices), EmitStmtIndices(EmitStmtIndices) {} bool VisitTranslationUnitDecl(TranslationUnitDecl *TU) { const ASTContext &Context = TU->getASTContext(); const SourceManager &SM = Context.getSourceManager(); -LexicallyOrderedDeclVisitor SubVisitor(*this, SM, EmitIndices); +LexicallyOrderedDeclVisitor SubVisitor(*this, SM, EmitDeclIndices, + EmitStmtIndices); SubVisitor.TraverseDecl(TU); return false; } - void match(StringRef Path, const Decl *D) { Match(Path, D->getLocStart()); } + template void match(StringRef Path, const T *D) { +Match(Path, D->getLocStart()); + } }; +bool LexicallyOrderedDeclVisitor::TraverseStmt(Stmt *S) { + Matcher.match("overridden TraverseStmt", S); + return LexicallyOrderedRecursiveASTVisitor::TraverseStmt(S); +} + bool LexicallyOrderedDeclVisitor::VisitNamedDecl(const NamedDecl *D) { std::string Path; llvm::raw_string_ostream OS(Path); @@ -73,7 +86,16 @@ if (isa(D) or isa(D)) OS << "/"; } - if (EmitIndices) + if (EmitDeclIndices) +OS << "@" << Index++; + Matcher.match(OS.str(), D); + return true; +} + +bool LexicallyOrderedDeclVisitor::VisitDeclRefExpr(const DeclRefExpr *D) { + std::string Name = D->getFoundDecl()->getNameAsString(); + llvm::raw_string_ostream OS(Name); + if (EmitStmtIndices) OS << "@" << Index++; Matcher.match(OS.str(), D); return true; @@ -151,13 +173,55 @@ template T f(); template class Class {}; )"; - DummyMatchVisitor Visitor(/*EmitIndices=*/true); + DummyMatchVisitor Visitor(/*EmitDeclIndices=*/true); Visitor.ExpectMatch("/f/T@0", 2, 11); Visitor.ExpectMatch("/f/f/@1", 2, 20); Visitor.ExpectMatch("/Class/U@2", 3, 11); Visitor.ExpectMatch("/Class/@3", 3, 20); Visitor.ExpectMatch("/Class/Class/@4", 3, 34); EXPECT_TRUE(Visitor.runOver(Source)); } +TEST(LexicallyOrderedRecursiveASTVisitor, VisitCXXOperatorCallExpr) { + StringRef Source = R"( +struct S { + S &operator+(S&); + S *operator->(); + S &operator++(); + S operator++(int); + void operator()(int, int); + void operator[](int); + void f(); +}; +S a, b, c; + +void test() { + a = b + c; + a->f(); + a(1, 2); + b[0]; + ++a; + b++; +} +)"; + DummyMatchVisitor Visitor(/*EmitDeclIndices=*/false, +/*EmitStmtIndices=*/true); + // There are two overloaded operators that start at this point + // This makes sure they are both traversed using the overridden + // TraverseStmt, as the traversal is implemented by us for + // CXXOperatorCallExpr. + Visitor.ExpectMatch("overridden TraverseStmt", 14, 3, 2); + Visitor.ExpectMatch("a@0", 14, 3); + Visitor.ExpectMatch("operator=@1", 14, 5); + Visitor.ExpectMatch("b@2", 14, 7); + Visitor.ExpectMatch("operator+@3", 14, 9); + Visitor.ExpectMatch("c@4", 14, 11); + Visitor.ExpectMatch("operator->@6", 15, 4); + Visitor.ExpectMatch("operator()@8", 16, 4); + Visitor.ExpectMatch("operator[]@10", 17, 4); + Visitor.ExpectMatch("operator++@11", 18, 3); +
r311923 - Reland r311836 - [Driver] Use arch type to find compiler-rt libraries (on Linux)
Author: mgorny Date: Mon Aug 28 13:29:52 2017 New Revision: 311923 URL: http://llvm.org/viewvc/llvm-project?rev=311923&view=rev Log: Reland r311836 - [Driver] Use arch type to find compiler-rt libraries (on Linux) Use llvm::Triple::getArchTypeName() when looking for compiler-rt libraries, rather than the exact arch string from the triple. This is more correct as it matches the values used when building compiler-rt (builtin-config-ix.cmake) which are the subset of the values allowed in triples. For example, this fixes an issue when the compiler set for i686-pc-linux-gnu triple would not find an i386 compiler-rt library, while this is the exact arch that is detected by compiler-rt. The same applies to any other i?86 variant allowed by LLVM. This also makes the special case for MSVC unnecessary, since now i386 will be used reliably for all 32-bit x86 variants. Differential Revision: https://reviews.llvm.org/D26796 Added: cfe/trunk/test/Driver/Inputs/basic_linux_tree/usr/i686-unknown-linux/lib/.keep cfe/trunk/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/i686-unknown-linux/4.6.0/crtbegin.o Modified: cfe/trunk/lib/Driver/ToolChain.cpp cfe/trunk/test/Driver/linux-ld.c cfe/trunk/test/Driver/nostdlib.c cfe/trunk/test/Driver/print-libgcc-file-name-clangrt.c cfe/trunk/test/Driver/windows-cross.c Modified: cfe/trunk/lib/Driver/ToolChain.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChain.cpp?rev=311923&r1=311922&r2=311923&view=diff == --- cfe/trunk/lib/Driver/ToolChain.cpp (original) +++ cfe/trunk/lib/Driver/ToolChain.cpp Mon Aug 28 13:29:52 2017 @@ -297,15 +297,12 @@ static StringRef getArchNameForCompilerR const llvm::Triple &Triple = TC.getTriple(); bool IsWindows = Triple.isOSWindows(); - if (Triple.isWindowsMSVCEnvironment() && TC.getArch() == llvm::Triple::x86) -return "i386"; - if (TC.getArch() == llvm::Triple::arm || TC.getArch() == llvm::Triple::armeb) return (arm::getARMFloatABI(TC, Args) == arm::FloatABI::Hard && !IsWindows) ? "armhf" : "arm"; - return TC.getArchName(); + return llvm::Triple::getArchTypeName(TC.getArch()); } std::string ToolChain::getCompilerRTPath() const { Added: cfe/trunk/test/Driver/Inputs/basic_linux_tree/usr/i686-unknown-linux/lib/.keep URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/basic_linux_tree/usr/i686-unknown-linux/lib/.keep?rev=311923&view=auto == (empty) Added: cfe/trunk/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/i686-unknown-linux/4.6.0/crtbegin.o URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/i686-unknown-linux/4.6.0/crtbegin.o?rev=311923&view=auto == (empty) Modified: cfe/trunk/test/Driver/linux-ld.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/linux-ld.c?rev=311923&r1=311922&r2=311923&view=diff == --- cfe/trunk/test/Driver/linux-ld.c (original) +++ cfe/trunk/test/Driver/linux-ld.c Mon Aug 28 13:29:52 2017 @@ -71,6 +71,27 @@ // CHECK-LD-RT: libclang_rt.builtins-x86_64.a" // // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ +// RUN: --target=i686-unknown-linux \ +// RUN: --gcc-toolchain="" \ +// RUN: --sysroot=%S/Inputs/basic_linux_tree \ +// RUN: --rtlib=compiler-rt \ +// RUN: | FileCheck --check-prefix=CHECK-LD-RT-I686 %s +// CHECK-LD-RT-I686-NOT: warning: +// CHECK-LD-RT-I686: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]" +// CHECK-LD-RT-I686: "--eh-frame-hdr" +// CHECK-LD-RT-I686: "-m" "elf_i386" +// CHECK-LD-RT-I686: "-dynamic-linker" +// CHECK-LD-RT-I686: "{{.*}}/usr/lib/gcc/i686-unknown-linux/4.6.0{{/|}}crtbegin.o" +// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib/gcc/i686-unknown-linux/4.6.0" +// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib/gcc/i686-unknown-linux/4.6.0/../../../../i686-unknown-linux/lib" +// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib/gcc/i686-unknown-linux/4.6.0/../../.." +// CHECK-LD-RT-I686: "-L[[SYSROOT]]/lib" +// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib" +// CHECK-LD-RT-I686: libclang_rt.builtins-i386.a" +// CHECK-LD-RT-I686: "-lc" +// CHECK-LD-RT-I686: libclang_rt.builtins-i386.a" +// +// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ // RUN: --target=arm-linux-androideabi \ // RUN: --gcc-toolchain="" \ // RUN: --sysroot=%S/Inputs/basic_android_tree/sysroot \ Modified: cfe/trunk/test/Driver/nostdlib.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/nostdlib.c?rev=311923&r1=311922&r2=311923&view=diff == --- cfe/trunk/test/Driver/nostdlib.c (original) +++ cfe/tru
[PATCH] D26796: [Driver] Use arch type to find compiler-rt libraries (on Linux)
This revision was automatically updated to reflect the committed changes. Closed by commit rL311923: Reland r311836 - [Driver] Use arch type to find compiler-rt libraries (on Linux) (authored by mgorny). Repository: rL LLVM https://reviews.llvm.org/D26796 Files: cfe/trunk/lib/Driver/ToolChain.cpp cfe/trunk/test/Driver/Inputs/basic_linux_tree/usr/i686-unknown-linux/lib/.keep cfe/trunk/test/Driver/Inputs/basic_linux_tree/usr/lib/gcc/i686-unknown-linux/4.6.0/crtbegin.o cfe/trunk/test/Driver/linux-ld.c cfe/trunk/test/Driver/nostdlib.c cfe/trunk/test/Driver/print-libgcc-file-name-clangrt.c cfe/trunk/test/Driver/windows-cross.c Index: cfe/trunk/test/Driver/windows-cross.c === --- cfe/trunk/test/Driver/windows-cross.c +++ cfe/trunk/test/Driver/windows-cross.c @@ -64,7 +64,7 @@ // RUN:| FileCheck %s --check-prefix CHECK-SANITIZE-ADDRESS-EXE-X86 // CHECK-SANITIZE-ADDRESS-EXE-X86: "-fsanitize=address" -// CHECK-SANITIZE-ADDRESS-EXE-X86: "{{.*}}clang_rt.asan_dynamic-i686.lib" "{{.*}}clang_rt.asan_dynamic_runtime_thunk-i686.lib" "--undefined" "___asan_seh_interceptor" +// CHECK-SANITIZE-ADDRESS-EXE-X86: "{{.*}}clang_rt.asan_dynamic-i386.lib" "{{.*}}clang_rt.asan_dynamic_runtime_thunk-i386.lib" "--undefined" "___asan_seh_interceptor" // RUN: %clang -### -target armv7-windows-itanium --sysroot %S/Inputs/Windows/ARM/8.1 -B %S/Inputs/Windows/ARM/8.1/usr/bin -fuse-ld=lld-link2 -shared -o shared.dll -fsanitize=tsan -x c++ %s 2>&1 \ // RUN:| FileCheck %s --check-prefix CHECK-SANITIZE-TSAN Index: cfe/trunk/test/Driver/linux-ld.c === --- cfe/trunk/test/Driver/linux-ld.c +++ cfe/trunk/test/Driver/linux-ld.c @@ -71,6 +71,27 @@ // CHECK-LD-RT: libclang_rt.builtins-x86_64.a" // // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ +// RUN: --target=i686-unknown-linux \ +// RUN: --gcc-toolchain="" \ +// RUN: --sysroot=%S/Inputs/basic_linux_tree \ +// RUN: --rtlib=compiler-rt \ +// RUN: | FileCheck --check-prefix=CHECK-LD-RT-I686 %s +// CHECK-LD-RT-I686-NOT: warning: +// CHECK-LD-RT-I686: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]" +// CHECK-LD-RT-I686: "--eh-frame-hdr" +// CHECK-LD-RT-I686: "-m" "elf_i386" +// CHECK-LD-RT-I686: "-dynamic-linker" +// CHECK-LD-RT-I686: "{{.*}}/usr/lib/gcc/i686-unknown-linux/4.6.0{{/|}}crtbegin.o" +// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib/gcc/i686-unknown-linux/4.6.0" +// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib/gcc/i686-unknown-linux/4.6.0/../../../../i686-unknown-linux/lib" +// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib/gcc/i686-unknown-linux/4.6.0/../../.." +// CHECK-LD-RT-I686: "-L[[SYSROOT]]/lib" +// CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib" +// CHECK-LD-RT-I686: libclang_rt.builtins-i386.a" +// CHECK-LD-RT-I686: "-lc" +// CHECK-LD-RT-I686: libclang_rt.builtins-i386.a" +// +// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \ // RUN: --target=arm-linux-androideabi \ // RUN: --gcc-toolchain="" \ // RUN: --sysroot=%S/Inputs/basic_android_tree/sysroot \ Index: cfe/trunk/test/Driver/nostdlib.c === --- cfe/trunk/test/Driver/nostdlib.c +++ cfe/trunk/test/Driver/nostdlib.c @@ -27,5 +27,5 @@ // // CHECK-LINUX-NOSTDLIB: warning: argument unused during compilation: '--rtlib=compiler-rt' // CHECK-LINUX-NOSTDLIB: "{{(.*[^.0-9A-Z_a-z])?}}ld{{(.exe)?}}" -// CHECK-LINUX-NOSTDLIB-NOT: "{{.*}}/Inputs/resource_dir{{/|}}lib{{/|}}linux{{/|}}libclang_rt.builtins-i686.a" +// CHECK-LINUX-NOSTDLIB-NOT: "{{.*}}/Inputs/resource_dir{{/|}}lib{{/|}}linux{{/|}}libclang_rt.builtins-i386.a" // CHECK-MSVC-NOSTDLIB: warning: argument unused during compilation: '--rtlib=compiler-rt' Index: cfe/trunk/test/Driver/print-libgcc-file-name-clangrt.c === --- cfe/trunk/test/Driver/print-libgcc-file-name-clangrt.c +++ cfe/trunk/test/Driver/print-libgcc-file-name-clangrt.c @@ -6,9 +6,15 @@ // CHECK-CLANGRT-X8664: libclang_rt.builtins-x86_64.a // RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \ +// RUN: --target=i386-pc-linux \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-I386 %s +// CHECK-CLANGRT-I386: libclang_rt.builtins-i386.a + +// Check whether alternate arch values map to the correct library. +// +// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \ // RUN: --target=i686-pc-linux \ -// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-I686 %s -// CHECK-CLANGRT-I686: libclang_rt.builtins-i686.a +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-I386 %s // RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name 2>&1 \ // RUN: --target=arm-linux-gnueabi \ Index: cfe/trunk/lib/Driver/ToolChain.cpp === --- cfe/trunk/lib/Driver/ToolCha
[PATCH] D26764: [compiler-rt] [cmake] Remove i686 target that is duplicate to i386
This revision was automatically updated to reflect the committed changes. Closed by commit rL311924: Reland r311842 - [cmake] Remove i686 target that is duplicate to i386 (authored by mgorny). Repository: rL LLVM https://reviews.llvm.org/D26764 Files: compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake compiler-rt/trunk/cmake/base-config-ix.cmake compiler-rt/trunk/cmake/builtin-config-ix.cmake compiler-rt/trunk/cmake/config-ix.cmake compiler-rt/trunk/lib/asan/CMakeLists.txt compiler-rt/trunk/lib/asan/scripts/asan_device_setup compiler-rt/trunk/lib/builtins/CMakeLists.txt compiler-rt/trunk/lib/ubsan/CMakeLists.txt compiler-rt/trunk/test/asan/CMakeLists.txt compiler-rt/trunk/test/asan/lit.cfg compiler-rt/trunk/test/lit.common.cfg compiler-rt/trunk/test/lsan/TestCases/Linux/use_tls_dynamic.cc compiler-rt/trunk/test/sanitizer_common/lit.common.cfg compiler-rt/trunk/test/scudo/random_shuffle.cpp Index: compiler-rt/trunk/cmake/builtin-config-ix.cmake === --- compiler-rt/trunk/cmake/builtin-config-ix.cmake +++ compiler-rt/trunk/cmake/builtin-config-ix.cmake @@ -25,7 +25,8 @@ set(ARM64 aarch64) set(ARM32 arm armhf armv6m armv7m armv7em armv7 armv7s armv7k) -set(X86 i386 i686) +set(ARM32 arm armhf) +set(X86 i386) set(X86_64 x86_64) set(MIPS32 mips mipsel) set(MIPS64 mips64 mips64el) Index: compiler-rt/trunk/cmake/config-ix.cmake === --- compiler-rt/trunk/cmake/config-ix.cmake +++ compiler-rt/trunk/cmake/config-ix.cmake @@ -174,7 +174,7 @@ set(ARM64 aarch64) set(ARM32 arm armhf) -set(X86 i386 i686) +set(X86 i386) set(X86_64 x86_64) set(MIPS32 mips mipsel) set(MIPS64 mips64 mips64el) Index: compiler-rt/trunk/cmake/base-config-ix.cmake === --- compiler-rt/trunk/cmake/base-config-ix.cmake +++ compiler-rt/trunk/cmake/base-config-ix.cmake @@ -139,10 +139,6 @@ elseif("${COMPILER_RT_DEFAULT_TARGET_ARCH}" MATCHES "i[2-6]86|x86|amd64") if(NOT MSVC) test_target_arch(x86_64 "" "-m64") -# FIXME: We build runtimes for both i686 and i386, as "clang -m32" may -# target different variant than "$CMAKE_C_COMPILER -m32". This part should -# be gone after we resolve PR14109. -test_target_arch(i686 __i686__ "-m32") test_target_arch(i386 __i386__ "-m32") else() if (CMAKE_SIZEOF_VOID_P EQUAL 4) Index: compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake === --- compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake +++ compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake @@ -163,7 +163,6 @@ check_symbol_exists(__arm__ "" __ARM) check_symbol_exists(__aarch64__ "" __AARCH64) check_symbol_exists(__x86_64__ "" __X86_64) - check_symbol_exists(__i686__ "" __I686) check_symbol_exists(__i386__ "" __I386) check_symbol_exists(__mips__ "" __MIPS) check_symbol_exists(__mips64__ "" __MIPS64) @@ -176,8 +175,6 @@ add_default_target_arch(aarch64) elseif(__X86_64) add_default_target_arch(x86_64) - elseif(__I686) -add_default_target_arch(i686) elseif(__I386) add_default_target_arch(i386) elseif(__MIPS64) # must be checked before __MIPS Index: compiler-rt/trunk/test/asan/CMakeLists.txt === --- compiler-rt/trunk/test/asan/CMakeLists.txt +++ compiler-rt/trunk/test/asan/CMakeLists.txt @@ -18,7 +18,7 @@ endif() macro(get_bits_for_arch arch bits) - if (${arch} MATCHES "i386|i686|arm|mips|mipsel") + if (${arch} MATCHES "i386|arm|mips|mipsel") set(${bits} 32) elseif (${arch} MATCHES "x86_64|powerpc64|powerpc64le|aarch64|mips64|mips64el|s390x") set(${bits} 64) Index: compiler-rt/trunk/test/asan/lit.cfg === --- compiler-rt/trunk/test/asan/lit.cfg +++ compiler-rt/trunk/test/asan/lit.cfg @@ -121,16 +121,11 @@ def build_invocation(compile_flags): return " " + " ".join([config.compile_wrapper, config.clang] + compile_flags) + " " -# Clang driver link 'x86' (i686) architecture to 'i386'. -target_arch = config.target_arch -if (target_arch == "i686"): - target_arch = "i386" - config.substitutions.append( ("%clang ", build_invocation(target_cflags)) ) config.substitutions.append( ("%clangxx ", build_invocation(target_cxxflags)) ) config.substitutions.append( ("%clang_asan ", build_invocation(clang_asan_cflags)) ) config.substitutions.append( ("%clangxx_asan ", build_invocation(clang_asan_cxxflags)) ) -config.substitutions.append( ("%shared_libasan", "libclang_rt.asan-%s.so" % target_arch)) +config.substitutions.append( ("%shared_libasan", "libclang_rt.asan-%s.so" % config.target_arch)) if config.asan_dynamic: config.substitutions.append( ("%clang_asan_static ", build_invocation(clang_asan_
[PATCH] D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor
johannes added a comment. Yeah, this seems to be the best solution for this. I think I ran into the same issue when working on the StmtDataCollector - basically there can only be two Traverse*, one in the Derived class and the other one needs to do all the magic with walking the specialisation hierarchy. https://reviews.llvm.org/D37200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits