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

Correct reporting of line numbers #103

Closed
yanniss opened this issue Apr 28, 2016 · 19 comments
Closed

Correct reporting of line numbers #103

yanniss opened this issue Apr 28, 2016 · 19 comments
Labels
bug - identified Bugs with an identified cause
Milestone

Comments

@yanniss
Copy link
Collaborator

yanniss commented Apr 28, 2016

After the latest additions, I get errors and warnings of the form:
Variable ?lastFld only occurs once in file analysis.dl at line 2966
on a file of only 2000 lines. I'm not sure if this is due to wave (suspect not, since I don't use preprocessor directives in that file, but it could be, since I've run into other wave issues without using preprocessor directives) or if it's an issue with the support of multiple heads of rules.

@yanniss yanniss added the bug - identified Bugs with an identified cause label Apr 28, 2016
@b-scholz
Copy link
Member

Please could you remove the wave executable and make a link to the standard GNU cpp using

 ln -sf /usr/bin/cpp wave

Does the problem still persist?

@yanniss
Copy link
Collaborator Author

yanniss commented Apr 29, 2016

No, the problem goes away with cpp. So, it's definitely wave.

@yanniss
Copy link
Collaborator Author

yanniss commented Apr 29, 2016

BTW:
$ wave -v
2.3.2.5250 linux/GNU C++ version 5.3.1 20151207 (Red Hat 5.3.1-2)

@yanniss yanniss mentioned this issue Apr 29, 2016
@b-scholz
Copy link
Member

When you use the wave executable of your Red Hat distribution, say if you link it with

ln -sf /usr/bin/wave wave

assuming /usr/bin/wave is the place where Red Hat stores wave, do you still have the same troubles? (NB: I would like to know whether just our build of wave is buggy or wave itself).

@yanniss
Copy link
Collaborator Author

yanniss commented Apr 30, 2016

There is no wave in the system, other than the one installed by souffle, nor do I see an easy way to install it. If you used a relatively recent version, I cannot imagine the one I'll get in binary being more up-to-date.

@b-scholz
Copy link
Member

b-scholz commented May 1, 2016

I have a question related to the line number problem. When you run the compiled version of wave on your source code, do you see #line directives in the generated code, i.e., run

wave <source-code> | grep '#line'

@yanniss
Copy link
Collaborator Author

yanniss commented May 1, 2016

No, there is only a #line 1 and then no more #line directives after that. It may be related to having lots of comment text in the file, since we've seen such wave bugs (see minimal example in #66, also https://github.com/souffle-lang/souffle/files/241062/temp.txt).
(I'm slowly converting between syntaxes by gradually uncommenting code in LogicBlox syntax.)

@b-scholz
Copy link
Member

b-scholz commented May 2, 2016

Unfortunately, I observed the same behaviour on a 32bit Ubuntu system. There is somewhere a macro definition that disables the context that is set by the wave driver to produce line number directives (and preserves white spaces). I know already the location in the header files. I would like to know the following about your system:

  • Are you on a 32bit or 64bit system?
  • Are you using clang or gcc?

(NB: the first line directive is printed by the wave driver - not the pre-processor code of boost).

It could be related to comments but I believe this is unlikely because I have seen this behaviour on our 32bit box, i.e.,
http://plang1.it.usyd.edu.au/jenkins

@yanniss
Copy link
Collaborator Author

yanniss commented May 2, 2016

I have a 64-bit system and use gcc.
Thanks!

@b-scholz
Copy link
Member

b-scholz commented May 4, 2016

I believe I identified the problem of this behaviour. On some versions of Linux the processing hook

template <typename ContextT, typename ContainerT>
    bool
    emit_line_directive(ContextT const& ctx, ContainerT &pending,
        typename ContextT::token_type const& act_token)

in the file wave_trace_macro_expansion.hpp is not invoked and hence the #line directive is not issued. I still need to understand why this hook is not triggered in some boost installations.

@b-scholz
Copy link
Member

b-scholz commented May 7, 2016

I believe I found out what is going on. We use an old wave driver for the wave library (boost V1.46) and most installations use boost >=V1.54. The wave library changed its interface after V1.46 and hence the hook for emitting line numbers and white spaces is not picked up.
Work around: install a new driver and modify it so that -p and -l flags are set with appropriate default values for Souffle. Let's give it a go.

@b-scholz b-scholz added this to the 1.0 milestone May 7, 2016
b-scholz added a commit to b-scholz/souffle that referenced this issue May 7, 2016
b-scholz added a commit that referenced this issue May 7, 2016
Fix of bug issue #103: updated wave driver
b-scholz added a commit that referenced this issue May 16, 2016
Fixed CPP issues for FreeBSD. Related to issue #103
@b-scholz
Copy link
Member

Yannis - is this issue resolved with the latest fixes?

@yanniss
Copy link
Collaborator Author

yanniss commented May 16, 2016

Put on my todo list to check thoroughly. May take me a couple of days, since I'm currently buried under. Thx!

@yanniss
Copy link
Collaborator Author

yanniss commented May 19, 2016

I'm afraid I'm still seeing the issue. Did "git pull; make distclean; sh bootstrap; ./configure; make -j; sudo make install". I'm definitely running the latest (checked git log, checked disk files). I get messages like:

Warning: Output relations with record types cannot be output precisely. Attribute ?ap has record type AccessPath in file analysis.dl at line 2691

on a file that only has ~2000 lines. With cpp (instead of souffle-wave) the warning is:
Warning: Output relations with record types cannot be output precisely. Attribute ?ap has record type AccessPath in file analysis.dl at line 1755

There are no #line directives output.

@yanniss yanniss reopened this May 19, 2016
@yanniss
Copy link
Collaborator Author

yanniss commented May 19, 2016

Here is a shorter file that demonstrates the issue (for me). You'll see that the warning reports are on the wrong lines. It may have to do with the large portions of code that are commented out.

temp-analysis2.dl.txt

(Sorry for the obfuscation--I'm too self-conscious to share this code while I'm still processing it.)

@b-scholz
Copy link
Member

I can reproduce this problem now. Thanks for the test-case. We will look into it.

@b-scholz
Copy link
Member

Unfortunately, there were two bugs: (1) we counted line numbers in comments twice and (2) the load operation in the interpreter did not terminate after failing to open a file. (NB: before wave-cpp had issue depending on the word width of an architecture, i.e., 32bit vs. 64bit, the output was different).

Yannis - can we add your temp-analysis2.dl to the testsuite as a testcase for checking line number correctness for warnings/errors?

@yanniss
Copy link
Collaborator Author

yanniss commented May 20, 2016

Sure, no problem. It's far from a minimal test case, though.

@b-scholz
Copy link
Member

Thanks!

b-scholz added a commit to b-scholz/souffle that referenced this issue May 22, 2016
b-scholz added a commit that referenced this issue May 22, 2016
Added testcases for comments. Issue #103
b-scholz added a commit to b-scholz/souffle that referenced this issue May 22, 2016
b-scholz added a commit that referenced this issue May 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug - identified Bugs with an identified cause
Projects
None yet
Development

No branches or pull requests

2 participants