arphaman (Alex Lorenz)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 25 2014, 4:17 PM (169 w, 4 d)

Recent Activity

Fri, Sep 22

arphaman accepted D37544: [ubsan] Skip alignment checks which are folded away.

LGTM

Fri, Sep 22, 11:17 AM
arphaman accepted D38081: Set completion priority of destructors and operators to CCP_Unlikely..

Please keep in mind that LLVM follows a weekly ping rate: https://llvm.org/docs/DeveloperPolicy.html#code-reviews .

Fri, Sep 22, 11:14 AM
arphaman added a comment to D37544: [ubsan] Skip alignment checks which are folded away.

This one passes for me too without the code change.

Fri, Sep 22, 10:09 AM

Thu, Sep 21

arphaman accepted D38107: [Coverage] Add an option to emit limited coverage info.

This makes sense.

Thu, Sep 21, 8:11 AM

Mon, Sep 18

arphaman added a dependency for D37976: [docs][refactor] add refactoring engine design documentation: D37856: [refactor] add support for refactoring options.
Mon, Sep 18, 6:53 AM
arphaman added a dependent revision for D37856: [refactor] add support for refactoring options: D37976: [docs][refactor] add refactoring engine design documentation.
Mon, Sep 18, 6:53 AM
arphaman created D37976: [docs][refactor] add refactoring engine design documentation.
Mon, Sep 18, 6:52 AM
arphaman accepted D37542: [ubsan] Save a ptrtoint when emitting alignment checks.

Nice, LGTM

Mon, Sep 18, 3:51 AM
arphaman added a comment to D37544: [ubsan] Skip alignment checks which are folded away.

It looks like this test passes prior to the code change in this patch. Can you please change the test?

Mon, Sep 18, 3:44 AM

Thu, Sep 14

arphaman added a dependency for D37856: [refactor] add support for refactoring options: D37681: [refactor] Simplify the interface and remove some template magic.
Thu, Sep 14, 11:07 AM
arphaman added a dependent revision for D37681: [refactor] Simplify the interface and remove some template magic: D37856: [refactor] add support for refactoring options.
Thu, Sep 14, 11:07 AM
arphaman created D37856: [refactor] add support for refactoring options.
Thu, Sep 14, 11:06 AM
arphaman committed rL313266: Fix Refactor/tool-test-support.c test on Windows by avoiding.
Fix Refactor/tool-test-support.c test on Windows by avoiding
Thu, Sep 14, 8:12 AM
arphaman updated the diff for D37681: [refactor] Simplify the interface and remove some template magic.

Make methods private

Thu, Sep 14, 6:30 AM
arphaman committed rL313260: [refactor] Use CommonOptionsParser in clang-refactor.
[refactor] Use CommonOptionsParser in clang-refactor
Thu, Sep 14, 6:19 AM
arphaman closed D37618: Use CommonOptionsParser in clang-refactor by committing rL313260: [refactor] Use CommonOptionsParser in clang-refactor.
Thu, Sep 14, 6:19 AM
arphaman committed rL313252: Link clang-refactor with clangFormat.
Link clang-refactor with clangFormat
Thu, Sep 14, 3:47 AM
arphaman committed rL313249: Link clang-refactor with clangAST and clangLex.
Link clang-refactor with clangAST and clangLex
Thu, Sep 14, 3:39 AM
arphaman committed rL313244: [refactor] add clang-refactor tool with initial testing support and.
[refactor] add clang-refactor tool with initial testing support and
Thu, Sep 14, 3:08 AM
arphaman closed D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action by committing rL313244: [refactor] add clang-refactor tool with initial testing support and.
Thu, Sep 14, 3:08 AM
arphaman added a comment to D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.

Yeah, I accidentally committed a prior patch with the link to this diff. I will commit this patch today though so it should be updated.

Thu, Sep 14, 3:08 AM
arphaman added inline comments to D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.
Thu, Sep 14, 2:34 AM

Tue, Sep 12

arphaman committed rL313027: Fix GCC build error and warnings from r313025.
Fix GCC build error and warnings from r313025
Tue, Sep 12, 6:05 AM
arphaman closed D37210: [refactor] add a refactoring action rule that returns symbol occurrences.

Committed in r313025

Tue, Sep 12, 5:52 AM
arphaman reopened D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.

Oops, I've put in the wrong diff link for r313025. This patch is still not committed.

