Page MenuHomePhabricator

martong (Gabor Marton)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 10 2017, 8:01 AM (97 w, 4 d)

Recent Activity

Yesterday

martong added a comment to D62131: [ASTImporter] Remove NonEquivalentDecls from ASTImporter.

... it is highly probable that if some pairs were ever non-equivalent they will stay non-equivalent.

Fri, Aug 23, 8:19 AM · Restricted Project
martong reclaimed D62131: [ASTImporter] Remove NonEquivalentDecls from ASTImporter.
Fri, Aug 23, 8:19 AM · Restricted Project
martong added a reviewer for D66538: [AST] AST structural equivalence to work internally with pairs.: a_sidorin.
Fri, Aug 23, 8:12 AM · Restricted Project
martong abandoned D62131: [ASTImporter] Remove NonEquivalentDecls from ASTImporter.

@a_sidorin Alexei,

Fri, Aug 23, 8:11 AM · Restricted Project
martong added a comment to D62131: [ASTImporter] Remove NonEquivalentDecls from ASTImporter.

Example about how to get wrong things into NonEquivalentDecls:
We want to compare class C for structural equivalence in the following codes:

class A {}; class B {int i;}; void x(A *); void y(A *); class C { friend void x(A *); friend void y(A *); };

and

class A {}; class B {int i;}; void x(A *); void y(B *); class C { friend void x(A *); friend void y(B *); };

The result is false for C but the NonEquivalentDecls will contain after the compare a void x(A*)<->void x(A*) pair.

The reason is that during the check we encounter first a A<->B pair (iterating over the friends and friend functions, first y is encountered and a check for A and B follows). The B is recorded as "tentative equivalence" to A. Then we try to check A to A (during check of x) but because there is a "tentative equivalence" with B from A the check returns false (not equivalent). This false result is recorded as a (incorrect) non-equivalence of the two x functions.
I want to replace the DeclsToCheck and TentativeEquivalences with a set of Decl pairs (like the NonEquivalentDecls) so it will be possible to record the same from Decl with multiple to values. (Is there a reason for why there is a NonEquivalentDecls cache but not a EquivalentDecls or simply a cache for result values?)

Fri, Aug 23, 7:43 AM · Restricted Project
martong added a comment to D66538: [AST] AST structural equivalence to work internally with pairs..

There is a third test which could be useful to test whether there is no faulty cache entries there:

Fri, Aug 23, 7:38 AM · Restricted Project
martong added a comment to D66538: [AST] AST structural equivalence to work internally with pairs..

I like this patch because it eliminates the need for checking the redeclaration chains.

Fri, Aug 23, 4:57 AM · Restricted Project
martong added a comment to D59692: [ASTImporter] Fix name conflict handling.

@shafik Ping

Fri, Aug 23, 4:26 AM · Restricted Project
martong added a comment to D66538: [AST] AST structural equivalence to work internally with pairs..

The link for the diff went off, sorry about that.
Here is the new link which is going to work:
https://github.com/martong/clang/compare/NonEqDecls_cache_in_an_upper_level_0...martong:NonEqDecls_cache_in_an_upper_level?expand=1

Fri, Aug 23, 1:52 AM · Restricted Project

Thu, Aug 22

martong added a comment to D66538: [AST] AST structural equivalence to work internally with pairs..

It looks like that the original code is correct in the decision of structural equivalence of the original pair. If we have an (A,B) and (A,C) to compare, B and C are in different decl chain, then (A,B) or (A,C) will be non-equivalent (unless B and C are equivalent, but what to do in this case?). The problem was that the code assumed that in this case always A and B (or A and C) are non-equivalent. If NonEquivalentDecls is not filled in this case (or not used at all) the problem does not exist.

Thu, Aug 22, 9:34 AM · Restricted Project
martong updated the diff for D66336: [ASTImporter] Add development internals docs.
  • Address review comments
Thu, Aug 22, 4:04 AM · Restricted Project
martong added a comment to D66336: [ASTImporter] Add development internals docs.

