ccFormat: Enable Clang-Tidy checks #708
Conversation
| @@ -168,6 +168,9 @@ class ABIInfo { | |||
| ABIType ResultType, llvm::ArrayRef<ABIType> ArgTypes, | |||
| ABIArgInfo &ResultInfo, | |||
| std::vector<ABIArgInfo> &ArgInfos) const = 0; | |||
|
|
|||
| /// \brief Virtual Destructor | |||
| virtual ~ABIInfo()=default; | |||
There was a problem hiding this comment.
To avoid warnings from [clang-diagnostic-delete-non-virtual-dtor]
warning: delete called on 'ABIInfo' that is abstract but has non-virtual destructor
|
@adiaaida please review |
| @@ -43,6 +43,8 @@ def runTidy(args): | |||
| "LLVM_BUILD environment variable." | |||
| return 1 | |||
|
|
|||
| coreclrbuild = expandPath(args.coreclr_build) | |||
There was a problem hiding this comment.
we need a check like above to confirm that coreclr_build is not none. Also, is this the path to just the coreclr directory or some other path? I need this to make the job work.
There was a problem hiding this comment.
This is a path to the CORECLR build directory CoreClr\bin\Product\Windows_NT.x64.debug
swaroop-sridhar
force-pushed
the
tidy
branch
3 times, most recently
from
f4631b0 to
8b09d41
Compare
|
@dotnet-bot test this please |
1 similar comment
|
@dotnet-bot test this please |
|
@swaroop-sridhar the code formatting job is still failing. Can you take a look to see if we are missing something? |
|
@adiaaida: Looks like this is because of an incompatibality between VS 2013 used to build LLILC Jit, and VS 2015 libraries in the include path. ccFormat picks up os-includes from the INCLUDE environment variable. Looks like the lab's INCLUDE points to VS 2015. |
|
@mmitche what do you think about this headers issue and the lab? |
|
@adiaaida @swaroop-sridhar Which version of vcvars/vsdevcmd are you using? It should totally depend on which of the development command prompts you open. They shouldn't leak include paths between. |
|
@dotnet-bot test this please |
|
we use the version in VS120COMNTOOLS. |
|
That's 2013, so it probably warrants some investigation as to why the other includes are somehow getting on the path |
|
My best guess is that we run vcvars all for the build, but for the run of clang-tidy, we're in a batch env that hasn't had it run. I updated the job to see if I could fix the issue |
|
I would imagine that without vcvars at all, there shouldn't be any includes. |
|
@dotnet-bot test this please |
|
Ok. It looks like I got the job all working now. Please take a look at the formatting errors that it's producing. |
|
The formatter correction it made all look like good correction. |
|
I'll revert the changes to ABI.h/,cpp (in response to Pat's feedback) and retry the testing. |
1) Change ccFormat to run clang-tidy checks by default Use ccFormat --untidy to only run formatting checks 2) Fix the warnings/errors generated by ccFormat
|
@dotnet-bot test this please |
|
@swaroop-sridhar Are you seeing issues with the infra or are you just iterating on something? |
|
@swaroop-sridhar I think the way I changed the lab job, it looks like it's running ccformat but it really is. It just isn't producing output after we call vcvarsall |
|
Some legs had failed because of github connection timeout, so retried. |
ccFormat: Enable Clang-Tidy checks
Use ccFormat --untidy to only run formatting checks