[PATCH] D34687: [Tooling] CompilationDatabase should be able to strip position arguments when `-fsyntax-only` is used

This revision was automatically updated to reflect the committed changes. Closed by commit rL306659: [Tooling] FixedCompilationDatabase should be able to strip positional (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D34687?vs=104161&id=104618#toc Repository: rL L

[PATCH] D34810: [Sema] -Wcomma should not warn for expression that return void

arphaman created this revision. Right now -Wcomma is too strict IMO, we shouldn't warn about expressions that return void. Repository: rL LLVM https://reviews.llvm.org/D34810 Files: lib/Sema/SemaExpr.cpp test/SemaCXX/warn-comma-operator.cpp Index: test/SemaCXX/warn-comma-operator.cpp

[PATCH] D34766: fix NSAPI constants to reflect the current state of NSStringMethodKind/NSDictionaryMethodKind enums

arphaman added a comment. LGTM, Thanks! Do you have commit access? https://reviews.llvm.org/D34766 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34766: fix NSAPI constants to reflect the current state of NSStringMethodKind/NSDictionaryMethodKind enums

This revision was automatically updated to reflect the committed changes. Closed by commit rL306680: Fix NSAPI constants to reflect the current state of (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D34766?vs=104455&id=104643#toc Repository: rL LLVM https://revie

[PATCH] D34329: [clang-diff] Initial implementation.

arphaman added inline comments. Comment at: include/clang/Tooling/ASTDiff/ASTDiffInternal.h:184 + + std::string getNodeValueI(NodeId Id) const; + std::string getNodeValueI(const DynTypedNode &DTN) const; `getNodeValueImpl`? Comment at: lib/To

[PATCH] D34810: [Sema] -Wcomma should not warn for expressions that return void

arphaman added a comment. Ah, I see, so this is more of a stylistic warning rather than "suspicious" use one. In https://reviews.llvm.org/D34810#795926, @rtrieu wrote: > What is the reason to exclude void expressions now? For the function case, > it is more consistent to warn on all functio

[PATCH] D34696: [refactor] Move the core of clang-rename to lib/Tooling/Refactoring

arphaman added a comment. I think I'd rather move `clang-rename` first. Comment at: tools/extra/clang-rename/tool/ClangRename.cpp:167 FindingAction.getUSRList(); const std::vector &PrevNames = FindingAction.getUSRSpellings(); if (PrintName) { alexs

[PATCH] D34329: [clang-diff] Initial implementation.

arphaman added a comment. I think that it's pretty much ready. I think that the test should be expanded though. At the very least it should check that all of the node types that are supported by `SyntaxTreeImpl::getNodeValueImpl` get matched. https://reviews.llvm.org/D34329

[PATCH] D34886: Add a fixit for -Wobjc-protocol-property-synthesis

arphaman created this revision. The fixit inserts `@synthesize` directives into the implementation. Repository: rL LLVM https://reviews.llvm.org/D34886 Files: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Parse/ParseObjc.cpp lib/Sema/SemaObjCProperty.cpp

[PATCH] D34696: [refactor] Move clang-rename to Clang

arphaman added a comment. I didn't notice that ClassReplacements.cpp had a dependency on clang-apply-replacements. I moved it back to clang-tools-extra to the clang-apply-replacements tests. Repository: rL LLVM https://reviews.llvm.org/D34696 _

[PATCH] D34810: [Sema] -Wcomma should not warn for expressions that return void

arphaman abandoned this revision. arphaman added a comment. Abandoning. The current behaviour makes sense. Thanks for the responses! Repository: rL LLVM https://reviews.llvm.org/D34810 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:

[PATCH] D34886: Add a fixit for -Wobjc-protocol-property-synthesis

arphaman marked an inline comment as done. arphaman added inline comments. Comment at: include/clang/Sema/Sema.h:3351 + ObjCInterfaceDecl *IDecl, SourceRange AtEnd); + void DefaultSynthesizeProperties(Scope *S, Decl *D, SourceRange AtEnd); --

[PATCH] D34886: Add a fixit for -Wobjc-protocol-property-synthesis

This revision was automatically updated to reflect the committed changes. arphaman marked an inline comment as done. Closed by commit rL307014: Add a fixit for -Wobjc-protocol-property-synthesis (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D34886?vs=104859&id=105069

[PATCH] D34949: [refactor][rename] Use a single base class for class that finds a declaration at location and for class that searches for all occurrences of a specific declaration

arphaman created this revision. - Use single `RecursiveSymbolVisitor` class for both `USRLocFindingASTVisitor` and `NamedDeclOccurrenceFindingVisitor` to avoid duplicate visiting code. - Traverse nested name specifier locs in the new class and remove the separate matching step. - New class `Name

[PATCH] D34949: [refactor][rename] Use a single base class for class that finds a declaration at location and for class that searches for all occurrences of a specific declaration

arphaman added a comment. This is meant to be NFC btw. Repository: rL LLVM https://reviews.llvm.org/D34949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34949: [refactor][rename] Use a single base class for class that finds a declaration at location and for class that searches for all occurrences of a specific declaration

arphaman updated this revision to Diff 105084. arphaman added a comment. Small fixup. Repository: rL LLVM https://reviews.llvm.org/D34949 Files: include/clang/Tooling/Refactoring/RecursiveSymbolVisitor.h include/clang/Tooling/Refactoring/Rename/USRFinder.h include/clang/module.modulema

[PATCH] D34329: [clang-diff] Initial implementation.

arphaman added a comment. @johannes Are you planning to work on integration with the `StmtDataCollector` in this patch or would you prefer to follow-up with additional patches? https://reviews.llvm.org/D34329 ___ cfe-commits mailing list cfe-commi

[PATCH] D34329: [clang-diff] Initial implementation.

arphaman added inline comments. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:385 + } + int getSizeS() const { return RootIds.size(); } + NodeId getIdInRoot(SNodeId Id) const { What's the purpose of the `S` prefix in the name of this method and a couple of othe

[PATCH] D34329: [clang-diff] Initial implementation.

arphaman added inline comments. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:359 + + explicit SNodeId(int Id) : Id(Id){}; + explicit SNodeId() = default; NIT: This ';' is redundant. Comment at: lib/Tooling/ASTDiff/ASTDiff.cpp:363 + operator i

[PATCH] D34329: [clang-diff] Initial implementation.

arphaman added inline comments. Comment at: include/clang/Tooling/ASTDiff/ASTDiffInternal.h:38 + operator int() const { return Id; } + NodeId &operator++() { return ++this->Id, *this; } + NodeId &operator--() { return --this->Id, *this; } NIT: You don't need `

[PATCH] D34329: [clang-diff] Initial implementation.

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM. You can request commit access at http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access. https://reviews.llvm.org/D34329 __

[PATCH] D34981: RecursiveASTVisitor should visit the nested name qualifiers in a template specialisation

arphaman created this revision. Right now because of this issue rename cannot be initiated at name qualifiers in a template specialisation, e.g.: struct Outer { template struct Nested { }; }; template<> struct Outer::Nested; // Rename cannot be initiated at 'Outer' because

[PATCH] D35012: [refactor] Add the AST source selection component

arphaman created this revision. Herald added a subscriber: mgorny. This patch adds the base AST source selection component to the refactoring library. AST selection is represented using a tree of `SelectedASTNode` values. Each selected node gets its own selection kind, which can actually be `Non

[PATCH] D35012: [refactor] Add the AST source selection component

arphaman updated this revision to Diff 105269. arphaman added a comment. A a test-case for implicit declarations. Repository: rL LLVM https://reviews.llvm.org/D35012 Files: include/clang/Tooling/Refactoring/ASTSelection.h lib/Tooling/Refactoring/ASTSelection.cpp lib/Tooling/Refactoring

[PATCH] D34992: Emit static constexpr member as available_externally definition

arphaman added a comment. Does this apply to all constexpr global variables? It could potentially fix https://bugs.llvm.org/show_bug.cgi?id=31860 . https://reviews.llvm.org/D34992 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists

[PATCH] D34512: Add preliminary Cross Translation Unit support library

arphaman added inline comments. Comment at: unittests/CrossTU/CMakeLists.txt:10 + +target_link_libraries(ToolingTests + clangAST `CrossTUTests`? https://reviews.llvm.org/D34512 ___ cfe-commits mailing list cfe-com

[PATCH] D35061: [ObjC] Avoid the -Wunguarded-availability warnings for protocol requirements in protocol/class/category declarations

arphaman created this revision. The unguarded availability warnings in the protocol requirements of a protocol/class/category declaration can be avoided. This matches the behaviour of Swift's diagnostics. Repository: rL LLVM https://reviews.llvm.org/D35061 Files: include/clang/Sema/Sema.

[PATCH] D35061: [ObjC] Avoid the -Wunguarded-availability warnings for protocol requirements in protocol/class/category declarations

arphaman added inline comments. Comment at: lib/Sema/SemaExpr.cpp:142 if (Result == AR_NotYetIntroduced) { +if (AvoidAvailabilityChecks) + return; erik.pilkington wrote: > Why are we doing this just for partials? Doesn't this also apply to > unavaila

[PATCH] D35069: [Frontend] Verify that the bitstream is not empty before reading the serialised diagnostics

arphaman created this revision. Clang should avoid calling `report_fatal_error` when the file with the serialised diagnostics is empty. This patch changes Clang's serialised diagnostic reader, now it reports an appropriate error instead of crashing. Repository: rL LLVM https://reviews.llvm.

[PATCH] D35061: [ObjC] Avoid the -Wunguarded-availability warnings for protocol requirements in protocol/class/category declarations

arphaman added inline comments. Comment at: lib/Sema/SemaExpr.cpp:142 if (Result == AR_NotYetIntroduced) { +if (AvoidAvailabilityChecks) + return; erik.pilkington wrote: > arphaman wrote: > > erik.pilkington wrote: > > > Why are we doing this just for

[PATCH] D35061: [ObjC] Avoid the -Wunguarded-availability warnings for protocol requirements in protocol/class/category declarations

This revision was automatically updated to reflect the committed changes. arphaman marked an inline comment as done. Closed by commit rL307368: [ObjC] Avoid the -Wunguarded-availability warnings for protocol (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D35061?vs=105

[PATCH] D35069: [Frontend] Verify that the bitstream is not empty before reading the serialised diagnostics

This revision was automatically updated to reflect the committed changes. Closed by commit rL307384: [Frontend] Verify that the bitstream is not empty before reading (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D35069?vs=105462&id=105612#toc Repository: rL LLVM

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

arphaman added a comment. Thanks, that's pretty cool! How bigger did the clang binary get after you've added all these strings? I feel like this is more of a CC1 option as well. I have some feedback for your additional ideas: > add another option to pass in the index (or enum) to force an asser

[PATCH] D35187: [libclang] Support for querying whether an enum is scoped

arphaman added a comment. Thanks for the patch. I have just one minor comment: Comment at: tools/libclang/CIndex.cpp:7815 + const Decl *D = cxcursor::getCursorDecl(C); + const EnumDecl *Enum = D ? dyn_cast_or_null(D) : nullptr; + return (Enum && Enum->isScoped()) ? 1 : 0; --

[PATCH] D34981: RecursiveASTVisitor should visit the nested name qualifiers in a template specialisation

This revision was automatically updated to reflect the committed changes. arphaman marked an inline comment as done. Closed by commit rL307638: RecursiveASTVisitor should visit the nested name qualifiers in (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D34981?vs=1051

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

arphaman added a comment. > Currently looks like around 200k (4534 @ 33 byte avg length + ptr). If this > is too much, we could make it conditional based on NDEBUG or some other macro > at compile time. I think this is mostly useful during development, so some conditional mechanism would make

[PATCH] D35268: [ObjC] Ambiguous property synthesis should pick the 'readwrite' property and check for incompatible attributes

arphaman created this revision. This patch changes the way ambiguous property synthesis (i.e. when synthesizing a property that's declared in multiple protocols) is performed. Previously, we synthesized the first property that was found. This lead to problems when the property was synthesized i

[PATCH] D35187: [libclang] Support for querying whether an enum is scoped

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM. Do you have commit access or would you like me to commit it on your behalf? https://reviews.llvm.org/D35187 ___ cfe-commits mailing li

[PATCH] D35187: [libclang] Support for querying whether an enum is scoped

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

[PATCH] D35187: [libclang] Support for querying whether an enum is scoped

This revision was automatically updated to reflect the committed changes. Closed by commit rL307769: [libclang] Support for querying whether an enum is scoped (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D35187?vs=106066&id=106173#toc Repository: rL LLVM https:/

[PATCH] D35187: [libclang] Support for querying whether an enum is scoped

arphaman added a comment. Committed r307771 with correct attribution. Repository: rL LLVM https://reviews.llvm.org/D35187 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

arphaman added a comment. My script relies on a hack to map the name of the diagnostic to the enum value. We need to come up with a better to map the diagnostic name to the enum value. I propose a new utility tool that would take the name of the diagnostic and map it back to the enum value.

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

arphaman added a comment. Right. I was aware of the `diagtool` before, but didn't really look into what it did. TIL! It would make sense to add this kind of mapping functionality to that tool then. https://reviews.llvm.org/D35175 ___ cfe-commits m

[PATCH] D34949: [refactor][rename] Use a single base class for class that finds a declaration at location and for class that searches for all occurrences of a specific declaration

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

[PATCH] D35175: New option that adds the DiagID enum name and index to Diagnostic output.

arphaman added a comment. I was impatient, so I already started on a patch for diagtool. I'll post it soon. https://reviews.llvm.org/D35175 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D35306: [diagtool] Add the 'find-diagnostic-id' subcommand that converts a name of the diagnostic to its enum value

arphaman created this revision. Herald added a subscriber: mgorny. The new subcommand prints out the enum value of the diagnostic. This will be used in a utility script that invokes clang and forces it to stop when some specific diagnostic is emitted (see https://reviews.llvm.org/D35175 for dis

[PATCH] D35306: [diagtool] Add the 'find-diagnostic-id' subcommand that converts a name of the diagnostic to its enum value

This revision was automatically updated to reflect the committed changes. Closed by commit rL307813: [diagtool] Add a 'find-diagnostic-id' subcommand that converts a name of (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D35306?vs=106211&id=106245#toc Repository: r

[PATCH] D34949: [refactor][rename] Use a single base class for class that finds a declaration at location and for class that searches for all occurrences of a specific declaration

This revision was automatically updated to reflect the committed changes. Closed by commit rL307898: [refactor][rename] Use a single base class for class that finds (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D34949?vs=105084&id=106388#toc Repository: rL LLVM h

[PATCH] D35268: [ObjC] Ambiguous property synthesis should pick the 'readwrite' property and check for incompatible attributes

This revision was automatically updated to reflect the committed changes. Closed by commit rL307903: [ObjC] Pick a 'readwrite' property when synthesizing ambiguous (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D35268?vs=106060&id=106394#toc Repository: rL LLVM ht

[PATCH] D35379: Add documentation for @available

arphaman added a comment. Thanks for doing this! I have some comments: Comment at: docs/LanguageExtensions.rst:1277 +It is possible use the newest SDK but still build a program that can run on +older macOS and iOS versions, by passing ``-mmacosx-version-info=`` /

[PATCH] D35405: [index] Added a method indexTopLevelDecls to run indexing on a list of Decls.

arphaman requested changes to this revision. arphaman added a comment. This revision now requires changes to proceed. Nice, I will need something like this for the refactoring stress test tool in the future that verifies that the indexer and renaming engine have a similar view of the code. ==

[PATCH] D35405: [index] Added a method indexTopLevelDecls to run indexing on a list of Decls.

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

[PATCH] D35379: Add documentation for @available

arphaman added inline comments. Comment at: docs/LanguageExtensions.rst:1347 + +In rare cases, the availability annotation on an API might be overly +conservative. For example, ``[NSProcessInfo processInfo]`` secretly responds to thakis wrote: > arphaman wrote:

[PATCH] D35012: [refactor] Add the AST source selection component

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

[PATCH] D35484: Add a warning for missing '#pragma pack (pop)' and suspicious '#pragma pack' uses when including files

arphaman created this revision. This patch adds a new `-Wpragma-pack` warning. It warns in the following cases: - When a translation unit is missing terminating `#pragma pack (pop)` directives. - When entering an included file if the current alignment value as determined by '#pragma pack' direc

[PATCH] D35012: [refactor] Add the AST source selection component

arphaman added inline comments. Comment at: include/clang/Tooling/Refactoring/ASTSelection.h:68-74 +/// Traverses the given ASTContext and creates a tree of selected AST nodes. +/// +/// \returns None if no nodes are selected in the AST, or a selected AST node +/// that correspon

[PATCH] D35012: [refactor] Add the AST source selection component

arphaman updated this revision to Diff 107067. arphaman marked 7 inline comments as done. arphaman added a comment. - Address review comments. - Remove the `Location` parameter and `ContainsSelectionPoint` enum value. - Stop traversing early when a declaration that ends after the selection range

[PATCH] D35012: [refactor] Add the AST source selection component

arphaman added inline comments. Comment at: include/clang/Tooling/Refactoring/ASTSelection.h:68-74 +/// Traverses the given ASTContext and creates a tree of selected AST nodes. +/// +/// \returns None if no nodes are selected in the AST, or a selected AST node +/// that correspon

[PATCH] D35520: [Sema] Improve diagnostic message for unavailable c++17 aligned allocation functions

arphaman added inline comments. Comment at: include/clang/Basic/AlignedAllocation.h:25 + +static inline VersionTuple alignedAllocMinVersion(llvm::Triple::OSType OS) { + switch (OS) { Redundant `static`? Comment at: include/clang/Basic/AlignedA

[PATCH] D35484: Add a warning for missing '#pragma pack (pop)' and suspicious '#pragma pack' uses when including files

arphaman updated this revision to Diff 107090. arphaman marked 5 inline comments as done. arphaman added a comment. Address review comments. Repository: rL LLVM https://reviews.llvm.org/D35484 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td inc

[PATCH] D35012: [refactor] Add the AST source selection component

arphaman added inline comments. Comment at: lib/Tooling/Refactoring/ASTSelection.cpp:164 + unsigned NumMatches = 0; + for (Decl *D : Context.getTranslationUnitDecl()->decls()) { +if (ObjCImplEndLoc.isValid() && klimek wrote: > arphaman wrote: > > klimek wro

[PATCH] D35012: [refactor] Add the AST source selection component

arphaman updated this revision to Diff 107121. arphaman added a comment. Factor out the lexical ordering code into a new visitor and simplify the implementation of the ast selection visitor Repository: rL LLVM https://reviews.llvm.org/D35012 Files: include/clang/AST/LexicallyOrderedRecurs

[PATCH] D35012: [refactor] Add the AST source selection component

arphaman added a comment. Oops, I just realised that now there's a small bug with early exits. Since we don't actually have true lexical order for declarations in the @implementation we might exit early after visiting a method in the @implementation before a function that's actually written bef

[PATCH] D35484: Add a warning for missing '#pragma pack (pop)' and suspicious '#pragma pack' uses when including files

This revision was automatically updated to reflect the committed changes. Closed by commit rL308327: Add a warning for missing '#pragma pack (pop)' and suspicious uses (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D35484?vs=107090&id=107130#toc Repository: rL LLVM

[PATCH] D35012: [refactor] Add the AST source selection component

arphaman updated this revision to Diff 107138. arphaman added a comment. rm early exit bug Repository: rL LLVM https://reviews.llvm.org/D35012 Files: include/clang/AST/LexicallyOrderedRecursiveASTVisitor.h include/clang/Basic/SourceLocation.h include/clang/Basic/SourceManager.h inclu

[PATCH] D35520: [Sema] Improve diagnostic message for unavailable c++17 aligned allocation functions

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

[PATCH] D35484: Add a warning for missing '#pragma pack (pop)' and suspicious '#pragma pack' uses when including files

arphaman added a comment. This triggered a warning in LLVM itself, in CoverageMapping.h : error: non-default #pragma pack value might change the alignment of struct or union members in the included file [-Werror,-Wpragma-pack] #include "llvm/ProfileData/InstrProfData.inc" ^ /m

[PATCH] D34329: [clang-diff] Initial implementation.

arphaman added a comment. I'll commit this on behalf of Johannes today as he didn't get his access yet https://reviews.llvm.org/D34329 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34329: [clang-diff] Initial implementation.

This revision was automatically updated to reflect the committed changes. Closed by commit rL308731: [clang-diff] Add initial implementation (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D34329?vs=105161&id=107661#toc Repository: rL LLVM https://reviews.llvm.org/

[PATCH] D35726: unguarded availability: add a fixit for the "annotate '...' with an availability attribute to silence" note

arphaman created this revision. The fixit is given only if the `API_AVAILABLE` macro is defined. Repository: rL LLVM https://reviews.llvm.org/D35726 Files: lib/Sema/SemaDeclAttr.cpp test/FixIt/fixit-availability.c test/FixIt/fixit-availability.mm Index: test/FixIt/fixit-availability.

[PATCH] D34748: [clang-diff] improve mapping accuracy, HTML side-by-side diff.

arphaman added a comment. Some comments: - I had to make a couple of post-commit fixes to appease the buildbots when I committed your first patch, so it looks like this one doesn't apply cleanly anymore. Can you please rebase it? - Let's separate the addition of HTML output and the improvements

[PATCH] D35735: [ubsan] Null-check pointers in -fsanitize=vptr (PR33881)

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM! Comment at: test/CodeGenCXX/ubsan-devirtualized-calls.cpp:67 static_cast(badp)->f1(); //< No devirt, test 'badp isa Base1'. + // We were able to skip the null c

[PATCH] D35736: [ubsan] -fsanitize=vptr now requires -fsanitize=null, update tests

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

[PATCH] D35735: [ubsan] Null-check pointers in -fsanitize=vptr (PR33881)

arphaman added a comment. You might also want to mention the fact that `-fsanitizer=vptr` requires `null` in the release notes. https://reviews.llvm.org/D35735 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/ma

[PATCH] D35726: unguarded availability: add a fixit for the "annotate '...' with an availability attribute to silence" note

arphaman marked 3 inline comments as done. arphaman added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:7230 +return; + for (const auto &M : S.getPreprocessor().macros()) { +if (M.first->getName() != "API_AVAILABLE") erik.pilkington

[PATCH] D35726: unguarded availability: add a fixit for the "annotate '...' with an availability attribute to silence" note

arphaman updated this revision to Diff 107903. arphaman marked 2 inline comments as done. arphaman added a comment. Addressed Erik's comments Repository: rL LLVM https://reviews.llvm.org/D35726 Files: lib/Sema/SemaDeclAttr.cpp test/FixIt/fixit-availability.c test/FixIt/fixit-availabili

[PATCH] D35012: [refactor] Add the AST source selection component

arphaman updated this revision to Diff 108078. arphaman added a comment. Simplified the implementation of `LexicallyOrderedRecursiveASTVisitor` and removed redundant DenseMap checks. Ping. Repository: rL LLVM https://reviews.llvm.org/D35012 Files: include/clang/AST/LexicallyOrderedRecursi

[PATCH] D35726: unguarded availability: add a fixit for the "annotate '...' with an availability attribute to silence" note

This revision was automatically updated to reflect the committed changes. arphaman marked 2 inline comments as done. Closed by commit rL309116: unguarded availability: add a fixit for the "annotate '...' (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D35726?vs=107903&

[PATCH] D35894: Code hover for Clangd

arphaman added a comment. arphaman added inline comments. Comment at: clangd/Protocol.h:106 Range range; + FileID unitFileID; It doesn't look like it's appropriate to store a translation unit specific value in the `Location` which, if I'm not mistaken, is

[PATCH] D34748: [clang-diff] improve mapping accuracy

arphaman added a comment. In general this patch is too big, and combines a lot of changes that could be separated using multiple patches. I've suggested one pre-commit and a new patch so far, but more changes might be required once I see the updated patch. Comment at: include

[PATCH] D41688: [Lex] Fix crash on code completion in comment in included file.

arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. I think that's a sensible fix, thanks! https://reviews.llvm.org/D41688 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D42561: [PR36008] Avoid -Wsign-compare warning for enum constants in typeof expressions

arphaman created this revision. arphaman added a reviewer: lebedev.ri. This patch looks through typeof type at the original expression when diagnosing -Wsign-compare to avoid an unfriendly diagnostic. Repository: rC Clang https://reviews.llvm.org/D42561 Files: lib/Sema/SemaChecking.cpp

[PATCH] D42660: [PR32482] Fix bitfield layout for -mms-bitfield and pragma pack

arphaman created this revision. arphaman added reviewers: ahatanak, rjmccall. Herald added a subscriber: jkorous-apple. Clang currently generates wrong record layout for `-mms-bitfield` and `#pragma pack`. https://godbolt.org/g/nQ4rVW shows how MSVC and GCC generate different layout to Clang. T

[PATCH] D42660: [PR32482] Fix bitfield layout for -mms-bitfield and pragma pack

arphaman added a comment. Oops, that logic turned out to be incorrect. We simply have to start a new storage unit when the new bitfield's size is wider than the available bits. Repository: rL LLVM https://reviews.llvm.org/D42660 ___ cfe-commits

[PATCH] D42660: [PR32482] Fix bitfield layout for -mms-bitfield and pragma pack

arphaman updated this revision to Diff 132034. arphaman added a comment. Herald added a subscriber: llvm-commits. Fix packing logic. Repository: rL LLVM https://reviews.llvm.org/D42660 Files: lib/AST/RecordLayoutBuilder.cpp test/CodeGen/mms-bitfields.c test/Sema/mms-bitfields.c Index

[PATCH] D42660: [PR32482] Fix bitfield layout for -mms-bitfield and pragma pack

This revision was automatically updated to reflect the committed changes. Closed by commit rC323921: [PR32482] Fix bitfield layout for -mms-bitfield and pragma pack (authored by arphaman, committed by ). Repository: rC Clang https://reviews.llvm.org/D42660 Files: lib/AST/RecordLayoutBuilder

[PATCH] D29819: Introduce an 'external_source_symbol' attribute that describes the origin and the nature of a declaration

arphaman created this revision. This patch adds an `external_source_symbol` attribute to Clang. This attribute specifies that a declaration originates from an external source and describes the nature of that source. This attribute is useful for mixed-language projects or project that use auto-g

[PATCH] D29819: Introduce an 'external_source_symbol' attribute that describes the origin and the nature of a declaration

arphaman updated this revision to Diff 88222. arphaman marked 9 inline comments as done. arphaman added a comment. Thanks for the feedback! I've addressed the majority of your comments. Repository: rL LLVM https://reviews.llvm.org/D29819 Files: include/clang/Basic/Attr.td include/clang/B

[PATCH] D29819: Introduce an 'external_source_symbol' attribute that describes the origin and the nature of a declaration

arphaman added inline comments. Comment at: include/clang/Basic/AttrDocs.td:1007 + +defined_in=\ *string-literal* + The name of the source container in which the declaration was defined. The aaron.ballman wrote: > Would this hold something like a file name? If s

[PATCH] D29886: [clangd] Wire up ASTUnit and publish diagnostics with it.

arphaman added inline comments. Comment at: clangd/ASTManager.cpp:69 + // our one-element queue is empty. + if (!RequestIsPending && !Done) +ClangRequestCV.wait(Lock); This is just a suggestion based on personal opinion: `RequestIsPending` and

[PATCH] D29530: [ubsan] Reduce null checking of C++ object pointers (PR27581)

arphaman added inline comments. Comment at: test/CodeGenCXX/ubsan-suppress-null-checks.cpp:8 + int load_member() { +// CHECK: call void @__ubsan_handle_type_mismatch +// CHECK-NOT: call void @__ubsan_handle_type_mismatch I think this kind of check would'

[PATCH] D28286: [CodeCompletion] Code complete the missing C++11 keywords

This revision was automatically updated to reflect the committed changes. Closed by commit rL295001: [CodeCompletion] Code complete the missing C++11 keywords (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D28286?vs=83030&id=88271#toc Repository: rL LLVM https://r

[PATCH] D29967: Get class property selectors from property decl if it exists

arphaman added a comment. Can you please post the patch with full context (http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)? https://reviews.llvm.org/D29967 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D30009: Add support for '#pragma clang attribute'

arphaman created this revision. This patch adds support for the `#pragma clang attribute` directive that was proposed recently at http://lists.llvm.org/pipermail/cfe-dev/2017-February/052689.html. Initially it supports the `annotate`, `require_constant_initialization` and `objc_subclassing_res

[PATCH] D29944: libclang: Print namespaces for typedefs and type aliases

arphaman added a comment. I'm quite surprised by the fact that we have to change so many diagnostics in tests. Do these diagnostics have the full qualifiers for errors that occur with record types instead of typedefs? https://reviews.llvm.org/D29944 _

[PATCH] D30009: Add support for '#pragma clang attribute'

arphaman added inline comments. Comment at: docs/LanguageExtensions.rst:2349 +attribute is supported by the pragma by referring to the +:doc:`individual documentation for that attribute `. efriedma wrote: > I'm wondering if we can tweak the approach so that we do

[PATCH] D29530: [ubsan] Reduce null checking of C++ object pointers (PR27581)

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

[PATCH] D30009: Add support for '#pragma clang attribute'

arphaman added a comment. In https://reviews.llvm.org/D30009#678091, @hfinkel wrote: > I don't understand why it only supports some attributes. Is there some > handling that needs to take place (I don't see anything obvious in this > patch)? If most attributes will "just work", I'd much rather

[PATCH] D30009: Add support for '#pragma clang attribute'

arphaman updated this revision to Diff 88764. arphaman marked an inline comment as done. arphaman added a comment. The updated patch switches over to the opt-out approach, allows the C++11 style syntax and improves documentation. Repository: rL LLVM https://reviews.llvm.org/D30009 Files:

[PATCH] D30009: Add support for '#pragma clang attribute'

arphaman added inline comments. Comment at: docs/LanguageExtensions.rst:2349 +attribute is supported by the pragma by referring to the +:doc:`individual documentation for that attribute `. arphaman wrote: > efriedma wrote: > > I'm wondering if we can tweak the ap

<    8   9   10   11   12   13   14   15   >