[PATCH] D134654: [clang] Detect header loops

urnathan added a comment. In D134654#4645969 , @cor3ntin wrote: > @aaron.ballman @urnathan What is the status of this PR? I'd kind of forgotten about it. Did get a query from, I think, boost about how I detected the loops I reported -- which was by

[PATCH] D157661: [clang][NFC] Robustify testcase

This revision was automatically updated to reflect the committed changes. Closed by commit rGe789bcbb967f: [clang][NFC] Robustify testcase (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D137941: [clang] NFC: Robustify test regex

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGeff9d72b9b63: [clang] NFC: Robustify sret test regex (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits.

[PATCH] D138091: [Clang] Fix Sema::ClassifyName so that it classifies EnumConstantDecl as NonType when they are brought into scope via using enum

urnathan added a comment. Thanks, I didn;'t know about ClassifyName, and obviously never hit a need to adjust it. Thanks for fixing. The comment above says we don't resolve member-access non-overload sets in order to check access. Do we still get that right for, say, class B { enum A

[PATCH] D134589: [C++20][Modules] Elide unused guard variables in Itanium ABI module initializers.

urnathan added a comment. It occurs to me that for systems that lack dynamic initialization (like PTX), it is trivial to implement the 'don't call NOP module inits', because we know that there can be no non-nop inits. Any code emission that tries to create a global init fn with contents will

[PATCH] D135958: [clang] modules: Fix callback argument thinko

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG245da0a451e1: [modules] Fix callback argument thinko (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits.

[PATCH] D134654: [clang] Detect header loops

urnathan added inline comments. Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:323 "whitespace recommended after macro name">; +def warn_include_cycle : Warning<"#include cycle">, + InGroup>, DefaultIgnore; philnik wrote: > aaron.ballman wrote:

[PATCH] D134589: [C++20][Modules] Elide unused guard variables in Itanium ABI module initializers.

urnathan accepted this revision. urnathan added a comment. This revision is now accepted and ready to land. This is ok, but see the note I added about object-file compatibility. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:646-647 + If there are no initalizers at all (and

[PATCH] D134654: [clang] Detect header loops

urnathan marked 2 inline comments as done. urnathan added inline comments. Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:323 "whitespace recommended after macro name">; +def warn_include_cycle : Warning<"#include cycle">, + InGroup>, DefaultIgnore;

[PATCH] D134654: [clang] Detect header loops

urnathan updated this revision to Diff 465007. urnathan marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134654/new/ https://reviews.llvm.org/D134654 Files: clang/include/clang/Basic/DiagnosticLexKinds.td clang/include/clang/Lex/Preprocessor.h

[PATCH] D134654: [clang] Detect header loops

urnathan updated this revision to Diff 465006. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134654/new/ https://reviews.llvm.org/D134654 Files: clang/include/clang/Basic/FileEntry.h clang/lib/Lex/PPDirectives.cpp clang/lib/Lex/PPLexerChange.cpp Index:

[PATCH] D134303: [AST] Preserve more structure in UsingEnumDecl node.

urnathan added a comment. Does this still apply with the fix for dr2621 landed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134303/new/ https://reviews.llvm.org/D134303 ___ cfe-commits mailing list

[PATCH] D134283: [clang][DR2621] using enum NAME lookup fix

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3d2080683f1d: [clang][DR2621] using enum NAME lookup fix (authored by urnathan). Herald added a project: clang. Herald added a subscriber:

[PATCH] D134545: [xmm] Remove duplicate #define

This revision was automatically updated to reflect the committed changes. Closed by commit rG53c3664f674c: [xmm] Remove duplicate #define (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D134589: [C++20][Modules] Elide unused guard variables in Itanium ABI module initializers.

urnathan added a comment. Richard & I discussed this, and decided it was a bad idea, as it means we could have an O(N^2) initialization walk, rather than O(N). IIUC this is being motivated by NVPTX, which cannot support dynamically initialized global vars. But in that case we know all the

[PATCH] D134303: [AST] Preserve more structure in UsingEnumDecl node.

urnathan added a comment. AFAICT the UsingDecl doesn't capture the NestedNameSpecifier location, is UsingEnumDecl special in some way, or would this information be better placed in BaseUsingDecl? Thanks for spotting the possible mid-air collistion with D134283

[PATCH] D134243: Don't crash when code completing `using enum ^Foo`.

urnathan added a comment. Might be mooted by fixing https://github.com/llvm/llvm-project/issues/57659, which I am working on Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134243/new/ https://reviews.llvm.org/D134243

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

urnathan added a comment. please sed /initialiser/initializer/, I noticed a few had crept in. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126189/new/ https://reviews.llvm.org/D126189 ___ cfe-commits

[PATCH] D127236: [clang][pr55896]:co_yield/co_await thread-safety

This revision was automatically updated to reflect the committed changes. urnathan marked an inline comment as done. Closed by commit rG65b34b78f8da: [clang][pr55896]:co_yield/co_await thread-safety (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits.

[PATCH] D125542: [clang] co_return cleanup

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4f8668d9f73a: [clang] co_return cleanup (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository:

[PATCH] D126399: [clang][PR55406] CFG for coroutine

This revision was automatically updated to reflect the committed changes. Closed by commit rG6b8c6f15fdd8: [clang][PR55406] CFG for coroutine (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D122741: [clang] Module global init mangling

This revision was automatically updated to reflect the committed changes. Closed by commit rGa1dcfb75ea8c: [clang] Module global init mangling (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

urnathan added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:626-627 + // We create the function, even if it is empty, since an importer of this + // module will refer to it unconditionally (there is no way for an importer + // to know if the function could be

[PATCH] D125535: [clang][NFC] Cleanup some coroutine tests

This revision was automatically updated to reflect the committed changes. Closed by commit rG80bebbc7cb77: [clang][NFC] Cleanup some coroutine tests (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D124946: [clang] serialize ORIGINAL_PCH_DIR relative to BaseDirectory

urnathan accepted this revision. urnathan added a comment. This revision is now accepted and ready to land. good to go, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124946/new/ https://reviews.llvm.org/D124946

[PATCH] D124874: [clang] add -fmodule-file-home-is-cwd

urnathan accepted this revision. urnathan added a comment. This revision is now accepted and ready to land. thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124874/new/ https://reviews.llvm.org/D124874

[PATCH] D124938: [clang] serialize SUBMODULE_TOPHEADER relative to BaseDirectory

urnathan accepted this revision. urnathan 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/D124938/new/ https://reviews.llvm.org/D124938

[PATCH] D125272: [clang] Add -fcheck-new support

urnathan added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:15440 + // + // However, -fcheck-new violates this possible assumption, so don't add + // NonNull when that is enabled. 'invalidates' or 'negates' are better words here. Repository:

[PATCH] D124874: [clang] add -fmodule-file-home-is-cwd

urnathan added inline comments. Comment at: clang/lib/Serialization/ASTWriter.cpp:1231-1236 +SmallString<128> BaseDir(CWD->getName()); +cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir); +BaseDirectory.assign(BaseDir.begin(), BaseDir.end()); +

[PATCH] D124874: [clang] add -fmodule-file-home-is-cwd

urnathan added inline comments. Comment at: clang/lib/Serialization/ASTWriter.cpp:1231-1236 +SmallString<128> BaseDir(CWD->getName()); +cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir); +BaseDirectory.assign(BaseDir.begin(), BaseDir.end()); +

[PATCH] D124946: [clang] serialize ORIGINAL_PCH_DIR relative to BaseDirectory

urnathan added a comment. Looks good to me, but perhaps leave it a few days for others to comment (my familiarity with this code is low). I do know people want relocatable outputs though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124946/new/

[PATCH] D125085: [clang-format] Correctly handle SpaceBeforeParens for builtins.

urnathan added a comment. cool Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125085/new/ https://reviews.llvm.org/D125085 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D122663: Mark identifier prefixes as substitutable

urnathan added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:6031 +bool CXXNameMangler::mangleSubstitution(NestedNameSpecifier *NNS) { + NNS = Context.getASTContext().getCanonicalNestedNameSpecifier(NNS); + return mangleSubstitution(reinterpret_cast(NNS));

[PATCH] D122663: Mark identifier prefixes as substitutable

urnathan added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:6031 +bool CXXNameMangler::mangleSubstitution(NestedNameSpecifier *NNS) { + NNS = Context.getASTContext().getCanonicalNestedNameSpecifier(NNS); + return mangleSubstitution(reinterpret_cast(NNS));

[PATCH] D122663: Mark identifier prefixes as substitutable

urnathan added a comment. [was out last week] It seems the demangler already gets this right, probably because it just looks like a regular nested name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122663/new/ https://reviews.llvm.org/D122663

[PATCH] D123220: [clang] Verify internal entity module mangling

This revision was automatically updated to reflect the committed changes. Closed by commit rG9eda5fc0c6ea: [clang] Verify internal entity module mangling (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D123141: [clang][DOC] Document module mangler changes

This revision was automatically updated to reflect the committed changes. Closed by commit rG482fad4a3fcb: [clang][DOC] Document module mangler changes (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D122885: [clang] Draft: Implement P1703R1

urnathan requested changes to this revision. urnathan added a comment. This revision now requires changes to proceed. Either update this when you're ready, or abandon and create a new one for 1857. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D123119: [clang][NFC] Add specificity to compatibility hack

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGba4482f48199: [clang][NFC] Add specificity to compatibility hack (authored by urnathan). Herald added a project: clang. Herald added a subscriber:

[PATCH] D123120: [clang] Document p1703 not needed

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG54c50336e4c1: [clang] Document p1703 not needed (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits.

[PATCH] D122885: [clang] Draft: Implement P1703R1

urnathan added a comment. Ah, I'd had a thinko about what 1703 was. that's superseded by p1757. That's what is needed. (your patch makes more sense in the 1703 context) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122885/new/

[PATCH] D122885: [clang] Draft: Implement P1703R1

urnathan added a comment. oh, also in 'import "bob";', "bob" needs to be lexed as a #include name, not a string. (and likewise 'import ;') Hence treating these lines in the preprocessor in directive-processing-mode was the way GCC went. Comment at:

[PATCH] D122885: [clang] Draft: Implement P1703R1

urnathan added a comment. I can;t really tell what you're doing. C++20 lexes import () declarations as preprocessor directives. the parser must also recognize only such declarations that came through from lexing such directives. It is unrelated to #import. GCC's implementation drops the

[PATCH] D122256: [clang][ABI] New C++20 module mangling scheme

This revision was automatically updated to reflect the committed changes. Closed by commit rGae4dce8659f3: [clang][ABI] New C++20 module mangling scheme (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit:

[PATCH] D122394: [C++20][Modules] Correct an assert for modules-ts.

urnathan accepted this revision. urnathan added a comment. This revision is now accepted and ready to land. Understood. To document the approach: 1. clearly separate -fmodules-ts semantics (which are vague in places, but whatever) 2. implement c++20 modules semantics 3a) switch -fmodules-ts

[PATCH] D122394: [C++20][Modules] Correct an assert for modules-ts.

urnathan added a comment. Is this because of history that ModulesTS option != p1103 modules? I thought we wanted to make the former become the latter (i.e. ModuleTS is the same as CPlusPlusModules) This seems to be moving in the wrong direction. Repository: rG LLVM Github Monorepo

[PATCH] D122314: [clang] Reformat

This revision was automatically updated to reflect the committed changes. Closed by commit rGde867c6d6ed8: [clang] Reformat (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D121099: [C++20][Modules][HU 5/5] Add fdirectives-only mode for preprocessing output.

urnathan accepted this revision. urnathan added a comment. This revision is now accepted and ready to land. Kind of surprised this wasn't already a thing :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121099/new/ https://reviews.llvm.org/D121099

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

urnathan added a comment. In D122119#3401322 , @ChuanqiXu wrote: > > I think the example is invalid since it violates [[ > http://eel.is/c++draft/module.interface#6 | [module.interface]p6 ]] > explicitly. If this is intended behavior or by design,

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

urnathan added a comment. In D122119#3398949 , @ChuanqiXu wrote: > > The first feeling I saw the change is that not every C++ programmer knows > about linkage. OK, it depends on the environment really and every one might > has their own opinion.

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

urnathan added a comment. In D122119#3398823 , @iains wrote: > > With the change here we now get: > cannot export redeclaration 'f' here since the previous declaration has > internal linkage > cannot export redeclaration 'S' here since the previous

[PATCH] D121098: [C++20][Modules][HU 4/5] Handle pre-processed header units.

urnathan added a comment. yeah, gcc's preprocessor has peeking code to deal with this (more generally than modules) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121098/new/ https://reviews.llvm.org/D121098

[PATCH] D121590: [C++20][Modules][Driver][HU 3/N] Handle foo.h with -fmodule-header and/or C++ invocation.

urnathan accepted this revision. urnathan added a comment. This revision is now accepted and ready to land. looks resonable, 1 nit. Comment at: clang/lib/Driver/Driver.cpp:2453 // Disambiguate headers that are meant to be header units from those -// intended

[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

This revision was automatically updated to reflect the committed changes. Closed by commit rG21e16ab6b8dd: [clang][ABI] New C++20 module mangling scheme (authored by urnathan). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

urnathan added a comment. In D118352#3362694 , @ChuanqiXu wrote: > In D118352#3359626 , @urnathan > wrote: > >> >> Correct, it is not called as the global initializer pieces are not yet >> implemented.

[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

urnathan updated this revision to Diff 413472. urnathan added a comment. added partition tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118352/new/ https://reviews.llvm.org/D118352 Files: clang/include/clang/AST/Mangle.h clang/lib/AST/Decl.cpp

[PATCH] D121063: [AST] Make the last element in the linked list null

urnathan resigned from this revision. urnathan added a comment. Huh? this web interface is confusing. How did this ever work -- doesn't the decl's ctor set this to null? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121063/new/

[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

urnathan added a comment. In D118352#3358864 , @ChuanqiXu wrote: > If I don't misread the codes, it looks like `mangleModuleInitializer` is not > called. > Now we could add test for partitions. Correct, it is not called as the global initializer

[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

urnathan added inline comments. Herald added a project: All. Comment at: clang/test/Modules/inconsist-export-template.cpp:19-23 +// FIXME: We should reject following specialization, +// since it tries to export a name which is already introduced. +export template <> +void f1() {

[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

urnathan updated this revision to Diff 412669. urnathan added a comment. Herald added a project: All. Updated to new partitions API CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118352/new/ https://reviews.llvm.org/D118352 Files: clang/include/clang/AST/Mangle.h

[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

urnathan added inline comments. Comment at: clang/test/Modules/inconsist-export-template.cpp:26-34 +export template +class C { + +}; + +template<> +class C { I think you'll need a member fn to have something to check the linkage of? Might be worth checking

[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

urnathan added a comment. In D120397#3340256 , @erichkeane wrote: > Linkage stuff is where I get lost quickly, hopefully @urnathan can comment > here. Codewise I think this looks right. looks right to me too -- take the linkage from the most general

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

urnathan added a comment. In D119409#3332313 , @dblaikie wrote: > That's interesting data. I guess one could emit the out-of-line bodies into their own sections, and then rely on linker section GC to elide them in the static link. But if you're

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

urnathan added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:2658-2682 +Expr::EvalResult EVRX, EVRY; +if (!DefaultArgumentX->EvaluateAsConstantExpr(EVRX, C) || +!DefaultArgumentY->EvaluateAsConstantExpr(EVRY, C)) + return false; + +APValue

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

urnathan added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:177 + if (IsPartition) { +ModuleName += ":"; +ModuleName += stringFromPath(Partition); iains wrote: > ChuanqiXu wrote: > > iains wrote: > > > urnathan wrote: > > > > iains wrote:

[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

urnathan added a comment. In D118352#3331396 , @ChuanqiXu wrote: > Thanks for explanation. Now it looks good to me. Let's accept it formally > after the series of partition landed and so that we could add test about > partitions. thanks, We're going

[PATCH] D120084: [clang][DOC] Document module mangler changes

urnathan created this revision. urnathan added reviewers: ChuanqiXu, iains, rsmith. urnathan requested review of this revision. Note that the mangling has changed and the demangler's learnt a new trick. Obviously dependent upon the mangler and demangler patches.

[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

urnathan updated this revision to Diff 409761. urnathan added a subscriber: cfe-commits. urnathan added a comment. rebase on top of D116773 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118352/new/ https://reviews.llvm.org/D118352 Files:

[PATCH] D114714: [C++20][Modules][2/8] Add enumerations for partition modules and stream them.

urnathan added a comment. please remind me when this is committed that I'll need to adjust D118352 -- at the moment that's searching for ':' to determine partitionness Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

urnathan added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:2658-2682 +Expr::EvalResult EVRX, EVRY; +if (!DefaultArgumentX->EvaluateAsConstantExpr(EVRX, C) || +!DefaultArgumentY->EvaluateAsConstantExpr(EVRY, C)) + return false; + +APValue

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

urnathan added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:2658-2682 +Expr::EvalResult EVRX, EVRY; +if (!DefaultArgumentX->EvaluateAsConstantExpr(EVRX, C) || +!DefaultArgumentY->EvaluateAsConstantExpr(EVRY, C)) + return false; + +APValue

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

urnathan accepted this revision. urnathan added a comment. This revision is now accepted and ready to land. thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118586/new/ https://reviews.llvm.org/D118586

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

urnathan added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:177 + if (IsPartition) { +ModuleName += ":"; +ModuleName += stringFromPath(Partition); iains wrote: > ChuanqiXu wrote: > > I chose '-' in my implementation since here ModuleName

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

urnathan added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:2658-2682 +Expr::EvalResult EVRX, EVRY; +if (!DefaultArgumentX->EvaluateAsConstantExpr(EVRX, C) || +!DefaultArgumentY->EvaluateAsConstantExpr(EVRY, C)) + return false; + +APValue

[PATCH] D119409: [C++20] [Modules] Remain internal-linkage variables in module interface unit

urnathan added inline comments. Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global -// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global +// CHECK-DAG:

[PATCH] D118598: [C++20][Modules][7/8] Find the primary interface name for a module.

urnathan added inline comments. Comment at: clang/include/clang/Basic/Module.h:522 + + std::string getPrimaryModuleInterfaceName() const { +std::string::size_type pos = Name.find(':'); Is it possible to return a StringRef rather than a copy? Repository:

[PATCH] D118598: [C++20][Modules][7/8] Find the primary interface name for a module.

urnathan added inline comments. Comment at: clang/include/clang/Basic/Module.h:520-522 + /// Get the primary module interface name from a partition. + + std::string getPrimaryModuleInterfaceName() const { stray blank line? Comment at:

[PATCH] D119823: [Modules] Add module structure output to -module-file-info.

urnathan added inline comments. Comment at: clang/test/Modules/module-file-info-cxx20.cpp:26 + +#if TU == 1 + iains wrote: > urnathan wrote: > > urnathan wrote: > > > ChuanqiXu wrote: > > > > iains wrote: > > > > > ChuanqiXu wrote: > > > > > > According to [[ >

[PATCH] D118587: [C++20][Modules][4/8] Handle generation of partition implementation CMIs.

urnathan accepted this revision. urnathan added a comment. This revision is now accepted and ready to land. with the assert fixed Comment at: clang/lib/Sema/SemaModule.cpp:119 +default: + assert(0 && "how did we get a partition type set?"); +}

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

urnathan added inline comments. Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global -// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global +// CHECK-DAG:

[PATCH] D119823: [Modules] Add module structure output to -module-file-info.

urnathan added inline comments. Comment at: clang/test/Modules/module-file-info-cxx20.cpp:26 + +#if TU == 1 + urnathan wrote: > ChuanqiXu wrote: > > iains wrote: > > > ChuanqiXu wrote: > > > > According to [[ > > > >

[PATCH] D119823: [Modules] Add module structure output to -module-file-info.

urnathan added inline comments. Comment at: clang/test/Modules/module-file-info-cxx20.cpp:26 + +#if TU == 1 + ChuanqiXu wrote: > iains wrote: > > ChuanqiXu wrote: > > > According to [[ > > > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1857r3.html |

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

urnathan added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:2654-2656 +if (!C.hasSameType(DefaultArgumentX->getType(), + DefaultArgumentY->getType())) + return false; Can this ever trigger and the below expression

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

urnathan added inline comments. Comment at: clang/include/clang/Sema/Sema.h:2989 /// \param ImportLoc The location of the 'import' keyword. - /// \param Path The module access path. + /// \param NamePath The module toplevel name as an access path. + /// \param Partition

[PATCH] D119550: [clang] Itanium mangler constructors

This revision was automatically updated to reflect the committed changes. Closed by commit rG082f328899be: [clang] Itanium mangler constructors (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D114714: [C++20][Modules][2/8] Add enumerations for partition modules and stream them.

urnathan accepted this revision. urnathan 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/D114714/new/ https://reviews.llvm.org/D114714

[PATCH] D118893: [C++20][Modules][1/8] Track valid import state.

urnathan accepted this revision. urnathan added a comment. This revision is now accepted and ready to land. thanks for indexing the subject lines! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118893/new/ https://reviews.llvm.org/D118893

[PATCH] D114714: [C++20][Modules] Add enumerations for partition modules and stream them.

urnathan added inline comments. Comment at: clang/include/clang/Basic/Module.h:109 -/// This is a C++ Modules TS module interface unit. +/// This is a C++ Modules TS or C++20 module interface unit. ModuleInterfaceUnit, iains wrote: > urnathan

[PATCH] D118893: [C++20][Modules] Track valid import state.

urnathan added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1543 +def err_import_not_allowed_here : Error< + "imports must be contiguous and immediately follow the module declaration">; +def err_import_in_global_fragment : Error<

[PATCH] D114714: [C++20][Modules] Add enumerations for partition modules and stream them.

urnathan added inline comments. Comment at: clang/include/clang/Basic/Module.h:109 -/// This is a C++ Modules TS module interface unit. +/// This is a C++ Modules TS or C++20 module interface unit. ModuleInterfaceUnit, I think it's confusing to

[PATCH] D118893: [C++20][Modules] Track valid import state.

urnathan added a comment. Looks right to me -- I also implemented such a state machine in GCC's parser. There are a bunch of formatting issues to fix of course. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1543 +def err_import_not_allowed_here : Error< +

[PATCH] D119823: [Modules] Add module structure output to -module-file-info.

urnathan added inline comments. Comment at: clang/lib/Frontend/FrontendActions.cpp:817-824 +#if 0 + case Module::ModuleHeaderUnit: +return "Header Unit"; + case Module::ModulePartitionInterface: +return "Partition Interface"; + case

[PATCH] D119748: [clang][NFC] Remove IgnoreLinkageSpecDecls

This revision was automatically updated to reflect the committed changes. Closed by commit rG02093906fa0f: [clang][NFC] Remove IgnoreLinkageSpecDecls (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

urnathan added inline comments. Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global -// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global +// CHECK-DAG:

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

urnathan added inline comments. Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global -// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global +// CHECK-DAG:

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

urnathan added inline comments. Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global -// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global +// CHECK-DAG:

[PATCH] D119409: [C++20] [Modules] Remain variable's definition in module interface

urnathan added inline comments. Comment at: clang/test/CodeGenCXX/static-variable-in-module.cpp:2-8 +// RUN: mkdir %t +// RUN: echo "struct S { S(); };" >> %t/foo.h +// RUN: echo "static S s = S();" >> %t/foo.h +// RUN: %clang -std=c++20 -I%t %s -S -emit-llvm -o - | FileCheck %s

[PATCH] D119333: [clang][NFC] Standard substitution checking cleanup

This revision was automatically updated to reflect the committed changes. urnathan marked an inline comment as done. Closed by commit rG815446cd3e16: [clang][NFC] Standard substitution checking cleanup (authored by urnathan). Herald added a project: clang. Herald added a subscriber: cfe-commits.

[PATCH] D118437: [NFC] [Modules] Refactor ODR checking for default template argument in ASTReader

urnathan accepted this revision. urnathan added a comment. This revision is now accepted and ready to land. ok, thanks for considering the suggestion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118437/new/ https://reviews.llvm.org/D118437

[PATCH] D118804: Revert "[Clang] Propagate guaranteed alignment for malloc and others"

urnathan added a comment. In D118804#3304280 , @aaron.ballman wrote: > In D118804#3304261 , @urnathan > wrote: > >> While C2X has blessed such smaller alignments, the x86_64 ABI (in >> particular), has not.

  1   2   >