Page MenuHomePhabricator

martong (Gabor Marton)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 10 2017, 8:01 AM (119 w, 3 d)

Recent Activity

Wed, Jan 22

martong accepted D73166: [ASTImporter] Properly delete decls from SavedImportPaths.

Thanks for the fix! Looks good to me!

Wed, Jan 22, 5:38 AM · Restricted Project

Tue, Jan 21

martong added a comment to D71612: [analyzer] Add PlacementNewChecker.
In D71612#1828059, @NoQ wrote:

This looks fantastic. Let's enable by it default!

Tue, Jan 21, 4:29 AM · Restricted Project
martong committed rGbc29069dc401: [analyzer] Enable PlacementNewChecker by default (authored by martong).
[analyzer] Enable PlacementNewChecker by default
Tue, Jan 21, 4:25 AM

Wed, Jan 15

martong added inline comments to D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc).
Wed, Jan 15, 8:16 AM · Restricted Project
martong updated the diff for D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc).
  • Move the setting of parameters of FunctionProtoTypeLoc to TypeLocImporter
  • Add tests for TypeLoc import in case of functions
Wed, Jan 15, 8:16 AM · Restricted Project

Tue, Jan 14

martong added a comment to D71612: [analyzer] Add PlacementNewChecker.
In D71612#1812476, @NoQ wrote:
In D71612#1790045, @NoQ wrote:

I wonder if this checker will find any misuses of placement into llvm::TrailingObjects.

I've evaluated this checker on LLVM/Clang source and it did not find any results, though there are many placement new calls there. I think this is good, because there are no false positives.

In such cases it's usually a good idea to verify that the checker works correctly by artificially injecting a bug into the codebase. If the bug is not found, the checker is probably not working. If the bug is found, change it to be more and more realistic, so that to see what limitations does the checker have in terms of false negatives. Monitor analyzer stats closely (max-nodes limits, block count limits, inlining limits) in order to see what exactly goes wrong (or debug on the Exploded Graph as usual, depending on how it goes wrong). This exercise often uncovers interesting omissions :)

Can we enable the check by default then? What pieces are missing? Like, the symbolic extent/offset case is not a must. I think we have everything except maybe some deeper testing. I'd rather spend some time on the above exercise, but then enable by default if it goes well.

Tue, Jan 14, 8:22 AM · Restricted Project

Fri, Jan 10

martong added a comment to D71612: [analyzer] Add PlacementNewChecker.

Related commit that moves to alpha:
https://github.com/llvm/llvm-project/commit/13ec473b9d4bd4f7a558272932b7c0806171c666

Fri, Jan 10, 10:42 AM · Restricted Project
martong committed rG13ec473b9d4b: [analyzer] Move PlacementNewChecker to alpha (authored by martong).
[analyzer] Move PlacementNewChecker to alpha
Fri, Jan 10, 10:41 AM
martong added a comment to D71612: [analyzer] Add PlacementNewChecker.
In D71612#1814225, @NoQ wrote:

I am going to execute the analysis on LLVM/Clang as you suggested, so then we'll see what the experiment will bring us. And then we could enable it by default I think. BTW how can we do that? I could not find any default enablement related config in Checkers.td ...

Enabling checkers by default is controlled by the messy code in RenderAnalyzerOptions() in the Driver.

Wait, you already made it on-by-default. Checkers that are currently under development go into the alpha package.

Fri, Jan 10, 9:27 AM · Restricted Project
martong committed rG5e7beb0a4146: [analyzer] Add PlacementNewChecker (authored by martong).
[analyzer] Add PlacementNewChecker
Fri, Jan 10, 9:03 AM
martong closed D71612: [analyzer] Add PlacementNewChecker.
Fri, Jan 10, 9:03 AM · Restricted Project
martong updated the diff for D71612: [analyzer] Add PlacementNewChecker.
  • Declare ElementCountNL in if stmt
  • Use makeArrayIndex instead makeIntVal
Fri, Jan 10, 5:16 AM · Restricted Project
martong added a comment to D71612: [analyzer] Add PlacementNewChecker.

