[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

This revision was automatically updated to reflect the committed changes. Closed by commit rL359967: [c++20] Implement P0428R2 - Familiar template syntax for generic lambdas (authored by hamzasood, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood marked 6 inline comments as done. hamzasood added inline comments. Comment at: lib/AST/DeclPrinter.cpp:107-108 -void printTemplateParameters(const TemplateParameterList *Params); +void printTemplateParameters(const TemplateParameterList *Params, +

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 177221. hamzasood added a comment. - Committed the test framework changes separately as r348589 and r348603. - Correctly cleanup the lambda scope if an error in encountered while parsing the template parameters. - Replaced

[PATCH] D51868: [libcxx] Build and test fixes for Windows

hamzasood added inline comments. Comment at: CMakeLists.txt:550 +endif() add_compile_flags_if_supported( +-Wextra -W -Wwrite-strings EricWF wrote: > Couldn't we keep the "-Wall" here? And just add something else that adds > "/W4"? > > Also, we don't yet

[PATCH] D51868: [libcxx] Build and test fixes for Windows

hamzasood added inline comments. Comment at: test/support/verbose_assert.h:26 static_assert(ST == -1, "specialization required for ST != -1"); static void Print(Tp const&) { std::clog << "Value Not Streamable!\n"; } }; mclow.lists wrote: > > The renaming

[PATCH] D51868: [libcxx] Build and test fixes for Windows

hamzasood added inline comments. Comment at: include/filesystem:1396 - _LIBCPP_FUNC_VIS void __create_what(int __num_paths); compnerd wrote: > This possibly changes the meaning on other targets. What was the error that > this triggered? I've re-uploaded

[PATCH] D51868: [libcxx] Build and test fixes for Windows

hamzasood updated this revision to Diff 166124. hamzasood added a comment. Herald added a subscriber: christof. I've added a fix for another related issue: the tests would fail to link if libc++ is built only as a DLL. The auto-linking doesn't look for a dynamic library unless `_DLL` is defined

[PATCH] D51868: [libcxx] Build and test fixes for Windows

hamzasood created this revision. hamzasood added reviewers: rnk, EricWF, compnerd, smeenai. Herald added subscribers: cfe-commits, ldionne, mgorny. The patch fixes a few issues that arise when using libc++ on Windows. Specifically: 1. The CMake configuration added `-Wall` to the compilation,

[PATCH] D51321: [Tooling] Improve handling of CL-style options

hamzasood added a comment. Thanks for the help with this. I reverted most of my changes (including the parameterised tests) before committing; I think I implemented all of your suggestions. Repository: rL LLVM https://reviews.llvm.org/D51321

[PATCH] D51321: [Tooling] Improve handling of CL-style options

This revision was automatically updated to reflect the committed changes. Closed by commit rL341760: [Tooling] Improve handling of CL-style options (authored by hamzasood, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D51321: [Tooling] Improve handling of CL-style options

hamzasood marked 10 inline comments as done. hamzasood added inline comments. Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:124 +// Determine whether the given file path is the clang-cl executable. +static bool checkIsCLMode(const std::vector , +

[PATCH] D51321: [Tooling] Improve handling of CL-style options

hamzasood updated this revision to Diff 163098. hamzasood added a comment. I've addressed most of your comments and resolved the merge conflicts introduced in r340838. https://reviews.llvm.org/D51321 Files: lib/Tooling/InterpolatingCompilationDatabase.cpp

[PATCH] D51321: [Tooling] Improve handling of CL-style options

hamzasood added a comment. Thanks for the detailed feedback; I’ll try to get a new patch up in a few hours. However I disagree with some of them (see replies). Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:124 +// Determine whether the given file path is the

[PATCH] D51321: [Tooling] Improve handling of CL-style options

hamzasood updated this revision to Diff 162872. hamzasood added a comment. - Re-uploaded with full context. Yeah, I encountered these issues while using clangd on Windows. I haven't run into any clangd issues after applying this patch, but admittedly my usage is very basic (pretty much just

[PATCH] D51321: [Tooling] Improve handling of CL-style options

hamzasood marked an inline comment as done. hamzasood added inline comments. Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:136 + // Otherwise just check the clang file name. + return llvm::sys::path::stem(CmdLine.front()).endswith_lower("clang-cl"); +}

[PATCH] D51321: [Tooling] Improve handling of CL-style options

hamzasood updated this revision to Diff 162762. hamzasood added a comment. Thanks, I've changed it so that the driver mode is calculated first. I've also extended the unit test to include the various `--driver-mode` combinations. https://reviews.llvm.org/D51321 Files:

[PATCH] D51321: [Tooling] Improve handling of CL-style options

hamzasood created this revision. hamzasood added reviewers: rnk, hokein, sammccall. Herald added a subscriber: cfe-commits. This patch fixes the handling of clang-cl options in `InterpolatingCompilationDatabase`. They were previously ignored completely, which led to a lot of bugs: - Additional

[PATCH] D50764: [AST] Make NullStmt final and give it factory functions

hamzasood added a comment. Thanks. I'm not planning to move forward with this until the contracts patch is ready (it might change enough to not even require this); that should leave plenty of time for others to take a look. Repository: rC Clang https://reviews.llvm.org/D50764

[PATCH] D50662: Add dump() method for SourceRange

hamzasood added a comment. This seems like a reasonable addition. Maybe it could also provide `print` and `printToString` to more closely match the `SourceLocation` API? Repository: rC Clang https://reviews.llvm.org/D50662 ___ cfe-commits

[PATCH] D50764: [AST] Make NullStmt final and give it factory functions

hamzasood created this revision. hamzasood added reviewers: klimek, steveire, bkramer. Herald added a subscriber: cfe-commits. I've submitted a patch for contracts (review linked to this) that adds trailing objects to NullStmt. This patch contains the changes to make that possible. Repository:

[PATCH] D50763: [Parser] Refactor and fix bugs in late-parsing

hamzasood created this revision. hamzasood added reviewers: erichkeane, v.g.vassilev, malcolm.parsons, rsmith. Herald added a reviewer: javed.absar. Herald added subscribers: cfe-commits, kristof.beyls, eraman. This patch extracts the eof/stop token pattern used in late-parsing to a re-usable

[PATCH] D49647: [libcxx] Library support for contracts (C++2a)

hamzasood updated this revision to Diff 156857. hamzasood added a comment. - Made the suggested test changes. The compiler is meant to create these; Clang doesn't do this yet but I'm working on the implementation. I was under the impression that the compiler support would require the library

[PATCH] D49647: [libcxx] Library support for contracts (C++2a)

hamzasood updated this revision to Diff 156808. hamzasood added a comment. - Added a missing visibility attribute. - Added a simple test that type-checks the expected public interface. Testing whether it's actually hooked up correctly is quite problematic; it isn't meant to be instantiable.

[PATCH] D49647: [libcxx] Library support for contracts (C++2a)

hamzasood created this revision. hamzasood added reviewers: EricWF, mclow.lists, rsmith. Herald added subscribers: cfe-commits, ldionne, christof. This patch adds the library components needed for contracts in C++2a. The wording says that a contract_violation object is populated in an

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 156693. hamzasood added a comment. - Resolved merge conflicts https://reviews.llvm.org/D36527 Files: include/clang/AST/DeclCXX.h include/clang/AST/DeclTemplate.h include/clang/AST/ExprCXX.h include/clang/AST/RecursiveASTVisitor.h

[PATCH] D41627: [Modules TS] Fix namespace visibility

hamzasood updated this revision to Diff 156689. hamzasood added a comment. - Don't assume that -fmodules-ts implies that we're in a TS module. - Don't ignore private namespaces when calculating the ownership kind. https://reviews.llvm.org/D41627 Files: include/clang/AST/DeclBase.h

[PATCH] D41566: [Modules TS] Diagnose exported internal linkage declarations

hamzasood updated this revision to Diff 156681. hamzasood added a comment. - Fixed the premature loop termination issue pointed out by @rsmith (and added a test case for it). - Fixed a few small issues regarding diagnosing exported enum/struct/union declarations without a name (especially

[PATCH] D40443: [Modules TS] Make imports from an interface unit visible to its implementation units

hamzasood marked 2 inline comments as done. hamzasood added inline comments. Comment at: lib/Sema/SemaDecl.cpp:16185-16194 + if (getLangOpts().ModulesTS) { +Module *CurrentModule = getCurrentModule(); +assert(CurrentModule && "Expected to be in a module scope"); + +

[PATCH] D40443: [Modules TS] Make imports from an interface unit visible to its implementation units

hamzasood updated this revision to Diff 150649. hamzasood added a comment. Addressed Richard's comments. https://reviews.llvm.org/D40443 Files: include/clang/Basic/Module.h lib/Basic/Module.cpp lib/Sema/SemaDecl.cpp test/CXX/modules-ts/basic/basic.def.odr/p6/module-vs-module.cpp

[PATCH] D41627: [Modules TS] Fix namespace visibility

hamzasood created this revision. hamzasood added reviewers: rsmith, bruno, boris. Namespaces without an explicit `export` were being mistakenly marked as module private. This patch fixes the visibility as per `[basic.namespace]p1` and adds a test case with previously rejected (yet valid) code.

[PATCH] D41566: [Modules TS] Diagnose exported internal linkage declarations

hamzasood updated this revision to Diff 128302. hamzasood added a comment. I've removed the `isExported` fix for namespaces as it's somewhat unrelated to this patch. I'll do a separate patch for fixing namespaces (which will include the stuff removed from here and a bit more)

[PATCH] D40443: [Modules TS] Make imports from an interface unit visible to its implementation units

hamzasood added inline comments. Comment at: lib/Basic/Module.cpp:349 + // Everything visible to the interface unit's global module fragment is + // visible to the interface unit. boris wrote: > This comment (not sure about the code) sounds wrong to me:

[PATCH] D40443: [Modules TS] Make imports from an interface unit visible to its implementation units

hamzasood updated this revision to Diff 128284. hamzasood added a comment. I've been investigating an issue regarding visibility during code synthesis, and I noticed that this patch half fixes it. I've added the rest of the fix since it's related to what's going on here, and I guess this is now

[PATCH] D41566: [Modules TS] Diagnose exported internal linkage declarations

hamzasood created this revision. hamzasood added reviewers: rsmith, bruno, boris. Diagnose attempts to export declarations with internal linkage, as mentioned in `[dcl.module.interface]p2` in the Modules TS. I would've liked to add a FixIt hint to remove the static keyword if present, but I

[PATCH] D40443: [Modules TS] Make imports from an interface unit visible to its implementation units

hamzasood added a comment. Ping https://reviews.llvm.org/D40443 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40443: [Modules TS] Make imports from an interface unit visible to its implementation units

hamzasood updated this revision to Diff 124231. hamzasood added a comment. Good idea, I've added that to the test. I'll give Richard some time to look over this before committing. https://reviews.llvm.org/D40443 Files: lib/Sema/SemaDecl.cpp

[PATCH] D40443: [Modules TS] Make imports from an interface unit visible to its implementation units

hamzasood created this revision. This provides an implementation for the changes outlined in section 4.1 of P0731r0, which clarifies the intended behaviour regarding implementation units being able to see imports made within their corresponding interface unit. https://reviews.llvm.org/D40443

[PATCH] D40270: [Modules TS] Added re-export support

This revision was automatically updated to reflect the committed changes. Closed by commit rL318744: [Modules TS] Added module re-export support. (authored by hamzasood). Changed prior to commit: https://reviews.llvm.org/D40270?vs=123666=123736#toc Repository: rL LLVM

[PATCH] D40270: [Modules TS] Added re-export support

hamzasood created this revision. This patch adds support for re-exporting modules as described in `[dcl.modules.export]` https://reviews.llvm.org/D40270 Files: include/clang/Parse/Parser.h lib/Parse/ParseObjc.cpp lib/Parse/Parser.cpp lib/Sema/SemaDecl.cpp

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 123015. hamzasood added a comment. Resolved merge conflicts from the last month of changes (`unittests/AST/StmtPrinterTest.cpp` and `lib/AST/DeclCXX.cpp`) https://reviews.llvm.org/D36527 Files: include/clang/AST/DeclCXX.h

[PATCH] D38919: [libcxx] Implemented from Library Fundamentals V2

hamzasood created this revision. This patch implements make_array and to_array from the Library Fundamentals V2 TS. https://reviews.llvm.org/D38919 Files: include/experimental/array test/std/experimental/container/array/creation/make_array.fail.cpp

[PATCH] D37182: [libcxx] Special visibility macros for the experimental library

hamzasood added a comment. I have access now, so I'm able to commit this myself. However it's been a while since it was approved, so I'd be grateful if someone could take another look to make sure nothing has changed in the meantime (besides potentially needing to re-tag some new APIs).

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 118261. hamzasood added a comment. - Updated lambda mangling to include explicit template parameters - Allow explicit template parameter lists on lambdas pre-c++2a as an extension. - Improved the somewhat fragile template depth handling. - Reformatted some

[PATCH] D37376: [libcxx] Fix libc++experimental build on Windows

hamzasood updated this revision to Diff 114784. hamzasood added a comment. Changed a CMake macro to a function. https://reviews.llvm.org/D37376 Files: CMakeLists.txt lib/CMakeLists.txt Index: lib/CMakeLists.txt === ---

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 114550. hamzasood added a comment. - Replaced an unneeded `STLExtras.h` include with ``. - Assert that the lambda template scope has a parent before accessing it. - Added an equals in a braced init as per the LLVM coding standards. Thanks @Rakete

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood added a comment. Ping https://reviews.llvm.org/D36527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37182: [libcxx] Special visibility macros for the experimental library

hamzasood added a comment. Thanks. Could someone commit this for me please? Comment at: include/experimental/__config:57 +#if defined(_LIBCPP_OBJECT_FORMAT_COFF) +# define _LIBCPPX_TYPE_VIS compnerd wrote: > hamzasood wrote: > > smeenai wrote: > > > I think

[PATCH] D37376: [libcxx] Fix libc++experimental build on Windows

hamzasood created this revision. Herald added a subscriber: mgorny. This patch fixes a few problems with building libc++experimental on Windows. Previously _LIBCPP_BUILDING_LIBRARY was defined for every source file, including files destined for the experimental library. As a result of this,

[PATCH] D37375: [libcxx] Test case for the experimental library visibility macros

hamzasood created this revision. This patch adds a test case to ensure that every library header uses the correct visibility macros. libc++ headers should use _LIBCPP_* macros whereas libc++experimental headers should use _LIBCPPX_* macros. This was originally part of

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 113271. hamzasood added a comment. Herald added a subscriber: klimek. Implemented pretty printing and recursive AST visiting (both with appropriate unit tests). The pretty printing implementation includes fixing a few printing bugs (which I needed fixed

[PATCH] D37182: [libcxx] Special visibility macros for the experimental library

hamzasood added a comment. In https://reviews.llvm.org/D37182#855262, @smeenai wrote: > The other question, of course, is why the experimental library needs to be > static. If it were built shared, the annotations would just work on Windows > in theory (though I'm sure there are other issues

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 112867. hamzasood added a comment. Hi Richard, thanks for your feedback. This update makes the changes that you requested. It turns out that the casting is no longer an issue since `ParseTemplateParameters` has been refactored to use `NamedDecl`.

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood added a comment. Thanks for all of your feedback! It’s really helped to improve this patch. Comment at: lib/AST/ExprCXX.cpp:979 +SourceRange LambdaExpr::getExplicitTemplateParameterListRange() const { + TemplateParameterList *List = getTemplateParameterList();

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood added inline comments. Comment at: lib/AST/ExprCXX.cpp:979 +SourceRange LambdaExpr::getExplicitTemplateParameterListRange() const { + TemplateParameterList *List = getTemplateParameterList(); faisalv wrote: > I think this should return an invalid

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 112018. hamzasood added a comment. - Info about a lambda's explicit template parameters is now exposed in the AST (see changes to LambdaExpr). It turns out that no extra storage was actually needed to achieve this. - Removed remnants of a previous

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood added inline comments. Comment at: lib/Parse/ParseExprCXX.cpp:1116 + if (HasExplicitTemplateParams) { +SmallVector TemplateParams; +SourceLocation LAngleLoc, RAngleLoc; faisalv wrote: > hamzasood wrote: > > faisalv wrote: > > > Why

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood marked 5 inline comments as done. hamzasood added a comment. So to clarify: NumExplicitTemplateParams should be stored in LambdaDefinitionData (with an accessor in LambdaExpr) and ExplicitTemplateParamRange (SourceRange) should be stored in LambdaExpr? That makes sense, but doesn't

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 111892. hamzasood added a comment. Sorry, I've just spotted a small mistake in how the auto parameter depth is recorded. This update fixes it. https://reviews.llvm.org/D36527 Files: include/clang/Basic/DiagnosticParseKinds.td

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood updated this revision to Diff 111887. hamzasood added a comment. - Corrected a typo. - Made HasExplicitTemplateParams const. - Reverted the change in where template depth is incremented and recorded. - Fixed the broken test. - Keep the template scope active instead of exiting it and

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood added inline comments. Comment at: lib/Parse/ParseExprCXX.cpp:1090 + TemplateParameterDepthRAII CurTemplateDepthTracker(TemplateParameterDepth); + Actions.RecordParsingTemplateParameterDepth(TemplateParameterDepth); + faisalv wrote: > Since you only

[PATCH] D36572: Implemented P0409R2 - Allow lambda capture [=, this]

hamzasood updated this revision to Diff 111681. hamzasood added a comment. It's Friday, so hopefully this is good to go. I don't have svn access, so could someone commit this for me? I've updated the patch so that it can be applied successfully to the latest revision. I've also renamed

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood added inline comments. Comment at: lib/Parse/ParseExprCXX.cpp:1112 + ParseScope TemplateParamScope(this, Scope::TemplateParamScope); + if (getLangOpts().CPlusPlus2a && Tok.is(tok::less)) { faisalv wrote: > We always create a template parameter

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood added a comment. Ping https://reviews.llvm.org/D36527 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36572: Implemented P0409R2 - Allow lambda capture [=, this]

hamzasood added inline comments. Comment at: test/FixIt/fixit-cxx0x.cpp:57 (void)[&, , ]{}; // expected-error 2{{'&' cannot precede a capture when the capture default is '&'}} - (void)[=, this]{ this->g(5); }; // expected-error{{'this' cannot be explicitly captured}}

[PATCH] D36572: Implemented P0409R2 - Allow lambda capture [=, this]

hamzasood added inline comments. Comment at: lib/Sema/SemaLambda.cpp:959 + ? diag::ext_equals_this_lambda_capture_cxx2a + : diag::warn_cxx1z_compat_equals_this_lambda_capture); faisalv wrote: > Shouldn't we try

[PATCH] D36572: Implemented P0409R2 - Allow lambda capture [=, this]

hamzasood updated this revision to Diff 110776. hamzasood added a comment. @rjmccall The warning is emitted, but you're right that there's no test to ensure that actually happens; test/CXX/expr/expr.prim/expr.prim.lambda/p8.cpp used Wno-c++2a-extensions to suppress the warning. I've changed it

[PATCH] D36611: Implemented P0683R1 - Default member initializers for bit-fields

hamzasood created this revision. This patch implements P0683R1 . Member initialisers are allowed pre-C++11 as an extension. So I've also allowed bitfield member initialisers pre-C++2a as an extension (with appropriate warnings) for consistency.

[PATCH] D36572: Implemented P0409R2 - Allow lambda capture [=, this]

hamzasood added inline comments. Comment at: test/FixIt/fixit-cxx0x.cpp:57 (void)[&, , ]{}; // expected-error 2{{'&' cannot precede a capture when the capture default is '&'}} - (void)[=, this]{ this->g(5); }; // expected-error{{'this' cannot be explicitly captured}}

[PATCH] D36572: Implemented P0409R2 - Allow lambda capture [=, this]

hamzasood added inline comments. Comment at: test/FixIt/fixit-cxx0x.cpp:57 (void)[&, , ]{}; // expected-error 2{{'&' cannot precede a capture when the capture default is '&'}} - (void)[=, this]{ this->g(5); }; // expected-error{{'this' cannot be explicitly captured}}

[PATCH] D36572: Implemented P0409R2 - Allow lambda capture [=, this]

hamzasood added inline comments. Comment at: test/FixIt/fixit-cxx0x.cpp:57 (void)[&, , ]{}; // expected-error 2{{'&' cannot precede a capture when the capture default is '&'}} - (void)[=, this]{ this->g(5); }; // expected-error{{'this' cannot be explicitly captured}}

[PATCH] D36584: Implemented P0704R1 - Fixing const-qualified pointers to members

hamzasood created this revision. This patch provides an implementation for P0704R1 . I also made it work pre-C++2a as an extension (with a warning) since that's how Clang seems to deal with small additions like this. Hopefully someone more knowledgable on the matter

[PATCH] D36572: Implemented P0409R2 - Allow lambda capture [=, this]

hamzasood created this revision. This patch implements P0409R2 . '*this' capture is allowed pre-C++17 as an extension. So I've also allowed [=, this] pre-C++2a as an extension (with appropriate warnings) for consistency. https://reviews.llvm.org/D36572 Files:

[PATCH] D36527: Implemented P0428R2 - Familiar template syntax for generic lambdas

hamzasood created this revision. This patch provides an implementation for P0428R2 . https://reviews.llvm.org/D36527 Files: include/clang/Basic/DiagnosticParseKinds.td include/clang/Sema/ScopeInfo.h include/clang/Sema/Sema.h lib/Parse/ParseExprCXX.cpp

[PATCH] D35729: [Frontend] - Mark some ASTUnit methods as const

hamzasood added a comment. Could someone commit this for me? I don't have access. https://reviews.llvm.org/D35729 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35729: [Frontend] - Mark some ASTUnit methods as const

hamzasood created this revision. While using the libtooling API, I ran into problems when using a const ASTUnit. There were a few functions I needed to use that don't seem to modify anything, but they weren't marked as const so I couldn't use them. This patch marks those methods (and a few

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

hamzasood added a comment. In https://reviews.llvm.org/D30991#703199, @zturner wrote: > lgtm, do you have commit access? No, I don't. https://reviews.llvm.org/D30991 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

hamzasood updated this revision to Diff 92050. hamzasood added a comment. @zturner: "it's just a vector of pointers" Oh.. I was so focussed on reducing copies that I completely forgot it's just an array of pointers being copied, not the entire environment block contents. In that case, yeah.

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

hamzasood added a comment. In https://reviews.llvm.org/D30991#702965, @thakis wrote: > When you say "cross-compiling", you mean targeting Windows while running on > non-Windows, right? How do dlls get loaded there at all? > > Also, when does clang invoke link.exe? Normally on Windows the linker

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

hamzasood added a comment. *dynamic loader And that's the cross-compiling link.exes that have those requirements. The native ones have the required dlls in their containing directories. https://reviews.llvm.org/D30991 ___ cfe-commits mailing list

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

hamzasood added a comment. In https://reviews.llvm.org/D30991#702941, @thakis wrote: > What env vars are needed here? Reading an env file seems a bit inelegant, > could we pass the values of these env vars as flags instead? > > For example, MSVC2015 needs %INCLUDE%, but for cross-compiling (and

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

hamzasood updated this revision to Diff 92019. hamzasood added a comment. - Added an assertion in Command::Execute to ensure the environment vector is null-terminated before using it. - Split Command::setEnvironment so that there's an overload to specifically handle the case where the input

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

hamzasood added inline comments. Comment at: lib/Driver/ToolChains/MSVC.cpp:487-488 + if (!llvm::convertUTF16ToUTF8String( + llvm::ArrayRef(reinterpret_cast(EnvBlockWide.get()), + EnvBlockLen * sizeof(EnvBlockWide[0])), +

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

hamzasood marked an inline comment as done. hamzasood added inline comments. Comment at: lib/Driver/ToolChains/MSVC.cpp:487-488 + if (!llvm::convertUTF16ToUTF8String( + llvm::ArrayRef(reinterpret_cast(EnvBlockWide.get()), +

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

hamzasood updated this revision to Diff 91962. hamzasood added a comment. Changed the call to GetEnvironmentStrings to use the unicode version https://reviews.llvm.org/D30991 Files: include/clang/Driver/Job.h lib/Driver/Job.cpp lib/Driver/ToolChains/MSVC.cpp Index:

[PATCH] D30991: [Driver] Fix cross compiling with Visual Studio 2017

hamzasood created this revision. - Adds a settable environment to clang::driver::Command. - Fixes cross compiling with Visual Studio 2017 by ensuring the linker has the correct environment. This was originally part of https://reviews.llvm.org/D28365, however it was removed for separate review

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

hamzasood added a comment. In https://reviews.llvm.org/D28365#665408, @rnk wrote: > Doesn't your fix mean that the tests will fail on a Windows machine that > doesn't have VS because LLVM was built with mingw? Usually in these > situations we provide some way to provide a fake toolchain. I'm

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

hamzasood added a comment. In https://reviews.llvm.org/D28365#664892, @rnk wrote: > This is ready to land. Do you need someone to commit this? I think so, yeah. https://reviews.llvm.org/D28365 ___ cfe-commits mailing list

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

hamzasood updated this revision to Diff 86502. hamzasood added a comment. Added a FIXME comment about configuring use of the Setup Configuration API. In its current form (RC3) the header is distributed in a nuget package, so it's installed per-project instead of being in a system location that

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

hamzasood marked 5 inline comments as done. hamzasood added a comment. Ping https://reviews.llvm.org/D28365 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

hamzasood added inline comments. Comment at: lib/Driver/MSVCToolChain.cpp:34 + #if 0 +#define USE_VS_SETUP_CONFIG + #endif rnk wrote: > What are the outstanding issues preventing you from enabling this by default? Building on Win32 doesn't imply that

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

hamzasood updated this revision to Diff 84446. hamzasood added a comment. Broke up findVCToolChainPath into a few smaller functions. https://reviews.llvm.org/D28365 Files: include/clang/Basic/DiagnosticDriverKinds.td lib/Driver/MSVCToolChain.cpp lib/Driver/ToolChains.h

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

hamzasood updated this revision to Diff 83943. hamzasood added a comment. Uploaded the correct diff this time (yes, really)... Not sure how to delete the previous one, but it's very incorrect. https://reviews.llvm.org/D28365 Files: include/clang/Basic/DiagnosticDriverKinds.td

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

hamzasood updated this revision to Diff 83941. hamzasood added a comment. - Rephrased the diagnostic message (and fixed an embarrassing typo). - Reverted the linker environment change for now; building for x86 with VS2017 won't work in the meantime. I'll submit it for review separately after

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

hamzasood added a comment. In https://reviews.llvm.org/D28365#640859, @rnk wrote: > In https://reviews.llvm.org/D28365#640825, @hamzasood wrote: > > > In https://reviews.llvm.org/D28365#640775, @rnk wrote: > > > > > Can we revert the linker environment change from this patch? It'll be > > >

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

hamzasood added a comment. In https://reviews.llvm.org/D28365#640775, @rnk wrote: > Can we revert the linker environment change from this patch? It'll be easier > to review separately. Sure. But that'll break compiling for x86 on Visual Studio 2017, is that okay?

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

hamzasood added a comment. In https://reviews.llvm.org/D28365#639260, @awson wrote: > It's weird th[[ URL | name ]]at cl.exe command-line compiler reports version > 19.10.24629 and lives in the 14.10.24629 directory (only first 2 digits are > different). > > Moreover, in their explanation

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

hamzasood updated this revision to Diff 83555. hamzasood added a comment. - Added an option to set the environment in a clang::driver::Command, which makes the environment modifying introduced in the last update a bit more reliable. @rnk I looked into using the new MSVC toolchain layout to get

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

hamzasood updated this revision to Diff 83473. hamzasood added a comment. - Modify the environment in some cases in buildLinker to keep link.exe happy. Thanks @rnk for taking the time to look at this. As suggested: - Replaced the confusing unique_ptr usage with a struct. - Renamed

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

hamzasood updated this revision to Diff 83325. hamzasood added a comment. Improved the code slightly. Sorry for the spam everyone, this is definitely the one. https://reviews.llvm.org/D28365 Files: include/clang/Basic/DiagnosticDriverKinds.td lib/Driver/MSVCToolChain.cpp

[PATCH] D28365: [Driver] Updated for Visual Studio 2017

hamzasood updated this revision to Diff 83318. hamzasood added a comment. - Re-implemented the PATH searching behaviour (thanks @amccarth for pointing that out) - Updated the base revision. https://reviews.llvm.org/D28365 Files: include/clang/Basic/DiagnosticDriverKinds.td

  1   2   >