Thanks for the review!

Thu, Aug 22, 4:04 AM · Restricted Project
martong added inline comments to D66538: [AST] AST structural equivalence to work internally with pairs..
Thu, Aug 22, 2:51 AM · Restricted Project
martong added inline comments to D66538: [AST] AST structural equivalence to work internally with pairs..
Thu, Aug 22, 2:43 AM · Restricted Project

Wed, Aug 21

martong added a comment to D66348: [ASTImporter] Do not look up lambda classes.

I am not enthusiastic about this solution but I need to think about it some more.

We can see that p0624r2 added copy assignable lambdas:

bool f1() {
    auto x = []{} = {}; auto x2 = x;

    return x == x2;
}

bool f2() {
    auto x = []{} = {};
    auto xb = []{} = {};

    return x == xb;
}

see godbolt

So I don't think this is a long-term solution, although I don't know what clang is doing to make this work yet.

Wed, Aug 21, 5:36 AM · Restricted Project
martong updated the diff for D66348: [ASTImporter] Do not look up lambda classes.
  • Add tests for default constructible and assignable stateless lambdas
Wed, Aug 21, 5:28 AM · Restricted Project

Fri, Aug 16

martong added a comment to D64078: [ASTImporter] Fix structural ineq of lambdas with different sloc.

Hi Gabor,
it is a nice design question if source locations can participate in structural match or not. The comparison tells us that the same code written in different files is not structurally equivalent and I cannot agree with it. They can be not the same, but their structure is the same. The question is: why do we get to this comparison? Do we find a non-equivalent decl by lookup? I guess there can be another way to resolve this issue, and I'll be happy to help if you share what is the problem behind the patch.

Fri, Aug 16, 7:17 AM · Restricted Project
martong created D66348: [ASTImporter] Do not look up lambda classes.
Fri, Aug 16, 7:17 AM · Restricted Project
martong added inline comments to D59692: [ASTImporter] Fix name conflict handling.
Fri, Aug 16, 6:21 AM · Restricted Project
martong updated the diff for D59692: [ASTImporter] Fix name conflict handling.
  • Name -> SearchName
  • Add tests for conflicting declarations
Fri, Aug 16, 6:19 AM · Restricted Project
martong committed rGb9a8ac74f14d: Fix typos in LibASTImporter.rst (authored by martong).
Fix typos in LibASTImporter.rst
Fri, Aug 16, 5:22 AM
martong committed rL369099: Fix typos in LibASTImporter.rst.
Fix typos in LibASTImporter.rst
Fri, Aug 16, 5:22 AM
martong added a comment to D65573: Add User docs for ASTImporter.

That's incredible. Thank you!

Fri, Aug 16, 5:22 AM · Restricted Project, Restricted Project
martong added inline comments to D64464: [CodeGen] Emit destructor calls for non-trivial C structs.
Fri, Aug 16, 5:03 AM · Restricted Project
martong added a comment to D59692: [ASTImporter] Fix name conflict handling.

Just wanted to see if you were planning on landing this soon, the fix Name -> SearchName is probably an important one since we have seen several issues w/ bugs like that but I really would like to see more tests. Are you having issues coming up w/ tests?

Fri, Aug 16, 3:23 AM · Restricted Project
martong created D66336: [ASTImporter] Add development internals docs.
Fri, Aug 16, 2:56 AM · Restricted Project

Mon, Aug 12

martong committed rG7b4b3305fff3: [CrossTU] User docs: remove temporary limiation with macro expansion (authored by martong).
[CrossTU] User docs: remove temporary limiation with macro expansion
Mon, Aug 12, 5:48 AM
martong committed rL368562: [CrossTU] User docs: remove temporary limiation with macro expansion.
[CrossTU] User docs: remove temporary limiation with macro expansion
Mon, Aug 12, 5:47 AM

Sat, Aug 10

martong accepted D66054: [CrossTU] Fix problem with CrossTU AST load limit and progress messages..

LGTM!
Thank you.

