Page MenuHomePhabricator

parallaxe (Hendrik von Prince)
User

Projects

User does not belong to any projects.

User Details

User Since
Jun 24 2014, 3:48 PM (285 w, 4 d)

Recent Activity

Oct 4 2016

parallaxe added a comment to D23236: When ARC is enabled, no warning will be generated when a method 1. Returns 'nil' in a method that is attributed to return a 'nonnull' 2. The return-statement is a ConditionalOperator, where the lhs is nil and rhs an objC-method-call (or the other....

Upon reflection, I don't think this is the right approach.

Desugaring any AttributedType in the return type seems like a really, really big hammer and could be an unexpected surprise for future attributed types where this is not the right behavior. I think a better approach for the nullability issue would be to update lookThroughImplicitCasts() in NullabilityChecker.cpp to look though ExprWithCleanups expressions just like it currently does for implicit casts.

What do you think?

Oct 4 2016, 2:15 PM

Sep 28 2016

parallaxe added a comment to D23236: When ARC is enabled, no warning will be generated when a method 1. Returns 'nil' in a method that is attributed to return a 'nonnull' 2. The return-statement is a ConditionalOperator, where the lhs is nil and rhs an objC-method-call (or the other....

I tried to commit by using arc, though it didn't work as I was prompted to login into the subversion-repository, which I can't. At this point I'm not sure if this should have worked or not, but I assumed that arc + phabricator would handle it in a way that i wouldn't need write-access to the repository itself.
So, yes, it would be nice when someone can actually commit it for me.

Sep 28 2016, 2:18 PM

Sep 24 2016

parallaxe added a comment to D23236: When ARC is enabled, no warning will be generated when a method 1. Returns 'nil' in a method that is attributed to return a 'nonnull' 2. The return-statement is a ConditionalOperator, where the lhs is nil and rhs an objC-method-call (or the other....

Uhm, is there something missing from my side that i have overseen to go forward with this patch?

Sep 24 2016, 4:24 AM

Aug 23 2016

parallaxe updated the diff for D23236: When ARC is enabled, no warning will be generated when a method 1. Returns 'nil' in a method that is attributed to return a 'nonnull' 2. The return-statement is a ConditionalOperator, where the lhs is nil and rhs an objC-method-call (or the other....

Moved the tests into nullability_nullonly.mm according to @dcoughlin.
Applied the changes according to @aaron.ballman.

Aug 23 2016, 8:35 AM
parallaxe added a comment to D23236: When ARC is enabled, no warning will be generated when a method 1. Returns 'nil' in a method that is attributed to return a 'nonnull' 2. The return-statement is a ConditionalOperator, where the lhs is nil and rhs an objC-method-call (or the other....

+cfe-commits

If this patch is applied, does clang issue a warning if a method marked "nonnull" returns a null value? I see a warning is issued for conditional expressions in the test case you've added, but I don't see a test case for a function returning just nil or 0.

I was wondering whether the change made in this patch contradicts what's stated in r240146's commit log:

"Note that we don't warn about nil returns from Objective-C methods,

because it's common for Objective-C methods to mimic the nil-swallowing
behavior of the receiver by checking ostensibly non-null parameters
and returning nil from otherwise non-null methods in that
case."
Aug 23 2016, 8:11 AM

Aug 6 2016

parallaxe added reviewers for D23236: When ARC is enabled, no warning will be generated when a method 1. Returns 'nil' in a method that is attributed to return a 'nonnull' 2. The return-statement is a ConditionalOperator, where the lhs is nil and rhs an objC-method-call (or the other...: aaron.ballman, krememek.
Aug 6 2016, 2:12 AM
parallaxe retitled D23236: When ARC is enabled, no warning will be generated when a method 1. Returns 'nil' in a method that is attributed to return a 'nonnull' 2. The return-statement is a ConditionalOperator, where the lhs is nil and rhs an objC-method-call (or the other... from to When ARC is enabled, no warning will be generated when a method 1. Returns 'nil' in a method that is attributed to return a 'nonnull' 2. The return-statement is a ConditionalOperator, where the lhs is nil and rhs an objC-method-call (or the other....
Aug 6 2016, 2:08 AM

Nov 25 2014

parallaxe added a comment to D4283: Adding 4 ASTMatchers: typedefDecl, isInMainFile, isInSystemFile, isInFileMatchingName.

Oh, great! Thanks for adapting it that it will (hopefully) don't cause any more problems on windows!

Nov 25 2014, 11:50 AM · deleted

Nov 22 2014

parallaxe added a comment to D4283: Adding 4 ASTMatchers: typedefDecl, isInMainFile, isInSystemFile, isInFileMatchingName.

So, what's the status? Anything wrong with it / anything I can do?

Nov 22 2014, 12:36 PM · deleted

Nov 4 2014

parallaxe added a comment to D4283: Adding 4 ASTMatchers: typedefDecl, isInMainFile, isInSystemFile, isInFileMatchingName.

Glad that my patch got accepted, thanks!
Well, as I have no commit access I thought that someone might apply that patch by himself - or that there is a magic mechanism that would have allowed me to commit the patch by using arcanist. Apparently, both assumptions seem to be wrong - may somebody apply that patch to the tree?

Nov 4 2014, 3:14 PM · deleted

Oct 31 2014

parallaxe updated the diff for D4283: Adding 4 ASTMatchers: typedefDecl, isInMainFile, isInSystemFile, isInFileMatchingName.
  • Reverted the changes in PreprocessorOptions.h
  • Updated the descriptions of the added matchers (added the dots and updated the matcher-names in their descriptions, which i forgot before)
  • Renamed isInSystemHeader to isExpansionInSystemHeader
Oct 31 2014, 3:02 PM · deleted

Oct 26 2014

parallaxe updated the diff for D4283: Adding 4 ASTMatchers: typedefDecl, isInMainFile, isInSystemFile, isInFileMatchingName.

isExpansionInMainFile and isExpansionInFileMatching - sounds reasonable :)
I'm going with FileContentMappings for now, but feel free to change anytime - I'm not enthusiastic about names.
I included Tooling.h in PreprocessorOptions.h to have access to the new typedef FileContentMappings. It feels kinda heavy to add that include for one typedef only. Furthermore, I'm unsure how to update the description of the RemappedFiles, as it directly relates to the std::pair that is now no longer visible in the typename. In my eyes, the usage of std::pair now becomes to an implementation-detail that should not be exposed in the comment, so i removed these mentions.

Oct 26 2014, 10:36 AM · deleted

Oct 18 2014

parallaxe updated the diff for D4283: Adding 4 ASTMatchers: typedefDecl, isInMainFile, isInSystemFile, isInFileMatchingName.

I updated it accordingly to the comments (thanks!).
One of the changes is the replacement of an initializer list through an explicit call to the default-constructor of std::vector<std::pair<std::string, std::string>>, to initialize the new introduced VirtualMappedFiles-parameter of the functions runToolOnCodeWithArgs and matchesConditionally. Due to the repeated usage of the type-declaration, it would be nice to declare a typedef for std::vector<std::pair<std::string, std::string>>, that could also be used for the RemappedFiles-instance-variable of the class PreprocessorOptions.
If it is appropriate to add the typedef, where should I add it and how should I name it?

Oct 18 2014, 2:06 PM · deleted

Oct 15 2014

parallaxe added a comment to D4283: Adding 4 ASTMatchers: typedefDecl, isInMainFile, isInSystemFile, isInFileMatchingName.
In D4283#15, @klimek wrote:

Is this still ongoing?

Oct 15 2014, 3:11 PM · deleted

Jun 25 2014

parallaxe changed the edit policy for D4283: Adding 4 ASTMatchers: typedefDecl, isInMainFile, isInSystemFile, isInFileMatchingName.
Jun 25 2014, 3:40 PM · deleted

Jun 24 2014

parallaxe added a reviewer for D4283: Adding 4 ASTMatchers: typedefDecl, isInMainFile, isInSystemFile, isInFileMatchingName: deleted.
Jun 24 2014, 4:12 PM · deleted
parallaxe retitled D4283: Adding 4 ASTMatchers: typedefDecl, isInMainFile, isInSystemFile, isInFileMatchingName from to Adding 4 ASTMatchers: typedefDecl, isInMainFile, isInSystemFile, isInFileMatchingName.
Jun 24 2014, 4:08 PM · deleted