[PATCH] D129531: [clang][C++20] P0960R3: Allow initializing aggregates from a parenthesized list of values
0x59616e added a comment.
I can only nitpick some of the peripheral issues since I have no knowledge in
most of the part of clang. Perhaps implementing the new standard feature is too
arduous for a tyro like me. It's great to see the real expert to complete this.
Comment at: clang/lib/AST/ExprConstant.cpp:9959-9962
+if (!FD->isUnnamedBitfield()) {
+ Field = FD;
+ break;
+}
nit: Maybe use early exit to reduce indentation for better readability ?
Comment at: clang/lib/AST/StmtPrinter.cpp:2465-2471
+ unsigned i = 0;
+ for (Expr *E : Node->getInitExprs()) {
+if (i)
+ OS << ", ";
+PrintExpr(E);
+i++;
+ }
nit: Is it possible to use `llvm::interleaveComma` [0] here without any bad
effect (e.g. increase in compiling time, since STLExtras.h is not a trivial
header) ?
[0]
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ADT/STLExtras.h#L1902
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129531/new/
https://reviews.llvm.org/D129531
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D129531: [clang][C++20] P0960R3: Allow initializing aggregates from a parenthesized list of values
0x59616e added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:6114-6116 +// const ValueDecl *VD = Entity.getDecl(); +// if (const VarDecl *VarD = dyn_cast_or_null(VD); +// VarD && VarD->hasInit() && !VarD->getInit()->containsErrors()) cor3ntin wrote: > Is that code meant to be commented? This is a legacy from me. I guess it is for debugging purpose IIRC (sorry, it is too long ago to remember clearly). If so, t should be deleted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129531/new/ https://reviews.llvm.org/D129531 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D140695: [M68k] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP macros
0x59616e added a comment. The pre-merge checks seem to have some issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140695/new/ https://reviews.llvm.org/D140695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D140695: [M68k] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP macros
0x59616e added a comment. Also, could you add "Fixes #58974" in the commit message so that the item will be closed automatically upon commiting ? Thtanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140695/new/ https://reviews.llvm.org/D140695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D140695: [M68k] Define __GCC_HAVE_SYNC_COMPARE_AND_SWAP macros
0x59616e accepted this revision. 0x59616e added a comment. The CI seems OK. My first LGTM is given to you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140695/new/ https://reviews.llvm.org/D140695 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D126947: [clang][doc][NFC] Update get_started.html
0x59616e created this revision.
Herald added a project: All.
0x59616e requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
-cc1 is replaced by -Xclang. Update get_started.html
to reflect this change.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D126947
Files:
clang/www/get_started.html
Index: clang/www/get_started.html
===
--- clang/www/get_started.html
+++ clang/www/get_started.html
@@ -279,13 +279,13 @@
Pretty printing from the AST:
-Note, the -cc1 argument indicates the compiler front-end, and
+Note, the -Xclang argument indicates the compiler front-end, and
not the driver, should be run. The compiler front-end has several additional
Clang specific features which are not exposed through the GCC compatible driver
interface.
-$ clang -cc1 ~/t.c -ast-print
+$ clang -Xclang ~/t.c -ast-print
typedef float V __attribute__(( vector_size(16) ));
V foo(V a, V b) {
return a + b * a;
Index: clang/www/get_started.html
===
--- clang/www/get_started.html
+++ clang/www/get_started.html
@@ -279,13 +279,13 @@
Pretty printing from the AST:
-Note, the -cc1 argument indicates the compiler front-end, and
+Note, the -Xclang argument indicates the compiler front-end, and
not the driver, should be run. The compiler front-end has several additional
Clang specific features which are not exposed through the GCC compatible driver
interface.
-$ clang -cc1 ~/t.c -ast-print
+$ clang -Xclang ~/t.c -ast-print
typedef float V __attribute__(( vector_size(16) ));
V foo(V a, V b) {
return a + b * a;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D149867: [Clang][M68k] Add Clang support for the new M68k_RTD CC
0x59616e added a comment. ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149867/new/ https://reviews.llvm.org/D149867 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143529: [M68k] Add support for basic memory constraints in inline asm
0x59616e requested changes to this revision. 0x59616e added a comment. This revision now requires changes to proceed. A few minor adjustments are required. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143529/new/ https://reviews.llvm.org/D143529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143529: [M68k] Add support for basic memory constraints in inline asm
0x59616e accepted this revision. 0x59616e added a comment. This revision is now accepted and ready to land. Shoot. I mixed it up with my own one. I'm sorry. I can't find any isseu in this patch and it LGTM. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143529/new/ https://reviews.llvm.org/D143529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D151062: [clang][Sema] Fix a crash when instantiating a non-type template argument in a dependent scope.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcde139016a4e: [clang][Sema] Fix a crash when instantiating a
non-type template argument in a… (authored by 0x59616e).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Changed prior to commit:
https://reviews.llvm.org/D151062?vs=525157&id=525160#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D151062/new/
https://reviews.llvm.org/D151062
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaTemplate.cpp
clang/test/SemaCXX/PR62533.cpp
clang/test/SemaTemplate/ms-sizeof-missing-typename.cpp
Index: clang/test/SemaTemplate/ms-sizeof-missing-typename.cpp
===
--- clang/test/SemaTemplate/ms-sizeof-missing-typename.cpp
+++ clang/test/SemaTemplate/ms-sizeof-missing-typename.cpp
@@ -50,7 +50,7 @@
}
namespace ambiguous_missing_parens {
-// expected-error@+1 {{'Q::U' instantiated to a class template, not a function template}}
+// expected-error@+1 {{'Q::U' is expected to be a non-type template, but instantiated to a class template}}
template void f() { int a = sizeof T::template U<0> + 4; }
struct Q {
// expected-note@+1 {{class template declared here}}
Index: clang/test/SemaCXX/PR62533.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/PR62533.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template
+struct test {
+ template using fun_diff = char; // expected-note 2{{class template declared here}}
+};
+
+template
+decltype(T::template fun_diff) foo1() {}
+// expected-note@-1 {{candidate template ignored: substitution failure [with T = test, V = int]: 'test::fun_diff' is expected to be a non-type template, but instantiated to a type alias template}}
+
+template
+void foo2() {
+ // expected-error@+1 {{test::fun_diff' is expected to be a non-type template, but instantiated to a type alias template}}
+ int a = test::template fun_diff;
+}
+
+template
+struct has_fun_diff {
+ using type = double;
+};
+
+template
+struct has_fun_diff {
+ // expected-error@+1 {{'test::fun_diff' is expected to be a non-type template, but instantiated to a type alias template}}
+ using type = decltype(T::template fun_diff);
+};
+
+void bar() {
+ foo1, int>(); // expected-error {{no matching function for call to 'foo1'}}
+ foo2(); // expected-note {{in instantiation of function template specialization}}
+ has_fun_diff, int>::type a; // expected-note {{in instantiation of template class}}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -5001,13 +5001,20 @@
return ExprError();
}
- if (ClassTemplateDecl *Temp = R.getAsSingle()) {
-Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_class_template)
- << SS.getScopeRep()
- << NameInfo.getName().getAsString() << SS.getRange();
-Diag(Temp->getLocation(), diag::note_referenced_class_template);
+ auto DiagnoseTypeTemplateDecl = [&](TemplateDecl *Temp,
+ bool isTypeAliasTemplateDecl) {
+Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_type_template)
+<< SS.getScopeRep() << NameInfo.getName().getAsString() << SS.getRange()
+<< isTypeAliasTemplateDecl;
+Diag(Temp->getLocation(), diag::note_referenced_type_template) << 0;
return ExprError();
- }
+ };
+
+ if (ClassTemplateDecl *Temp = R.getAsSingle())
+return DiagnoseTypeTemplateDecl(Temp, false);
+
+ if (TypeAliasTemplateDecl *Temp = R.getAsSingle())
+return DiagnoseTypeTemplateDecl(Temp, true);
return BuildTemplateIdExpr(SS, TemplateKWLoc, R, /*ADL*/ false, TemplateArgs);
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5490,10 +5490,10 @@
def err_template_kw_refers_to_dependent_non_template : Error<
"%0%select{| following the 'template' keyword}1 "
"cannot refer to a dependent template">;
-def err_template_kw_refers_to_class_template : Error<
- "'%0%1' instantiated to a class template, not a function template">;
-def note_referenced_class_template : Note<
- "class template declared here">;
+def err_template_kw_refers_to_type_template : Error<
+ "'%0%1' is expected to be a non-type template, but instantiated to a %select{class|type alias}2 template">;
+def note_referenced_type_template : Note<
+ "%select{class|type alias}0 template declared here">;
def err_template_kw_missing : Error<
"missing 'template' keyword prior to dependent template name '%0%1'">;
def ext_
[PATCH] D149867: [M68k] Add Clang support for the new M68k_RTD CC
0x59616e accepted this revision. 0x59616e added a comment. This revision is now accepted and ready to land. Though creating our own calling convention is better, I think Min's path is correct at this moment given that m68k is still an experimental target. We can reignite this discussion once we're approaching to becoming a offiical target. @myhsu could you open an issue to cite this problem ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149867/new/ https://reviews.llvm.org/D149867 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D149867: [M68k] Add Clang support for the new M68k_RTD CC
0x59616e added inline comments.
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:559
+emitError |= DefaultCC == LangOptions::DCC_StdCall &&
+ !(Arch == llvm::Triple::m68k || Arch == llvm::Triple::x86);
emitError |= (DefaultCC == LangOptions::DCC_VectorCall ||
nit: Is the expanded one better in terms of readability ?
Comment at: clang/test/CodeGen/mrtd.c:9
+// X86: define{{.*}} x86_stdcallcc void @foo(i32 noundef %arg) [[NUW:#[0-9]+]]
+// M68k: define{{.*}} cc104 void @foo(i32 noundef %arg)
void foo(int arg) {
Just curious, why do we have to use such an arcane name instead of a more lucid
one , such as `m68k_rtdcc`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149867/new/
https://reviews.llvm.org/D149867
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D149867: [M68k] Add Clang support for the new M68k_RTD CC
0x59616e added inline comments.
Comment at: clang/test/CodeGen/mrtd.c:9
+// X86: define{{.*}} x86_stdcallcc void @foo(i32 noundef %arg) [[NUW:#[0-9]+]]
+// M68k: define{{.*}} cc104 void @foo(i32 noundef %arg)
void foo(int arg) {
0x59616e wrote:
> Just curious, why do we have to use such an arcane name instead of a more
> lucid one , such as `m68k_rtdcc`.
I guess this involves more work ?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149867/new/
https://reviews.llvm.org/D149867
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D149867: [M68k] Add Clang support for the new M68k_RTD CC
0x59616e added a comment. I didn't see any issue here. Let's wait for the approval from other (more senior) reviewers. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149867/new/ https://reviews.llvm.org/D149867 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D147481: [M68k] Add basic Clang supports for M68881/2
0x59616e added a comment. LGTM. Thanks ! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147481/new/ https://reviews.llvm.org/D147481 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