Tue, Sep 12, 5:51 AM
arphaman committed rL313025: [refactor] add a refactoring action rule that returns symbol occurrences.
[refactor] add a refactoring action rule that returns symbol occurrences
Tue, Sep 12, 5:50 AM
arphaman closed D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action by committing rL313025: [refactor] add a refactoring action rule that returns symbol occurrences.
Tue, Sep 12, 5:50 AM

Mon, Sep 11

arphaman added a comment to D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.

@klimek I've added a patch that simplifies the interface and the template code - https://reviews.llvm.org/D37681. Hopefully you'll find that the code there is cleaner.

Mon, Sep 11, 5:59 AM
arphaman added a dependency for D37681: [refactor] Simplify the interface and remove some template magic: D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.
Mon, Sep 11, 5:59 AM
arphaman added a dependent revision for D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action: D37681: [refactor] Simplify the interface and remove some template magic.
Mon, Sep 11, 5:59 AM
arphaman created D37681: [refactor] Simplify the interface and remove some template magic.
Mon, Sep 11, 5:53 AM
arphaman added a comment to D37671: [index] Generate class & metaclass manglings for objc.

You should be able to test it using a unittest if this is not exposed in the C API.

Mon, Sep 11, 3:28 AM
arphaman accepted D37642: [pp-trace] Update skipped source ranges in tests.

LG

Mon, Sep 11, 3:16 AM
arphaman accepted D36642: [Lexer] Report more precise skipped regions (PR34166).

Thanks, LGTM

Mon, Sep 11, 3:15 AM

Fri, Sep 8

arphaman created D37618: Use CommonOptionsParser in clang-refactor.
Fri, Sep 8, 2:55 AM
arphaman added a comment to D33960: ErrorHandling: report_fatal_error: call abort() instead of exit().

Did a quick ad-hoc and incomplete survey of report_fatal_error usage in LLVM.

Abort Worthy:

  • Situations where IR/MI violated assumptions: IR/Verifier, MachineVerifier, WinEHPrepare, LiveRangeCalc, TargetLoweringObjFile, Coro*, Analysis,
  • Unimplemented functionality: Support/YAMLParser,
  • Out of resources/memory: MachineOutliner
  • Internal compiler problems: Expected passes not in pipeline, target callbacks mibehaving: SafeStack, SplitKit, TargetInstrInfo

    User/Input errors:
  • Wrong options: TargetPassConfig
  • Parse Errors: Bitcode/Reader, IR/DataLayout
Fri, Sep 8, 1:36 AM
arphaman added inline comments to D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.
Fri, Sep 8, 12:51 AM
arphaman updated the diff for D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.
  • rename namespace
  • try to further decouple SourceSelectionArgument
Fri, Sep 8, 12:50 AM

Thu, Sep 7

arphaman accepted D37599: Add '\n' in ClangDataCollectorsEmitter.

LGTM. Do you have commit access?

Thu, Sep 7, 10:48 PM
arphaman added inline comments to D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.
Thu, Sep 7, 12:48 AM
arphaman updated the diff for D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.

Refactor the selection argument handling by using a SourceSelectionArgument interface with subclass for test selection and separate out the test code from the tool class.

Thu, Sep 7, 12:48 AM

Wed, Sep 6

arphaman updated the diff for D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.

Address review comments

Wed, Sep 6, 3:32 AM
arphaman added inline comments to D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.
Wed, Sep 6, 3:08 AM

Tue, Sep 5

arphaman added inline comments to D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.
Tue, Sep 5, 3:32 AM
arphaman updated the diff for D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.
  • Always pass in RefactoringRuleContext to the refactoring rule's function.
  • Remove now redundant overloads.
  • Make LocalRename's actions members private.
  • Add CommandLienParser FIXME comment.
  • Verify that refactoring actions have unique command names.
  • Add a comment that explains how a subcommand is selected.
Tue, Sep 5, 3:32 AM
arphaman added inline comments to D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.
Tue, Sep 5, 12:56 AM
arphaman updated the diff for D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.
  • Test by checking the modified source directly. This allowed me to get rid of the refactoring result wrapper as well.
  • Remove ASTRefactoringContext
  • Address the other review comments
Tue, Sep 5, 12:53 AM

Mon, Sep 4

arphaman accepted D37390: [diagtool] Change default tree behavior to print only flags.

Please precommit the auto loop modernization though. You might want to use llvm::make_range for for-ranged loops as well.

Mon, Sep 4, 9:58 AM · Restricted Project
arphaman added a comment to D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.

