[PATCH] D28081: Make GetStyle return Expected<FormatStyle> instead of FormatStyle

Antonio Maiorano via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 2 19:54:45 PST 2017


amaiorano added inline comments.


================
Comment at: unittests/Format/FormatTestObjC.cpp:72
 TEST_F(FormatTestObjC, DetectsObjCInHeaders) {
-  Style = getStyle("LLVM", "a.h", "none", "@interface\n"
+  Style = *getStyle("LLVM", "a.h", "none", "@interface\n"
                                           "- (id)init;");
----------------
amaiorano wrote:
> ioeric wrote:
> > amaiorano wrote:
> > > In these tests, I'm assuming getStyle returns a valid FormatSyle. I could add the same types of validation as in the FormatStyle.GetStyleOfFile tests.
> > Please add proper checking as above for returned values.
> Hmm, so I could replace the Style member of the fixture class with Expected<FormatStyle>, and then change all "Style." with "Style->" in the rest of the test file, or only in this specific test, I could store the result in a local Expected<FormatStyle>, check that it's valid, and then assign to Style. The latter is simpler; only question I have is how to name the local variable - can I go with StyleOrError? Style2?
Another option here is to make this a non-fixture TEST and just declare a local Expected<FormatStyle> Style for this specific test, which would work fine.


https://reviews.llvm.org/D28081





More information about the cfe-commits mailing list