Sat, Aug 10, 6:24 AM · Restricted Project, Restricted Project
martong added inline comments to D65577: [ASTImporter] Import default expression of param before creating the param..
Sat, Aug 10, 1:44 AM · Restricted Project

Fri, Aug 9

martong added a comment to D65999: [ASTImporter] Import additional flags for functions..

LGTM, but I don't exactly see how the first test is related. Could you please explain?

Fri, Aug 9, 3:35 AM · Restricted Project, Restricted Project
martong added a reviewer for D65999: [ASTImporter] Import additional flags for functions.: a_sidorin.
Fri, Aug 9, 3:29 AM · Restricted Project, Restricted Project

Thu, Aug 8

martong added a comment to D65577: [ASTImporter] Import default expression of param before creating the param..

Would it be an alternative solution if we imported the function parameters after we created the FunctionDecl? I guess it would be. Maybe that would result in a smaller change, what do you think?

Thu, Aug 8, 6:31 AM · Restricted Project
martong edited reviewers for D65935: [ASTImporter] Import ctor initializers after setting flags., added: a_sidorin; removed: a.sidorin.
Thu, Aug 8, 5:31 AM · Restricted Project, Restricted Project

Tue, Aug 6

martong accepted D65577: [ASTImporter] Import default expression of param before creating the param..

Still looks good to me, other than some style nits.

Tue, Aug 6, 3:49 AM · Restricted Project
martong committed rGf89c8f20e1e6: Add User docs for ASTImporter (authored by martong).
Add User docs for ASTImporter
Tue, Aug 6, 2:54 AM
martong committed rL368009: Add User docs for ASTImporter.
Add User docs for ASTImporter
Tue, Aug 6, 2:53 AM
martong closed D65573: Add User docs for ASTImporter.
Tue, Aug 6, 2:53 AM · Restricted Project, Restricted Project

Mon, Aug 5

martong updated the diff for D65573: Add User docs for ASTImporter.
  • Add description for -ast-merge
Mon, Aug 5, 5:33 AM · Restricted Project, Restricted Project

Fri, Aug 2

martong updated the diff for D65573: Add User docs for ASTImporter.
  • Address comments
Fri, Aug 2, 7:25 AM · Restricted Project, Restricted Project
martong added a comment to D65573: Add User docs for ASTImporter.

Thanks for the review!

Fri, Aug 2, 7:25 AM · Restricted Project, Restricted Project

Thu, Aug 1

martong edited reviewers for D65573: Add User docs for ASTImporter, added: dkrupp; removed: a.sidorin.
Thu, Aug 1, 9:26 AM · Restricted Project, Restricted Project
martong accepted D65577: [ASTImporter] Import default expression of param before creating the param..

LGTM, though I have some comments.

Thu, Aug 1, 8:32 AM · Restricted Project
martong accepted D64753: [CrossTU][NFCI] Refactor loadExternalAST function.

LGTM! Thanks! Please do the commit.

Thu, Aug 1, 6:38 AM · Restricted Project, Restricted Project
martong created D65573: Add User docs for ASTImporter.
Thu, Aug 1, 6:30 AM · Restricted Project, Restricted Project
martong resigned from D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure.
Thu, Aug 1, 2:36 AM · Restricted Project, Restricted Project, Restricted Project
martong accepted D65445: [CrossTU] Handle case when no USR could be generated during Decl search..

LGTM! Please proceed and commit! :)

Thu, Aug 1, 2:34 AM · Restricted Project, Restricted Project
martong added a comment to D65445: [CrossTU] Handle case when no USR could be generated during Decl search..

Probably this is a problem case too but only if the f or i has an initializer expression and really no USR is generated for f or i. But what I have found is that there is a VarDecl for a "invisible" variable whose type is the anonymous union and it has a (default) initializer too. If the search reaches DC of the f it will try to get the USR for this variable.

Thu, Aug 1, 2:34 AM · Restricted Project, Restricted Project

Wed, Jul 31

