This is an archive of the discontinued LLVM Phabricator instance.

[clang-tidy] New checker to replace deprecated throw() specifications
AbandonedPublic

Authored by hintonda on Mar 29 2016, 1:55 PM.

Details

Reviewers
aaron.ballman
Summary

This checker allows users to replace deprecated throw()
specifications with noexcept.

It also allows users to specify a user defined macro to be used
instead of noexect, which is useful when supporting older compilers.
However, the macro will not be used for noexcect(false) since that
doesn't make sense.

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
aaron.ballman added inline comments.Mar 30 2016, 7:19 AM
clang-tidy/modernize/UseNoexceptCheck.cpp
25

Why 5?

34

This should not be at file scope; if it really clarifies the code, it should be at function scope where needed.

66

Instead of indenting a considerable amount of code, I think an early return may be a bit more clear.

clang-tidy/modernize/UseNoexceptCheck.h
22

I think you mean "Replace dynamic exception specifications" instead of "noexcept specifications".

It's much better to add information in release notes in same patch as check itself. See also D18509 for related add_new_check.py improvement.

Sounds like a good idea. I'll add the additional transformations you mentioned and remove s/noexcept(true)/noexcept/.

clang-tidy/modernize/UseNoexceptCheck.cpp
25

No particular reason -- copied basic implementation from utils::parseHeaderFileExtensions() which did something similar.

34

Will remove/refactor -- was just following the examples I found in other checkers.

84

Ah, that helps a lot. I'll use getExceptionSpecType(), but will still need to get the location of the end of the token sequence for replacement purposes, e.g., throw() is 3 tokens.

hintonda updated this revision to Diff 52322.Mar 31 2016, 7:02 PM

Address comments and refactor.

hintonda updated this object.Mar 31 2016, 7:19 PM
hintonda marked 8 inline comments as done.Mar 31 2016, 7:28 PM

Addressed comments and believe it's ready for review.

alexfh requested changes to this revision.Apr 1 2016, 8:19 PM
alexfh edited edge metadata.
alexfh added inline comments.
clang-tidy/modernize/UseNoexceptCheck.cpp
17

Yes, it might make sense to move it to ASTMatchers.h.

32

Please rename the matcher to hasDynamicExceptionSpec().

36
if (const auto *FnTy = Node.getType()->getAs<FunctionProtoType>())
  return FnTy->hasDynamicExceptionSpec();
return false;
62

I suspect that repeated calls to the GetBeginningOfToken method might be rather expensive. An alternative (and possibly more efficient) approach would be to re-lex the range from the start of the function declaration to the current location (in the forward direction) and then just operate on the array of tokens. See UseOverrideCheck.cpp, for example. If you decide to go that way, we could move the ParseTokens function to clang-tidy/utils/LexerUtils.{h,cpp}.

85

We usually add using namespace ast_matchers; to make matchers more readable.

111

I don't like this use of Tok as an alias of Lexer::Tok. It makes the code using it harder to understand.

124

Please remove the commented-out code or uncomment it, if it's needed.

157

The format of the message is not consistent with other messages. I'd suggest something along the lines of: "function '%0' uses dynamic exception specification '%1'; use '%2' instead".

clang-tidy/modernize/UseNoexceptCheck.h
36

As soon as a constructor starts doing something but just calling the base constructor (and sometimes before that), we usually move it to a .cpp file.

Please also move the storeOptions definition to the .cpp file.

This revision now requires changes to proceed.Apr 1 2016, 8:19 PM
aaron.ballman added inline comments.Apr 5 2016, 9:46 AM
clang-tidy/modernize/UseNoexceptCheck.cpp
17

Yes, please (along with tests and documentation, etc).

95

Can use auto here to not repeat the type name from the initializer.

149

I think all of this custom logic can be replaced by looking at the FunctionType. If it has a dynamic exception specification (hasDynamicExceptionSpec()) then whether it is throwing or not throwing (isNothrow()) determines whether you replace with noexcept(false) or noexcept. But then you don't have to go back to the source to see what is written.

I moved over the weekend, but will try to take a look at this tonight and address all your comments. Thanks again...

hintonda updated this revision to Diff 54022.Apr 17 2016, 6:54 PM
hintonda edited edge metadata.

Addressed most comments.

hintonda marked 9 inline comments as done.Apr 17 2016, 8:11 PM
hintonda added inline comments.
clang-tidy/modernize/UseNoexceptCheck.cpp
62

