hintonda added inline comments.
================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:46
@@ +45,3 @@
+ const SmallVector<Token, 16> &Tokens) {
+ // Find throw token -- it's a keyword, so there can't be more than one.
Also,
+ // it should be near the end of the declaration, so search from the end.
----------------
aaron.ballman wrote:
> hintonda wrote:
> > aaron.ballman wrote:
> > > Pathologically terrible counter-case: `void func() throw(decltype(throw
> > > 12) *)`
> > Good point, looks like I need a full fledged parser to catch 100% or the
> > cases -- or we could ignore these corner cases.
> 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));`
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.
================
Comment at: clang-tidy/modernize/UseNoexceptCheck.cpp:49
@@ +48,3 @@
+ int TokenIndex;
+ for (TokenIndex = Tokens.size() - 1; TokenIndex != -1; --TokenIndex) {
+ if (Tokens[TokenIndex].is(tok::kw_throw))
----------------
aaron.ballman wrote:
> Can we use `>= 0` instead of `!= -1`? It makes it more immediately obvious
> that the array index will not underflow.
Sure.
================
Comment at: test/clang-tidy/modernize-use-noexcept-macro.cpp:11
@@ +10,3 @@
+
+// Should not trigger a FixItHint
+class A {};
----------------
aaron.ballman wrote:
> I may have missed some context in the discussion, but why shouldn't this
> trigger a FixItHint?
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.
http://reviews.llvm.org/D18575
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
