Page MenuHomePhabricator

m4tx (Mateusz Maćkowski)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 8 2018, 6:49 AM (10 w, 6 d)

Recent Activity

Sun, Jan 13

m4tx added a comment to D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403).

I've added the implicit access specifier check and run it on some bigger codebases. My findings are as below:

Sun, Jan 13, 8:36 AM · Restricted Project
m4tx updated the diff for D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403).
Sun, Jan 13, 8:36 AM · Restricted Project

Dec 20 2018

m4tx updated the diff for D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403).
Dec 20 2018, 1:08 PM · Restricted Project
m4tx added a comment to D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403).

Please add tests with preprocessor (#if ...) that will show that it ignores disabled code. e.g.:

class ProbablyValid {
private:
  int a;
#if defined(ZZ)
public:
  int b;
#endif
private:
  int c;
protected:
  int d;
public:
  int e;
};

Is this actually possible?
It seems that macros are ran through the preprocessor before one can fiddle with them in clang-tidy.
In other words, int b is not at all present in the AST.

.. and by "ignores" i meant that it will be diagnosing this code, since it did not know anything about the code within the preprocessor-disabled section.

However, I added a code to detect macro expansions, so duplicated access specifiers are ignored if at least one of them comes from a macro. If there is a way to cover your case as well, please let me know, because even after looking at the code of other checks I haven't found out a solution for this.

Dec 20 2018, 1:08 PM · Restricted Project

Dec 18 2018

m4tx added a comment to D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403).

Please add tests with preprocessor (#if ...) that will show that it ignores disabled code. e.g.:

class ProbablyValid {
private:
  int a;
#if defined(ZZ)
public:
  int b;
#endif
private:
  int c;
protected:
  int d;
public:
  int e;
};
Dec 18 2018, 1:49 PM · Restricted Project
m4tx added a comment to D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403).

Don't use CXXRecordDecl::accessSpecs(), use unique comments in tests.

Thanks! CXXRecordDecl is already huge and so adding iterators for a single checker is in my opinion not a good idea (especially if the alternative is actually less code).
Would it make sense to also issue a diagnostic where the first access specifier is redundant (ie public in a struct, and private in a class) ?

Dec 18 2018, 1:45 PM · Restricted Project
m4tx added inline comments to D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403).
Dec 18 2018, 1:43 PM · Restricted Project
m4tx updated the diff for D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403).

Fix variable names, add macro expansion checking as well as macro and inner class tests.

Dec 18 2018, 1:41 PM · Restricted Project

Dec 17 2018

m4tx abandoned D55790: [AST] Add accessSpecs() iterator range for CXXRecordDecl.
Dec 17 2018, 3:39 PM
m4tx updated the diff for D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403).

Don't use CXXRecordDecl::accessSpecs(), use unique comments in tests.

Dec 17 2018, 3:29 PM · Restricted Project
m4tx added a comment to D55790: [AST] Add accessSpecs() iterator range for CXXRecordDecl.

Do you really have to add an iterator for this particular thing ?
Why not just use specific_decl_iterator<AccessSpecDecl> in your clang-tidy checker ?

Dec 17 2018, 3:05 PM
m4tx updated the diff for D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403).

Use alphabetical order in ReleaseNotes.rst

Dec 17 2018, 2:59 PM · Restricted Project
m4tx retitled D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403) from [clang-tidy] Add duplicated access specifier readability check to [clang-tidy] Add duplicated access specifier readability check (PR25403).
Dec 17 2018, 2:55 PM · Restricted Project
m4tx added reviewers for D55790: [AST] Add accessSpecs() iterator range for CXXRecordDecl: aaron.ballman, klimek.
Dec 17 2018, 2:47 PM
m4tx added a child revision for D55790: [AST] Add accessSpecs() iterator range for CXXRecordDecl: D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403).
Dec 17 2018, 2:40 PM
m4tx added a parent revision for D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403): D55790: [AST] Add accessSpecs() iterator range for CXXRecordDecl.
Dec 17 2018, 2:40 PM · Restricted Project
m4tx created D55793: [clang-tidy] Add duplicated access specifier readability check (PR25403).
Dec 17 2018, 2:40 PM · Restricted Project
m4tx created D55790: [AST] Add accessSpecs() iterator range for CXXRecordDecl.
Dec 17 2018, 2:33 PM

Nov 26 2018

m4tx abandoned D54262: [clang-tidy] Add `delete this` bugprone check (PR38741).
Nov 26 2018, 11:03 AM · Restricted Project

Nov 19 2018

m4tx added a comment to D54262: [clang-tidy] Add `delete this` bugprone check (PR38741).

After thinking about the possible use cases (and the difficulty of implementing heuristics for them) as well as fiddling with Clang Static Analyzer it seems that this patch can be discarded as the Analyzer already handles delete this pretty well. I've posted an update to the Bugzilla ticket.

Nov 19 2018, 2:36 PM · Restricted Project

Nov 8 2018

m4tx created D54262: [clang-tidy] Add `delete this` bugprone check (PR38741).
Nov 8 2018, 7:11 AM · Restricted Project