Skip to content

[clang]improve diagnosing redefined defaulted constructor with different exception specs #69688

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 25, 2023
Merged

[clang]improve diagnosing redefined defaulted constructor with different exception specs #69688

merged 3 commits into from
Oct 25, 2023

Conversation

HerrCai0907
Copy link
Contributor

It is misleading when diagnosing 'ExplicitlySpecialMethod' is missing exception specification 'noexcept' for

struct ExplicitlySpecialMethod {
  ExplicitlySpecialMethod() = default;
};
ExplicitlySpecialMethod::ExplicitlySpecialMethod() {}

Fixes: #69094

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 20, 2023
@HerrCai0907 HerrCai0907 requested a review from shafik October 20, 2023 07:56
@llvmbot
Copy link
Member

llvmbot commented Oct 20, 2023

@llvm/pr-subscribers-clang

Author: Congcong Cai (HerrCai0907)

Changes

It is misleading when diagnosing 'ExplicitlySpecialMethod' is missing exception specification 'noexcept' for

struct ExplicitlySpecialMethod {
  ExplicitlySpecialMethod() = default;
};
ExplicitlySpecialMethod::ExplicitlySpecialMethod() {}

Fixes: #69094


Full diff: https://github.com/llvm/llvm-project/pull/69688.diff

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+3)
  • (modified) clang/lib/Sema/SemaDecl.cpp (+13-12)
  • (added) clang/test/SemaCXX/diagnoise-prioritiy-exception-redefining.cpp (+9)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index fc8caf9221b9d29..bf53dfe6c415245 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -325,6 +325,9 @@ Improvements to Clang's diagnostics
       |               ~~~~~~~~~^~~~~~~~
 - Clang now always diagnoses when using non-standard layout types in ``offsetof`` .
   (`#64619: <https://github.com/llvm/llvm-project/issues/64619>`_)
+- Clang nows diagnose redefined defaulted default constructor when redefined
+  defaulted default constructor with different exception specs.
+  (`#69094: <https://github.com/llvm/llvm-project/issues/69094>`_)
 
 Bug Fixes in This Version
 -------------------------
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index b363b0db79f164d..c4979b51e68f5e2 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -3922,18 +3922,6 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
   }
 
   if (getLangOpts().CPlusPlus) {
-    // C++1z [over.load]p2
-    //   Certain function declarations cannot be overloaded:
-    //     -- Function declarations that differ only in the return type,
-    //        the exception specification, or both cannot be overloaded.
-
-    // Check the exception specifications match. This may recompute the type of
-    // both Old and New if it resolved exception specifications, so grab the
-    // types again after this. Because this updates the type, we do this before
-    // any of the other checks below, which may update the "de facto" NewQType
-    // but do not necessarily update the type of New.
-    if (CheckEquivalentExceptionSpec(Old, New))
-      return true;
     OldQType = Context.getCanonicalType(Old->getType());
     NewQType = Context.getCanonicalType(New->getType());
 
@@ -4055,6 +4043,19 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl *&OldD, Scope *S,
       }
     }
 
+    // C++1z [over.load]p2
+    //   Certain function declarations cannot be overloaded:
+    //     -- Function declarations that differ only in the return type,
+    //        the exception specification, or both cannot be overloaded.
+
+    // Check the exception specifications match. This may recompute the type of
+    // both Old and New if it resolved exception specifications, so grab the
+    // types again after this. Because this updates the type, we do this before
+    // any of the other checks below, which may update the "de facto" NewQType
+    // but do not necessarily update the type of New.
+    if (CheckEquivalentExceptionSpec(Old, New))
+      return true;
+
     // C++11 [dcl.attr.noreturn]p1:
     //   The first declaration of a function shall specify the noreturn
     //   attribute if any declaration of that function specifies the noreturn
diff --git a/clang/test/SemaCXX/diagnoise-prioritiy-exception-redefining.cpp b/clang/test/SemaCXX/diagnoise-prioritiy-exception-redefining.cpp
new file mode 100644
index 000000000000000..ad025bf041ce519
--- /dev/null
+++ b/clang/test/SemaCXX/diagnoise-prioritiy-exception-redefining.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fcxx-exceptions -std=c++11 %s
+
+struct ExplicitlySpecialMethod {
+  ExplicitlySpecialMethod() = default;
+};
+ExplicitlySpecialMethod::ExplicitlySpecialMethod() {} // expected-error{{definition of explicitly defaulted default constructor}}
+
+struct ImplicitlySpecialMethod {};
+ImplicitlySpecialMethod::ImplicitlySpecialMethod() {} // expected-error{{definition of implicitly declared default constructor}}

