[PATCH] D34185: [Parser][ObjC] Avoid crashing when skipping to EOF while parsing an ObjC interface/implementation

arphaman updated this revision to Diff 103013. arphaman marked an inline comment as done. arphaman added a comment. Remove the old checks and consume EOF unconditionally. Repository: rL LLVM https://reviews.llvm.org/D34185 Files: lib/Parse/ParseObjc.cpp test/Parser/objc-at-implementation

[PATCH] D34329: [GSoC] Clang AST diffing

arphaman added a comment. Congratulations of the first GSoC patch! I have some below comments: - Patches should be submitted using the full context (`git diff -U`). This makes it easier for reviewers to understand the change. This patch mainly adds new code, so this won't make much differen

[PATCH] D34329: [GSoC] Clang AST diffing

arphaman added inline comments. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:277 +Mappings::~Mappings() { + delete[] SrcToDst; + delete[] DstToSrc; Please use `std::unique_ptr` for `SrcToDst` and `DstToSrc` as well and remove the destructor. https://reviews.l

[PATCH] D34329: [GSoC] Clang AST diffing

arphaman added a comment. Generally we shouldn't have untested code in trunk, so I think that we need to find a way to test this before committing. We can start off by testing the output of the diff tool. Since there will be a lot of changes in the future, you don't have to have everything cove

[PATCH] D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets

arphaman added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:7031 +Introduced) && +!S.Diags.isIgnored(diag::warn_unguarded_availability_new, Loc); +diag = NewWarning ? diag::warn_partial_availability_new erik.pilkington wrote: > S

[PATCH] D34185: [Parser][ObjC] Avoid crashing when skipping to EOF while parsing an ObjC interface/implementation

