alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
Awesome!
A few comments.
================
Comment at: CMakeLists.txt:6
@@ -5,2 +5,3 @@
add_subdirectory(clang-tidy)
+add_subdirectory(clang-tidy-vs)
endif()
----------------
Should the plugin be placed inside clang-tidy directory?
================
Comment at: clang-tidy-vs/ClangTidy/CategoryVerb.cs:29
@@ +28,3 @@
+ {
+
+ }
----------------
Remove the empty line.
================
Comment at: clang-tidy-vs/ClangTidy/CategoryVerb.cs:34
@@ +33,3 @@
+ {
+ if (value.GetType() == typeof(string))
+ {
----------------
Should this be `value is string`?
================
Comment at: clang-tidy-vs/ClangTidy/CheckTree.cs:130
@@ +129,3 @@
+ CheckTree Root = new CheckTree();
+ string[][] Groups = new string[][] {
+ new string[] {"boost"},
----------------
Ideally, this should be configurable without re-compilation. Not necessary for
now, but please add a FIXME.
================
Comment at: clang-tidy-vs/ClangTidy/ClangTidyConfigurationPage.cs:10
@@ +9,3 @@
+using System.ComponentModel;
+using System.Linq;
+using System.Runtime.InteropServices;
----------------
Are all of these actually used?
================
Comment at: clang-tidy-vs/ClangTidy/ClangTidyConfigurationPage.cs:26
@@ +25,3 @@
+ get
+ { if (Grid == null)
+ Grid = new ClangTidyPropertyGrid();
----------------
^K^F or whatever poor C# folks do to format code without clang-format ;)
================
Comment at: clang-tidy-vs/ClangTidy/ClangTidyPackage.cs:58
@@ +57,3 @@
+ {
+ //var page = (OptionPageGrid)GetDialogPage(typeof(OptionPageGrid));
+ //return page.Style;
----------------
Remove commented-out code?
================
Comment at: clang-tidy-vs/ClangTidy/ClangTidyProperties.cs:81
@@ +80,3 @@
+ [ClangTidyCheck("cert-dcl50-cpp")]
+ public bool CERTDCL50
+ {
----------------
I hope, this file is generated?
A way to update this file should be documented.
================
Comment at: clang-tidy-vs/ClangTidy/Properties/AssemblyInfo.cs:32
@@ +31,3 @@
+
+[assembly: AssemblyVersion("1.1.0.0")]
+[assembly: AssemblyFileVersion("1.1.0.0")]
----------------
Ideally, the file should be generated by CMake to use proper version number.
Please add a FIXME.
================
Comment at: clang-tidy-vs/ClangTidy/PropertyFileParser.cs:80
@@ +79,3 @@
+
+ string ClangTidy = Path.Combine(ClangTidyFilePath, ".clang-tidy");
+ using (StreamWriter Writer = new StreamWriter(ClangTidy))
----------------
s/ClangTidy/ConfigFile/
================
Comment at: clang-tidy-vs/ClangTidy/PropertyFileParser.cs:208
@@ +207,3 @@
+
+ ClangTidyProperties.CheckMapping[] Matches =
Props.FindChecksMatching(Check).ToArray();
+ foreach (var Match in Matches)
----------------
Is the copy needed to avoid aliasing? (i.e. will this break if you iterate over
`Props.FindChecksMatching(Check)` instead?)
================
Comment at: clang-tidy-vs/ClangTidy/Utility.cs:31
@@ +30,3 @@
+ {
+ string RE = Regex.Escape(Pattern).Replace(@"\*",
".*").Replace(@"\?", ".");
+ return Regex.IsMatch(Value, RE);
----------------
Question marks are not supported in check patterns, so `.Replace(@"\?", ".")`
can be dropped.
https://reviews.llvm.org/D23848
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