…ent exception specs

It is misleading when diagnosing 'ExplicitlySpecialMethod' is missing exception specification 'noexcept' for
```c++
struct ExplicitlySpecialMethod {
  ExplicitlySpecialMethod() = default;
};
ExplicitlySpecialMethod::ExplicitlySpecialMethod() {}
```
Fixes: #69094
@HerrCai0907 HerrCai0907 requested a review from tbaederr October 24, 2023 10:09
@HerrCai0907 HerrCai0907 merged commit 53705dd into llvm:main Oct 25, 2023
@HerrCai0907 HerrCai0907 deleted the better-diagnose branch October 25, 2023 14:22
qtprojectorg pushed a commit to qt/qttools that referenced this pull request Jun 12, 2024
Due to an upstream change [0], QDoc no longer generates the `noexcept`
specifier for compiler generated member functions when QDoc is linked
against LLVM 18. This change modifies the test projects for the output
validation test such that documentation isn't generated for such
functions in tests, thus ensuring that QDoc generates identical output
for the test projects when linked against both LLVM 17 and 18.

[0] llvm/llvm-project#69688

Task-number: QTBUG-123130
Change-Id: I0d1fd48be08ec4beb11e1d74ae574ddb2c914a4a
Reviewed-by: Topi Reiniö <[email protected]>
qtprojectorg pushed a commit to qt/qttools that referenced this pull request Jul 29, 2024
Due to an upstream change [0], QDoc no longer generates the `noexcept`
specifier for compiler generated member functions when QDoc is linked
against LLVM 18. This change modifies the test projects for the output
validation test such that documentation isn't generated for such
functions in tests, thus ensuring that QDoc generates identical output
for the test projects when linked against both LLVM 17 and 18.

[0] llvm/llvm-project#69688

Task-number: QTBUG-123130
Change-Id: I0d1fd48be08ec4beb11e1d74ae574ddb2c914a4a
Reviewed-by: Topi Reiniö <[email protected]>
(cherry picked from commit 9f2cbfc)
qtprojectorg pushed a commit to qt/qttools that referenced this pull request Jul 29, 2024
Due to an upstream change [0], QDoc no longer generates the `noexcept`
specifier for compiler generated member functions when QDoc is linked
against LLVM 18. The issue has been addressed previously for the
`tst_validateQdocOutputFiles` test [1], by modifying the impacted test
data such that documentation isn't generated for such functions in
tests, thus ensuring that QDoc generates identical output for the test
projects when linked against both LLVM 17 and 18.

Due to an oversight, the duplicate test data in `tst_generatedOutput`
was not modified accordingly. This change addresses that mistake.

[0] llvm/llvm-project#69688
[1] 9f2cbfc (qttools.git)

Task-number: QTBUG-123130
Change-Id: I72da94b49144649d787158894ae1bf09589fc7f3
Reviewed-by: Luca Di Sera <[email protected]>
qtprojectorg pushed a commit to qt/qttools that referenced this pull request Aug 1, 2024
Due to an upstream change [0], QDoc no longer generates the `noexcept`
specifier for compiler generated member functions when QDoc is linked
against LLVM 18. The issue has been addressed previously for the
`tst_validateQdocOutputFiles` test [1], by modifying the impacted test
data such that documentation isn't generated for such functions in
tests, thus ensuring that QDoc generates identical output for the test
projects when linked against both LLVM 17 and 18.

Due to an oversight, the duplicate test data in `tst_generatedOutput`
was not modified accordingly. This change addresses that mistake.

[0] llvm/llvm-project#69688
[1] 9f2cbfc (qttools.git)

Task-number: QTBUG-123130
Change-Id: I72da94b49144649d787158894ae1bf09589fc7f3
Reviewed-by: Luca Di Sera <[email protected]>
(cherry picked from commit 274a589)
Reviewed-by: Topi Reiniö <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clang: misleading error message when redefining the body of a constructor ctor() = default;
4 participants