Can we enable the check by default then? What pieces are missing? Like, the symbolic extent/offset case is not a must. I think we have everything except maybe some deeper testing. I'd rather spend some time on the above exercise, but then enable by default if it goes well.

Fri, Jan 10, 5:16 AM · Restricted Project

Thu, Jan 9

martong added a comment to D71612: [analyzer] Add PlacementNewChecker.
In D71612#1790045, @NoQ wrote:

I wonder if this checker will find any misuses of placement into llvm::TrailingObjects.

Thu, Jan 9, 6:46 AM · Restricted Project
martong updated the diff for D71612: [analyzer] Add PlacementNewChecker.
  • Add documentation to checkers.rst
  • Pass CheckerContext as last param
  • Use BugType and default member initializer
Thu, Jan 9, 6:46 AM · Restricted Project
martong added inline comments to D71612: [analyzer] Add PlacementNewChecker.
Thu, Jan 9, 6:46 AM · Restricted Project

Tue, Jan 7

martong resigned from D71920: [AST] Refactor propagation of dependency bits. NFC.
Tue, Jan 7, 7:42 AM · Restricted Project
martong resigned from D50119: P1144 "Trivially relocatable" (0/3): Compiler support for `__is_trivially_relocatable(T)`.
Tue, Jan 7, 7:35 AM · Restricted Project

Dec 20 2019

martong updated the diff for D71612: [analyzer] Add PlacementNewChecker.
  • Better handling of unknown values
Dec 20 2019, 7:48 AM · Restricted Project
martong added inline comments to D71612: [analyzer] Add PlacementNewChecker.
Dec 20 2019, 5:09 AM · Restricted Project
martong updated the diff for D71612: [analyzer] Add PlacementNewChecker.
  • Bugtype by value
  • Decompose Place to base region and offset
  • Add test for type hierarchy
  • Remove State check
  • Return directly with the size of the type in case of non-array new
  • Use Optional for ElementCount
  • Update warning message
Dec 20 2019, 4:02 AM · Restricted Project
martong added a comment to D71612: [analyzer] Add PlacementNewChecker.

Thank you guys for the assiduous review!

Dec 20 2019, 4:02 AM · Restricted Project
martong added inline comments to D71734: [Modules] Handle tag types and complain about bad merges in C/Objective-C mode.
Dec 20 2019, 2:30 AM · Restricted Project

Dec 19 2019

martong accepted D71378: Modifying ImportDeclContext(...) to ensure that we complete each FieldDecl of a RecordDecl when we are importing the definiton.
Dec 19 2019, 2:25 AM · Restricted Project, Restricted Project
martong resigned from D71630: [lldb] Add a SubsystemRAII that takes care of calling Initialize and Terminate in the unit tests.
Dec 19 2019, 2:25 AM · Restricted Project

Dec 18 2019

martong added a comment to D71020: [ASTImporter] Friend class decl should not be visible in its context.

Recommited in bc5b7e21e32 . I changed llvm:is_contained to a simple for loop over the lookup result. This way the copy assignment of the iterator is avoided even if windows STL is used.

Dec 18 2019, 2:59 AM · Restricted Project
martong committed rGbc5b7e21e32b: recommit: [ASTImporter] Friend class decl should not be visible in its context (authored by martong).
recommit: [ASTImporter] Friend class decl should not be visible in its context
Dec 18 2019, 2:58 AM

Dec 17 2019

martong created D71612: [analyzer] Add PlacementNewChecker.
Dec 17 2019, 8:28 AM · Restricted Project
martong committed rG4becf68c6f17: [ASTImporter] Friend class decl should not be visible in its context (authored by martong).
[ASTImporter] Friend class decl should not be visible in its context
Dec 17 2019, 5:51 AM
martong closed D71020: [ASTImporter] Friend class decl should not be visible in its context.
Dec 17 2019, 5:51 AM · Restricted Project
martong added a comment to D71562: [lldb] Remove modern-type-lookup.