martong added inline comments to D64753: [CrossTU][NFCI] Refactor loadExternalAST function.
Wed, Jul 31, 5:41 AM · Restricted Project, Restricted Project
martong added a comment to D65445: [CrossTU] Handle case when no USR could be generated during Decl search..

It looks like that the problem can happen when the anonymous union is in any DeclContext and for CTU import the import of a variable is requested and that variable is in a related DeclContext (it can be at upper or lower level). (See code of findDefInDeclContext: If used for variables, it goes through every DeclContext and computes USR for specific VarDecls until the one is found. If we have luck it may be found before the union. An anonymous union can not be imported because we get no USR for it so it is OK to ignore these.) So the test can be written in many other forms but this case (the code of f) was encountered in protobuf code.

Wed, Jul 31, 5:25 AM · Restricted Project, Restricted Project

Tue, Jul 30

martong added a comment to D65445: [CrossTU] Handle case when no USR could be generated during Decl search..

Thank you for this patch Balazs! Could you please elaborate in which cases we cannot generate the USR? Does it happen only if we have an anonymous union inside a function? Are there no other cases?

Tue, Jul 30, 8:06 AM · Restricted Project, Restricted Project

Mon, Jul 29

martong added a comment to D65203: [ASTImporter] Do not import FunctionTemplateDecl in record twice..

@a_sidorin This is a polite Ping.

Mon, Jul 29, 8:43 AM · Restricted Project, Restricted Project

Jul 25 2019

martong added a reviewer for D65203: [ASTImporter] Do not import FunctionTemplateDecl in record twice.: a_sidorin.
Jul 25 2019, 4:41 AM · Restricted Project, Restricted Project
martong added a reviewer for D65269: [ASTImporter] Fix for import of friend class template with definition.: a_sidorin.
Jul 25 2019, 4:39 AM · Restricted Project, Restricted Project
martong added a comment to D44100: [ASTImporter] Reorder fields after structure import is finished.

Jenkins is green: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31743/

Jul 25 2019, 2:49 AM · Restricted Project, Restricted Project
martong committed rG48b16e1005df: [ASTImporter] Reorder fields after structure import is finished (authored by martong).
[ASTImporter] Reorder fields after structure import is finished
Jul 25 2019, 2:10 AM
martong committed rL366997: [ASTImporter] Reorder fields after structure import is finished.
[ASTImporter] Reorder fields after structure import is finished
Jul 25 2019, 2:07 AM
martong closed D44100: [ASTImporter] Reorder fields after structure import is finished.
Jul 25 2019, 2:07 AM · Restricted Project, Restricted Project

Jul 24 2019

martong added a comment to D64753: [CrossTU][NFCI] Refactor loadExternalAST function.

I have a feeling that LoadPass and LoagGuard could be (should be) merged together, other than that we are getting close.

Jul 24 2019, 2:32 AM · Restricted Project, Restricted Project

Jul 23 2019

martong added inline comments to D44100: [ASTImporter] Reorder fields after structure import is finished.
Jul 23 2019, 9:22 AM · Restricted Project, Restricted Project
martong updated the diff for D44100: [ASTImporter] Reorder fields after structure import is finished.
  • Address Alexei's comments
Jul 23 2019, 9:22 AM · Restricted Project, Restricted Project
martong updated the diff for D59692: [ASTImporter] Fix name conflict handling.
  • Rebase to master
Jul 23 2019, 9:01 AM · Restricted Project
martong committed rG123f6ff299ea: [ASTImporter] Fix inequivalence of ClassTemplateInstantiations (authored by martong).
[ASTImporter] Fix inequivalence of ClassTemplateInstantiations
Jul 23 2019, 8:48 AM
martong committed rL366818: [ASTImporter] Fix inequivalence of ClassTemplateInstantiations.
[ASTImporter] Fix inequivalence of ClassTemplateInstantiations
Jul 23 2019, 8:48 AM
martong closed D64241: [ASTImporter] Fix inequivalence of ClassTemplateInstantiations.
Jul 23 2019, 8:48 AM · Restricted Project, Restricted Project
martong resigned from D65128: [Logging] Replace Log::Printf with LLDB_LOG macro (NFC).
Jul 23 2019, 8:00 AM · Restricted Project, Restricted Project
martong updated subscribers of D64638: [CrossTU] Fix plist macro expansion if macro in other file..

