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

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu created this revision.
ChuanqiXu added reviewers: rsmith, urnathan, aaron.ballman, erichkeane.
ChuanqiXu added a project: clang.
ChuanqiXu requested review of this revision.
Herald added a subscriber: cfe-commits.

See https://github.com/cplusplus/draft/pull/5204 for a detailed background.

Simply, the test attached to this patch should be accepted instead of being complained about the redefinition.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118034

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Modules/Inputs/redundant-template-default-arg/foo.cppm
  clang/test/Modules/Inputs/redundant-template-default-arg/foo.h
  clang/test/Modules/redundant-template-default-arg.cpp


Index: clang/test/Modules/redundant-template-default-arg.cpp
===================================================================
--- /dev/null
+++ clang/test/Modules/redundant-template-default-arg.cpp
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: %clang_cc1  -std=c++20 %S/Inputs/redundant-template-default-arg/foo.cppm -I%S/Inputs/redundant-template-default-arg/. -emit-module-interface -o %t/foo.pcm
+// RUN: %clang_cc1  -fprebuilt-module-path=%t -std=c++20 %s -I%S/Inputs/redundant-template-default-arg/. -fsyntax-only -verify
+// expected-no-diagnostics
+import foo;
+#include "foo.h"
Index: clang/test/Modules/Inputs/redundant-template-default-arg/foo.h
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/redundant-template-default-arg/foo.h
@@ -0,0 +1,11 @@
+template <typename T = int>
+T v;
+
+template <int T = 8>
+int v2;
+
+template <typename T>
+class my_array {};
+
+template <template <typename> typename C = my_array>
+int v3;
Index: clang/test/Modules/Inputs/redundant-template-default-arg/foo.cppm
===================================================================
--- /dev/null
+++ clang/test/Modules/Inputs/redundant-template-default-arg/foo.cppm
@@ -0,0 +1,3 @@
+module;
+#include "foo.h"
+export module foo;
Index: clang/lib/Sema/SemaTemplate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2678,6 +2678,23 @@
        NewParam != NewParamEnd; ++NewParam) {
     // Variables used to diagnose redundant default arguments
     bool RedundantDefaultArg = false;
+    // Variable used to not diagnose redundant default arguments
+    // from modules.
+    //
+    // [basic.def.odr]/13:
+    //     There can be more than one definition of a
+    //     ...
+    //     default template argument
+    //     ...
+    //     in a program provided that each definition appears in a different
+    //     translation unit and the definitions satisfy the [same-meaning
+    //     criteria of the ODR].
+    //
+    // Simply, the design of modules allows the definition of template default
+    // argument to be repeated across translation unit. Note that the ASTReader
+    // would check the ODR criteria mentioned above so we could skip it here.
+    bool IsOldParamFromModule = false;
+
     SourceLocation OldDefaultLoc;
     SourceLocation NewDefaultLoc;
 
@@ -2710,6 +2727,8 @@
         OldDefaultLoc = OldTypeParm->getDefaultArgumentLoc();
         NewDefaultLoc = NewTypeParm->getDefaultArgumentLoc();
         SawDefaultArgument = true;
+        if (OldTypeParm->isFromASTFile())
+          IsOldParamFromModule = true;
         RedundantDefaultArg = true;
         PreviousDefaultArgLoc = NewDefaultLoc;
       } else if (OldTypeParm && OldTypeParm->hasDefaultArgument()) {
@@ -2755,6 +2774,8 @@
         OldDefaultLoc = OldNonTypeParm->getDefaultArgumentLoc();
         NewDefaultLoc = NewNonTypeParm->getDefaultArgumentLoc();
         SawDefaultArgument = true;
+        if (OldNonTypeParm->isFromASTFile())
+          IsOldParamFromModule = true;
         RedundantDefaultArg = true;
         PreviousDefaultArgLoc = NewDefaultLoc;
       } else if (OldNonTypeParm && OldNonTypeParm->hasDefaultArgument()) {
@@ -2799,6 +2820,8 @@
         OldDefaultLoc = OldTemplateParm->getDefaultArgument().getLocation();
         NewDefaultLoc = NewTemplateParm->getDefaultArgument().getLocation();
         SawDefaultArgument = true;
+        if (OldTemplateParm->isFromASTFile())
+          IsOldParamFromModule = true;
         RedundantDefaultArg = true;
         PreviousDefaultArgLoc = NewDefaultLoc;
       } else if (OldTemplateParm && OldTemplateParm->hasDefaultArgument()) {
@@ -2826,7 +2849,7 @@
       Invalid = true;
     }
 
-    if (RedundantDefaultArg) {
+    if (RedundantDefaultArg && !IsOldParamFromModule) {
       // C++ [temp.param]p12:
       //   A template-parameter shall not be given default arguments
       //   by two different declarations in the same scope.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D118034.402472.patch
Type: text/x-patch
Size: 4084 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220124/4f11f4e9/attachment.bin>