I'm curious what you think should happen to the clang-import-test. We could either rewrite the tests as unit tests in the ASTImporterTest you guys are already using or we move the necessary parts of the ExternalASTMerger into the clang-import-test

Dec 17 2019, 4:06 AM · Restricted Project

Dec 16 2019

martong added inline comments to D71020: [ASTImporter] Friend class decl should not be visible in its context.
Dec 16 2019, 7:48 AM · Restricted Project
martong updated the diff for D71020: [ASTImporter] Friend class decl should not be visible in its context.

Simplify getRecordDeclOfFriend() further.

Dec 16 2019, 7:48 AM · Restricted Project

Dec 12 2019

martong added inline comments to D71020: [ASTImporter] Friend class decl should not be visible in its context.
Dec 12 2019, 1:26 PM · Restricted Project
martong updated the diff for D71020: [ASTImporter] Friend class decl should not be visible in its context.

Addressing Alexeis comments.

Dec 12 2019, 1:26 PM · Restricted Project
martong added a comment to D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc).

Are you going to add any tests in the following patches?

I don't have any tests for now, but in a later patch we can create them.

Dec 12 2019, 1:06 PM · Restricted Project
martong committed rG25234fd69e32: [ASTImporter] Support functions with placeholder return types ... (authored by martong).
[ASTImporter] Support functions with placeholder return types ...
Dec 12 2019, 8:51 AM
martong closed D70819: [ASTImporter] Support functions with placeholder return types ....
Dec 12 2019, 8:51 AM · Restricted Project
martong added a comment to D70819: [ASTImporter] Support functions with placeholder return types ....

Apologies for wacky C++ code that follows but will this also work for the following cases:

auto f2() { 
  auto l = []() {
      struct X{};
      return X();
  };
 return l(); 
 }

 auto f3() { 
  if ( struct X{} x; true) 
      return X();
  else return X();    
 }

 auto f4() {
    for(struct X{} x;;)
       return X();
 }

 auto f5() {
    switch(struct X{} x; 10) {
      case 10:
       return X();
    }
 }

godbolt live example

Thanks for these cases! I am going to write unit tests for these as well.

Dec 12 2019, 8:14 AM · Restricted Project
martong committed rG4cfb91f1ef1b: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools (authored by martong).
[Analyzer][Docs][NFC] Add CodeChecker to the command line tools
Dec 12 2019, 5:28 AM
martong closed D70439: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools.
Dec 12 2019, 5:28 AM · Restricted Project
martong added a comment to D70819: [ASTImporter] Support functions with placeholder return types ....

Apologies for wacky C++ code that follows but will this also work for the following cases:

auto f2() { 
  auto l = []() {
      struct X{};
      return X();
  };
 return l(); 
 }

 auto f3() { 
  if ( struct X{} x; true) 
      return X();
  else return X();    
 }

 auto f4() {
    for(struct X{} x;;)
       return X();
 }

 auto f5() {
    switch(struct X{} x; 10) {
      case 10:
       return X();
    }
 }

godbolt live example

Dec 12 2019, 5:10 AM · Restricted Project
martong added a comment to D71378: Modifying ImportDeclContext(...) to ensure that we complete each FieldDecl of a RecordDecl when we are importing the definiton.

Just one more thing, maybe that is too overkill, but I think on a long term we could benefit from a unittest for this case. You could create a test similar to LLDBLookupTest in ASTImporterTest.cpp. In that Fixture we use Minimal import and the regular lookup (that is the exact case in LLDB).
In the test itself we could call ImportDefinition on a struct that has fields with record types. And then we could assert that all field's type's have complete types.

Dec 12 2019, 5:03 AM · Restricted Project, Restricted Project
martong added a comment to D71378: Modifying ImportDeclContext(...) to ensure that we complete each FieldDecl of a RecordDecl when we are importing the definiton.

Thanks for the patch! It look almost good to me, but I have a comment about the error handling.

Dec 12 2019, 5:01 AM · Restricted Project, Restricted Project

Dec 7 2019

martong added a reviewer for D70819: [ASTImporter] Support functions with placeholder return types ...: teemperor.
Dec 7 2019, 5:04 AM · Restricted Project