Still need to fix the call in UseOverrideCheck.cpp, but will wait till this has been reviewed.

132

I still need to parse it to find the range, so I'm not sure I'm not convinced the code will be any shorter/faster, but if there is a faster way, please let me know.

alexfh requested changes to this revision.Apr 18 2016, 6:37 AM
alexfh edited edge metadata.
alexfh added inline comments.
clang-tidy/modernize/UseNoexceptCheck.cpp
18

Please send a separate patch adding this matcher to ASTMatchers.h (with tests and docs).

74

http://llvm.org/docs/CodingStandards.html#do-not-use-braced-initializer-lists-to-call-a-constructor

Use the least powerful tool for the job: std::string Replacement = ReplacementStr;, this syntax makes it obvious that ReplacementStr is a std::string (or is implicitly converted to std::string).

Same elsewhere.

80
  1. Variable names should be CamelCase.
  2. It's more common to use I for array indices or iterators in LLVM.
109

Braces should be used consistently on both sides of else if.

115

CamelCase

117

Looks like Lexer::makeFileCharRange will do this job better.

122

Just FuncDecl should be enough.

clang-tidy/modernize/UseNoexceptCheck.h
22

Please enclose inline code snippets in backquotes.

docs/clang-tidy/checks/modernize-use-noexcept.rst
32

There's an alternative approach: the appropriate compatibility macro can be detected automatically using the Preprocessor::getLastMacroWithSpelling method. This approach will only work, if the header defining the macro is already included. However, if it's not included, the check must also add the corresponding #include statement to avoid breaking the code.

Also, if you detect the macro name automatically, you can provide a default macro by the means of the command line options (-extra-arg=-DNOEXCEPT=noexcept(false) or the same via the configuration file).

This revision now requires changes to proceed.Apr 18 2016, 6:37 AM
hintonda updated this revision to Diff 56511.May 7 2016, 7:57 PM
hintonda edited edge metadata.

Addressed additional comments.

hintonda updated this revision to Diff 56521.May 8 2016, 12:31 PM
hintonda edited edge metadata.
  • Fix typo in docs.
alexfh requested changes to this revision.May 11 2016, 9:55 AM
alexfh edited edge metadata.
alexfh added inline comments.
clang-tidy/modernize/UseNoexceptCheck.cpp
40

s/FunctionDecl/auto/

41

s/clang:://

50–51

These two seem to represent a SourceRange.

52

s/std::string/StringRef/

55–91

I'd pull this to a separate function to make the check() method easier to read.

94

Lexer::makeFileCharRange is a convenient way to ensure a range is a contiguous range in the same file.

This revision now requires changes to proceed.May 11 2016, 9:55 AM
hintonda updated this revision to Diff 56984.May 11 2016, 4:49 PM
hintonda edited edge metadata.

Created helper function and moved most logic there. Also address a
few comments.

alexfh requested changes to this revision.May 11 2016, 5:06 PM
alexfh edited edge metadata.
alexfh added inline comments.
clang-tidy/modernize/UseNoexceptCheck.cpp
37

The name "helper" doesn't say much. I'm sure we can come up with a much more useful name. The interface could also be more convenient. It looks like this function could return a FixItHint (which could possibly be isNull(), if no replacement is possible/needed). It could also use an Optional<FixItHint> or some other way to inform the caller that there's no action required.

This revision now requires changes to proceed.May 11 2016, 5:06 PM
hintonda added inline comments.May 11 2016, 5:15 PM
clang-tidy/modernize/UseNoexceptCheck.cpp
37

Happy to rename it, but not sure making it more convenient serves much of a purpose. For this checker, we either find a dynamic exception specification and produce a FixItHint, or we don't.

Did you have some other behavior you wanted me to add?

hintonda updated this revision to Diff 57065.May 12 2016, 10:21 AM
hintonda edited edge metadata.
  • Renamed private helper method
docs/ReleaseNotes.rst
152

Please highlight noexcept with ``.

docs/clang-tidy/checks/modernize-use-noexcept.rst
7

Please highlight throw() and other keywords with ``.

hintonda updated this revision to Diff 57070.EditedMay 12 2016, 10:44 AM
hintonda edited edge metadata.
  • Added ``'s around keywords in docs.
docs/clang-tidy/checks/modernize-use-noexcept.rst
33

