[PATCH] D136515: [builtins] Add __builtin_assume_separate_storage.

bruno added inline comments. Comment at: clang/docs/LanguageExtensions.rst:2393 +particular object (so for example, it's never correct to call this function +passing the addresses of fields in the same struct, elements of the same array, +etc.). Not necessarily a

[PATCH] D145641: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

bruno marked 3 inline comments as done. bruno added inline comments. Comment at: clang/lib/CodeGen/CGCoroutine.cpp:483-499 +// The call to get_­return_­object is sequenced before the call to +// initial_­suspend and is invoked at most once, but there are caveats +// r

[PATCH] D145641: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

bruno updated this revision to Diff 505588. bruno marked 2 inline comments as done. bruno added a comment. Update after reviewer comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145641/new/ https://reviews.llvm.org/D145641 Files: clang/inc

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

bruno added a comment. > @bruno, @ChuanqiXu please let us know if you have any objections, otherwise > we will land the revert in ~2 hours. No sweat, I didn't see this in time. Thanks for the reduced testcase. > What's the resolution here? Can we revert this and continue the discussions > inde

[PATCH] D146187: [docs] Update the status for coroutines

bruno added a comment. > ... I think in general we want feature testing macros to be treated as our > signal to the user that we think something is complete, not that we think > something is in progress but pretty usable. +1 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146187/new/ h

[PATCH] D136515: [builtins] Add __builtin_assume_separate_storage.

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/docs/LanguageExtensions.rst:2393 +particular object (so for example, it's never correct to call this function +passing the addresses of fields in the sa

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

bruno updated this revision to Diff 507084. bruno added a comment. Put dependency in place for D145641 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145639/new/ https://reviews.llvm.org/D145639 Files: clang/inc

[PATCH] D145641: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

bruno updated this revision to Diff 507085. bruno added a comment. Address last round of review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145641/new/ https://reviews.llvm.org/D145641 Files: clang/include/clang/AST/StmtCXX.h clang/lib/CodeG

[PATCH] D145641: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

bruno updated this revision to Diff 507087. bruno added a comment. Add release notes entry. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145641/new/ https://reviews.llvm.org/D145641 Files: clang/docs/ReleaseNotes.rst clang/include/clang/AST/S

[PATCH] D145641: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

bruno updated this revision to Diff 507224. bruno added a comment. Apply last round of reviews before landing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145641/new/ https://reviews.llvm.org/D145641 Files: clang/docs/ReleaseNotes.rst clang/i

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

This revision was automatically updated to reflect the committed changes. Closed by commit rG43f5085fa80f: [Coroutines] Fix premature conversion of return object (authored by bruno). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145639/new/ https://

[PATCH] D145641: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfa0d4e1f12a3: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain… (authored by bruno). Repository: rG LLVM Github Monorepo

[PATCH] D136515: [builtins] Add __builtin_assume_separate_storage.

This revision was automatically updated to reflect the committed changes. Closed by commit rG07ef7b1ff21e: [Builtins] Add __builtin_assume_separate_storage (authored by bruno). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136515/new/ https://review

[PATCH] D146758: Fix codegen for coroutine with function-try-block

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM! @ChuanqiXu wdyt? Comment at: clang/test/CodeGenCoroutines/coro-function-try-block.cpp:1 +// RUN: %clang_cc1 -std=c++20 -triple=x86_64-- -emit-llvm -fcxx-exceptions \ +//

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

bruno created this revision. bruno added a reviewer: ChuanqiXu. Herald added subscribers: hoy, modimo, wenlei. Herald added a project: All. bruno requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix https://github.com/llvm/llvm-project/issue

[PATCH] D145641: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

bruno created this revision. bruno added reviewers: ChuanqiXu, clang-language-wg. Herald added subscribers: hoy, modimo, wenlei. Herald added a project: All. bruno requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. - The cwg2563 issue is fixed

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

bruno added a comment. Test failures are unrelated, thanks for the review @ChuanqiXu Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145639/new/ https://reviews.llvm.org/D145639 ___ cfe-commits mailing lis

[PATCH] D145639: [Coroutines] Fix premature conversion of return object

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG54225c457a33: [Coroutines] Fix premature conversion of return object (authored by bruno). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D144680: [Coroutines] Avoid creating conditional cleanup markers in suspend block

bruno added a comment. Thanks for working on this Wei! Can you please add a testcase? Comment at: clang/lib/CodeGen/CodeGenFunction.h:336 std::unique_ptr Data; +bool InSuspendBlock = false; CGCoroInfo(); Should this live inside CGCoroData instead

[PATCH] D149850: [Clang][Modules] Support `requires cplusplus20` in a modulemap

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149850/new/ https://reviews.llvm.org/D149850 ___ c

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

bruno added reviewers: Bigcheese, ChuanqiXu, jansvoboda11. bruno added a comment. Adding code owners and more relevant folks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103930/new/ https://reviews.llvm.org/D103930 ___

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

bruno added a comment. Thanks Duncan. > Given that header maps are somewhat Apple-specific Some non-obvious background: It began Apple specific, but Meta uses them at scale as well, pretty important for us to get this right. > and unit test coverage is a bit lacking for these sorts of interact

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

bruno added a comment. > Looks like this patch causes a number of issues for us, I will work with > @jansvoboda11 to see if there's some way to resolve them. If you can share a reproducer I'm pretty sure @ivanmurashko can help make it work for y'all too. Thanks! Repository: rG LLVM Github

[PATCH] D26267: [Modules] Include builtins with #include instead of #import for ObjC

bruno updated this revision to Diff 81478. bruno added a comment. Here is the Decl dump in the end of `ReadDeclRecord` for each Decl before it fails: - ParmVarDecl 0x7ffe23053b20 col:12 in libc.math hidden 'int' -- FunctionDecl 0x7ffe23053a48 col:5 in libc.math hidden abs 'int (int)' `-Parm

[PATCH] D27796: Fix printf specifier handling: invalid specifier should not be marked as "consuming data arguments"

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

[PATCH] D27832: Add -plugin-opt=sample-profile for thinLTO build.

bruno added inline comments. Comment at: lib/Driver/Tools.cpp:2211 + if (Arg *A = Args.getLastArg(options::OPT_fprofile_sample_use_EQ)) { +StringRef fname = A->getValue(); +if (!llvm::sys::fs::exists(fname)) "fname" -> "FName", "FileName", etc, or any na

[PATCH] D26267: [Modules] Include builtins with #include instead of #import for ObjC

bruno marked 3 inline comments as done. bruno added a comment. > Do we need to perfectly preserve the functionality of `#pragma once` / > `#import`? That is, would it be acceptable to > you if we spuriously enter files that have been imported in that way, while > building a module? Unfortun

[PATCH] D26267: [Modules] Include builtins with #include instead of #import for ObjC

bruno updated this revision to Diff 82067. bruno marked 3 inline comments as done. bruno added a comment. Uploaded new patch with another approach to fix the issue. https://reviews.llvm.org/D26267 Files: include/clang/Lex/HeaderSearch.h include/clang/Lex/ModuleMap.h lib/Lex/HeaderSearch.c

[PATCH] D27852: [modules] Handle modules with nonstandard names in module.private.modulemaps

bruno added a comment. Hi, Thanks for working on this. Comment at: include/clang/Basic/DiagnosticLexKinds.td:647 + "top-level module '%0' in private module map, expected a submodule of '%1'">, + InGroup>; It would be nice if we could also emit a FixIt to t

[PATCH] D27604: [Driver] Add compiler option to generate a reproducer

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

[PATCH] D27852: [modules] Handle modules with nonstandard names in module.private.modulemaps

bruno added a reviewer: bruno. bruno added inline comments. Comment at: test/Modules/implicit-private-with-different-name.m:13 +// expected-warning@Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap:1{{top-level module 'APrivate' in private

[PATCH] D27852: [modules] Handle modules with nonstandard names in module.private.modulemaps

bruno accepted this revision. bruno added a comment. Right, I missed it, LGTM https://reviews.llvm.org/D27852 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26267: [Modules] Include builtins with #include instead of #import for ObjC

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

[PATCH] D27429: [Chrono][Darwin] On Darwin use CLOCK_UPTIME_RAW instead of CLOCK_MONOTONIC

bruno updated this revision to Diff 83158. bruno added a comment. Thanks for the reviews. @howard.hinnant thanks for the great explanation & examples. Attached a new version of the patch, addressing all suggestions. The logic became a bit simpler after Saleem's r290804, which already moved the

[PATCH] D27429: [Chrono][Darwin] On Darwin use CLOCK_UPTIME_RAW instead of CLOCK_MONOTONIC

bruno added a comment. @EricWF ok to commit? https://reviews.llvm.org/D27429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26267: [Modules] Include builtins with #include instead of #import for ObjC

bruno added a comment. @rsmith ping! https://reviews.llvm.org/D26267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26267: [Modules] Include builtins with #include instead of #import for ObjC

bruno closed this revision. bruno added a comment. Thanks for the review Richard. Committed 291644 Comment at: lib/Lex/HeaderSearch.cpp:1082 + auto TryEnterImported = [&](void) -> bool { +if (!ModulesEnabled) rsmith wrote: > Maybe add a FIXME here indic

[PATCH] D30810: Preserve vec3 type.

bruno added a comment. > Thanks for your comment. We have a pass to undo the vec4 to vec3. I wondered > why clang generates the vec4 for vec3 load/store. As you can see the comment > on clang's code, they are generated for better performance. I had a questions > at this point. clang should cons

[PATCH] D30810: Preserve vec3 type.

bruno added a comment. > As you can see, the type legalizer handle vec3 load/store properly. It does > not write 4th element. The vec3 load/store generates more instructions but it > has correct behavior. I am not 100% sure the vec3 --> vec4 load/store is > correct or not because no one has c

[PATCH] D31134: [Serialization] Serialize DependentSizedExtVectorType

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM Alex, thanks! Repository: rL LLVM https://reviews.llvm.org/D31134 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

[PATCH] D31269: [Modules] Allow modules specified by -fmodule-map-file to shadow implicitly found ones

bruno created this revision. When modules come from module map files explicitly specified by -fmodule-map-file= arguments, allow those to override/shadow modules with the same name that are found implicitly by header search. If such a module is looked up by name (e.g. @import), we will always find

[PATCH] D30810: Preserve vec3 type.

bruno added a comment. > I would like to see this patch committed. I see clear evidence of it > improving existing GPU targets in the master repo as well as outside of the > main tree implementations. Bruno, do you have any more concerns? I believe the argument lacks numbers (or at least you ha

[PATCH] D29923: PPCallbacks::MacroUndefined, change signature and add test.

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

[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

bruno added inline comments. Comment at: test/Sema/vector-gcc-compat.c:61 + //match. + v2i64_r = v2i64_a == 1; // expected-warning {{incompatible vector types assigning to 'v2i64' (vector of 2 'long long' values) from 'long __attribute__((ext_vector_type(2)))' (vector

[PATCH] D31269: [Modules] Allow modules specified by -fmodule-map-file to shadow implicitly found ones

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

[PATCH] D31378: [PCH] Attach instance's dependency collectors to PCH external AST sources.

bruno accepted this revision. bruno added a comment. Thanks for working on this! LGTM too https://reviews.llvm.org/D31378 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31241: [Modules][PCH] Serialize #pragma pack

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

[PATCH] D31667: [Sema] Extend GetSignedVectorType to deal with non ExtVector types

bruno added a comment. Great! Thanks for improving this. Comment at: include/clang/Sema/Sema.h:9287 bool AllowBothBool, bool AllowBoolConversion); - QualType GetSignedVectorType(QualType V); + QualType GetSignedVectorType(QualType V, bool WantE

[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

bruno added inline comments. Comment at: lib/Sema/SemaExpr.cpp:8032 + + return InvalidOperands(Loc, LHS, RHS); +} Double checking here: are there tests for the `InvalidOperands` case above? Comment at: lib/Sema/SemaExpr.cpp:8163 +/// type with

[PATCH] D31667: [Sema] Extend GetSignedVectorType to deal with non ExtVector types

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Thanks Simon! LGTM https://reviews.llvm.org/D31667 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

bruno added inline comments. Comment at: lib/Sema/SemaExpr.cpp:8188 + //type and then perform the rest of the checks here. GCC as of + //pre-release 7.0 does not accept this though. + if (VectorEltTy->isIntegralType(S.Context) && sdardis wrote:

[PATCH] D31778: [Modules] Implement ODR-like semantics for tag types in C/ObjC

bruno created this revision. Allow ODR for ObjC/C in the sense that we won't keep more that one definition around (merge them). However, ensure the decl pass the structural compatibility check in C11 6.2.7/1, for that, reuse the structural equivalence checks used by the ASTImporter. Few other

[PATCH] D31781: [Modules] Allow local submodule visibility without c++

bruno created this revision. Removing this restriction will make it handy to explore local submodule visibility without c++ being turned on. We're currently testing it out with C/ObjC. https://reviews.llvm.org/D31781 Files: lib/Frontend/CompilerInvocation.cpp Index: lib/Frontend/CompilerI

[PATCH] D31778: [Modules] Implement ODR-like semantics for tag types in C/ObjC

bruno added inline comments. Comment at: include/clang/AST/ASTStructuralEquivalence.h:52 + /// \brief Whether warn or error on tag type mismatches. + bool ErrorOnTagTypeMismatch; aprantl wrote: > No need to use \brief in new code any more. We are compiling wi

[PATCH] D31778: [Modules] Implement ODR-like semantics for tag types in C/ObjC

bruno updated this revision to Diff 94471. bruno added a comment. Update patch on top of Adrian comments https://reviews.llvm.org/D31778 Files: include/clang/AST/ASTStructuralEquivalence.h include/clang/Basic/DiagnosticASTKinds.td include/clang/Parse/Parser.h include/clang/Sema/Sema.h

[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

bruno added inline comments. Comment at: lib/Sema/SemaExpr.cpp:10024 + // FIXME: The check for C++ here is for GCC compatibility. GCC rejects the + //usage of logical operators with vectors in C. This check could be + //notionally dropped. Plea

[PATCH] D31781: [Modules] Allow local submodule visibility without c++

This revision was automatically updated to reflect the committed changes. Closed by commit rL300108: [Modules] Enable local submodule visibility for ObjC/C (authored by bruno). Changed prior to commit: https://reviews.llvm.org/D31781?vs=94407&id=95034#toc Repository: rL LLVM https://reviews

[PATCH] D27604: [Driver] Add compiler option to generate a reproducer

This revision was automatically updated to reflect the committed changes. Closed by commit rL300109: [Driver] Add compiler option to generate a reproducer (authored by bruno). Changed prior to commit: https://reviews.llvm.org/D27604?vs=80981&id=95035#toc Repository: rL LLVM https://reviews.

[PATCH] D28670: [ObjC] Disallow vector parameters and return values in Objective-C methods on older X86 targets

bruno added inline comments. Comment at: lib/Sema/SemaDeclObjC.cpp:4309 + const ObjCMethodDecl *Method) { + SourceLocation Loc; + QualType T; Maybe add an assert for `Triple.getArch() == llvm::Triple::x86)` here? Repos

[PATCH] D25383: [libcxx][modules] Add a submodule for math.h

bruno abandoned this revision. bruno added a comment. This isn't needed anymore. https://reviews.llvm.org/D25383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29001: [Modules] Fallback to building the module if a timeout occurs

bruno closed this revision. bruno added a comment. Updating: already done in r298175 https://reviews.llvm.org/D29001 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D13117: [DarwinDriver] Use -lto_library to specify the path for libLTO.dylib

bruno closed this revision. bruno added a comment. Done way back in r248932 Repository: rL LLVM https://reviews.llvm.org/D13117 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27429: [Chrono][Darwin] On Darwin use CLOCK_UPTIME_RAW instead of CLOCK_MONOTONIC

bruno closed this revision. bruno added a comment. r291466 & r291517 https://reviews.llvm.org/D27429 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27546: [ASTReader] Sort RawComments before merging

bruno closed this revision. bruno added a comment. Done in r290134 https://reviews.llvm.org/D27546 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31778: [Modules] Implement ODR-like semantics for tag types in C/ObjC

bruno added a comment. @rsmith ping! https://reviews.llvm.org/D31778 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27546: [ASTReader] Sort RawComments before merging

bruno added a comment. > Does this cause us to deserialize the SLocEntry for every FileID referenced > by a RawComment? That would seem pretty bad. I don't recall because It's been a while, but I'm gonna take a look and get back to you. Thanks https://reviews.llvm.org/D27546 __

[PATCH] D28832: Improve redefinition errors pointing to the same header.

bruno marked 5 inline comments as done. bruno added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8708 +def note_redefinition_modules_same_file : Note< + "'%0' included multiple times, additional (likely non-modular) include site in module '%1'">;

[PATCH] D28832: Improve redefinition errors pointing to the same header.

bruno updated this revision to Diff 95369. bruno marked 2 inline comments as done. bruno added a comment. Updated the patch after Richard's comments: Before -- In file included from x.c:2: Inputs4/C.h:3:5: error: redefinition of 'c' int c = 1; ^ In module 'X' imported from x.c:

[PATCH] D28832: Improve redefinition errors pointing to the same header.

bruno added a comment. @v.g.vassilev your testcase is interesting but it's unrelated to this specific improvement. The error message it's triggered by (a) the decl name not being found because `kROOTD_OPEN` is hidden, (b) type transforms looks for an alternative (c) `Sema::diagnoseMissingImport

[PATCH] D31778: [Modules] Implement ODR-like semantics for tag types in C/ObjC

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

[PATCH] D28832: Improve redefinition errors pointing to the same header.

bruno created this revision. Diagnostics related to redefinition errors that point to the same header file do not provide much information that helps fixing the issue. In modules context it usually happens because of a non modular include, in non-module context it might happen because of the la

[PATCH] D25213: Fix PR28181: Prevent operator overloading related assertion failure crash that happens in C only

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. LGTM! Comment at: lib/Sema/SemaExpr.cpp:11523 // being assigned to. if (Opc == BO_Assign && pty->getKind() == BuiltinType::Overload) { + if (getLangOpts().CPlusP

[PATCH] D28832: Improve redefinition errors pointing to the same header.

bruno added a comment. Thanks Vassil. @rsmith is this ok for you? https://reviews.llvm.org/D28832 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27257: [CodeCompletion] Ensure that ObjC root class completes instance methods from protocols and categories as well

bruno added a comment. How does this interact (if at all) with classes annotated with `__attribute__((objc_root_class))`? Comment at: lib/Sema/SemaCodeComplete.cpp:5259 SelIdents, CurContext, Selectors, AllowSameLength, Results, false);

[PATCH] D29001: [Modules] Fallback to building the module if a timeout occurs

bruno created this revision. Herald added a subscriber: fhahn. After https://reviews.llvm.org/D28299 gets in, clang will use PCMCache to manage memory buffers for pcm files. This means the lock file manager isn't needed for correctness anymore, but only as an optimization: clang waits for other

[PATCH] D27257: [CodeCompletion] Ensure that ObjC root class completes instance methods from protocols and categories as well

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Ok. Thanks Alex, LGTM Repository: rL LLVM https://reviews.llvm.org/D27257 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D28299: Module: use PCMCache to manage memory buffers for pcm files.

bruno added inline comments. Comment at: lib/Serialization/ASTReader.cpp:3692 +ValidationConflicts); +for (auto N : ValidationConflicts) + Diag(diag::err_module_ancestor_conflict) << N; This should honor `ARR_OutOfDate`, so th

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

bruno created this revision. This fixes PR31863, a regression introduced in r276159. Consider this snippet: struct FVector; struct FVector {}; struct FBox { FVector Min; FBox(int); }; namespace { FBox InvalidBoundingBox(0); } While parsing the DECL_VAR for 'struct FBox', clang recursively

[PATCH] D34788: [ASTReader] Add test for previous change r306583 / 145692e.

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

[PATCH] D31778: [Modules] Implement ODR-like semantics for tag types in C/ObjC

bruno updated this revision to Diff 104881. bruno added a comment. > Digging into this a bit more, I think the root of the problem is that the > `ND->isExternallyVisible()` call in > `LookupResult::isHiddenDeclarationVisible` is wrong. Redeclaration lookup > should never find hidden enumerato

[PATCH] D34510: Teach clang how to merge typedef over anonymous structs in C mode.

bruno added a comment. Hi @v.g.vassilev, sorry for the delay. Just updated https://reviews.llvm.org/D31778. I agree with Richard's observations, do you think you can extend it to work in the same way as https://reviews.llvm.org/D31778 does? The structural checking is already abstracted there (

[PATCH] D31778: [Modules] Implement ODR-like semantics for tag types in C/ObjC

This revision was automatically updated to reflect the committed changes. Closed by commit rL306918: [Modules] Implement ODR-like semantics for tag types in C/ObjC (authored by bruno). Changed prior to commit: https://reviews.llvm.org/D31778?vs=104881&id=104965#toc Repository: rL LLVM https

[PATCH] D34510: Teach clang how to merge typedef over anonymous structs in C mode.

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. Tested it locally, we actually need all these changes on top of https://reviews.llvm.org/D31778. LGTM Repository: rL LLVM https://reviews.llvm.org/D34510

[PATCH] D33275: Keep into account if files were virtual.

bruno accepted this revision. bruno added a comment. This revision is now accepted and ready to land. We're hitting a very similar problem to this one, which I think this patch will likely fix. I can't come up with a reproducible testcase either. If you can write one better yet, but I'm ok with

[PATCH] D33275: Keep into account if files were virtual.

bruno added a comment. That sounds even better. Thanks @v.g.vassilev https://reviews.llvm.org/D33275 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34985: Do not read the file to determine its name.

bruno added a comment. @v.g.vassilev will this also benefit from `ContentCache::getBuffer` improvements from https://reviews.llvm.org/D33275? I guess if `getBuffer` transparently handles pointing to right buffer we won't need this change? Repository: rL LLVM https://reviews.llvm.org/D34985

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

bruno added a comment. > Thinking more about this, on Windows, is there a strong reason to default to > a different libc by default on Windows? I'm mostly concerned with `-fms-extensions` + darwin, which doesn't apply to this scenario, maybe someone else knows a better answer here. > @bruno

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

This revision was automatically updated to reflect the committed changes. Closed by commit rL308225: Check for _MSC_VER before defining _LIBCPP_MSVCRT (authored by bruno). Changed prior to commit: https://reviews.llvm.org/D34588?vs=103975&id=106960#toc Repository: rL LLVM https://reviews.ll

[PATCH] D34588: Check for _MSC_VER before define _LIBCPP_MSVCRT

bruno added a comment. > Could something like _WIN32 && _MSC_VER make this better? That would allow > us to differentiate between the various environments. It's kinda icky, but, > does make sense in a round about way. The check in the patch was already under a `#if defined(_WIN32)`, having

[PATCH] D34878: [ARM] Option for reading thread pointer from coprocessor register

bruno added a comment. Hi Strahinja, Does GCC already has a similar option? If so, does this share the same name? https://reviews.llvm.org/D34878 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo

[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.

bruno added a comment. Hi Peter, Please add a testcase for this change, it also seems that you should update the comment. https://reviews.llvm.org/D34030 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman

[PATCH] D34878: [ARM] Option for reading thread pointer from coprocessor register

bruno added a comment. If there's a precedence for a flag name there, it's nice to be compatible here. You don't necessarily need to use the same name as the backend. https://reviews.llvm.org/D34878 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.

bruno added a comment. In https://reviews.llvm.org/D34030#819699, @MontyKutyi wrote: > As I can see the `clang/test` contains a lot of different simple tests, but > for testing this I think it is not enough to run the clang with some > arguments on a specific input. So I should create a new ex

[PATCH] D34030: Fix the postorder visting of the ClassTemplateSpecializationDecl nodes in the RecursiveASTVisitor.

bruno added a comment. In https://reviews.llvm.org/D34030#821282, @MontyKutyi wrote: > Added test for the fix. Great! The test code you added doesn't seem to be compatible with clang style, can you run clang format and update the test? https://reviews.llvm.org/D34030 _

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

bruno added a comment. > It seems to me that the problem here is that `DeclMustBeEmitted` is not safe > to call in the middle of deserialization if anything partially-deserialized > might be reachable from the declaration we're querying, and yet we're > currently calling it in that case. `De

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

bruno added a comment. > What do folks think? IMO we should do it. https://reviews.llvm.org/D29753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29753: [PCH] Avoid early VarDecl emission attempt if no owning module avaiable

This revision was automatically updated to reflect the committed changes. Closed by commit rL296656: [PCH] Avoid VarDecl emission attempt if no owning module avaiable (authored by bruno). Changed prior to commit: https://reviews.llvm.org/D29753?vs=87774&id=90212#toc Repository: rL LLVM http

[PATCH] D25866: [Sema] Support implicit scalar to vector conversions

bruno added a comment. Hey, really sorry for the delay here. Comment at: lib/Sema/SemaExpr.cpp:8007 +static bool canConvertIntToOtherIntTy(Sema &S, ExprResult *Int, + QualType OtherIntTy) { + QualType IntTy = Int->get()->getType().getU

[PATCH] D28218: Small optimizations for SourceManager::getFileID()

bruno added a comment. Hi Daniel, This seems pretty nice. Can you share how much performance improvements you got by this / how did you test it? https://reviews.llvm.org/D28218 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D29117: SPARC: allow usage of floating-point registers in inline ASM

bruno added a comment. Hi, Thanks for working on this. Few questions: - What happens with the validation if +soft-float is used? - What about the 'e' mode, can you double check if the sparc backend support these instructions? If so it might be interesting to add it here. Also, please attach pa

<    1   2   3   4   5   >