Dec 6 2019

martong accepted D71112: [ASTImporter] Implicitly declare parameters for imported ObjCMethodDecls.
Dec 6 2019, 7:15 AM · Restricted Project
martong added inline comments to D71112: [ASTImporter] Implicitly declare parameters for imported ObjCMethodDecls.
Dec 6 2019, 6:21 AM · Restricted Project
martong updated the diff for D70439: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools.

Put back the menu, add "open source" property.

Dec 6 2019, 6:03 AM · Restricted Project
martong added a comment to D70439: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools.

I would change the order of CCh and scan-build because we usually list stuff in alphabetical order. Also the chronological order is that, the newest is the first.

Dec 6 2019, 6:03 AM · Restricted Project
martong added a comment to D71112: [ASTImporter] Implicitly declare parameters for imported ObjCMethodDecls.

Thanks for addressing this, I just have some minor comments.

Dec 6 2019, 5:43 AM · Restricted Project

Dec 5 2019

martong added inline comments to D70439: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools.
Dec 5 2019, 9:46 AM · Restricted Project
martong updated the diff for D70439: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools.

Addressing Artem's review comments.
Artem, thanks for the review!

Dec 5 2019, 9:45 AM · Restricted Project
martong updated the diff for D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc).

IMPORT -> IMPORT_TYPE_LOC

Dec 5 2019, 6:29 AM · Restricted Project

Dec 4 2019

martong created D71020: [ASTImporter] Friend class decl should not be visible in its context.
Dec 4 2019, 8:17 AM · Restricted Project
martong added a child revision for D60499: [ASTImporter] Various source location and range import fixes.: D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc).
Dec 4 2019, 7:58 AM · Restricted Project
martong added a parent revision for D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc): D60499: [ASTImporter] Various source location and range import fixes..
Dec 4 2019, 7:58 AM · Restricted Project
martong created D71018: [ASTImporter] Improved import of TypeSourceInfo (TypeLoc).
Dec 4 2019, 7:58 AM · Restricted Project

Dec 3 2019

martong added a comment to D60499: [ASTImporter] Various source location and range import fixes..

Ping

Dec 3 2019, 10:15 AM · Restricted Project
martong added a comment to D70439: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools.

@NoQ Ping.

Dec 3 2019, 7:27 AM · Restricted Project
martong accepted D69948: [Checkers] Added support for freopen to StreamChecker..

LGTM, but wait for others please.

Dec 3 2019, 7:27 AM · Restricted Project

Dec 2 2019

martong added a comment to D70819: [ASTImporter] Support functions with placeholder return types ....

Alexei, thanks for the assiduous review!

Dec 2 2019, 9:11 AM · Restricted Project
martong updated the diff for D70819: [ASTImporter] Support functions with placeholder return types ....
  • Address Alexei's comments
Dec 2 2019, 9:03 AM · Restricted Project
martong added inline comments to D70819: [ASTImporter] Support functions with placeholder return types ....
Dec 2 2019, 9:02 AM · Restricted Project

Nov 28 2019

martong updated the summary of D70819: [ASTImporter] Support functions with placeholder return types ....
Nov 28 2019, 8:05 AM · Restricted Project
martong updated the summary of D70819: [ASTImporter] Support functions with placeholder return types ....
Nov 28 2019, 8:05 AM · Restricted Project
martong added inline comments to D69948: [Checkers] Added support for freopen to StreamChecker..
Nov 28 2019, 7:52 AM · Restricted Project
martong added a comment to D70439: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools.

@Szelethus, Kristof thanks for the review.
@NoQ Ping. I'd like to have an approve from somebody who is outside the CodeChecker/E/// gang.

Nov 28 2019, 6:28 AM · Restricted Project
martong created D70819: [ASTImporter] Support functions with placeholder return types ....
Nov 28 2019, 6:16 AM · Restricted Project

Nov 19 2019

martong created D70439: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools.
Nov 19 2019, 6:05 AM · Restricted Project

Nov 14 2019

martong added a comment to D60499: [ASTImporter] Various source location and range import fixes..

Ping @shafik @balazske

Nov 14 2019, 6:08 AM · Restricted Project
martong accepted D67543: [Clang][ASTImporter] Added visibility check for ClassTemplateDecl..
Nov 14 2019, 5:58 AM · Restricted Project

Nov 5 2019

martong resigned from D68108: Redeclare Objective-C property accessors inside the ObjCImplDecl in which they are synthesized..
Nov 5 2019, 2:18 AM · Restricted Project

Oct 30 2019

martong resigned from D69322: [hip][cuda] Enable extended lambda support on Windows..
Oct 30 2019, 2:31 AM · Restricted Project

Oct 29 2019

martong added inline comments to D69360: [NFC] Refactor representation of materialized temporaries.
Oct 29 2019, 9:38 AM · Restricted Project, Restricted Project
martong accepted D69566: [ASTImporter] Add support for BuiltinTemplateDecl.

LGTM! Thanks!

Oct 29 2019, 9:29 AM · Restricted Project
GitHub <noreply@github.com> committed rGa6358f4edb13: Merge 9099b02638758ef01dc887a8078e7300a66aa82e into… (authored by martong).
Merge 9099b02638758ef01dc887a8078e7300a66aa82e into…
Oct 29 2019, 7:52 AM
martong committed rG9099b0263875: Add CodeChecker to the command line tools (authored by martong).
Add CodeChecker to the command line tools
Oct 29 2019, 7:52 AM

Oct 9 2019

martong retitled D68634: [ASTImporter] Imported FunctionDecls inherit __attribute__((noreturn)) and analyzer_noreturn attrs from existing functions from [ASTImporter] Imported FunctionDecls inherit attributes from existing functions to [ASTImporter] Imported FunctionDecls inherit __attribute__((noreturn)) and analyzer_noreturn attrs from existing functions.
Oct 9 2019, 5:16 AM · Restricted Project
martong added a comment to D68634: [ASTImporter] Imported FunctionDecls inherit __attribute__((noreturn)) and analyzer_noreturn attrs from existing functions.

Hello Balasz,
In my opinion, importing and setting the attributes should be handled by the stuff used in InitializeImportedDecl(). Can we extend it or reuse the code? It will allow us not to miss the required actions while importing a new function too.

I was thinking about that too, but it turned out to be a way more intrusive change.

We have to work with the most recent existing decl (PrevDecl) of the FunctionDecl whose attribute we currently import.
We can get the PrevDecl only with the help of the lookup. We can't get it from the ToD param of InitializeImportedDecl because by the time when we call InitializeImportedDecl the redecl chain is not connected. So, we should pass PrevDecl then into GetImportedOrCreateDecl. It would require to change all call expressions of GetImportedOrCreateDecl (58 occurences). Besides, we would use the PrevDecl only in case of inheritable attributes, only they require this special care. Note that there are a bunch of non-inheritable attributes which are already handled correctly in InitializeImportedDecl.

Oct 9 2019, 5:13 AM · Restricted Project
martong added a comment to D68634: [ASTImporter] Imported FunctionDecls inherit __attribute__((noreturn)) and analyzer_noreturn attrs from existing functions.

Hello Balasz,
In my opinion, importing and setting the attributes should be handled by the stuff used in InitializeImportedDecl(). Can we extend it or reuse the code? It will allow us not to miss the required actions while importing a new function too.

Oct 9 2019, 5:04 AM · Restricted Project

Oct 8 2019

martong created D68634: [ASTImporter] Imported FunctionDecls inherit __attribute__((noreturn)) and analyzer_noreturn attrs from existing functions.
Oct 8 2019, 4:33 AM · Restricted Project

Oct 7 2019