One of my main concerns is still that I don't see the need for all the template magic yet :) Why doesn't everybody use the RefactoringResult we define here?

@klimek, are you talking about the template usage in this patch or the whole approach in general? I can probably think of some way to reduce the template boilerplate if you are talking about the general approach.

Talking about the approach in general :)

Mon, Sep 4, 4:08 AM
arphaman added a comment to D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.

One of my main concerns is still that I don't see the need for all the template magic yet :) Why doesn't everybody use the RefactoringResult we define here?

Mon, Sep 4, 3:23 AM
arphaman updated the diff for D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.

Get rid of inheritance of RefactoringEngine by getting rid of the class itself.

Mon, Sep 4, 3:13 AM
arphaman added a comment to D37390: [diagtool] Change default tree behavior to print only flags.

The diagtool tests should go to test/Misc, and there's an existing diagtool tree test there that has to be updated as well.

Mon, Sep 4, 2:37 AM · Restricted Project
arphaman accepted D36998: [AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor.

LGTM

Mon, Sep 4, 12:48 AM
arphaman added a comment to D37383: [AST] Add TableGen for StmtDataCollectors.

LGTM with one inline comment below:

Mon, Sep 4, 12:45 AM
arphaman accepted D37383: [AST] Add TableGen for StmtDataCollectors.

defs could work, but I think that we should stick with TableGen. As you've said, we'd be able to introduce some sort of structure instead of just code in the future which will be better (Ideally we'd try to do it now, but given the fact that the GSoC is done the current approach is acceptable). I would also like to see this analyzed and tested better in the future to ensure a complete coverage of AST, and I think the use of TableGen can potentially help with that.

Mon, Sep 4, 12:44 AM

Fri, Sep 1

arphaman added a comment to D37383: [AST] Add TableGen for StmtDataCollectors.

@arphaman I suggested tablegen in D36664 because I remembered we had some CMake sanity check about not having an *.inc in our include dir: https://github.com/llvm-mirror/clang/blob/master/CMakeLists.txt#L266 Not sure if it actually fires for our case, but it looks like it does.

Fri, Sep 1, 10:48 AM
arphaman added inline comments to D36998: [AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor.
Fri, Sep 1, 10:02 AM
arphaman added a comment to D37383: [AST] Add TableGen for StmtDataCollectors.

TableGen is great for structured data, but we shouldn't use it just to embed code in it. What was the issue you've had with the build when the inc file was in include?

Fri, Sep 1, 9:58 AM
arphaman added inline comments to D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.
Fri, Sep 1, 7:46 AM
arphaman added a comment to D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.

One of my main concerns is still that I don't see the need for all the template magic yet :) Why doesn't everybody use the RefactoringResult we define here?

Fri, Sep 1, 7:43 AM
arphaman added a dependent revision for D37210: [refactor] add a refactoring action rule that returns symbol occurrences: D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.
Fri, Sep 1, 4:21 AM
arphaman added a dependency for D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action: D37210: [refactor] add a refactoring action rule that returns symbol occurrences.
Fri, Sep 1, 4:21 AM
arphaman updated the diff for D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action.

Rebase on top of trunk and https://reviews.llvm.org/D37210.

Fri, Sep 1, 4:20 AM
arphaman updated the diff for D37210: [refactor] add a refactoring action rule that returns symbol occurrences.

Rebase on ToT

Fri, Sep 1, 2:24 AM
arphaman committed rL312316: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring.
[refactor] Use a RefactoringResultConsumer instead of tagged refactoring
Fri, Sep 1, 2:17 AM
arphaman closed D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes by committing rL312316: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring.
Fri, Sep 1, 2:17 AM

Thu, Aug 31

arphaman added a comment to D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes.

Thanks! I think this makes the code easier to understand. Now my remaining question is why the ResultType is templates vs. also being an interface (sorry for being late, was out on vacation the past 2 weeks :)

Thu, Aug 31, 9:56 AM
arphaman added inline comments to D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes.
Thu, Aug 31, 9:52 AM
arphaman updated the summary of D37341: [Sema] Fix an assert-on-invalid by avoiding function template specialisation deduction for invalid functions with fabricated template arguments.
Thu, Aug 31, 9:35 AM
arphaman created D37341: [Sema] Fix an assert-on-invalid by avoiding function template specialisation deduction for invalid functions with fabricated template arguments.
Thu, Aug 31, 9:34 AM
arphaman committed rL312246: Revert r312240.
Revert r312240
Thu, Aug 31, 8:52 AM
arphaman added inline comments to D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes.
Thu, Aug 31, 6:46 AM
arphaman updated the diff for D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes.
  • Add default implementation for handle()
  • Merge two error handling functions into one
Thu, Aug 31, 6:43 AM
arphaman committed rL312240: Build LLVM with -Wstrict-prototypes enabled.
Build LLVM with -Wstrict-prototypes enabled
Thu, Aug 31, 6:26 AM
arphaman closed D36669: Build LLVM with -Wstrict-prototypes enabled by committing rL312240: Build LLVM with -Wstrict-prototypes enabled.
Thu, Aug 31, 6:26 AM
arphaman added inline comments to D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes.
Thu, Aug 31, 6:18 AM

Wed, Aug 30

arphaman committed rL312133: Avoid 'size_t' typedef in the unittest ObjC code.
Avoid 'size_t' typedef in the unittest ObjC code
Wed, Aug 30, 8:40 AM
arphaman committed rL312132: Recommit r312127: [refactor] AST selection tree should contain syntactic.
Recommit r312127: [refactor] AST selection tree should contain syntactic
Wed, Aug 30, 8:30 AM
arphaman added inline comments to D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes.
Wed, Aug 30, 8:20 AM
arphaman committed rL312131: Revert r312127 as the ObjC unittest code fails to compile on Linux.
Revert r312127 as the ObjC unittest code fails to compile on Linux
Wed, Aug 30, 8:13 AM
arphaman committed rL312127: [refactor] AST selection tree should contain syntactic form.
[refactor] AST selection tree should contain syntactic form
Wed, Aug 30, 8:01 AM
arphaman committed rL312121: [refactor] Examine the whole range for ObjC @implementation decls.
[refactor] Examine the whole range for ObjC @implementation decls
Wed, Aug 30, 6:25 AM
arphaman updated the diff for D37210: [refactor] add a refactoring action rule that returns symbol occurrences.

Rebase on top of https://reviews.llvm.org/D37291

Wed, Aug 30, 3:36 AM
arphaman updated the diff for D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes.

Use std::move

Wed, Aug 30, 3:34 AM
arphaman updated the diff for D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes.

Fixed a comment

Wed, Aug 30, 3:26 AM
arphaman created D37291: [refactor] Use a RefactoringResultConsumer instead of tagged refactoring rule classes.
Wed, Aug 30, 3:16 AM
arphaman added inline comments to D37210: [refactor] add a refactoring action rule that returns symbol occurrences.
Wed, Aug 30, 2:24 AM

Tue, Aug 29

arphaman added inline comments to D37001: [clang-diff] Use data collectors for node comparison.
Tue, Aug 29, 9:00 AM
arphaman added inline comments to D37005: [clang-diff] Initial implementation of patching.
Tue, Aug 29, 5:36 AM
arphaman added inline comments to D37005: [clang-diff] Initial implementation of patching.
Tue, Aug 29, 5:32 AM
arphaman accepted D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor.

Lgtm with two requests below

Tue, Aug 29, 5:25 AM

Mon, Aug 28

arphaman added inline comments to D36075: [refactor] Initial support for refactoring action rules .
Mon, Aug 28, 8:11 AM
arphaman added a comment to D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor.

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":

Mon, Aug 28, 7:29 AM
arphaman added inline comments to D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor.
Mon, Aug 28, 6:33 AM
arphaman added inline comments to D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor.
Mon, Aug 28, 5:06 AM
arphaman added a comment to D37200: [AST] Traverse CXXOperatorCallExpr in LexicallyOrderedRecursiveASTVisitor.

This works as well I think

Mon, Aug 28, 5:05 AM
arphaman committed rL311886: Avoid missing std error code in RefactoringActionRulesTest.cpp.
Avoid missing std error code in RefactoringActionRulesTest.cpp
Mon, Aug 28, 5:04 AM
arphaman created D37210: [refactor] add a refactoring action rule that returns symbol occurrences.
Mon, Aug 28, 4:31 AM
arphaman committed rL311884: [refactor] initial support for refactoring action rules.
[refactor] initial support for refactoring action rules
Mon, Aug 28, 4:13 AM
arphaman closed D36075: [refactor] Initial support for refactoring action rules by committing rL311884: [refactor] initial support for refactoring action rules.
Mon, Aug 28, 4:13 AM