Skip to content
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

Automate most of the 3C code style checks. #716

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
20 changes: 11 additions & 9 deletions clang/docs/checkedc/3C/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,19 @@ problems arising from your 3C pull request and/or we may ask you to
make specific changes to your pull request to accommodate 5C's code.

At the appropriate time during development of a pull request, please
run the [regression tests](development.md#regression-tests) and
run the [regression tests](development.md#regression-tests) and the
[automated style checks](development.md#code-style-automation) and
correct any failures. (For example, it may not make sense to do this
on a draft pull request containing an unfinished demonstration of an
idea.) All regression tests must pass (or be disabled if appropriate)
before your pull request can be merged. If you're changing behavior
(as opposed to just cleaning up the code), we'll typically require you
to add or update enough tests to exercise the important behavior
changes (i.e., those tests fail before your code change and pass after
it). If there's a concern that your change might affect other cases
that are not adequately tested yet, we may ask you to add tests for
those cases as well.
idea.) All regression tests and automated style checks must pass (or
be disabled if appropriate) before your pull request can be merged.

If you're changing behavior (as opposed to just cleaning up the code),
we'll typically require you to add or update enough regression tests
to exercise the important behavior changes (i.e., those tests fail
before your code change and pass after it). If there's a concern that
your change might affect other cases that are not adequately tested
yet, we may ask you to add tests for those cases as well.

See the [developer's guide](development.md) for additional information
that may be helpful as you work on 3C.
35 changes: 28 additions & 7 deletions clang/docs/checkedc/3C/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,31 @@ in your code. Specifically:

* Space before and after `:` in iterators, i.e., `for (auto &k : List)`

Our goal is that all files should be formatted with `clang-format` and
pass `clang-tidy` ([more information](clang-tidy.md)), and nonempty
files should have a final newline (surprisingly, `clang-format` cannot
enforce this). However, until we have better automation, we decided it
isn't reasonable to require contributors to manually run these tools
and fix style nits in each change; instead, we periodically run the
tools on the entire 3C codebase.
* Nonempty files should have a final newline.

All 3C C++ source files should be formatted with `clang-format` and
pass `clang-tidy` ([more information](clang-tidy.md)) based on the
`.clang-format` and `.clang-tidy` files in the repository (which the
tools will automatically use by default). (We are not using either of
these tools on the 3C regression tests at this time.) Likewise, 3C
Python files should pass `yapf` based on the `.style.yapf` files
(except for `clang/tools/3c/utils/*.py`, which are pretty much
unmaintained at this point). Where appropriate, code may be excluded
from these checks via `// clang-format off`, `NOLINTNEXTLINE(...)`,
and `yapf: disable`. These tools (as currently configured) enforce
some but not all of the specific guidelines above, and they also
enforce many other standards not listed above.

### Code style automation

`ninja lint-3c` runs the `clang-format`, `clang-tidy`, and `yapf`
checks described above. We generally want it to pass on the `main`
branch at all times. It requires the relevant tools to be available on
your system; see `clang/tools/3c/utils/code_style/CMakeLists.txt` for
details. `clang/tools/3c/utils/code_style/fix-all.sh` will try to fix
all style violations that can be fixed automatically, but see the
caveats in that script before running it.

As a convenience for fully validating a change to 3C, `ninja
validate-3c` runs both the regression tests (`check-3c`) and the
automated style checks (`lint-3c`).
4 changes: 1 addition & 3 deletions clang/include/clang/3C/3CInteractiveData.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ class RootCauseDiagnostic {

const PersistentSourceLoc &getLocation() const { return Main.Location; }

void addReason(const ReasonLoc &Rsn) {
Supplemental.push_back(Rsn);
}
void addReason(const ReasonLoc &Rsn) { Supplemental.push_back(Rsn); }

std::vector<ReasonLoc> &additionalNotes() { return Supplemental; }

Expand Down
3 changes: 1 addition & 2 deletions clang/include/clang/3C/CastPlacement.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ class CastPlacementVisitor : public RecursiveASTVisitor<CastPlacementVisitor> {
ConstraintVariable *TypeVar,
CastNeeded CastKind);

void surroundByCast(ConstraintVariable *Dst,
ConstraintVariable *TypeVar,
void surroundByCast(ConstraintVariable *Dst, ConstraintVariable *TypeVar,
CastNeeded CastKind, Expr *E);
void reportCastInsertionFailure(Expr *E, const std::string &CastStr);
void updateRewriteStats(CastNeeded CastKind);
Expand Down
6 changes: 2 additions & 4 deletions clang/include/clang/3C/ConstraintResolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,16 @@ class ConstraintResolver {

CVarSet getInvalidCastPVCons(CastExpr *E);

CVarSet addAtomAll(CVarSet CVS, ConstAtom *PtrTyp,
ReasonLoc &Rsn, Constraints &CS);
CVarSet addAtomAll(CVarSet CVS, ConstAtom *PtrTyp, ReasonLoc &Rsn,
Constraints &CS);
CVarSet pvConstraintFromType(QualType TypE);

CSetBkeyPair getAllSubExprConstraintVars(std::vector<Expr *> &Exprs);
CVarSet getBaseVarPVConstraint(DeclRefExpr *Decl);

PVConstraint *getRewritablePVConstraint(Expr *E);


bool isNonPtrType(QualType &TE);

};

#endif // LLVM_CLANG_3C_CONSTRAINTRESOLVER_H
88 changes: 40 additions & 48 deletions clang/include/clang/3C/ConstraintVariables.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,13 @@ class ConstraintVariable {
// Only subclasses should call this
ConstraintVariable(ConstraintVariableKind K, std::string T, std::string N,
std::string TN)
: Kind(K), OriginalType(T), Name(N), OriginalTypeWithName(TN),
HasEqArgumentConstraints(false), ValidBoundsKey(false),
IsForDecl(false) {}
: Kind(K), OriginalType(T), Name(N), OriginalTypeWithName(TN),
HasEqArgumentConstraints(false), ValidBoundsKey(false),
IsForDecl(false) {}

ConstraintVariable(ConstraintVariableKind K, QualType QT, std::string N)
: ConstraintVariable(K, qtyToStr(QT), N,
qtyToStr(QT, N == RETVAR ? "" : N)) {}
: ConstraintVariable(K, qtyToStr(QT), N,
qtyToStr(QT, N == RETVAR ? "" : N)) {}

public:
// Create a "for-rewriting" representation of this ConstraintVariable.
Expand Down Expand Up @@ -235,16 +235,16 @@ enum ConsAction { Safe_to_Wild, Wild_to_Safe, Same_to_Same };

void constrainConsVarGeq(const std::set<ConstraintVariable *> &LHS,
const std::set<ConstraintVariable *> &RHS,
Constraints &CS, const ReasonLoc &Rsn,
ConsAction CA, bool DoEqType, ProgramInfo *Info,
Constraints &CS, const ReasonLoc &Rsn, ConsAction CA,
bool DoEqType, ProgramInfo *Info,
bool HandleBoundsKey = true);
void constrainConsVarGeq(ConstraintVariable *LHS, const CVarSet &RHS,
Constraints &CS, const ReasonLoc &Rsn,
ConsAction CA, bool DoEqType, ProgramInfo *Info,
Constraints &CS, const ReasonLoc &Rsn, ConsAction CA,
bool DoEqType, ProgramInfo *Info,
bool HandleBoundsKey = true);
void constrainConsVarGeq(ConstraintVariable *LHS, ConstraintVariable *RHS,
Constraints &CS, const ReasonLoc &Rsn,
ConsAction CA, bool DoEqType, ProgramInfo *Info,
Constraints &CS, const ReasonLoc &Rsn, ConsAction CA,
bool DoEqType, ProgramInfo *Info,
bool HandleBoundsKey = true);

// True if [C] is a PVConstraint that contains at least one Atom (i.e.,
Expand Down Expand Up @@ -280,8 +280,8 @@ class PointerVariableConstraint : public ConstraintVariable {
// TODO: This method always returns a constraint variable containing one atom.
// This causes problems if the variable is later used as a deeper
// pointer type. See correctcomputation/checkedc-clang#673.
static PointerVariableConstraint *
getWildPVConstraint(Constraints &CS, const ReasonLoc &Rsn);
static PointerVariableConstraint *getWildPVConstraint(Constraints &CS,
const ReasonLoc &Rsn);

// Get constraint variables representing values that are not pointers. If a
// meaningful name can be assigned to the value, the second method should be
Expand Down Expand Up @@ -405,12 +405,12 @@ class PointerVariableConstraint : public ConstraintVariable {
// Construct and empty PointerVariableConstraint with only the name set. All
// other fields are initialized to default values. This is used to construct
// variables for non-pointer expressions.
PointerVariableConstraint(std::string Name) :
ConstraintVariable(PointerVariable, "", Name, ""), FV(nullptr),
SrcHasItype(false), PartOfFuncPrototype(false), Parent(nullptr),
SourceGenericIndex(-1), InferredGenericIndex(-1),
IsZeroWidthArray(false), IsTypedef(false),
TypedefLevelInfo({}), IsVoidPtr(false) {}
PointerVariableConstraint(std::string Name)
: ConstraintVariable(PointerVariable, "", Name, ""), FV(nullptr),
SrcHasItype(false), PartOfFuncPrototype(false), Parent(nullptr),
SourceGenericIndex(-1), InferredGenericIndex(-1),
IsZeroWidthArray(false), IsTypedef(false), TypedefLevelInfo({}),
IsVoidPtr(false) {}

public:
std::string getTy() const { return BaseType; }
Expand All @@ -437,7 +437,7 @@ class PointerVariableConstraint : public ConstraintVariable {

bool isGeneric() const { return InferredGenericIndex >= 0; }
int getGenericIndex() const { return InferredGenericIndex; }
void setGenericIndex(int idx) { InferredGenericIndex = idx; }
void setGenericIndex(int Idx) { InferredGenericIndex = Idx; }
bool isGenericChanged() const {
return SourceGenericIndex != InferredGenericIndex;
}
Expand Down Expand Up @@ -491,15 +491,12 @@ class PointerVariableConstraint : public ConstraintVariable {
// code representation of the type. Allows for more precise rewriting by
// preserving the exact syntax used to write types that aren't rewritten
// by 3C. If this is null, then the type will be reconstructed from QT.
PointerVariableConstraint(const clang::QualType &QT, clang::DeclaratorDecl *D,
std::string N, ProgramInfo &I,
const clang::ASTContext &C,
std::string *InFunc = nullptr,
int ForceGenericIndex = -1,
bool PotentialGeneric = false,
bool VarAtomForChecked = false,
TypeSourceInfo *TSI = nullptr,
const clang::QualType &ItypeT = QualType());
PointerVariableConstraint(
const clang::QualType &QT, clang::DeclaratorDecl *D, std::string N,
ProgramInfo &I, const clang::ASTContext &C, std::string *InFunc = nullptr,
int ForceGenericIndex = -1, bool PotentialGeneric = false,
bool VarAtomForChecked = false, TypeSourceInfo *TSI = nullptr,
const clang::QualType &ItypeT = QualType());

const CAtoms &getCvars() const { return Vars; }

Expand All @@ -526,8 +523,8 @@ class PointerVariableConstraint : public ConstraintVariable {
void constrainOuterTo(Constraints &CS, ConstAtom *C, const ReasonLoc &Rsn,
bool DoLB = false, bool Soft = false);
void constrainIdxTo(Constraints &CS, ConstAtom *C, unsigned int Idx,
const ReasonLoc &Rsn,
bool DoLB = false, bool Soft = false);
const ReasonLoc &Rsn, bool DoLB = false,
bool Soft = false);
bool anyChanges(const EnvironmentMap &E) const override;
bool anyArgumentIsWild(const EnvironmentMap &E);
bool hasWild(const EnvironmentMap &E, int AIdx = -1) const override;
Expand All @@ -542,8 +539,8 @@ class PointerVariableConstraint : public ConstraintVariable {

bool isPartOfFunctionPrototype() const { return PartOfFuncPrototype; }
// Add the provided constraint variable as an argument constraint.
bool addArgumentConstraint(ConstraintVariable *DstCons,
ReasonLoc &Rsn, ProgramInfo &Info);
bool addArgumentConstraint(ConstraintVariable *DstCons, ReasonLoc &Rsn,
ProgramInfo &Info);
// Get the set of constraint variables corresponding to the arguments.
const std::set<ConstraintVariable *> &getArgumentConstraints() const;

Expand Down Expand Up @@ -608,9 +605,9 @@ class FVComponentVariable {
PVConstraint *getInternal() const { return InternalConstraint; }
PVConstraint *getExternal() const { return ExternalConstraint; }

void setGenericIndex(int idx) {
ExternalConstraint->setGenericIndex(idx);
InternalConstraint->setGenericIndex(idx);
void setGenericIndex(int Idx) {
ExternalConstraint->setGenericIndex(Idx);
InternalConstraint->setGenericIndex(Idx);
}

void equateWithItype(ProgramInfo &CS,
Expand All @@ -637,7 +634,6 @@ class FunctionVariableConstraint : public ConstraintVariable {
bool Hasproto;
bool Hasbody;
bool IsStatic;
FunctionVariableConstraint *Parent;
// Flag to indicate whether this is a function pointer or not.
bool IsFunctionPtr;

Expand All @@ -653,9 +649,9 @@ class FunctionVariableConstraint : public ConstraintVariable {
FunctionVariableConstraint(clang::TypedefDecl *D, ProgramInfo &I,
const clang::ASTContext &C);

FunctionVariableConstraint(const clang::QualType Ty,
clang::DeclaratorDecl *D, std::string N,
ProgramInfo &I, const clang::ASTContext &C,
FunctionVariableConstraint(const clang::QualType Ty, clang::DeclaratorDecl *D,
std::string N, ProgramInfo &I,
const clang::ASTContext &C,
TypeSourceInfo *TSI = nullptr);

PVConstraint *getExternalReturn() const {
Expand Down Expand Up @@ -701,22 +697,18 @@ class FunctionVariableConstraint : public ConstraintVariable {
bool srcHasBounds() const override;

// The number of type variables
int getGenericParams() const {
return TypeParams;
}
int getGenericParams() const { return TypeParams; }
// remove added generics
// use when we constrain a potential generic param to wild
void resetGenericParams() {
TypeParams = 0;
}
void resetGenericParams() { TypeParams = 0; }

// The type parameter index of the return
int getGenericIndex() const {
return ReturnVar.ExternalConstraint->getGenericIndex();
}
// Change the type parameter index of the return
void setGenericIndex(int idx) {
ReturnVar.ExternalConstraint->setGenericIndex(idx);
void setGenericIndex(int Idx) {
ReturnVar.ExternalConstraint->setGenericIndex(Idx);
}

bool solutionEqualTo(Constraints &CS, const ConstraintVariable *CV,
Expand Down
8 changes: 2 additions & 6 deletions clang/include/clang/3C/Constraints.h
Original file line number Diff line number Diff line change
Expand Up @@ -314,13 +314,9 @@ class Constraint {
}
virtual std::vector<ReasonLoc> &additionalReasons() { return ExtraReasons; }
// include additional reasons that will appear in output as notes
virtual void addReason(const ReasonLoc &Rsn) {
ExtraReasons.push_back(Rsn);
}
virtual void addReason(const ReasonLoc &Rsn) { ExtraReasons.push_back(Rsn); }

bool isUnwritable(void) const {
return getReasonText() == UNWRITABLE_REASON;
}
bool isUnwritable(void) const { return getReasonText() == UNWRITABLE_REASON; }

const PersistentSourceLoc &getLocation() const { return Reason.Location; }
};
Expand Down
22 changes: 12 additions & 10 deletions clang/include/clang/3C/ConstraintsGraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,9 @@ class DataGraph

// This version provides more info by returning graph edges
// rather than data items
bool getIncidentEdges(Data D, std::set<EdgeType*> &EdgeSet, bool Succ,
bool Append = false, bool IgnoreSoftEdges = false) const {
bool getIncidentEdges(Data D, std::set<EdgeType *> &EdgeSet, bool Succ,
bool Append = false,
bool IgnoreSoftEdges = false) const {
NodeType *N = this->findNode(D);
if (N == nullptr)
return false;
Expand Down Expand Up @@ -188,23 +189,23 @@ class DataGraph
return !DataSet.empty();
}

bool getSuccessorsEdges(Atom *A, std::set<EdgeType*> &EdgeSet,
bool Append = false) {
bool getSuccessorsEdges(Atom *A, std::set<EdgeType *> &EdgeSet,
bool Append = false) {
return getIncidentEdges(A, EdgeSet, true, Append);
}

bool getPredecessorsEdges(Atom *A, std::set<EdgeType*> &EdgeSet,
bool getPredecessorsEdges(Atom *A, std::set<EdgeType *> &EdgeSet,
bool Append = false) {
return getIncidentEdges(A, EdgeSet, false, Append);
}

bool
getSuccessors(Data D, std::set<Data> &DataSet, bool Append = false) const {
bool getSuccessors(Data D, std::set<Data> &DataSet,
bool Append = false) const {
return getNeighbors(D, DataSet, true, Append);
}

bool
getPredecessors(Data D, std::set<Data> &DataSet, bool Append = false) const {
bool getPredecessors(Data D, std::set<Data> &DataSet,
bool Append = false) const {
return getNeighbors(D, DataSet, false, Append);
}

Expand Down Expand Up @@ -267,7 +268,8 @@ class ConstraintsGraph : public DataGraph<Atom *> {
// be able to retrieve them from the graph.
std::set<ConstAtom *> &getAllConstAtoms();

typedef DataEdge<Atom*> EdgeType;
typedef DataEdge<Atom *> EdgeType;

protected:
// Add vertex is overridden to save const atoms as they are added to the graph
// so that getAllConstAtoms can efficiently retrieve them.
Expand Down
17 changes: 8 additions & 9 deletions clang/include/clang/3C/DeclRewriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,9 @@ class DeclRewriter {
// Info parameter are rewritten.
static void rewriteDecls(ASTContext &Context, ProgramInfo &Info, Rewriter &R);

static void
buildItypeDecl(PVConstraint *Defn, DeclaratorDecl *Decl, std::string &Type,
std::string &IType, ProgramInfo &Info,
ArrayBoundsRewriter &ABR);
static void buildItypeDecl(PVConstraint *Defn, DeclaratorDecl *Decl,
std::string &Type, std::string &IType,
ProgramInfo &Info, ArrayBoundsRewriter &ABR);

private:
static RecordDecl *LastRecordDecl;
Expand Down Expand Up @@ -102,11 +101,11 @@ class FunctionDeclBuilder : public RecursiveASTVisitor<FunctionDeclBuilder> {
// Get existing itype string from constraint variables.
std::string getExistingIType(ConstraintVariable *DeclC);

virtual void buildDeclVar(const FVComponentVariable *CV,
DeclaratorDecl *Decl, std::string &Type,
std::string &IType, std::string UseName,
bool &RewriteGen, bool &RewriteParm,
bool &RewriteRet, bool StaticFunc);
virtual void buildDeclVar(const FVComponentVariable *CV, DeclaratorDecl *Decl,
std::string &Type, std::string &IType,
std::string UseName, bool &RewriteGen,
bool &RewriteParm, bool &RewriteRet,
bool StaticFunc);
void buildCheckedDecl(PVConstraint *Defn, DeclaratorDecl *Decl,
std::string &Type, std::string &IType,
std::string UseName, bool &RewriteParm,
Expand Down
Loading