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

Enhance GCC backward compatibility #1226

Merged
merged 2 commits into from May 7, 2017

Conversation

Projects
None yet
3 participants
@fmdkdd
Member

fmdkdd commented Mar 1, 2017

Closes #1224.

@Apakollaps, can you check this branch works for you? And also report your GCC version?

@Apakollaps

This comment has been minimized.

Apakollaps commented Mar 2, 2017

@fmdkdd Looks like it works fine. Our gcc version is 4.4.7 😞.

@fmdkdd

This comment has been minimized.

Member

fmdkdd commented Mar 2, 2017

@Apakollaps Good to know.

@cpitclaudel is this fine? The flags we drop enhance backward-compatibility without affecting parsing; there is just more output that isn't matched to any :error-pattern. While I was at it, I captured warning groups as a flycheck-error-id.

@cpitclaudel

Sorry for the delay. Looks great to me.

flycheck.el Outdated
@@ -6381,7 +6378,8 @@ Requires GCC 4.8 or newer. See URL `https://gcc.gnu.org/'."
(info line-start (or "<stdin>" (file-name)) ":" line ":" column
": note: " (message) line-end)
(warning line-start (or "<stdin>" (file-name)) ":" line ":" column
": warning: " (message) line-end)
": warning: " (message (one-or-more (not (any "\n["))))
(optional "[" (id (one-or-more (any "A-Z-"))) "]") line-end)

This comment has been minimized.

@cpitclaudel

cpitclaudel May 5, 2017

Member

Are we already parsing case-insensitively? (I guess so, since it worked^^)

This comment has been minimized.

@fmdkdd

fmdkdd May 6, 2017

Member

Ha! That did not even phase me. But on a second reading, I'm puzzled.

I've looked into it, and we don't seem to pass anything related to case-sensitivity in Flycheck. So I guess case-sensitivity is controlled by case-fold-search.

To be safe, I've changed the regexp to what other checkers do: (one-or-more not-newline).

@fmdkdd fmdkdd force-pushed the 1224-enhance-gcc-backward-compatibility branch 2 times, most recently from ac5560c to 8dce838 May 6, 2017

fmdkdd added some commits Mar 1, 2017

Remove unnecessary GCC flag for better backward-compatibility
We can leave the source location lines in the output, since our error-patterns
will just ignore these lines.

@fmdkdd fmdkdd force-pushed the 1224-enhance-gcc-backward-compatibility branch from 8dce838 to 9e75644 May 6, 2017

@fmdkdd

This comment has been minimized.

Member

fmdkdd commented May 6, 2017

@cpitclaudel I've changed the regexp and updated the changelog. Should be good to merge.

@cpitclaudel

This comment has been minimized.

Member

cpitclaudel commented May 6, 2017

Thanks. Shouldn't it be a non-greedy match though? Otherwise I won't a ] in the message confuse us?

@fmdkdd

This comment has been minimized.

Member

fmdkdd commented May 6, 2017

Hmm I don't think so. A ] in the message would just be part of the message. The message is everything up to a newline or [, then, if there was a [, that's the ID.

(let ((str ": warning: foo ] message [-Wcpp]"))
  (string-match (rx ": warning: " (group-n 1 (one-or-more (not (any "\n["))))
                    (optional "[" (group-n 2 (one-or-more not-newline)) "]"))
                str)
  (list 
   (match-string 0 str)
   (match-string 1 str)
   (match-string 2 str)))

returns:

(": warning: foo ] message [-Wcpp]" "foo ] message " "-Wcpp")
@cpitclaudel

This comment has been minimized.

Member

cpitclaudel commented May 6, 2017

Ouch, sorry :/ For some reason I thought the warning id came first, not last. Please go ahead and merge :)

@fmdkdd

This comment has been minimized.

Member

fmdkdd commented May 7, 2017

No worries :) The warning id comes first in most checkers.

@fmdkdd fmdkdd merged commit 4cb411b into master May 7, 2017

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@fmdkdd fmdkdd deleted the 1224-enhance-gcc-backward-compatibility branch May 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment