aaron.ballman added a comment.
In D124534#3481264 <https://reviews.llvm.org/D124534#3481264>, @ken-matsui
wrote:
> Hi, @aaron.ballman
>
> Thanks for your kind reviews!
> I’ve updated the code, but is this what you’d expected?
It looks like the precommit CI pipeline can't test your patch -- I think you
may have uploaded just the changes since last time rather than the full set of
changes against the tree. I don't think we're quite there yet, but we're not
too far off either, I think. Thank you for continuing to work on this!
================
Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1114-1116
GNUImaginaryConstant, GNUIncludeNext,
GNULabelsAsValue, GNUNullPointerArithmetic,
GNUPointerArith, RedeclaredClassMember,
----------------
You'll likely also need to re-flow the line to the usual 80-col limit.
================
Comment at: clang/lib/Lex/PPDirectives.cpp:1356
+
+ PP.Diag(FlagTok, diag::ext_pp_gnu_line_directive);
} else if (FlagVal == 2) {
----------------
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.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124534/new/
https://reviews.llvm.org/D124534
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
