Skip to content

Only allow the Checked C extension flag for C. #16

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 7, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions docs/checkedc/Test-Baselines.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Test base lines

## Current baseline
## Testing baseline for the base line branch


Here is the current baseline for testing just clang with the x86 target (using the check-clang project)
Expand Down Expand Up @@ -52,7 +52,14 @@ Here is the current base line for testing LLVM + clang on all targets (check-all
Unexpected Failures: 6
```

## Differences in test results between branches

The master branch in the checkedc-clang repo has additional tests for
Checked C. It is expected that the master branch will have more
`Expected Passes` than the baseline branch.

## In-progress baseline updates

This section records the test results for an in-progress update to latest sources in the baseline branch. It is currently empty because
no update is in-progress.
This section records the test results for an in-progress update to latest
sources in the baseline branch. It is currently empty because no update
is in-progress.
15 changes: 9 additions & 6 deletions docs/checkedc/Testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ target to the build system for running the test suite: check-checkedc.

The Checked C version of clang/LLVM should pass the same tests as the baseline version
of clang/LLVM (in the baseline branch) and the Checked C specific tests. The testing
results for the baseline branch are recorded in [testing baselines](Test-Baselines.md)
results for the baseline branch are recorded in [testing baselines](Test-Baselines.md).
A developer should confirm that no new unexpected failures occur as a result of a change
before committing a change.

When testing a change, the testing should be sufficient for the type of change. For changes
to parsing and typechecking, it is usually sufficient to pass the Checked C and clang tests.
Expand Down Expand Up @@ -49,8 +51,9 @@ The clang-specific documentation on running tests appears to be out-of-date, so
llvm-lit d:\autobahn1\llvm\tools\clang\test

## Test baselines
We have observed a few tests fail on Windows on a clean LLVM/clang enlistment that don't seem to fail on the buildbots.
We have not tracked down the source of the failures. For now, we are using [testing baselines](Test-Baselines.md) to exclude these
tests. LLVM/clang have an optimistic check-in policy, so it is possible that a few tests may fail in the
main-line repos when we update to the latest sources.

We have observed a few tests fail unexpectedly on Windows on a clean LLVM/clang
enlistment. These tests don't seem to fail on the buildbots. We have not
tracked down the source of the failures. For now, we are using
[testing baselines](Test-Baselines.md) to exclude these tests. LLVM/clang has
an optimistic check-in policy, so it is possible that a few tests may fail in
the main-line repos when we update to the latest sources.
16 changes: 11 additions & 5 deletions docs/checkedc/Update-to-latest-LLVM-sources.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,12 @@ Then do

## Update the baseline branch on Github

You will then need to build and run tests to establish test baselines. Assuming that the tests results are good, you can push them to your personal
Github forks:
You will then need to build and run tests to establish test baselines. Assuming that the tests results are good,
you can push them to your personal Github forks:

git push origin baseline

You can then issue a pull request to pull the changes into the mainline change.
You can then issue pull requests to pull the changes into the Microsift Github repos.

## Update the master branch.

Expand All @@ -58,6 +58,12 @@ After you have updated the baseline branch, you can update the master branch. Ch
git checkout master
git merge baseline

Then set up the build system and compile. Fix any issues that you encounter. Then run tests. Once you are passing the same
testing as the baseline branch, push this up to personal Github fork and issue a pull request.
Set up the build system and compile. Fix any issues that you encounter.

Then run tests. We have added tests for Checked C to the clang master branch, so these additional tests need to be taken
into account during testing. Make sure the code passes the following tests:

- The same tests as the baseline branch, _plus_ the Checked C specific tests for clang in the master branch.
- The Checked C languages tests for the Checked C project.

Once the tests are passing, push the changes up to a personal Github fork and issue a pull request.
23 changes: 22 additions & 1 deletion lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1701,7 +1701,28 @@ static void ParseLangArgs(LangOptions &Opts, ArgList &Args, InputKind IK,
Opts.PascalStrings = Args.hasArg(OPT_fpascal_strings);
Opts.VtorDispMode = getLastArgIntValue(Args, OPT_vtordisp_mode_EQ, 1, Diags);
Opts.Borland = Args.hasArg(OPT_fborland_extensions);
Opts.CheckedC = Args.hasArg(OPT_fcheckedc_extension);
if (Args.hasArg(OPT_fcheckedc_extension)) {
std::string disallowed;
if (Opts.CUDA)
disallowed = "CUDA";
else if (Opts.OpenCL)
disallowed = "OpenCL";
else if (Opts.ObjC1 || Opts.ObjC2) {
if (Opts.CPlusPlus)
disallowed = "Objective C/C++";
else
disallowed = "Objective C";
}
else if (Opts.CPlusPlus) {
disallowed = "C++";
}

if (disallowed.size() > 0) {
Diags.Report(diag::err_drv_argument_not_allowed_with) <<
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this end up being fatal? Or just a warning? Or will it just not enable the Checked C Extensions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ends up being fatal.

"-fcheckedc-extension" << disallowed;
} else
Opts.CheckedC = true;
}
Opts.WritableStrings = Args.hasArg(OPT_fwritable_strings);
Opts.ConstStrings = Args.hasFlag(OPT_fconst_strings, OPT_fno_const_strings,
Opts.ConstStrings);
Expand Down
14 changes: 14 additions & 0 deletions test/Driver/checkedc-notsupported.cl
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Checked C extension is not supported for OpenCL. Make sure driver
// rejects the flag.
//
// RUN: not %clang -fcheckedc-extension %s 2>&1 | FileCheck %s
// CHECK: error: invalid argument '-fcheckedc-extension' not allowed with 'OpenCL'
//
// Have clang compile this file as a C file.
// RUN: %clang -c -fcheckedc-extension -x c %s
//
// Have clang-cl compile this file as a C file.
// RUN: %clang_cl -c -Xclang -fcheckedc-extension /TC %s

extern void f() {}

15 changes: 15 additions & 0 deletions test/Driver/checkedc-notsupported.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Checked C extension is not supported for C++. Make sure driver
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably low priority, but we could also add a test case for a .c file that is forced to be compiled as C++ using command-line options. That's a pattern I've certainly seen before.

Copy link
Member Author

@dtarditi dtarditi Jul 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added test cases forcing files to be compiled as different file types than the file extension specifies, with the Checked C extension flag enabled:

  • I added a new test with a C file type. It is compiled as a regular C file and also as a C++ file.. I also added run lines where the C file type is compiled as several other language types.
  • I added run lines where several file types are forced to be compiled as C files..
  • Finally, I also added tests for clang-cl involving the Microsoft command-line options /TP and /TC for compiling files as C++ and C files respectively.

// rejects the flag.
//
// RUN: not %clang -fcheckedc-extension %s 2>&1 | FileCheck %s
// CHECK: error: invalid argument '-fcheckedc-extension' not allowed with 'C++'
//
// Have clang compile this file as a C file.
// RUN: %clang -c -fcheckedc-extension -x c %s
//
// Have clang-cl compile this file as a C file.
// RUN: %clang_cl -c -Xclang -fcheckedc-extension /TC %s

extern void f() {}


15 changes: 15 additions & 0 deletions test/Driver/checkedc-notsupported.cu
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Checked C extension is not supported for CUDA. Make sure driver
// rejects the flag.
//
// RUN: not %clang -fcheckedc-extension %s 2>&1 | FileCheck %s
// CHECK: error: invalid argument '-fcheckedc-extension' not allowed with 'CUDA'
//
// Have clang compile this file as a C file.
// RUN: %clang -c -fcheckedc-extension -x c %s
//
// Have clang-cl compile this file as a C file.
// RUN: %clang_cl -c -Xclang -fcheckedc-extension /TC %s

extern void f() {}


15 changes: 15 additions & 0 deletions test/Driver/checkedc-notsupported.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Checked C extension is not supported for Objective C. Make sure driver
// rejects the flag.
//
// RUN: not %clang -fcheckedc-extension %s 2>&1 | FileCheck %s
// CHECK: error: invalid argument '-fcheckedc-extension' not allowed with 'Objective C'
//
// Have clang compile this file as a C file.
// RUN: %clang -c -fcheckedc-extension -x c %s
//
// Have clang-cl compile this file as a C file.
// RUN: %clang_cl -c -Xclang -fcheckedc-extension /TC %s

extern void f() {}


35 changes: 35 additions & 0 deletions test/Driver/checkedc-supported.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// Checked C extension is supported for C. Make sure driver
// accepts the flag for C and rejects it when the file is
// compiled as another language.
//
// RUN: %clang -c -fcheckedc-extension %s
// RUN: %clang_cl -c -Xclang -fcheckedc-extension %s
//
// Have clang compile this file as C++ file.
// RUN: not %clang -c -fcheckedc-extension -x c++ %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=check-cpp
// check-cpp: error: invalid argument '-fcheckedc-extension' not allowed with 'C++'
//
// Have clang-cl compile this file as a C++ file.
// RUN: not %clang_cl -c -Xclang -fcheckedc-extension /TP %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=clcheck-cpp
// clcheck-cpp: error: invalid argument '-fcheckedc-extension' not allowed with 'C++'
//
// RUN: not %clang -c -fcheckedc-extension -x cuda %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=check-cuda
// check-cuda: error: invalid argument '-fcheckedc-extension' not allowed with 'CUDA'
//
// RUN: not %clang -c -fcheckedc-extension -x cl %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=check-opencl
// check-opencl: error: invalid argument '-fcheckedc-extension' not allowed with 'OpenCL'
//
// RUN: not %clang -c -fcheckedc-extension -x objective-c %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=check-objc
// check-objc: error: invalid argument '-fcheckedc-extension' not allowed with 'Objective C'
//
// RUN: not %clang -c -fcheckedc-extension -x objective-c++ %s 2>&1 \
// RUN: | FileCheck %s -check-prefix=check-objcpp
// check-objcpp: error: invalid argument '-fcheckedc-extension' not allowed with 'Objective C/C++'


extern void f(ptr<int> p) {}