StaticAnalyzer/Core does not depend on clangFrontend now, you can see this by looking at lib/StaticAnalyzer/Core/CMakeLists.txt:

add_clang_library(clangStaticAnalyzerCore
...
  LINK_LIBS
  clangAST
  clangASTMatchers
  clangAnalysis
  clangBasic
  clangCrossTU
  clangLex
  clangRewrite
  )

Not a StaticAnalyzer expert, so I don't know whether it's acceptable to add this dependency to clangStaticAnalyzerCore, you'll have to find someone who owns the code to know whether this dependency is justified.
(My wild guess from looking at the names of the libraries would be that this dependency is not ok and the code should go into clangStaticAnalyzerFrontend instead. But again, not an expert here, just a guess).

But please add a dependency into LINK_LIBS inside CMakeLists.txt if you start depending on clangFrontend.
Most of these violations are found if you build in a cmake -DBUILD_SHARED_LIBS=On configuration.

Jul 23 2019, 6:55 AM · Restricted Project, Restricted Project

Jul 22 2019

martong added a comment to D64241: [ASTImporter] Fix inequivalence of ClassTemplateInstantiations.

@a_sidorin This is a polite Ping.

Jul 22 2019, 8:21 AM · Restricted Project, Restricted Project
martong added a comment to D44100: [ASTImporter] Reorder fields after structure import is finished.

@a_sidorin, @shafik This is a polite Ping.

Jul 22 2019, 7:26 AM · Restricted Project, Restricted Project
martong added inline comments to D65064: [CrossTU] Add a function to retrieve original source location..
Jul 22 2019, 2:45 AM · Restricted Project, Restricted Project
martong accepted D65064: [CrossTU] Add a function to retrieve original source location..

LGTM! Thanks!

Jul 22 2019, 2:38 AM · Restricted Project, Restricted Project
martong resigned from D65042: [Concept] Placeholder constraints and abbreviated templates.
Jul 22 2019, 2:28 AM · Restricted Project
martong added a comment to D65042: [Concept] Placeholder constraints and abbreviated templates.

ASTImporter.cpp and ASTStructuralEquivalence.cpp looks good to me!

Jul 22 2019, 2:26 AM · Restricted Project

Jul 19 2019

martong updated the diff for D44100: [ASTImporter] Reorder fields after structure import is finished.
  • Further simplify by removing the last for loop
Jul 19 2019, 10:43 AM · Restricted Project, Restricted Project
martong updated the diff for D44100: [ASTImporter] Reorder fields after structure import is finished.
  • Rebase to master
  • Some refactor is done mostly because since D63603 ([ASTImporter] Propagate

error from ImportDeclContext) we may not imported successfully all decls of a
DC.

  • Made the code simpler and shorter by adding a local vector to hold the decls

of the "from" DC in the original order.

Jul 19 2019, 6:17 AM · Restricted Project, Restricted Project

Jul 18 2019

martong added a comment to D64241: [ASTImporter] Fix inequivalence of ClassTemplateInstantiations.

Hi Gabor,
This looks fine, but could you please add a test showing how decl shadowing is handled? I.e. if we have Arg in one TU and both Arg and N::Arg in another TU.

Jul 18 2019, 8:25 AM · Restricted Project, Restricted Project
martong updated the diff for D64241: [ASTImporter] Fix inequivalence of ClassTemplateInstantiations.
  • Add test case ClassTemplSpecWithInequivalentShadowedTemplArg