This revision was automatically updated to reflect the committed changes. arphaman marked an inline comment as done. Closed by commit rL305719: [Parser][ObjC] Use an artificial EOF token while parsing lexed ObjC methods (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D

[PATCH] D34268: [clang] Fix format specifiers fixits for nested macros

arphaman added inline comments. Comment at: include/clang/Edit/EditedSource.h:50 +// Location of argument use inside the top-level macro +SourceLocation Use; + I think a name like `UseLoc` is more appropriate. Comment at: lib/Edit/Edite

[PATCH] D34392: [index] Nested class declarations should be annotated with the "specializationOf" relation if they pseudo-override a type in the base template

arphaman created this revision. This fixes an issue where Xcode's renaming engine couldn't find the reference to the second occurrence of "InnerClass" in this example: template struct Ts { template struct InnerClass { }; }; template<> struct Ts { template struct InnerClass; // This occu

[PATCH] D34268: [clang] Fix format specifiers fixits for nested macros

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM. Repository: rL LLVM https://reviews.llvm.org/D34268 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D34329: [GSoC] Clang AST diffing

arphaman added a comment. Looking at the output of the tool, I have the following suggestion: - We should avoid implicit expressions (We don't need to see things like `Insert ImplicitCastExpr(21) into BinaryOperator: *(22) at 0`). This can be done in a follow-up patch though. ===

[PATCH] D34329: [GSoC] Clang AST diffing

arphaman added inline comments. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:474 +for (SNodeId D1 = LMD1 + 1; D1 <= Id1; ++D1) { + double DeletionCost = 1.0; + ForestDist[D1][LMD2] = ForestDist[D1 - 1][LMD2] + DeletionCost; johannes wrote: > arphaman

[PATCH] D34329: [GSoC] Clang AST diffing

arphaman added inline comments. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:303 +/// Identifies a node in this subtree by its postorder offset. +using SNodeId = int; + What's the difference between `SNodeId` and `NodeId`? https://reviews.llvm.org/D34329

[PATCH] D34329: [GSoC] Clang AST diffing

arphaman added inline comments. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:171 + +std::string TreeRoot::label(NodeId Id) const { + const Node &N = getNode(Id); I believe that this method that you call `label` actually represents the `value` attribute that's de

[PATCH] D34329: [GSoC] Clang AST diffing

arphaman added inline comments. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:303 +/// Identifies a node in this subtree by its postorder offset. +using SNodeId = int; + johannes wrote: > arphaman wrote: > > What's the difference between `SNodeId` and `NodeId`? > N

[PATCH] D34329: [GSoC] Clang AST diffing

arphaman added inline comments. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:156 +int TreeRoot::getSubtreePostorder(std::vector &Ids, NodeId Root) const { + int Leaves = 0; + std::function Traverse = [&](NodeId Id) { Please use a more descriptive name e.g. `Num

[PATCH] D34392: [index] Nested class declarations should be annotated with the "specializationOf" relation if they pseudo-override a type in the base template

This revision was automatically updated to reflect the committed changes. Closed by commit rL305911: [index] Nested class declarations should be annotated with the (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D34392?vs=103179&id=103369#toc Repository: rL LLVM ht

[PATCH] D32439: Fix for incorrect source position of dependent c'tor initializer (bug:26195)

arphaman added a comment. Thanks for the fix! The implementation looks correct. A couple of misc comments: - Please move the test to `test/Index` (Since it uses `c-index-test`). - In the future please upload patches with full context (`git diff -U`). Comment at: lib/Sema/S

[PATCH] D34462: [index] "SpecializationOf" relation should be added even to forward declarations of class template specializations

arphaman created this revision. This patch fixes an issue where a forward declaration of a class template specialization was not related to the base template. We need to relate even forward declarations because specializations don't have to be defined. Repository: rL LLVM https://reviews.ll

[PATCH] D34329: [GSoC] Clang AST diffing

arphaman added a comment. The API looks better IMHO. Some more comments: Comment at: include/clang/Tooling/ASTDiff/ASTDiff.h:57 +/// Within a tree, this identifies a node by its preorder offset. +using NodeId = int; + I think that it's better to make make `Node

[PATCH] D34299: [ubsan] Improve diagnostics for return value checks (clang)

arphaman added a comment. It looks like if we have a function without the `return` (like the sample below), we will pass in a `0` as the location pointer. This will prevent a report of a runtime error as your compiler-rt change ignores the location pointers that are `nil`. Is this a bug or is t

[PATCH] D34462: [index] "SpecializationOf" relation should be added even to forward declarations of class template specializations

This revision was automatically updated to reflect the committed changes. Closed by commit rL305996: [index] Add the "SpecializationOf" relation to the forward declarations (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D34462?vs=103411&id=103554#toc Repository: rL

[PATCH] D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets

arphaman updated this revision to Diff 103577. arphaman added a comment. Update the logic for `-Wunguarded-availability -Wno-unguarded-availability-new` so that it only warns for the old APIs. Repository: rL LLVM https://reviews.llvm.org/D34264 Files: include/clang/Basic/DiagnosticGroups.

[PATCH] D34264: Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets

This revision was automatically updated to reflect the committed changes. Closed by commit rL306033: [Sema] Add -Wunguarded-availability-new (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D34264?vs=103577&id=103598#toc Repository: rL LLVM https://reviews.llvm.org/

[PATCH] D34299: [ubsan] Improve diagnostics for return value checks (clang)

arphaman added a comment. In https://reviews.llvm.org/D34299#788152, @vsk wrote: > In https://reviews.llvm.org/D34299#787795, @arphaman wrote: > > > It looks like if we have a function without the `return` (like the sample > > below), we will pass in a `0` as the location pointer. This will prev

[PATCH] D34299: [ubsan] Improve diagnostics for return value checks (clang)

arphaman added a comment. Ok, so now the null check `return.sloc.load` won't call the checker in compiler-rt and so the program won't `abort` and won't hit the `unreachable`. I have one question tough: This patch changes the behavior of this sanitizer for the example that I gave above. Previou

[PATCH] D32439: Fix for incorrect source position of dependent c'tor initializer (bug:26195)

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. This LGTM. I will commit it for you. https://reviews.llvm.org/D32439 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D32439: Fix for incorrect source position of dependent c'tor initializer (bug:26195)

This revision was automatically updated to reflect the committed changes. Closed by commit rL306103: PR26195: Set correct NestedNameSpecifierLoc for the dependent initializer (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D32439?vs=103703&id=103727#toc Repository:

[PATCH] D32439: Fix for incorrect source position of dependent c'tor initializer (bug:26195)

arphaman added a comment. Serge, the test seems to be failing on linux (http://bb.pgr.jp/builders/test-clang-msc-x64-on-i686-linux-RA). Can you take a look? I will have to revert it if we can't fix it soon. Repository: rL LLVM https://reviews.llvm.org/D32439 _

[PATCH] D32439: Fix for incorrect source position of dependent c'tor initializer (bug:26195)

arphaman added a comment. Link to failure: http://bb.pgr.jp/builders/test-clang-msc-x64-on-i686-linux-RA/builds/3777/steps/test_clang/logs/Clang%20%3A%3A%20Index__ctor-init-source-loc.cpp Repository: rL LLVM https://reviews.llvm.org/D32439 ___ c

[PATCH] D34329: [GSoC] Clang AST diffing

arphaman added inline comments. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:730 + +Mapping TreeComparator::matchTopDown() const { + PriorityList L1(T1); Johannes, it seems to me that your implementation of the top-down portion of the GumTree algorithm doesn't u

[PATCH] D32439: Fix for incorrect source position of dependent c'tor initializer (bug:26195)

arphaman added a comment. I reverted it in r306111 for now. Repository: rL LLVM https://reviews.llvm.org/D32439 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34299: [ubsan] Improve diagnostics for return value checks (clang)

arphaman added a comment. Fair enough. LGTM from my side. https://reviews.llvm.org/D34299 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33816: [Sema][ObjC] Don't allow -Wunguarded-availability to be silenced with redeclarations

arphaman added a comment. I don't think the new warning is ideal, e.g.: #import __attribute__((visibility("default"))) __attribute__((availability(macosx,introduced=10_10))) @interface Test : NSObject @end Test *testVar; E.g.: test.m:13:1: warning: 'Test' is partial: intr

[PATCH] D33816: [Sema][ObjC] Don't allow -Wunguarded-availability to be silenced with redeclarations

arphaman added inline comments. Comment at: test/SemaObjC/unguarded-availability.m:243 + + struct { new_int mem; } anon; // expected-warning{{'new_int' is partial}} expected-note{{annotate '' with an availability attribute}} +}; We should never have something l

[PATCH] D41087: [Preprocessor] Implement __is_target_{arch|vendor|os|environment} function-like builtin macros

arphaman updated this revision to Diff 126856. arphaman marked an inline comment as done. arphaman added a comment. - Use separate functions for checks. - "ARM" should match "thumb" arch too. https://reviews.llvm.org/D41087 Files: include/clang/Lex/Preprocessor.h lib/Lex/PPMacroExpansion.cp

[PATCH] D41087: [Preprocessor] Implement __is_target_{arch|vendor|os|environment} function-like builtin macros

This revision was automatically updated to reflect the committed changes. Closed by commit rC320734: [Preprocessor] Implement __is_target_{arch|vendor|os|environment} function-like (authored by arphaman, committed by ). Changed prior to commit: https://reviews.llvm.org/D41087?vs=126856&id=1270

[PATCH] D36952: [libclang] Add support for checking abstractness of records

arphaman added a comment. Sure, I'll do it today. https://reviews.llvm.org/D36952 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36952: [libclang] Add support for checking abstractness of records

This revision was automatically updated to reflect the committed changes. Closed by commit rL320748: [libclang] Add support for checking abstractness of records (authored by arphaman, committed by ). Changed prior to commit: https://reviews.llvm.org/D36952?vs=126298&id=127021#toc Repository:

[PATCH] D36790: [ObjC] Messages to 'self' in class methods that return 'instancetype' should use the pointer to the class as the result type of the message

arphaman added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D36790 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36952: [libclang] Add support for checking abstractness of records

arphaman added a comment. In https://reviews.llvm.org/D36952#955937, @jklaehn wrote: > Thanks! However, compared to diff 126298 > (https://reviews.llvm.org/differential/diff/126298/), there seems to be > duplication in the committed change (maybe some artifacts due to > rebasing/merging?). Sho

[PATCH] D36952: [libclang] Add support for checking abstractness of records

arphaman added a comment. Fixed in r320766. Sorry about that! Repository: rL LLVM https://reviews.llvm.org/D36952 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41087: [Preprocessor] Implement __is_target_{arch|vendor|os|environment} function-like builtin macros

arphaman added a comment. In https://reviews.llvm.org/D41087#956379, @bob.wilson wrote: > I'm concerned here about the check for the names as written vs. the canonical > names. @compnerd pointed out one specific case with armv7, and I see that > you've got special handling for "darwin", but I t

[PATCH] D36790: [ObjC] Messages to 'self' in class methods that return 'instancetype' should use the pointer to the class as the result type of the message

arphaman updated this revision to Diff 127192. arphaman marked an inline comment as done. arphaman added a comment. - Remove redundant checks. - Remove the ARC-specific check. https://reviews.llvm.org/D36790 Files: include/clang/Sema/Sema.h lib/Sema/SemaExprObjC.cpp test/SemaObjC/multiple

[PATCH] D36790: [ObjC] Messages to 'self' in class methods that return 'instancetype' should use the pointer to the class as the result type of the message

arphaman added inline comments. Comment at: lib/Sema/SemaExprObjC.cpp:1357 +(ReceiverType->isObjCClassOrClassKindOfType() || + ReceiverType->isObjCQualifiedClassType()) && +Receiver->isObjCSelfExpr() && getLangOpts().ObjCAutoRefCount) { rj

[PATCH] D41405: Fix an assertion failure regression in isDesignatorAtObjectEnd for __builtin_object_size with incomplete array type in struct

arphaman created this revision. arphaman added reviewers: vsapsai, rsmith, george.burgess.iv. The commit r316245 introduced a regression that causes an assertion failure when Clang tries to cast an `IncompleteArrayType` to a `PointerType` when evaluating __builtin_object_size in this sample:

[PATCH] D41405: Fix an assertion failure regression in isDesignatorAtObjectEnd for __builtin_object_size with incomplete array type in struct

arphaman added a comment. Note that even though there is a discrepancy between GCC and Clang, this patch does not change Clang's behavior in this instance as it emitted -1 previously as well Repository: rC Clang https://reviews.llvm.org/D41405

[PATCH] D40998: [driver][darwin] Take the OS version specified in "-target" as the target OS instead of inferring it from SDK / environment

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rC321099: [driver][darwin] Take the OS version specified in "-target" as the target (authored by arphaman, committed by ).

[PATCH] D41076: [driver][darwin] Set the 'simulator' environment when it's specified in '-target'

This revision was automatically updated to reflect the committed changes. Closed by commit rC321102: [driver][darwin] Set the 'simulator' environment when it's specified (authored by arphaman, committed by ). Repository: rC Clang https://reviews.llvm.org/D41076 Files: lib/Driver/ToolChains/

[PATCH] D41424: [darwin][driver] Warn about mismatching --version-min rather than superfl

arphaman created this revision. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D41424 Files: lib/Driver/ToolChains/Darwin.cpp test/Driver/darwin-version.c Index: test/Driver/darwin-version.c ==

[PATCH] D41425: [darwin][driver] Warn about mismatching --version-min rather than superfluous --version-min compiler option

arphaman created this revision. arphaman added reviewers: bob.wilson, dexonsmith, steven_wu. The warning about the superfluous `--version-min` compiler option seems a little too strong right now. For now we should only warn about `--version-min` options that specify a different OS version to the

[PATCH] D41425: [darwin][driver] Warn about mismatching --version-min rather than superfluous --version-min compiler option

arphaman added inline comments. Comment at: lib/Driver/ToolChains/Darwin.cpp:1536 + Driver::GetReleaseVersion(OSVersionArgTarget->getOSVersion(), + ArgMajor, ArgMinor, ArgMicro, HadExtra) && + VersionTuple(TargetMajor, Targe

[PATCH] D41425: [darwin][driver] Warn about mismatching --version-min rather than superfluous --version-min compiler option

arphaman added inline comments. Comment at: lib/Driver/ToolChains/Darwin.cpp:1536 + Driver::GetReleaseVersion(OSVersionArgTarget->getOSVersion(), + ArgMajor, ArgMinor, ArgMicro, HadExtra) && + VersionTuple(TargetMajor, Targe

[PATCH] D41425: [darwin][driver] Warn about mismatching --version-min rather than superfluous --version-min compiler option

This revision was automatically updated to reflect the committed changes. Closed by commit rL321145: [darwin][driver] Warn about mismatching --version-min rather than (authored by arphaman, committed by ). Changed prior to commit: https://reviews.llvm.org/D41425?vs=127637&id=127648#toc Rep

[PATCH] D41405: Fix an assertion failure regression in isDesignatorAtObjectEnd for __builtin_object_size with incomplete array type in struct

arphaman added a comment. In https://reviews.llvm.org/D41405#960008, @george.burgess.iv wrote: > LGTM assuming my nit gets addressed. > > Thank you! > > > Maybe someone who's more familiar with this builtin could point to the > > cause of this discrepancy > > Yeah, the documentation for this bui

[PATCH] D41405: Fix an assertion failure regression in isDesignatorAtObjectEnd for __builtin_object_size with incomplete array type in struct

This revision was automatically updated to reflect the committed changes. arphaman marked 2 inline comments as done. Closed by commit rC321222: Fix an assertion failure regression in isDesignatorAtObjectEnd for (authored by arphaman, committed by ). Changed prior to commit: https://reviews.llvm

[PATCH] D40983: Generate Libclang invocation reproducers using a new -cc1gen-reproducer option

arphaman marked 2 inline comments as done. arphaman added a comment. In https://reviews.llvm.org/D40983#958809, @bruno wrote: > Hi Alex, > > Thanks for improving this. > > - Instead of adding `-cc1gen-reproducer`, why can't you run that through > `-cc1` and have a flag similar to `-###`, which j

[PATCH] D40983: Generate Libclang invocation reproducers using a new -cc1gen-reproducer option

arphaman updated this revision to Diff 127935. arphaman marked an inline comment as done. arphaman added a comment. Address review comments https://reviews.llvm.org/D40983 Files: include/clang/Driver/Driver.h lib/Driver/Driver.cpp test/Index/create-libclang-completion-reproducer.c test/

[PATCH] D41526: [libclang] Avoid builtin trap for __debug parser_crash pragma

arphaman created this revision. arphaman added a reviewer: benlangmuir. Herald added a subscriber: JDevlieghere. This patch allows libclang to report a parser crash that's caused by `#pragma clang __debug parser_crash` without using the `LLVM_BUILTIN_TRAP`. Instead a new flag in PPOpts is used t

[PATCH] D41528: [Sema] Don't emit the -Wstrict-prototypes warning for variadic functions.

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM, thank you https://reviews.llvm.org/D41528 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D41526: [libclang] Avoid builtin trap for __debug parser_crash pragma

arphaman abandoned this revision. arphaman added a comment. It turns out there's already something that works much better: `#pragma clang __debug handle_crash`. Sorry about the redundant patch! Repository: rC Clang https://reviews.llvm.org/D41526 __

[PATCH] D41750: Fix TLS support check for Darwin 32-bit simulator targets.

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM, thanks https://reviews.llvm.org/D41750 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D25820: [Sema][Objective-C] Formatting warnings should see through Objective-C message sends

arphaman added a comment. @benhamilton We're planning to use the new attribute in https://reviews.llvm.org/D27165 to address this issue. I'll get back to that patch in the near future. Repository: rL LLVM https://reviews.llvm.org/D25820 ___ cf

[PATCH] D48297: [Darwin] Add a warning for missing include path for libstdc++

arphaman created this revision. arphaman added reviewers: bruno, vsapsai. Xcode 10 removes support for libstdc++, but the users just get a confusing include not file warning when including an STL header (when building for iOS6 which uses libstdc++ by default for example). This patch adds a new w

[PATCH] D48297: [Darwin] Add a warning for missing include path for libstdc++

arphaman added inline comments. Comment at: lib/Frontend/InitHeaderSearch.cpp:491 } else { - AddDefaultCPlusPlusIncludePaths(triple, HSOpts); + AddDefaultCPlusPlusIncludePaths(Lang, triple, HSOpts); } bruno wrote: > If we happen to suggest lib

[PATCH] D48297: [Darwin] Add a warning for missing include path for libstdc++

arphaman marked 3 inline comments as done. arphaman added inline comments. Comment at: lib/Frontend/InitHeaderSearch.cpp:374-377 + IsBaseFound = AddGnuCPlusPlusIncludePaths("/usr/include/c++/4.2.1", +"powerpc-apple-darwin10", "

[PATCH] D48297: [Darwin] Add a warning for missing include path for libstdc++

arphaman updated this revision to Diff 151944. arphaman marked 3 inline comments as done. arphaman added a comment. Addressed review comments https://reviews.llvm.org/D48297 Files: include/clang/Basic/DiagnosticFrontendKinds.td include/clang/Lex/HeaderSearch.h lib/Frontend/InitHeaderSearc

[PATCH] D48297: [Darwin] Add a warning for missing include path for libstdc++

This revision was automatically updated to reflect the committed changes. Closed by commit rL335063: [Darwin] Add a warning for missing include path for libstdc++ (authored by arphaman, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D4

[PATCH] D47290: [Sema] -Wformat-pedantic only for NSInteger/NSUInteger %zu/%zi on Darwin

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM. Thanks for working on this! Repository: rC Clang https://reviews.llvm.org/D47290 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype

arphaman added inline comments. Comment at: lib/Lex/FilterToIncludes.cpp:628 + First = Id.Last; + auto Kind = llvm::StringSwitch(Id.Name) + .Case("include", pp_include) ddunbar wrote: > What is our feeling w.r.t. _Pragma, which can in theory in

[PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype

arphaman added inline comments. Comment at: lib/Lex/FilterToIncludes.cpp:628 + First = Id.Last; + auto Kind = llvm::StringSwitch(Id.Name) + .Case("include", pp_include) arphaman wrote: > ddunbar wrote: > > What is our feeling w.r.t. _Pragma, wh

[PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype

arphaman added inline comments. Comment at: lib/Lex/FilterToIncludes.cpp:628 + First = Id.Last; + auto Kind = llvm::StringSwitch(Id.Name) + .Case("include", pp_include) arphaman wrote: > arphaman wrote: > > ddunbar wrote: > > > What is our feel

[PATCH] D53354: [WIP][NOT FOR COMMIT][PROTOTYPE] clang-scan-deps: dependency scanning tool rough prototype

arphaman added a comment. In https://reviews.llvm.org/D53354#1267376, @whisperity wrote: > With regards to https://reviews.llvm.org/D53352: you can change the diff > related to a patch whilst keeping discuccion and metadata of the diff. Good point, thanks! > Please add a generic description t

[PATCH] D53621: Support for groups of attributes in #pragma clang attribute

arphaman added a comment. Thanks! It looks pretty good, just minor nit. could you also update the language extension doc and release notes? Comment at: clang/lib/Sema/SemaAttr.cpp:645 + + for (const PragmaAttributeEntry &Entry : PragmaAttributeStack.back().Entries) +if (!

[PATCH] D53621: Support for groups of attributes in #pragma clang attribute

arphaman accepted this revision. arphaman added a comment. LGTM too when Aaron's comments are addressed https://reviews.llvm.org/D53621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

arphaman added inline comments. Comment at: include/clang/Driver/Driver.h:508 + /// GetTemporaryPath - Return the pathname of a temporary directory to use + /// as part of compilation; the directory will have the given prefix. Old function name in the comment

[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56608/new/ https://reviews.llvm.org/D56608 ___ cfe-commit

[PATCH] D56523: Improve a -Wunguarded-availability note

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56523/new/ https://reviews.llvm.org/D56523 ___ cfe-commits mailing list cfe-commi

[PATCH] D54428: [clangd] XPC transport layer, framework, test-client

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54428/new/ https://reviews.llvm.org/D54428 ___ cfe-commits mailing list cfe-commi

[PATCH] D56766: [Frontend] Make WrapperFrontendAction call WrappedAction.PrepareToExecuteAction.

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. That LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56766/new/ https://reviews.llvm.org/D56766 ___ cfe-commits mailing list cfe

[PATCH] D56816: [ObjC] Follow-up r350768 and allow the use of unavailable methods that are declared in a parent class from within the @implementation context

arphaman created this revision. arphaman added a reviewer: erik.pilkington. Herald added subscribers: cfe-commits, dexonsmith, jkorous. This patch extends r350768 and allows the use of methods marked as `unavailable` that are declared in a parent class/category from within the `@implementation`

[PATCH] D56816: [ObjC] Follow-up r350768 and allow the use of unavailable methods that are declared in a parent class from within the @implementation context

arphaman updated this revision to Diff 182191. arphaman added a comment. Follow Erik's suggestion and don't check for definition/declaration in parent class. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56816/new/ https://reviews.llvm.org/D56816 Files: lib/S

[PATCH] D56816: [ObjC] Follow-up r350768 and allow the use of unavailable methods that are declared in a parent class from within the @implementation context

This revision was automatically updated to reflect the committed changes. Closed by commit rC351459: [ObjC] Follow-up r350768 and allow the use of unavailable methods that are (authored by arphaman, committed by ). Changed prior to commit: https://reviews.llvm.org/D56816?vs=182191&id=182334#toc

[PATCH] D56892: Add a priority field to availability attributes to prioritize explicit attributes from declaration over attributes from '#pragma clang attribute'

arphaman created this revision. arphaman added reviewers: erik.pilkington, aaron.ballman. Herald added subscribers: dexonsmith, jkorous. We have an issue when using `#pragma clang attribute` with availability attributes: - The explicit attribute that's specified next to the declaration is not g

[PATCH] D56892: Add a priority field to availability attributes to prioritize explicit attributes from declaration over attributes from '#pragma clang attribute'

arphaman updated this revision to Diff 182432. arphaman added a comment. Reverse the priority numbering direction as suggested by @dexonsmith . Now the lower priority is the most important, and the less important priorities have a higher numerical value. Repository: rC Clang CHANGES SINCE L

[PATCH] D56892: Add a priority field to availability attributes to prioritize explicit attributes from declaration over attributes from '#pragma clang attribute'

arphaman updated this revision to Diff 182608. arphaman marked 3 inline comments as done. arphaman added a comment. - Add documentation and expand a comment as suggested by Aaron. - Address Erik's comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56892/new/

[PATCH] D56892: Add a priority field to availability attributes to prioritize explicit attributes from declaration over attributes from '#pragma clang attribute'

arphaman added a comment. In D56892#1363089 , @aaron.ballman wrote: > I think the documentation for the attribute should be updated to explain this > new behavior, otherwise this will be a very inexplicable feature to users. Done > One question I have

[PATCH] D56892: Add a priority field to availability attributes to prioritize explicit attributes from declaration over attributes from '#pragma clang attribute'

This revision was automatically updated to reflect the committed changes. Closed by commit rL352084: Add a priority field to availability attributes to prioritize explicit (authored by arphaman, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.ll

[PATCH] D56226: [clang-format] square parens that are followed by '=' are not Objective-C message sends

arphaman updated this revision to Diff 183372. arphaman marked 3 inline comments as done. arphaman added a comment. Update for Ben's comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56226/new/ https://reviews.llvm.org/D56226 Files: lib/Format/TokenAnnot

[PATCH] D56226: [clang-format] square parens that are followed by '=' are not Objective-C message sends

This revision was automatically updated to reflect the committed changes. Closed by commit rC352125: [clang-format] square parens with one token are not Objective-C message sends (authored by arphaman, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D5

[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

arphaman added a comment. In D55673#1377404 , @tra wrote: > Would that be OK to use target_sdk_version to pass *CUDA* SDK version to the > CC1 compilations? > I have upcoming changes that need to know the version to generate correct > glue IR for CUDA.

[PATCH] D52339: Support enums with a fixed underlying type in all language modes

arphaman added a comment. Herald added a subscriber: jkorous. @erik.pilkington the change to make `objc_fixed_enum` true in non-ObjC mode turned out to be problematic for our adoption. Could you please fix it before LLVM8 is wrapped up so that we don't ship this extension as on by default for a

[PATCH] D52339: Support enums with a fixed underlying type in all language modes

arphaman added a comment. We could introduce a new extension flag like `c_fixed_enum` that can be set for all language modes. `objc_fixed_enum` should apply to ObjC only. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52339/new/ https://reviews.llvm.org/D52339

[PATCH] D57345: Make clang/test/Index/pch-from-libclang.c pass in more places

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. Makes sense. The comments aren't really enforceable though, can we just factor this out into one function in lib/Basic that can be shared between all sites? It seems like there's nothing t

[PATCH] D57345: Make clang/test/Index/pch-from-libclang.c pass in more places

arphaman added a comment. Yes, that should work. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57345/new/ https://reviews.llvm.org/D57345 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D56924: Handle ObjCCategoryDecl class extensions for print

arphaman requested changes to this revision. arphaman added a comment. This revision now requires changes to proceed. Do you know if this have an effect on the output of completion results or other tooling-based output? A couple of requests: - This behavior should be controlled by a printing po

[PATCH] D54310: Make clang-based tools find libc++ on MacOS

arphaman added a comment. Apologies for not seeing this earlier. I agree with George, this behavior doesn't seem right to me. > The logic to do this was based on resource dir as an approximation of > where the compiler is installed. This broke the tools that read > 'compile_commands.json' and d

[PATCH] D54503: [HeaderSearch] loadSubdirectoryModuleMaps should respect -working-directory

arphaman created this revision. arphaman added a reviewer: bruno. Herald added a subscriber: dexonsmith. Include search paths can be relative paths. The `loadSubdirectoryModuleMaps` function should account for that and respect the `-working-directory` parameter given to Clang. Repository: rC

[PATCH] D54503: [HeaderSearch] loadSubdirectoryModuleMaps should respect -working-directory

This revision was automatically updated to reflect the committed changes. Closed by commit rL346822: [HeaderSearch] loadSubdirectoryModuleMaps should respect -working-directory (authored by arphaman, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://revie

[PATCH] D54529: [clangd] Add USR to textDocument/definition response

arphaman added a comment. I don't think we should be using `textDocument/definition` here, and I agree with Sam that a new method would be better. We don't actually need the semantic guarantees/constrains imposed by LSP's description of `textDocument/definition`, as we want to find any declara

[PATCH] D54547: PTH-- Remove feature entirely-

arphaman added a comment. Would it be better to deprecate the use of PTH right now for the current release, so that the users will be aware that we will remove PTH support in LLVM 9 once they upgrade to LLVM 8? Then we can remove it right after LLVM 8 branch is created. https://reviews.llvm.o

<    1   2   3   4   5   6   7   8   9   10   >