Skip to content
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

Improve LLVM bug 9069 workaround #804

Closed
jethrogb opened this issue Jul 10, 2017 · 4 comments
Closed

Improve LLVM bug 9069 workaround #804

jethrogb opened this issue Jul 10, 2017 · 4 comments

Comments

@jethrogb
Copy link
Contributor

jethrogb commented Jul 10, 2017

Due to LLVM bug 9069, an extra token is included when tokenizing ranges. This is used to parse macros. This bug is fixed in LLVM 4.0.

Our current workaround just tries parsing the macro twice, removing the last token if it failed the first time. This is not very robust and can result in unwanted behavior:

  • In LLVM 3.9 and before, if the extra token happens to make sense syntactically, it is erroneously included in the macro evaluation
  • In LLVM 4.0 and after, if the original macro definition had a syntax error, the macro will now get reevaluated with the last token removed, which might unintentionally parse correctly.

In the cexpr tests, instead we check which behavior we get at runtime by checking the result of parsing a know macro: jethrogb/rust-cexpr@0ea1367 . @emilio suggested the same could be implemented here with a CXUnsavedFile.

@emilio
Copy link
Contributor

emilio commented Jul 12, 2017

Actually, wouldn't detecting that and always removing the last token fail if the macro declaration is the last thing in the file?

I guess improving this only allows us to avoid trying without the token if we detect the bug is not present.

@jethrogb
Copy link
Contributor Author

jethrogb commented Jul 12, 2017

Hmm, yes, you're right. Unconditionally removing the last token if you detect the bug is not sufficient. This is why instead of working around the bug at the token level, in cexpr I look at it at the range level. In the buggy case, the range does extend past the end of the file, but since there is no data there, no extra token is returned. Adjusting the range always fixes the bug. This also matches the bug fix of LLVM itself.

@kulp
Copy link
Member

kulp commented Jun 21, 2020

This bug would be closed by #1807.

@kulp
Copy link
Member

kulp commented Mar 16, 2022

In 99b147d (part of #2155, fulfilling the intent of #1807), the workaround for bug 9069 was removed.

Consequently I am marking this issue closed.

@kulp kulp closed this as completed Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants