[PATCH] D39122: [Sema] Fixes for enum handling for tautological comparison diagnostics

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Oct 21 09:30:47 PDT 2017


aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!



================
Comment at: lib/Sema/SemaChecking.cpp:8619
+  if (OtherRange.Width == 0)
+    return Value == 0 ? LimitType::Both : llvm::Optional<LimitType>();
+
----------------
lebedev.ri wrote:
> aaron.ballman wrote:
> > Instead of default constructing the Optional, you should use `llvm::None` instead.
> Thank you for the suggestion, i did not really know about `llvm::None`.
> However only the last `return` can be enhanced:
> ```
> clang/lib/Sema/SemaChecking.cpp:8619:23: error: incompatible operand types ('(anonymous namespace)::LimitType' and 'llvm::NoneType')
>     return Value == 0 ? LimitType::Both : llvm::None;
>                       ^ ~~~~~~~~~~~~~~~   ~~~~~~~~~~
> ```
> 
Blech, that's because there's no common type for the `:?` operator. I think it'd be worse to split this into multiple statements just to use `llvm::None`, so it's fine as-is.


Repository:
  rL LLVM

https://reviews.llvm.org/D39122





More information about the cfe-commits mailing list