[cfe-commits] Patch for bug 14744

Dmitri Gribenko gribozavr at gmail.com
Tue Jan 1 15:43:55 PST 2013


On Wed, Jan 2, 2013 at 12:54 AM, Krzysztof Parzyszek
<kparzysz at codeaurora.org> wrote:
> On 1/1/2013 4:28 PM, Dmitri Gribenko wrote:
>>
>> On Wed, Jan 2, 2013 at 12:07 AM, Krzysztof Parzyszek
>> <kparzysz at codeaurora.org> wrote:
>>>
>>> I think I sent it to the wrong list the first time.
>>> Could someone review and commit if ok?
>>
>>
>> Is it possible to add a test for this?
>
> Certainly.  Not sure if it's very meaningful though.  Let me know what you
> think.

Should this test include
// REQUIRES: hexagon-registered-target
so that it is properly disabled when hexagon target is not compiled?

IIRC, a translation unit without an external definitions is not valid.

This test could also have a better name -- something about "simple-tu".

Although this seems trivial, I don't think I can LGTM this patch. (I
don't know about Hexagon a lot)

In general we don't have -c tests, except for
tools/clang/test/CodeGenCXX/vtable-debug-info.cpp.  This test
should've caught this bug, shouldn't it?

It might be a good idea to have one 'end-to-end' test for each
supported target, but this needs discussion.  (I remember that such
tests were previously rejected because 'test-suite' is the place for
end-to-end tests.)

Dmitri

-- 
main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
(j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/



More information about the cfe-commits mailing list