ken-matsui added inline comments.
================
Comment at: clang/lib/Lex/PPDirectives.cpp:1356
+
+ PP.Diag(FlagTok, diag::ext_pp_gnu_line_directive);
} else if (FlagVal == 2) {
----------------
aaron.ballman wrote:
> ken-matsui wrote:
> > aaron.ballman wrote:
> > > I speculate that this change is wrong.
> > >
> > > The goal here is to diagnose any time there's a GNU line marker and now
> > > we're only trigging the diagnostic when the line marker's value is 1;
> > > that misses diagnostics when the marker value is something else.
> > >
> > > That's why I suggested warning each place we return `false` from this
> > > function -- those are the situations when the line marker is
> > > syntactically correct and we're going to make use of it in the caller.
> > > (We don't want to warn about use of a line marker when we're going to
> > > generate an error anyway.)
> > @aaron.ballman
> >
> > Thank you!
> >
> > Just to confirm, do I need to remove the call of `Diag` after
> > `GetLineValue` and put `Diag`s into all branches of returning `false` in
> > this function?
> > If so, I think putting `Diag` after the call of this function would be
> > better.
> > If so, I think putting Diag after the call of this function would be better.
>
> You are correct and I agree, good suggestion!
@aaron.ballman
Thank you for your response!
I've updated the code as mentioned, but a bunch of other tests with the
`-pedantic` option failed as the following warnings:
```
******************** TEST 'Clang :: CXX/expr/expr.const/p2-0x.cpp' FAILED
********************
Script:
--
: 'RUN: at line 1'; /tmp/llvm/llvm-project/build/bin/clang -cc1
-internal-isystem /tmp/llvm/llvm-project/build/lib/clang/15.0.0/include
-nostdsysteminc -fsyntax-only -std=c++11 -pedantic -verify=expected,cxx11
-fcxx-exceptions
/tmp/llvm/llvm-project/clang/test/CXX/expr/expr.const/p2-0x.cpp
-fconstexpr-depth 128 -triple i686-pc-linux-gnu
: 'RUN: at line 2'; /tmp/llvm/llvm-project/build/bin/clang -cc1
-internal-isystem /tmp/llvm/llvm-project/build/lib/clang/15.0.0/include
-nostdsysteminc -fsyntax-only -std=c++2a -pedantic -verify=expected,cxx20
-fcxx-exceptions
/tmp/llvm/llvm-project/clang/test/CXX/expr/expr.const/p2-0x.cpp
-fconstexpr-depth 128 -triple i686-pc-linux-gnu
--
Exit Code: 1
Command Output (stderr):
--
error: 'warning' diagnostics seen but not expected:
Line 0: this style of line directive is a GNU extension
Line 0: this style of line directive is a GNU extension
2 errors generated.
...
```
I personally think it would be preferable if the only change of tests would be
`line-directive.c`.
So, how about reducing `Diag` calls until the warning doesn't spill over into
other tests?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124534/new/
https://reviews.llvm.org/D124534
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
