clean up tests without calls to clang #348

kyleheadley · 2020-12-08T17:18:32Z

I added calls to clang in tests returned by grep -L "clang" *, or an explanation of why it cannot be done at this time. I also removed some dashes as requested in #346 while there, and removed commented lines in lit.local.cfg since they referred to a private file system. Post-merge tests were run on my local Mac.

There were some files that I did not (yet) change:

lowerboundBUG.c & lowerbound.c
- these are identical and don't fail as indicated by comments inside
basic_local_ntarr.c
- variable c is commented as different from the test script, and bounds don't match
basic_inter_field_ntarr.c
- clang complains about an incompatible type

kyleheadley · 2020-12-09T18:09:44Z

I needed to reorder some statements in arrboundsadvanced.c because FL was declared before FooLen which was used in its itype. Is this a change our users will need to do manually or is it an error that needs its own issue?

mwhicks1 · 2020-12-09T19:15:21Z

I needed to reorder some statements in arrboundsadvanced.c because FL was declared before FooLen which was used in its itype. Is this a change our users will need to do manually or is it an error that needs its own issue?

Yes, users will have to make this change manually. It's a "feature" of 3C to infer the length even for variables that are defined in the wrong order. Fine to move them in this case.

mwhicks1 · 2020-12-09T19:17:04Z

There were some files that I did not (yet) change:

lowerboundBUG.c & lowerbound.c

these are identical and don't fail as indicated by comments inside

We should delete lowerboundBUG.c and if the test is expected to failure, we can include it in the expected failures list (is that the case already?).

basic_local_ntarr.c

variable c is commented as different from the test script, and bounds don't match

Is this another test that we are expecting to fail?

basic_inter_field_ntarr.c

clang complains about an incompatible type

Same question? Perhaps make an issue for addressing/fixing these tests.

mwhicks1 · 2020-12-09T19:18:33Z

clang/test/3C/allarrays.c

@@ -1,4 +1,5 @@
-// RUN: 3c -alltypes %s -- | FileCheck -match-full-lines --check-prefixes="CHECK" %s
+// RUN: 3c -alltypes %s | FileCheck -match-full-lines --check-prefixes="CHECK" %s
+// RUN: 3c -alltypes %s | %clang -c -f3c-tool -fcheckedc-extension -x c -o /dev/null -


I think we discussed dropping use of /dev/null entirely and using %t instead? Doing so is to enhance Windows compatibility. This way, we would be indicating this is a temp file, it will get removed by FileCheck automatically, right?

I didn't realize it was more than a bugfix when piping on windows. But this seems reasonable. I'll change it and add it to the other list of test changes.

I don't think /dev/null is a problem here. llvm-lit is supposed to automatically substitute temp files for /dev/null on Windows, but was not doing this correctly when dealing with pipes and io redirection.

mwhicks1 · 2020-12-09T19:20:16Z

clang/test/3C/checkedregionvoid.c

+// RUN: 3c -addcr %s | FileCheck -match-full-lines --check-prefixes="CHECK" %s
+
+// Currently not running clang on the output,
+// since it cannot find "dummy.h", even with -L%S


There is no dummy.h in the test directory? If that's right, I think we should delete its usage.

There is a dummy.h in the test directory, required for the test as it is. I couldn't get clang to recognize it from the testing harness.

If it's a problem then we should just get rid of it. I don't see the benefit of having it when you can just inline its contents in the test.

I removed it

mwhicks1

Question and comment; might require small changes.

mattmccutchen-cci · 2020-12-10T18:29:40Z

@kyleheadley I hope you're OK with me editing your PR title so people aren't misled that it only adds clang calls. Feel free to edit it further.

mattmccutchen-cci · 2020-12-10T18:41:45Z

@kyleheadley Please follow these instructions to ensure that all your future commits for CCI get your CCI email address.

If it were me, I would go ahead and fix this PR branch too via the "Using Interactive Rebase" method here and a force-push, but since the default branch already has a few commits from you with the other email address and it's impractical to do anything about them now, I don't feel strongly about adding a few more.

kyleheadley · 2020-12-10T19:27:52Z

after using -f3c-tool, the files I highlighted above compile. The reasons were to do with array bounds, though not directly, and they were older tests, so I've rolled them into the rest and dealt with them.

I also updated for the recent comments here and changes in #346. This is ready for review again.

mwhicks1 · 2020-12-15T21:26:16Z

clang/test/3C/allarrays.c

@@ -1,5 +1,5 @@
 // RUN: 3c -alltypes %s | FileCheck -match-full-lines --check-prefixes="CHECK" %s
-// RUN: 3c -alltypes %s | %clang -c -f3c-tool -fcheckedc-extension -x c -o /dev/null -
+// RUN: 3c -alltypes %s | %clang -c -f3c-tool -fcheckedc-extension -x c -o %t1.unused -


Why do you need the .unused bit at the end? I presume you've confirmed that this file is deleted after all the tests are run?

I don't need it, but I wanted an indication of what this particular temp file is used for.

mwhicks1

Looks good! Merge when ready (be sure to use your correctcomputation.com address).

add clang call in tests, or reason not to

ab598df

kyleheadley requested a review from john-h-kastner December 8, 2020 17:18

kyleheadley added the testing label Dec 8, 2020

add clang with -f3c-tool to tests

9bcc559

mwhicks1 reviewed Dec 9, 2020

View reviewed changes

mattmccutchen-cci changed the title ~~add clang call in tests~~ add clang call in tests (and more) Dec 10, 2020

kyleheadley added 2 commits December 10, 2020 13:53

another round ot test cleanup - dev/null, f3c-tool, xfail

addfe37

Merge branch 'master-post-microsoft' into addclang

c0a8bc2

kyleheadley changed the title ~~add clang call in tests (and more)~~ clean up tests without calls to clang Dec 10, 2020

mwhicks1 reviewed Dec 15, 2020

View reviewed changes

mwhicks1 approved these changes Dec 15, 2020

View reviewed changes

Merge branch 'master-post-microsoft' into addclang

ea794d6

kyleheadley merged commit 531e239 into correctcomputation:master-post-microsoft Dec 16, 2020

kyleheadley deleted the addclang branch December 16, 2020 16:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

clean up tests without calls to clang #348

clean up tests without calls to clang #348

kyleheadley commented Dec 8, 2020

kyleheadley commented Dec 9, 2020

mwhicks1 commented Dec 9, 2020

mwhicks1 commented Dec 9, 2020

mwhicks1 Dec 9, 2020

kyleheadley Dec 9, 2020

john-h-kastner Dec 9, 2020

mwhicks1 Dec 9, 2020

kyleheadley Dec 9, 2020

mwhicks1 Dec 9, 2020

kyleheadley Dec 10, 2020

mwhicks1 left a comment

mattmccutchen-cci commented Dec 10, 2020

mattmccutchen-cci commented Dec 10, 2020

kyleheadley commented Dec 10, 2020

mwhicks1 Dec 15, 2020

kyleheadley Dec 16, 2020

mwhicks1 left a comment

clean up tests without calls to clang #348

clean up tests without calls to clang #348

Conversation

kyleheadley commented Dec 8, 2020

kyleheadley commented Dec 9, 2020

mwhicks1 commented Dec 9, 2020

mwhicks1 commented Dec 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwhicks1 left a comment

Choose a reason for hiding this comment

mattmccutchen-cci commented Dec 10, 2020

mattmccutchen-cci commented Dec 10, 2020

kyleheadley commented Dec 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwhicks1 left a comment

Choose a reason for hiding this comment