hazohelet added inline comments.
================
Comment at: clang/lib/AST/ExprConstant.cpp:2422
+ << BS.getType();
+ Info.Note(BS.getBeginLoc(),
diag::note_constexpr_base_inherited_here);
+ return false;
----------------
hazohelet wrote:
> tbaeder wrote:
> > Can you pass `<< BS.getSourceRange()` here? Does that improve things?
> Currently, `DiagLoc` points to the variable declaration and the
> `BS.getSourceRange` covers the line where the base class is inherited. This
> causes distant source range and thus unnecessarily many lines of snippet
> printing.
> e.g.
> ```
> struct Base {
> Base() = delete;
> };
> struct Derived : Base {
> constexpr Derived() {}
> };
> constexpr Derived dd;
> ```
> Output:
> ```
> source.cpp:7:19: error: constexpr variable 'dd' must be initialized by a
> constant expression
> 7 | constexpr Derived dd;
> | ^~
> source.cpp:7:19: note: constructor of base class 'Base' is not called
> 7 | struct Derived : Base {
> | ~~~~
> 8 | constexpr Derived() {}
> 9 | };
> 10 | constexpr Derived dd;
> | ^
> source.cpp:4:18: note: base class inherited here
> 4 | struct Derived : Base {
> | ^
> ```
> (The line numbers seem incorrect but is already reported in
> https://github.com/llvm/llvm-project/issues/63524)
>
> I think we don't need two notes here because the error is already pointing to
> the variable declaration. Having something like the following would be
> succint.
> ```
> source.cpp:7:19: error: constexpr variable 'dd' must be initialized by a
> constant expression
> 7 | constexpr Derived dd;
> | ^~
> source.cpp:4:18: note: constructor of base class 'Base' is not called
> 4 | struct Derived : Base {
> | ^~~~
> ```
> Providing source range would be beneficial because the inherited class often
> spans in a few lines (the code in the crashing report, for example)
Sorry, I was looking at the line above. The distant source range problem
doesn't occur.
I tested another input
```
struct Base {
Base() = delete;
constexpr Base(int){}
};
struct Derived : Base {
constexpr Derived() {}
constexpr Derived(int n): Base(n) {}
};
constexpr Derived darr[3] = {1, Derived(), 3};
```
expecting that the `DiagLoc` points to the second initializer `Derived()`, but
it pointed to the same location as the error, so I'm still in favor of the idea
of having a single note here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153969/new/
https://reviews.llvm.org/D153969
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
