Skip to content

3C code style pass 2021-02 #462

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 2 commits into from
Mar 1, 2021
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
9 changes: 6 additions & 3 deletions clang-tools-extra/clangd/3CCommands.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,17 @@ void as3CCommands(const Diagnostic &D, std::vector<Command> &OutCommands) {
}

bool is3CCommand(const ExecuteCommandParams &Params) {
return (Params.command.rfind(std::string(Command::_3C_APPLY_ONLY_FOR_THIS), 0) == 0) ||
(Params.command.rfind(std::string(Command::_3C_APPLY_FOR_ALL), 0) == 0);
return (Params.command.rfind(std::string(Command::_3C_APPLY_ONLY_FOR_THIS),
0) == 0) ||
(Params.command.rfind(std::string(Command::_3C_APPLY_FOR_ALL), 0) ==
0);
}

bool execute3CCommand(const ExecuteCommandParams &Params,
std::string &ReplyMessage, _3CInterface &CcInterface) {
ReplyMessage = "Checked C Pointer Modified.";
if (Params.command.rfind(std::string(Command::_3C_APPLY_ONLY_FOR_THIS), 0) == 0) {
if (Params.command.rfind(std::string(Command::_3C_APPLY_ONLY_FOR_THIS), 0) ==
0) {
int PtrId = Params.The3CManualFix->PtrId;
CcInterface.makeSinglePtrNonWild(PtrId);
return true;
Expand Down
6 changes: 4 additions & 2 deletions clang-tools-extra/clangd/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,11 @@ endfunction()
set(CLANGD_KNOWN_VARIANTS "clangd;clangd3c")
set(CLANGD_EXCLUSIVE_VARIANT "" CACHE STRING
"Variant of clangd to include in the configuration. If empty, include all variants.")
set_property(CACHE CLANGD_EXCLUSIVE_VARIANT PROPERTY STRINGS ";${CLANGD_KNOWN_VARIANTS}")
set_property(CACHE CLANGD_EXCLUSIVE_VARIANT
PROPERTY STRINGS ";${CLANGD_KNOWN_VARIANTS}")
foreach(variant ${CLANGD_KNOWN_VARIANTS})
if(CLANGD_EXCLUSIVE_VARIANT STREQUAL "" OR CLANGD_EXCLUSIVE_VARIANT STREQUAL variant)
if(CLANGD_EXCLUSIVE_VARIANT STREQUAL "" OR
CLANGD_EXCLUSIVE_VARIANT STREQUAL variant)
set(CLANGD_INCLUDE_VARIANT_${variant} ON)
endif()
endforeach()
Expand Down
8 changes: 5 additions & 3 deletions clang-tools-extra/clangd/ClangdLSPServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1485,7 +1485,8 @@ ClangdLSPServer::ClangdLSPServer(
llvm::Optional<OffsetEncoding> ForcedOffsetEncoding,
const ClangdServer::Options &Opts
#ifdef INTERACTIVE3C
, _3CInterface &Cinter
,
_3CInterface &Cinter
#endif
)
: BackgroundContext(Context::current().clone()), Transp(Transp),
Expand All @@ -1496,9 +1497,10 @@ ClangdLSPServer::ClangdLSPServer(
CompileCommandsDir(std::move(CompileCommandsDir)), ClangdServerOpts(Opts),
NegotiatedOffsetEncoding(ForcedOffsetEncoding)
#ifdef INTERACTIVE3C
, The3CInterface(Cinter)
,
The3CInterface(Cinter)
#endif
{
{
// clang-format off
#ifdef INTERACTIVE3C
// We only support these methods in Interactive 3C mode.
Expand Down
8 changes: 5 additions & 3 deletions clang-tools-extra/clangd/ClangdLSPServer.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ class SymbolIndex;
/// The server also supports $/cancelRequest (MessageHandler provides this).
class ClangdLSPServer : private ClangdServer::Callbacks
#ifdef INTERACTIVE3C
, public _3CLSPCallBack
,
public _3CLSPCallBack
#endif
{
public:
Expand All @@ -55,9 +56,10 @@ class ClangdLSPServer : private ClangdServer::Callbacks
llvm::Optional<OffsetEncoding> ForcedOffsetEncoding,
const ClangdServer::Options &Opts
#ifdef INTERACTIVE3C
, _3CInterface &Cinter
,
_3CInterface &Cinter
#endif
);
);
/// The destructor blocks on any outstanding background tasks.
~ClangdLSPServer();

Expand Down
5 changes: 3 additions & 2 deletions clang-tools-extra/clangd/ClangdServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,10 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
std::make_unique<UpdateIndexCallbacks>(
DynamicIdx.get(), Callbacks, Opts.TheiaSemanticHighlighting))
#ifdef INTERACTIVE3C
, _3CInter(_3CInterface)
,
_3CInter(_3CInterface)
#endif
{
{
// Adds an index to the stack, at higher priority than existing indexes.
auto AddIndex = [&](SymbolIndex *Idx) {
if (this->Index != nullptr) {
Expand Down
4 changes: 4 additions & 0 deletions clang-tools-extra/clangd/Protocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -860,6 +860,10 @@ bool fromJSON(const llvm::json::Value &, CodeActionParams &);
#ifdef INTERACTIVE3C
struct CodeLensParams {
/// The document in which the command was invoked.
// These fields seem to be intentionally named after the JSON fields in spite
// of the LLVM naming guidelines. Does the rest of clangd just ignore the
// readability-identifier-naming warnings?
// NOLINTNEXTLINE(readability-identifier-naming)
TextDocumentIdentifier textDocument;
};

Expand Down
5 changes: 3 additions & 2 deletions clang-tools-extra/clangd/tool/ClangdMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -977,9 +977,10 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
/*UseDirBasedCDB=*/CompileArgsFrom == FilesystemCompileArgs,
OffsetEncodingFromFlag, Opts
#ifdef INTERACTIVE3C
, _3CInterface
,
_3CInterface
#endif
);
);
llvm::set_thread_name("clangd.main");
int ExitCode = LSPServer.run()
? 0
Expand Down
3 changes: 2 additions & 1 deletion clang/docs/checkedc/3C/CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# Contributing to 3C

Issues and pull requests related to 3C should be submitted to [CCI's
`checkedc-clang` repository](https://github.com/correctcomputation/checkedc-clang), not
`checkedc-clang`
repository](https://github.com/correctcomputation/checkedc-clang), not
[Microsoft's](https://github.com/microsoft/checkedc-clang), except as
stated below.

Expand Down
27 changes: 13 additions & 14 deletions clang/docs/checkedc/3C/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,21 @@ Issues and pull requests related to 3C should be submitted to CCI's
repository; see [CONTRIBUTING.md](CONTRIBUTING.md) for more
information.

As of March 2021, 3C is pre-alpha quality and we are just starting
to establish its public presence and processes. CCI is also working on
a proprietary extension of 3C called 5C ("**C**orrect
**C**omputation's **C**hecked-**C**-**C**onvert"). Our current plan is
that 3C will contain the core inference logic, while 5C will add
features to enhance developer productivity. If you'd like more
information about 5C, please contact us at
[email protected].
As of March 2021, 3C is pre-alpha quality and we are just starting to
establish its public presence and processes. CCI is also working on a
proprietary extension of 3C called 5C ("**C**orrect **C**omputation's
**C**hecked-**C**-**C**onvert"). Our current plan is that 3C will
contain the core inference logic, while 5C will add features to
enhance developer productivity. If you'd like more information about
5C, please contact us at [email protected].

### Which `checkedc-clang` repository to use?

We typically recommend that serious 3C users use CCI's repository to
get 3C fixes and enhancements sooner, but in some scenarios, you may
be better off with Microsoft's repository. Here, in detail, are the
factors we can think of that might affect your decision (as of
March 2021):
factors we can think of that might affect your decision (as of March
2021):

- CCI strives to merge changes reasonably quickly from Microsoft's
repository to CCI's, but most 3C-specific changes are made first in
Expand Down Expand Up @@ -94,10 +93,10 @@ March 2021):
- On the other hand, some CCI engineers work on Mac OS X and
frequently run the regression tests and other manual tests on CCI's
copy of 3C on Mac OS X, while we are unaware of any testing of
Microsoft's copy of 3C on Mac OS X. So you may be less
likely to encounter Mac-specific problems with CCI's repository. But
so far, when we've seen Mac-specific problems, we've usually gotten
a fix into Microsoft's repository reasonably quickly.
Microsoft's copy of 3C on Mac OS X. So you may be less likely to
encounter Mac-specific problems with CCI's repository. But so far,
when we've seen Mac-specific problems, we've usually gotten a fix
into Microsoft's repository reasonably quickly.

As noted in the [setup instructions](INSTALL.md#basics), both 3C and
the Checked C compiler depend on the Checked C system headers, which
Expand Down
43 changes: 28 additions & 15 deletions clang/docs/checkedc/3C/clang-tidy.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,11 @@ yet decided what to do about; please try to avoid adding more though.

We're currently using version 11.0.0 due to bugs in older versions, so if you
want to verify that our code is compliant with `clang-tidy`, you'll need to get
a copy of that version. If it isn't available from your OS distribution, you may
need to build it from source. This repository is currently based on a
pre-release of LLVM 9.0.0, so you can't use its version of `clang-tidy`.
However, Microsoft plans to upgrade it to LLVM 11.0.0 soon, at which point
you'll have the option to use `clang-tidy` built from this repository. It's
possible that using a newer, unstable version of `clang-tidy` (e.g., LLVM
master) might avoid more bugs, saving us the trouble of researching them, but we
doubt it's worth relying on a version that is unstable _and_ newer than what
Microsoft is about to upgrade to. This calculus might change when LLVM 12 is
released, expected around March 2021.
a copy of that version. Now that this repository is based on a pre-release of
LLVM 11, `clang-tidy` built from it is probably OK. But you probably want a
release build of `clang-tidy` rather than a debug build (a release build is
significantly faster), so if your main build from this repository is debug, it
might be easier to get `clang-tidy` another way.

This file maintains a list of issues we've run into that you should be aware of
when using `clang-tidy`. Many of these could in principle be researched and (if
Expand Down Expand Up @@ -136,11 +131,29 @@ suppression.
- Unwrapping an `else` branch may leave an unconditional return in an outer
`if` branch that then makes it possible to unwrap the outer `else` branch.

- `readability-identifier-naming` may fail to update a reference to an element
that it renamed. It turns out that the `--fix-errors` option not only makes
`clang-tidy` proceed in the presence of compile errors but also makes it fix
certain compile errors, including certain broken references, so a subsequent
`clang-tidy --fix-errors` pass may be helpful.
- We've seen many cases in which `readability-identifier-naming` fails to update
some references to an element that it renamed. There may be several causes for
this; we haven't found a comprehensive list on the web, so it's hard to know
what the problem is in any given case.

The biggest cause seems to be elements that are defined in shared header files
and referenced in multiple translation units. Our understanding is that
`clang-tidy` processes each translation unit independently, so the
`readability-identifer-naming` check generates a warning at the definition and
a fix to rename all the references in _that_ translation unit. When
`clang-tidy` aggregates its warnings across translation units, it
[deduplicates them by
location](https://github.com/llvm/llvm-project/blob/5de09ef02e24d234d9fc0cd1c6dfe18a1bb784b0/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp#L732)
and retains the fix from an arbitrary one of the duplicate warnings rather
than merging the fixes. The result is that only the references to the element
in an arbitrary one of the translation units that includes its definition get
renamed: a fundamental design flaw.

The upshot is that you're probably better off excluding
`readability-identifier-naming` from your `clang-tidy --fix` pass, running a
second pass without `--fix`, and fixing the `readability-identifier-naming`
warnings with a tool that handles multiple translation units properly (such as
a `clangd`-based IDE) along with the warnings that don't come with fixes.

- `readability-identifier-naming` may introduce a name collision. This may lead
to an error on one of the colliding definitions, an error on a reference that
Expand Down
16 changes: 8 additions & 8 deletions clang/include/clang/3C/3C.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
#ifndef LLVM_CLANG_3C_3C_H
#define LLVM_CLANG_3C_3C_H

#include "3CInteractiveData.h"
#include "ConstraintVariables.h"
#include "PersistentSourceLoc.h"
#include "ProgramInfo.h"
#include "clang/3C/3CInteractiveData.h"
#include "clang/3C/ConstraintVariables.h"
#include "clang/3C/PersistentSourceLoc.h"
#include "clang/3C/ProgramInfo.h"
#include "clang/Tooling/CommonOptionsParser.h"
#include <mutex>

Expand Down Expand Up @@ -99,10 +99,10 @@ class _3CInterface {
// datatype that is accepted in our codebase
// (https://llvm.org/docs/ProgrammersManual.html#fallible-constructors) seems
// too unwieldy to use right now.
static std::unique_ptr<_3CInterface> create(
const struct _3COptions &CCopt,
const std::vector<std::string> &SourceFileList,
clang::tooling::CompilationDatabase *CompDB);
static std::unique_ptr<_3CInterface>
create(const struct _3COptions &CCopt,
const std::vector<std::string> &SourceFileList,
clang::tooling::CompilationDatabase *CompDB);

// Constraint Building.

Expand Down
4 changes: 2 additions & 2 deletions clang/include/clang/3C/3CInteractiveData.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
#ifndef LLVM_CLANG_3C_3CINTERACTIVEDATA_H
#define LLVM_CLANG_3C_3CINTERACTIVEDATA_H

#include "ConstraintVariables.h"
#include "PersistentSourceLoc.h"
#include "clang/3C/ConstraintVariables.h"
#include "clang/3C/PersistentSourceLoc.h"

// Source info and reason for each wild pointer.
class WildPointerInferenceInfo {
Expand Down
6 changes: 3 additions & 3 deletions clang/include/clang/3C/ABounds.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
#ifndef LLVM_CLANG_3C_ABOUNDS_H
#define LLVM_CLANG_3C_ABOUNDS_H

#include "ProgramVar.h"
#include <clang/AST/Decl.h>
#include <clang/AST/Expr.h>
#include "clang/3C/ProgramVar.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"

using namespace clang;

Expand Down
7 changes: 4 additions & 3 deletions clang/include/clang/3C/AVarBoundsInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,15 @@
// This file contains the bounds information about various ARR atoms.
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_CLANG_3C_AVARBOUNDSINFO_H
#define LLVM_CLANG_3C_AVARBOUNDSINFO_H

#include "ABounds.h"
#include "AVarGraph.h"
#include "ProgramVar.h"
#include "clang/3C/ABounds.h"
#include "clang/3C/AVarGraph.h"
#include "clang/3C/ConstraintVariables.h"
#include "clang/3C/PersistentSourceLoc.h"
#include "clang/3C/ProgramVar.h"
#include "clang/AST/Decl.h"

class ProgramInfo;
Expand Down
4 changes: 2 additions & 2 deletions clang/include/clang/3C/AVarGraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
#ifndef LLVM_CLANG_3C_AVARGRAPH_H
#define LLVM_CLANG_3C_AVARGRAPH_H

#include "ABounds.h"
#include "ConstraintsGraph.h"
#include "clang/3C/ABounds.h"
#include "clang/3C/ConstraintsGraph.h"

// Graph that keeps tracks of direct assignments between various variables.
class AVarGraph : public DataGraph<BoundsKey> {
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/3C/ArrayBoundsInferenceConsumer.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
#ifndef LLVM_CLANG_3C_ARRAYBOUNDSINFERENCECONSUMER_H
#define LLVM_CLANG_3C_ARRAYBOUNDSINFERENCECONSUMER_H

#include "ProgramInfo.h"
#include "clang/3C/ProgramInfo.h"
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/StmtVisitor.h"
#include "clang/Analysis/Analyses/Dominators.h"
Expand Down
21 changes: 10 additions & 11 deletions clang/include/clang/3C/CastPlacement.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,30 +13,30 @@
#define LLVM_CLANG_3C_CASTPLACEMENT_H

#include "clang/3C/ConstraintResolver.h"
#include "clang/3C/RewriteUtils.h"
#include "clang/AST/RecursiveASTVisitor.h"
#include "RewriteUtils.h"

// Locates expressions which are children of explicit cast expressions after
// ignoring any intermediate implicit expressions introduced in the clang AST.
class CastLocatorVisitor : public RecursiveASTVisitor<CastLocatorVisitor> {
public:
explicit CastLocatorVisitor(ASTContext *C) : Context(C) {}
// This visitor happens to not use the ASTContext itself.
explicit CastLocatorVisitor(ASTContext *C) {}

bool VisitCastExpr(CastExpr *C);

std::set<Expr *> &getExprsWithCast() { return ExprsWithCast; }

private:
ASTContext *Context;
std::set<Expr *> ExprsWithCast;
};

class CastPlacementVisitor : public RecursiveASTVisitor<CastPlacementVisitor> {
public:
explicit CastPlacementVisitor
(ASTContext *C, ProgramInfo &I, Rewriter &R, std::set<Expr *> &EWC)
: Context(C), Info(I), Writer(R), CR(Info, Context), ABRewriter(I),
ExprsWithCast(EWC) {}
explicit CastPlacementVisitor(ASTContext *C, ProgramInfo &I, Rewriter &R,
std::set<Expr *> &EWC)
: Context(C), Info(I), Writer(R), CR(Info, Context), ABRewriter(I),
ExprsWithCast(EWC) {}

bool VisitCallExpr(CallExpr *C);

Expand All @@ -55,13 +55,12 @@ class CastPlacementVisitor : public RecursiveASTVisitor<CastPlacementVisitor> {
CAST_TO_WILD // A standard C explicit cast required (checked -> wild)
};

CastNeeded needCasting(ConstraintVariable *SrcInt,
ConstraintVariable *SrcExt,
CastNeeded needCasting(ConstraintVariable *SrcInt, ConstraintVariable *SrcExt,
ConstraintVariable *DstInt,
ConstraintVariable *DstExt);

std::pair<std::string, std::string>
getCastString(ConstraintVariable *Dst, CastNeeded CastKind);
std::pair<std::string, std::string> getCastString(ConstraintVariable *Dst,
CastNeeded CastKind);

void surroundByCast(ConstraintVariable *Dst, CastNeeded CastKind, Expr *E);
void reportCastInsertionFailure(Expr *E, const std::string &CastStr);
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/3C/CheckedRegions.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#ifndef LLVM_CLANG_3C_CHECKEDREGIONS_H
#define LLVM_CLANG_3C_CHECKEDREGIONS_H

#include "ProgramInfo.h"
#include "clang/3C/ProgramInfo.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/Decl.h"
#include "clang/AST/Stmt.h"
Expand Down
4 changes: 2 additions & 2 deletions clang/include/clang/3C/ConstraintBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
#ifndef LLVM_CLANG_3C_CONSTRAINTBUILDER_H
#define LLVM_CLANG_3C_CONSTRAINTBUILDER_H

#include "ProgramInfo.h"
#include "TypeVariableAnalysis.h"
#include "clang/3C/ProgramInfo.h"
#include "clang/3C/TypeVariableAnalysis.h"
#include "clang/AST/ASTConsumer.h"

class ConstraintBuilderConsumer : public clang::ASTConsumer {
Expand Down
Loading