[cfe-dev] clang-format: confused C style cast in template, moving AnnotatingParser and add unit tests?

[Daniel Jasper]

On Sun, Sep 21, 2014 at 7:33 PM, Jean-philippe Dufraigne <
j.dufraigne at gmail.com> wrote:

> Hello Daniel,
>
> I was looking at a formatting issue with "std::function<void(int, int)>
> callback;". "(int, int)" is understood as a C-style cast and so the spacing
> follow this understanding.
>
> While trying to address this, I found that the existing test for member
> function reference qualifier relied on the confusion. it results in
> inconsistent format when a name is added thought (see last 2 lines of my
> test function)
>
> This is not really dependent on the different style, and it would seem
> unit test for AnnotatingParser would help when updating it.
>
> Would it make sense to move AnnotatingParser in its own files and then
> start adding unit tests for it?
>

I don't think so. I like the unit tests as they are, testing the format of
small snippets of actual code. Directly testing the AnnotatingParser feels
a bit like testing implementation details. Also, the test you have written
shows that you can test exactly what you want to test with the existing
test infrastructure, or am I missing something?

I would think that we would want to split the class in its .h and .cpp file.
> I'd be happy to provide a patch for this, though reviewing this kind of
> changes is often tricky as diffs can be unhelpful, do you have any
> preference on how it is done?
>
> The test function that shows the issues:
>
> TEST_F(FormatTest, ConfigurableSpacesInParenthesesTryThings) {
>   FormatStyle Spaces = getLLVMStyle();
>
>   Spaces.SpacesInParentheses = true;
>   verifyFormat("void function( int, int );", Spaces);
>   verifyFormat("std::function<void(int, int)> callback;", Spaces); //
> inconsistent
>   verifyFormat("#define x ( (int)-1 )", Spaces);
>
>   Spaces.SpacesInParentheses = false;
>   Spaces.SpacesInCStyleCastParentheses = true;
>   verifyFormat("void function(int, int);", Spaces);
>   verifyFormat("std::function<void( int, int )> callback;", Spaces); //
> inconsistent
>   verifyFormat("#define x (( int )-1)", Spaces);
>
>   verifyFormat("Deleted &operator=(const Deleted &)& = default;");
>   verifyFormat("Deleted &operator=(const Deleted &other) & = default;");
> // inconsitent
> }
>
> Thanks,
> JP
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140921/ce428180/attachment.html>