Sorry, missed last time. Please use ` for check option and its value(s). Same for last line in file.

hintonda added inline comments.May 12 2016, 11:22 AM
docs/clang-tidy/checks/modernize-use-noexcept.rst
33

I'm not sure I understand what you want me to change. Did you want me to append that phrase, or replace something?

docs/clang-tidy/checks/modernize-use-noexcept.rst
33

Replace :option:`ReplacementString with :option:ReplacementString`. Same in last line with option and NOEXCEPT.

'' is for language constructs, ' is for command line options and their parameters.

hintonda added inline comments.May 12 2016, 11:31 AM
docs/clang-tidy/checks/modernize-use-noexcept.rst
33

Ah, got it. Sorry for being so dense... Will checkin shortly...

hintonda updated this revision to Diff 57078.May 12 2016, 11:38 AM
  • Fix quote problem in docs.
docs/clang-tidy/checks/modernize-use-noexcept.rst
51

Actually :option: still need to prepend ReplacementString.

hintonda updated this revision to Diff 57080.May 12 2016, 11:44 AM
  • Add :option: prefix for clarity.
docs/clang-tidy/checks/modernize-use-noexcept.rst
51

I'll get it eventually... ;-)

drive by

clang-tidy/modernize/UseNoexceptCheck.cpp
41

nit: const

98

nit: const auto FuncDecl*

115

I'm not sure, but I think you can replace

FuncDecl->getNameInfo().getAsString() by FuncDecl.

IIRC, there is a automatic conversion for the name.

121

FileMoveRange is a CharSourceRange, so why to you need to do getTokenRange?

CharSourceRange FileMoveRange;
clang-tidy/utils/LexerUtils.cpp
37

I like having lexer functions lifted out.
But 'ParseTokens' doesn't reflect that it stop when reaching a semicolon or a brace.

if (Tok.is(tok::semi) || Tok.is(tok::l_brace))
test/clang-tidy/modernize-use-noexcept.cpp
5

nits: space after A and B
class A {};

hintonda updated this revision to Diff 57097.May 12 2016, 1:59 PM
  • Addressed a few comments for clarity/readability.
hintonda added inline comments.May 12 2016, 2:05 PM
clang-tidy/modernize/UseNoexceptCheck.cpp
98

I originally had const *FuncDecl, but was told to use auto instead. I'll go ahead and make the change you suggest, but prefer to be explicit unless the type if hard to discern.

121

I'm probably missing something, but if I return FileMoveRange, i'm one token short, i.e., it doesn't cover the entire range I'm interested in.

However, if I extract begin and end, it works. I'll look into the difference, but if you have a better solution, please let me know.

clang-tidy/utils/LexerUtils.cpp
37

Not sure what you mean. Do you want me to change the name or add a comment to make that explicit?

etienneb added inline comments.May 12 2016, 2:16 PM
clang-tidy/modernize/UseNoexceptCheck.cpp
99

I'm not saying to use 'const *FuncDecl', but 'const auto*'

clang-tidy/utils/LexerUtils.cpp
37

If you call ParseTokens, it will parse tokens until it reach a semicolon/lbrace? exact?

hintonda added inline comments.May 12 2016, 2:31 PM
clang-tidy/modernize/UseNoexceptCheck.cpp
100

Sorry, I mis-typed. I meant to say I had, and prefer, "const FunctionDecl*" to "const auto *" in cases like this, but will do whatever you want.

clang-tidy/utils/LexerUtils.cpp
37

Yes, but again, I'm not sure what you are asking me to do/change.

Are you asking me to change something?

hintonda updated this revision to Diff 57103.May 12 2016, 2:49 PM

Regenerated diff to make sure I pick up all changed files.

hintonda added inline comments.May 12 2016, 2:53 PM
clang-tidy/utils/LexerUtils.cpp
37

I regenerated the diff -- the last few arc diff commands left out a few files.

Anyway, as you can see, I lifted the ParseTokens() functions out of UseOverrideCheck.cpp and moved it here.

aaron.ballman added inline comments.May 13 2016, 7:29 AM
clang-tidy/modernize/UseNoexceptCheck.h
45–46

What is the difference between these two fields? The names are kind of confusing to me given their similarity, so perhaps renaming one to be more descriptive would be useful.

hintonda added inline comments.May 13 2016, 8:02 AM
clang-tidy/modernize/UseNoexceptCheck.h
45–46

The first is the option passed in, i.e., the replacement string we will use (either noexcept or a user macro like libcxx's _NOEXCEPT) for throw() and the second one is the string we'll actually use for a particular replacement, which can be what the user supplied, or noexcept(false) when the function can throw.

This was originally done with an auto in check(), but Alex wanted me to add a helper function to make check() easier to understand. So, I had to put it somewhere. I actually prefer the simpler original version.

I'll see if I can come up with better names, but suggestions are always welcome.

alexfh requested changes to this revision.May 14 2016, 5:32 PM
alexfh edited edge metadata.
alexfh added inline comments.
clang-tidy/modernize/UseNoexceptCheck.cpp
38

Happy to rename it, but not sure making it more convenient serves much of a purpose.

Well, the initial problem was that the check method was too long and complicated. I suggested to make it easier to read by pulling a part of it in a separate method. I pointed to a part that seemed like something worth extracting to a method, and you mechanically created a method with unclear responsibilities, an awkward interface (two output parameters, a bool return value and a class data member) and a name that doesn't help understanding what the method does. That doesn't make the code any better. Let's try once again. We need to pull out an abstraction (at least one, but maybe you see more) that will make sense on its own and will have a reasonable interface. It could be SourceRange findDynamicExceptionSpecification(ArrayRef<Token> Tokens), for example.

122

I think, the problem is in the way you assign MoveRange in the checkHelper method. It is created as a character range and then you just assign its begin and end location. Instead, try MoveRange = CharSourceRange::getTokenRange(I->getLocation(), Loc).

clang-tidy/modernize/UseNoexceptCheck.h
45–46

The StringRef Replacement; should not be a member, since it's just one of the results of the checkHelper method.

This revision now requires changes to proceed.May 14 2016, 5:32 PM

I agree -- things got messy.

I'm in the process of completely refactoring the whole thing -- including the ast matchers that I'll checkin later today. Thanks for your patience.

hintonda updated this revision to Diff 57298.May 15 2016, 9:38 AM
hintonda edited edge metadata.
  • Complete refactor
hintonda updated this revision to Diff 57299.May 15 2016, 10:15 AM
hintonda edited edge metadata.
  • Make helper functions static.
hintonda updated this revision to Diff 57301.May 15 2016, 11:16 AM
  • Added asserts for indexes into Tokens array.
hintonda updated this revision to Diff 57305.May 15 2016, 12:44 PM
  • A bit more refactoring and cleanup.

I think this is now ready for review.

Thanks again for your patience...

etienneb added inline comments.May 15 2016, 5:40 PM
clang-tidy/modernize/UseNoexceptCheck.cpp
23

coding style:
MakeDynamicExceptionString -> makeDynamicExceptionString

35

MakeMoveRange -> makeMoveRange

(and all other occurrences below)

hintonda updated this revision to Diff 57315.May 15 2016, 8:51 PM
  • Fix some function names.
hintonda updated this revision to Diff 57357.May 16 2016, 9:11 AM
  • Revert back to previous version of the matcher and use Aaron's syntax with allOf and unless to achieve the same results -- much simpler.
aaron.ballman added inline comments.May 16 2016, 1:39 PM
clang-tidy/modernize/UseNoexceptCheck.cpp
22

Instead of a bunch of static functions, would an unnamed namespace make more sense?

35

Since this function is only called one time and is a one-liner, perhaps it makes more sense to inline the body at the call site?

47

Pathologically terrible counter-case: void func() throw(decltype(throw 12) *)

50

Can we use >= 0 instead of != -1? It makes it more immediately obvious that the array index will not underflow.

101

Please don't use auto here; I have no idea what type FileMoveRange will have from inspecting the code.

109

No need to quote %0, the diagnostics engine will already do it when passed a NamedDecl, so this will improperly double-quote the diagnostic.

clang-tidy/modernize/UseNoexceptCheck.h
23

I think this comment means noexcept(false) instead? Or is there a reason to replace with noexcept(true) instead of just noexcept?

test/clang-tidy/modernize-use-noexcept-macro.cpp
12

I may have missed some context in the discussion, but why shouldn't this trigger a FixItHint?

hintonda added inline comments.May 16 2016, 1:52 PM
clang-tidy/modernize/UseNoexceptCheck.cpp
22

Just following the pattern established in other checkers so as to minimize the number of changes, but will change to namespace.

35

Was a bit more involved, but last round of changes simplified it. Will simplify even more.

47

Good point, looks like I need a full fledged parser to catch 100% or the cases -- or we could ignore these corner cases.

clang-tidy/modernize/UseNoexceptCheck.h
23

Typo, should just be noexcept.

aaron.ballman added inline comments.May 16 2016, 2:14 PM
clang-tidy/modernize/UseNoexceptCheck.cpp
22

I don't have strong opinions; I think the usual rule of thumb boils down to whether the code fits on a page or not (if it does, use an unnamed namespace, if not, use static functions).

47

At the very least, we should have test cases showing what the behavior is with a big FIXME around this code, should you decide to keep it. I'm not keen on the idea of this being part of a fixit that may destroy well-defined user code. Same for the assumptions about the location of right parens. That code looks equally broken even without multiple throw tokens in the stream. Consider:
void func() throw(int(int));

hintonda added inline comments.May 16 2016, 2:26 PM
clang-tidy/modernize/UseNoexceptCheck.cpp
47

Had thought about adding paren parsing, but wasn't sure it was needed -- thanks for pointing out that it is. Of course, I'll have to parse from the beginning to do this correctly, but that's not a big deal.

50

Sure.

test/clang-tidy/modernize-use-noexcept-macro.cpp
12

Because the user provided a macro. The only reason you would do that is if you want the macro to expand to something different depending on whether or not 'noexcept' is supported.

Perhaps this can be done more elegantly, but the use case for this entire checker was libcxx. It defines _NOEXCEPT as either noexcept or throw() depending on whether or not noexcept is supported. I don't see a good way of doing that, other than removing it completely, so I just reported it without supplying a FixItHint.

aaron.ballman added inline comments.May 16 2016, 2:29 PM
test/clang-tidy/modernize-use-noexcept-macro.cpp
12

Ah, thank you for the context! That information should probably go in the user facing documentation somewhere.

hintonda updated this revision to Diff 57432.May 16 2016, 9:31 PM
  • Address additional comments.
aaron.ballman added inline comments.May 17 2016, 6:11 AM
clang-tidy/modernize/UseNoexceptCheck.cpp
40

@alexfh, what are your feelings on this FIXME? I am a bit concerned because these examples will cause the replacement range to be incorrect, which will turn working code into ill-formed code. The alternative, as I see it, is to instead properly track the exception specification source range information as part of the FunctionDecl (akin to FunctionDecl::getReturnTypeSourceRange()).

docs/clang-tidy/checks/modernize-use-noexcept.rst
41

This seems to run contrary to one of the examples below where a fixit is provided for `throw(A, B)`. If I understood properly, this statement is only true when using a ReplacementString. Perhaps it should instead say, "this check will detect, but not
provide a FixItHint for this case when a :option:ReplacementString is provided."

hintonda added inline comments.May 17 2016, 10:47 AM
clang-tidy/modernize/UseNoexceptCheck.cpp
40

Btw, I'm working on a fix I believe will handle all cases -- plan to checkin later today. However, it won't be that efficient unless I can find a way to match params that contain dynamic exception specifications. If they are only legal for function pointers -- which I think is the case -- that would make it easy and efficient, i.e., I wouldn't have to match all FunctionDecl's with one or more parameter and test them.

Is it possible to match a parameter that is a function pointer?

docs/clang-tidy/checks/modernize-use-noexcept.rst
41

This is only applicable when the user provided a replacement string other than noexcept. I'll try to make it clearer, however, there is a FIXME in the code concerning this. Specifically, should we allow replacement options for both noexcept and noexcept(false).

alexfh added inline comments.May 17 2016, 2:35 PM
clang-tidy/modernize/UseNoexceptCheck.cpp
23

It's much more common for LLVM code to use static functions:

http://llvm.org/docs/CodingStandards.html#anonymous-namespaces
"make anonymous namespaces as small as possible, and only use them for class declarations"

40

The alternative, as I see it, is to instead properly track the exception specification source range information as part of the FunctionDecl (akin to FunctionDecl::getReturnTypeSourceRange()).

It's all trade-offs: adding this information to the AST allows certain tasks in tooling to be done easier. On the other hand, this leads to bloating of the AST, which can already be huge. In this specific case, always keeping the source range of the exception specifier might be wasteful, since exception specifiers are rather rare (in my world, at least). But there might be a way to optionally store this information, e.g. in the ASTContext. In any case, makes sense to ask Richard Smith.

aaron.ballman added inline comments.May 17 2016, 2:41 PM
clang-tidy/modernize/UseNoexceptCheck.cpp
40

I am more curious as to your comfort level about committing a fixit that we know will break working code. ;-)

As for the suggested approach, I would obviously cover that with Richard, but I think we're going to need exception specifications more now that they're going to be part of the type system for C++. We have FunctionProtoTypeLoc for tracking this information.

alexfh added inline comments.May 17 2016, 3:24 PM
clang-tidy/modernize/UseNoexceptCheck.cpp
40

The important question here is how frequently is this likely to happen. I guess, it depends on the codebase, but maybe you have a rough guess?

aaron.ballman added inline comments.May 17 2016, 3:45 PM
clang-tidy/modernize/UseNoexceptCheck.cpp
40

Multiple uses of throw within a dynamic exception specification? Rare. Paren use that doesn't match what we expect? Infrequent, but likely to generate a bug report at some point depending on how often people use clang-tidy to check extensive code bases that actually *use* dynamic exception specifications. So my biggest concern really boils down to the fact that we're assuming the first right paren we come to after a "throw" keyword in a dynamic exception specification is the terminating right paren. I think we need to use paren matching instead. It would be nice if we could use BalancedDelimiterTracker to solve this, come to think of it.

alexfh added inline comments.May 17 2016, 3:54 PM
clang-tidy/modernize/UseNoexceptCheck.cpp
40

Yes, tracking balanced parenthesis sequences makes sense in any case.

hintonda updated this revision to Diff 57547.May 17 2016, 6:08 PM
  • First cut on a simple parser for decls.

Successfully parses all the examples I've been given so far. Please
help me break it.

hintonda updated this revision to Diff 57555.May 17 2016, 7:42 PM
  • Added another test.
hintonda updated this revision to Diff 57686.May 18 2016, 3:25 PM

Improved matcher logic and add better range handling to try to deal
with multiple asserts concerning bad ranges when running checker again
real code, e.g., libcxx.

Even so, still seeing some asserts in Lexer::getSourceLocation(), e.g.:

Assertion failed: (Loc >= BufferStart && Loc <= BufferEnd &&
"Location out of range for this buffer!"), function
getSourceLocation, file
/Users/dhinton/projects/clang/llvm/tools/clang/lib/Lex/Lexer.cpp, l
ine 1073.

in a call to Sources.isBeforeInTranslationUnit(Range.getEnd(),
Tok.getLocation()) in parseDeclTokens().

Looks like my matcher is returning stuff that doesn't parse correctly.
Investigating.

hintonda updated this revision to Diff 57811.May 19 2016, 9:17 AM

Adjust matcher, add better error checking, and additional test cases.

We can now parse all dynamic exception specifications, but haven't
been able to develop a matcher that will find the following function
declaration without finding other nodes that aren't parsable.

void g(void (*fp)(void) throw());

Btw, this version can successfully check libcxx.

alexfh requested changes to this revision.May 20 2016, 2:40 AM
alexfh edited edge metadata.
This revision now requires changes to proceed.May 20 2016, 2:40 AM
hintonda updated this revision to Diff 58151.May 23 2016, 2:19 PM
hintonda edited edge metadata.

Fixed matcher -- added 'unless(isImplicit())'. Thanks to Aaron Ballman for the suggestion.

Actually, this will never work correctly -- in fact, raw lexing will always be problematic. Consider:

void foo()
#if !__has_feature(cxx_noexcept)

throw(std::bad_alloc)

#endif
{}

In this case we *could* figure out if __has_feature(cxx_noexcept) evaluated to true or false, but what if it was a macro instead? We can't reliably handle it without reparsing the whole thing.

I'll leave it as is for now, but once http://reviews.llvm.org/D20428 is available, I'll start a new diff using it.

Please see D20693 for an alternative approach.

alexfh resigned from this revision.Jun 3 2016, 3:13 PM
alexfh removed a reviewer: alexfh.

Cleaning up my Phab dashboard. If http://reviews.llvm.org/D20428 doesn't happen and we'll have to return to this implementation, please re-add me to the reviewers.

hintonda abandoned this revision.Jun 3 2016, 3:17 PM

This revision is OBE.