Jul 18 2019, 8:25 AM · Restricted Project, Restricted Project
martong committed rG6d3bb71c8f80: [analyzer] Add CTU user docs (authored by martong).
[analyzer] Add CTU user docs
Jul 18 2019, 7:05 AM
martong committed rL366439: [analyzer] Add CTU user docs.
[analyzer] Add CTU user docs
Jul 18 2019, 7:04 AM
martong closed D64801: [analyzer] Add CTU user docs.
Jul 18 2019, 7:04 AM · Restricted Project, Restricted Project
martong updated the diff for D64801: [analyzer] Add CTU user docs.
  • Add notes about macro expansion crash with CTU
Jul 18 2019, 6:47 AM · Restricted Project, Restricted Project
martong accepted D64554: [CrossTU] Add a function to retrieve original source location..

LGTM.

Jul 18 2019, 6:38 AM · Restricted Project, Restricted Project
martong added inline comments to D64554: [CrossTU] Add a function to retrieve original source location..
Jul 18 2019, 1:23 AM · Restricted Project, Restricted Project

Jul 17 2019

martong added a comment to D64075: [ASTImporter] Fix structural eq of lambdas.

Jenkins looks okay: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31160/ .The build is red but the the previous run has the very same failing test case:
expression_command/weak_symbols/TestWeakSymbols.py

Jul 17 2019, 9:23 AM · Restricted Project, Restricted Project
martong updated the diff for D64801: [analyzer] Add CTU user docs.
  • Address dkrupp's comments
Jul 17 2019, 9:01 AM · Restricted Project, Restricted Project
martong committed rGae512b83d5fc: [ASTImporter] Fix structural eq of lambdas (authored by martong).
[ASTImporter] Fix structural eq of lambdas
Jul 17 2019, 7:41 AM
martong committed rL366332: [ASTImporter] Fix structural eq of lambdas.
[ASTImporter] Fix structural eq of lambdas
Jul 17 2019, 7:41 AM
martong closed D64075: [ASTImporter] Fix structural eq of lambdas.
Jul 17 2019, 7:40 AM · Restricted Project, Restricted Project
martong added a comment to D64075: [ASTImporter] Fix structural eq of lambdas.

@shafik I've been looking for any lldb regression in our Mac machine, could not find any. Now I am looking at http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/ .

Jul 17 2019, 7:35 AM · Restricted Project, Restricted Project
martong added a comment to D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src.

Jenkins looks okay: http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31157/ .The build is red but the the previous run has the very same failing test case:
expression_command/weak_symbols/TestWeakSymbols.py

Jul 17 2019, 7:29 AM · Restricted Project, Restricted Project, Restricted Project
martong committed rGaefcf5100aae: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src (authored by martong).
[ASTImporter] Fix LLDB lookup in transparent ctx and with ext src
Jul 17 2019, 6:49 AM
martong committed rL366325: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src.
[ASTImporter] Fix LLDB lookup in transparent ctx and with ext src
Jul 17 2019, 6:48 AM
martong closed D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src.
Jul 17 2019, 6:48 AM · Restricted Project, Restricted Project, Restricted Project
martong added a comment to D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src.

Thank you guys for the review!

Jul 17 2019, 6:48 AM · Restricted Project, Restricted Project, Restricted Project
martong added inline comments to D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src.
Jul 17 2019, 3:05 AM · Restricted Project, Restricted Project, Restricted Project
martong updated the diff for D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src.
  • Applied clang-format on lldb parts (this changed two lines)
  • Added a comment for predicate
  • Merged the test into TestCModules.py
Jul 17 2019, 3:05 AM · Restricted Project, Restricted Project, Restricted Project

Jul 16 2019

martong created D64801: [analyzer] Add CTU user docs.
Jul 16 2019, 8:35 AM · Restricted Project, Restricted Project
martong added inline comments to D64554: [CrossTU] Add a function to retrieve original source location..
Jul 16 2019, 6:36 AM · Restricted Project, Restricted Project
martong added inline comments to D64554: [CrossTU] Add a function to retrieve original source location..
Jul 16 2019, 6:35 AM · Restricted Project, Restricted Project
martong added inline comments to D64554: [CrossTU] Add a function to retrieve original source location..
Jul 16 2019, 3:21 AM · Restricted Project, Restricted Project