martong committed rG305a11d40911: [ASTImporter][NFC] Enable disabled but passing test (authored by martong).
[ASTImporter][NFC] Enable disabled but passing test
Oct 7 2019, 10:11 PM
martong committed rL373896: [ASTImporter][NFC] Enable disabled but passing test.
[ASTImporter][NFC] Enable disabled but passing test
Oct 7 2019, 10:10 PM
martong committed rG8f7fbed85e4b: [ASTImporter][NFC] Update ASTImporter internals docs (authored by martong).
[ASTImporter][NFC] Update ASTImporter internals docs
Oct 7 2019, 10:10 PM
martong committed rG579882ae4407: [ASTImporter][NFC] Fix typo in user docs (authored by martong).
[ASTImporter][NFC] Fix typo in user docs
Oct 7 2019, 10:10 PM
martong committed rL373895: [ASTImporter][NFC] Update ASTImporter internals docs.
[ASTImporter][NFC] Update ASTImporter internals docs
Oct 7 2019, 10:10 PM
martong committed rL373894: [ASTImporter][NFC] Fix typo in user docs.
[ASTImporter][NFC] Fix typo in user docs
Oct 7 2019, 10:10 PM

Oct 3 2019

martong accepted D68326: [lldb][modern-type-lookup] No longer import temporary declarations into the persistent AST.

LGTM!

Oct 3 2019, 9:15 AM · Restricted Project, Restricted Project

Oct 2 2019

martong added a comment to D68326: [lldb][modern-type-lookup] No longer import temporary declarations into the persistent AST.

With [modern-type-lookup], we completely evade the use of ASTImporterDelegate? That would be a wonderful thing to use only the the ExternalASTMerger on a long term...

Oct 2 2019, 7:07 AM · Restricted Project, Restricted Project

Oct 1 2019

martong added a comment to D68140: [lldb][clang][modern-type-lookup] Use ASTImporterSharedState in ExternalASTMerger.

Raphael, thanks for working on this. Overall, the changes look good to me, but please see my comment for the constructor.

Oct 1 2019, 2:58 AM · Restricted Project, Restricted Project

Sep 24 2019

martong committed rG9223d438db3a: [ASTImporter] 4th attempt to fix Windows buildbot test errors (authored by martong).
[ASTImporter] 4th attempt to fix Windows buildbot test errors
Sep 24 2019, 2:01 AM
martong committed rL372705: [ASTImporter] 4th attempt to fix Windows buildbot test errors.
[ASTImporter] 4th attempt to fix Windows buildbot test errors
Sep 24 2019, 1:59 AM

Sep 23 2019

martong committed rG3135a01da825: [ASTImporter] 3rd attempt to fix Windows buildbot test errors (authored by martong).
[ASTImporter] 3rd attempt to fix Windows buildbot test errors
Sep 23 2019, 10:49 PM
martong committed rL372688: [ASTImporter] 3rd attempt to fix Windows buildbot test errors.
[ASTImporter] 3rd attempt to fix Windows buildbot test errors
Sep 23 2019, 10:48 PM
martong committed rG174d43d123f5: [ASTImporter] 2nd attempt to fix Windows buildbot test errors (authored by martong).
[ASTImporter] 2nd attempt to fix Windows buildbot test errors
Sep 23 2019, 12:56 PM
martong committed rL372646: [ASTImporter] 2nd attempt to fix Windows buildbot test errors.
[ASTImporter] 2nd attempt to fix Windows buildbot test errors
Sep 23 2019, 12:52 PM
martong added a comment to D66951: [ASTImporter] Add comprehensive tests for ODR violation handling strategies.

2nd attempt to fix the windows build errors: 372646

Sep 23 2019, 12:49 PM · Restricted Project, Restricted Project
martong committed rG4d51c6ff2311: [ASTImporter] Attempt to fix Windows buildbot test errors (authored by martong).
[ASTImporter] Attempt to fix Windows buildbot test errors
Sep 23 2019, 10:28 AM
martong added a comment to D66951: [ASTImporter] Add comprehensive tests for ODR violation handling strategies.

Trying to fix in svn commit 372633.

Sep 23 2019, 10:28 AM · Restricted Project, Restricted Project
martong committed rL372633: [ASTImporter] Attempt to fix Windows buildbot test errors.
[ASTImporter] Attempt to fix Windows buildbot test errors
Sep 23 2019, 10:27 AM